Skip to content

fix(replay): Ignore older performance entries when starting manually #13969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading