-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OTLP exporters should not percent decode the key when parsing HEADERS env vars #5623
Labels
bug
Something isn't working
contribfest
Issue good for KubeCon contribfest
good first issue
Good for newcomers
help wanted
Extra attention is needed
pkg:exporter:otlp
Related to the OTLP exporter package
Comments
pellared
added
bug
Something isn't working
pkg:exporter:otlp
Related to the OTLP exporter package
labels
Jul 16, 2024
pellared
changed the title
OTLP exporters should not percent decode the key when parsing header env vars
OTLP exporters should not percent decode the key when parsing HEADERS env vars
Jul 16, 2024
pellared
added
help wanted
Extra attention is needed
good first issue
Good for newcomers
labels
Jul 31, 2024
I will open a PR for it... |
This was referenced Aug 11, 2024
Closed
the OTLPlog should be updated as well, I have raised an issue related #5722, and will implement a fix soon |
MrAlias
added a commit
that referenced
this issue
Aug 21, 2024
… HEADERS env vars (#5705) Bugfix #5623 As stated in the issue, we need to avoid parsing the key and instead implement a validation check for it. I've added some unit tests to verify this fix. However, I noticed a comment at the top of this file: ``` // Code created by gotmpl. DO NOT MODIFY. // source: internal/shared/otlp/envconfig/envconfig.go.tmpl ``` It seems that `internal/shared/otlp/envconfig/envconfig.go.tmpl` is the source template for this file. Since this template matches `exporters/otlp/otlptrace/otlptracegrpc/internal/envconfig/envconfig.go`, I updated the template to maintain consistency. I’m not entirely sure if this approach is correct, so please confirm if this is the right course of action. --------- Co-authored-by: Fools <[email protected]> Co-authored-by: Damien Mathieu <[email protected]> Co-authored-by: Robert Pająk <[email protected]> Co-authored-by: Chester Cheung <[email protected]> Co-authored-by: Tyler Yahn <[email protected]>
@zhihali, this is for all OTLP exporters. I see that OTLP trace and metric exporters are already fixed. Therefore, only OTLP log exporters need bugfixes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bug
Something isn't working
contribfest
Issue good for KubeCon contribfest
good first issue
Good for newcomers
help wanted
Extra attention is needed
pkg:exporter:otlp
Related to the OTLP exporter package
The OTLP exporters SHOULD NOT percent decode the header keys when parsing
OTEL_EXPORTER_OTLP_HEADERS
,OTEL_EXPORTER_OTLP_TRACES_HEADERS
,OTEL_EXPORTER_OTLP_METRICS_HEADERS
,OTEL_EXPORTER_OTLP_LOGS_HEADERS
env vars. Moreover, only valid header keys should be parsed.Current behavior:
opentelemetry-go/exporters/otlp/otlptrace/otlptracegrpc/internal/envconfig/envconfig.go
Lines 166 to 170 in 29bdfd2
The specification says https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#specifying-headers-via-environment-variables:
From W3C Baggage spec https://www.w3.org/TR/baggage/#key:
There is nothing about percent encoding. Only the values are supposed to be percent decoded.
Notice that the implementation should also make sure that only valid key characters are used.
The text was updated successfully, but these errors were encountered: