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

Refactor spec towards trace-parent and trace-state #57

Closed
codefromthecrypt opened this issue Feb 5, 2018 · 4 comments
Closed

Refactor spec towards trace-parent and trace-state #57

codefromthecrypt opened this issue Feb 5, 2018 · 4 comments

Comments

@codefromthecrypt
Copy link

codefromthecrypt commented Feb 5, 2018

During the last workshop, we discussed the role of the two headers somewhat differently than before

trace-parent and trace-state are the headers defined in the trace context specification, these replace trace-context and trace-context-ext respectively. The major point here is that we understand there will be multiple trace graphs possible related to an incoming request. For example, an api gateway from amazon and zipkin. In brief, the new names clarify the portable format describes the incoming request, and the second header is not an extension of it.

MVP @erabug for giving us the push from the cryptic name trace-context-ext -> trace-state


  • trace-parent (formerly known as trace-context) describes the position of the incoming request in its trace graph, in a portable format

  • trace-state includes a mapping of all graphs the incoming parent is a part of in potentially vendor-specific formats. For example, if an ancestor of this request was in a different trace, there would be a separate entry for the last position it was in that trace graph

The trace-state section is not reliant on data on the trace-parent, except if the parent has no data beyond trace-context format. For example, the below hints that there is only one trace graph and that's amazon's

trace-parent: 00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01
trace-state: aws=BleGNlZWRzIHRo
ZSBzaG9ydCB2ZWhlbWVuY2Ugb2YgYW55IGNhcm5hbCBwbGVhc3VyZS4=

The trace-state section is not an extension of the data in trace-parent, rather it is the gold copy per vendor.

Please don't get caught up on the label "aws" or the base 64 encoding. The above example isn't realistic in either case as we don't know the label or encoding AWS will use. Suffice to say this demonstrates the values are different!

In the case that the incoming trace is in generic format, the value of trace-state can be left out. This can help with header compression while still allowing branding, tenant, or otherwise custom labels to be used. These labels can help differentiate different systems that use the same format

trace-parent: 00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01
trace-state: yelp

The above is shorthand for

trace-parent: 00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01
trace-state: yelp=00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01

but better for http compression and performance in general

While I mentioned some details, in order to keep this issue focused on the rename part, it is intentionally not covering other parts of the state model. These can be opened later, for example elaborating what happens hop-to-hop. This issue should be closed out with a rename and a corrected description of the two headers.

@wu-sheng
Copy link
Contributor

wu-sheng commented Feb 5, 2018

Glad we make the progress.

@SergeyKanzhelev
Copy link
Member

Thinking more about parent - it may not be logical to return it in response for #50. Something to consider with the renaming

@wu-sheng
Copy link
Contributor

wu-sheng commented Feb 7, 2018

I think the context could be in response, but the name shouldn't be parent prefix. In response propagation, the info includes the context most likely from direct child/service-provider.

@codefromthecrypt
Copy link
Author

codefromthecrypt commented Feb 7, 2018 via email

codefromthecrypt pushed a commit that referenced this issue Feb 27, 2018
The following is an "all-in-one" change to capture the essence of changes
described since the February workshop. Notably, this includes:

* lowercase concatenation naming convention (ex `traceparent`)
* applied description of the multiple trace states

As the two headers are tightly coupled, this places them in the same document.

This is done as one change as there is pressure for folks to do implementations
and so far very few changes have actually taken place. By spiking a review to
integrate feedback, we are more likely to have interoperable implementations
without accidentally implementing old versions of the standard. Please take this
practical concern into consideration when reviewing.

Fixes #57
AloisReitbauer pushed a commit that referenced this issue Mar 12, 2018
* Reorganizes content to reflect decisions made since last workshop

The following is an "all-in-one" change to capture the essence of changes
described since the February workshop. Notably, this includes:

* lowercase concatenation naming convention (ex `traceparent`)
* applied description of the multiple trace states

As the two headers are tightly coupled, this places them in the same document.

This is done as one change as there is pressure for folks to do implementations
and so far very few changes have actually taken place. By spiking a review to
integrate feedback, we are more likely to have interoperable implementations
without accidentally implementing old versions of the standard. Please take this
practical concern into consideration when reviewing.

Fixes #57

* Weaves in feedback
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

No branches or pull requests

3 participants