search

getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.94k stars 1.56k forks source link

W3C Trace Context support #11171

Open stephanie-anderson opened 7 months ago

stephanie-anderson commented 7 months ago

https://github.com/getsentry/team-sdks/issues/41

jamescrowley commented 1 week ago

@stephanie-anderson @AbhiPrasad would you be open to a PR for this, along the lines of the PHP library support (https://github.com/getsentry/sentry-php/pull/1680, https://github.com/getsentry/sentry-php/pull/1713/) using https://github.com/getsentry/team-sdks/issues/41#issuecomment-2090098812 to generate the traceparent value? I'd be happy to put something together

Lms24 commented 1 week ago

Hi @jamescrowley thanks for offering to open a PR! At the moment we're still figuring out a somewhat related issue with trace propagation (more specifically, how to avoid propagating traces across multiple sentry orgs/customers). So we didn't move forward with implementing this. In JS (browser) specifically, we also can't hard-switch to traceparent because it would break all users who have to allow-list Http headers in their CORS configuration. Once we have things figured out, we'll ping you if we deem it okay to be implemented by the community. Thanks again!

cleptric commented 1 week ago

@jamescrowley could you shed some light on your supposed use-case for the traceparent header?

jamescrowley commented 1 week ago

@cleptric we have an React SPA with no server-side rendering. We're looking to align the trace identifiers that Sentry generates on the client, with the server logs on the Google Cloud side.

At the moment, server-side requests receive a conflicting traceparent from the Google infrastructure and a sentry-trace from the client-side request that has been intercepted by Sentry, but that's not something we can get the GCP side to use.

Hope that makes sense, happy to explore further - or indeed if there are alternatives to achieving this. I had a look at hooking into the fetch interception logic on the Sentry client when you create the span ids and inject the sentry-trace header (addTracingHeadersToFetchRequest), but there doesn't appear to be a 'nice' point to do that.

Given ^^ I'll look now at creating our own span when making web requests, and adding our own header using the logic outlined here https://github.com/getsentry/team-sdks/issues/41#issuecomment-2090098812 - which (not verified yet) but hopefully Sentry will then re-use the existing span for the request so the two match.

cleptric commented 5 days ago

Our main goal, if we add support for the traceparent header, would be to support use-cases where all applications in the "trace chain" would report their telemetry to Sentry. An application receiving a 3rd party tracing header and thus sampling a trace results into a quota issue, as you typically do not want to continue a trace from an unknown 3rd party. Fwiw, we actually ran into a couple of issues with the implementation in PHP, hence we held off adding this to other SDKs for the time being.