-
Notifications
You must be signed in to change notification settings - Fork 15
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
Rename the header to Baggage #17
Comments
I would definitely prefer this to |
I am indifferent towards the name that we choose |
Please see this document for more context: https://docs.google.com/document/d/1Crnp9XguH3BY5b1hcAV2QNiHZV0SKyIuC3moZgdpgiE/edit# |
What about |
We should get buy in from @JonathanMace and/or @rfonseca as I believe the term Baggage was coined at Brown University with a different meaning albeit propagation related. This is less about this header being used for something lightly inspired by Baggage, rather any unlikely IP concerns that might exist https://groups.google.com/forum/#!msg/distributed-tracing/m2ztbNAPsjY/4-mQkrfvBwAJ |
There's some baggage around the term baggage, but it has some weight to it. I know the tracing inner circle is familiar with the name baggage, as it was suggested to b3 some time back openzipkin/b3-propagation#22 Some end users may also be familiar with this as spring-cloud-sleuth uses a similar terminology from the early days of OpenTracing. https://cloud.spring.io/spring-cloud-sleuth/reference/html/#propagating-span-context |
🤦♂ Baggage is an appropriate alternative name for correlation context, and it would be consistent with the way that other distributed tracing tools (Jaeger, OpenTracing) use the term (since they also use it to refer to K-V pairs). Originally we picked the name baggage to refer to "arbitrary stuff" that you propagate with a request because we liked the metaphor. In this case the metaphor is the same, so I don't see why not to call it baggage. For some background on baggage:
It's true that the implementations we proposed in these papers are different to the K-V pair implementation proposed here, but conceptually the goal is the same. |
Personally, I would suggest |
+1 for baggage |
+1 for baggage as well |
Hi all,
I second what Jonathan said above. The only other thing I want to add is
that there is absolutely no IP issue, the term and use was introduced in
published work, with no encumbrances or claims of intellectual property.
Eventually, when use cases arise when baggage from different paths have to
end up merging, the BaggageContex paper describes ways of implementing this
allowing for different ways to merge (set union, max, min, last, etc), in a
generic way.
Glad to see this moving forward!
Best,
Rodrigo
…On Fri, Mar 27, 2020 at 9:50 AM Christoph Neumüller < ***@***.***> wrote:
+1 for baggage as well
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#17 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABHZR5OYW25GLAW2OJNCFTRJTKMBANCNFSM4LUBF7BQ>
.
|
+1 to |
I thumbs upped the original suggestion to use baggage, but doing more explicitly here +1 Thanks @JonathanMace and @rfonseca for clearing the name! I recall the SPDY days which turned into http/2 over name related stuff. nice to be able to avoid that. |
FYI: In Zipkin Brave we'll rename our stuff around baggage naming, since it is general purpose now. will slide into concepts here nicely. |
Nice! Sounds like we have consensus? |
Also, we can consider if renaming the spec itself makes sense.. |
How are we going to decide & close this? |
Jumping in at the last minute here. I'm fine with a changing the header name, so long as the semantic meaning is also changing. When this working group started, there was an intention to say that correlations were for observability only. Crucially, that meant it was acceptable to trim or drop correlations - or at least that was the thinking at the time. If we are going to propose a more general purpose header, then users will begin relying on this propagation mechanism for features beyond observability. That makes baggage a critical system, and dropping baggage could have a serious impact. My main concern is that this debate appears to be focused on a naming preference, and whether Therefore, @yurishkuro I suggest the decision to close these issues should be made based on what scope and responsibility this header should have. If it is only for observability, and potentially lossy, the name should be Open to other interpretations, but want to verify that this working group has consensus on which target they are trying to shoot for, and not just which name they prefer. :) |
@tedsuo I agree with that. The current wording about the purpose of this header is very vague, and makes many references to trace context.
|
per above mentions about opentracing, jaeger, sleuth etc. baggage is indeed already used in practice to implement random fields. They are already lossy in practice and there are a couple years of this in practice. There are other lossy variants out there like Amazon's trace format which permits arbitrary fields and no means to identify if they are correlation or not. I think the header should be renamed as regardless of who is expecting loss (observability or otherwise) it exists. Also even in observability there is state that precedes a trace, such as sampling details etc, which are not correlation in nature. The name "correlations" if somehow restricted to things only correlation in nature would simply result in something creating the baggage header separately and repeating the work. Currently more people are interested in this spec using the name baggage, still.. so maybe rewording the description is the best way out vs fork another name. |
Despite the semantics of |
This kind of reasoning reminds me of XKCD standards... https://xkcd.com/927/ There is value in standardizing what's already the custom / expectation in the wild because it has heavy push moving forward. Let's not change or walk away from that because we want to put the cat back into the bag at quite the cost. Propagating baggage context properly is the largest and most difficult part to get right of all instrumentation concerns. Obviously platform owners want to reuse instead of having to reinvent the wheel for their own baggage concerns while the transport for it is already loaded up. There is quite the brownfield out there already stuffing their own baggage into the instrumentation context because it was available and works well. Let's not break their ecosystems and assumptions.
|
Great, glad to hear it. And to be clear, I'm all for |
Are there any dissenting opinions to renaming the header @dyladan is filing a PR for the rename, but please comment here or on the forthcoming PR if you disagree |
If the overwhelming majority likes the Again, my reasoning towards keeping the As we moving this forward we need to start collecting information on who will be actually using the header so we will demonstrate adoption. |
any updates on this? |
@adriancole it was concluded that the header should be renamed to |
any update? |
@r12a , any thoughts on the use of the name "baggage" (see previous comment from @SergeyKanzhelev ). |
[disclaimer: I'm not familiar with this technology, and have only scanned the long discussion above] Could someone (perhaps @SergeyKanzhelev) give me an idea of what kinds of i18n issues might be at issue here? Is it a question of whether the word 'baggage' has undesirable connotations, or something else? (fwiw, in the UK this word can have a pejorative connotation - indicating uninteresting stuff that it's annoying to have to lug around). |
nailed it! That's exactly what we want it to mean :-) |
Yes - exactly that. If there is no obvious offensive connotation, I think we are good. |
I'm a little surprised this is not merged. For example, it appears that open telemetry is using Lightstep's "Correlations" term in the header name otcorrelations. While OpenTelemetry are free to use vendor terms, it will bring pressure to continue using them, and then not use the cleared term "baggage". Plus, using a word used in trade seems a severe step backwards, especially considering there's no notable reason that this shouldn't have been complete months ago. What else needs to happen here? |
See this PR for more discussion regarding the header change in OpenTelemetry: open-telemetry/opentelemetry-specification#517. The name was selected by the poll in this comment: open-telemetry/opentelemetry-specification#517 (comment). |
I am really curious on what needs to happen for this to be merged given the
consensus.
…On Wed, 8 Jul 2020, 23:52 Matthew Wear, ***@***.***> wrote:
See this PR for more discussion regarding the header change in
OpenTelemetry: open-telemetry/opentelemetry-specification#517
<open-telemetry/opentelemetry-specification#517>.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#17 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXOYARK7NKZM627UMCZJ43R2TS3RANCNFSM4LUBF7BQ>
.
|
this is an alternative proposal to #13. During the workshop today there was a proposal to rename header to the
Baggage
. Creating issue to discuss this proposalThe text was updated successfully, but these errors were encountered: