Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
tracing: add baggage methods to Tracing::Span #12260
tracing: add baggage methods to Tracing::Span #12260
Changes from 8 commits
0fc9517
d22aa9e
2d2a59b
e8db6dd
78a6870
41d1323
9259977
68df9d1
92e862f
4abed0b
dd0933a
e33c479
fe78194
6004faa
bad6068
e75f095
05dbaa5
446ef0a
f86f0f8
a277e37
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
What's the difference with setTag?
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.
baggage is persisted in the
SpanContext
, which means that any subsequent spans can access data in the baggage (it gets injected/extracted across process boundaries). This is useful when you need to embed information that is available for the entirety of the requestmore reading: https://opentracing.io/docs/overview/tags-logs-baggage/, https://github.com/opentracing/specification/blob/master/specification.md#set-a-baggage-item
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.
consider logging a warning here that this tracer doesn't support baggage (and likewise for the other tracers that do not support it)
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.
you could even log an error saying baggage isn't supported similar to
envoy/source/extensions/tracers/opencensus/opencensus_tracer_impl.cc
Line 358 in 68df9d1
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.
A warning isn't safe in the data path. I would either do nothing and leave a TODO or have a stat for dropped baggage. Same elsewhere.
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 left comments for the spans that don't support baggage (opencensus, xray) and a TODO for zipkin