-
-
Notifications
You must be signed in to change notification settings - Fork 255
feat: Add W3C traceparent support to tracing client and utils #2843
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
base: main
Are you sure you want to change the base?
Conversation
056279e
to
0a688f2
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.
hey, thanks for the PR, changes look good to me overall but during the review I noticed a bug in the tracing client (unrelated to this PR) which I'd like to fix first before adding this
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.
thx for the contribution!
looks good to me overall, merging this will be a bit later after we released v9
Co-authored-by: Giancarlo Buenaflor <giancarlobuenaflor97@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2843 +/- ##
==========================================
- Coverage 87.68% 87.64% -0.05%
==========================================
Files 272 272
Lines 9031 9058 +27
==========================================
+ Hits 7919 7939 +20
- Misses 1112 1119 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@hangy hey sorry I think we'll have to close this PR. Sentry has decided not to continue supporting the One of the reasons: https://blog.thms.uk/2025/05/openai-sucks |
What's the difference to a OpenAI potentially sending a |
📜 Description
Based on the changes in getsentry/sentry-php@a31d418, passes the Sentry traceId and spanId as a W3C traceparent header in HTTP requests to allow trace propagation using W3C standards.
💡 Motivation and Context
This adds support for propagating the Sentry TraceId as a W3C TraceSpan header to support #1831.
This is probably the easiest way to implement W3C
traceparent
headers without introducing breaking changes. There might be a performance implication as the header value is recomputed every time. As an alternative, I considered adding a computed value toSentryTraceHeader
(akin toSentryTraceHeader.value
), but that would violate that class' responsibility. Another alternative might be adding aISentrySpan.toW3CTrace()
method, but introducing this as a breaking change feels kinda clunky.💚 How did you test it?
I currently don't have a full test setup for dart, so I tested with integration tests only for now.
📝 Checklist
sendDefaultPii
is enabled🔮 Next steps
If anyone had a Sentry integrated dart client that called a server that supports the W3C traceparent header, it would be great if they could test this PR.