-
Notifications
You must be signed in to change notification settings - Fork 541
feat(tracing): Handle tracestate
HTTP headers/correlation context envelope headers
#971
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
feat(tracing): Handle tracestate
HTTP headers/correlation context envelope headers
#971
Conversation
a352af7
to
4f188d3
Compare
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.
Nice implementation.
I'm not really familiar with the SDK so I might miss more subtle points.
I think you have a small bug on extracting the elements from the tracestate
header.
Since it is a bit tricky I would add a parameterized test for extract_tracestate_data
in which I would pass a few plausible headers to make sure it does what we need.
I would also do a separate test for extract_sentrytrace_data
to make sure I'm not missing anything
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.
Looks good to me.
I would move the massaging of '=' out of capture_event
and into from_base64
but it's you call.
3389284
to
2377371
Compare
9a878fc
to
41dc24a
Compare
2377371
to
eed2ca1
Compare
41dc24a
to
91cfd62
Compare
tracestate
correlation context to envelope headerstracestate
HTTP headers/correlation context envelope headers
91cfd62
to
464886e
Compare
@lobsterkatie This is off-topic to this PR; would it be possible to pass around parent transaction name within tracestate? This could help unlock some product features such as knowing the next transaction(s) from a given transaction. For example, from |
@dashed we don't persist tracestate in the db. let's discuss this somewhere else. |
…velope is created
eed2ca1
to
84b225d
Compare
464886e
to
97a4aaf
Compare
This introduces handling of the `tracestate` header, as described in the W3C Trace Context spec[1] and our own corresponding spec[2]. Key features: - Deprecation of `from_traceparent` in favor of `continue_from_headers`, which now propagates both incoming `sentry-trace` and incoming `tracestate` headers. - Propagation of `tracestate` value as a header on outgoing HTTP requests when they're made during a transaction. - Addition of `tracestate` data to transaction envelope headers. Supporting changes: - New utility methods for converting strings to and from base64. - Some refactoring vis-à-vis the links between transactions, span recorders, and spans. See #1173 and #1184. - Moving of some tracing code to a separate `tracing_utils` file. Note: `tracestate` handling is currently feature-gated by the flag `propagate_tracestate` in the `_experiments` SDK option. More details can be found in the main PR on this branch, #971. [1] https://www.w3.org/TR/trace-context/#tracestate-header [2] https://develop.sentry.dev/sdk/performance/trace-context/
This PR is an initial implementation of the official
tracestate
spec (detailed here), which specifies a multi-vendor, vendor-neutral method of propagating trace data between services. The basic format of atracestate
header isvendor1=value1,vendor2=value2,vendor3=value3,...
.Key Behaviors:
tracestate
header is parsed, and Sentry and third-party portions are stored separately on the transactiontracestate
value is calculated and stored on the transactiontracestate
header of the formsentry=sentry_value,vendor1=value1,vendor2=value2
(in other words, wherever the Sentry data was in the incoming list, it's moved to be the first entry)tracestate
value in their envelope headers* Now that there's more than one header, the
from_traceparent
method mentioned in an older version of our tracing spec (since replaced byfrom_sentrytrace
in the spec, but not the SDK) is deprecated in favor of two new helper functions called fromcontinue_from_headers
.** Error events with attachments just because those are the only ones currently sent in envelopes.
Sentry's
tracestate
Value:{ "trace_id": ... , "release": ... , "environment": ... , "public_key": ... }
, base64-encoding it, and stripping off any trailing=
. (release
andenvironment
reflect the values set insentry_sdk.init()
, andpublic_key
comes from the part of the DSN between thehttps://
and the@xxx.ingest.sentry.io
.)Known Limitations:
Compared to the spec, this does not handle:
tracestate
headersx=y,,z=w
)tracestate
header valuetracestate
value or the overalltracestate
headerIt also does not consume, create, or propagate
traceparent
headers. (We continue to use our own similarsentry-trace
header.)Follow-up Tasks
continue_from_headers
/helper function model, and update dev docstracestate
headers and handle third-partytracestate
values