From b0425e42d2dae0e7108866fd77fa52f2e971b980 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 27 Mar 2024 11:29:42 +0100 Subject: [PATCH 01/11] Remove Http integration from Next.js --- .../src/common/wrapRouteHandlerWithSentry.ts | 110 +++++++----------- packages/nextjs/src/server/index.ts | 5 +- 2 files changed, 48 insertions(+), 67 deletions(-) diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index edee1e54cec4..483bc0350880 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -1,16 +1,14 @@ import { - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SPAN_STATUS_ERROR, addTracingExtensions, captureException, - continueTrace, + getActiveSpan, + getIsolationScope, + getRootSpan, handleCallbackErrors, setHttpStatus, - startSpan, } from '@sentry/core'; import { winterCGHeadersToDict } from '@sentry/utils'; - import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils'; import type { RouteHandlerContext } from './types'; import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; @@ -26,73 +24,55 @@ export function wrapRouteHandlerWithSentry any>( context: RouteHandlerContext, ): (...args: Parameters) => ReturnType extends Promise ? ReturnType : Promise> { addTracingExtensions(); - const { method, parameterizedRoute, headers } = context; + + const { headers } = context; + return new Proxy(routeHandler, { - apply: (originalFunction, thisArg, args) => { - return withIsolationScopeOrReuseFromRootSpan(async isolationScope => { - isolationScope.setSDKProcessingMetadata({ - request: { - headers: headers ? winterCGHeadersToDict(headers) : undefined, - }, - }); - return continueTrace( - { - // TODO(v8): Make it so that continue trace will allow null as sentryTrace value and remove this fallback here - sentryTrace: headers?.get('sentry-trace') ?? undefined, - baggage: headers?.get('baggage'), - }, - async () => { - try { - return await startSpan( - { - op: 'http.server', - name: `${method} ${parameterizedRoute}`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', - }, - }, - 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.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); - } else { - captureException(error, { - mechanism: { - handled: false, - }, - }); - } - }, - ); + apply: async (originalFunction, thisArg, args) => { + getIsolationScope().setSDKProcessingMetadata({ + request: { + headers: headers ? winterCGHeadersToDict(headers) : undefined, + }, + }); - try { - if (span && response.status) { - setHttpStatus(span, response.status); - } - } catch { - // best effort - response may be undefined? - } + try { + const activeSpan = getActiveSpan(); + const rootSpan = activeSpan && getRootSpan(activeSpan); - return response; + 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, }, - ); - } 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(); - } + }); } }, ); - }); + + try { + if (rootSpan && response.status) { + setHttpStatus(rootSpan, response.status); + } + } catch { + // best effort - response may be undefined? + } + + 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/server/index.ts b/packages/nextjs/src/server/index.ts index 4dc6910d49eb..aa4c585325d0 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -75,8 +75,9 @@ export function init(options: NodeOptions): void { ...getDefaultIntegrations(options).filter( integration => integration.name !== 'OnUncaughtException' && - // 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 Node-Fetch and Http instrumentation, so we shouldn't add ours on-top + integration.name !== 'NodeFetch' && + integration.name !== 'Http', ), onUncaughtExceptionIntegration(), ]; From 7b9431b3ddb7f83315423e23bc1f46f2f26ac5cd Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 27 Mar 2024 16:26:39 +0100 Subject: [PATCH 02/11] review comments --- .../src/common/wrapRouteHandlerWithSentry.ts | 80 ++++++++++--------- packages/nextjs/src/server/index.ts | 3 +- 2 files changed, 43 insertions(+), 40 deletions(-) diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 483bc0350880..8e9206a626c1 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -3,10 +3,10 @@ import { addTracingExtensions, captureException, getActiveSpan, - getIsolationScope, getRootSpan, handleCallbackErrors, setHttpStatus, + withIsolationScope, } from '@sentry/core'; import { winterCGHeadersToDict } from '@sentry/utils'; import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils'; @@ -29,50 +29,52 @@ export function wrapRouteHandlerWithSentry any>( return new Proxy(routeHandler, { apply: async (originalFunction, thisArg, args) => { - getIsolationScope().setSDKProcessingMetadata({ - request: { - headers: headers ? winterCGHeadersToDict(headers) : undefined, - }, - }); - - try { - const activeSpan = getActiveSpan(); - const rootSpan = activeSpan && getRootSpan(activeSpan); - - 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, - }, - }); - } + return withIsolationScope(async isolationScope => { + isolationScope.setSDKProcessingMetadata({ + request: { + headers: headers ? winterCGHeadersToDict(headers) : undefined, }, - ); + }); try { - if (rootSpan && response.status) { - setHttpStatus(rootSpan, response.status); + const activeSpan = getActiveSpan(); + const rootSpan = activeSpan && getRootSpan(activeSpan); + + 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, + }, + }); + } + }, + ); + + try { + if (rootSpan && response.status) { + setHttpStatus(rootSpan, response.status); + } + } catch { + // best effort - response may be undefined? } - } catch { - // best effort - response may be undefined? - } - 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(); + 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/server/index.ts b/packages/nextjs/src/server/index.ts index aa4c585325d0..6d4135f736a5 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -75,8 +75,9 @@ export function init(options: NodeOptions): void { ...getDefaultIntegrations(options).filter( integration => integration.name !== 'OnUncaughtException' && - // Next.js comes with its own Node-Fetch and Http instrumentation, so we shouldn't add ours on-top + // 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 integration.name !== 'Http', ), onUncaughtExceptionIntegration(), From f3808b04cb1428079d898416126dff6062b481d1 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Tue, 2 Apr 2024 12:59:20 +0200 Subject: [PATCH 03/11] start span if there is none --- .../tests/route-handlers.test.ts | 1 + .../src/common/wrapRouteHandlerWithSentry.ts | 89 +++++++++++++------ 2 files changed, 62 insertions(+), 28 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts index 84969d7c84ca..21e0bf4c745f 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/route-handlers.test.ts @@ -56,6 +56,7 @@ test('Should record exceptions and transactions for faulty route handlers', asyn expect(routehandlerTransaction.contexts?.trace?.status).toBe('unknown_error'); expect(routehandlerTransaction.contexts?.trace?.op).toBe('http.server'); + expect(routehandlerTransaction.contexts?.trace?.origin).toBe('auto.function.nextjs'); expect(routehandlerError.exception?.values?.[0].value).toBe('route-handler-error'); // TODO: Uncomment once we update the scope transaction name on the server side diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 8e9206a626c1..9dd0f64e8d8b 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -1,4 +1,6 @@ import { + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SPAN_STATUS_ERROR, addTracingExtensions, captureException, @@ -6,8 +8,10 @@ import { getRootSpan, handleCallbackErrors, setHttpStatus, + startSpan, withIsolationScope, } from '@sentry/core'; +import type { Span } from '@sentry/types'; import { winterCGHeadersToDict } from '@sentry/utils'; import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils'; import type { RouteHandlerContext } from './types'; @@ -15,6 +19,44 @@ import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { flushQueue } from './utils/responseEnd'; import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +async function addSpanAttributes any>( + originalFunction: F, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + thisArg: any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + args: any[], + rootSpan?: Span, +): Promise { + 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, + }, + }); + } + }, + ); + + try { + if (rootSpan && response.status) { + setHttpStatus(rootSpan, response.status); + } + } catch { + // best effort - response may be undefined? + } + + return response; +} + /** * Wraps a Next.js route handler with performance and error instrumentation. */ @@ -25,10 +67,10 @@ export function wrapRouteHandlerWithSentry any>( ): (...args: Parameters) => ReturnType extends Promise ? ReturnType : Promise> { addTracingExtensions(); - const { headers } = context; + const { method, parameterizedRoute, headers } = context; return new Proxy(routeHandler, { - apply: async (originalFunction, thisArg, args) => { + apply: (originalFunction, thisArg, args) => { return withIsolationScope(async isolationScope => { isolationScope.setSDKProcessingMetadata({ request: { @@ -40,33 +82,24 @@ export function wrapRouteHandlerWithSentry any>( const activeSpan = getActiveSpan(); const rootSpan = activeSpan && getRootSpan(activeSpan); - 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, - }, - }); - } - }, - ); - - try { - if (rootSpan && response.status) { - setHttpStatus(rootSpan, response.status); - } - } catch { - // best effort - response may be undefined? + if (rootSpan) { + return await addSpanAttributes(originalFunction, thisArg, args, rootSpan); + } else { + /** As our own HTTP integration is disabled (src/server/index.ts) the rootSpan comes from Next.js. + * In case there is not root span, we start a new one. */ + return await startSpan( + { + op: 'http.server', + name: `${method} ${parameterizedRoute}`, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', + }, + }, + async span => addSpanAttributes(originalFunction, thisArg, args, span), + ); } - - return response; } finally { if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') { // 1. Edge transport requires manual flushing From 2f16e1b801e8dabaf1c086565f48105a88ad2f02 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 27 Mar 2024 16:15:16 +0100 Subject: [PATCH 04/11] feat(nextjs): Fork isolation scopes when incoming HTTP spans are created --- packages/nextjs/package.json | 2 ++ .../requestIsolationScopeIntegration.ts | 29 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 packages/nextjs/src/server/requestIsolationScopeIntegration.ts diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 293401d7d41e..fd88dbac90a2 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -65,12 +65,14 @@ "access": "public" }, "dependencies": { + "@opentelemetry/api": "1.7.0", "@rollup/plugin-commonjs": "24.0.0", "@sentry/core": "8.0.0-alpha.9", "@sentry/node": "8.0.0-alpha.9", "@sentry/react": "8.0.0-alpha.9", "@sentry/types": "8.0.0-alpha.9", "@sentry/utils": "8.0.0-alpha.9", + "@sentry/opentelemetry": "8.0.0-alpha.9", "@sentry/vercel-edge": "8.0.0-alpha.9", "@sentry/webpack-plugin": "2.16.0", "chalk": "3.0.0", diff --git a/packages/nextjs/src/server/requestIsolationScopeIntegration.ts b/packages/nextjs/src/server/requestIsolationScopeIntegration.ts new file mode 100644 index 000000000000..44a118419025 --- /dev/null +++ b/packages/nextjs/src/server/requestIsolationScopeIntegration.ts @@ -0,0 +1,29 @@ +import { SpanKind } from '@opentelemetry/api'; +import { defineIntegration, spanToJSON } from '@sentry/core'; +import { getSpanKind, getSpanScopes } 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); + + // 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 && spanJson.data && 'http.method' in spanJson.data) { + const scopes = getSpanScopes(span); + if (scopes) { + scopes.isolationScope = scopes.isolationScope.clone(); + } + } + }); + }, + }; +}); From 0c82b979c8a28be955e44107a86a34e4df821ed2 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Thu, 4 Apr 2024 11:42:41 +0200 Subject: [PATCH 05/11] add startOrUpdateSpan function --- .../tests/request-instrumentation.test.ts | 23 ++-- .../src/common/wrapRouteHandlerWithSentry.ts | 109 +++++++++--------- packages/nextjs/src/server/index.ts | 2 + packages/opentelemetry/src/utils/mapStatus.ts | 10 +- 4 files changed, 79 insertions(+), 65 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts index 7dbe20e2efb6..3e25a99133da 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-14/tests/request-instrumentation.test.ts @@ -19,16 +19,15 @@ test('Should send a transaction with a fetch span', async ({ page }) => { }), ); - // TODO: Uncomment the below when fixed. For whatever reason that we now have accepted, spans created with Node.js' http.get() will not attach themselves to transactions. - // More info: https://github.com/getsentry/sentry-javascript/pull/11016/files#diff-10fa195789425786c6e5e769380be18790768f0b76319ee41bbb488d9fe50405R4 - // expect((await transactionPromise).spans).toContainEqual( - // expect.objectContaining({ - // data: expect.objectContaining({ - // 'http.method': 'GET', - // 'sentry.op': 'http.client', - // 'sentry.origin': 'auto.http.otel.http', - // }), - // description: 'GET http://example.com/', - // }), - // ); + expect((await transactionPromise).spans).toContainEqual( + expect.objectContaining({ + data: expect.objectContaining({ + 'http.method': 'GET', + 'sentry.op': 'http.client', + // todo: without the HTTP integration in the Next.js SDK, this is set to 'manual' -> we could rename this to be more specific + 'sentry.origin': 'manual', + }), + description: 'GET http://example.com/', + }), + ); }); diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 9dd0f64e8d8b..fe205f23a0cf 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -1,4 +1,5 @@ import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SPAN_STATUS_ERROR, @@ -19,42 +20,38 @@ import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { flushQueue } from './utils/responseEnd'; import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScopeOrReuseFromRootSpan'; -// eslint-disable-next-line @typescript-eslint/no-explicit-any -async function addSpanAttributes any>( - originalFunction: F, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - thisArg: any, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - args: any[], - rootSpan?: Span, +/** As our own HTTP integration is disabled (src/server/index.ts) the rootSpan comes from Next.js. + * In case there is not root span, we start a new span. */ +function startOrUpdateSpan( + spanName: string, + handleResponseErrors: (rootSpan: Span) => Promise, ): Promise { - 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 activeSpan = getActiveSpan(); + const rootSpan = activeSpan && getRootSpan(activeSpan); - try { - if (rootSpan && response.status) { - setHttpStatus(rootSpan, response.status); - } - } catch { - // best effort - response may be undefined? - } + if (rootSpan) { + rootSpan.updateName(spanName); + rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.server'); + rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.function.nextjs'); - return response; + return handleResponseErrors(rootSpan); + } else { + return startSpan( + { + op: 'http.server', + name: spanName, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', + }, + }, + (span: Span) => { + return handleResponseErrors(span); + }, + ); + } } /** @@ -79,27 +76,35 @@ export function wrapRouteHandlerWithSentry any>( }); try { - const activeSpan = getActiveSpan(); - const rootSpan = activeSpan && getRootSpan(activeSpan); - - if (rootSpan) { - return await addSpanAttributes(originalFunction, thisArg, args, rootSpan); - } else { - /** As our own HTTP integration is disabled (src/server/index.ts) the rootSpan comes from Next.js. - * In case there is not root span, we start a new one. */ - return await startSpan( - { - op: 'http.server', - name: `${method} ${parameterizedRoute}`, - forceTransaction: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', - }, + 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, + }, + }); + } }, - async span => addSpanAttributes(originalFunction, thisArg, args, span), ); - } + + try { + if (rootSpan && response.status) { + setHttpStatus(rootSpan, response.status); + } + } catch { + // best effort - response may be undefined? + } + + return response; + }); } finally { if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') { // 1. Edge transport requires manual flushing diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 6d4135f736a5..de5ac52c4f18 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -12,6 +12,7 @@ import { onUncaughtExceptionIntegration } from './onUncaughtExceptionIntegration export * from '@sentry/node'; import type { EventProcessor } from '@sentry/types'; +import { requestIsolationScopeIntegration } from './requestIsolationScopeIntegration'; export { captureUnderscoreErrorException } from '../common/_error'; export { onUncaughtExceptionIntegration } from './onUncaughtExceptionIntegration'; @@ -81,6 +82,7 @@ export function init(options: NodeOptions): void { integration.name !== 'Http', ), onUncaughtExceptionIntegration(), + requestIsolationScopeIntegration(), ]; // This value is injected at build time, based on the output directory specified in the build config. Though a default diff --git a/packages/opentelemetry/src/utils/mapStatus.ts b/packages/opentelemetry/src/utils/mapStatus.ts index ab40589e813c..7e8682e92b55 100644 --- a/packages/opentelemetry/src/utils/mapStatus.ts +++ b/packages/opentelemetry/src/utils/mapStatus.ts @@ -26,6 +26,10 @@ const canonicalGrpcErrorCodesMap: Record = { '16': 'unauthenticated', } as const; +const isStatusErrorMessageValid = (message: string): boolean => { + return Object.values(canonicalGrpcErrorCodesMap).includes(message as SpanStatus['message']); +}; + /** * Get a Sentry span status from an otel span. */ @@ -39,7 +43,11 @@ export function mapStatus(span: AbstractSpan): SpanStatus { return { code: SPAN_STATUS_OK }; // If the span is already marked as erroneous we return that exact status } else if (status.code === SpanStatusCode.ERROR) { - return { code: SPAN_STATUS_ERROR, message: status.message }; + if (typeof status.message === 'undefined' || isStatusErrorMessageValid(status.message)) { + return { code: SPAN_STATUS_ERROR, message: status.message }; + } else { + return { code: SPAN_STATUS_ERROR, message: 'unknown_error' }; + } } } From 77204f42985e8ecce0e55a89715d634988c6c3c2 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Fri, 5 Apr 2024 14:48:36 +0200 Subject: [PATCH 06/11] change nextjs version in e2e test --- .../test-applications/nextjs-app-dir/package.json | 2 +- packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts | 9 +++------ packages/nextjs/src/server/index.ts | 3 +-- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/package.json b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/package.json index 4bfb163d9885..13d6070bd48a 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/package.json +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/package.json @@ -20,7 +20,7 @@ "@types/node": "18.11.17", "@types/react": "18.0.26", "@types/react-dom": "18.0.9", - "next": "14.0.2", + "next": "14.0.0", "react": "18.2.0", "react-dom": "18.2.0", "typescript": "4.9.5", diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index fe205f23a0cf..07fe035b0875 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -22,10 +22,7 @@ import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScop /** As our own HTTP integration is disabled (src/server/index.ts) the rootSpan comes from Next.js. * In case there is not root span, we start a new span. */ -function startOrUpdateSpan( - spanName: string, - handleResponseErrors: (rootSpan: Span) => Promise, -): Promise { +function startOrUpdateSpan(spanName: string, cb: (rootSpan: Span) => Promise): Promise { const activeSpan = getActiveSpan(); const rootSpan = activeSpan && getRootSpan(activeSpan); @@ -35,7 +32,7 @@ function startOrUpdateSpan( rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.server'); rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.function.nextjs'); - return handleResponseErrors(rootSpan); + return cb(rootSpan); } else { return startSpan( { @@ -48,7 +45,7 @@ function startOrUpdateSpan( }, }, (span: Span) => { - return handleResponseErrors(span); + return cb(span); }, ); } diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index de5ac52c4f18..c80e503e5095 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -12,7 +12,6 @@ import { onUncaughtExceptionIntegration } from './onUncaughtExceptionIntegration export * from '@sentry/node'; import type { EventProcessor } from '@sentry/types'; -import { requestIsolationScopeIntegration } from './requestIsolationScopeIntegration'; export { captureUnderscoreErrorException } from '../common/_error'; export { onUncaughtExceptionIntegration } from './onUncaughtExceptionIntegration'; @@ -82,7 +81,7 @@ export function init(options: NodeOptions): void { integration.name !== 'Http', ), onUncaughtExceptionIntegration(), - requestIsolationScopeIntegration(), + // requestIsolationScopeIntegration(), ]; // This value is injected at build time, based on the output directory specified in the build config. Though a default From 7daa83789f9d705538e6cc82c28f83b70ed9da23 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 8 Apr 2024 16:13:23 +0200 Subject: [PATCH 07/11] Revert "change nextjs version in e2e test" This reverts commit fda5f323784605ccba2d6ef4963f838e93e83364. --- .../test-applications/nextjs-app-dir/package.json | 2 +- packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts | 9 ++++++--- packages/nextjs/src/server/index.ts | 3 ++- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/package.json b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/package.json index 13d6070bd48a..4bfb163d9885 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/package.json +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/package.json @@ -20,7 +20,7 @@ "@types/node": "18.11.17", "@types/react": "18.0.26", "@types/react-dom": "18.0.9", - "next": "14.0.0", + "next": "14.0.2", "react": "18.2.0", "react-dom": "18.2.0", "typescript": "4.9.5", diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 07fe035b0875..fe205f23a0cf 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -22,7 +22,10 @@ import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScop /** As our own HTTP integration is disabled (src/server/index.ts) the rootSpan comes from Next.js. * In case there is not root span, we start a new span. */ -function startOrUpdateSpan(spanName: string, cb: (rootSpan: Span) => Promise): Promise { +function startOrUpdateSpan( + spanName: string, + handleResponseErrors: (rootSpan: Span) => Promise, +): Promise { const activeSpan = getActiveSpan(); const rootSpan = activeSpan && getRootSpan(activeSpan); @@ -32,7 +35,7 @@ function startOrUpdateSpan(spanName: string, cb: (rootSpan: Span) => Promise Promise { - return cb(span); + return handleResponseErrors(span); }, ); } diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index c80e503e5095..de5ac52c4f18 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -12,6 +12,7 @@ import { onUncaughtExceptionIntegration } from './onUncaughtExceptionIntegration export * from '@sentry/node'; import type { EventProcessor } from '@sentry/types'; +import { requestIsolationScopeIntegration } from './requestIsolationScopeIntegration'; export { captureUnderscoreErrorException } from '../common/_error'; export { onUncaughtExceptionIntegration } from './onUncaughtExceptionIntegration'; @@ -81,7 +82,7 @@ export function init(options: NodeOptions): void { integration.name !== 'Http', ), onUncaughtExceptionIntegration(), - // requestIsolationScopeIntegration(), + requestIsolationScopeIntegration(), ]; // This value is injected at build time, based on the output directory specified in the build config. Though a default From 2a54bd2eb4acc48e1b26f8586fe8d3d1f042e6d4 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 8 Apr 2024 15:49:25 +0100 Subject: [PATCH 08/11] fix nextjs PR (#11450) Just trying some things... --- .../src/common/wrapRouteHandlerWithSentry.ts | 12 +++------ .../requestIsolationScopeIntegration.ts | 25 +++++++++++++------ 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index fe205f23a0cf..52b6bbf7bb17 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -10,7 +10,6 @@ import { handleCallbackErrors, setHttpStatus, startSpan, - withIsolationScope, } from '@sentry/core'; import type { Span } from '@sentry/types'; import { winterCGHeadersToDict } from '@sentry/utils'; @@ -22,10 +21,7 @@ import { withIsolationScopeOrReuseFromRootSpan } from './utils/withIsolationScop /** As our own HTTP integration is disabled (src/server/index.ts) the rootSpan comes from Next.js. * In case there is not root span, we start a new span. */ -function startOrUpdateSpan( - spanName: string, - handleResponseErrors: (rootSpan: Span) => Promise, -): Promise { +function startOrUpdateSpan(spanName: string, cb: (rootSpan: Span) => Promise): Promise { const activeSpan = getActiveSpan(); const rootSpan = activeSpan && getRootSpan(activeSpan); @@ -35,7 +31,7 @@ function startOrUpdateSpan( rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.server'); rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.function.nextjs'); - return handleResponseErrors(rootSpan); + return cb(rootSpan); } else { return startSpan( { @@ -48,7 +44,7 @@ function startOrUpdateSpan( }, }, (span: Span) => { - return handleResponseErrors(span); + return cb(span); }, ); } @@ -68,7 +64,7 @@ export function wrapRouteHandlerWithSentry any>( return new Proxy(routeHandler, { apply: (originalFunction, thisArg, args) => { - return withIsolationScope(async isolationScope => { + return withIsolationScopeOrReuseFromRootSpan(async isolationScope => { isolationScope.setSDKProcessingMetadata({ request: { headers: headers ? winterCGHeadersToDict(headers) : undefined, diff --git a/packages/nextjs/src/server/requestIsolationScopeIntegration.ts b/packages/nextjs/src/server/requestIsolationScopeIntegration.ts index 44a118419025..ee6e205caebc 100644 --- a/packages/nextjs/src/server/requestIsolationScopeIntegration.ts +++ b/packages/nextjs/src/server/requestIsolationScopeIntegration.ts @@ -1,6 +1,13 @@ import { SpanKind } from '@opentelemetry/api'; -import { defineIntegration, spanToJSON } from '@sentry/core'; -import { getSpanKind, getSpanScopes } from '@sentry/opentelemetry'; +import { + defineIntegration, + getCapturedScopesOnSpan, + getCurrentScope, + getIsolationScope, + setCapturedScopesOnSpan, + spanToJSON, +} from '@sentry/core'; +import { getSpanKind } from '@sentry/opentelemetry'; /** * This integration is responsible for creating isolation scopes for incoming Http requests. @@ -15,13 +22,17 @@ export const requestIsolationScopeIntegration = defineIntegration(() => { 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 && spanJson.data && 'http.method' in spanJson.data) { - const scopes = getSpanScopes(span); - if (scopes) { - scopes.isolationScope = scopes.isolationScope.clone(); - } + if ((getSpanKind(span) === SpanKind.SERVER && data['http.method']) || 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 35b5ac8d2a3fb53ab81825189274c676400b18c4 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Mon, 8 Apr 2024 17:23:17 +0200 Subject: [PATCH 09/11] add OP --- packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 52b6bbf7bb17..8a9cb0ed96ce 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -40,6 +40,7 @@ function startOrUpdateSpan(spanName: string, cb: (rootSpan: Span) => Promise Date: Tue, 9 Apr 2024 08:58:03 +0200 Subject: [PATCH 10/11] Apply suggestions from code review Co-authored-by: Francesco Novy --- packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts | 2 +- packages/nextjs/src/server/requestIsolationScopeIntegration.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 8a9cb0ed96ce..72748ad1f636 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -20,7 +20,7 @@ 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 not root span, we start a new span. */ + * 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); diff --git a/packages/nextjs/src/server/requestIsolationScopeIntegration.ts b/packages/nextjs/src/server/requestIsolationScopeIntegration.ts index ee6e205caebc..786f196c2443 100644 --- a/packages/nextjs/src/server/requestIsolationScopeIntegration.ts +++ b/packages/nextjs/src/server/requestIsolationScopeIntegration.ts @@ -4,6 +4,7 @@ import { getCapturedScopesOnSpan, getCurrentScope, getIsolationScope, + getRootSpan, setCapturedScopesOnSpan, spanToJSON, } from '@sentry/core'; @@ -25,7 +26,7 @@ export const requestIsolationScopeIntegration = defineIntegration(() => { 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']) || data['next.route']) { + 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 From 1674e111349061371371f74b38dcc7d6252cdbd0 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Tue, 9 Apr 2024 13:42:46 +0200 Subject: [PATCH 11/11] fix format --- packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts | 8 +++++--- .../nextjs/src/server/requestIsolationScopeIntegration.ts | 5 ++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index 72748ad1f636..e82b4a3b4bdd 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -27,9 +27,11 @@ function startOrUpdateSpan(spanName: string, cb: (rootSpan: Span) => Promise { 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'])) { + 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