fix(browser): Correctly parse sampleRate when consistentTraceSampling is enabled (#21281)

This PR fixes 2 bugs in our trace linking logic that would emerge when
`consistentTraceSampling` was enabled:

1. We'd incorrectly parse an `undefined` sample rate as `0` on the last
DSC instead of falling back to the sampling rate on the span
2. The order of checking for the sample rate on the span vs. DSC was
incorrect. This just didn't surface because of bug 1

closes #21264

Author: Lms24

packages/browser/src/tracing/linkedTraces.ts

@@ -141,9 +141,10 @@ export function addPreviousTraceSpanLink(
 
   function getSampleRate(): number {
     try {
-      return (
-        Number(oldPropagationContext.dsc?.sample_rate) ?? Number(spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE])
+      const oldSampleRate = Number(
+        spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] ?? oldPropagationContext.dsc?.sample_rate,
       );
+      return Number.isNaN(oldSampleRate) ? 0 : oldSampleRate;
     } catch {
       return 0;
     }

packages/browser/test/tracing/linkedTraces.test.ts

@@ -1,5 +1,12 @@
-import type { Span } from '@sentry/core/browser';
-import { addChildSpanToSpan, debug, SentrySpan, spanToJSON, timestampInSeconds } from '@sentry/core/browser';
+import type { PropagationContext, Span } from '@sentry/core/browser';
+import {
+  addChildSpanToSpan,
+  debug,
+  SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
+  SentrySpan,
+  spanToJSON,
+  timestampInSeconds,
+} from '@sentry/core/browser';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 import { BrowserClient } from '../../src';
 import type { PreviousTraceInfo } from '../../src/tracing/linkedTraces';
@@ -201,6 +208,123 @@ describe('addPreviousTraceSpanLink', () => {
     });
   });
 
+  it.each([NaN, undefined])(
+    'falls back to sampleRate 0 if the sample rate in the previous trace info is %s',
+    sampleRate => {
+      const currentSpanStart = timestampInSeconds();
+
+      const previousTraceInfo: PreviousTraceInfo = {
+        spanContext: { traceId: '123', spanId: '456', traceFlags: 1 },
+        startTimestamp: currentSpanStart - PREVIOUS_TRACE_MAX_DURATION + 1,
+        sampleRand: 0.0126,
+        sampleRate: sampleRate as number,
+      };
+
+      const currentSpan = new SentrySpan({
+        name: 'test',
+        startTimestamp: currentSpanStart,
+        parentSpanId: '789',
+        spanId: 'abc',
+        traceId: 'def',
+        sampled: true,
+      });
+
+      const oldPropagationContext: PropagationContext = {
+        sampleRand: 0.0126,
+        traceId: '123',
+        sampled: true,
+        dsc: { sample_rand: '0.0126', sample_rate: String(sampleRate) },
+      };
+
+      const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan, oldPropagationContext);
+
+      expect(updatedPreviousTraceInfo).toEqual({
+        spanContext: currentSpan.spanContext(),
+        startTimestamp: currentSpanStart,
+        sampleRand: 0.0126,
+        sampleRate: 0,
+      });
+    },
+  );
+
+  it.each([NaN, undefined])('falls back to sampleRate 0 if the sample rate on the spa or DSC is %s', sampleRate => {
+    const currentSpanStart = timestampInSeconds();
+
+    const previousTraceInfo: PreviousTraceInfo = {
+      spanContext: { traceId: '123', spanId: '456', traceFlags: 1 },
+      startTimestamp: currentSpanStart - PREVIOUS_TRACE_MAX_DURATION + 1,
+      sampleRand: 0.0126,
+      sampleRate: 0,
+    };
+
+    const currentSpan = new SentrySpan({
+      name: 'test',
+      startTimestamp: currentSpanStart,
+      parentSpanId: '789',
+      spanId: 'abc',
+      traceId: 'def',
+      sampled: true,
+      attributes: {
+        [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate,
+      },
+    });
+
+    const oldPropagationContext: PropagationContext = {
+      sampleRand: 0.0126,
+      traceId: '123',
+      sampled: true,
+      dsc: { sample_rand: '0.0126', sample_rate: String(sampleRate) },
+    };
+
+    const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan, oldPropagationContext);
+
+    expect(updatedPreviousTraceInfo).toEqual({
+      spanContext: currentSpan.spanContext(),
+      startTimestamp: currentSpanStart,
+      sampleRand: 0.0126,
+      sampleRate: 0,
+    });
+  });
+
+  it('prefers sample rate from span over sample rate from DSC', () => {
+    const currentSpanStart = timestampInSeconds();
+
+    const previousTraceInfo: PreviousTraceInfo = {
+      spanContext: { traceId: '123', spanId: '456', traceFlags: 1 },
+      startTimestamp: currentSpanStart - PREVIOUS_TRACE_MAX_DURATION + 1,
+      sampleRand: 0.0126,
+      sampleRate: 0.4,
+    };
+
+    const currentSpan = new SentrySpan({
+      name: 'test',
+      startTimestamp: currentSpanStart,
+      parentSpanId: '789',
+      spanId: 'abc',
+      traceId: 'def',
+      sampled: true,
+      attributes: {
+        [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 0.5,
+      },
+    });
+
+    const oldPropagationContext: PropagationContext = {
+      sampleRand: 0.0126,
+      traceId: '123',
+      sampled: true,
+      dsc: { sample_rand: '0.0126', sample_rate: '0.6' },
+    };
+
+    const updatedPreviousTraceInfo = addPreviousTraceSpanLink(previousTraceInfo, currentSpan, oldPropagationContext);
+
+    expect(updatedPreviousTraceInfo).toEqual({
+      spanContext: currentSpan.spanContext(),
+      startTimestamp: currentSpanStart,
+      sampleRand: 0.0126,
+      sampleRate: 0.5,
+    });
+  });
+
   it('logs a debug message when adding a previous trace link (with stringified context)', () => {
     const debugLogSpy = vi.spyOn(debug, 'log');