feat(nexjs): Sample out low-quality spans on older Next.js versions (#11722)

Luca Forstner · web-flow · commit 92bc6c6f257d · 2024-04-24T12:40:48.000+02:00
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -22,6 +22,8 @@ import type {
   SessionAggregates,
   SeverityLevel,
   Span,
+  SpanAttributes,
+  SpanContextData,
   StartSpanOptions,
   TransactionEvent,
   Transport,
@@ -416,6 +418,20 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
     callback: (feedback: FeedbackEvent, options?: { includeReplay: boolean }) => void,
   ): void;
 
+  /** @inheritdoc */
+  public on(
+    hook: 'beforeSampling',
+    callback: (
+      samplingData: {
+        spanAttributes: SpanAttributes;
+        spanName: string;
+        parentSampled?: boolean;
+        parentContext?: SpanContextData;
+      },
+      samplingDecision: { decision: boolean },
+    ) => void,
+  ): void;
+
   /** @inheritdoc */
   public on(
     hook: 'startPageLoadSpan',
@@ -442,6 +458,18 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
     this._hooks[hook].push(callback);
   }
 
+  /** @inheritdoc */
+  public emit(
+    hook: 'beforeSampling',
+    samplingData: {
+      spanAttributes: SpanAttributes;
+      spanName: string;
+      parentSampled?: boolean;
+      parentContext?: SpanContextData;
+    },
+    samplingDecision: { decision: boolean },
+  ): void;
+
   /** @inheritdoc */
   public emit(hook: 'spanStart', span: Span): void;
 
diff --git a/packages/nextjs/src/common/utils/wrapperUtils.ts b/packages/nextjs/src/common/utils/wrapperUtils.ts
@@ -17,7 +17,7 @@ import { isString } from '@sentry/utils';
 
 import { platformSupportsStreaming } from './platformSupportsStreaming';
 import { autoEndSpanOnResponseEnd, flushQueue } from './responseEnd';
-import { commonObjectToIsolationScope } from './tracingUtils';
+import { commonObjectToIsolationScope, escapeNextjsTracing } from './tracingUtils';
 
 declare module 'http' {
   interface IncomingMessage {
@@ -90,44 +90,46 @@ export function withTracedServerSideDataFetcher<F extends (...args: any[]) => Pr
   },
 ): (...params: Parameters<F>) => Promise<ReturnType<F>> {
   return async function (this: unknown, ...args: Parameters<F>): Promise<ReturnType<F>> {
-    const isolationScope = commonObjectToIsolationScope(req);
-    return withIsolationScope(isolationScope, () => {
-      isolationScope.setSDKProcessingMetadata({
-        request: req,
-      });
+    return escapeNextjsTracing(() => {
+      const isolationScope = commonObjectToIsolationScope(req);
+      return withIsolationScope(isolationScope, () => {
+        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',
+        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 });
-              try {
-                return 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();
-                if (!platformSupportsStreaming()) {
-                  await flushQueue();
+              async dataFetcherSpan => {
+                dataFetcherSpan.setStatus({ code: SPAN_STATUS_OK });
+                try {
+                  return 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();
+                  if (!platformSupportsStreaming()) {
+                    await flushQueue();
+                  }
                 }
-              }
-            },
-          );
+              },
+            );
+          });
         });
       });
     });
diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts
@@ -22,6 +22,22 @@ const globalWithInjectedValues = GLOBAL_OBJ as typeof GLOBAL_OBJ & {
   __sentryRewritesTunnelPath__?: string;
 };
 
+// https://github.com/lforst/nextjs-fork/blob/9051bc44d969a6e0ab65a955a2fc0af522a83911/packages/next/src/server/lib/trace/constants.ts#L11
+const NEXTJS_SPAN_NAME_PREFIXES = [
+  'BaseServer.',
+  'LoadComponents.',
+  'NextServer.',
+  'createServer.',
+  'startServer.',
+  'NextNodeServer.',
+  'Render.',
+  'AppRender.',
+  'Router.',
+  'Node.',
+  'AppRouteRouteHandlers.',
+  'ResolveMetadata.',
+];
+
 /**
  * A passthrough error boundary for the server that doesn't depend on any react. Error boundaries don't catch SSR errors
  * so they should simply be a passthrough.
@@ -90,7 +106,7 @@ export function init(options: NodeOptions): void {
     customDefaultIntegrations.push(distDirRewriteFramesIntegration({ distDirName }));
   }
 
-  const opts = {
+  const opts: NodeOptions = {
     environment: process.env.SENTRY_ENVIRONMENT || getVercelEnv(false) || process.env.NODE_ENV,
     defaultIntegrations: customDefaultIntegrations,
     ...options,
@@ -113,6 +129,20 @@ export function init(options: NodeOptions): void {
 
   nodeInit(opts);
 
+  const client = getClient();
+  client?.on('beforeSampling', ({ spanAttributes, spanName, parentSampled, parentContext }, samplingDecision) => {
+    // If we encounter a span emitted by Next.js, we do not want to sample it
+    // The reason for this is that the data quality of the spans varies, it is different per version of Next,
+    // and we need to keep our manual instrumentation around for the edge runtime anyhow.
+    // BUT we only do this if we don't have a parent span with a sampling decision yet (or if the parent is remote)
+    if (
+      (spanAttributes['next.span_type'] || NEXTJS_SPAN_NAME_PREFIXES.some(prefix => spanName.startsWith(prefix))) &&
+      (parentSampled === undefined || parentContext?.isRemote)
+    ) {
+      samplingDecision.decision = false;
+    }
+  });
+
   addEventProcessor(
     Object.assign(
       (event => {
diff --git a/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts b/packages/nextjs/test/integration/test/server/errorApiEndpoint.test.ts
@@ -34,7 +34,7 @@ describe('Error API Endpoints', () => {
     const envelopes = await env.getMultipleEnvelopeRequest({
       url,
       envelopeType: 'transaction',
-      count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
+      count: 1,
     });
 
     const sentryTransactionEnvelope = envelopes.find(envelope => {
diff --git a/packages/nextjs/test/integration/test/server/errorServerSideProps.test.ts b/packages/nextjs/test/integration/test/server/errorServerSideProps.test.ts
@@ -33,7 +33,7 @@ describe('Error Server-side Props', () => {
     const envelopes = await env.getMultipleEnvelopeRequest({
       url,
       envelopeType: 'transaction',
-      count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
+      count: 1,
     });
 
     const sentryTransactionEnvelope = envelopes.find(envelope => {
diff --git a/packages/nextjs/test/integration/test/server/tracing200.test.ts b/packages/nextjs/test/integration/test/server/tracing200.test.ts
@@ -8,7 +8,7 @@ describe('Tracing 200', () => {
     const envelopes = await env.getMultipleEnvelopeRequest({
       url,
       envelopeType: 'transaction',
-      count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
+      count: 1,
     });
 
     const sentryTransactionEnvelope = envelopes.find(envelope => {
diff --git a/packages/nextjs/test/integration/test/server/tracing500.test.ts b/packages/nextjs/test/integration/test/server/tracing500.test.ts
@@ -8,7 +8,7 @@ describe('Tracing 500', () => {
     const envelopes = await env.getMultipleEnvelopeRequest({
       url,
       envelopeType: 'transaction',
-      count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
+      count: 1,
     });
 
     const sentryTransactionEnvelope = envelopes.find(envelope => {
diff --git a/packages/nextjs/test/integration/test/server/tracingHttp.test.ts b/packages/nextjs/test/integration/test/server/tracingHttp.test.ts
@@ -8,7 +8,7 @@ describe('Tracing HTTP', () => {
     const envelopes = await env.getMultipleEnvelopeRequest({
       url,
       envelopeType: 'transaction',
-      count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
+      count: 1,
     });
 
     const sentryTransactionEnvelope = envelopes.find(envelope => {
diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.test.ts b/packages/nextjs/test/integration/test/server/tracingServerGetInitialProps.test.ts
@@ -8,7 +8,7 @@ describe('getInitialProps', () => {
     const envelopes = await env.getMultipleEnvelopeRequest({
       url,
       envelopeType: 'transaction',
-      count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
+      count: 1,
     });
 
     const sentryTransactionEnvelope = envelopes.find(envelope => {
diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.test.ts b/packages/nextjs/test/integration/test/server/tracingServerGetServerSideProps.test.ts
@@ -8,7 +8,7 @@ describe('getServerSideProps', () => {
     const envelopes = await env.getMultipleEnvelopeRequest({
       url,
       envelopeType: 'transaction',
-      count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
+      count: 1,
     });
 
     const sentryTransactionEnvelope = envelopes.find(envelope => {
diff --git a/packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.test.ts b/packages/nextjs/test/integration/test/server/tracingServerGetServerSidePropsCustomPageExtension.test.ts
@@ -8,7 +8,7 @@ describe('tracingServerGetServerSidePropsCustomPageExtension', () => {
     const envelopes = await env.getMultipleEnvelopeRequest({
       url,
       envelopeType: 'transaction',
-      count: 2, // We will receive 2 transactions - one from Next.js instrumentation and one from our SDK
+      count: 1,
     });
 
     const sentryTransactionEnvelope = envelopes.find(envelope => {
diff --git a/packages/opentelemetry/src/sampler.ts b/packages/opentelemetry/src/sampler.ts
@@ -64,11 +64,18 @@ export class SentrySampler implements Sampler {
 
     const parentSampled = parentSpan ? getParentSampled(parentSpan, traceId, spanName) : undefined;
 
-    // If we encounter a span emitted by Next.js, we do not want to sample it
-    // The reason for this is that the data quality of the spans varies, it is different per version of Next,
-    // and we need to keep our manual instrumentation around for the edge runtime anyhow.
-    // BUT we only do this if we don't have a parent span with a sampling decision yet (or if the parent is remote)
-    if (spanAttributes['next.span_type'] && (typeof parentSampled !== 'boolean' || parentContext?.isRemote)) {
+    const mutableSamplingDecision = { decision: true };
+    this._client.emit(
+      'beforeSampling',
+      {
+        spanAttributes: spanAttributes,
+        spanName: spanName,
+        parentSampled: parentSampled,
+        parentContext: parentContext,
+      },
+      mutableSamplingDecision,
+    );
+    if (!mutableSamplingDecision.decision) {
       return { decision: SamplingDecision.NOT_RECORD, traceState: traceState };
     }
 
diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts
@@ -14,7 +14,7 @@ import type { Scope } from './scope';
 import type { SdkMetadata } from './sdkmetadata';
 import type { Session, SessionAggregates } from './session';
 import type { SeverityLevel } from './severity';
-import type { Span } from './span';
+import type { Span, SpanAttributes, SpanContextData } from './span';
 import type { StartSpanOptions } from './startSpanOptions';
 import type { Transport, TransportMakeRequestResponse } from './transport';
 
@@ -178,6 +178,23 @@ export interface Client<O extends ClientOptions = ClientOptions> {
    */
   on(hook: 'spanStart', callback: (span: Span) => void): void;
 
+  /**
+   * Register a callback before span sampling runs. Receives a `samplingDecision` object argument with a `decision`
+   * property that can be used to make a sampling decision that will be enforced, before any span sampling runs.
+   */
+  on(
+    hook: 'beforeSampling',
+    callback: (
+      samplingData: {
+        spanAttributes: SpanAttributes;
+        spanName: string;
+        parentSampled?: boolean;
+        parentContext?: SpanContextData;
+      },
+      samplingDecision: { decision: boolean },
+    ) => void,
+  ): void;
+
   /**
    * Register a callback for whenever a span is ended.
    * Receives the span as argument.
@@ -262,6 +279,18 @@ export interface Client<O extends ClientOptions = ClientOptions> {
   /** Fire a hook whener a span starts. */
   emit(hook: 'spanStart', span: Span): void;
 
+  /** A hook that is called every time before a span is sampled. */
+  emit(
+    hook: 'beforeSampling',
+    samplingData: {
+      spanAttributes: SpanAttributes;
+      spanName: string;
+      parentSampled?: boolean;
+      parentContext?: SpanContextData;
+    },
+    samplingDecision: { decision: boolean },
+  ): void;
+
   /** Fire a hook whener a span ends. */
   emit(hook: 'spanEnd', span: Span): void;
 
