feat(nextjs): Add status to data-fetcher spans (#5777)

getsentry · Sep 20, 2022 · 17e4cb8 · 17e4cb8
1 parent 9a6f98e
commit 17e4cb8
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 4 deletions.
diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts
@@ -98,6 +98,7 @@ export function callTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
         op: 'nextjs.data.server',
         name: options.requestedRouteName,
         ...traceparentData,
+        status: 'ok',
         metadata: {
           source: 'route',
           dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
@@ -116,6 +117,7 @@ export function callTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
     const dataFetcherSpan = requestTransaction.startChild({
       op: 'nextjs.data.server',
       description: `${options.dataFetchingMethodName} (${options.dataFetcherRouteName})`,
+      status: 'ok',
     });
 
     const currentScope = getCurrentHub().getScope();
@@ -137,6 +139,17 @@ export function callTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
 
     try {
       return await origFunction(...origFunctionArguments);
+    } 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');
+
+      const transaction = dataFetcherSpan.transaction;
+      if (transaction) {
+        transaction.setStatus('internal_error');
+      }
+
+      throw e;
     } finally {
       dataFetcherSpan.finish();
     }
@@ -178,14 +191,17 @@ export async function callDataFetcherTraced<F extends (...args: any[]) => Promis
   const span = transaction.startChild({
     op: 'nextjs.data.server',
     description: `${dataFetchingMethodName} (${parameterizedRoute})`,
+    status: 'ok',
   });
 
   try {
     return await origFunction(...origFunctionArgs);
   } catch (err) {
-    if (span) {
-      span.finish();
-    }
+    // 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
+    transaction.setStatus('internal_error');
+    span.setStatus('internal_error');
+    span.finish();
 
     // TODO Copy more robust error handling over from `withSentry`
     captureException(err);

diff --git a/packages/nextjs/test/integration/test/server/errorServerSideProps.js b/packages/nextjs/test/integration/test/server/errorServerSideProps.js
@@ -1,7 +1,7 @@
 const assert = require('assert');
 
 const { sleep } = require('../utils/common');
-const { getAsync, interceptEventRequest } = require('../utils/server');
+const { getAsync, interceptEventRequest, interceptTracingRequest } = require('../utils/server');
 
 module.exports = async ({ url: urlBase, argv }) => {
   const url = `${urlBase}/withErrorServerSideProps`;
@@ -28,8 +28,32 @@ module.exports = async ({ url: urlBase, argv }) => {
     'errorServerSideProps',
   );
 
+  const capturedTransactionRequest = interceptTracingRequest(
+    {
+      contexts: {
+        trace: {
+          op: 'nextjs.data.server',
+          status: 'internal_error',
+        },
+      },
+      transaction: '/withErrorServerSideProps',
+      transaction_info: {
+        source: 'route',
+        changes: [],
+        propagations: 0,
+      },
+      type: 'transaction',
+      request: {
+        url,
+      },
+    },
+    argv,
+    'errorServerSideProps',
+  );
+
   await getAsync(url);
   await sleep(250);
 
   assert.ok(capturedRequest.isDone(), 'Did not intercept expected request');
+  assert.ok(capturedTransactionRequest.isDone(), 'Did not intercept expected transaction request');
 };
diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.js b/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.js
@@ -11,6 +11,7 @@ module.exports = async ({ url: urlBase, argv }) => {
       contexts: {
         trace: {
           op: 'nextjs.data.server',
+          status: 'ok',
         },
       },
       transaction: '/[id]/withInitialProps',

diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.js b/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.js
@@ -11,6 +11,7 @@ module.exports = async ({ url: urlBase, argv }) => {
       contexts: {
         trace: {
           op: 'nextjs.data.server',
+          status: 'ok',
         },
       },
       transaction: '/[id]/withServerSideProps',