From 248632fe7ec3abbd286209285b4623760c2a8345 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 16 Apr 2024 11:14:21 +0000 Subject: [PATCH 1/9] fix(nextjs): Escape Next.js' OpenTelemetry instrumentation --- .../src/common/utils/commonObjectTracing.ts | 27 -- .../src/common/utils/edgeWrapperUtils.ts | 103 +++---- .../nextjs/src/common/utils/tracingUtils.ts | 94 +++++++ .../withIsolationScopeOrReuseFromRootSpan.ts | 39 --- .../nextjs/src/common/utils/wrapperUtils.ts | 7 +- .../common/withServerActionInstrumentation.ts | 171 ++++++------ .../src/common/wrapApiHandlerWithSentry.ts | 263 +++++++++--------- .../wrapApiHandlerWithSentryVercelCrons.ts | 150 +++++----- .../wrapGenerationFunctionWithSentry.ts | 127 +++++---- .../src/common/wrapMiddlewareWithSentry.ts | 13 +- .../src/common/wrapPageComponentWithSentry.ts | 104 +++---- .../src/common/wrapRouteHandlerWithSentry.ts | 150 +++++----- .../common/wrapServerComponentWithSentry.ts | 93 ++++--- .../src/edge/wrapApiHandlerWithSentry.ts | 24 +- packages/nextjs/src/server/index.ts | 5 +- .../requestIsolationScopeIntegration.ts | 44 --- 16 files changed, 703 insertions(+), 711 deletions(-) delete mode 100644 packages/nextjs/src/common/utils/commonObjectTracing.ts create mode 100644 packages/nextjs/src/common/utils/tracingUtils.ts delete mode 100644 packages/nextjs/src/common/utils/withIsolationScopeOrReuseFromRootSpan.ts delete mode 100644 packages/nextjs/src/server/requestIsolationScopeIntegration.ts diff --git a/packages/nextjs/src/common/utils/commonObjectTracing.ts b/packages/nextjs/src/common/utils/commonObjectTracing.ts deleted file mode 100644 index 988dee391dc4..000000000000 --- a/packages/nextjs/src/common/utils/commonObjectTracing.ts +++ /dev/null @@ -1,27 +0,0 @@ -import type { PropagationContext } from '@sentry/types'; - -const commonMap = new WeakMap(); - -/** - * Takes a shared (garbage collectable) object between resources, e.g. a headers object shared between Next.js server components and returns a common propagation context. - * - * @param commonObject The shared object. - * @param propagationContext The propagation context that should be shared between all the resources if no propagation context was registered yet. - * @returns the shared propagation context. - */ -export function commonObjectToPropagationContext( - commonObject: unknown, - propagationContext: PropagationContext, -): PropagationContext { - if (typeof commonObject === 'object' && commonObject) { - const memoPropagationContext = commonMap.get(commonObject); - if (memoPropagationContext) { - return memoPropagationContext; - } else { - commonMap.set(commonObject, propagationContext); - return propagationContext; - } - } else { - return propagationContext; - } -} diff --git a/packages/nextjs/src/common/utils/edgeWrapperUtils.ts b/packages/nextjs/src/common/utils/edgeWrapperUtils.ts index 272889fa76db..bf526c401d55 100644 --- a/packages/nextjs/src/common/utils/edgeWrapperUtils.ts +++ b/packages/nextjs/src/common/utils/edgeWrapperUtils.ts @@ -4,7 +4,6 @@ import { SPAN_STATUS_OK, captureException, continueTrace, - getIsolationScope, handleCallbackErrors, setHttpStatus, startSpan, @@ -13,6 +12,7 @@ import { winterCGRequestToRequestData } from '@sentry/utils'; import type { EdgeRouteHandler } from '../../edge/types'; import { flushQueue } from './responseEnd'; +import { commonObjectToIsolationScope, escapeNextjsTracing } from './tracingUtils'; /** * Wraps a function on the edge runtime with error and performance monitoring. @@ -22,61 +22,66 @@ export function withEdgeWrapping( options: { spanDescription: string; spanOp: string; mechanismFunctionName: string }, ): (...params: Parameters) => Promise> { return async function (this: unknown, ...args) { - const req: unknown = args[0]; + return escapeNextjsTracing(() => { + const req: unknown = args[0]; - let sentryTrace; - let baggage; + let sentryTrace; + let baggage; - if (req instanceof Request) { - sentryTrace = req.headers.get('sentry-trace') || ''; - baggage = req.headers.get('baggage'); - } + const isolationScope = commonObjectToIsolationScope(req); - return continueTrace( - { - sentryTrace, - baggage, - }, - () => { - getIsolationScope().setSDKProcessingMetadata({ - request: req instanceof Request ? winterCGRequestToRequestData(req) : undefined, + if (req instanceof Request) { + sentryTrace = req.headers.get('sentry-trace') || ''; + baggage = req.headers.get('baggage'); + + isolationScope.setSDKProcessingMetadata({ + request: winterCGRequestToRequestData(req), }); - return startSpan( - { - name: options.spanDescription, - op: options.spanOp, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping', + } + + return continueTrace( + { + sentryTrace, + baggage, + }, + () => { + return startSpan( + { + name: options.spanDescription, + op: options.spanOp, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping', + }, }, - }, - async span => { - const handlerResult = await handleCallbackErrors( - () => handler.apply(this, args), - error => { - captureException(error, { - mechanism: { - type: 'instrument', - handled: false, - data: { - function: options.mechanismFunctionName, + async span => { + const handlerResult = await handleCallbackErrors( + () => handler.apply(this, args), + error => { + captureException(error, { + mechanism: { + type: 'instrument', + handled: false, + data: { + function: options.mechanismFunctionName, + }, }, - }, - }); - }, - ); + }); + }, + ); - if (handlerResult instanceof Response) { - setHttpStatus(span, handlerResult.status); - } else { - span.setStatus({ code: SPAN_STATUS_OK }); - } + if (handlerResult instanceof Response) { + setHttpStatus(span, handlerResult.status); + } else { + span.setStatus({ code: SPAN_STATUS_OK }); + } - return handlerResult; - }, - ).finally(() => flushQueue()); - }, - ); + return handlerResult; + }, + ).finally(() => flushQueue()); + }, + ); + }); }; } diff --git a/packages/nextjs/src/common/utils/tracingUtils.ts b/packages/nextjs/src/common/utils/tracingUtils.ts new file mode 100644 index 000000000000..91e74494d3c2 --- /dev/null +++ b/packages/nextjs/src/common/utils/tracingUtils.ts @@ -0,0 +1,94 @@ +import { Scope, getCurrentScope, withActiveSpan } from '@sentry/core'; +import type { PropagationContext } from '@sentry/types'; +import { GLOBAL_OBJ, logger, uuid4 } from '@sentry/utils'; +import { DEBUG_BUILD } from '../debug-build'; + +const commonPropagationContextMap = new WeakMap(); + +/** + * Takes a shared (garbage collectable) object between resources, e.g. a headers object shared between Next.js server components and returns a common propagation context. + * + * @param commonObject The shared object. + * @param propagationContext The propagation context that should be shared between all the resources if no propagation context was registered yet. + * @returns the shared propagation context. + */ +export function commonObjectToPropagationContext( + commonObject: unknown, + propagationContext: PropagationContext, +): PropagationContext { + if (typeof commonObject === 'object' && commonObject) { + const memoPropagationContext = commonPropagationContextMap.get(commonObject); + if (memoPropagationContext) { + return memoPropagationContext; + } else { + commonPropagationContextMap.set(commonObject, propagationContext); + return propagationContext; + } + } else { + return propagationContext; + } +} + +const commonIsolationScopeMap = new WeakMap(); + +/** + * Takes a shared (garbage collectable) object between resources, e.g. a headers object shared between Next.js server components and returns a common propagation context. + * + * @param commonObject The shared object. + * @param isolationScope The isolationScope that should be shared between all the resources if no isolation scope was created yet. + * @returns the shared isolation scope. + */ +export function commonObjectToIsolationScope(commonObject: unknown): Scope { + if (typeof commonObject === 'object' && commonObject) { + const memoIsolationScope = commonIsolationScopeMap.get(commonObject); + if (memoIsolationScope) { + return memoIsolationScope; + } else { + const newIsolationScope = new Scope(); + commonIsolationScopeMap.set(commonObject, newIsolationScope); + return newIsolationScope; + } + } else { + return new Scope(); + } +} + +interface AsyncLocalStorage { + getStore(): T | undefined; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + run(store: T, callback: (...args: TArgs) => R, ...args: TArgs): R; +} + +let nextjsEscapedAsyncStorage: AsyncLocalStorage; + +/** + * TODO + */ +export function escapeNextjsTracing(cb: () => T): T { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any + const MaybeGlobalAsyncLocalStorage = (GLOBAL_OBJ as any).AsyncLocalStorage; + + if (!MaybeGlobalAsyncLocalStorage) { + DEBUG_BUILD && + logger.warn( + "Tried to register AsyncLocalStorage async context strategy in a runtime that doesn't support AsyncLocalStorage.", + ); + return cb(); + } + + if (!nextjsEscapedAsyncStorage) { + nextjsEscapedAsyncStorage = new MaybeGlobalAsyncLocalStorage(); + } + + if (nextjsEscapedAsyncStorage.getStore()) { + return cb(); + } else { + return withActiveSpan(null, () => { + getCurrentScope().setPropagationContext({ + traceId: uuid4(), + spanId: uuid4().substring(16), + }); + return cb(); + }); + } +} diff --git a/packages/nextjs/src/common/utils/withIsolationScopeOrReuseFromRootSpan.ts b/packages/nextjs/src/common/utils/withIsolationScopeOrReuseFromRootSpan.ts deleted file mode 100644 index 82231022b980..000000000000 --- a/packages/nextjs/src/common/utils/withIsolationScopeOrReuseFromRootSpan.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { - getActiveSpan, - getCapturedScopesOnSpan, - getDefaultIsolationScope, - getRootSpan, - spanToJSON, - withIsolationScope, -} from '@sentry/core'; -import type { Scope } from '@sentry/types'; - -/** - * Wrap a callback with a new isolation scope. - * However, if we have an active root span that was generated by next, we want to reuse the isolation scope from that span. - */ -export function withIsolationScopeOrReuseFromRootSpan(cb: (isolationScope: Scope) => T): T { - const activeSpan = getActiveSpan(); - - if (!activeSpan) { - return withIsolationScope(cb); - } - - const rootSpan = getRootSpan(activeSpan); - - // Verify this is a next span - if (!spanToJSON(rootSpan).data?.['next.route']) { - return withIsolationScope(cb); - } - - const scopes = getCapturedScopesOnSpan(rootSpan); - - const isolationScope = scopes.isolationScope; - - // If this is the default isolation scope, we still want to fork one - if (isolationScope === getDefaultIsolationScope()) { - return withIsolationScope(cb); - } - - return withIsolationScope(isolationScope, cb); -} diff --git a/packages/nextjs/src/common/utils/wrapperUtils.ts b/packages/nextjs/src/common/utils/wrapperUtils.ts index ef14e19b02fa..20ea3a80bad9 100644 --- a/packages/nextjs/src/common/utils/wrapperUtils.ts +++ b/packages/nextjs/src/common/utils/wrapperUtils.ts @@ -10,13 +10,14 @@ import { startSpan, startSpanManual, withActiveSpan, + withIsolationScope, } from '@sentry/core'; import type { Span } from '@sentry/types'; import { isString } from '@sentry/utils'; import { platformSupportsStreaming } from './platformSupportsStreaming'; import { autoEndSpanOnResponseEnd, flushQueue } from './responseEnd'; -import { withIsolationScopeOrReuseFromRootSpan } from './withIsolationScopeOrReuseFromRootSpan'; +import { commonObjectToIsolationScope } from './tracingUtils'; declare module 'http' { interface IncomingMessage { @@ -89,7 +90,8 @@ export function withTracedServerSideDataFetcher Pr }, ): (...params: Parameters) => Promise> { return async function (this: unknown, ...args: Parameters): Promise> { - return withIsolationScopeOrReuseFromRootSpan(async isolationScope => { + const isolationScope = commonObjectToIsolationScope(req); + return withIsolationScope(isolationScope, () => { isolationScope.setSDKProcessingMetadata({ request: req, }); @@ -100,7 +102,6 @@ export function withTracedServerSideDataFetcher Pr return continueTrace({ sentryTrace, baggage }, () => { const requestSpan = getOrStartRequestSpan(req, res, options.requestedRouteName); - return withActiveSpan(requestSpan, () => { return startSpanManual( { diff --git a/packages/nextjs/src/common/withServerActionInstrumentation.ts b/packages/nextjs/src/common/withServerActionInstrumentation.ts index 83bdc9b7bbd3..a97d8fc17888 100644 --- a/packages/nextjs/src/common/withServerActionInstrumentation.ts +++ b/packages/nextjs/src/common/withServerActionInstrumentation.ts @@ -1,4 +1,9 @@ -import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SPAN_STATUS_ERROR, getIsolationScope } from '@sentry/core'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + SPAN_STATUS_ERROR, + getIsolationScope, + withIsolationScope, +} from '@sentry/core'; import { captureException, continueTrace, getClient, handleCallbackErrors, startSpan } from '@sentry/core'; import { logger } from '@sentry/utils'; @@ -6,11 +11,11 @@ import { DEBUG_BUILD } from './debug-build'; import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { flushQueue } from './utils/responseEnd'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; +import { escapeNextjsTracing } from './utils/tracingUtils'; interface Options { formData?: FormData; - // TODO: Whenever we decide to drop support for Next.js <= 12 we can automatically pick up the headers becauase "next/headers" will be resolvable. + // TODO(v8): Whenever we decide to drop support for Next.js <= 12 we can automatically pick up the headers becauase "next/headers" will be resolvable. headers?: Headers; recordResponse?: boolean; } @@ -50,93 +55,95 @@ async function withServerActionInstrumentationImplementation> { - return withIsolationScopeOrReuseFromRootSpan(isolationScope => { - const sendDefaultPii = getClient()?.getOptions().sendDefaultPii; + return escapeNextjsTracing(() => { + return withIsolationScope(isolationScope => { + const sendDefaultPii = getClient()?.getOptions().sendDefaultPii; - let sentryTraceHeader; - let baggageHeader; - const fullHeadersObject: Record = {}; - try { - sentryTraceHeader = options.headers?.get('sentry-trace') ?? undefined; - baggageHeader = options.headers?.get('baggage'); - options.headers?.forEach((value, key) => { - fullHeadersObject[key] = value; - }); - } catch (e) { - DEBUG_BUILD && - logger.warn( - "Sentry wasn't able to extract the tracing headers for a server action. Will not trace this request.", - ); - } + let sentryTraceHeader; + let baggageHeader; + const fullHeadersObject: Record = {}; + try { + sentryTraceHeader = options.headers?.get('sentry-trace') ?? undefined; + baggageHeader = options.headers?.get('baggage'); + options.headers?.forEach((value, key) => { + fullHeadersObject[key] = value; + }); + } catch (e) { + DEBUG_BUILD && + logger.warn( + "Sentry wasn't able to extract the tracing headers for a server action. Will not trace this request.", + ); + } - isolationScope.setSDKProcessingMetadata({ - request: { - headers: fullHeadersObject, - }, - }); + isolationScope.setSDKProcessingMetadata({ + request: { + headers: fullHeadersObject, + }, + }); - return continueTrace( - { - sentryTrace: sentryTraceHeader, - baggage: baggageHeader, - }, - async () => { - try { - return await startSpan( - { - op: 'function.server_action', - name: `serverAction/${serverActionName}`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + return continueTrace( + { + sentryTrace: sentryTraceHeader, + baggage: baggageHeader, + }, + async () => { + try { + return await startSpan( + { + op: 'function.server_action', + name: `serverAction/${serverActionName}`, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + }, }, - }, - async span => { - const result = await handleCallbackErrors(callback, error => { - if (isNotFoundNavigationError(error)) { - // We don't want to report "not-found"s - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); - } else if (isRedirectNavigationError(error)) { - // Don't do anything for redirects - } else { - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(error, { - mechanism: { - handled: false, - }, - }); - } - }); + async span => { + const result = await handleCallbackErrors(callback, error => { + if (isNotFoundNavigationError(error)) { + // We don't want to report "not-found"s + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + } else if (isRedirectNavigationError(error)) { + // Don't do anything for redirects + } else { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureException(error, { + mechanism: { + handled: false, + }, + }); + } + }); - if (options.recordResponse !== undefined ? options.recordResponse : sendDefaultPii) { - getIsolationScope().setExtra('server_action_result', result); - } + if (options.recordResponse !== undefined ? options.recordResponse : sendDefaultPii) { + getIsolationScope().setExtra('server_action_result', result); + } - if (options.formData) { - options.formData.forEach((value, key) => { - getIsolationScope().setExtra( - `server_action_form_data.${key}`, - typeof value === 'string' ? value : '[non-string value]', - ); - }); - } + if (options.formData) { + options.formData.forEach((value, key) => { + getIsolationScope().setExtra( + `server_action_form_data.${key}`, + typeof value === 'string' ? value : '[non-string value]', + ); + }); + } - return result; - }, - ); - } finally { - if (!platformSupportsStreaming()) { - // Lambdas require manual flushing to prevent execution freeze before the event is sent - await flushQueue(); - } + return result; + }, + ); + } finally { + if (!platformSupportsStreaming()) { + // Lambdas require manual flushing to prevent execution freeze before the event is sent + await flushQueue(); + } - if (process.env.NEXT_RUNTIME === 'edge') { - // flushQueue should not throw - // eslint-disable-next-line @typescript-eslint/no-floating-promises - flushQueue(); + if (process.env.NEXT_RUNTIME === 'edge') { + // flushQueue should not throw + // eslint-disable-next-line @typescript-eslint/no-floating-promises + flushQueue(); + } } - } - }, - ); + }, + ); + }); }); } diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts index 37d0b83c7c5b..bdfd3c27d896 100644 --- a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts @@ -4,6 +4,7 @@ import { continueTrace, setHttpStatus, startSpanManual, + withIsolationScope, } from '@sentry/core'; import { consoleSandbox, isString, logger, objectify, stripUrlQueryAndFragment } from '@sentry/utils'; @@ -11,7 +12,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import type { AugmentedNextApiRequest, AugmentedNextApiResponse, NextApiHandler } from './types'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { flushQueue } from './utils/responseEnd'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; +import { commonObjectToIsolationScope, escapeNextjsTracing } from './utils/tracingUtils'; /** * Wrap the given API route handler for tracing and error capturing. Thin wrapper around `withSentry`, which only @@ -29,146 +30,150 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz thisArg, args: [AugmentedNextApiRequest | undefined, AugmentedNextApiResponse | undefined], ) => { - const [req, res] = args; - - if (!req) { - logger.debug( - `Wrapped API handler on route "${parameterizedRoute}" was not passed a request object. Will not instrument.`, - ); - return wrappingTarget.apply(thisArg, args); - } else if (!res) { - logger.debug( - `Wrapped API handler on route "${parameterizedRoute}" was not passed a response object. Will not instrument.`, - ); - return wrappingTarget.apply(thisArg, args); - } - - // We're now auto-wrapping API route handlers using `wrapApiHandlerWithSentry` (which uses `withSentry` under the hood), but - // users still may have their routes manually wrapped with `withSentry`. This check makes `sentryWrappedHandler` - // idempotent so that those cases don't break anything. - if (req.__withSentry_applied__) { - return wrappingTarget.apply(thisArg, args); - } - req.__withSentry_applied__ = true; - - return withIsolationScopeOrReuseFromRootSpan(isolationScope => { - return continueTrace( - { - // TODO(v8): Make it so that continue trace will allow null as sentryTrace value and remove this fallback here - sentryTrace: req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined, - baggage: req.headers?.baggage, - }, - () => { - // prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler) - let reqPath = parameterizedRoute; - - // If not, fake it by just replacing parameter values with their names, hoping that none of them match either - // each other or any hard-coded parts of the path - if (!reqPath) { - const url = `${req.url}`; - // pull off query string, if any - reqPath = stripUrlQueryAndFragment(url); - // Replace with placeholder - if (req.query) { - for (const [key, value] of Object.entries(req.query)) { - reqPath = reqPath.replace(`${value}`, `[${key}]`); + return escapeNextjsTracing(() => { + const [req, res] = args; + + if (!req) { + logger.debug( + `Wrapped API handler on route "${parameterizedRoute}" was not passed a request object. Will not instrument.`, + ); + return wrappingTarget.apply(thisArg, args); + } else if (!res) { + logger.debug( + `Wrapped API handler on route "${parameterizedRoute}" was not passed a response object. Will not instrument.`, + ); + return wrappingTarget.apply(thisArg, args); + } + + // We're now auto-wrapping API route handlers using `wrapApiHandlerWithSentry` (which uses `withSentry` under the hood), but + // users still may have their routes manually wrapped with `withSentry`. This check makes `sentryWrappedHandler` + // idempotent so that those cases don't break anything. + if (req.__withSentry_applied__) { + return wrappingTarget.apply(thisArg, args); + } + req.__withSentry_applied__ = true; + + const isolationScope = commonObjectToIsolationScope(req); + isolationScope.setSDKProcessingMetadata({ request: req }); + + return withIsolationScope(isolationScope, () => { + return continueTrace( + { + // TODO(v8): Make it so that continue trace will allow null as sentryTrace value and remove this fallback here + sentryTrace: + req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined, + baggage: req.headers?.baggage, + }, + () => { + // prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler) + let reqPath = parameterizedRoute; + + // If not, fake it by just replacing parameter values with their names, hoping that none of them match either + // each other or any hard-coded parts of the path + if (!reqPath) { + const url = `${req.url}`; + // pull off query string, if any + reqPath = stripUrlQueryAndFragment(url); + // Replace with placeholder + if (req.query) { + for (const [key, value] of Object.entries(req.query)) { + reqPath = reqPath.replace(`${value}`, `[${key}]`); + } } } - } - - const reqMethod = `${(req.method || 'GET').toUpperCase()} `; - isolationScope.setSDKProcessingMetadata({ request: req }); + const reqMethod = `${(req.method || 'GET').toUpperCase()} `; - return startSpanManual( - { - name: `${reqMethod}${reqPath}`, - op: 'http.server', - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs', + return startSpanManual( + { + name: `${reqMethod}${reqPath}`, + op: 'http.server', + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs', + }, }, - }, - 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(); - if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { - target.apply(thisArg, argArray); - } else { - // flushQueue will not reject - // eslint-disable-next-line @typescript-eslint/no-floating-promises - flushQueue().then(() => { + 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(); + if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { target.apply(thisArg, argArray); + } else { + // flushQueue will not reject + // eslint-disable-next-line @typescript-eslint/no-floating-promises + flushQueue().then(() => { + target.apply(thisArg, argArray); + }); + } + }, + }); + + try { + const handlerResult = await wrappingTarget.apply(thisArg, args); + if ( + process.env.NODE_ENV === 'development' && + !process.env.SENTRY_IGNORE_API_RESOLUTION_ERROR && + !res.writableEnded + ) { + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.warn( + '[sentry] If Next.js logs a warning "API resolved without sending a response", it\'s a false positive, which may happen when you use `wrapApiHandlerWithSentry` manually to wrap your routes. To suppress this warning, set `SENTRY_IGNORE_API_RESOLUTION_ERROR` to 1 in your env. To suppress the nextjs warning, use the `externalResolver` API route option (see https://nextjs.org/docs/api-routes/api-middlewares#custom-config for details).', + ); }); } - }, - }); - - try { - const handlerResult = await wrappingTarget.apply(thisArg, args); - if ( - process.env.NODE_ENV === 'development' && - !process.env.SENTRY_IGNORE_API_RESOLUTION_ERROR && - !res.writableEnded - ) { - consoleSandbox(() => { - // eslint-disable-next-line no-console - console.warn( - '[sentry] If Next.js logs a warning "API resolved without sending a response", it\'s a false positive, which may happen when you use `wrapApiHandlerWithSentry` manually to wrap your routes. To suppress this warning, set `SENTRY_IGNORE_API_RESOLUTION_ERROR` to 1 in your env. To suppress the nextjs warning, use the `externalResolver` API route option (see https://nextjs.org/docs/api-routes/api-middlewares#custom-config for details).', - ); - }); - } - return handlerResult; - } 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); - - captureException(objectifiedErr, { - mechanism: { - type: 'instrument', - handled: false, - data: { - wrapped_handler: wrappingTarget.name, - function: 'withSentry', + return handlerResult; + } 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); + + captureException(objectifiedErr, { + mechanism: { + type: 'instrument', + handled: false, + data: { + wrapped_handler: wrappingTarget.name, + function: 'withSentry', + }, }, - }, - }); + }); - // Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet - // have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that - // the transaction was error-free - res.statusCode = 500; - res.statusMessage = 'Internal Server Error'; - - setHttpStatus(span, res.statusCode); - span.end(); - - // Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors - // out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the - // moment they detect an error, so it's important to get this done before rethrowing the error. Apps not - // deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already - // be finished and the queue will already be empty, so effectively it'll just no-op.) - if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { - await flushQueue(); - } + // Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet + // have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that + // the transaction was error-free + res.statusCode = 500; + res.statusMessage = 'Internal Server Error'; - // 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; - } - }, - ); - }, - ); + setHttpStatus(span, res.statusCode); + span.end(); + + // Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors + // out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the + // moment they detect an error, so it's important to get this done before rethrowing the error. Apps not + // deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already + // be finished and the queue will already be empty, so effectively it'll just no-op.) + if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { + await flushQueue(); + } + + // 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; + } + }, + ); + }, + ); + }); }); }, }); diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts index 637f602b52ec..84889518e020 100644 --- a/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts +++ b/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts @@ -1,8 +1,8 @@ -import { captureCheckIn } from '@sentry/core'; +import { captureCheckIn, withIsolationScope } from '@sentry/core'; import type { NextApiRequest } from 'next'; import type { VercelCronsConfig } from './types'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; +import { escapeNextjsTracing } from './utils/tracingUtils'; type EdgeRequest = { nextUrl: URL; @@ -20,91 +20,93 @@ export function wrapApiHandlerWithSentryVercelCrons { - return withIsolationScopeOrReuseFromRootSpan(() => { - if (!args || !args[0]) { - return originalFunction.apply(thisArg, args); - } + return escapeNextjsTracing(() => { + return withIsolationScope(() => { + if (!args || !args[0]) { + return originalFunction.apply(thisArg, args); + } - const [req] = args as [NextApiRequest | EdgeRequest]; + const [req] = args as [NextApiRequest | EdgeRequest]; - let maybePromiseResult; - const cronsKey = 'nextUrl' in req ? req.nextUrl.pathname : req.url; - const userAgentHeader = 'nextUrl' in req ? req.headers.get('user-agent') : req.headers['user-agent']; + let maybePromiseResult; + const cronsKey = 'nextUrl' in req ? req.nextUrl.pathname : req.url; + const userAgentHeader = 'nextUrl' in req ? req.headers.get('user-agent') : req.headers['user-agent']; - if ( - !vercelCronsConfig || // do nothing if vercel crons config is missing - !userAgentHeader?.includes('vercel-cron') // do nothing if endpoint is not called from vercel crons - ) { - return originalFunction.apply(thisArg, args); - } + if ( + !vercelCronsConfig || // do nothing if vercel crons config is missing + !userAgentHeader?.includes('vercel-cron') // do nothing if endpoint is not called from vercel crons + ) { + return originalFunction.apply(thisArg, args); + } - const vercelCron = vercelCronsConfig.find(vercelCron => vercelCron.path === cronsKey); + const vercelCron = vercelCronsConfig.find(vercelCron => vercelCron.path === cronsKey); - if (!vercelCron || !vercelCron.path || !vercelCron.schedule) { - return originalFunction.apply(thisArg, args); - } + if (!vercelCron || !vercelCron.path || !vercelCron.schedule) { + return originalFunction.apply(thisArg, args); + } - const monitorSlug = vercelCron.path; + const monitorSlug = vercelCron.path; - const checkInId = captureCheckIn( - { - monitorSlug, - status: 'in_progress', - }, - { - maxRuntime: 60 * 12, // (minutes) so 12 hours - just a very high arbitrary number since we don't know the actual duration of the users cron job - schedule: { - type: 'crontab', - value: vercelCron.schedule, + const checkInId = captureCheckIn( + { + monitorSlug, + status: 'in_progress', }, - }, - ); + { + maxRuntime: 60 * 12, // (minutes) so 12 hours - just a very high arbitrary number since we don't know the actual duration of the users cron job + schedule: { + type: 'crontab', + value: vercelCron.schedule, + }, + }, + ); - const startTime = Date.now() / 1000; + const startTime = Date.now() / 1000; - const handleErrorCase = (): void => { - captureCheckIn({ - checkInId, - monitorSlug, - status: 'error', - duration: Date.now() / 1000 - startTime, - }); - }; + const handleErrorCase = (): void => { + captureCheckIn({ + checkInId, + monitorSlug, + status: 'error', + duration: Date.now() / 1000 - startTime, + }); + }; - try { - maybePromiseResult = originalFunction.apply(thisArg, args); - } catch (e) { - handleErrorCase(); - throw e; - } + try { + maybePromiseResult = originalFunction.apply(thisArg, args); + } catch (e) { + handleErrorCase(); + throw e; + } - if (typeof maybePromiseResult === 'object' && maybePromiseResult !== null && 'then' in maybePromiseResult) { - Promise.resolve(maybePromiseResult).then( - () => { - captureCheckIn({ - checkInId, - monitorSlug, - status: 'ok', - duration: Date.now() / 1000 - startTime, - }); - }, - () => { - handleErrorCase(); - }, - ); + if (typeof maybePromiseResult === 'object' && maybePromiseResult !== null && 'then' in maybePromiseResult) { + Promise.resolve(maybePromiseResult).then( + () => { + captureCheckIn({ + checkInId, + monitorSlug, + status: 'ok', + duration: Date.now() / 1000 - startTime, + }); + }, + () => { + handleErrorCase(); + }, + ); - // It is very important that we return the original promise here, because Next.js attaches various properties - // to that promise and will throw if they are not on the returned value. - return maybePromiseResult; - } else { - captureCheckIn({ - checkInId, - monitorSlug, - status: 'ok', - duration: Date.now() / 1000 - startTime, - }); - return maybePromiseResult; - } + // It is very important that we return the original promise here, because Next.js attaches various properties + // to that promise and will throw if they are not on the returned value. + return maybePromiseResult; + } else { + captureCheckIn({ + checkInId, + monitorSlug, + status: 'ok', + duration: Date.now() / 1000 - startTime, + }); + return maybePromiseResult; + } + }); }); }, }); diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index 9931e856e12f..d0e0573109a8 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -7,6 +7,7 @@ import { getCurrentScope, handleCallbackErrors, startSpanManual, + withIsolationScope, } from '@sentry/core'; import type { WebFetchHeaders } from '@sentry/types'; import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils'; @@ -14,8 +15,11 @@ import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/ut import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import type { GenerationFunctionContext } from '../common/types'; import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils'; -import { commonObjectToPropagationContext } from './utils/commonObjectTracing'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; +import { + commonObjectToIsolationScope, + commonObjectToPropagationContext, + escapeNextjsTracing, +} from './utils/tracingUtils'; /** * Wraps a generation function (e.g. generateMetadata) with Sentry error and performance instrumentation. @@ -28,75 +32,78 @@ export function wrapGenerationFunctionWithSentry a const { requestAsyncStorage, componentRoute, componentType, generationFunctionIdentifier } = context; return new Proxy(generationFunction, { apply: (originalFunction, thisArg, args) => { - let headers: WebFetchHeaders | undefined = undefined; - // We try-catch here just in case anything goes wrong with the async storage here goes wrong since it is Next.js internal API - try { - headers = requestAsyncStorage?.getStore()?.headers; - } catch (e) { - /** empty */ - } - - let data: Record | undefined = undefined; - if (getClient()?.getOptions().sendDefaultPii) { - const props: unknown = args[0]; - const params = props && typeof props === 'object' && 'params' in props ? props.params : undefined; - const searchParams = - props && typeof props === 'object' && 'searchParams' in props ? props.searchParams : undefined; - data = { params, searchParams }; - } + return escapeNextjsTracing(() => { + let headers: WebFetchHeaders | undefined = undefined; + // We try-catch here just in case anything goes wrong with the async storage here goes wrong since it is Next.js internal API + try { + headers = requestAsyncStorage?.getStore()?.headers; + } catch (e) { + /** empty */ + } - return withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setSDKProcessingMetadata({ - request: { - headers: headers ? winterCGHeadersToDict(headers) : undefined, - }, - }); - isolationScope.setExtra('route_data', data); + let data: Record | undefined = undefined; + if (getClient()?.getOptions().sendDefaultPii) { + const props: unknown = args[0]; + const params = props && typeof props === 'object' && 'params' in props ? props.params : undefined; + const searchParams = + props && typeof props === 'object' && 'searchParams' in props ? props.searchParams : undefined; + data = { params, searchParams }; + } const incomingPropagationContext = propagationContextFromHeaders( headers?.get('sentry-trace') ?? undefined, headers?.get('baggage'), ); + const isolationScope = commonObjectToIsolationScope(headers); const propagationContext = commonObjectToPropagationContext(headers, incomingPropagationContext); - isolationScope.setPropagationContext(propagationContext); - getCurrentScope().setPropagationContext(propagationContext); - return startSpanManual( - { - op: 'function.nextjs', - name: `${componentType}.${generationFunctionIdentifier} (${componentRoute})`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', + return withIsolationScope(isolationScope, () => { + isolationScope.setSDKProcessingMetadata({ + request: { + headers: headers ? winterCGHeadersToDict(headers) : undefined, }, - }, - span => { - return handleCallbackErrors( - () => originalFunction.apply(thisArg, args), - err => { - if (isNotFoundNavigationError(err)) { - // We don't want to report "not-found"s - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); - } else if (isRedirectNavigationError(err)) { - // We don't want to report redirects - span.setStatus({ code: SPAN_STATUS_OK }); - } else { - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(err, { - mechanism: { - handled: false, - }, - }); - } - }, - () => { - span.end(); + }); + + getCurrentScope().setExtra('route_data', data); + getCurrentScope().setPropagationContext(propagationContext); + + return startSpanManual( + { + op: 'function.nextjs', + name: `${componentType}.${generationFunctionIdentifier} (${componentRoute})`, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', }, - ); - }, - ); + }, + span => { + return handleCallbackErrors( + () => originalFunction.apply(thisArg, args), + err => { + if (isNotFoundNavigationError(err)) { + // We don't want to report "not-found"s + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + } else if (isRedirectNavigationError(err)) { + // We don't want to report redirects + span.setStatus({ code: SPAN_STATUS_OK }); + } else { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureException(err, { + mechanism: { + handled: false, + }, + }); + } + }, + () => { + span.end(); + }, + ); + }, + ); + }); }); }, }); diff --git a/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts b/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts index 25b0e2b6d5d4..66cbbb046300 100644 --- a/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts +++ b/packages/nextjs/src/common/wrapMiddlewareWithSentry.ts @@ -1,6 +1,5 @@ import type { EdgeRouteHandler } from '../edge/types'; import { withEdgeWrapping } from './utils/edgeWrapperUtils'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; /** * Wraps Next.js middleware with Sentry error and performance instrumentation. @@ -13,13 +12,11 @@ export function wrapMiddlewareWithSentry( ): (...params: Parameters) => Promise> { return new Proxy(middleware, { apply: (wrappingTarget, thisArg, args: Parameters) => { - return withIsolationScopeOrReuseFromRootSpan(() => { - return withEdgeWrapping(wrappingTarget, { - spanDescription: 'middleware', - spanOp: 'middleware.nextjs', - mechanismFunctionName: 'withSentryMiddleware', - }).apply(thisArg, args); - }); + return withEdgeWrapping(wrappingTarget, { + spanDescription: 'middleware', + spanOp: 'middleware.nextjs', + mechanismFunctionName: 'withSentryMiddleware', + }).apply(thisArg, args); }, }); } diff --git a/packages/nextjs/src/common/wrapPageComponentWithSentry.ts b/packages/nextjs/src/common/wrapPageComponentWithSentry.ts index b6ab022afb21..8cd4a250ac14 100644 --- a/packages/nextjs/src/common/wrapPageComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapPageComponentWithSentry.ts @@ -1,6 +1,6 @@ -import { captureException, getCurrentScope } from '@sentry/core'; +import { captureException, getCurrentScope, withIsolationScope } from '@sentry/core'; import { extractTraceparentData } from '@sentry/utils'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; +import { escapeNextjsTracing } from './utils/tracingUtils'; interface FunctionComponent { (...args: unknown[]): unknown; @@ -25,64 +25,68 @@ export function wrapPageComponentWithSentry(pageComponent: FunctionComponent | C if (isReactClassComponent(pageComponent)) { return class SentryWrappedPageComponent extends pageComponent { public render(...args: unknown[]): unknown { - return withIsolationScopeOrReuseFromRootSpan(() => { - const scope = getCurrentScope(); - // We extract the sentry trace data that is put in the component props by datafetcher wrappers - const sentryTraceData = - typeof this.props === 'object' && - this.props !== null && - '_sentryTraceData' in this.props && - typeof this.props._sentryTraceData === 'string' - ? this.props._sentryTraceData - : undefined; + return escapeNextjsTracing(() => { + return withIsolationScope(() => { + const scope = getCurrentScope(); + // We extract the sentry trace data that is put in the component props by datafetcher wrappers + const sentryTraceData = + typeof this.props === 'object' && + this.props !== null && + '_sentryTraceData' in this.props && + typeof this.props._sentryTraceData === 'string' + ? this.props._sentryTraceData + : undefined; - if (sentryTraceData) { - const traceparentData = extractTraceparentData(sentryTraceData); - scope.setContext('trace', { - span_id: traceparentData?.parentSpanId, - trace_id: traceparentData?.traceId, - }); - } + if (sentryTraceData) { + const traceparentData = extractTraceparentData(sentryTraceData); + scope.setContext('trace', { + span_id: traceparentData?.parentSpanId, + trace_id: traceparentData?.traceId, + }); + } - try { - return super.render(...args); - } catch (e) { - captureException(e, { - mechanism: { - handled: false, - }, - }); - throw e; - } + try { + return super.render(...args); + } catch (e) { + captureException(e, { + mechanism: { + handled: false, + }, + }); + throw e; + } + }); }); } }; } else if (typeof pageComponent === 'function') { return new Proxy(pageComponent, { apply(target, thisArg, argArray: [{ _sentryTraceData?: string } | undefined]) { - return withIsolationScopeOrReuseFromRootSpan(() => { - const scope = getCurrentScope(); - // We extract the sentry trace data that is put in the component props by datafetcher wrappers - const sentryTraceData = argArray?.[0]?._sentryTraceData; + return escapeNextjsTracing(() => { + return withIsolationScope(() => { + const scope = getCurrentScope(); + // We extract the sentry trace data that is put in the component props by datafetcher wrappers + const sentryTraceData = argArray?.[0]?._sentryTraceData; - if (sentryTraceData) { - const traceparentData = extractTraceparentData(sentryTraceData); - scope.setContext('trace', { - span_id: traceparentData?.parentSpanId, - trace_id: traceparentData?.traceId, - }); - } + if (sentryTraceData) { + const traceparentData = extractTraceparentData(sentryTraceData); + scope.setContext('trace', { + span_id: traceparentData?.parentSpanId, + trace_id: traceparentData?.traceId, + }); + } - try { - return target.apply(thisArg, argArray); - } catch (e) { - captureException(e, { - mechanism: { - handled: false, - }, - }); - throw e; - } + try { + return target.apply(thisArg, argArray); + } catch (e) { + captureException(e, { + mechanism: { + handled: false, + }, + }); + throw e; + } + }); }); }, }); diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 1dadd72e3f43..ec074bb3075e 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -4,63 +4,22 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SPAN_STATUS_ERROR, captureException, - getActiveSpan, - getRootSpan, + getCurrentScope, handleCallbackErrors, setHttpStatus, - spanToJSON, startSpan, + withIsolationScope, } from '@sentry/core'; -import type { Span } from '@sentry/types'; -import { winterCGHeadersToDict } from '@sentry/utils'; +import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils'; import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils'; import type { RouteHandlerContext } from './types'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { flushQueue } from './utils/responseEnd'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; - -/** As our own HTTP integration is disabled (src/server/index.ts) the rootSpan comes from Next.js. - * In case there is no root span, we start a new span. */ -function startOrUpdateSpan(spanName: string, cb: (rootSpan: Span) => Promise): Promise { - const activeSpan = getActiveSpan(); - const rootSpan = activeSpan && getRootSpan(activeSpan); - - // We have different possible scenarios here: - // 1. If we have no root span, we just create a new span - // 2. We have a root span that that we want to update here - // 3. We have a root span that was already updated (e.g. if this is a nested call) - - const attributes = { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', - } as const; - - if (!rootSpan) { - return startSpan( - { - name: spanName, - forceTransaction: true, - attributes, - }, - cb, - ); - } - - // If `op` is set, we assume this was already processed before - // Probably this is a nested call, no need to update anything anymore - // OR, if we don't have next.span_type, we don't know where this comes from and don't want to mess with it - const existingAttributes = spanToJSON(rootSpan).data || {}; - if (existingAttributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !existingAttributes['next.span_type']) { - return cb(rootSpan); - } - - // Finally, we want to update the root span, as the ones generated by next are often not good enough for us - rootSpan.updateName(spanName); - rootSpan.setAttributes(attributes); - - return cb(rootSpan); -} +import { + commonObjectToIsolationScope, + commonObjectToPropagationContext, + escapeNextjsTracing, +} from './utils/tracingUtils'; /** * Wraps a Next.js route handler with performance and error instrumentation. @@ -74,50 +33,75 @@ export function wrapRouteHandlerWithSentry any>( return new Proxy(routeHandler, { apply: (originalFunction, thisArg, args) => { - return withIsolationScopeOrReuseFromRootSpan(async isolationScope => { + return escapeNextjsTracing(() => { + const isolationScope = commonObjectToIsolationScope(headers); + + const completeHeadersDict: Record = headers ? winterCGHeadersToDict(headers) : {}; + isolationScope.setSDKProcessingMetadata({ request: { - headers: headers ? winterCGHeadersToDict(headers) : undefined, + headers: completeHeadersDict, }, }); - try { - return await startOrUpdateSpan(`${method} ${parameterizedRoute}`, async (rootSpan: Span) => { - const response: Response = await handleCallbackErrors( - () => originalFunction.apply(thisArg, args), - error => { - // Next.js throws errors when calling `redirect()`. We don't wanna report these. - if (isRedirectNavigationError(error)) { - // Don't do anything - } else if (isNotFoundNavigationError(error) && rootSpan) { - rootSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); - } else { - captureException(error, { - mechanism: { - handled: false, - }, - }); + const incomingPropagationContext = propagationContextFromHeaders( + completeHeadersDict['sentry-trace'], + completeHeadersDict['baggage'], + ); + + const propagationContext = commonObjectToPropagationContext(headers, incomingPropagationContext); + + return withIsolationScope(isolationScope, async () => { + getCurrentScope().setPropagationContext(propagationContext); + try { + return startSpan( + { + name: `${method} ${parameterizedRoute}`, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', + }, + forceTransaction: true, + }, + async span => { + const response: Response = await handleCallbackErrors( + () => originalFunction.apply(thisArg, args), + error => { + // Next.js throws errors when calling `redirect()`. We don't wanna report these. + if (isRedirectNavigationError(error)) { + // Don't do anything + } else if (isNotFoundNavigationError(error) && span) { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + } else { + captureException(error, { + mechanism: { + handled: false, + }, + }); + } + }, + ); + + try { + if (span && response.status) { + setHttpStatus(span, response.status); + } + } catch { + // best effort - response may be undefined? } + + return response; }, ); - - try { - if (rootSpan && response.status) { - setHttpStatus(rootSpan, response.status); - } - } catch { - // best effort - response may be undefined? + } finally { + if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') { + // 1. Edge transport requires manual flushing + // 2. Lambdas require manual flushing to prevent execution freeze before the event is sent + await flushQueue(); } - - return response; - }); - } finally { - if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') { - // 1. Edge transport requires manual flushing - // 2. Lambdas require manual flushing to prevent execution freeze before the event is sent - await flushQueue(); } - } + }); }); }, }); diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts index a41cfd2fb052..165754614a22 100644 --- a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts @@ -6,15 +6,19 @@ import { getCurrentScope, handleCallbackErrors, startSpanManual, + withIsolationScope, } from '@sentry/core'; import { propagationContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import { isNotFoundNavigationError, isRedirectNavigationError } from '../common/nextNavigationErrorUtils'; import type { ServerComponentContext } from '../common/types'; -import { commonObjectToPropagationContext } from './utils/commonObjectTracing'; import { flushQueue } from './utils/responseEnd'; -import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; +import { + commonObjectToIsolationScope, + commonObjectToPropagationContext, + escapeNextjsTracing, +} from './utils/tracingUtils'; /** * Wraps an `app` directory server component with Sentry error instrumentation. @@ -25,14 +29,14 @@ export function wrapServerComponentWithSentry any> context: ServerComponentContext, ): F { const { componentRoute, componentType } = context; - // Even though users may define server components as async functions, for the client bundles // Next.js will turn them into synchronous functions and it will transform any `await`s into instances of the `use` // hook. 🤯 return new Proxy(appDirComponent, { apply: (originalFunction, thisArg, args) => { - // TODO: If we ever allow withIsolationScope to take a scope, we should pass a scope here that is shared between all of the server components, similar to what `commonObjectToPropagationContext` does. - return withIsolationScopeOrReuseFromRootSpan(isolationScope => { + return escapeNextjsTracing(() => { + const isolationScope = commonObjectToIsolationScope(context.headers); + const completeHeadersDict: Record = context.headers ? winterCGHeadersToDict(context.headers) : {}; @@ -50,47 +54,48 @@ export function wrapServerComponentWithSentry any> const propagationContext = commonObjectToPropagationContext(context.headers, incomingPropagationContext); - getCurrentScope().setPropagationContext(propagationContext); - - return startSpanManual( - { - op: 'function.nextjs', - name: `${componentType} Server Component (${componentRoute})`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', - }, - }, - span => { - return handleCallbackErrors( - () => originalFunction.apply(thisArg, args), - error => { - if (isNotFoundNavigationError(error)) { - // We don't want to report "not-found"s - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); - } else if (isRedirectNavigationError(error)) { - // We don't want to report redirects - span.setStatus({ code: SPAN_STATUS_OK }); - } else { - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(error, { - mechanism: { - handled: false, - }, - }); - } + return withIsolationScope(isolationScope, () => { + getCurrentScope().setPropagationContext(propagationContext); + return startSpanManual( + { + op: 'function.nextjs', + name: `${componentType} Server Component (${componentRoute})`, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', }, - () => { - span.end(); + }, + span => { + return handleCallbackErrors( + () => originalFunction.apply(thisArg, args), + error => { + if (isNotFoundNavigationError(error)) { + // We don't want to report "not-found"s + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + } else if (isRedirectNavigationError(error)) { + // We don't want to report redirects + span.setStatus({ code: SPAN_STATUS_OK }); + } else { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + captureException(error, { + mechanism: { + handled: false, + }, + }); + } + }, + () => { + span.end(); - // flushQueue should not throw - // eslint-disable-next-line @typescript-eslint/no-floating-promises - flushQueue(); - }, - ); - }, - ); + // flushQueue should not throw + // eslint-disable-next-line @typescript-eslint/no-floating-promises + flushQueue(); + }, + ); + }, + ); + }); }); }, }); diff --git a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts index 717090905f6a..e5191ea27dbe 100644 --- a/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts @@ -1,7 +1,4 @@ -import { getActiveSpan } from '@sentry/vercel-edge'; - import { withEdgeWrapping } from '../common/utils/edgeWrapperUtils'; -import { withIsolationScopeOrReuseFromRootSpan } from '../common/utils/withIsolationScopeOrReuseFromRootSpan'; import type { EdgeRouteHandler } from './types'; /** @@ -15,20 +12,15 @@ export function wrapApiHandlerWithSentry( apply: (wrappingTarget, thisArg, args: Parameters) => { const req = args[0]; - const activeSpan = getActiveSpan(); - - return withIsolationScopeOrReuseFromRootSpan(() => { - const wrappedHandler = withEdgeWrapping(wrappingTarget, { - spanDescription: - activeSpan || !(req instanceof Request) - ? `handler (${parameterizedRoute})` - : `${req.method} ${parameterizedRoute}`, - spanOp: activeSpan ? 'function' : 'http.server', - mechanismFunctionName: 'wrapApiHandlerWithSentry', - }); - - return wrappedHandler.apply(thisArg, args); + const wrappedHandler = withEdgeWrapping(wrappingTarget, { + spanDescription: !(req instanceof Request) + ? `handler (${parameterizedRoute})` + : `${req.method} ${parameterizedRoute}`, + spanOp: 'http.server', + mechanismFunctionName: 'wrapApiHandlerWithSentry', }); + + return wrappedHandler.apply(thisArg, args); }, }); } diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 7d8f9a7a7d94..6895f1457727 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -11,7 +11,6 @@ import { distDirRewriteFramesIntegration } from './distDirRewriteFramesIntegrati export * from '@sentry/node'; import type { EventProcessor } from '@sentry/types'; -import { requestIsolationScopeIntegration } from './requestIsolationScopeIntegration'; export { captureUnderscoreErrorException } from '../common/_error'; @@ -73,10 +72,9 @@ export function init(options: NodeOptions): void { integration => // Next.js comes with its own Node-Fetch instrumentation, so we shouldn't add ours on-top integration.name !== 'NodeFetch' && - // Next.js comes with its own Http instrumentation for OTel which lead to double spans for route handler requests + // Next.js comes with its own Http instrumentation for OTel which would lead to double spans for route handler requests integration.name !== 'Http', ), - requestIsolationScopeIntegration(), ]; // This value is injected at build time, based on the output directory specified in the build config. Though a default @@ -147,6 +145,7 @@ export function init(options: NodeOptions): void { ), ); + // TODO(v8): Remove this because we have `suppressTracing` addEventProcessor( Object.assign( (event => { diff --git a/packages/nextjs/src/server/requestIsolationScopeIntegration.ts b/packages/nextjs/src/server/requestIsolationScopeIntegration.ts deleted file mode 100644 index b6020d891d8b..000000000000 --- a/packages/nextjs/src/server/requestIsolationScopeIntegration.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { SpanKind } from '@opentelemetry/api'; -import { - defineIntegration, - getCapturedScopesOnSpan, - getCurrentScope, - getIsolationScope, - getRootSpan, - setCapturedScopesOnSpan, - spanToJSON, -} from '@sentry/core'; -import { getSpanKind } from '@sentry/opentelemetry'; - -/** - * This integration is responsible for creating isolation scopes for incoming Http requests. - * We do so by waiting for http spans to be created and then forking the isolation scope. - * - * Normally the isolation scopes would be created by our Http instrumentation, however Next.js brings it's own Http - * instrumentation so we had to disable ours. - */ -export const requestIsolationScopeIntegration = defineIntegration(() => { - return { - name: 'RequestIsolationScope', - setup(client) { - client.on('spanStart', span => { - const spanJson = spanToJSON(span); - const data = spanJson.data || {}; - - // The following check is a heuristic to determine whether the started span is a span that tracks an incoming HTTP request - if ( - (getSpanKind(span) === SpanKind.SERVER && data['http.method']) || - (span === getRootSpan(span) && data['next.route']) - ) { - const scopes = getCapturedScopesOnSpan(span); - - // Update the isolation scope, isolate this request - const isolationScope = (scopes.isolationScope || getIsolationScope()).clone(); - const scope = scopes.scope || getCurrentScope(); - - setCapturedScopesOnSpan(span, scope, isolationScope); - } - }); - }, - }; -}); From a1fbc6885f405032613243bd3be23f78fee92d0b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 16 Apr 2024 11:31:04 +0000 Subject: [PATCH 2/9] tests --- .../create-next-app/pages/api/success.ts | 2 +- .../tests/behaviour-server.test.ts | 2 + .../src/common/utils/edgeWrapperUtils.ts | 102 +++++++++--------- 3 files changed, 54 insertions(+), 52 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/success.ts b/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/success.ts index 94f7b003ffcb..45ff2aac795e 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/success.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/success.ts @@ -3,7 +3,7 @@ import * as Sentry from '@sentry/nextjs'; import type { NextApiRequest, NextApiResponse } from 'next'; export default function handler(req: NextApiRequest, res: NextApiResponse) { - Sentry.startSpan({ name: 'test-span' }, span => undefined); + Sentry.startSpan({ name: 'test-span', forceTransaction: true }, span => undefined); Sentry.flush().then(() => { res.status(200).json({ diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-server.test.ts b/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-server.test.ts index becf9bf9bce7..1610ac549e7e 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-server.test.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-server.test.ts @@ -42,6 +42,8 @@ test('Sends server-side transactions to Sentry', async ({ baseURL }) => { const { data } = await axios.get(`${baseURL}/api/success`); const { transactionIds } = data; + expect(transactionIds.length).toBeGreaterThanOrEqual(1); + console.log(`Polling for transaction eventIds: ${JSON.stringify(transactionIds)}`); await Promise.all( diff --git a/packages/nextjs/src/common/utils/edgeWrapperUtils.ts b/packages/nextjs/src/common/utils/edgeWrapperUtils.ts index bf526c401d55..df12a99259fa 100644 --- a/packages/nextjs/src/common/utils/edgeWrapperUtils.ts +++ b/packages/nextjs/src/common/utils/edgeWrapperUtils.ts @@ -7,6 +7,7 @@ import { handleCallbackErrors, setHttpStatus, startSpan, + withIsolationScope, } from '@sentry/core'; import { winterCGRequestToRequestData } from '@sentry/utils'; @@ -24,64 +25,63 @@ export function withEdgeWrapping( return async function (this: unknown, ...args) { return escapeNextjsTracing(() => { const req: unknown = args[0]; + return withIsolationScope(commonObjectToIsolationScope(req), isolationScope => { + let sentryTrace; + let baggage; - let sentryTrace; - let baggage; + if (req instanceof Request) { + sentryTrace = req.headers.get('sentry-trace') || ''; + baggage = req.headers.get('baggage'); - const isolationScope = commonObjectToIsolationScope(req); + isolationScope.setSDKProcessingMetadata({ + request: winterCGRequestToRequestData(req), + }); + } - if (req instanceof Request) { - sentryTrace = req.headers.get('sentry-trace') || ''; - baggage = req.headers.get('baggage'); - - isolationScope.setSDKProcessingMetadata({ - request: winterCGRequestToRequestData(req), - }); - } - - return continueTrace( - { - sentryTrace, - baggage, - }, - () => { - return startSpan( - { - name: options.spanDescription, - op: options.spanOp, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping', + return continueTrace( + { + sentryTrace, + baggage, + }, + () => { + return startSpan( + { + name: options.spanDescription, + op: options.spanOp, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping', + }, }, - }, - async span => { - const handlerResult = await handleCallbackErrors( - () => handler.apply(this, args), - error => { - captureException(error, { - mechanism: { - type: 'instrument', - handled: false, - data: { - function: options.mechanismFunctionName, + async span => { + const handlerResult = await handleCallbackErrors( + () => handler.apply(this, args), + error => { + captureException(error, { + mechanism: { + type: 'instrument', + handled: false, + data: { + function: options.mechanismFunctionName, + }, }, - }, - }); - }, - ); + }); + }, + ); - if (handlerResult instanceof Response) { - setHttpStatus(span, handlerResult.status); - } else { - span.setStatus({ code: SPAN_STATUS_OK }); - } + if (handlerResult instanceof Response) { + setHttpStatus(span, handlerResult.status); + } else { + span.setStatus({ code: SPAN_STATUS_OK }); + } - return handlerResult; - }, - ).finally(() => flushQueue()); - }, - ); + return handlerResult; + }, + ).finally(() => flushQueue()); + }, + ); + }); }); }; } From d7e704ba5bc25f8ef69d6236cb042e42b7afbeeb Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 16 Apr 2024 12:01:43 +0000 Subject: [PATCH 3/9] test --- .../nextjs/test/config/withSentry.test.ts | 6 +- ...hIsolationScopeOrReuseFromRootSpan.test.ts | 96 ------------------- .../nextjs/test/edge/withSentryAPI.test.ts | 6 -- ...hIsolationScopeOrReuseFromRootSpan.test.ts | 96 ------------------- 4 files changed, 1 insertion(+), 203 deletions(-) delete mode 100644 packages/nextjs/test/edge/withIsolationScopeOrReuseFromRootSpan.test.ts delete mode 100644 packages/nextjs/test/server/withIsolationScopeOrReuseFromRootSpan.test.ts diff --git a/packages/nextjs/test/config/withSentry.test.ts b/packages/nextjs/test/config/withSentry.test.ts index 7a29ab80afb7..30f34634d9fd 100644 --- a/packages/nextjs/test/config/withSentry.test.ts +++ b/packages/nextjs/test/config/withSentry.test.ts @@ -36,7 +36,7 @@ describe('withSentry', () => { }); describe('tracing', () => { - it('starts a transaction and sets metadata when tracing is enabled', async () => { + it('starts a transaction when tracing is enabled', async () => { await wrappedHandlerNoError(req, res); expect(startSpanManualSpy).toHaveBeenCalledWith( expect.objectContaining({ @@ -49,10 +49,6 @@ describe('withSentry', () => { }), expect.any(Function), ); - - expect(SentryCore.getIsolationScope().getScopeData().sdkProcessingMetadata).toEqual({ - request: expect.objectContaining({ url: 'http://dogs.are.great' }), - }); }); }); }); diff --git a/packages/nextjs/test/edge/withIsolationScopeOrReuseFromRootSpan.test.ts b/packages/nextjs/test/edge/withIsolationScopeOrReuseFromRootSpan.test.ts deleted file mode 100644 index 8f42b8a9264b..000000000000 --- a/packages/nextjs/test/edge/withIsolationScopeOrReuseFromRootSpan.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { - Scope, - getCurrentScope, - getGlobalScope, - getIsolationScope, - setCapturedScopesOnSpan, - startSpan, -} from '@sentry/core'; -import { GLOBAL_OBJ } from '@sentry/utils'; -import { init } from '@sentry/vercel-edge'; -import { AsyncLocalStorage } from 'async_hooks'; - -import { withIsolationScopeOrReuseFromRootSpan } from '../../src/common/utils/withIsolationScopeOrReuseFromRootSpan'; - -describe('withIsolationScopeOrReuseFromRootSpan', () => { - beforeEach(() => { - getIsolationScope().clear(); - getCurrentScope().clear(); - getGlobalScope().clear(); - (GLOBAL_OBJ as any).AsyncLocalStorage = AsyncLocalStorage; - - init({ - enableTracing: true, - }); - }); - - it('works without any span', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).not.toBe(initialIsolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); - }); - }); - - it('works with a non-next.js span', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - const customScope = new Scope(); - - startSpan({ name: 'other' }, span => { - setCapturedScopesOnSpan(span, getCurrentScope(), customScope); - - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).not.toBe(initialIsolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); - }); - }); - }); - - it('works with a next.js span', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - const customScope = new Scope(); - - startSpan( - { - name: 'other', - attributes: { 'next.route': 'aha' }, - }, - span => { - setCapturedScopesOnSpan(span, getCurrentScope(), customScope); - - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).toBe(customScope); - expect(isolationScope.getScopeData().tags).toEqual({ bb: 'bb' }); - }); - }, - ); - }); - - it('works with a next.js span that has default isolation scope', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - startSpan( - { - name: 'other', - attributes: { 'next.route': 'aha' }, - }, - () => { - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).not.toBe(initialIsolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); - }); - }, - ); - }); -}); diff --git a/packages/nextjs/test/edge/withSentryAPI.test.ts b/packages/nextjs/test/edge/withSentryAPI.test.ts index 2b232c922968..6e24eca21bfe 100644 --- a/packages/nextjs/test/edge/withSentryAPI.test.ts +++ b/packages/nextjs/test/edge/withSentryAPI.test.ts @@ -58,10 +58,6 @@ describe('wrapApiHandlerWithSentry', () => { }), expect.any(Function), ); - - expect(coreSdk.getIsolationScope().getScopeData().sdkProcessingMetadata).toEqual({ - request: { headers: {}, method: 'POST', url: 'https://sentry.io/' }, - }); }); it('should return a function that calls trace without throwing when no request is passed', async () => { @@ -83,7 +79,5 @@ describe('wrapApiHandlerWithSentry', () => { }), expect.any(Function), ); - - expect(coreSdk.getIsolationScope().getScopeData().sdkProcessingMetadata).toEqual({}); }); }); diff --git a/packages/nextjs/test/server/withIsolationScopeOrReuseFromRootSpan.test.ts b/packages/nextjs/test/server/withIsolationScopeOrReuseFromRootSpan.test.ts deleted file mode 100644 index 0cfc53e0a5b2..000000000000 --- a/packages/nextjs/test/server/withIsolationScopeOrReuseFromRootSpan.test.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { - Scope, - getCurrentScope, - getGlobalScope, - getIsolationScope, - setCapturedScopesOnSpan, - startSpan, -} from '@sentry/core'; -import { init } from '@sentry/node'; -import { GLOBAL_OBJ } from '@sentry/utils'; -import { AsyncLocalStorage } from 'async_hooks'; - -import { withIsolationScopeOrReuseFromRootSpan } from '../../src/common/utils/withIsolationScopeOrReuseFromRootSpan'; - -describe('withIsolationScopeOrReuseFromRootSpan', () => { - beforeEach(() => { - getIsolationScope().clear(); - getCurrentScope().clear(); - getGlobalScope().clear(); - (GLOBAL_OBJ as any).AsyncLocalStorage = AsyncLocalStorage; - - init({ - enableTracing: true, - }); - }); - - it('works without any span', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).not.toBe(initialIsolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); - }); - }); - - it('works with a non-next.js span', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - const customScope = new Scope(); - - startSpan({ name: 'other' }, span => { - setCapturedScopesOnSpan(span, getCurrentScope(), customScope); - - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).not.toBe(initialIsolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); - }); - }); - }); - - it('works with a next.js span', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - const customScope = new Scope(); - - startSpan( - { - name: 'other', - attributes: { 'next.route': 'aha' }, - }, - span => { - setCapturedScopesOnSpan(span, getCurrentScope(), customScope); - - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).toBe(customScope); - expect(isolationScope.getScopeData().tags).toEqual({ bb: 'bb' }); - }); - }, - ); - }); - - it('works with a next.js span that has default isolation scope', () => { - const initialIsolationScope = getIsolationScope(); - initialIsolationScope.setTag('aa', 'aa'); - - startSpan( - { - name: 'other', - attributes: { 'next.route': 'aha' }, - }, - () => { - withIsolationScopeOrReuseFromRootSpan(isolationScope => { - isolationScope.setTag('bb', 'bb'); - expect(isolationScope).not.toBe(initialIsolationScope); - expect(isolationScope.getScopeData().tags).toEqual({ aa: 'aa', bb: 'bb' }); - }); - }, - ); - }); -}); From 9efc1a7f8ab78b599aba756f1b02722c2f9632b2 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 16 Apr 2024 12:13:46 +0000 Subject: [PATCH 4/9] maybe? --- packages/nextjs/src/common/utils/tracingUtils.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/src/common/utils/tracingUtils.ts b/packages/nextjs/src/common/utils/tracingUtils.ts index 91e74494d3c2..94f789f1b0fa 100644 --- a/packages/nextjs/src/common/utils/tracingUtils.ts +++ b/packages/nextjs/src/common/utils/tracingUtils.ts @@ -88,7 +88,9 @@ export function escapeNextjsTracing(cb: () => T): T { traceId: uuid4(), spanId: uuid4().substring(16), }); - return cb(); + return nextjsEscapedAsyncStorage.run(true, () => { + return cb(); + }); }); } } From f89016a157db3fae6b7dff703320c56d3b1f0225 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 16 Apr 2024 12:19:29 +0000 Subject: [PATCH 5/9] comment --- .../nextjs/src/common/utils/tracingUtils.ts | 4 +- .../wrapApiHandlerWithSentryVercelCrons.ts | 139 +++++++++--------- 2 files changed, 70 insertions(+), 73 deletions(-) diff --git a/packages/nextjs/src/common/utils/tracingUtils.ts b/packages/nextjs/src/common/utils/tracingUtils.ts index 94f789f1b0fa..0c03bc8f0ec9 100644 --- a/packages/nextjs/src/common/utils/tracingUtils.ts +++ b/packages/nextjs/src/common/utils/tracingUtils.ts @@ -62,7 +62,9 @@ interface AsyncLocalStorage { let nextjsEscapedAsyncStorage: AsyncLocalStorage; /** - * TODO + * Will mark the execution context of the callback as "escaped" from Next.js internal tracing by unsetting the active + * span and propagation context. When an execution passes through this function multiple times, it is a noop after the + * first time. */ export function escapeNextjsTracing(cb: () => T): T { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts index 84889518e020..4974cd827e9a 100644 --- a/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts +++ b/packages/nextjs/src/common/wrapApiHandlerWithSentryVercelCrons.ts @@ -1,8 +1,7 @@ -import { captureCheckIn, withIsolationScope } from '@sentry/core'; +import { captureCheckIn } from '@sentry/core'; import type { NextApiRequest } from 'next'; import type { VercelCronsConfig } from './types'; -import { escapeNextjsTracing } from './utils/tracingUtils'; type EdgeRequest = { nextUrl: URL; @@ -20,94 +19,90 @@ export function wrapApiHandlerWithSentryVercelCrons { - return escapeNextjsTracing(() => { - return withIsolationScope(() => { - if (!args || !args[0]) { - return originalFunction.apply(thisArg, args); - } + if (!args || !args[0]) { + return originalFunction.apply(thisArg, args); + } - const [req] = args as [NextApiRequest | EdgeRequest]; + const [req] = args as [NextApiRequest | EdgeRequest]; - let maybePromiseResult; - const cronsKey = 'nextUrl' in req ? req.nextUrl.pathname : req.url; - const userAgentHeader = 'nextUrl' in req ? req.headers.get('user-agent') : req.headers['user-agent']; + let maybePromiseResult; + const cronsKey = 'nextUrl' in req ? req.nextUrl.pathname : req.url; + const userAgentHeader = 'nextUrl' in req ? req.headers.get('user-agent') : req.headers['user-agent']; - if ( - !vercelCronsConfig || // do nothing if vercel crons config is missing - !userAgentHeader?.includes('vercel-cron') // do nothing if endpoint is not called from vercel crons - ) { - return originalFunction.apply(thisArg, args); - } + if ( + !vercelCronsConfig || // do nothing if vercel crons config is missing + !userAgentHeader?.includes('vercel-cron') // do nothing if endpoint is not called from vercel crons + ) { + return originalFunction.apply(thisArg, args); + } - const vercelCron = vercelCronsConfig.find(vercelCron => vercelCron.path === cronsKey); + const vercelCron = vercelCronsConfig.find(vercelCron => vercelCron.path === cronsKey); - if (!vercelCron || !vercelCron.path || !vercelCron.schedule) { - return originalFunction.apply(thisArg, args); - } + if (!vercelCron || !vercelCron.path || !vercelCron.schedule) { + return originalFunction.apply(thisArg, args); + } - const monitorSlug = vercelCron.path; + const monitorSlug = vercelCron.path; - const checkInId = captureCheckIn( - { - monitorSlug, - status: 'in_progress', - }, - { - maxRuntime: 60 * 12, // (minutes) so 12 hours - just a very high arbitrary number since we don't know the actual duration of the users cron job - schedule: { - type: 'crontab', - value: vercelCron.schedule, - }, - }, - ); + const checkInId = captureCheckIn( + { + monitorSlug, + status: 'in_progress', + }, + { + maxRuntime: 60 * 12, // (minutes) so 12 hours - just a very high arbitrary number since we don't know the actual duration of the users cron job + schedule: { + type: 'crontab', + value: vercelCron.schedule, + }, + }, + ); - const startTime = Date.now() / 1000; + const startTime = Date.now() / 1000; - const handleErrorCase = (): void => { - captureCheckIn({ - checkInId, - monitorSlug, - status: 'error', - duration: Date.now() / 1000 - startTime, - }); - }; - - try { - maybePromiseResult = originalFunction.apply(thisArg, args); - } catch (e) { - handleErrorCase(); - throw e; - } + const handleErrorCase = (): void => { + captureCheckIn({ + checkInId, + monitorSlug, + status: 'error', + duration: Date.now() / 1000 - startTime, + }); + }; - if (typeof maybePromiseResult === 'object' && maybePromiseResult !== null && 'then' in maybePromiseResult) { - Promise.resolve(maybePromiseResult).then( - () => { - captureCheckIn({ - checkInId, - monitorSlug, - status: 'ok', - duration: Date.now() / 1000 - startTime, - }); - }, - () => { - handleErrorCase(); - }, - ); + try { + maybePromiseResult = originalFunction.apply(thisArg, args); + } catch (e) { + handleErrorCase(); + throw e; + } - // It is very important that we return the original promise here, because Next.js attaches various properties - // to that promise and will throw if they are not on the returned value. - return maybePromiseResult; - } else { + if (typeof maybePromiseResult === 'object' && maybePromiseResult !== null && 'then' in maybePromiseResult) { + Promise.resolve(maybePromiseResult).then( + () => { captureCheckIn({ checkInId, monitorSlug, status: 'ok', duration: Date.now() / 1000 - startTime, }); - return maybePromiseResult; - } + }, + () => { + handleErrorCase(); + }, + ); + + // It is very important that we return the original promise here, because Next.js attaches various properties + // to that promise and will throw if they are not on the returned value. + return maybePromiseResult; + } else { + captureCheckIn({ + checkInId, + monitorSlug, + status: 'ok', + duration: Date.now() / 1000 - startTime, }); - }); + return maybePromiseResult; + } }, }); } From a1e28b1d3f17a3b7dfb6b820a6b36eb5b643ec0d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 16 Apr 2024 12:20:14 +0000 Subject: [PATCH 6/9] flipflop --- .../nextjs/src/config/templates/apiWrapperTemplate.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/nextjs/src/config/templates/apiWrapperTemplate.ts b/packages/nextjs/src/config/templates/apiWrapperTemplate.ts index d5eae3687403..80b9a4f51d60 100644 --- a/packages/nextjs/src/config/templates/apiWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/apiWrapperTemplate.ts @@ -54,14 +54,14 @@ declare const __VERCEL_CRONS_CONFIGURATION__: VercelCronsConfig; let wrappedHandler = userProvidedHandler; -if (wrappedHandler) { - wrappedHandler = Sentry.wrapApiHandlerWithSentry(wrappedHandler, '__ROUTE__'); -} - if (wrappedHandler && __VERCEL_CRONS_CONFIGURATION__) { wrappedHandler = Sentry.wrapApiHandlerWithSentryVercelCrons(wrappedHandler, __VERCEL_CRONS_CONFIGURATION__); } +if (wrappedHandler) { + wrappedHandler = Sentry.wrapApiHandlerWithSentry(wrappedHandler, '__ROUTE__'); +} + export default wrappedHandler; // Re-export anything exported by the page module we're wrapping. When processing this code, Rollup is smart enough to From 85b2f4331f531e5c7a702837b48512bb74564f52 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 16 Apr 2024 12:41:25 +0000 Subject: [PATCH 7/9] :thinking: --- .../test-applications/create-next-app/pages/api/success.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/success.ts b/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/success.ts index 45ff2aac795e..94f7b003ffcb 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/success.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/pages/api/success.ts @@ -3,7 +3,7 @@ import * as Sentry from '@sentry/nextjs'; import type { NextApiRequest, NextApiResponse } from 'next'; export default function handler(req: NextApiRequest, res: NextApiResponse) { - Sentry.startSpan({ name: 'test-span', forceTransaction: true }, span => undefined); + Sentry.startSpan({ name: 'test-span' }, span => undefined); Sentry.flush().then(() => { res.status(200).json({ From b861870ad5c7a6bf85d6ef0ef0544216790b59bb Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 16 Apr 2024 12:53:36 +0000 Subject: [PATCH 8/9] . --- .../create-next-app/tests/behaviour-server.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-server.test.ts b/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-server.test.ts index 1610ac549e7e..becf9bf9bce7 100644 --- a/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-server.test.ts +++ b/dev-packages/e2e-tests/test-applications/create-next-app/tests/behaviour-server.test.ts @@ -42,8 +42,6 @@ test('Sends server-side transactions to Sentry', async ({ baseURL }) => { const { data } = await axios.get(`${baseURL}/api/success`); const { transactionIds } = data; - expect(transactionIds.length).toBeGreaterThanOrEqual(1); - console.log(`Polling for transaction eventIds: ${JSON.stringify(transactionIds)}`); await Promise.all( From 2a69ea200f6acd426f9a16a205a419baa826a9ed Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 16 Apr 2024 13:24:29 +0000 Subject: [PATCH 9/9] revert a bunch of stuff? --- packages/nextjs/src/common/wrapApiHandlerWithSentry.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts index bdfd3c27d896..2a3ecd02bdb5 100644 --- a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts @@ -12,7 +12,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core'; import type { AugmentedNextApiRequest, AugmentedNextApiResponse, NextApiHandler } from './types'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { flushQueue } from './utils/responseEnd'; -import { commonObjectToIsolationScope, escapeNextjsTracing } from './utils/tracingUtils'; +import { escapeNextjsTracing } from './utils/tracingUtils'; /** * Wrap the given API route handler for tracing and error capturing. Thin wrapper around `withSentry`, which only @@ -53,10 +53,7 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz } req.__withSentry_applied__ = true; - const isolationScope = commonObjectToIsolationScope(req); - isolationScope.setSDKProcessingMetadata({ request: req }); - - return withIsolationScope(isolationScope, () => { + return withIsolationScope(isolationScope => { return continueTrace( { // TODO(v8): Make it so that continue trace will allow null as sentryTrace value and remove this fallback here @@ -84,6 +81,8 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz const reqMethod = `${(req.method || 'GET').toUpperCase()} `; + isolationScope.setSDKProcessingMetadata({ request: req }); + return startSpanManual( { name: `${reqMethod}${reqPath}`,