Skip to content
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

fix env variables as per changes in the otel spec #255

Merged
merged 1 commit into from
Oct 27, 2021
Merged

fix env variables as per changes in the otel spec #255

merged 1 commit into from
Oct 27, 2021

Conversation

vvydier
Copy link
Contributor

@vvydier vvydier commented Oct 21, 2021

@bryce-b
Copy link
Member

bryce-b commented Oct 21, 2021

the link for TRACEPARENT & TRACESTATE goes to a git ignore update pr. can you link the intended spec doc? I was having issues finding it

@vvydier
Copy link
Contributor Author

vvydier commented Oct 21, 2021

the link for TRACEPARENT & TRACESTATE goes to a git ignore update pr. can you link the intended spec doc? I was having issues finding it

I had the incorrect link in my comment, meant to link issue#740 (not 74)

@nachoBonafonte
Copy link
Member

My only concern with the change as it is, is that it implies a breaking change is someone is using any of those environment variables already. Should we add support for both naming or should we go further and modify the minor version in the next release?

@vvydier
Copy link
Contributor Author

vvydier commented Oct 22, 2021

My only concern with the change as it is, is that it implies a breaking change is someone is using any of those environment variables already. Should we add support for both naming or should we go further and modify the minor version in the next release?

I vote for your second option to modify the minor version number for the next release. I think it's simpler to keep in sync with the spec than carrying over older environment variables.

Copy link
Member

@nachoBonafonte nachoBonafonte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, works for me

@nachoBonafonte nachoBonafonte merged commit 1583678 into open-telemetry:main Oct 27, 2021
@vvydier vvydier deleted the fix_env_vars branch October 28, 2021 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants