feat(nextjs): Enable nextjs otel spans pages router (#13960)

Author: chargome

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,
     });
   }
 }

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"

dev-packages/e2e-tests/test-applications/nextjs-13/pages/customPageExtension.page.tsx renamed to dev-packages/e2e-tests/test-applications/nextjs-13/pages/[param]/customPageExtension.page.tsx

@@ -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');
+});

dev-packages/e2e-tests/test-applications/nextjs-13/pages/error-getServerSideProps.tsx renamed to dev-packages/e2e-tests/test-applications/nextjs-13/pages/[param]/error-getServerSideProps.tsx

@@ -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',
       },

dev-packages/e2e-tests/test-applications/nextjs-13/tests/client/pages-dir-pageload.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',
       },

dev-packages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getInitialProps.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',
   });
 });

dev-packages/e2e-tests/test-applications/nextjs-13/tests/isomorphic/getServerSideProps.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
     );
   });

dev-packages/e2e-tests/test-applications/nextjs-13/tests/server/getServerSideProps.test.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,

dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/pages-ssr-errors.test.ts

@@ -14,7 +14,7 @@ type Props = { [key: string]: unknown };
  */
 export function wrapGetStaticPropsWithSentry(
   origGetStaticPropsa: GetStaticProps<Props>,
-  parameterizedRoute: string,
+  _parameterizedRoute: string,
 ): GetStaticProps<Props> {
   return new Proxy(origGetStaticPropsa, {
     apply: async (wrappingTarget, thisArg, args: Parameters<GetStaticProps<Props>>) => {
@@ -23,10 +23,7 @@ export function wrapGetStaticPropsWithSentry(
       }
 
       const errorWrappedGetStaticProps = withErrorInstrumentation(wrappingTarget);
-      return callDataFetcherTraced(errorWrappedGetStaticProps, args, {
-        parameterizedRoute,
-        dataFetchingMethodName: 'getStaticProps',
-      });
+      return callDataFetcherTraced(errorWrappedGetStaticProps, args);
     },
   });
 }

packages/nextjs/src/client/routing/pagesRouterRoutingInstrumentation.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;
+          }
         });
       },
     });

packages/nextjs/src/common/pages-router-instrumentation/wrapGetStaticPropsWithSentry.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';