Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Follow modification to TraceContext HTTP propagation specs #847

Merged
merged 1 commit into from
Jul 30, 2018
Merged

Follow modification to TraceContext HTTP propagation specs #847

merged 1 commit into from
Jul 30, 2018

Conversation

munisystem
Copy link
Contributor

Hi,

According to the TraceContext specification, it should use traceparent in HTTP request header when propagating context.

This PR is that follow modification to specs.
I changed to usetraceparent instead of Trace-Parent.

basvanbeek
basvanbeek approved these changes Jul 29, 2018
View reviewed changes
Copy link
Member

@basvanbeek basvanbeek left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This update is correct. It was decided at the distributed tracing workshop in Solingen to be explicit about lowercase and without special characters to have maximum interoperability if also used in structured logging, other transport headers, etc.

SergeyKanzhelev
SergeyKanzhelev approved these changes Jul 29, 2018
View reviewed changes
Copy link
Member

@SergeyKanzhelev SergeyKanzhelev left a comment

Choose a reason for hiding this comment

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

Cool, thanks

@munisystem
Copy link
Contributor Author

This update is correct. It was decided at the distributed tracing workshop in Solingen to be explicit about lowercase and without special characters to have maximum interoperability if also used in structured logging, other transport headers, etc.

I see!
By the way, can I browse the document or the note about this decision?
I was interested it but I couldn't find documents referring to change in the repository.

@rakyll rakyll merged commit 72a330c into census-instrumentation:master Jul 30, 2018
@rakyll
Copy link
Contributor

rakyll commented Jul 30, 2018

Thanks!

munisystem added a commit to wantedly/opencensus-ruby that referenced this pull request Jul 30, 2018
@munisystem
Follow modification to TraceContext HTTP propagation specs
140b451 
I fixed that Go implementation doesn't fill specs
ref. census-instrumentation/opencensus-go#847
So I tailor ruby's to specs
@SergeyKanzhelev
Copy link
Member

By the way, can I browse the document or the note about this decision?

https://github.com/w3c/distributed-tracing/blob/master/trace_context/HTTP_HEADER_FORMAT_RATIONALE.md#lowercase-concatenated-header-names

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@basvanbeek basvanbeek basvanbeek approved these changes

@SergeyKanzhelev SergeyKanzhelev SergeyKanzhelev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@munisystem @rakyll @SergeyKanzhelev @basvanbeek