work around otel reading envvars #181 #208
Merged
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
added 3 commits
May 22, 2023 16:14
We have to delete envvars during configuration that might also be read by the OTel collector client. Also, users of otel-cli exec expect envvars including OTEL stuff to pass through so they don't have to do weird backflips to configure embedded exec usage, which is one of the earliest documented features of otel-cli. To accomplish this, refactored the PersistentPreRun func to be a full func instead of inline. Modified it to inject os.Environ() into the private config field before calling the envvar config func.
When I rewrote the envvar handling for otel-cli and removed Viper, one of the problems I tried to solve was envvars being read by both otel-cli and the otel collector backend it uses. Otel-cli allows some things that the collector doesn't, and besides, otel-cli can't make any guarantees for code paths that it doesn't test. So, envvars have to be deleted from the os environment to prevent that, but we still need to pass the original env through. The easy path forward is to back up the env early at startup, which happens now, and then have exec and status use that backup instead of os.Environ(). This commit implements that. status was updated to also pass through so it's easier to test. Added a test for pass-through, as well as updated tests that use `status` so they now expect envvars to pass through again.
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.
I thought this was going to be more complex but thought of a simpler solution that is completely obvious in retrospect. This PR backs up the env at config time, then modifies exec and status to use that backup. This solves the problem.