fix(replay): Do not add replay_id to DSC while buffering (#8020)

Author: mydea

packages/browser-integration-tests/suites/replay/dsc/init.js

@@ -13,6 +13,7 @@ Sentry.init({
   integrations: [new Integrations.BrowserTracing({ tracingOrigins: [/.*/] }), window.Replay],
   environment: 'production',
   tracesSampleRate: 1,
+  // Needs manual start!
   replaysSessionSampleRate: 0.0,
-  replaysOnErrorSampleRate: 1.0,
+  replaysOnErrorSampleRate: 0.0,
 });

packages/browser-integration-tests/suites/replay/dsc/test.ts

@@ -3,54 +3,102 @@ import type * as Sentry from '@sentry/browser';
 import type { EventEnvelopeHeaders } from '@sentry/types';
 
 import { sentryTest } from '../../../utils/fixtures';
-import {
-  envelopeHeaderRequestParser,
-  envelopeRequestParser,
-  getFirstSentryEnvelopeRequest,
-  shouldSkipTracingTest,
-  waitForTransactionRequest,
-} from '../../../utils/helpers';
+import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../utils/helpers';
 import { getReplaySnapshot, shouldSkipReplayTest, waitForReplayRunning } from '../../../utils/replayHelpers';
 
 type TestWindow = Window & { Sentry: typeof Sentry; Replay: Sentry.Replay };
 
-sentryTest('should add replay_id to dsc of transactions', async ({ getLocalTestPath, page, browserName }) => {
-  // This is flaky on webkit, so skipping there...
-  if (shouldSkipReplayTest() || shouldSkipTracingTest() || browserName === 'webkit') {
-    sentryTest.skip();
-  }
+sentryTest(
+  'should add replay_id to dsc of transactions when in session mode',
+  async ({ getLocalTestPath, page, browserName }) => {
+    // This is flaky on webkit, so skipping there...
+    if (shouldSkipReplayTest() || shouldSkipTracingTest() || browserName === 'webkit') {
+      sentryTest.skip();
+    }
 
-  const url = await getLocalTestPath({ testDir: __dirname });
-  await page.goto(url);
+    const url = await getLocalTestPath({ testDir: __dirname });
+    await page.goto(url);
 
-  await waitForReplayRunning(page);
+    const transactionReq = waitForTransactionRequest(page);
+
+    await page.evaluate(() => {
+      (window as unknown as TestWindow).Replay.start();
+    });
 
-  await page.evaluate(() => {
-    (window as unknown as TestWindow).Sentry.configureScope(scope => {
-      scope.setUser({ id: 'user123', segment: 'segmentB' });
-      scope.setTransactionName('testTransactionDSC');
+    await waitForReplayRunning(page);
+
+    await page.evaluate(() => {
+      (window as unknown as TestWindow).Sentry.configureScope(scope => {
+        scope.setUser({ id: 'user123', segment: 'segmentB' });
+        scope.setTransactionName('testTransactionDSC');
+      });
     });
-  });
 
-  const envHeader = await getFirstSentryEnvelopeRequest<EventEnvelopeHeaders>(page, url, envelopeHeaderRequestParser);
+    const req0 = await transactionReq;
 
-  const replay = await getReplaySnapshot(page);
+    const envHeader = envelopeRequestParser(req0, 0) as EventEnvelopeHeaders;
+    const replay = await getReplaySnapshot(page);
 
-  expect(replay.session?.id).toBeDefined();
+    expect(replay.session?.id).toBeDefined();
 
-  expect(envHeader.trace).toBeDefined();
-  expect(envHeader.trace).toEqual({
-    environment: 'production',
-    user_segment: 'segmentB',
-    sample_rate: '1',
-    trace_id: expect.any(String),
-    public_key: 'public',
-    replay_id: replay.session?.id,
-  });
-});
+    expect(envHeader.trace).toBeDefined();
+    expect(envHeader.trace).toEqual({
+      environment: 'production',
+      user_segment: 'segmentB',
+      sample_rate: '1',
+      trace_id: expect.any(String),
+      public_key: 'public',
+      replay_id: replay.session?.id,
+    });
+  },
+);
 
 sentryTest(
-  'should not add replay_id to dsc of transactions if replay is not enabled',
+  'should not add replay_id to dsc of transactions when in buffer mode',
+  async ({ getLocalTestPath, page, browserName }) => {
+    // This is flaky on webkit, so skipping there...
+    if (shouldSkipReplayTest() || shouldSkipTracingTest() || browserName === 'webkit') {
+      sentryTest.skip();
+    }
+
+    const url = await getLocalTestPath({ testDir: __dirname });
+    await page.goto(url);
+
+    const transactionReq = waitForTransactionRequest(page);
+
+    await page.evaluate(() => {
+      (window as unknown as TestWindow).Replay.startBuffering();
+    });
+
+    await waitForReplayRunning(page);
+
+    await page.evaluate(() => {
+      (window as unknown as TestWindow).Sentry.configureScope(scope => {
+        scope.setUser({ id: 'user123', segment: 'segmentB' });
+        scope.setTransactionName('testTransactionDSC');
+      });
+    });
+
+    const req0 = await transactionReq;
+
+    const envHeader = envelopeRequestParser(req0, 0) as EventEnvelopeHeaders;
+    const replay = await getReplaySnapshot(page);
+
+    expect(replay.session?.id).toBeDefined();
+
+    expect(envHeader.trace).toBeDefined();
+    expect(envHeader.trace).toEqual({
+      environment: 'production',
+      user_segment: 'segmentB',
+      sample_rate: '1',
+      trace_id: expect.any(String),
+      public_key: 'public',
+    });
+  },
+);
+
+sentryTest(
+  'should add replay_id to dsc of transactions when switching from buffer to session mode',
   async ({ getLocalTestPath, page, browserName }) => {
     // This is flaky on webkit, so skipping there...
     if (shouldSkipReplayTest() || shouldSkipTracingTest() || browserName === 'webkit') {
@@ -68,13 +116,68 @@ sentryTest(
     const url = await getLocalTestPath({ testDir: __dirname });
     await page.goto(url);
 
+    const transactionReq = waitForTransactionRequest(page);
+
+    await page.evaluate(() => {
+      (window as unknown as TestWindow).Replay.startBuffering();
+    });
+
     await waitForReplayRunning(page);
 
+    await page.evaluate(async () => {
+      // Manually trigger error handling
+      await (window as unknown as TestWindow).Replay.flush();
+    });
+
+    await page.evaluate(() => {
+      (window as unknown as TestWindow).Sentry.configureScope(scope => {
+        scope.setUser({ id: 'user123', segment: 'segmentB' });
+        scope.setTransactionName('testTransactionDSC');
+      });
+    });
+
+    const req0 = await transactionReq;
+
+    const envHeader = envelopeRequestParser(req0, 0) as EventEnvelopeHeaders;
+    const replay = await getReplaySnapshot(page);
+
+    expect(replay.session?.id).toBeDefined();
+    expect(replay.recordingMode).toBe('session');
+
+    expect(envHeader.trace).toBeDefined();
+    expect(envHeader.trace).toEqual({
+      environment: 'production',
+      user_segment: 'segmentB',
+      sample_rate: '1',
+      trace_id: expect.any(String),
+      public_key: 'public',
+      replay_id: replay.session?.id,
+    });
+  },
+);
+
+sentryTest(
+  'should not add replay_id to dsc of transactions if replay is not enabled',
+  async ({ getLocalTestPath, page, browserName }) => {
+    // This is flaky on webkit, so skipping there...
+    if (shouldSkipReplayTest() || shouldSkipTracingTest() || browserName === 'webkit') {
+      sentryTest.skip();
+    }
+
+    await page.route('https://dsn.ingest.sentry.io/**/*', route => {
+      return route.fulfill({
+        status: 200,
+        contentType: 'application/json',
+        body: JSON.stringify({ id: 'test-id' }),
+      });
+    });
+
+    const url = await getLocalTestPath({ testDir: __dirname });
+    await page.goto(url);
+
     const transactionReq = waitForTransactionRequest(page);
 
     await page.evaluate(async () => {
-      await (window as unknown as TestWindow).Replay.stop();
-
       (window as unknown as TestWindow).Sentry.configureScope(scope => {
         scope.setUser({ id: 'user123', segment: 'segmentB' });
         scope.setTransactionName('testTransactionDSC');

packages/replay/src/util/addGlobalListeners.ts

@@ -35,7 +35,8 @@ export function addGlobalListeners(replay: ReplayContainer): void {
     client.on('afterSendEvent', handleAfterSendEvent(replay));
     client.on('createDsc', (dsc: DynamicSamplingContext) => {
       const replayId = replay.getSessionId();
-      if (replayId && replay.isEnabled()) {
+      // We do not want to set the DSC when in buffer mode, as that means the replay has not been sent (yet)
+      if (replayId && replay.isEnabled() && replay.recordingMode === 'session') {
         dsc.replay_id = replayId;
       }
     });