Skip to content

Commit

Permalink
fix(replay): Ignore older performance entries when starting manually
Browse files Browse the repository at this point in the history
  • Loading branch information
mydea committed Oct 14, 2024
1 parent 0ccf8ce commit 5ee3f10
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,112 @@ sentryTest(
},
);

sentryTest(
'[buffer-mode] manually starting replay ignores earlier performance entries',
async ({ getLocalTestUrl, page, browserName }) => {
// This was sometimes flaky on webkit, so skipping for now
if (shouldSkipReplayTest() || browserName === 'webkit') {
sentryTest.skip();
}

const reqPromise0 = waitForReplayRequest(page, 0);

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 getLocalTestUrl({ testDir: __dirname });

await page.goto(url);

// Wait for everything to be initialized - Replay is not running yet
await page.waitForFunction('!!window.Replay');

// Wait for a second, then start replay
await new Promise(resolve => setTimeout(resolve, 1000));
await page.evaluate('window.Replay.start()');

const req0 = await reqPromise0;

const event0 = getReplayEvent(req0);
const content0 = getReplayRecordingContent(req0);

expect(event0).toEqual(
getExpectedReplayEvent({
replay_type: 'session',
}),
);

const { performanceSpans } = content0;

// Here, we test that this does not contain any web-vital etc. performance spans
// as these have been emitted _before_ the replay was manually started
expect(performanceSpans).toEqual([
{
op: 'memory',
description: 'memory',
startTimestamp: expect.any(Number),
endTimestamp: expect.any(Number),
data: {
memory: {
jsHeapSizeLimit: expect.any(Number),
totalJSHeapSize: expect.any(Number),
usedJSHeapSize: expect.any(Number),
},
},
},
]);
},
);

sentryTest(
'[buffer-mode] manually starting replay includes performance entries with 1s wiggle room',
async ({ getLocalTestUrl, page, browserName }) => {
// This was sometimes flaky on webkit, so skipping for now
if (shouldSkipReplayTest() || browserName === 'webkit') {
sentryTest.skip();
}

const reqPromise0 = waitForReplayRequest(page, 0);

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 getLocalTestUrl({ testDir: __dirname });

page.goto(url);

// Wait for everything to be initialized, then start replay as soon as possible
await page.waitForFunction('!!window.Replay');
await page.evaluate('window.Replay.start()');

const req0 = await reqPromise0;

const event0 = getReplayEvent(req0);
const content0 = getReplayRecordingContent(req0);

expect(event0).toEqual(
getExpectedReplayEvent({
replay_type: 'session',
}),
);

const { performanceSpans } = content0;

// web vitals etc. are included with 1s wiggle room, to accomodate "immediate" start
expect(performanceSpans.length).toBeGreaterThan(1);
},
);

// Doing this in buffer mode to test changing error sample rate after first
// error happens.
sentryTest(
Expand Down
14 changes: 13 additions & 1 deletion packages/replay-internal/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1046,11 +1046,23 @@ export class ReplayContainer implements ReplayContainerInterface {
* are included in the replay event before it is finished and sent to Sentry.
*/
private _addPerformanceEntries(): Promise<Array<AddEventResult | null>> {
const performanceEntries = createPerformanceEntries(this.performanceEntries).concat(this.replayPerformanceEntries);
let performanceEntries = createPerformanceEntries(this.performanceEntries).concat(this.replayPerformanceEntries);

this.performanceEntries = [];
this.replayPerformanceEntries = [];

// If we are manually starting, we want to ensure we only include performance entries
// that are after the initial timestamp, with 1s wiggle room.
// The reason for this is that we may have performance entries from the page load, but may decide to start
// the replay later on, in which case we do not want to include these entries.
// without this, manually started replays can have events long before the actual replay recording starts,
// which messes with the timeline etc.
if (this._requiresManualStart) {
// We leave 1s wiggle room to accomodate timing differences for "immedidate" manual starts
const initialTimestampInSeconds = this._context.initialTimestamp / 1000 - 1;
performanceEntries = performanceEntries.filter(entry => entry.start >= initialTimestampInSeconds);
}

return Promise.all(createPerformanceSpans(this, performanceEntries));
}

Expand Down

0 comments on commit 5ee3f10

Please sign in to comment.