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/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/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/common/wrapRouteHandlerWithSentry.ts b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts index edee1e54cec4..e82b4a3b4bdd 100644 --- a/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts @@ -1,22 +1,58 @@ import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, SPAN_STATUS_ERROR, addTracingExtensions, captureException, - continueTrace, + getActiveSpan, + getRootSpan, handleCallbackErrors, setHttpStatus, startSpan, } from '@sentry/core'; +import type { Span } from '@sentry/types'; import { 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); + + if (rootSpan) { + rootSpan.updateName(spanName); + rootSpan.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', + }); + + return cb(rootSpan); + } else { + return startSpan( + { + op: 'http.server', + name: spanName, + forceTransaction: true, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'http.server', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', + }, + }, + (span: Span) => { + return cb(span); + }, + ); + } +} + /** * Wraps a Next.js route handler with performance and error instrumentation. */ @@ -26,7 +62,9 @@ export function wrapRouteHandlerWithSentry any>( context: RouteHandlerContext, ): (...args: Parameters) => ReturnType extends Promise ? ReturnType : Promise> { addTracingExtensions(); + const { method, parameterizedRoute, headers } = context; + return new Proxy(routeHandler, { apply: (originalFunction, thisArg, args) => { return withIsolationScopeOrReuseFromRootSpan(async isolationScope => { @@ -35,63 +73,44 @@ export function wrapRouteHandlerWithSentry any>( 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, - }, - }); - } - }, - ); - try { - if (span && response.status) { - setHttpStatus(span, response.status); - } - } catch { - // best effort - response may be undefined? - } + 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, + }, + }); + } + }, + ); - 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(); + 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..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'; @@ -75,10 +76,13 @@ 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 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(), + 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/nextjs/src/server/requestIsolationScopeIntegration.ts b/packages/nextjs/src/server/requestIsolationScopeIntegration.ts new file mode 100644 index 000000000000..b6020d891d8b --- /dev/null +++ b/packages/nextjs/src/server/requestIsolationScopeIntegration.ts @@ -0,0 +1,44 @@ +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); + } + }); + }, + }; +}); 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' }; + } } }