fix(nextjs): Continue traces in data fetchers when there is an already active transaction on the hub (#8073)

Author: Luca Forstner

packages/nextjs/src/server/utils/wrapperUtils.ts

@@ -5,7 +5,7 @@ import {
   runWithAsyncContext,
   startTransaction,
 } from '@sentry/core';
-import type { Transaction } from '@sentry/types';
+import type { Span, Transaction } from '@sentry/types';
 import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@sentry/utils';
 import type { IncomingMessage, ServerResponse } from 'http';
 
@@ -82,7 +82,8 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
   return async function (this: unknown, ...args: Parameters<F>): Promise<ReturnType<F>> {
     return runWithAsyncContext(async () => {
       const hub = getCurrentHub();
-      let requestTransaction: Transaction | undefined = getTransactionFromRequest(req);
+      const scope = hub.getScope();
+      const previousSpan: Span | undefined = getTransactionFromRequest(req) ?? scope.getSpan();
       let dataFetcherSpan;
 
       const sentryTraceHeader = req.headers['sentry-trace'];
@@ -93,7 +94,8 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
       const dynamicSamplingContext = baggageHeaderToDynamicSamplingContext(rawBaggageString);
 
       if (platformSupportsStreaming()) {
-        if (requestTransaction === undefined) {
+        let spanToContinue: Span;
+        if (previousSpan === undefined) {
           const newTransaction = startTransaction(
             {
               op: 'http.server',
@@ -109,8 +111,6 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
             { request: req },
           );
 
-          requestTransaction = newTransaction;
-
           if (platformSupportsStreaming()) {
             // On platforms that don't support streaming, doing things after res.end() is unreliable.
             autoEndTransactionOnResponseEnd(newTransaction, res);
@@ -119,9 +119,12 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
           // Link the transaction and the request together, so that when we would normally only have access to one, it's
           // still possible to grab the other.
           setTransactionOnRequest(newTransaction, req);
+          spanToContinue = newTransaction;
+        } else {
+          spanToContinue = previousSpan;
         }
 
-        dataFetcherSpan = requestTransaction.startChild({
+        dataFetcherSpan = spanToContinue.startChild({
           op: 'function.nextjs',
           description: `${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`,
           status: 'ok',
@@ -140,22 +143,20 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
         });
       }
 
-      const currentScope = hub.getScope();
-      if (currentScope) {
-        currentScope.setSpan(dataFetcherSpan);
-        currentScope.setSDKProcessingMetadata({ request: req });
-      }
+      scope.setSpan(dataFetcherSpan);
+      scope.setSDKProcessingMetadata({ request: req });
 
       try {
         return await origDataFetcher.apply(this, args);
       } catch (e) {
         // Since we finish the span before the error can bubble up and trigger the handlers in `registerErrorInstrumentation`
         // that set the transaction status, we need to manually set the status of the span & transaction
         dataFetcherSpan.setStatus('internal_error');
-        requestTransaction?.setStatus('internal_error');
+        previousSpan?.setStatus('internal_error');
         throw e;
       } finally {
         dataFetcherSpan.finish();
+        scope.setSpan(previousSpan);
         if (!platformSupportsStreaming()) {
           await flushQueue();
         }

packages/nextjs/src/server/wrapAppGetInitialPropsWithSentry.ts

@@ -31,7 +31,8 @@ export function wrapAppGetInitialPropsWithSentry(origAppGetInitialProps: AppGetI
       const { req, res } = context.ctx;
 
       const errorWrappedAppGetInitialProps = withErrorInstrumentation(wrappingTarget);
-      const options = getCurrentHub().getClient()?.getOptions();
+      const hub = getCurrentHub();
+      const options = hub.getClient()?.getOptions();
 
       // Generally we can assume that `req` and `res` are always defined on the server:
       // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object
@@ -51,7 +52,7 @@ export function wrapAppGetInitialPropsWithSentry(origAppGetInitialProps: AppGetI
           };
         } = await tracedGetInitialProps.apply(thisArg, args);
 
-        const requestTransaction = getTransactionFromRequest(req);
+        const requestTransaction = getTransactionFromRequest(req) ?? hub.getScope().getTransaction();
 
         // Per definition, `pageProps` is not optional, however an increased amount of users doesn't seem to call
         // `App.getInitialProps(appContext)` in their custom `_app` pages which is required as per

packages/nextjs/src/server/wrapErrorGetInitialPropsWithSentry.ts

@@ -34,7 +34,8 @@ export function wrapErrorGetInitialPropsWithSentry(
       const { req, res } = context;
 
       const errorWrappedGetInitialProps = withErrorInstrumentation(wrappingTarget);
-      const options = getCurrentHub().getClient()?.getOptions();
+      const hub = getCurrentHub();
+      const options = hub.getClient()?.getOptions();
 
       // Generally we can assume that `req` and `res` are always defined on the server:
       // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object
@@ -52,7 +53,7 @@ export function wrapErrorGetInitialPropsWithSentry(
           _sentryBaggage?: string;
         } = await tracedGetInitialProps.apply(thisArg, args);
 
-        const requestTransaction = getTransactionFromRequest(req);
+        const requestTransaction = getTransactionFromRequest(req) ?? hub.getScope().getTransaction();
         if (requestTransaction) {
           errorGetInitialProps._sentryTraceData = requestTransaction.toTraceparent();
 

packages/nextjs/src/server/wrapGetInitialPropsWithSentry.ts

@@ -30,7 +30,8 @@ export function wrapGetInitialPropsWithSentry(origGetInitialProps: GetInitialPro
       const { req, res } = context;
 
       const errorWrappedGetInitialProps = withErrorInstrumentation(wrappingTarget);
-      const options = getCurrentHub().getClient()?.getOptions();
+      const hub = getCurrentHub();
+      const options = hub.getClient()?.getOptions();
 
       // Generally we can assume that `req` and `res` are always defined on the server:
       // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object
@@ -48,7 +49,7 @@ export function wrapGetInitialPropsWithSentry(origGetInitialProps: GetInitialPro
           _sentryBaggage?: string;
         } = await tracedGetInitialProps.apply(thisArg, args);
 
-        const requestTransaction = getTransactionFromRequest(req);
+        const requestTransaction = getTransactionFromRequest(req) ?? hub.getScope().getTransaction();
         if (requestTransaction) {
           initialProps._sentryTraceData = requestTransaction.toTraceparent();
 

packages/nextjs/src/server/wrapGetServerSidePropsWithSentry.ts

@@ -31,7 +31,8 @@ export function wrapGetServerSidePropsWithSentry(
       const { req, res } = context;
 
       const errorWrappedGetServerSideProps = withErrorInstrumentation(wrappingTarget);
-      const options = getCurrentHub().getClient()?.getOptions();
+      const hub = getCurrentHub();
+      const options = hub.getClient()?.getOptions();
 
       if (hasTracingEnabled() && options?.instrumenter === 'sentry') {
         const tracedGetServerSideProps = withTracedServerSideDataFetcher(errorWrappedGetServerSideProps, req, res, {
@@ -45,7 +46,7 @@ export function wrapGetServerSidePropsWithSentry(
         >);
 
         if (serverSideProps && 'props' in serverSideProps) {
-          const requestTransaction = getTransactionFromRequest(req);
+          const requestTransaction = getTransactionFromRequest(req) ?? hub.getScope().getTransaction();
           if (requestTransaction) {
             serverSideProps.props._sentryTraceData = requestTransaction.toTraceparent();
 

packages/nextjs/test/config/wrappers.test.ts

@@ -6,6 +6,7 @@ import type { IncomingMessage, ServerResponse } from 'http';
 import { wrapGetInitialPropsWithSentry, wrapGetServerSidePropsWithSentry } from '../../src/server';
 
 const startTransactionSpy = jest.spyOn(SentryCore, 'startTransaction');
+const originalGetCurrentHub = jest.requireActual('@sentry/node').getCurrentHub;
 
 // The wrap* functions require the hub to have tracing extensions. This is normally called by the NodeClient
 // constructor but the client isn't used in these tests.
@@ -21,13 +22,18 @@ describe('data-fetching function wrappers', () => {
       req = { headers: {}, url: 'http://dogs.are.great/tricks/kangaroo' } as IncomingMessage;
       res = { end: jest.fn() } as unknown as ServerResponse;
 
-      jest.spyOn(SentryCore, 'hasTracingEnabled').mockReturnValueOnce(true);
-      jest.spyOn(SentryNode, 'getCurrentHub').mockReturnValueOnce({
-        getClient: () =>
+      jest.spyOn(SentryCore, 'hasTracingEnabled').mockReturnValue(true);
+      jest.spyOn(SentryNode, 'getCurrentHub').mockImplementation(() => {
+        const hub = originalGetCurrentHub();
+
+        hub.getClient = () =>
           ({
             getOptions: () => ({ instrumenter: 'sentry' }),
-          } as any),
-      } as any);
+            getDsn: () => {},
+          } as any);
+
+        return hub;
+      });
     });
 
     afterEach(() => {