fix(astro): Isolate request instrumentation middleware on server side #9709
Conversation
Lms24
changed the title
size-limit report 📦
|
| headers.forEach((value, key) => { | ||
| allHeaders[key] = value; | ||
| }); | ||
| const { dynamicSamplingContext, traceparentData, propagationContext } = tracingContextFromHeaders( |
There was a problem hiding this comment.
We can now use continueTrace for this.
There was a problem hiding this comment.
good point, will change it!
There was a problem hiding this comment.
(Great call - avoiding duplicating this logic is worth the refactor)
Lms24
force-pushed
the
lms/fix-astro-isolate-requests
branch
from
d3a2f0e to
7f908d9
Compare
| @@ -226,7 +226,6 @@ export function continueTrace<V>( | |||
| * These values can be obtained from incoming request headers, | |||
| * or in the browser from `<meta name="sentry-trace">` and `<meta name="baggage">` HTML tags. | |||
| * | |||
| * It also takes an optional `request` option, which if provided will also be added to the scope & transaction metadata. | |||
There was a problem hiding this comment.
This line was outdated, the function doesn't take a request arg anymore
|
@anonrig if you're curious, this PR deals with Node AsyncLocalStorage and how we use it to isolate request context |
| return res; | ||
| } catch (e) { | ||
| sendErrorToSentry(e); | ||
| throw e; |
There was a problem hiding this comment.
Question: This will change the error stack. If we want to change this, we need something like hideStackFrames in Node.js. Is it intended?
There was a problem hiding this comment.
Yes this creates stack frames but when processing them in the Sentry backend, we remove them. You can compare the stack trace and the raw original stack trace on this event: https://sentry-sdks.sentry.io/issues/4674142395/?project=4506099854475265&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=14d&stream_index=1
There was a problem hiding this comment.
This link gave Too Many Redirects error to me.
Lms24
force-pushed
the
lms/fix-astro-isolate-requests
branch
from
b59ebe3 to
988990c
Compare
This PR adds isolation to our sever-side request instrumentation, similarly to the request handler in
Sveltekit.
Note for reviewers: The changes look more drastic than they are. The gist is, we run the previous instrumentation in a new async context if there's no ongoing span. Otherwise we just call the instrumentation.