Skip to content

Commit 77b3355

Browse files
authored
fix(replay): Ignore older performance entries when starting manually (#13969)
For replay, performance entries are captured separately, and added to the replay before we send it. Generally, this works just fine, because when we buffer, we clear the captured performance events whenever the buffer is cleared. However, when we manually start the replay, it can happen that we capture performane entries from way before we started the replay manually, which will in turn extend the timeline of the replay potentially drastically. To fix this, we now drop any performance events, when starting manually, that happened more than a second before we manually started the replay.
1 parent bece3e5 commit 77b3355

File tree

2 files changed

+131
-1
lines changed
  • dev-packages/browser-integration-tests/suites/replay/bufferModeManual
  • packages/replay-internal/src

2 files changed

+131
-1
lines changed

dev-packages/browser-integration-tests/suites/replay/bufferModeManual/test.ts

+119
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,125 @@ sentryTest(
287287
},
288288
);
289289

290+
sentryTest(
291+
'[buffer-mode] manually starting replay ignores earlier performance entries',
292+
async ({ getLocalTestUrl, page, browserName }) => {
293+
// This was sometimes flaky on webkit, so skipping for now
294+
if (shouldSkipReplayTest() || browserName === 'webkit') {
295+
sentryTest.skip();
296+
}
297+
298+
const reqPromise0 = waitForReplayRequest(page, 0);
299+
300+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
301+
return route.fulfill({
302+
status: 200,
303+
contentType: 'application/json',
304+
body: JSON.stringify({ id: 'test-id' }),
305+
});
306+
});
307+
308+
const url = await getLocalTestUrl({ testDir: __dirname });
309+
310+
await page.goto(url);
311+
312+
// Wait for everything to be initialized - Replay is not running yet
313+
await page.waitForFunction('!!window.Replay');
314+
315+
// Wait for a second, then start replay
316+
await new Promise(resolve => setTimeout(resolve, 1000));
317+
await page.evaluate('window.Replay.start()');
318+
319+
const req0 = await reqPromise0;
320+
321+
const event0 = getReplayEvent(req0);
322+
const content0 = getReplayRecordingContent(req0);
323+
324+
expect(event0).toEqual(
325+
getExpectedReplayEvent({
326+
replay_type: 'session',
327+
}),
328+
);
329+
330+
const { performanceSpans } = content0;
331+
332+
// Here, we test that this does not contain any web-vital etc. performance spans
333+
// as these have been emitted _before_ the replay was manually started
334+
expect(performanceSpans).toEqual([
335+
{
336+
op: 'memory',
337+
description: 'memory',
338+
startTimestamp: expect.any(Number),
339+
endTimestamp: expect.any(Number),
340+
data: {
341+
memory: {
342+
jsHeapSizeLimit: expect.any(Number),
343+
totalJSHeapSize: expect.any(Number),
344+
usedJSHeapSize: expect.any(Number),
345+
},
346+
},
347+
},
348+
]);
349+
},
350+
);
351+
352+
sentryTest(
353+
'[buffer-mode] manually starting replay ignores earlier performance entries when starting immediately',
354+
async ({ getLocalTestUrl, page, browserName }) => {
355+
// This was sometimes flaky on webkit, so skipping for now
356+
if (shouldSkipReplayTest() || browserName === 'webkit') {
357+
sentryTest.skip();
358+
}
359+
360+
const reqPromise0 = waitForReplayRequest(page, 0);
361+
362+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
363+
return route.fulfill({
364+
status: 200,
365+
contentType: 'application/json',
366+
body: JSON.stringify({ id: 'test-id' }),
367+
});
368+
});
369+
370+
const url = await getLocalTestUrl({ testDir: __dirname });
371+
372+
page.goto(url);
373+
374+
// Wait for everything to be initialized, then start replay as soon as possible
375+
await page.waitForFunction('!!window.Replay');
376+
await page.evaluate('window.Replay.start()');
377+
378+
const req0 = await reqPromise0;
379+
380+
const event0 = getReplayEvent(req0);
381+
const content0 = getReplayRecordingContent(req0);
382+
383+
expect(event0).toEqual(
384+
getExpectedReplayEvent({
385+
replay_type: 'session',
386+
}),
387+
);
388+
389+
const { performanceSpans } = content0;
390+
391+
expect(performanceSpans).toEqual([
392+
{
393+
op: 'memory',
394+
description: 'memory',
395+
startTimestamp: expect.any(Number),
396+
endTimestamp: expect.any(Number),
397+
data: {
398+
memory: {
399+
jsHeapSizeLimit: expect.any(Number),
400+
totalJSHeapSize: expect.any(Number),
401+
usedJSHeapSize: expect.any(Number),
402+
},
403+
},
404+
},
405+
]);
406+
},
407+
);
408+
290409
// Doing this in buffer mode to test changing error sample rate after first
291410
// error happens.
292411
sentryTest(

packages/replay-internal/src/replay.ts

+12-1
Original file line numberDiff line numberDiff line change
@@ -1046,11 +1046,22 @@ export class ReplayContainer implements ReplayContainerInterface {
10461046
* are included in the replay event before it is finished and sent to Sentry.
10471047
*/
10481048
private _addPerformanceEntries(): Promise<Array<AddEventResult | null>> {
1049-
const performanceEntries = createPerformanceEntries(this.performanceEntries).concat(this.replayPerformanceEntries);
1049+
let performanceEntries = createPerformanceEntries(this.performanceEntries).concat(this.replayPerformanceEntries);
10501050

10511051
this.performanceEntries = [];
10521052
this.replayPerformanceEntries = [];
10531053

1054+
// If we are manually starting, we want to ensure we only include performance entries
1055+
// that are after the initial timestamp
1056+
// The reason for this is that we may have performance entries from the page load, but may decide to start
1057+
// the replay later on, in which case we do not want to include these entries.
1058+
// without this, manually started replays can have events long before the actual replay recording starts,
1059+
// which messes with the timeline etc.
1060+
if (this._requiresManualStart) {
1061+
const initialTimestampInSeconds = this._context.initialTimestamp / 1000;
1062+
performanceEntries = performanceEntries.filter(entry => entry.start >= initialTimestampInSeconds);
1063+
}
1064+
10541065
return Promise.all(createPerformanceSpans(this, performanceEntries));
10551066
}
10561067

0 commit comments

Comments
 (0)