Skip to content

Commit a0ff516

Browse files
mydeamydea
authored
feat(replay): Share performance instrumentation with tracing (#9296)
This streamlines web-vital & performance observer handling, by exposing a new `addPerformanceInstrumentationHandler` method from `@sentry-internal/tracing`. This works similar to the instrumentation in utils, where the first time you add instrumentation for a given type, it will add a performance observer. And any further calls will just add more callbacks. This way, we avoid having multiple of the same performance observers. Furthermore, this also aligns the handling of LCP capturing for replay. We used to do this separately, now we use the same data as for performance. Finally, while doing this I noticed that a whole bunch of performance observer stuff we used to capture in Replay, was actually discarded 😬 so no need to capture these anymore at all. (We can always add it back later, if needed) Some integration tests needed slight adjustments for this, probably due to minor timing semantics. But I think all the changes are good/"correct". I _also_ got rid of the event deduplication in replay. Closes #9246
1 parent 921b8df commit a0ff516

File tree

33 files changed

+1108
-1010
lines changed

33 files changed

+1108
-1010
lines changed

‎packages/browser-integration-tests/scripts/detectFlakyTests.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import * as glob from 'glob';
22
import * as path from 'path';
33
import * as childProcess from 'child_process';
4-
import { promisify } from 'util';
54

65
async function run(): Promise<void> {
76
let testPaths: string[] = [];

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { envelopeRequestParser } from '../../../utils/helpers';
55
import {
66
getDecompressedRecordingEvents,
77
getReplaySnapshot,
8+
isCustomSnapshot,
89
isReplayEvent,
910
REPLAY_DEFAULT_FLUSH_MAX_DELAY,
1011
shouldSkipReplayTest,
@@ -41,8 +42,8 @@ sentryTest(
4142
// We only want to count replays here
4243
if (event && isReplayEvent(event)) {
4344
const events = getDecompressedRecordingEvents(route.request());
44-
// this makes sure we ignore e.g. mouse move events which can otherwise lead to flakes
45-
if (events.length > 0) {
45+
// Make sure to not count mouse moves or performance spans
46+
if (events.filter(event => !isCustomSnapshot(event) || event.data.tag !== 'performanceSpan').length > 0) {
4647
called++;
4748
}
4849
}

‎packages/browser-integration-tests/suites/replay/largeMutations/defaultOptions/template.html‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
<meta charset="utf-8" />
55
</head>
66
<body>
7-
<button id="button-add">Add items</button>
8-
<button id="button-modify">Modify items</button>
9-
<button id="button-remove">Remove items</button>
7+
<button id="noop" type="button">Noop</button>
8+
<button id="button-add" type="button">Add items</button>
9+
<button id="button-modify" type="button">Modify items</button>
10+
<button id="button-remove" type="button">Remove items</button>
1011
<ul class="list"></ul>
1112

1213
<script>

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

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,25 +20,47 @@ sentryTest(
2020

2121
const url = await getLocalTestPath({ testDir: __dirname });
2222

23-
const [res0] = await Promise.all([waitForReplayRequest(page, 0), page.goto(url)]);
23+
// We have to click in order to ensure the LCP is generated, leading to consistent results
24+
async function gotoPageAndClick() {
25+
await page.goto(url);
26+
await page.click('#noop');
27+
}
28+
const [res0] = await Promise.all([waitForReplayRequest(page, 0), gotoPageAndClick()]);
2429
await forceFlushReplay();
2530

26-
const [res1] = await Promise.all([waitForReplayRequest(page), page.click('#button-add')]);
27-
await forceFlushReplay();
31+
const [res1] = await Promise.all([
32+
waitForReplayRequest(page, (_event, res) => {
33+
const parsed = getReplayRecordingContent(res);
34+
return !!parsed.incrementalSnapshots.length || !!parsed.fullSnapshots.length;
35+
}),
36+
page.click('#button-add'),
37+
forceFlushReplay(),
38+
]);
2839

29-
const [res2] = await Promise.all([waitForReplayRequest(page), page.click('#button-modify')]);
30-
await forceFlushReplay();
40+
const [res2] = await Promise.all([
41+
waitForReplayRequest(page, (_event, res) => {
42+
const parsed = getReplayRecordingContent(res);
43+
return !!parsed.incrementalSnapshots.length || !!parsed.fullSnapshots.length;
44+
}),
45+
page.click('#button-modify'),
46+
forceFlushReplay(),
47+
]);
3148

32-
const [res3] = await Promise.all([waitForReplayRequest(page), page.click('#button-remove')]);
33-
await forceFlushReplay();
49+
const [res3] = await Promise.all([
50+
waitForReplayRequest(page, (_event, res) => {
51+
const parsed = getReplayRecordingContent(res);
52+
return !!parsed.incrementalSnapshots.length || !!parsed.fullSnapshots.length;
53+
}),
54+
page.click('#button-remove'),
55+
forceFlushReplay(),
56+
]);
3457

3558
const replayData0 = getReplayRecordingContent(res0);
3659
const replayData1 = getReplayRecordingContent(res1);
3760
const replayData2 = getReplayRecordingContent(res2);
3861
const replayData3 = getReplayRecordingContent(res3);
3962

4063
expect(replayData0.fullSnapshots.length).toBe(1);
41-
expect(replayData0.incrementalSnapshots.length).toBe(0);
4264

4365
expect(replayData1.fullSnapshots.length).toBe(0);
4466
expect(replayData1.incrementalSnapshots.length).toBeGreaterThan(0);

‎packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/template.html‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
<meta charset="utf-8" />
55
</head>
66
<body>
7-
<button id="button-add">Add items</button>
8-
<button id="button-modify">Modify items</button>
9-
<button id="button-remove">Remove items</button>
7+
<button id="noop" type="button">Noop</button>
8+
<button id="button-add" type="button">Add items</button>
9+
<button id="button-modify" type="button">Modify items</button>
10+
<button id="button-remove" type="button">Remove items</button>
1011
<ul class="list"></ul>
1112

1213
<script>

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

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,26 @@ sentryTest(
2323
});
2424
});
2525

26-
const reqPromise0 = waitForReplayRequest(page, 0);
27-
2826
const url = await getLocalTestPath({ testDir: __dirname });
2927

30-
const [res0] = await Promise.all([reqPromise0, page.goto(url)]);
31-
await forceFlushReplay();
32-
33-
const reqPromise1 = waitForReplayRequest(page);
28+
// We have to click in order to ensure the LCP is generated, leading to consistent results
29+
async function gotoPageAndClick() {
30+
await page.goto(url);
31+
await page.click('#noop');
32+
}
3433

35-
const [res1] = await Promise.all([reqPromise1, page.click('#button-add')]);
34+
const [res0] = await Promise.all([waitForReplayRequest(page, 0), gotoPageAndClick()]);
3635
await forceFlushReplay();
3736

37+
const [res1] = await Promise.all([
38+
waitForReplayRequest(page, (_event, res) => {
39+
const parsed = getReplayRecordingContent(res);
40+
return !!parsed.incrementalSnapshots.length || !!parsed.fullSnapshots.length;
41+
}),
42+
page.click('#button-add'),
43+
forceFlushReplay(),
44+
]);
45+
3846
// replay should be stopped due to mutation limit
3947
let replay = await getReplaySnapshot(page);
4048
expect(replay.session).toBe(undefined);
@@ -48,7 +56,6 @@ sentryTest(
4856

4957
const replayData0 = getReplayRecordingContent(res0);
5058
expect(replayData0.fullSnapshots.length).toBe(1);
51-
expect(replayData0.incrementalSnapshots.length).toBe(0);
5259

5360
// Breadcrumbs (click and mutation);
5461
const replayData1 = getReplayRecordingContent(res1);

‎packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/template.html‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,6 @@
66
<body>
77
<div id="content"></div>
88
<img src="https://example.com/path/to/image.png" />
9+
<button type="button">Test button</button>
910
</body>
1011
</html>

‎packages/browser-integration-tests/suites/tracing/metrics/web-vitals-lcp/test.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@ sentryTest('should capture a LCP vital with element details.', async ({ browserN
1515
);
1616

1717
const url = await getLocalTestPath({ testDir: __dirname });
18-
await page.goto(url);
19-
20-
const eventData = await getFirstSentryEnvelopeRequest<Event>(page);
18+
const [eventData] = await Promise.all([
19+
getFirstSentryEnvelopeRequest<Event>(page),
20+
page.goto(url),
21+
page.click('button'),
22+
]);
2123

2224
expect(eventData.measurements).toBeDefined();
2325
expect(eventData.measurements?.lcp?.value).toBeDefined();

‎packages/browser-integration-tests/utils/replayHelpers.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ function isFullSnapshot(event: RecordingEvent): event is FullRecordingSnapshot {
153153
return event.type === EventType.FullSnapshot;
154154
}
155155

156-
function isCustomSnapshot(event: RecordingEvent): event is RecordingEvent & { data: CustomRecordingEvent } {
156+
export function isCustomSnapshot(event: RecordingEvent): event is RecordingEvent & { data: CustomRecordingEvent } {
157157
return event.type === EventType.Custom;
158158
}
159159

‎packages/e2e-tests/test-applications/react-create-hash-router/tests/behaviour-test.spec.ts‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,9 @@ test('Sends a Replay recording to Sentry', async ({ browser }) => {
183183
return window.sentryReplayId;
184184
});
185185

186+
// Keypress event ensures LCP is finished
187+
await page.type('body', 'Y');
188+
186189
// Wait for replay to be sent
187190

188191
if (replayId === undefined) {
@@ -229,10 +232,7 @@ test('Sends a Replay recording to Sentry', async ({ browser }) => {
229232
{ headers: { Authorization: `Bearer ${authToken}` } },
230233
);
231234

232-
return {
233-
status: response.status,
234-
data: response.data,
235-
};
235+
return response.status === 200 ? response.data[0] : response.status;
236236
} catch (e) {
237237
if (e instanceof AxiosError && e.response) {
238238
if (e.response.status !== 404) {
@@ -249,8 +249,5 @@ test('Sends a Replay recording to Sentry', async ({ browser }) => {
249249
timeout: EVENT_POLLING_TIMEOUT,
250250
},
251251
)
252-
.toEqual({
253-
status: 200,
254-
data: ReplayRecordingData,
255-
});
252+
.toEqual(ReplayRecordingData);
256253
});

0 commit comments

Comments
 (0)