Skip to content

Commit cfc401e

Browse files
mydeamydea
authored
fix(replay): Do not add replay_id to DSC while buffering (#8020)
1 parent 4b8cd5d commit cfc401e

File tree

3 files changed

+143
-38
lines changed

3 files changed

+143
-38
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ Sentry.init({
1313
integrations: [new Integrations.BrowserTracing({ tracingOrigins: [/.*/] }), window.Replay],
1414
environment: 'production',
1515
tracesSampleRate: 1,
16+
// Needs manual start!
1617
replaysSessionSampleRate: 0.0,
17-
replaysOnErrorSampleRate: 1.0,
18+
replaysOnErrorSampleRate: 0.0,
1819
});

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

Lines changed: 139 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,54 +3,102 @@ import type * as Sentry from '@sentry/browser';
33
import type { EventEnvelopeHeaders } from '@sentry/types';
44

55
import { sentryTest } from '../../../utils/fixtures';
6-
import {
7-
envelopeHeaderRequestParser,
8-
envelopeRequestParser,
9-
getFirstSentryEnvelopeRequest,
10-
shouldSkipTracingTest,
11-
waitForTransactionRequest,
12-
} from '../../../utils/helpers';
6+
import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../utils/helpers';
137
import { getReplaySnapshot, shouldSkipReplayTest, waitForReplayRunning } from '../../../utils/replayHelpers';
148

159
type TestWindow = Window & { Sentry: typeof Sentry; Replay: Sentry.Replay };
1610

17-
sentryTest('should add replay_id to dsc of transactions', async ({ getLocalTestPath, page, browserName }) => {
18-
// This is flaky on webkit, so skipping there...
19-
if (shouldSkipReplayTest() || shouldSkipTracingTest() || browserName === 'webkit') {
20-
sentryTest.skip();
21-
}
11+
sentryTest(
12+
'should add replay_id to dsc of transactions when in session mode',
13+
async ({ getLocalTestPath, page, browserName }) => {
14+
// This is flaky on webkit, so skipping there...
15+
if (shouldSkipReplayTest() || shouldSkipTracingTest() || browserName === 'webkit') {
16+
sentryTest.skip();
17+
}
2218

23-
const url = await getLocalTestPath({ testDir: __dirname });
24-
await page.goto(url);
19+
const url = await getLocalTestPath({ testDir: __dirname });
20+
await page.goto(url);
2521

26-
await waitForReplayRunning(page);
22+
const transactionReq = waitForTransactionRequest(page);
23+
24+
await page.evaluate(() => {
25+
(window as unknown as TestWindow).Replay.start();
26+
});
2727

28-
await page.evaluate(() => {
29-
(window as unknown as TestWindow).Sentry.configureScope(scope => {
30-
scope.setUser({ id: 'user123', segment: 'segmentB' });
31-
scope.setTransactionName('testTransactionDSC');
28+
await waitForReplayRunning(page);
29+
30+
await page.evaluate(() => {
31+
(window as unknown as TestWindow).Sentry.configureScope(scope => {
32+
scope.setUser({ id: 'user123', segment: 'segmentB' });
33+
scope.setTransactionName('testTransactionDSC');
34+
});
3235
});
33-
});
3436

35-
const envHeader = await getFirstSentryEnvelopeRequest<EventEnvelopeHeaders>(page, url, envelopeHeaderRequestParser);
37+
const req0 = await transactionReq;
3638

37-
const replay = await getReplaySnapshot(page);
39+
const envHeader = envelopeRequestParser(req0, 0) as EventEnvelopeHeaders;
40+
const replay = await getReplaySnapshot(page);
3841

39-
expect(replay.session?.id).toBeDefined();
42+
expect(replay.session?.id).toBeDefined();
4043

41-
expect(envHeader.trace).toBeDefined();
42-
expect(envHeader.trace).toEqual({
43-
environment: 'production',
44-
user_segment: 'segmentB',
45-
sample_rate: '1',
46-
trace_id: expect.any(String),
47-
public_key: 'public',
48-
replay_id: replay.session?.id,
49-
});
50-
});
44+
expect(envHeader.trace).toBeDefined();
45+
expect(envHeader.trace).toEqual({
46+
environment: 'production',
47+
user_segment: 'segmentB',
48+
sample_rate: '1',
49+
trace_id: expect.any(String),
50+
public_key: 'public',
51+
replay_id: replay.session?.id,
52+
});
53+
},
54+
);
5155

5256
sentryTest(
53-
'should not add replay_id to dsc of transactions if replay is not enabled',
57+
'should not add replay_id to dsc of transactions when in buffer mode',
58+
async ({ getLocalTestPath, page, browserName }) => {
59+
// This is flaky on webkit, so skipping there...
60+
if (shouldSkipReplayTest() || shouldSkipTracingTest() || browserName === 'webkit') {
61+
sentryTest.skip();
62+
}
63+
64+
const url = await getLocalTestPath({ testDir: __dirname });
65+
await page.goto(url);
66+
67+
const transactionReq = waitForTransactionRequest(page);
68+
69+
await page.evaluate(() => {
70+
(window as unknown as TestWindow).Replay.startBuffering();
71+
});
72+
73+
await waitForReplayRunning(page);
74+
75+
await page.evaluate(() => {
76+
(window as unknown as TestWindow).Sentry.configureScope(scope => {
77+
scope.setUser({ id: 'user123', segment: 'segmentB' });
78+
scope.setTransactionName('testTransactionDSC');
79+
});
80+
});
81+
82+
const req0 = await transactionReq;
83+
84+
const envHeader = envelopeRequestParser(req0, 0) as EventEnvelopeHeaders;
85+
const replay = await getReplaySnapshot(page);
86+
87+
expect(replay.session?.id).toBeDefined();
88+
89+
expect(envHeader.trace).toBeDefined();
90+
expect(envHeader.trace).toEqual({
91+
environment: 'production',
92+
user_segment: 'segmentB',
93+
sample_rate: '1',
94+
trace_id: expect.any(String),
95+
public_key: 'public',
96+
});
97+
},
98+
);
99+
100+
sentryTest(
101+
'should add replay_id to dsc of transactions when switching from buffer to session mode',
54102
async ({ getLocalTestPath, page, browserName }) => {
55103
// This is flaky on webkit, so skipping there...
56104
if (shouldSkipReplayTest() || shouldSkipTracingTest() || browserName === 'webkit') {
@@ -68,13 +116,68 @@ sentryTest(
68116
const url = await getLocalTestPath({ testDir: __dirname });
69117
await page.goto(url);
70118

119+
const transactionReq = waitForTransactionRequest(page);
120+
121+
await page.evaluate(() => {
122+
(window as unknown as TestWindow).Replay.startBuffering();
123+
});
124+
71125
await waitForReplayRunning(page);
72126

127+
await page.evaluate(async () => {
128+
// Manually trigger error handling
129+
await (window as unknown as TestWindow).Replay.flush();
130+
});
131+
132+
await page.evaluate(() => {
133+
(window as unknown as TestWindow).Sentry.configureScope(scope => {
134+
scope.setUser({ id: 'user123', segment: 'segmentB' });
135+
scope.setTransactionName('testTransactionDSC');
136+
});
137+
});
138+
139+
const req0 = await transactionReq;
140+
141+
const envHeader = envelopeRequestParser(req0, 0) as EventEnvelopeHeaders;
142+
const replay = await getReplaySnapshot(page);
143+
144+
expect(replay.session?.id).toBeDefined();
145+
expect(replay.recordingMode).toBe('session');
146+
147+
expect(envHeader.trace).toBeDefined();
148+
expect(envHeader.trace).toEqual({
149+
environment: 'production',
150+
user_segment: 'segmentB',
151+
sample_rate: '1',
152+
trace_id: expect.any(String),
153+
public_key: 'public',
154+
replay_id: replay.session?.id,
155+
});
156+
},
157+
);
158+
159+
sentryTest(
160+
'should not add replay_id to dsc of transactions if replay is not enabled',
161+
async ({ getLocalTestPath, page, browserName }) => {
162+
// This is flaky on webkit, so skipping there...
163+
if (shouldSkipReplayTest() || shouldSkipTracingTest() || browserName === 'webkit') {
164+
sentryTest.skip();
165+
}
166+
167+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
168+
return route.fulfill({
169+
status: 200,
170+
contentType: 'application/json',
171+
body: JSON.stringify({ id: 'test-id' }),
172+
});
173+
});
174+
175+
const url = await getLocalTestPath({ testDir: __dirname });
176+
await page.goto(url);
177+
73178
const transactionReq = waitForTransactionRequest(page);
74179

75180
await page.evaluate(async () => {
76-
await (window as unknown as TestWindow).Replay.stop();
77-
78181
(window as unknown as TestWindow).Sentry.configureScope(scope => {
79182
scope.setUser({ id: 'user123', segment: 'segmentB' });
80183
scope.setTransactionName('testTransactionDSC');

‎packages/replay/src/util/addGlobalListeners.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ export function addGlobalListeners(replay: ReplayContainer): void {
3535
client.on('afterSendEvent', handleAfterSendEvent(replay));
3636
client.on('createDsc', (dsc: DynamicSamplingContext) => {
3737
const replayId = replay.getSessionId();
38-
if (replayId && replay.isEnabled()) {
38+
// We do not want to set the DSC when in buffer mode, as that means the replay has not been sent (yet)
39+
if (replayId && replay.isEnabled() && replay.recordingMode === 'session') {
3940
dsc.replay_id = replayId;
4041
}
4142
});

0 commit comments

Comments
 (0)