Skip to content
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

Add getTracingMetaTags helper function to Node SDK #8843

Open
2 tasks
Tracked by #9508
Lms24 opened this issue Aug 18, 2023 · 6 comments
Open
2 tasks
Tracked by #9508

Add getTracingMetaTags helper function to Node SDK #8843

Lms24 opened this issue Aug 18, 2023 · 6 comments
Labels
Package: node Issues related to the Sentry Node SDK Type: Improvement

Comments

@Lms24
Copy link
Member

Lms24 commented Aug 18, 2023

Problem Statement

Sentry's browser SDKs pick up <meta name="sentry-trace" .../> and <meta name="baggage" /> tags in a users' HTML when starting the initial pageload transaction. This is how SSR transactions can be connected to their frontend pageload transactions. We use this mechanism internally in our Full-Stack framework SDKs (NextJS, Remix, SvelteKit) but also users can add this to their own SSR setup.

Right now, this feature is neither well documented nor easy to use on the server side.

One use case for the Node SDK: An express app using our Node SDK that renders HTML pages.

Tasks

@swantzter
Copy link

swantzter commented Apr 5, 2024

Hi, This seems like it could be a relevant update in time for v8 with the multiple deprecations and removals of API's used to get those headers. I'm trying to implement the generation of these headers in our in-house SSR framework right now and am running into the issue that, well, all the methods described on https://docs.sentry.io/platforms/node/distributed-tracing/custom-instrumentation/#inject-tracing-information-to-outgoing-requests to get the sentry-trace and baggage headers seems to be deprecated.

I found #10031 which adds a new way to get the sentry-trace header/meta value, but I've yet to find the replacement for dynamicSamplingContextToSentryBaggageHeader which (no longer?) seems importable from @sentry/node nor @sentry/core?

@swantzter
Copy link

Oh! I see there's work on this in https://github.com/getsentry/sentry-javascript/blob/3d82cdb8a4c5d1642976332b6c522498c99901bd/packages/astro/src/server/meta.ts already, I'll copy that to my code for now.

A feedback for when that moves to core, it'd be nice to be able to get the values as unwrapped strings, I would like to be able to use it with something like unhead / useHead

@Lms24
Copy link
Member Author

Lms24 commented Apr 8, 2024

Hey @swantzter thx for sharing your thoughts!. First of all, apologies for the outdated docs around obtaining the meta tag values - these are no longer valid in version 8 of the SDK and in v7, most of the APIs are deprecated. We're unfortunately lagging a bit behind with docs at the moment but we have a huge list of doc updates we'll work through soon (#11064).

The snippet you found in the Astro SDK is spot on! As the JSDoc suggests, we'll move this implementation to core sometime soon (TM).

A feedback for when that moves to core, it'd be nice to be able to get the values as unwrapped strings, I would like to be able to use it with something like unhead / useHead

That's valuable feedback indeed, thanks! I think Remix exposes similar functionality there's already two use cases to obtain the tags as objects rather than stringified html. However, there's also demand for the fully stringified version we currently use in the Astro package (and a couple of other packages).

I think we ideally expose:

  • One function that just obtains all tracing-relevant data for a given span, scope and client. This returns the contents of the sentry-trace and baggage <meta> tags or Http headers. The values are the same in both cases and I think we have enough internal cases where this would also be super helpful in addition to exporting it outside.
    Mental model: One function call to obtain all "raw" tracing data - perfect for your described use case
  • One function that stringifies the returned values of the function above to <meta> tags. We need this mechanism at least twice in our own code so it probably also makes sense to expose this. Mental model: One function call to inject pre-built meta tags.

@Lms24 Lms24 mentioned this issue Apr 8, 2024
@swantzter
Copy link

@Lms24 absolutely, exposing two functions or one function with two different possible return types depending on options was what I had in mind as well, that sounds promising :)

@swantzter
Copy link

@Lms24 related, (or maybe more to #11064 ?) is there any recommended way to have multiple continueTrace on a page? For us with a SSR microfrontend architecture it'd be nice to get errors etc connected to the right parent trace instead of right now where they're orphaned.

So the request chain today looks something like below, where I don't necessarily want another full 'pageload' span but perhaps a 'fragment-mounted' span as children for each of the 'http.server - GET /' spans, with, ideally, any errors encountered in that microfrontend connected to that (or as a child of) that child span

screenshot 1712671881

However from the distributed tracing docs and the micro frontend docs neither details if this is a) a recommended way of doing things, or b) how to initialise these spans "correctly"

I already have a way to send the specific sentry-trace and baggage values to the correct microfrontend that isn't the page-global meta tags - but I'm unsure where to go from there

@Lms24
Copy link
Member Author

Lms24 commented Apr 9, 2024

Hmm this is a tricky problem. Solving sending errors correctly in MFE's is already hard enough but spans/traces is another level 😅 The fundamental problem here is that the browser doesn't have a concept of an Async Context yet. It's proposed in W3C but looks like it'll still take some time. Node already has this, so we (thankfully can) use it to isolate concurrent requests.

We need some sort of context isolation (e.g. between MFE modules/fragments) to e.g. return the correct scope when you'd call getCurrentScope(). If this worked then yes, maybe you could create one scope per fragment and set the propagation context (scope.setPropagationContext) per fragment. However right now, you can only set the current scope globally, meaning you'll run into race conditions when multiple fragments call getCurrentScope().getPropagationContext() to obtain the correct traceId and trace data. So unfortunately, I don't have a good answer how to solve this.

One thing: I'd strongly argue that the pageload span should be seen as a representation of an actual browser page load. The only reason why it's shown as a child of a http.server span in Sentry is because there's a fundamental timing problem:

  1. Browser makes initial request
  2. Server-side, the SSR response is rendered, injecting <meta> tags in the backend which "starts" the trace and makes the http.server span the first (head of trace) span to be created.
  3. Once the client receives the initial response and JS, the Sentry SDK intializes. Directly at this point we check for the <meta> tags to continue the trace. Now we create the pageload span and "backdate" its start time via browser APIs to the time of (1). However, the parent<>child relation is still inverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: node Issues related to the Sentry Node SDK Type: Improvement
Projects
Status: No status
Development

No branches or pull requests

3 participants