fix(node): Fix sample rand propagation for negative sampling decisions (#15045)

Author: lforst

dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js

@@ -4,7 +4,7 @@ const Sentry = require('@sentry/node');
 Sentry.init({
   dsn: 'https://public@dsn.ingest.sentry.io/1337',
   transport: loggingTransport,
-  tracesSampleRate: 1,
+  tracesSampleRate: 0.00000001, // It's important that this is not 1, so that we also check logic for NonRecordingSpans, which is usually the edge-case
 });
 
 // express must be required after Sentry is initialized

dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts

@@ -11,6 +11,7 @@ test('envelope header for error event during active unsampled span is correct',
           environment: 'production',
           release: '1.0',
           sampled: 'false',
+          sample_rand: expect.any(String),
         },
       },
     })

packages/core/src/tracing/dynamicSamplingContext.ts

@@ -117,7 +117,14 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<
   // So we end up with an active span that is not sampled (neither positively nor negatively)
   if (hasTracingEnabled()) {
     dsc.sampled = String(spanIsSampled(rootSpan));
-    dsc.sample_rand = getCapturedScopesOnSpan(rootSpan).scope?.getPropagationContext().sampleRand.toString();
+    dsc.sample_rand =
+      // In OTEL we store the sample rand on the trace state because we cannot access scopes for NonRecordingSpans
+      // The Sentry OTEL SpanSampler takes care of writing the sample rand on the root span
+      traceState?.get('sentry.sample_rand') ??
+      // On all other platforms we can actually get the scopes from a root span (we use this as a fallback)
+      getCapturedScopesOnSpan(rootSpan)
+        .scope?.getPropagationContext()
+        .sampleRand.toString();
   }
 
   client.emit('createDsc', dsc, rootSpan);

packages/opentelemetry/src/constants.ts

@@ -6,6 +6,7 @@ export const SENTRY_BAGGAGE_HEADER = 'baggage';
 export const SENTRY_TRACE_STATE_DSC = 'sentry.dsc';
 export const SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING = 'sentry.sampled_not_recording';
 export const SENTRY_TRACE_STATE_URL = 'sentry.url';
+export const SENTRY_TRACE_STATE_SAMPLE_RAND = 'sentry.sample_rand';
 
 export const SENTRY_SCOPES_CONTEXT_KEY = createContextKey('sentry_scopes');
 

packages/opentelemetry/src/sampler.ts

@@ -18,7 +18,11 @@ import {
   logger,
   sampleSpan,
 } from '@sentry/core';
-import { SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING, SENTRY_TRACE_STATE_URL } from './constants';
+import {
+  SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING,
+  SENTRY_TRACE_STATE_SAMPLE_RAND,
+  SENTRY_TRACE_STATE_URL,
+} from './constants';
 import { DEBUG_BUILD } from './debug-build';
 import { getScopesFromContext } from './utils/contextData';
 import { getSamplingDecision } from './utils/getSamplingDecision';
@@ -99,11 +103,10 @@ export class SentrySampler implements Sampler {
 
     const isRootSpan = !parentSpan || parentContext?.isRemote;
 
-    const { isolationScope, scope } = getScopesFromContext(context) ?? {};
-
     // We only sample based on parameters (like tracesSampleRate or tracesSampler) for root spans (which is done in sampleSpan).
     // Non-root-spans simply inherit the sampling decision from their parent.
     if (isRootSpan) {
+      const { isolationScope, scope } = getScopesFromContext(context) ?? {};
       const sampleRand = scope?.getPropagationContext().sampleRand ?? Math.random();
       const [sampled, sampleRate] = sampleSpan(
         options,
@@ -126,7 +129,7 @@ export class SentrySampler implements Sampler {
         DEBUG_BUILD && logger.log(`[Tracing] Not sampling span because HTTP method is '${method}' for ${spanName}`);
 
         return {
-          ...wrapSamplingDecision({ decision: SamplingDecision.NOT_RECORD, context, spanAttributes }),
+          ...wrapSamplingDecision({ decision: SamplingDecision.NOT_RECORD, context, spanAttributes, sampleRand }),
           attributes,
         };
       }
@@ -145,6 +148,7 @@ export class SentrySampler implements Sampler {
           decision: sampled ? SamplingDecision.RECORD_AND_SAMPLED : SamplingDecision.NOT_RECORD,
           context,
           spanAttributes,
+          sampleRand,
         }),
         attributes,
       };
@@ -196,8 +200,18 @@ export function wrapSamplingDecision({
   decision,
   context,
   spanAttributes,
-}: { decision: SamplingDecision | undefined; context: Context; spanAttributes: SpanAttributes }): SamplingResult {
-  const traceState = getBaseTraceState(context, spanAttributes);
+  sampleRand,
+}: {
+  decision: SamplingDecision | undefined;
+  context: Context;
+  spanAttributes: SpanAttributes;
+  sampleRand?: number;
+}): SamplingResult {
+  let traceState = getBaseTraceState(context, spanAttributes);
+
+  if (sampleRand !== undefined) {
+    traceState = traceState.set(SENTRY_TRACE_STATE_SAMPLE_RAND, `${sampleRand}`);
+  }
 
   // If the decision is undefined, we treat it as NOT_RECORDING, but we don't propagate this decision to downstream SDKs
   // Which is done by not setting `SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING` traceState

packages/opentelemetry/test/sampler.test.ts

@@ -26,11 +26,14 @@ describe('SentrySampler', () => {
     const links = undefined;
 
     const actual = sampler.shouldSample(ctx, traceId, spanName, spanKind, spanAttributes, links);
-    expect(actual).toEqual({
-      decision: SamplingDecision.NOT_RECORD,
-      attributes: { 'sentry.sample_rate': 0 },
-      traceState: new TraceState().set('sentry.sampled_not_recording', '1'),
-    });
+    expect(actual).toEqual(
+      expect.objectContaining({
+        decision: SamplingDecision.NOT_RECORD,
+        attributes: { 'sentry.sample_rate': 0 },
+      }),
+    );
+    expect(actual.traceState?.get('sentry.sampled_not_recording')).toBe('1');
+    expect(actual.traceState?.get('sentry.sample_rand')).toEqual(expect.any(String));
     expect(spyOnDroppedEvent).toHaveBeenCalledTimes(1);
     expect(spyOnDroppedEvent).toHaveBeenCalledWith('sample_rate', 'transaction');
 
@@ -81,7 +84,7 @@ describe('SentrySampler', () => {
     expect(actual).toEqual({
       decision: SamplingDecision.RECORD_AND_SAMPLED,
       attributes: { 'sentry.sample_rate': 1 },
-      traceState: new TraceState(),
+      traceState: expect.any(TraceState),
     });
     expect(spyOnDroppedEvent).toHaveBeenCalledTimes(0);