From d74f629fae87b08c275744bb9ce1c9c33c28ca61 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 3 May 2023 09:11:03 +0200 Subject: [PATCH] fix(replay): Do not add replay_id to DSC while buffering --- .../suites/replay/dsc/init.js | 3 +- .../suites/replay/dsc/test.ts | 175 ++++++++++++++---- .../replay/src/util/addGlobalListeners.ts | 3 +- 3 files changed, 143 insertions(+), 38 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/dsc/init.js b/packages/browser-integration-tests/suites/replay/dsc/init.js index da0177961ea2..90919aeaeb70 100644 --- a/packages/browser-integration-tests/suites/replay/dsc/init.js +++ b/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, }); diff --git a/packages/browser-integration-tests/suites/replay/dsc/test.ts b/packages/browser-integration-tests/suites/replay/dsc/test.ts index f8f9560c00fc..54de1ee7db81 100644 --- a/packages/browser-integration-tests/suites/replay/dsc/test.ts +++ b/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(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'); diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts index 189867c721bb..e5096f397fae 100644 --- a/packages/replay/src/util/addGlobalListeners.ts +++ b/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; } });