fix(headers): also populate headers from specified environment variables #99
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smoyer64
changed the title
JamieDanielson
added
the
status: oncall
JamieDanielson
added
status: revision needed
JamieDanielson
previously requested changes
Jan 10, 2024
MikeGoldsmith
added
version: bump minor
MikeGoldsmith
approved these changes
Feb 6, 2024
MikeGoldsmith
dismissed
JamieDanielson’s stale review
Will add tests separately
|
Sorry about that ... was slammed at work throughout January! |
MikeGoldsmith
added a commit
that referenced
this pull request
Mar 1, 2024
## Which problem is this PR solving? When setting configuration values by code and environment variables, environment variables should win. This is a design choice so that operators of the library can change behaviour without requiring rebuilding an application. This PR updates the sequence of applying configuration options so that env vars are applied last and will replace any code specificied options. Tests are updated to reflect this, and includes adding additional tests to verify expected behaviour of setting generic, trace and metrics headers via env var which was added in the following P; - Follow up to #99 ## Short description of the changes - Updates all string config options to use the `overwrite` go-envconfig option so env var values replace code-set options - Remove unused ResourceAttributesFromEnv config option - Move applying env var options to the config struct after applying code options - Update tests to reflect env vars have priority over code-provided options, includes fixing a couple of tests where env vars were being used by error ## How to verify that this has the expected result Unit tests exercise expected behaviour and help prevent future regressions. --------- Co-authored-by: Steve Moyer <[email protected]> Co-authored-by: Robb Kidd <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Headers can't be set via the environment variables specified in the OTEL SDK configuration documentation. Since headers are often used to affect authentication, using the provided options isn't always desirable or safe.
Which problem is this PR solving?
Short description of the changes
This PR adds the required field tags to the
Configstruct and documents the existing options as well as the affected environment variables.How to verify that this has the expected result
Set headers via environment variables and verify that the headers are received by an OTEL collector.