fix(nextjs): Trace with performance disabled (#9389)

getsentry · Oct 27, 2023 · 3379f93 · 3379f93
1 parent 43ddbbe
commit 3379f93
Show file tree

Hide file tree

Showing 10 changed files with 47 additions and 79 deletions.
diff --git a/packages/nextjs/src/common/utils/edgeWrapperUtils.ts b/packages/nextjs/src/common/utils/edgeWrapperUtils.ts
@@ -1,4 +1,4 @@
-import { captureException, flush, getCurrentHub, hasTracingEnabled, startTransaction } from '@sentry/core';
+import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/core';
 import type { Span } from '@sentry/types';
 import { addExceptionMechanism, logger, objectify, tracingContextFromHeaders } from '@sentry/utils';
 
@@ -18,40 +18,38 @@ export function withEdgeWrapping<H extends EdgeRouteHandler>(
 
     let span: Span | undefined;
 
-    if (hasTracingEnabled()) {
-      if (prevSpan) {
-        span = prevSpan.startChild({
-          description: options.spanDescription,
-          op: options.spanOp,
-          origin: 'auto.function.nextjs',
-        });
-      } else if (req instanceof Request) {
-        const sentryTrace = req.headers.get('sentry-trace') || '';
-        const baggage = req.headers.get('baggage');
-        const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders(
-          sentryTrace,
-          baggage,
-        );
-        currentScope.setPropagationContext(propagationContext);
-        if (traceparentData) {
-          __DEBUG_BUILD__ && logger.log(`[Tracing] Continuing trace ${traceparentData.traceId}.`);
-        }
-
-        span = startTransaction({
-          name: options.spanDescription,
-          op: options.spanOp,
-          origin: 'auto.ui.nextjs.withEdgeWrapping',
-          ...traceparentData,
-          metadata: {
-            dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
-            source: 'route',
-          },
-        });
+    if (prevSpan) {
+      span = prevSpan.startChild({
+        description: options.spanDescription,
+        op: options.spanOp,
+        origin: 'auto.function.nextjs',
+      });
+    } else if (req instanceof Request) {
+      const sentryTrace = req.headers.get('sentry-trace') || '';
+      const baggage = req.headers.get('baggage');
+      const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders(
+        sentryTrace,
+        baggage,
+      );
+      currentScope.setPropagationContext(propagationContext);
+      if (traceparentData) {
+        __DEBUG_BUILD__ && logger.log(`[Tracing] Continuing trace ${traceparentData.traceId}.`);
       }
 
-      currentScope?.setSpan(span);
+      span = startTransaction({
+        name: options.spanDescription,
+        op: options.spanOp,
+        origin: 'auto.ui.nextjs.withEdgeWrapping',
+        ...traceparentData,
+        metadata: {
+          dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
+          source: 'route',
+        },
+      });
     }
 
+    currentScope?.setSpan(span);
+
     try {
       const handlerResult: ReturnType<H> = await handler.apply(this, args);
 

diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts
@@ -1,10 +1,4 @@
-import {
-  captureException,
-  getCurrentHub,
-  hasTracingEnabled,
-  runWithAsyncContext,
-  startTransaction,
-} from '@sentry/core';
+import { captureException, getCurrentHub, runWithAsyncContext, startTransaction } from '@sentry/core';
 import type { Transaction } from '@sentry/types';
 import {
   addExceptionMechanism,
@@ -91,7 +85,7 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri
 
           currentScope.setSDKProcessingMetadata({ request: req });
 
-          if (hasTracingEnabled(options) && options?.instrumenter === 'sentry') {
+          if (options?.instrumenter === 'sentry') {
             const sentryTrace =
               req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined;
             const baggage = req.headers?.baggage;

diff --git a/packages/nextjs/src/common/wrapAppGetInitialPropsWithSentry.ts b/packages/nextjs/src/common/wrapAppGetInitialPropsWithSentry.ts
@@ -1,4 +1,4 @@
-import { getCurrentHub, hasTracingEnabled } from '@sentry/core';
+import { getCurrentHub } from '@sentry/core';
 import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils';
 import type App from 'next/app';
 
@@ -37,7 +37,7 @@ export function wrapAppGetInitialPropsWithSentry(origAppGetInitialProps: AppGetI
       // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object
       // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher
       // span with each other when there are no req or res objects, we simply do not trace them at all here.
-      if (hasTracingEnabled() && req && res && options?.instrumenter === 'sentry') {
+      if (req && res && options?.instrumenter === 'sentry') {
         const tracedGetInitialProps = withTracedServerSideDataFetcher(errorWrappedAppGetInitialProps, req, res, {
           dataFetcherRouteName: '/_app',
           requestedRouteName: context.ctx.pathname,

diff --git a/packages/nextjs/src/common/wrapDocumentGetInitialPropsWithSentry.ts b/packages/nextjs/src/common/wrapDocumentGetInitialPropsWithSentry.ts
@@ -1,4 +1,4 @@
-import { getCurrentHub, hasTracingEnabled } from '@sentry/core';
+import { getCurrentHub } from '@sentry/core';
 import type Document from 'next/document';
 
 import { isBuild } from './utils/isBuild';
@@ -33,7 +33,7 @@ export function wrapDocumentGetInitialPropsWithSentry(
       // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object
       // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher
       // span with each other when there are no req or res objects, we simply do not trace them at all here.
-      if (hasTracingEnabled() && req && res && options?.instrumenter === 'sentry') {
+      if (req && res && options?.instrumenter === 'sentry') {
         const tracedGetInitialProps = withTracedServerSideDataFetcher(errorWrappedGetInitialProps, req, res, {
           dataFetcherRouteName: '/_document',
           requestedRouteName: context.pathname,

diff --git a/packages/nextjs/src/common/wrapErrorGetInitialPropsWithSentry.ts b/packages/nextjs/src/common/wrapErrorGetInitialPropsWithSentry.ts
@@ -1,4 +1,4 @@
-import { getCurrentHub, hasTracingEnabled } from '@sentry/core';
+import { getCurrentHub } from '@sentry/core';
 import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils';
 import type { NextPageContext } from 'next';
 import type { ErrorProps } from 'next/error';
@@ -40,7 +40,7 @@ export function wrapErrorGetInitialPropsWithSentry(
       // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object
       // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher
       // span with each other when there are no req or res objects, we simply do not trace them at all here.
-      if (hasTracingEnabled() && req && res && options?.instrumenter === 'sentry') {
+      if (req && res && options?.instrumenter === 'sentry') {
         const tracedGetInitialProps = withTracedServerSideDataFetcher(errorWrappedGetInitialProps, req, res, {
           dataFetcherRouteName: '/_error',
           requestedRouteName: context.pathname,

diff --git a/packages/nextjs/src/common/wrapGetInitialPropsWithSentry.ts b/packages/nextjs/src/common/wrapGetInitialPropsWithSentry.ts
@@ -1,4 +1,4 @@
-import { getCurrentHub, hasTracingEnabled } from '@sentry/core';
+import { getCurrentHub } from '@sentry/core';
 import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils';
 import type { NextPage } from 'next';
 
@@ -36,7 +36,7 @@ export function wrapGetInitialPropsWithSentry(origGetInitialProps: GetInitialPro
       // https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object
       // This does not seem to be the case in dev mode. Because we have no clean way of associating the the data fetcher
       // span with each other when there are no req or res objects, we simply do not trace them at all here.
-      if (hasTracingEnabled() && req && res && options?.instrumenter === 'sentry') {
+      if (req && res && options?.instrumenter === 'sentry') {
         const tracedGetInitialProps = withTracedServerSideDataFetcher(errorWrappedGetInitialProps, req, res, {
           dataFetcherRouteName: context.pathname,
           requestedRouteName: context.pathname,

diff --git a/packages/nextjs/src/common/wrapGetServerSidePropsWithSentry.ts b/packages/nextjs/src/common/wrapGetServerSidePropsWithSentry.ts
@@ -1,4 +1,4 @@
-import { getCurrentHub, hasTracingEnabled } from '@sentry/core';
+import { getCurrentHub } from '@sentry/core';
 import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils';
 import type { GetServerSideProps } from 'next';
 
@@ -33,7 +33,7 @@ export function wrapGetServerSidePropsWithSentry(
       const hub = getCurrentHub();
       const options = hub.getClient()?.getOptions();
 
-      if (hasTracingEnabled() && options?.instrumenter === 'sentry') {
+      if (options?.instrumenter === 'sentry') {
         const tracedGetServerSideProps = withTracedServerSideDataFetcher(errorWrappedGetServerSideProps, req, res, {
           dataFetcherRouteName: parameterizedRoute,
           requestedRouteName: parameterizedRoute,

diff --git a/packages/nextjs/src/common/wrapGetStaticPropsWithSentry.ts b/packages/nextjs/src/common/wrapGetStaticPropsWithSentry.ts
@@ -1,4 +1,4 @@
-import { getCurrentHub, hasTracingEnabled } from '@sentry/core';
+import { getCurrentHub } from '@sentry/core';
 import type { GetStaticProps } from 'next';
 
 import { isBuild } from './utils/isBuild';
@@ -26,7 +26,7 @@ export function wrapGetStaticPropsWithSentry(
       const errorWrappedGetStaticProps = withErrorInstrumentation(wrappingTarget);
       const options = getCurrentHub().getClient()?.getOptions();
 
-      if (hasTracingEnabled() && options?.instrumenter === 'sentry') {
+      if (options?.instrumenter === 'sentry') {
         return callDataFetcherTraced(errorWrappedGetStaticProps, args, {
           parameterizedRoute,
           dataFetchingMethodName: 'getStaticProps',

diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts
@@ -1,4 +1,3 @@
-import { hasTracingEnabled } from '@sentry/core';
 import { RewriteFrames } from '@sentry/integrations';
 import type { NodeOptions } from '@sentry/node';
 import { configureScope, getCurrentHub, init as nodeInit, Integrations } from '@sentry/node';
@@ -142,12 +141,10 @@ function addServerIntegrations(options: NodeOptions): void {
     _options: { exitEvenIfOtherHandlersAreRegistered: false },
   });
 
-  if (hasTracingEnabled(options)) {
-    const defaultHttpTracingIntegration = new Integrations.Http({ tracing: true });
-    integrations = addOrUpdateIntegration(defaultHttpTracingIntegration, integrations, {
-      _tracing: {},
-    });
-  }
+  const defaultHttpTracingIntegration = new Integrations.Http({ tracing: true });
+  integrations = addOrUpdateIntegration(defaultHttpTracingIntegration, integrations, {
+    _tracing: {},
+  });
 
   options.integrations = integrations;
 }

diff --git a/packages/nextjs/test/serverSdk.test.ts b/packages/nextjs/test/serverSdk.test.ts
@@ -186,15 +186,6 @@ describe('Server init()', () => {
         expect(httpIntegration).toEqual(expect.objectContaining({ _tracing: {} }));
       });
 
-      it('does not add `Http` integration if tracing not enabled in SDK', () => {
-        init({});
-
-        const nodeInitOptions = nodeInit.mock.calls[0][0] as ModifiedInitOptions;
-        const httpIntegration = findIntegrationByName(nodeInitOptions.integrations, 'Http');
-
-        expect(httpIntegration).toBeUndefined();
-      });
-
       it('forces `_tracing = true` if `tracesSampleRate` is set', () => {
         init({
           tracesSampleRate: 1.0,
@@ -220,18 +211,6 @@ describe('Server init()', () => {
         expect(httpIntegration).toBeDefined();
         expect(httpIntegration).toEqual(expect.objectContaining({ _tracing: {} }));
       });
-
-      it('does not force `_tracing = true` if tracing not enabled in SDK', () => {
-        init({
-          integrations: [new Integrations.Http({ tracing: false })],
-        });
-
-        const nodeInitOptions = nodeInit.mock.calls[0][0] as ModifiedInitOptions;
-        const httpIntegration = findIntegrationByName(nodeInitOptions.integrations, 'Http');
-
-        expect(httpIntegration).toBeDefined();
-        expect(httpIntegration).toEqual(expect.objectContaining({ _tracing: undefined }));
-      });
     });
   });
 });