ref(node)!: Consolidate httpIntegration options - #23443
Conversation
size-limit report 📦
|
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fc42fd3. Configure here.
0dcceac to
c1f4d06
Compare
1cbbdc4 to
a8d5090
Compare
isaacs
left a comment
There was a problem hiding this comment.
This looks great. Some really minor suggestions, if you agree, but this is overall just a nice improvement.
| propagateTraceInOutgoingRequests: options.tracePropagation ?? true, | ||
| // oxlint-disable-next-line typescript/no-deprecated -- deprecated alias kept until removal | ||
| createSpansForOutgoingRequests: spans, | ||
| propagateTrace: options.tracePropagation ?? true, |
There was a problem hiding this comment.
I think this might be the time (or another follow-up) to pick one: propagateTrace or tracePropagation. (Removing propagateTraceInOutgongRequests is good! But may as well go all the way.)
There was a problem hiding this comment.
Good call! i'll track the full rename as a follow up in https://linear.app/getsentry/issue/JS-3427/unify-tracepropagation-vs-propagatetrace-option-names
1475f9a to
b0af040
Compare
Public option names now match httpServerIntegration / httpServerSpansIntegration. Nested instrumentation hooks are first-class outgoing hooks instead of a leftover OTEL-era nest. Fixes #22255
Serverless freeze happens at the end of the incoming request, which onSpanCreated and Nitro's event-handler patch already cover. Flushing again on each outbound response starts extra 2s flushes on platforms without waitUntil.
patchEventHandler already flushes after the Nitro handler. The onSpanCreated hook ran at span start, before the request had events to send. Co-Authored-By: Cursor Grok 4.6 <cursoragent@cursor.com>
Precomputing them here pinned the selection to the raw options, before `initNode` resolves `SENTRY_TRACES_SAMPLE_RATE`. Anyone enabling tracing purely through the environment got `hasSpansEnabled() === false` at this call site and lost every performance integration, while the channel injection inside `initNode` was still gated on the resolved value. Now that the Http override is gone there is nothing left to customize, so the whole array can be left to `initNode`.
`httpServerSpansIntegration` only instruments incoming requests, so pointing its migration note at `outgoingRequestHook` and friends sent readers looking for options that integration does not have. Those live on `httpIntegration`. Also drops the before/after block under the consolidation table. It restated every row it sat under, and the sibling "Removed option" table for `@sentry/nextjs` carries no example either.
…mple `instrumentation.requestHook` ran for incoming and outgoing spans in v10, so a reader migrating an outgoing hook would copy the sample onto `onSpanCreated` and get a hook that never fires for client spans. Restores the consolidation before/after block dropped earlier in this branch.
`outgoingRequestHook`, `outgoingResponseHook` and `outgoingRequestApplyCustomAttributes` had no coverage after #23396 dropped the incoming `instrumentation.*` assertions without replacing the outgoing side. Each hook derives its attribute from the objects it is handed, so a hook wired to the wrong span, request or response fails rather than passing silently.
It was an intersection of the outgoing options with nine deprecated no-ops. Removing those collapsed it into a bare alias of `OutgoingHttpRequestInstrumentationOptions`, leaving two names for one type — and the surviving name points at `SentryHttpInstrumentation`, an export that no longer exists. Neither name is re-exported from `@sentry/node`, so this is internal only.
…ssage The real Node request/response objects already satisfy the core minimal types, so the unions were redundant. Cast at the core boundary because those hooks are typed as HttpClientRequest / HttpIncomingMessage. Co-Authored-By: Cursor Grok 4.6 <cursoragent@cursor.com>
b0af040 to
7ef796f
Compare

httpIntegrationnow uses the same option names ashttpServerIntegration/httpServerSpansIntegrationand the other server SDKs. After #23396, incominginstrumentation.*hooks were already gone; this PR finishes the leftover public API: flatten outgoinginstrumentationinto first-class hooks, and drop the long aliases that only existed onhttpIntegration.Fixes #22255
Why
The public
httpIntegrationsurface had long names (trackIncomingRequestsAsSessions,maxIncomingRequestBodySize, …) mapped 1:1 onto shorter names already used by the sub-integrations, Bun, Deno, and Cloudflare. Those aliases were leftover from the OTEL HttpInstrumentation era; they were never deprecated, and v11 is the point to collapse them rather than carry both.Incoming
instrumentation.{requestHook,responseHook,applyCustomAttributesOnSpan}already all ran at span create after dropping OTEL, which is why #23396 collapsed them toonSpanCreated. Outgoing still has real request vs response timing, so those becomeoutgoingRequestHook/outgoingResponseHook/outgoingRequestApplyCustomAttributesinstead of staying nested.Nuxt's
httpIntegrationoverride is dropped with them. It flushed on every outgoing HTTP response, which is the wrong place: the serverless freeze happens at the end of the incoming request, andpatchEventHandleralready flushes there after the Nitro handler. On platforms withoutwaitUntil, the old hook also started an extra 2s flush per outbound request.Nuxt also stops pre-resolving
defaultIntegrations. That line only existed to filter outHttpand splice the customized integration back in, so removing the override left it doing nothing but harm: it computed the list from the raw options, pinning integration selection beforeinitNoderesolvesSENTRY_TRACES_SAMPLE_RATE. Anyone enabling tracing purely through the environment silently lost every performance integration, while the channel injection insideinitNodewas still gated on the resolved value.