You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "flea1lt (via GitHub)" <gi...@apache.org> on 2024/05/11 10:02:41 UTC
[PR] Fix/opentelemetry [apisix]
flea1lt opened a new pull request, #11247:
URL: https://github.com/apache/apisix/pull/11247
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
Fixes #11242
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: correct the default collector config apisix actually used for opentelemetry plugin [apisix]
Posted by "flea1lt (via GitHub)" <gi...@apache.org>.
flea1lt commented on PR #11247:
URL: https://github.com/apache/apisix/pull/11247#issuecomment-2132734933
@kayx23 @shreemaan-abhishek Any suggestions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: correct the default collector config apisix actually used for opentelemetry plugin [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1615755685
##########
docs/en/latest/plugins/opentelemetry.md:
##########
@@ -53,18 +53,26 @@ You can set up the collector by configuring it in you configuration file (`conf/
| Name | Type | Default | Description |
|--------------------------------------------|---------|---------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| trace_id_source | enum | random | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that is matches the regex pattern `[0-9a-f]{32}`. |
+| trace_id_source | enum | x-request-id | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that it matches the regex pattern `[0-9a-f]{32}`. |
| resource | object | | Additional [resource](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md) appended to the trace. |
| collector | object | {address = "127.0.0.1:4318", request_timeout = 3} | OpenTelemetry Collector configuration. |
| collector.address | string | 127.0.0.1:4318 | Collector address. If the collector serves on https, use https://127.0.0.1:4318 as the address. |
| collector.request_timeout | integer | 3 | Report request timeout in seconds. |
| collector.request_headers | object | | Report request HTTP headers. |
| batch_span_processor | object | | Trace span processor. |
-| batch_span_processor.drop_on_queue_full | boolean | true | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
-| batch_span_processor.max_queue_size | integer | 2048 | Maximum queue size for buffering spans for delayed processing. |
-| batch_span_processor.batch_timeout | number | 5 | Maximum time in seconds for constructing a batch. |
-| batch_span_processor.max_export_batch_size | integer | 256 | Maximum number of spans to process in a single batch. |
-| batch_span_processor.inactive_timeout | number | 2 | Time interval in seconds between processing batches. |
+| batch_span_processor.drop_on_queue_full | boolean | false | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
+| batch_span_processor.max_queue_size | integer | 1024 | Maximum queue size for buffering spans for delayed processing. |
+| batch_span_processor.batch_timeout | number | 2 | Maximum time in seconds for constructing a batch. |
+| batch_span_processor.max_export_batch_size | integer | 16 | Maximum number of spans to process in a single batch. |
+| batch_span_processor.inactive_timeout | number | 1 | Time interval in seconds between processing batches. |
+
+:::note
+
+When the `trace_id_source` is set to `x-request-id` and requests carry an `x-request-id` header generated by Envoy, there may be issues pushing traces to the collector.
Review Comment:
`there may be issues` --> wondering if we should be more specific about the issues? e.g. what specific error?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: correct the default collector config apisix actually used for opentelemetry plugin [apisix]
Posted by "flea1lt (via GitHub)" <gi...@apache.org>.
flea1lt commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1616502108
##########
docs/en/latest/plugins/opentelemetry.md:
##########
@@ -53,18 +53,26 @@ You can set up the collector by configuring it in you configuration file (`conf/
| Name | Type | Default | Description |
|--------------------------------------------|---------|---------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| trace_id_source | enum | random | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that is matches the regex pattern `[0-9a-f]{32}`. |
+| trace_id_source | enum | x-request-id | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that it matches the regex pattern `[0-9a-f]{32}`. |
| resource | object | | Additional [resource](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md) appended to the trace. |
| collector | object | {address = "127.0.0.1:4318", request_timeout = 3} | OpenTelemetry Collector configuration. |
| collector.address | string | 127.0.0.1:4318 | Collector address. If the collector serves on https, use https://127.0.0.1:4318 as the address. |
| collector.request_timeout | integer | 3 | Report request timeout in seconds. |
| collector.request_headers | object | | Report request HTTP headers. |
| batch_span_processor | object | | Trace span processor. |
-| batch_span_processor.drop_on_queue_full | boolean | true | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
-| batch_span_processor.max_queue_size | integer | 2048 | Maximum queue size for buffering spans for delayed processing. |
-| batch_span_processor.batch_timeout | number | 5 | Maximum time in seconds for constructing a batch. |
-| batch_span_processor.max_export_batch_size | integer | 256 | Maximum number of spans to process in a single batch. |
-| batch_span_processor.inactive_timeout | number | 2 | Time interval in seconds between processing batches. |
+| batch_span_processor.drop_on_queue_full | boolean | false | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
+| batch_span_processor.max_queue_size | integer | 1024 | Maximum queue size for buffering spans for delayed processing. |
+| batch_span_processor.batch_timeout | number | 2 | Maximum time in seconds for constructing a batch. |
+| batch_span_processor.max_export_batch_size | integer | 16 | Maximum number of spans to process in a single batch. |
+| batch_span_processor.inactive_timeout | number | 1 | Time interval in seconds between processing batches. |
+
+:::note
+
+When the `trace_id_source` is set to `x-request-id` and requests carry an `x-request-id` header generated by Envoy, there may be issues pushing traces to the collector.
Review Comment:
In this situation, users will encounter a "bad argument # 1 to '?' (invalid value)" error triggered by `hex2bytes` function.
I will add this error message at this node.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(opentelemetry): correct the default value for the opentelemetry plugin attr and update the docs [apisix]
Posted by "flea1lt (via GitHub)" <gi...@apache.org>.
flea1lt commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1597790338
##########
docs/en/latest/plugins/opentelemetry.md:
##########
@@ -66,6 +66,14 @@ You can set up the collector by configuring it in you configuration file (`conf/
| batch_span_processor.max_export_batch_size | integer | 256 | Maximum number of spans to process in a single batch. |
| batch_span_processor.inactive_timeout | number | 2 | Time interval in seconds between processing batches. |
+:::note
+
+When the `trace_id_source` is set to `x-request-id` and requests carry an `x-request-id` header generated by Envoy, there may be issues pushing traces to the collector.
+
+This occurs because Envoy generates its `x-request-id` header using [UUID](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/observability/tracing#trace-context-propagation) by default, which does not match the specified regex pattern `[0-9a-f]{32}`.
Review Comment:
No. OpenTelemetry requires a 32-hex-character lowercase string as traceId. Users must ensure that the traceId matches the pattern [0-9a-f]{32}. I have linked OpenTelemetry's traceId format at commit https://github.com/apache/apisix/pull/11247/commits/83181fce6dbf8e00e55318a16cece89ca2681dc8.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: correct the default collector config apisix actually used for opentelemetry plugin [apisix]
Posted by "flea1lt (via GitHub)" <gi...@apache.org>.
flea1lt commented on PR #11247:
URL: https://github.com/apache/apisix/pull/11247#issuecomment-2134448537
@kayx23 Please review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(opentelemetry): correct the default value for the opentelemetry plugin attr and update the docs [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1597611146
##########
docs/en/latest/plugins/opentelemetry.md:
##########
@@ -66,6 +66,14 @@ You can set up the collector by configuring it in you configuration file (`conf/
| batch_span_processor.max_export_batch_size | integer | 256 | Maximum number of spans to process in a single batch. |
| batch_span_processor.inactive_timeout | number | 2 | Time interval in seconds between processing batches. |
+:::note
+
+When the `trace_id_source` is set to `x-request-id` and requests carry an `x-request-id` header generated by Envoy, there may be issues pushing traces to the collector.
+
+This occurs because Envoy generates its `x-request-id` header using [UUID](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/observability/tracing#trace-context-propagation) by default, which does not match the specified regex pattern `[0-9a-f]{32}`.
Review Comment:
Q: so should the regex pattern be updated?
##########
docs/en/latest/plugins/opentelemetry.md:
##########
@@ -53,7 +53,7 @@ You can set up the collector by configuring it in you configuration file (`conf/
| Name | Type | Default | Description |
|--------------------------------------------|---------|---------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| trace_id_source | enum | random | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that is matches the regex pattern `[0-9a-f]{32}`. |
+| trace_id_source | enum | x-request-id | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that is matches the regex pattern `[0-9a-f]{32}`. |
Review Comment:
```suggestion
| trace_id_source | enum | x-request-id | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that it matches the regex pattern `[0-9a-f]{32}`. |
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: correct the default collector config apisix actually used for opentelemetry plugin [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1615755150
##########
docs/zh/latest/plugins/opentelemetry.md:
##########
@@ -54,18 +54,26 @@ description: 本文介绍了关于 Apache APISIX `opentelemetry` 插件的基本
| 名称 | 类型 | 默认值 | 描述 |
| ------------------------------------------ | ------- | ------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- |
-| trace_id_source | enum | random | trace ID 的来源。有效值为：`random` 或 `x-request-id`。当设置为 `x-request-id` 时，`x-request-id` 头的值将用作跟踪 ID。请确保当前请求 ID 是符合 TraceID 规范的：`[0-9a-f]{32}`。 |
+| trace_id_source | enum | x-request-id | trace ID 的来源。有效值为：`random` 或 `x-request-id`。当设置为 `x-request-id` 时，`x-request-id` 头的值将用作跟踪 ID。请确保当前请求 ID 是符合 TraceID 规范的：`[0-9a-f]{32}`。 |
| resource | object | | 追加到 trace 的额外 [resource](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md)。 |
| collector | object | {address = "127.0.0.1:4318", request_timeout = 3} | OpenTelemetry Collector 配置。 |
| collector.address | string | 127.0.0.1:4318 | 数据采集服务的地址。如果数据采集服务使用的是 HTTPS 协议，可以将 address 设置为 https://127.0.0.1:4318。 |
| collector.request_timeout | integer | 3 | 数据采集服务上报请求超时时长，单位为秒。 |
| collector.request_headers | object | | 数据采集服务上报请求附加的 HTTP 请求头。 |
| batch_span_processor | object | | trace span 处理器参数配置。 |
-| batch_span_processor.drop_on_queue_full | boolean | true | 如果设置为 `true` 时，则在队列排满时删除 span。否则，强制处理批次。|
-| batch_span_processor.max_queue_size | integer | 2048 | 处理器缓存队列容量的最大值。 |
-| batch_span_processor.batch_timeout | number | 5 | 构造一批 span 超时时间，单位为秒。 |
-| batch_span_processor.max_export_batch_size | integer | 256 | 单个批次中要处理的 span 数量。 |
-| batch_span_processor.inactive_timeout | number | 2 | 两个处理批次之间的时间间隔，单位为秒。 |
+| batch_span_processor.drop_on_queue_full | boolean | false | 如果设置为 `true` 时，则在队列排满时删除 span。否则，强制处理批次。|
+| batch_span_processor.max_queue_size | integer | 1024 | 处理器缓存队列容量的最大值。 |
+| batch_span_processor.batch_timeout | number | 2 | 构造一批 span 超时时间，单位为秒。 |
+| batch_span_processor.max_export_batch_size | integer | 16 | 单个批次中要处理的 span 数量。 |
+| batch_span_processor.inactive_timeout | number | 1 | 两个处理批次之间的时间间隔，单位为秒。 |
+
+:::note
+
+当`trace_id_source`设置为`x-request-id`且请求携带由 Envoy 默认生成的`x-request-id`标头时，将链路数据推送到采集器时可能会出现问题。
Review Comment:
```suggestion
当 `trace_id_source` 设置为 `x-request-id` 且请求携带由 Envoy 默认生成的 `x-request-id` 标头时，将链路数据推送到采集器时可能会出现问题。
```
```suggestion
当`trace_id_source`设置为`x-request-id`且请求携带由 Envoy 默认生成的`x-request-id`标头时，将链路数据推送到采集器时可能会出现问题。
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: correct the default collector config apisix actually used for opentelemetry plugin [apisix]
Posted by "pottekkat (via GitHub)" <gi...@apache.org>.
pottekkat merged PR #11247:
URL: https://github.com/apache/apisix/pull/11247
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: correct the default collector config apisix actually used for opentelemetry plugin [apisix]
Posted by "flea1lt (via GitHub)" <gi...@apache.org>.
flea1lt commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1616502108
##########
docs/en/latest/plugins/opentelemetry.md:
##########
@@ -53,18 +53,26 @@ You can set up the collector by configuring it in you configuration file (`conf/
| Name | Type | Default | Description |
|--------------------------------------------|---------|---------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| trace_id_source | enum | random | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that is matches the regex pattern `[0-9a-f]{32}`. |
+| trace_id_source | enum | x-request-id | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that it matches the regex pattern `[0-9a-f]{32}`. |
| resource | object | | Additional [resource](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md) appended to the trace. |
| collector | object | {address = "127.0.0.1:4318", request_timeout = 3} | OpenTelemetry Collector configuration. |
| collector.address | string | 127.0.0.1:4318 | Collector address. If the collector serves on https, use https://127.0.0.1:4318 as the address. |
| collector.request_timeout | integer | 3 | Report request timeout in seconds. |
| collector.request_headers | object | | Report request HTTP headers. |
| batch_span_processor | object | | Trace span processor. |
-| batch_span_processor.drop_on_queue_full | boolean | true | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
-| batch_span_processor.max_queue_size | integer | 2048 | Maximum queue size for buffering spans for delayed processing. |
-| batch_span_processor.batch_timeout | number | 5 | Maximum time in seconds for constructing a batch. |
-| batch_span_processor.max_export_batch_size | integer | 256 | Maximum number of spans to process in a single batch. |
-| batch_span_processor.inactive_timeout | number | 2 | Time interval in seconds between processing batches. |
+| batch_span_processor.drop_on_queue_full | boolean | false | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
+| batch_span_processor.max_queue_size | integer | 1024 | Maximum queue size for buffering spans for delayed processing. |
+| batch_span_processor.batch_timeout | number | 2 | Maximum time in seconds for constructing a batch. |
+| batch_span_processor.max_export_batch_size | integer | 16 | Maximum number of spans to process in a single batch. |
+| batch_span_processor.inactive_timeout | number | 1 | Time interval in seconds between processing batches. |
+
+:::note
+
+When the `trace_id_source` is set to `x-request-id` and requests carry an `x-request-id` header generated by Envoy, there may be issues pushing traces to the collector.
Review Comment:
In this situation, users will encounter a "bad argument # 1 to '?' (invalid value)" error triggered by `hex2bytes` function.
I will add this error message at this note.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(opentelemetry): correct the default value for the opentelemetry plugin attr and update the docs [apisix]
Posted by "flea1lt (via GitHub)" <gi...@apache.org>.
flea1lt commented on PR #11247:
URL: https://github.com/apache/apisix/pull/11247#issuecomment-2114318349
@kayx23 @shreemaan-abhishek PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(opentelemetry): correct the default value for the opentelemetry plugin attr and update the docs [apisix]
Posted by "flea1lt (via GitHub)" <gi...@apache.org>.
flea1lt commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1599491187
##########
conf/config-default.yaml:
##########
@@ -571,13 +571,13 @@ plugin_attr: # Plugin attributes
request_headers: # Set the headers to include in requests to the OpenTelemetry collector.
Authorization: token # Set the authorization header to include an access token.
batch_span_processor:
- drop_on_queue_full: false # Drop spans when the export queue is full.
- max_queue_size: 1024 # Set the maximum size of the span export queue.
- batch_timeout: 2 # Set the timeout for span batches to wait in the export queue before
+ drop_on_queue_full: true # Drop spans when the export queue is full.
Review Comment:
Because the doc and the values in config-default.yaml are different, we need to align them by updating either the doc or the default config file.
The current apisix doc aligns its values with [opentelemetry-lua](https://github.com/yangxikun/opentelemetry-lua/blob/main/lib/opentelemetry/trace/batch_span_processor.lua#L106), which is the dependency used by the plugin. Therefore, it may be preferable to correct the default configuration file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: correct the default collector config apisix actually used for opentelemetry plugin [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1616546180
##########
docs/en/latest/plugins/opentelemetry.md:
##########
@@ -53,18 +53,26 @@ You can set up the collector by configuring it in you configuration file (`conf/
| Name | Type | Default | Description |
|--------------------------------------------|---------|---------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| trace_id_source | enum | random | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that is matches the regex pattern `[0-9a-f]{32}`. |
+| trace_id_source | enum | x-request-id | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that it matches the regex pattern `[0-9a-f]{32}`. |
| resource | object | | Additional [resource](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md) appended to the trace. |
| collector | object | {address = "127.0.0.1:4318", request_timeout = 3} | OpenTelemetry Collector configuration. |
| collector.address | string | 127.0.0.1:4318 | Collector address. If the collector serves on https, use https://127.0.0.1:4318 as the address. |
| collector.request_timeout | integer | 3 | Report request timeout in seconds. |
| collector.request_headers | object | | Report request HTTP headers. |
| batch_span_processor | object | | Trace span processor. |
-| batch_span_processor.drop_on_queue_full | boolean | true | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
-| batch_span_processor.max_queue_size | integer | 2048 | Maximum queue size for buffering spans for delayed processing. |
-| batch_span_processor.batch_timeout | number | 5 | Maximum time in seconds for constructing a batch. |
-| batch_span_processor.max_export_batch_size | integer | 256 | Maximum number of spans to process in a single batch. |
-| batch_span_processor.inactive_timeout | number | 2 | Time interval in seconds between processing batches. |
+| batch_span_processor.drop_on_queue_full | boolean | false | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
+| batch_span_processor.max_queue_size | integer | 1024 | Maximum queue size for buffering spans for delayed processing. |
+| batch_span_processor.batch_timeout | number | 2 | Maximum time in seconds for constructing a batch. |
+| batch_span_processor.max_export_batch_size | integer | 16 | Maximum number of spans to process in a single batch. |
+| batch_span_processor.inactive_timeout | number | 1 | Time interval in seconds between processing batches. |
+
+:::note
+
+When the `trace_id_source` is set to `x-request-id` and requests carry an `x-request-id` header generated by Envoy, there may be issues pushing traces to the collector.
Review Comment:
Thanks for the explanation. Please go ahead with the update. All else LGTM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(opentelemetry): correct the default value for the opentelemetry plugin attr and update the docs [apisix]
Posted by "flea1lt (via GitHub)" <gi...@apache.org>.
flea1lt commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1602782241
##########
conf/config-default.yaml:
##########
@@ -571,13 +571,13 @@ plugin_attr: # Plugin attributes
request_headers: # Set the headers to include in requests to the OpenTelemetry collector.
Authorization: token # Set the authorization header to include an access token.
batch_span_processor:
- drop_on_queue_full: false # Drop spans when the export queue is full.
- max_queue_size: 1024 # Set the maximum size of the span export queue.
- batch_timeout: 2 # Set the timeout for span batches to wait in the export queue before
+ drop_on_queue_full: true # Drop spans when the export queue is full.
Review Comment:
Well. I will update the doc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: correct the default collector config apisix actually used for opentelemetry plugin [apisix]
Posted by "flea1lt (via GitHub)" <gi...@apache.org>.
flea1lt commented on PR #11247:
URL: https://github.com/apache/apisix/pull/11247#issuecomment-2138615719
@shreemaan-abhishek PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: correct the default collector config apisix actually used for opentelemetry plugin [apisix]
Posted by "flea1lt (via GitHub)" <gi...@apache.org>.
flea1lt commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1616530886
##########
docs/en/latest/plugins/opentelemetry.md:
##########
@@ -53,18 +53,26 @@ You can set up the collector by configuring it in you configuration file (`conf/
| Name | Type | Default | Description |
|--------------------------------------------|---------|---------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| trace_id_source | enum | random | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that is matches the regex pattern `[0-9a-f]{32}`. |
+| trace_id_source | enum | x-request-id | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that it matches the regex pattern `[0-9a-f]{32}`. |
| resource | object | | Additional [resource](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md) appended to the trace. |
| collector | object | {address = "127.0.0.1:4318", request_timeout = 3} | OpenTelemetry Collector configuration. |
| collector.address | string | 127.0.0.1:4318 | Collector address. If the collector serves on https, use https://127.0.0.1:4318 as the address. |
| collector.request_timeout | integer | 3 | Report request timeout in seconds. |
| collector.request_headers | object | | Report request HTTP headers. |
| batch_span_processor | object | | Trace span processor. |
-| batch_span_processor.drop_on_queue_full | boolean | true | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
-| batch_span_processor.max_queue_size | integer | 2048 | Maximum queue size for buffering spans for delayed processing. |
-| batch_span_processor.batch_timeout | number | 5 | Maximum time in seconds for constructing a batch. |
-| batch_span_processor.max_export_batch_size | integer | 256 | Maximum number of spans to process in a single batch. |
-| batch_span_processor.inactive_timeout | number | 2 | Time interval in seconds between processing batches. |
+| batch_span_processor.drop_on_queue_full | boolean | false | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
+| batch_span_processor.max_queue_size | integer | 1024 | Maximum queue size for buffering spans for delayed processing. |
+| batch_span_processor.batch_timeout | number | 2 | Maximum time in seconds for constructing a batch. |
+| batch_span_processor.max_export_batch_size | integer | 16 | Maximum number of spans to process in a single batch. |
+| batch_span_processor.inactive_timeout | number | 1 | Time interval in seconds between processing batches. |
+
+:::note
+
+When the `trace_id_source` is set to `x-request-id` and requests carry an `x-request-id` header generated by Envoy, there may be issues pushing traces to the collector.
Review Comment:
No 500 error occurs here and requests are handled normally.
Users only notice:
1. Error message above in the error log.
2. No span being sent to the collector.
Only a note is because:
1. This is due to a conflict between OpenTelemetry and Envoy's default x-request-id implementation. We could do nothing about this.
2. Only a small number of users will encounter this issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: correct the default collector config apisix actually used for opentelemetry plugin [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1616507743
##########
docs/en/latest/plugins/opentelemetry.md:
##########
@@ -53,18 +53,26 @@ You can set up the collector by configuring it in you configuration file (`conf/
| Name | Type | Default | Description |
|--------------------------------------------|---------|---------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| trace_id_source | enum | random | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that is matches the regex pattern `[0-9a-f]{32}`. |
+| trace_id_source | enum | x-request-id | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that it matches the regex pattern `[0-9a-f]{32}`. |
| resource | object | | Additional [resource](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md) appended to the trace. |
| collector | object | {address = "127.0.0.1:4318", request_timeout = 3} | OpenTelemetry Collector configuration. |
| collector.address | string | 127.0.0.1:4318 | Collector address. If the collector serves on https, use https://127.0.0.1:4318 as the address. |
| collector.request_timeout | integer | 3 | Report request timeout in seconds. |
| collector.request_headers | object | | Report request HTTP headers. |
| batch_span_processor | object | | Trace span processor. |
-| batch_span_processor.drop_on_queue_full | boolean | true | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
-| batch_span_processor.max_queue_size | integer | 2048 | Maximum queue size for buffering spans for delayed processing. |
-| batch_span_processor.batch_timeout | number | 5 | Maximum time in seconds for constructing a batch. |
-| batch_span_processor.max_export_batch_size | integer | 256 | Maximum number of spans to process in a single batch. |
-| batch_span_processor.inactive_timeout | number | 2 | Time interval in seconds between processing batches. |
+| batch_span_processor.drop_on_queue_full | boolean | false | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
+| batch_span_processor.max_queue_size | integer | 1024 | Maximum queue size for buffering spans for delayed processing. |
+| batch_span_processor.batch_timeout | number | 2 | Maximum time in seconds for constructing a batch. |
+| batch_span_processor.max_export_batch_size | integer | 16 | Maximum number of spans to process in a single batch. |
+| batch_span_processor.inactive_timeout | number | 1 | Time interval in seconds between processing batches. |
+
+:::note
+
+When the `trace_id_source` is set to `x-request-id` and requests carry an `x-request-id` header generated by Envoy, there may be issues pushing traces to the collector.
Review Comment:
Does this throw a 500 error?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(opentelemetry): correct the default value for the opentelemetry plugin attr and update the docs [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1602590535
##########
conf/config-default.yaml:
##########
@@ -571,13 +571,13 @@ plugin_attr: # Plugin attributes
request_headers: # Set the headers to include in requests to the OpenTelemetry collector.
Authorization: token # Set the authorization header to include an access token.
batch_span_processor:
- drop_on_queue_full: false # Drop spans when the export queue is full.
- max_queue_size: 1024 # Set the maximum size of the span export queue.
- batch_timeout: 2 # Set the timeout for span batches to wait in the export queue before
+ drop_on_queue_full: true # Drop spans when the export queue is full.
Review Comment:
the default values in apisix config file would have been set considering APISIX's use cases. Updating the doc would be better also because existing users might already be relying on the default values, changing them would lead to another effort being required during version upgrade, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(opentelemetry): correct the default value for the opentelemetry plugin attr and update the docs [apisix]
Posted by "flea1lt (via GitHub)" <gi...@apache.org>.
flea1lt commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1597787620
##########
docs/en/latest/plugins/opentelemetry.md:
##########
@@ -53,7 +53,7 @@ You can set up the collector by configuring it in you configuration file (`conf/
| Name | Type | Default | Description |
|--------------------------------------------|---------|---------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| trace_id_source | enum | random | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that is matches the regex pattern `[0-9a-f]{32}`. |
+| trace_id_source | enum | x-request-id | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that is matches the regex pattern `[0-9a-f]{32}`. |
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(opentelemetry): correct the default value for the opentelemetry plugin attr and update the docs [apisix]
Posted by "flea1lt (via GitHub)" <gi...@apache.org>.
flea1lt commented on PR #11247:
URL: https://github.com/apache/apisix/pull/11247#issuecomment-2106505796
@kayx23 PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(opentelemetry): correct the default value for the opentelemetry plugin attr and update the docs [apisix]
Posted by "shreemaan-abhishek (via GitHub)" <gi...@apache.org>.
shreemaan-abhishek commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1599457296
##########
conf/config-default.yaml:
##########
@@ -571,13 +571,13 @@ plugin_attr: # Plugin attributes
request_headers: # Set the headers to include in requests to the OpenTelemetry collector.
Authorization: token # Set the authorization header to include an access token.
batch_span_processor:
- drop_on_queue_full: false # Drop spans when the export queue is full.
- max_queue_size: 1024 # Set the maximum size of the span export queue.
- batch_timeout: 2 # Set the timeout for span batches to wait in the export queue before
+ drop_on_queue_full: true # Drop spans when the export queue is full.
Review Comment:
I don't understand why these default values need to change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: correct the default collector config apisix actually used for opentelemetry plugin [apisix]
Posted by "kayx23 (via GitHub)" <gi...@apache.org>.
kayx23 commented on code in PR #11247:
URL: https://github.com/apache/apisix/pull/11247#discussion_r1616507743
##########
docs/en/latest/plugins/opentelemetry.md:
##########
@@ -53,18 +53,26 @@ You can set up the collector by configuring it in you configuration file (`conf/
| Name | Type | Default | Description |
|--------------------------------------------|---------|---------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| trace_id_source | enum | random | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that is matches the regex pattern `[0-9a-f]{32}`. |
+| trace_id_source | enum | x-request-id | Source of the trace ID. Valid values are `random` or `x-request-id`. When set to `x-request-id`, the value of the `x-request-id` header will be used as trace ID. Make sure that it matches the regex pattern `[0-9a-f]{32}`. |
| resource | object | | Additional [resource](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md) appended to the trace. |
| collector | object | {address = "127.0.0.1:4318", request_timeout = 3} | OpenTelemetry Collector configuration. |
| collector.address | string | 127.0.0.1:4318 | Collector address. If the collector serves on https, use https://127.0.0.1:4318 as the address. |
| collector.request_timeout | integer | 3 | Report request timeout in seconds. |
| collector.request_headers | object | | Report request HTTP headers. |
| batch_span_processor | object | | Trace span processor. |
-| batch_span_processor.drop_on_queue_full | boolean | true | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
-| batch_span_processor.max_queue_size | integer | 2048 | Maximum queue size for buffering spans for delayed processing. |
-| batch_span_processor.batch_timeout | number | 5 | Maximum time in seconds for constructing a batch. |
-| batch_span_processor.max_export_batch_size | integer | 256 | Maximum number of spans to process in a single batch. |
-| batch_span_processor.inactive_timeout | number | 2 | Time interval in seconds between processing batches. |
+| batch_span_processor.drop_on_queue_full | boolean | false | When set to `true`, drops the span when queue is full. Otherwise, force process batches. |
+| batch_span_processor.max_queue_size | integer | 1024 | Maximum queue size for buffering spans for delayed processing. |
+| batch_span_processor.batch_timeout | number | 2 | Maximum time in seconds for constructing a batch. |
+| batch_span_processor.max_export_batch_size | integer | 16 | Maximum number of spans to process in a single batch. |
+| batch_span_processor.inactive_timeout | number | 1 | Time interval in seconds between processing batches. |
+
+:::note
+
+When the `trace_id_source` is set to `x-request-id` and requests carry an `x-request-id` header generated by Envoy, there may be issues pushing traces to the collector.
Review Comment:
Does this throw a 500 error?
imo, 500 errors need to be handled programmatically whenever possible, rather than leveraging docs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org