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
…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.
  • Loading branch information
mydea authored Oct 17, 2024
1 parent bece3e5 commit 77b3355
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,125 @@ 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 ignores earlier performance entries when starting immediately',
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;

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),
},
},
},
]);
},
);

// Doing this in buffer mode to test changing error sample rate after first
// error happens.
sentryTest(
Expand Down
13 changes: 12 additions & 1 deletion packages/replay-internal/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1046,11 +1046,22 @@ 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
// 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) {
const initialTimestampInSeconds = this._context.initialTimestamp / 1000;
performanceEntries = performanceEntries.filter(entry => entry.start >= initialTimestampInSeconds);
}

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

Expand Down

0 comments on commit 77b3355

Please sign in to comment.