Skip to content

Remove traceparent propagation (per default) #4185

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

Closed
Flash0ver opened this issue May 13, 2025 · 9 comments · Fixed by #4204
Closed

Remove traceparent propagation (per default) #4185

Flash0ver opened this issue May 13, 2025 · 9 comments · Fixed by #4204

Comments

@Flash0ver
Copy link
Member

Flash0ver commented May 13, 2025

Support for W3C traceparent header is removed again from other SDKs, too.
It seems to do more harm than good: https://blog.thms.uk/2025/05/openai-sucks

See also getsentry/sentry-docs#13686
I don't think we added any .NET specific docs yet

Was originally added via #4084
Was initially published in Sentry 5.6.0

Alternatively, we could make this an explicit opt-in instead, where the default is disabled.

@Flash0ver Flash0ver changed the title Remove traceparent propagation Remove traceparent propagation (per default) May 14, 2025
@bruno-garcia
Copy link
Member

@jamescrosswell
Copy link
Collaborator

Looks like he's OK for us just to remove that... it's a pity 😞

@hangy
Copy link
Contributor

hangy commented May 16, 2025

FWIW I don't use it, as I implemented the issue/feature for other people that might need it. Since we can't know if anyone else depends on it, it's still a breaking change

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 16, 2025
@Flash0ver
Copy link
Member Author

Thanks for the reply and more context. And sorry for the confusion we've caused. We should have closed the original issue earlier.
I am thinking about three options:

  • remove it soon, so that it isn't out in too many versions
  • remove in the next major release
  • make the feature opt-in (previous behavior per default)
    • not necessarily via a new SentryOption property, as it's getting quite large and not to make this opt-in "too discoverable"
    • but perhaps via an AppContext switch, still documented, but as a more advanced opt-in
    • but there is an argument to be made about testability and maintainability

@jamescrosswell
Copy link
Collaborator

jamescrosswell commented May 19, 2025

I think the goal is to get rid of it by the next major version, at the latest.

It's been out in the wild for about a week so I think we're probably pretty safe to just remove it.

If anyone complains, then we can add it back as an OptIn and decide how we want to do that based on why folks want it back.

@ericsampson
Copy link

ericsampson commented May 23, 2025

Kinda a bummer 😔

Seems like there should be some ways to allow traceparent inheritance from an allowlist, or other ways of avoiding this "external services sending trace data" issue

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 May 23, 2025
@jamescrosswell
Copy link
Collaborator

Hm, I'm not sure this is something we can unilaterally decide to support in just the .NET SDK. @cleptric do you have a bit more context around this that we can share with @ericsampson ?

@jamescrosswell
Copy link
Collaborator

@ericsampson what is the scenario where you would need/want to propagate trace context either to Sentry or from Sentry using the traceparent header?

@cleptric
Copy link
Member

cleptric commented May 27, 2025

We no longer want to support traceparent in any of our SDKs for the time being. All our SDKs support trace propagation via the sentry-trace header. If someone needs to have a mixed setup with some services only emitting a traceparent header, we have ample of APIs available to allow these cases to be supported manually.
The current state of affairs is that we see hundreds-of-thousands of useless traces being generated by unrelated 3rd parties, blasting their traceparent header into the world without any considerations about the consequences. We have plans to fix this for our Sentry tracing header and will later look again into how we can support traceparent properly.

@jamescrosswell @Flash0ver for now, please get the PR that reverts this out in due time 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants