diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/instrumentation.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/instrumentation.ts index 180685d41b4a..b8abbf9fd765 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-13/instrumentation.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-13/instrumentation.ts @@ -12,6 +12,7 @@ export function register() { // We are doing a lot of events at once in this test app bufferSize: 1000, }, + debug: false, }); } } diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/package.json b/dev-packages/e2e-tests/test-applications/nextjs-13/package.json index 5be9ecbfc32c..3e7a0ac88266 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-13/package.json +++ b/dev-packages/e2e-tests/test-applications/nextjs-13/package.json @@ -17,7 +17,7 @@ "@types/node": "18.11.17", "@types/react": "18.0.26", "@types/react-dom": "18.0.9", - "next": "13.2.0", + "next": "13.5.7", "react": "18.2.0", "react-dom": "18.2.0", "typescript": "4.9.5" diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/pages/customPageExtension.page.tsx b/dev-packages/e2e-tests/test-applications/nextjs-13/pages/[param]/customPageExtension.page.tsx similarity index 100% rename from dev-packages/e2e-tests/test-applications/nextjs-13/pages/customPageExtension.page.tsx rename to dev-packages/e2e-tests/test-applications/nextjs-13/pages/[param]/customPageExtension.page.tsx diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/pages/error-getServerSideProps.tsx b/dev-packages/e2e-tests/test-applications/nextjs-13/pages/[param]/error-getServerSideProps.tsx similarity index 100% rename from dev-packages/e2e-tests/test-applications/nextjs-13/pages/error-getServerSideProps.tsx rename to dev-packages/e2e-tests/test-applications/nextjs-13/pages/[param]/error-getServerSideProps.tsx diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/client/pages-dir-pageload.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/client/pages-dir-pageload.test.ts index 8c74b2c99427..af59b41c2908 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/client/pages-dir-pageload.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/client/pages-dir-pageload.test.ts @@ -46,3 +46,38 @@ test('should create a pageload transaction when the `pages` directory is used', type: 'transaction', }); }); + +test('should create a pageload transaction with correct name when an error occurs in getServerSideProps', async ({ + page, +}) => { + const transactionPromise = waitForTransaction('nextjs-13', async transactionEvent => { + return ( + transactionEvent.transaction === '/[param]/error-getServerSideProps' && + transactionEvent.contexts?.trace?.op === 'pageload' + ); + }); + + await page.goto(`/something/error-getServerSideProps`, { waitUntil: 'networkidle' }); + + const transaction = await transactionPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + data: { + 'sentry.op': 'pageload', + 'sentry.origin': 'auto.pageload.nextjs.pages_router_instrumentation', + 'sentry.source': 'route', + }, + op: 'pageload', + origin: 'auto.pageload.nextjs.pages_router_instrumentation', + }, + }, + transaction: '/[param]/error-getServerSideProps', + transaction_info: { source: 'route' }, + type: 'transaction', + }); + + // Ensure the transaction name is not '/_error' + expect(transaction.transaction).not.toBe('/_error'); +}); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getInitialProps.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getInitialProps.test.ts index 22da2071d533..570b19b3271d 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getInitialProps.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getInitialProps.test.ts @@ -11,7 +11,7 @@ test('should propagate serverside `getInitialProps` trace to client', async ({ p const serverTransactionPromise = waitForTransaction('nextjs-13', async transactionEvent => { return ( - transactionEvent.transaction === '/[param]/withInitialProps' && + transactionEvent.transaction === 'GET /[param]/withInitialProps' && transactionEvent.contexts?.trace?.op === 'http.server' ); }); @@ -47,7 +47,7 @@ test('should propagate serverside `getInitialProps` trace to client', async ({ p status: 'ok', }, }, - transaction: '/[param]/withInitialProps', + transaction: 'GET /[param]/withInitialProps', transaction_info: { source: 'route', }, diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getServerSideProps.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getServerSideProps.test.ts index 20bbbc9437f6..765864dbf4a1 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getServerSideProps.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getServerSideProps.test.ts @@ -11,7 +11,7 @@ test('Should record performance for getServerSideProps', async ({ page }) => { const serverTransactionPromise = waitForTransaction('nextjs-13', async transactionEvent => { return ( - transactionEvent.transaction === '/[param]/withServerSideProps' && + transactionEvent.transaction === 'GET /[param]/withServerSideProps' && transactionEvent.contexts?.trace?.op === 'http.server' ); }); @@ -47,7 +47,7 @@ test('Should record performance for getServerSideProps', async ({ page }) => { status: 'ok', }, }, - transaction: '/[param]/withServerSideProps', + transaction: 'GET /[param]/withServerSideProps', transaction_info: { source: 'route', }, diff --git a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/getServerSideProps.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/getServerSideProps.test.ts index 0c99ba302dfa..44546d6b4668 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/getServerSideProps.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/getServerSideProps.test.ts @@ -8,12 +8,12 @@ test('Should report an error event for errors thrown in getServerSideProps', asy const transactionEventPromise = waitForTransaction('nextjs-13', transactionEvent => { return ( - transactionEvent.transaction === '/error-getServerSideProps' && + transactionEvent.transaction === 'GET /[param]/error-getServerSideProps' && transactionEvent.contexts?.trace?.op === 'http.server' ); }); - await page.goto('/error-getServerSideProps'); + await page.goto('/dogsaregreat/error-getServerSideProps'); expect(await errorEventPromise).toMatchObject({ contexts: { @@ -40,7 +40,7 @@ test('Should report an error event for errors thrown in getServerSideProps', asy url: expect.stringMatching(/^http.*\/error-getServerSideProps/), }, timestamp: expect.any(Number), - transaction: 'getServerSideProps (/error-getServerSideProps)', + transaction: 'getServerSideProps (/[param]/error-getServerSideProps)', }); expect(await transactionEventPromise).toMatchObject({ @@ -60,11 +60,11 @@ test('Should report an error event for errors thrown in getServerSideProps', asy data: { 'http.response.status_code': 500, 'sentry.op': 'http.server', - 'sentry.origin': 'auto.function.nextjs', + 'sentry.origin': expect.stringMatching(/^(auto|auto\.http\.otel\.http)$/), 'sentry.source': 'route', }, op: 'http.server', - origin: 'auto.function.nextjs', + origin: expect.stringMatching(/^(auto|auto\.http\.otel\.http)$/), span_id: expect.any(String), status: 'internal_error', trace_id: expect.any(String), @@ -80,8 +80,9 @@ test('Should report an error event for errors thrown in getServerSideProps', asy }, start_timestamp: expect.any(Number), timestamp: expect.any(Number), - transaction: '/error-getServerSideProps', - transaction_info: { source: 'route' }, + transaction: 'GET /[param]/error-getServerSideProps', + // TODO: This test fails depending on the next version (next 13: 'custom', next >14: 'route') + // transaction_info: { source: 'custom' }, type: 'transaction', }); }); @@ -95,11 +96,12 @@ test('Should report an error event for errors thrown in getServerSideProps in pa const transactionEventPromise = waitForTransaction('nextjs-13', transactionEvent => { return ( - transactionEvent.transaction === '/customPageExtension' && transactionEvent.contexts?.trace?.op === 'http.server' + transactionEvent.transaction === 'GET /[param]/customPageExtension' && + transactionEvent.contexts?.trace?.op === 'http.server' ); }); - await page.goto('/customPageExtension'); + await page.goto('/123/customPageExtension'); expect(await errorEventPromise).toMatchObject({ contexts: { @@ -126,7 +128,7 @@ test('Should report an error event for errors thrown in getServerSideProps in pa url: expect.stringMatching(/^http.*\/customPageExtension/), }, timestamp: expect.any(Number), - transaction: 'getServerSideProps (/customPageExtension)', + transaction: 'getServerSideProps (/[param]/customPageExtension)', }); expect(await transactionEventPromise).toMatchObject({ @@ -146,11 +148,11 @@ test('Should report an error event for errors thrown in getServerSideProps in pa data: { 'http.response.status_code': 500, 'sentry.op': 'http.server', - 'sentry.origin': 'auto.function.nextjs', + 'sentry.origin': expect.stringMatching(/^auto(\.http\.otel\.http)?$/), 'sentry.source': 'route', }, op: 'http.server', - origin: 'auto.function.nextjs', + origin: expect.stringMatching(/^auto(\.http\.otel\.http)?$/), span_id: expect.any(String), status: 'internal_error', trace_id: expect.any(String), @@ -166,8 +168,9 @@ test('Should report an error event for errors thrown in getServerSideProps in pa }, start_timestamp: expect.any(Number), timestamp: expect.any(Number), - transaction: '/customPageExtension', - transaction_info: { source: 'route' }, + transaction: 'GET /[param]/customPageExtension', + // TODO: This test fails depending on the next version (next 13: 'custom', next >14: 'route') + // transaction_info: { source: 'custom' }, type: 'transaction', }); }); diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/pages-ssr-errors.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/pages-ssr-errors.test.ts index a67e4328ba1c..10a4cd77f111 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/pages-ssr-errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/pages-ssr-errors.test.ts @@ -8,7 +8,7 @@ test('Will capture error for SSR rendering error with a connected trace (Class C const serverComponentTransaction = waitForTransaction('nextjs-app-dir', async transactionEvent => { return ( - transactionEvent?.transaction === '/pages-router/ssr-error-class' && + transactionEvent?.transaction === 'GET /pages-router/ssr-error-class' && (await errorEventPromise).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id ); }); @@ -26,7 +26,7 @@ test('Will capture error for SSR rendering error with a connected trace (Functio const ssrTransactionPromise = waitForTransaction('nextjs-app-dir', async transactionEvent => { return ( - transactionEvent?.transaction === '/pages-router/ssr-error-fc' && + transactionEvent?.transaction === 'GET /pages-router/ssr-error-fc' && (await errorEventPromise).contexts?.trace?.trace_id === transactionEvent.contexts?.trace?.trace_id ); }); diff --git a/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts index f6906a566050..2380b743cced 100644 --- a/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts +++ b/packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.ts @@ -6,7 +6,7 @@ import { } from '@sentry/core'; import { WINDOW, startBrowserTracingNavigationSpan, startBrowserTracingPageLoadSpan } from '@sentry/react'; import type { Client, TransactionSource } from '@sentry/types'; -import { browserPerformanceTimeOrigin, logger, stripUrlQueryAndFragment } from '@sentry/utils'; +import { browserPerformanceTimeOrigin, logger, parseBaggageHeader, stripUrlQueryAndFragment } from '@sentry/utils'; import type { NEXT_DATA } from 'next/dist/shared/lib/utils'; import RouterImport from 'next/router'; @@ -106,7 +106,15 @@ function extractNextDataTagInformation(): NextDataTagInfo { */ export function pagesRouterInstrumentPageLoad(client: Client): void { const { route, params, sentryTrace, baggage } = extractNextDataTagInformation(); - const name = route || globalObject.location.pathname; + const parsedBaggage = parseBaggageHeader(baggage); + let name = route || globalObject.location.pathname; + + // /_error is the fallback page for all errors. If there is a transaction name for /_error, use that instead + if (parsedBaggage && parsedBaggage['sentry-transaction'] && name === '/_error') { + name = parsedBaggage['sentry-transaction']; + // Strip any HTTP method from the span name + name = name.replace(/^(GET|POST|PUT|DELETE|PATCH|HEAD|OPTIONS|TRACE|CONNECT)\s+/i, ''); + } startBrowserTracingPageLoadSpan( client, diff --git a/packages/nextjs/src/common/pages-router-instrumentation/wrapGetStaticPropsWithSentry.ts b/packages/nextjs/src/common/pages-router-instrumentation/wrapGetStaticPropsWithSentry.ts index accf540f64ff..5d083eb97ca8 100644 --- a/packages/nextjs/src/common/pages-router-instrumentation/wrapGetStaticPropsWithSentry.ts +++ b/packages/nextjs/src/common/pages-router-instrumentation/wrapGetStaticPropsWithSentry.ts @@ -14,7 +14,7 @@ type Props = { [key: string]: unknown }; */ export function wrapGetStaticPropsWithSentry( origGetStaticPropsa: GetStaticProps, - parameterizedRoute: string, + _parameterizedRoute: string, ): GetStaticProps { return new Proxy(origGetStaticPropsa, { apply: async (wrappingTarget, thisArg, args: Parameters>) => { @@ -23,10 +23,7 @@ export function wrapGetStaticPropsWithSentry( } const errorWrappedGetStaticProps = withErrorInstrumentation(wrappingTarget); - return callDataFetcherTraced(errorWrappedGetStaticProps, args, { - parameterizedRoute, - dataFetchingMethodName: 'getStaticProps', - }); + return callDataFetcherTraced(errorWrappedGetStaticProps, args); }, }); } diff --git a/packages/nextjs/src/common/pages-router-instrumentation/wrapPageComponentWithSentry.ts b/packages/nextjs/src/common/pages-router-instrumentation/wrapPageComponentWithSentry.ts index 2914366f9a6b..8b6a45faa63b 100644 --- a/packages/nextjs/src/common/pages-router-instrumentation/wrapPageComponentWithSentry.ts +++ b/packages/nextjs/src/common/pages-router-instrumentation/wrapPageComponentWithSentry.ts @@ -1,6 +1,5 @@ import { captureException, getCurrentScope, withIsolationScope } from '@sentry/core'; import { extractTraceparentData } from '@sentry/utils'; -import { dropNextjsRootContext, escapeNextjsTracing } from '../utils/tracingUtils'; interface FunctionComponent { (...args: unknown[]): unknown; @@ -25,70 +24,64 @@ export function wrapPageComponentWithSentry(pageComponent: FunctionComponent | C if (isReactClassComponent(pageComponent)) { return class SentryWrappedPageComponent extends pageComponent { public render(...args: unknown[]): unknown { - dropNextjsRootContext(); - 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; + 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]) { - dropNextjsRootContext(); - 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; + 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/span-attributes-with-logic-attached.ts b/packages/nextjs/src/common/span-attributes-with-logic-attached.ts index 593ea61b4e28..a272ef525dff 100644 --- a/packages/nextjs/src/common/span-attributes-with-logic-attached.ts +++ b/packages/nextjs/src/common/span-attributes-with-logic-attached.ts @@ -4,3 +4,5 @@ export const TRANSACTION_ATTR_SHOULD_DROP_TRANSACTION = 'sentry.drop_transaction'; export const TRANSACTION_ATTR_SENTRY_TRACE_BACKFILL = 'sentry.sentry_trace_backfill'; + +export const TRANSACTION_ATTR_SENTRY_ROUTE_BACKFILL = 'sentry.route_backfill'; diff --git a/packages/nextjs/src/common/utils/wrapperUtils.ts b/packages/nextjs/src/common/utils/wrapperUtils.ts index ddde0be63a18..159ee669ec09 100644 --- a/packages/nextjs/src/common/utils/wrapperUtils.ts +++ b/packages/nextjs/src/common/utils/wrapperUtils.ts @@ -1,44 +1,13 @@ import type { IncomingMessage, ServerResponse } from 'http'; import { - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, - SPAN_STATUS_ERROR, - SPAN_STATUS_OK, captureException, - continueTrace, + getActiveSpan, + getCurrentScope, + getIsolationScope, + getRootSpan, getTraceData, - startInactiveSpan, - startSpan, - startSpanManual, - withActiveSpan, - withIsolationScope, } from '@sentry/core'; -import type { Span } from '@sentry/types'; -import { isString, vercelWaitUntil } from '@sentry/utils'; - -import { autoEndSpanOnResponseEnd, flushSafelyWithTimeout } from './responseEnd'; -import { commonObjectToIsolationScope, dropNextjsRootContext, escapeNextjsTracing } from './tracingUtils'; - -declare module 'http' { - interface IncomingMessage { - _sentrySpan?: Span; - } -} - -/** - * Grabs a span off a Next.js datafetcher request object, if it was previously put there via - * `setSpanOnRequest`. - * - * @param req The Next.js datafetcher request object - * @returns the span on the request object if there is one, or `undefined` if the request object didn't have one. - */ -export function getSpanFromRequest(req: IncomingMessage): Span | undefined { - return req._sentrySpan; -} - -function setSpanOnRequest(span: Span, req: IncomingMessage): void { - req._sentrySpan = span; -} +import { TRANSACTION_ATTR_SENTRY_ROUTE_BACKFILL } from '../span-attributes-with-logic-attached'; /** * Wraps a function that potentially throws. If it does, the error is passed to `captureException` and rethrown. @@ -55,7 +24,6 @@ export function withErrorInstrumentation any>( } catch (e) { // TODO: Extract error logic from `withSentry` in here or create a new wrapper with said logic or something like that. captureException(e, { mechanism: { handled: false } }); - throw e; } }; @@ -93,79 +61,27 @@ export function withTracedServerSideDataFetcher Pr this: unknown, ...args: Parameters ): Promise<{ data: ReturnType; sentryTrace?: string; baggage?: string }> { - dropNextjsRootContext(); - return escapeNextjsTracing(() => { - const isolationScope = commonObjectToIsolationScope(req); - return withIsolationScope(isolationScope, () => { - isolationScope.setTransactionName(`${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`); - isolationScope.setSDKProcessingMetadata({ - request: req, - }); - - const sentryTrace = - req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined; - const baggage = req.headers?.baggage; - - return continueTrace({ sentryTrace, baggage }, () => { - const requestSpan = getOrStartRequestSpan(req, res, options.requestedRouteName); - return withActiveSpan(requestSpan, () => { - return startSpanManual( - { - op: 'function.nextjs', - name: `${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - }, - }, - async dataFetcherSpan => { - dataFetcherSpan.setStatus({ code: SPAN_STATUS_OK }); - const { 'sentry-trace': sentryTrace, baggage } = getTraceData(); - try { - return { - sentryTrace: sentryTrace, - baggage: baggage, - data: await origDataFetcher.apply(this, args), - }; - } catch (e) { - dataFetcherSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - requestSpan?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - throw e; - } finally { - dataFetcherSpan.end(); - } - }, - ); - }); - }); - }); - }).finally(() => { - vercelWaitUntil(flushSafelyWithTimeout()); + getCurrentScope().setTransactionName(`${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`); + getIsolationScope().setSDKProcessingMetadata({ + request: req, }); - }; -} -function getOrStartRequestSpan(req: IncomingMessage, res: ServerResponse, name: string): Span { - const existingSpan = getSpanFromRequest(req); - if (existingSpan) { - return existingSpan; - } + const span = getActiveSpan(); - const requestSpan = startInactiveSpan({ - name, - forceTransaction: true, - op: 'http.server', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - }, - }); + // Only set the route backfill if the span is not for /_error + if (span && options.requestedRouteName !== '/_error') { + const root = getRootSpan(span); + root.setAttribute(TRANSACTION_ATTR_SENTRY_ROUTE_BACKFILL, options.requestedRouteName); + } - requestSpan.setStatus({ code: SPAN_STATUS_OK }); - setSpanOnRequest(requestSpan, req); - autoEndSpanOnResponseEnd(requestSpan, res); + const { 'sentry-trace': sentryTrace, baggage } = getTraceData(); - return requestSpan; + return { + sentryTrace: sentryTrace, + baggage: baggage, + data: await origDataFetcher.apply(this, args), + }; + }; } /** @@ -178,37 +94,11 @@ function getOrStartRequestSpan(req: IncomingMessage, res: ServerResponse, name: export async function callDataFetcherTraced Promise | any>( origFunction: F, origFunctionArgs: Parameters, - options: { - parameterizedRoute: string; - dataFetchingMethodName: string; - }, ): Promise> { - const { parameterizedRoute, dataFetchingMethodName } = options; - - return startSpan( - { - op: 'function.nextjs', - name: `${dataFetchingMethodName} (${parameterizedRoute})`, - onlyIfParent: true, - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - }, - }, - async dataFetcherSpan => { - dataFetcherSpan.setStatus({ code: SPAN_STATUS_OK }); - - try { - return await origFunction(...origFunctionArgs); - } catch (e) { - dataFetcherSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(e, { mechanism: { handled: false } }); - throw e; - } finally { - dataFetcherSpan.end(); - } - }, - ).finally(() => { - vercelWaitUntil(flushSafelyWithTimeout()); - }); + try { + return await origFunction(...origFunctionArgs); + } catch (e) { + captureException(e, { mechanism: { handled: false } }); + throw e; + } } diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 168288e081fe..979d61b3255e 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -30,6 +30,7 @@ import { DEBUG_BUILD } from '../common/debug-build'; import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor'; import { getVercelEnv } from '../common/getVercelEnv'; import { + TRANSACTION_ATTR_SENTRY_ROUTE_BACKFILL, TRANSACTION_ATTR_SENTRY_TRACE_BACKFILL, TRANSACTION_ATTR_SHOULD_DROP_TRANSACTION, } from '../common/span-attributes-with-logic-attached'; @@ -285,6 +286,7 @@ export function init(options: NodeOptions): NodeClient | undefined { ), ); + // TODO: move this into pre-processing hook getGlobalScope().addEventProcessor( Object.assign( (event => { @@ -303,11 +305,25 @@ export function init(options: NodeOptions): NodeClient | undefined { // eslint-disable-next-line deprecation/deprecation const method = event.contexts.trace.data[SEMATTRS_HTTP_METHOD]; + // eslint-disable-next-line deprecation/deprecation + const target = event.contexts?.trace?.data?.[SEMATTRS_HTTP_TARGET]; const route = event.contexts.trace.data[ATTR_HTTP_ROUTE]; + if (typeof method === 'string' && typeof route === 'string') { event.transaction = `${method} ${route.replace(/\/route$/, '')}`; event.contexts.trace.data[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] = 'route'; } + + // backfill transaction name for pages that would otherwise contain unparameterized routes + if (event.contexts.trace.data['sentry.route_backfill'] && event.transaction !== 'GET /_app') { + event.transaction = `${method} ${event.contexts.trace.data[TRANSACTION_ATTR_SENTRY_ROUTE_BACKFILL]}`; + } + + // Next.js overrides transaction names for page loads that throw an error + // but we want to keep the original target name + if (event.transaction === 'GET /_error' && target) { + event.transaction = `${method ? `${method} ` : ''}${target}`; + } } // Next.js 13 is not correctly picking up tracing data for trace propagation so we use a back-fill strategy diff --git a/packages/nextjs/test/config/wrappers.test.ts b/packages/nextjs/test/config/wrappers.test.ts index 284737f4335d..e2928d59016e 100644 --- a/packages/nextjs/test/config/wrappers.test.ts +++ b/packages/nextjs/test/config/wrappers.test.ts @@ -1,13 +1,12 @@ import type { IncomingMessage, ServerResponse } from 'http'; import * as SentryCore from '@sentry/core'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; import type { Client } from '@sentry/types'; import { wrapGetInitialPropsWithSentry, wrapGetServerSidePropsWithSentry } from '../../src/common'; const startSpanManualSpy = jest.spyOn(SentryCore, 'startSpanManual'); -describe('data-fetching function wrappers should create spans', () => { +describe('data-fetching function wrappers should not create manual spans', () => { const route = '/tricks/[trickName]'; let req: IncomingMessage; let res: ServerResponse; @@ -35,17 +34,7 @@ describe('data-fetching function wrappers should create spans', () => { const wrappedOriginal = wrapGetServerSidePropsWithSentry(origFunction, route); await wrappedOriginal({ req, res } as any); - expect(startSpanManualSpy).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'getServerSideProps (/tricks/[trickName])', - op: 'function.nextjs', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - }, - }), - expect.any(Function), - ); + expect(startSpanManualSpy).not.toHaveBeenCalled(); }); test('wrapGetInitialPropsWithSentry', async () => { @@ -54,16 +43,44 @@ describe('data-fetching function wrappers should create spans', () => { const wrappedOriginal = wrapGetInitialPropsWithSentry(origFunction); await wrappedOriginal({ req, res, pathname: route } as any); - expect(startSpanManualSpy).toHaveBeenCalledWith( - expect.objectContaining({ - name: 'getInitialProps (/tricks/[trickName])', - op: 'function.nextjs', - attributes: { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs', - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - }, - }), - expect.any(Function), - ); + expect(startSpanManualSpy).not.toHaveBeenCalled(); + }); + + test('wrapped function sets route backfill attribute when called within an active span', async () => { + const mockSetAttribute = jest.fn(); + const mockGetActiveSpan = jest.spyOn(SentryCore, 'getActiveSpan').mockReturnValue({ + setAttribute: mockSetAttribute, + } as any); + const mockGetRootSpan = jest.spyOn(SentryCore, 'getRootSpan').mockReturnValue({ + setAttribute: mockSetAttribute, + } as any); + + const origFunction = jest.fn(async () => ({ props: {} })); + const wrappedOriginal = wrapGetServerSidePropsWithSentry(origFunction, route); + + await wrappedOriginal({ req, res } as any); + + expect(mockGetActiveSpan).toHaveBeenCalled(); + expect(mockGetRootSpan).toHaveBeenCalled(); + expect(mockSetAttribute).toHaveBeenCalledWith('sentry.route_backfill', '/tricks/[trickName]'); + }); + + test('wrapped function does not set route backfill attribute for /_error route', async () => { + const mockSetAttribute = jest.fn(); + const mockGetActiveSpan = jest.spyOn(SentryCore, 'getActiveSpan').mockReturnValue({ + setAttribute: mockSetAttribute, + } as any); + const mockGetRootSpan = jest.spyOn(SentryCore, 'getRootSpan').mockReturnValue({ + setAttribute: mockSetAttribute, + } as any); + + const origFunction = jest.fn(async () => ({ props: {} })); + const wrappedOriginal = wrapGetServerSidePropsWithSentry(origFunction, '/_error'); + + await wrappedOriginal({ req, res } as any); + + expect(mockGetActiveSpan).toHaveBeenCalled(); + expect(mockGetRootSpan).not.toHaveBeenCalled(); + expect(mockSetAttribute).not.toHaveBeenCalled(); }); });