-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(nextjs): remove tracing from pages router API routes #18394
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,24 +1,17 @@ | ||
| import { | ||
| captureException, | ||
| continueTrace, | ||
| debug, | ||
| getActiveSpan, | ||
| getCurrentScope, | ||
| getRootSpan, | ||
| httpRequestToRequestData, | ||
| isString, | ||
| isURLObjectRelative, | ||
| objectify, | ||
| parseStringToURLObject, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, | ||
| setHttpStatus, | ||
| startSpanManual, | ||
| setCapturedScopesOnSpan, | ||
| withIsolationScope, | ||
| } from '@sentry/core'; | ||
| import type { NextApiRequest } from 'next'; | ||
| import type { AugmentedNextApiResponse, NextApiHandler } from '../types'; | ||
| import { flushSafelyWithTimeout, waitUntil } from '../utils/responseEnd'; | ||
| import { dropNextjsRootContext, escapeNextjsTracing } from '../utils/tracingUtils'; | ||
| import { HTTP_ROUTE, SENTRY_KIND, URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; | ||
| import { flushSafelyWithTimeout } from '../utils/responseEnd'; | ||
|
|
||
| export type AugmentedNextApiRequest = NextApiRequest & { | ||
| __withSentry_applied__?: boolean; | ||
|
|
@@ -34,117 +27,75 @@ export type AugmentedNextApiRequest = NextApiRequest & { | |
| */ | ||
| export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameterizedRoute: string): NextApiHandler { | ||
| return new Proxy(apiHandler, { | ||
| apply: ( | ||
| apply: async ( | ||
| wrappingTarget, | ||
| thisArg, | ||
| args: [AugmentedNextApiRequest | undefined, AugmentedNextApiResponse | undefined], | ||
| ) => { | ||
| dropNextjsRootContext(); | ||
| return escapeNextjsTracing(() => { | ||
| const [req, res] = args; | ||
| const [req, res] = args; | ||
| if (!req) { | ||
| debug.log( | ||
| `Wrapped API handler on route "${parameterizedRoute}" was not passed a request object. Will not instrument.`, | ||
| ); | ||
| return wrappingTarget.apply(thisArg, args); | ||
| } else if (!res) { | ||
| debug.log( | ||
| `Wrapped API handler on route "${parameterizedRoute}" was not passed a response object. Will not instrument.`, | ||
| ); | ||
| return wrappingTarget.apply(thisArg, args); | ||
| } | ||
|
|
||
| if (!req) { | ||
| debug.log( | ||
| `Wrapped API handler on route "${parameterizedRoute}" was not passed a request object. Will not instrument.`, | ||
| ); | ||
| return wrappingTarget.apply(thisArg, args); | ||
| } else if (!res) { | ||
| debug.log( | ||
| `Wrapped API handler on route "${parameterizedRoute}" was not passed a response object. Will not instrument.`, | ||
| ); | ||
| return wrappingTarget.apply(thisArg, args); | ||
| } | ||
|
|
||
| // Prevent double wrapping of the same request. | ||
| if (req.__withSentry_applied__) { | ||
| return wrappingTarget.apply(thisArg, args); | ||
| } | ||
| req.__withSentry_applied__ = true; | ||
| // Prevent double wrapping of the same request. | ||
| if (req.__withSentry_applied__) { | ||
| return wrappingTarget.apply(thisArg, args); | ||
| } | ||
|
|
||
| return withIsolationScope(isolationScope => { | ||
| // Normally, there is an active span here (from Next.js OTEL) and we just use that as parent | ||
| // Else, we manually continueTrace from the incoming headers | ||
| const continueTraceIfNoActiveSpan = getActiveSpan() | ||
| ? <T>(_opts: unknown, callback: () => T) => callback() | ||
| : continueTrace; | ||
|
|
||
| return continueTraceIfNoActiveSpan( | ||
| { | ||
| sentryTrace: | ||
| req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined, | ||
| baggage: req.headers?.baggage, | ||
| }, | ||
| () => { | ||
| const reqMethod = `${(req.method || 'GET').toUpperCase()} `; | ||
| const normalizedRequest = httpRequestToRequestData(req); | ||
| req.__withSentry_applied__ = true; | ||
|
|
||
| isolationScope.setSDKProcessingMetadata({ normalizedRequest }); | ||
| isolationScope.setTransactionName(`${reqMethod}${parameterizedRoute}`); | ||
| return withIsolationScope(async isolationScope => { | ||
| const reqMethod = `${(req.method || 'GET').toUpperCase()} `; | ||
|
|
||
| const requestUrl = normalizedRequest.url || req.url; | ||
| const urlObject = requestUrl ? parseStringToURLObject(requestUrl) : undefined; | ||
| isolationScope.setSDKProcessingMetadata({ normalizedRequest: httpRequestToRequestData(req) }); | ||
| isolationScope.setTransactionName(`${reqMethod}${parameterizedRoute}`); | ||
|
|
||
| return startSpanManual( | ||
| { | ||
| name: `${reqMethod}${parameterizedRoute}`, | ||
| op: 'http.server', | ||
| forceTransaction: true, | ||
| attributes: { | ||
| [SENTRY_KIND]: 'server', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs', | ||
| [URL_FULL]: urlObject && !isURLObjectRelative(urlObject) ? urlObject.href : undefined, | ||
| [URL_PATH]: urlObject?.pathname, | ||
| [HTTP_ROUTE]: parameterizedRoute, | ||
| }, | ||
| }, | ||
| async span => { | ||
| // eslint-disable-next-line @typescript-eslint/unbound-method | ||
| res.end = new Proxy(res.end, { | ||
| apply(target, thisArg, argArray) { | ||
| setHttpStatus(span, res.statusCode); | ||
| span.end(); | ||
| waitUntil(flushSafelyWithTimeout()); | ||
| return target.apply(thisArg, argArray); | ||
| }, | ||
| }); | ||
| try { | ||
| return await wrappingTarget.apply(thisArg, args); | ||
| } catch (e) { | ||
| // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can | ||
| // store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced | ||
| // to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a | ||
| // way to prevent it from actually being reported twice.) | ||
| const objectifiedErr = objectify(e); | ||
| // We no longer create the transaction ourselves: it's the Next.js root span, which captured a different | ||
| // isolation scope than the one forked here. Bind this scope to that span so the request data and anything | ||
| // set on the scope during the handler (tags, breadcrumbs) land on the transaction. | ||
| const activeSpan = getActiveSpan(); | ||
| const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; | ||
| if (rootSpan) { | ||
| setCapturedScopesOnSpan(rootSpan, getCurrentScope(), isolationScope); | ||
| } | ||
|
|
||
| captureException(objectifiedErr, { | ||
| mechanism: { | ||
| type: 'auto.http.nextjs.api_handler', | ||
| handled: false, | ||
| data: { | ||
| wrapped_handler: wrappingTarget.name, | ||
| function: 'withSentry', | ||
| }, | ||
| }, | ||
| }); | ||
| try { | ||
| return await wrappingTarget.apply(thisArg, args); | ||
| } catch (e) { | ||
| // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can | ||
| // store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced | ||
| // to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a | ||
| // way to prevent it from actually being reported twice.) | ||
| const objectifiedErr = objectify(e); | ||
|
|
||
| setHttpStatus(span, 500); | ||
| span.end(); | ||
| captureException(objectifiedErr, { | ||
| mechanism: { | ||
| type: 'auto.http.nextjs.api_handler', | ||
| handled: false, | ||
| data: { | ||
| wrapped_handler: wrappingTarget.name, | ||
| function: 'withSentry', | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| // we need to await the flush here to ensure that the error is captured | ||
| // as the runtime freezes as soon as the error is thrown below | ||
| await flushSafelyWithTimeout(); | ||
| // we need to await the flush here to ensure that the error is captured | ||
| // as the runtime freezes as soon as the error is thrown below | ||
| await flushSafelyWithTimeout(); | ||
|
|
||
| // We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it | ||
| // would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark | ||
| // the error as already having been captured.) | ||
| throw objectifiedErr; | ||
| } | ||
| }, | ||
| ); | ||
| }, | ||
| ); | ||
| }); | ||
| // We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it | ||
| // would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark | ||
| // the error as already having been captured.) | ||
| throw objectifiedErr; | ||
| } | ||
|
Comment on lines
+88
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: In Node.js serverless environments, Suggested FixReintroduce a call to Prompt for AI AgentDid we get this right? 👍 / 👎 to inform future reviews. |
||
| }); | ||
| }, | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import { HTTP_METHOD, HTTP_REQUEST_METHOD } from '@sentry/conventions/attributes'; | ||
| import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; | ||
| import { ATTR_NEXT_SPAN_NAME, ATTR_NEXT_SPAN_TYPE } from '../common/nextSpanAttributes'; | ||
| import { ATTR_NEXT_PAGES_API_ROUTE_TYPE } from '../common/span-attributes-with-logic-attached'; | ||
|
|
||
| export interface MutableRootSpan { | ||
| attributes: Record<string, unknown>; | ||
| getName(): string | undefined; | ||
| setName(name: string): void; | ||
| setOp(op: string): void; | ||
| } | ||
|
|
||
| /** | ||
| * Normalizes name, op and source for the root span of a pages-router API route on the Edge runtime. | ||
| * | ||
| * We no longer create this transaction ourselves in `wrapApiHandlerWithSentry`, so the root span is the | ||
| * Next.js `Node.runHandler` span. Next.js names it `executing api route (pages) /some/route`, which we | ||
| * turn into a proper `${METHOD} ${route}` transaction with the `http.server` op and `route` source. | ||
| * | ||
| * Applied from both `preprocessEvent` (legacy transaction events) and `processSegmentSpan` (streamed spans), | ||
| * mirroring how `enhanceMiddlewareRootSpan` is wired. | ||
| */ | ||
| export function enhanceRunHandlerRootSpan(span: MutableRootSpan): void { | ||
| const { attributes } = span; | ||
|
|
||
| if (attributes[ATTR_NEXT_SPAN_TYPE] !== 'Node.runHandler') { | ||
| return; | ||
| } | ||
|
|
||
| const spanName = attributes[ATTR_NEXT_SPAN_NAME]; | ||
| if (typeof spanName !== 'string' || !spanName.startsWith(ATTR_NEXT_PAGES_API_ROUTE_TYPE)) { | ||
| return; | ||
| } | ||
|
|
||
| span.setOp('http.server'); | ||
| attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'route'; | ||
|
Comment on lines
+35
to
+36
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: The Suggested FixIn Prompt for AI AgentDid we get this right? 👍 / 👎 to inform future reviews. |
||
|
|
||
| const path = spanName.replace(ATTR_NEXT_PAGES_API_ROUTE_TYPE, '').trim(); | ||
| // eslint-disable-next-line typescript/no-deprecated | ||
| const method = attributes[HTTP_REQUEST_METHOD] ?? attributes[HTTP_METHOD]; | ||
| span.setName(`${typeof method === 'string' ? method : 'GET'} ${path}`); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing
|
||
| } | ||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing flush on successful API responses
High Severity
The Node pages-router wrapper no longer calls
waitUntil(flushSafelyWithTimeout())on successful responses. Flushing only happens in thecatchpath, so on serverless (e.g. Vercel) the runtime can freeze before successful transactions and other pending telemetry are sent. Edge still flushes onspanEnd, and App Router’swrapRouteHandlerWithSentrystill flushes on completion.Reviewed by Cursor Bugbot for commit 7d305c8. Configure here.