Repository navigation
fix(middleware-sentry): propagate and continue sender trace across runs - #1882
princejain756 wants to merge 1 commit into
Conversation
Closes inngest#1807 Previously `wrapRequest` always synthesized a `sentry-trace` header from `ulidToTraceId(runId)`, so events dispatched inside an active Sentry span started an isolated trace instead of continuing the sender's trace. This adds `transformSendEvent` and `transformStepInput` hooks to propagate active `sentry-trace` and `baggage` data onto outgoing events, strips the internal carrier in `transformFunctionInput`, and updates `wrapRequest` to continue the sender's trace when present (configurable via `continueSenderTrace`, defaulting to `true`).
🦋 Changeset detectedLatest commit: a9ca664 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
 [Medium impact] The change affects trace reporting for users of the Sentry middleware. Fix incoming HTTP trace headers and waiting-event cleanup before merging. FindingsFix with agent prompt### Issue 1
packages/middleware-sentry/src/middleware.ts:619-622
Incoming `sentry-trace` headers never reach this branch through the HTTP adapter. `requestInfo.headers` comes from `getHeaders()`, which builds response headers and forwards only `traceparent` and `tracestate`. A request with a valid Sentry header but no event carrier therefore starts a separate trace.
Read the original request headers through the adapter, or expose them through `requestInfo`. The new test calls `wrapRequest` directly, so it misses this failure.
### Issue 2
packages/middleware-sentry/src/middleware.ts:530-532
Adding the carrier to every outgoing event also changes events returned by `step.waitForEvent()`. The new cleanup only covers `ctx.event` and `ctx.events`, not step results. A waiting function therefore receives `__inngest_sentry_trace` in its business data. If its `EventType` schema rejects extra fields, the engine rejects the result before returning it to the handler.
Keep the carrier outside business data, or strip it from waiting-event results before the engine checks their schemas.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryThis PR carries Sentry trace data on outgoing events and uses it to continue traces in triggered runs.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (isRecord(requestInfo.headers)) { | ||
| let headerSentryTrace: string | undefined; | ||
| let headerBaggage: string | undefined; | ||
| for (const [key, value] of Object.entries(requestInfo.headers)) { |
There was a problem hiding this comment.
Incoming sentry-trace headers never reach this branch through the HTTP adapter. requestInfo.headers comes from getHeaders(), which builds response headers and forwards only traceparent and tracestate. A request with a valid Sentry header but no event carrier therefore starts a separate trace.
Read the original request headers through the adapter, or expose them through requestInfo. The new test calls wrapRequest directly, so it misses this failure.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/middleware-sentry/src/middleware.ts
Line: 619-622
Comment:
**HTTP trace headers are lost**
Incoming `sentry-trace` headers never reach this branch through the HTTP adapter. `requestInfo.headers` comes from `getHeaders()`, which builds response headers and forwards only `traceparent` and `tracestate`. A request with a valid Sentry header but no event carrier therefore starts a separate trace.
Read the original request headers through the adapter, or expose them through `requestInfo`. The new test calls `wrapRequest` directly, so it misses this failure.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| data: { | ||
| ...existingData, | ||
| [SENTRY_TRACE_DATA_KEY]: traceData, |
There was a problem hiding this comment.
Adding the carrier to every outgoing event also changes events returned by step.waitForEvent(). The new cleanup only covers ctx.event and ctx.events, not step results. A waiting function therefore receives __inngest_sentry_trace in its business data. If its EventType schema rejects extra fields, the engine rejects the result before returning it to the handler.
Keep the carrier outside business data, or strip it from waiting-event results before the engine checks their schemas.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/middleware-sentry/src/middleware.ts
Line: 530-532
Comment:
**Waiting events keep internal data**
Adding the carrier to every outgoing event also changes events returned by `step.waitForEvent()`. The new cleanup only covers `ctx.event` and `ctx.events`, not step results. A waiting function therefore receives `__inngest_sentry_trace` in its business data. If its `EventType` schema rejects extra fields, the engine rejects the result before returning it to the handler.
Keep the carrier outside business data, or strip it from waiting-event results before the engine checks their schemas.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Fixes #1807 (EXE-2292)
wrapRequestin@inngest/middleware-sentryalways built a syntheticsentry-traceheader fromulidToTraceId(runId), so events dispatched inside an active Sentry span started an isolated trace instead of continuing the sender's trace.This adds
transformSendEventandtransformStepInputhooks to attach activesentry-traceandbaggagedata onto outgoing events, strips the internal carrier key intransformFunctionInputso user handlers still receive cleanevent.data, and updateswrapRequestto continue the sender's trace from the incoming request headers or event payload when present (falling back toulidToTraceId(runId)when no sender trace exists or whencontinueSenderTrace: falseis configured).