feat(browser): Add option to sample linked traces consistently

Author: Lms24

dev-packages/browser-integration-tests/utils/helpers.ts

@@ -1,6 +1,8 @@
+/* eslint-disable max-lines */
 import type { Page, Request } from '@playwright/test';
 import { parseEnvelope } from '@sentry/core';
 import type {
+  ClientReport,
   Envelope,
   EnvelopeItem,
   EnvelopeItemType,
@@ -254,6 +256,31 @@ export function waitForTransactionRequest(
   });
 }
 
+export function waitForClientReportRequest(page: Page, callback?: (report: ClientReport) => boolean): Promise<Request> {
+  return page.waitForRequest(req => {
+    const postData = req.postData();
+    if (!postData) {
+      return false;
+    }
+
+    try {
+      const maybeReport = envelopeRequestParser<Partial<ClientReport>>(req);
+
+      if (typeof maybeReport.discarded_events !== 'object') {
+        return false;
+      }
+
+      if (callback) {
+        return callback(maybeReport as ClientReport);
+      }
+
+      return true;
+    } catch {
+      return false;
+    }
+  });
+}
+
 export async function waitForSession(page: Page): Promise<SessionContext> {
   const req = await page.waitForRequest(req => {
     const postData = req.postData();

packages/browser/src/tracing/browserTracingIntegration.ts

@@ -14,6 +14,7 @@ import {
   GLOBAL_OBJ,
   SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  SEMANTIC_ATTRIBUTE_SENTRY_PREVIOUS_TRACE_SAMPLE_RATE,
   SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
   TRACING_DEFAULTS,
   addNonEnumerableProperty,
@@ -36,10 +37,10 @@ import { DEBUG_BUILD } from '../debug-build';
 import { WINDOW } from '../helpers';
 import { registerBackgroundTabDetection } from './backgroundtab';
 import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request';
-import type { PreviousTraceInfo } from './previousTrace';
 import {
   addPreviousTraceSpanLink,
   getPreviousTraceFromSessionStorage,
+  spanContextSampled,
   storePreviousTraceInSessionStorage,
 } from './previousTrace';
 
@@ -172,6 +173,23 @@ export interface BrowserTracingOptions {
    */
   linkPreviousTrace: 'in-memory' | 'session-storage' | 'off';
 
+  /**
+   * If true, Sentry will consistently sample subsequent traces based on the
+   * sampling decision of the initial trace. For example, if the initial page
+   * load trace was sampled positively, all subsequent traces (e.g. navigations)
+   * are also sampled positively. In case the initial trace was sampled negatively,
+   * all subsequent traces are also sampled negatively.
+   *
+   * This option lets you get consistent, linked traces within a user journey
+   * while maintaining an overall quota based on your trace sampling settings.
+   *
+   * This option is only effective if {@link BrowserTracingOptions.linkPreviousTrace}
+   * is enabled (i.e. not set to `'off'`).
+   *
+   * @default `false` - this is an opt-in feature.
+   */
+  sampleLinkedTracesConsistently: boolean;
+
   /**
    * _experiments allows the user to send options to define how this integration works.
    *
@@ -213,6 +231,7 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = {
   enableLongAnimationFrame: true,
   enableInp: true,
   linkPreviousTrace: 'in-memory',
+  sampleLinkedTracesConsistently: false,
   _experiments: {},
   ...defaultRequestInstrumentationOptions,
 };
@@ -253,6 +272,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
     instrumentPageLoad,
     instrumentNavigation,
     linkPreviousTrace,
+    sampleLinkedTracesConsistently,
     onRequestSpanStart,
   } = {
     ...DEFAULT_BROWSER_TRACING_OPTIONS,
@@ -330,6 +350,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         });
       },
     });
+
     setActiveIdleSpan(client, idleSpan);
 
     function emitFinish(): void {
@@ -397,20 +418,67 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
       });
 
       if (linkPreviousTrace !== 'off') {
-        let inMemoryPreviousTraceInfo: PreviousTraceInfo | undefined = undefined;
+        const useSessionStorage = linkPreviousTrace === 'session-storage';
+
+        let inMemoryPreviousTraceInfo = useSessionStorage ? getPreviousTraceFromSessionStorage() : undefined;
 
         client.on('spanStart', span => {
           if (getRootSpan(span) !== span) {
             return;
           }
 
-          if (linkPreviousTrace === 'session-storage') {
-            const updatedPreviousTraceInfo = addPreviousTraceSpanLink(getPreviousTraceFromSessionStorage(), span);
-            storePreviousTraceInSessionStorage(updatedPreviousTraceInfo);
-          } else {
-            inMemoryPreviousTraceInfo = addPreviousTraceSpanLink(inMemoryPreviousTraceInfo, span);
+          const scope = getCurrentScope();
+          const oldPropagationContext = scope.getPropagationContext();
+          inMemoryPreviousTraceInfo = addPreviousTraceSpanLink(inMemoryPreviousTraceInfo, span, oldPropagationContext);
+
+          if (useSessionStorage) {
+            storePreviousTraceInSessionStorage(inMemoryPreviousTraceInfo);
           }
         });
+
+        if (sampleLinkedTracesConsistently) {
+          /*
+          This is a massive hack I'm really not proud of:
+
+          When users opt into `sampleLinkedTracesConsistently`, we need to make sure that we "propagate"
+          the previous trace's sample rate and rand to the current trace. This is necessary because otherwise, span
+          metric extrapolation is off, as we'd be propagating a too high sample rate for the subsequent traces.
+
+          So therefore, we pretend that the previous trace was the parent trace of the newly started trace. To do that,
+          we mutate the propagation context of the current trace and set the sample rate and sample rand of the previous trace.
+          Timing-wise, it is fine because it happens before we even sample the root span.
+
+          @see https://github.com/getsentry/sentry-javascript/issues/15754
+          */
+          client.on('beforeSampling', mutableSamplingContextData => {
+            if (!inMemoryPreviousTraceInfo) {
+              return;
+            }
+
+            const scope = getCurrentScope();
+            const currentPropagationContext = scope.getPropagationContext();
+
+            scope.setPropagationContext({
+              ...currentPropagationContext,
+              dsc: {
+                ...currentPropagationContext.dsc,
+                // The fallback to 0 should never happen; this is rather to satisfy the types
+                sample_rate: String(inMemoryPreviousTraceInfo.sampleRate ?? 0),
+                sampled: String(spanContextSampled(inMemoryPreviousTraceInfo.spanContext)),
+              },
+              sampleRand: inMemoryPreviousTraceInfo.sampleRand,
+            });
+
+            mutableSamplingContextData.parentSampled = spanContextSampled(inMemoryPreviousTraceInfo.spanContext);
+            mutableSamplingContextData.parentSampleRate = inMemoryPreviousTraceInfo.sampleRate;
+
+            mutableSamplingContextData.spanAttributes = {
+              ...mutableSamplingContextData.spanAttributes,
+              // record an attribute that this span was "force-sampled", so that we can later check on this.
+              [SEMANTIC_ATTRIBUTE_SENTRY_PREVIOUS_TRACE_SAMPLE_RATE]: inMemoryPreviousTraceInfo.sampleRate,
+            };
+          });
+        }
       }
 
       if (WINDOW.location) {

packages/browser/src/tracing/previousTrace.ts

@@ -1,5 +1,11 @@
-import type { Span } from '@sentry/core';
-import { logger, SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE, spanToJSON, type SpanContextData } from '@sentry/core';
+import type { PropagationContext, Span } from '@sentry/core';
+import {
+  logger,
+  SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
+  SEMANTIC_LINK_ATTRIBUTE_LINK_TYPE,
+  spanToJSON,
+  type SpanContextData,
+} from '@sentry/core';
 import { WINDOW } from '../exports';
 import { DEBUG_BUILD } from '../debug-build';
 
@@ -13,6 +19,16 @@ export interface PreviousTraceInfo {
    * Timestamp in seconds when the previous trace was started
    */
   startTimestamp: number;
+
+  /**
+   * sample rate of the previous trace
+   */
+  sampleRate: number;
+
+  /**
+   * The sample rand of the previous trace
+   */
+  sampleRand: number;
 }
 
 // 1h in seconds
@@ -33,14 +49,29 @@ export const PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE = 'sentry.previous_trace';
 export function addPreviousTraceSpanLink(
   previousTraceInfo: PreviousTraceInfo | undefined,
   span: Span,
+  oldPropagationContext: PropagationContext,
 ): PreviousTraceInfo {
   const spanJson = spanToJSON(span);
 
+  function getSampleRate(): number {
+    try {
+      return (
+        Number(oldPropagationContext.dsc?.sample_rate) ?? Number(spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE])
+      );
+    } catch {
+      return 0;
+    }
+  }
+
+  const updatedPreviousTraceInfo = {
+    spanContext: span.spanContext(),
+    startTimestamp: spanJson.start_timestamp,
+    sampleRate: getSampleRate(),
+    sampleRand: oldPropagationContext.sampleRand,
+  };
+
   if (!previousTraceInfo) {
-    return {
-      spanContext: span.spanContext(),
-      startTimestamp: spanJson.start_timestamp,
-    };
+    return updatedPreviousTraceInfo;
   }
 
   const previousTraceSpanCtx = previousTraceInfo.spanContext;
@@ -80,15 +111,12 @@ export function addPreviousTraceSpanLink(
     span.setAttribute(
       PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE,
       `${previousTraceSpanCtx.traceId}-${previousTraceSpanCtx.spanId}-${
-        previousTraceSpanCtx.traceFlags === 0x1 ? 1 : 0
+        spanContextSampled(previousTraceSpanCtx) ? 1 : 0
       }`,
     );
   }
 
-  return {
-    spanContext: span.spanContext(),
-    startTimestamp: spanToJSON(span).start_timestamp,
-  };
+  return updatedPreviousTraceInfo;
 }
 
 /**
@@ -115,3 +143,10 @@ export function getPreviousTraceFromSessionStorage(): PreviousTraceInfo | undefi
     return undefined;
   }
 }
+
+/**
+ * see {@link import('@sentry/core').spanIsSampled}
+ */
+export const spanContextSampled = (ctx: SpanContextData): boolean => {
+  return ctx.traceFlags === 0x1;
+};