-
Notifications
You must be signed in to change notification settings - Fork 452
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
Allow specifying OTLP HTTP headers from env variable #1290
Conversation
Codecov ReportAttention:
... and 18 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a changelog entry.
Also left a comment about improving test coverage for this area to adequately cover all scenarios.
Fixes #1336 As per the [specs](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.25.0/specification/protocol/exporter.md#specifying-headers-via-environment-variables), the custom headers for OTLP exporter can be specified through env variables - `OTEL_EXPORTER_OTLP_HEADERS`, `OTEL_EXPORTER_OTLP_TRACES_HEADERS`, `OTEL_EXPORTER_OTLP_METRICS_HEADERS`. This PR completes the work already done in PR #1290 adding support for tonic metadata To reproduce the same behavior as http exporter, the env-variable takes precedence (as discussed in open-telemetry/opentelemetry-rust-contrib#10) * Move common code for http and tonic exporters in `exporter/mod.rs` (function to parse header from string and test helper to run tests with isolated env variables) I wanted to minimize the changes but maybe it should be a good idea to use a crate like https://crates.io/crates/temp-env for environment related testing
Fixes #
Design discussion issue (if applicable) #
Changes
As per the specs, the custom HTTP headers for OTLP exporter can be specified through env variables -
OTEL_EXPORTER_OTLP_HEADERS
,OTEL_EXPORTER_OTLP_TRACES_HEADERS
,OTEL_EXPORTER_OTLP_METRICS_HEADERS
. This PR add the support.The existing implementation supports specifying the header at compile time. And as discussed in #1255, the env-variable takes precedence. Currently in this PR, headers specified through configuration and environment variables are merged, with the environment variable keys overriding existing header keys."
Compiler time:
k1=v1, k2=v2
Env variable:
k1=v1_new, k3=v3
final:
k1=v1_new, k2=v2, k3=v3
As of now, change are only done for HTTP, and not gRPC. The MetadataMap in gRPC
tonic
crate only allows to specify metadata headers at compile time (through&'static str
), so it seems not possible to populate this structure from env-var.Please provide a brief description of the changes here.
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes