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 5 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 <giancarlobuenaflor97@gmail.com>
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.

@buenaflor
Copy link
Contributor

@hangy hey sorry I think we'll have to close this PR. Sentry has decided not to continue supporting the traceparent header

getsentry/sentry-docs#13686

One of the reasons: https://blog.thms.uk/2025/05/openai-sucks

@hangy
Copy link
Author

hangy commented May 13, 2025

One of the reasons: https://blog.thms.uk/2025/05/openai-sucks

What's the difference to a OpenAI potentially sending a sentry-trace header with the sampled bit set to 1? That should virtually be identical to the equivalent standard W3C traceparent header. As long as the header is accepted as the truth even when coming from an untrusted Internet source, the problem should be the same? @buenaflor
Maybe it's because I'm a bit sad I spent time implementing this in Dart, Python, and .NET just for the feature to get dropped directly after working on the publicly posted issues, but I am having a hard time understanding the reasoning.

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