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

Use the draft of Baggage w3c spec instead of custom otcorrelation header #879

Closed
SergeyKanzhelev opened this issue Aug 26, 2020 · 9 comments · Fixed by #993
Closed

Use the draft of Baggage w3c spec instead of custom otcorrelation header #879

SergeyKanzhelev opened this issue Aug 26, 2020 · 9 comments · Fixed by #993
Assignees
Labels
area:api Cross language API specification issue priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:baggage Related to the specification/baggage directory

Comments

@SergeyKanzhelev
Copy link
Member

Follow up from #857. See related issues

@SergeyKanzhelev SergeyKanzhelev added the spec:baggage Related to the specification/baggage directory label Aug 26, 2020
@SergeyKanzhelev
Copy link
Member Author

CC: @dyladan @mwear

@bogdandrutu
Copy link
Member

the reason to use a different name is to protect against backwards incompatible changes in the current w3c spec. If we are confident that it will not happen we can use the w3c name.

@cijothomas
Copy link
Member

Based on this comment, (#536 (comment)), we should now start using the w3c name, right?

@andrewhsu andrewhsu added release:required-for-ga Must be resolved before GA release, or nice to have before GA area:api Cross language API specification issue priority:p2 Medium priority level labels Aug 28, 2020
@bogdandrutu
Copy link
Member

Because changes can still happen, for us to have a way to support this I would suggest that we use the format with a custom name. When the format becomes official we will have a simple change to do: read any of the headers and maybe emit both headers; in 1y remove the support to read both and only emit the new.

If we use the same name and in an unlikely event of the format change we will not be able to make a transition, so I am willing to pay the cost of double writes and transition to avoid the case of never being compatible

@dyladan
Copy link
Member

dyladan commented Sep 2, 2020

In the end, the TC has to decide what level of risk they want to accept here. The W3C talked about this and decided we think it is unlikely that there will be backwards incompatible changes, but that doesn't mean impossible. An advantage of using the W3C header is that when the spec is complete, otel doesn't have to worry about a migration strategy.

@andrewhsu
Copy link
Member

From the issue triage mtg today, discussed this with @carlosalberto and this one looks like it may be a P1 so we can make sure we have this in before trace spec freeze.

@andrewhsu andrewhsu added priority:p1 Highest priority level and removed priority:p2 Medium priority level labels Sep 9, 2020
@andrewhsu
Copy link
Member

@bogdandrutu or @yurishkuro are either one of you able to be assigned this issue?

@tedsuo
Copy link
Contributor

tedsuo commented Sep 10, 2020

I am in favor of adopting this header now, if it looks like it is stable.

BUT. Know ye W3C members: once OpenTelemetry is out in the wild, it shall be YOU who will have to contend with the fact that this header will now be in production everywhere. So please make sure you are prepared for that. We may choose not to adopt a future breaking change, and the current version will become the de facto standard. So it goes both ways.

In all seriousness, given the overlap between OpenTelemetry and the W3C working group, I won't accept any kind of "whoopsies" on this front. A breaking change that cannot be handled effectively will probably result in the situation above. Let us do all we can to avoid that. If the above warning makes you shake in your boots, please retract the offer.

@andrewhsu
Copy link
Member

from the maintainers mtg today, assigning to @bogdandrutu for initial assignee

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Cross language API specification issue priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:baggage Related to the specification/baggage directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants