-
Notifications
You must be signed in to change notification settings - Fork 911
Change w3c correlation context to custom header #517
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
Change w3c correlation context to custom header #517
Conversation
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.
I am fine with this. QQ: should this be part of the default API?
Think this discussion is in #428 |
@SergeyKanzhelev please review. also @open-telemetry/specs-approvers :) |
The W3C Distributed Tracing WG is discussing this issue this week. Please do not merge this PR yet. |
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.
requesting changing to W3C editor draft format
The WG decided not to go with IETF dict, and since this isn't merged yet it isn't too late to change |
fe91d70
to
951bd06
Compare
@yurishkuro would appreciate a ✅ here |
btw, why not |
Originally I had been trying to keep the header name as similar as possible to the name I thought the w3c would likely end up with. Because of the recent naming discussions, I think it's clear that we should just use whatever header name makes sense for us. |
@dyladan: it seems like the above poll shows |
@MrAlias oh man I just had a brain fart I guess. updating now. |
55a919d
to
a39edbf
Compare
What I'm seeing:
|
That Makefile command is not really helpful. Running manually:
|
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.
Is "ot" as a prefix potentially confusing when opentracing still exists but is deprecated?
@lizthegrey would you rather we use |
there was never |
There seems to be some continued confusion about what we're actually changing the header name to. I see discussions about |
Yes my fault. |
Is this waiting on something to merge? |
@open-telemetry/technical-committee can we merge this so language SIGs have direction on this? |
…open-telemetry#517) * Change w3c correlation context to custom header * use set-cookie format * rename header to otbaggage * update to header name in poll * fix indentation level * fix links
…etry#517) Signed-off-by: Alexander Wert <[email protected]>
After discussions with the W3C Distributed Tracing Working Group, it has been determined that the Correlation-Context header is not ready for implementation and is being actively changed. As recommended by the working group, we should implement our own header until the specification is an official recommendation.
/cc @tedsuo @mwear