Skip to content

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hangy
Copy link

@hangy hangy commented Apr 5, 2025

📜 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 to SentryTraceHeader (akin to SentryTraceHeader.value), but that would violate that class' responsibility. Another alternative might be adding a ISentrySpan.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

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed - the referenced change of the PHP SDK didn't introduce any doc updates.
  • All tests passing
  • No breaking changes

🔮 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.

@hangy hangy force-pushed the 1831-tracespan-header branch from 056279e to 0a688f2 Compare April 6, 2025 09:46
Copy link
Contributor

@buenaflor buenaflor left a 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

Copy link
Contributor

@buenaflor buenaflor left a 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 <[email protected]>
Copy link

codecov bot commented Apr 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.64%. Comparing base (886a258) to head (6308b4f).
Report is 31 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Successfully merging this pull request may close these issues.

2 participants