Skip to content

Commit dbdfe48

Browse files
Lms24Lms24
authored
ref(browser): Unify CLS/LCP recording and add recording event attribute (#16866)
Refactor our CLS and LCP tracking and recording logic. We now have a `listenForWebVitalReportEvents` helper function which listens to page hides and navigation span starts and ensures that the standalone span logic is only called once per web vital. Previously, we had this logic in the CLS and LCP function and it was quite a lot of duplicated and identical code.
1 parent b9db2e6 commit dbdfe48

File tree

5 files changed

+125
-124
lines changed
  • dev-packages/browser-integration-tests/suites/tracing/metrics
    • web-vitals-cls-standalone-spans
    • web-vitals-lcp-standalone-spans
  • packages/browser-utils/src/metrics

5 files changed

+125
-124
lines changed

‎dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ sentryTest('captures a "GOOD" CLS vital with its source as a standalone span', a
6565
'sentry.exclusive_time': 0,
6666
'sentry.op': 'ui.webvital.cls',
6767
'sentry.origin': 'auto.http.browser.cls',
68+
'sentry.report_event': 'pagehide',
6869
transaction: expect.stringContaining('index.html'),
6970
'user_agent.original': expect.stringContaining('Chrome'),
7071
'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/),
@@ -134,6 +135,7 @@ sentryTest('captures a "MEH" CLS vital with its source as a standalone span', as
134135
'sentry.exclusive_time': 0,
135136
'sentry.op': 'ui.webvital.cls',
136137
'sentry.origin': 'auto.http.browser.cls',
138+
'sentry.report_event': 'pagehide',
137139
transaction: expect.stringContaining('index.html'),
138140
'user_agent.original': expect.stringContaining('Chrome'),
139141
'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/),
@@ -201,6 +203,7 @@ sentryTest('captures a "POOR" CLS vital with its source as a standalone span.',
201203
'sentry.exclusive_time': 0,
202204
'sentry.op': 'ui.webvital.cls',
203205
'sentry.origin': 'auto.http.browser.cls',
206+
'sentry.report_event': 'pagehide',
204207
transaction: expect.stringContaining('index.html'),
205208
'user_agent.original': expect.stringContaining('Chrome'),
206209
'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/),
@@ -269,6 +272,7 @@ sentryTest(
269272
'sentry.exclusive_time': 0,
270273
'sentry.op': 'ui.webvital.cls',
271274
'sentry.origin': 'auto.http.browser.cls',
275+
'sentry.report_event': 'pagehide',
272276
transaction: expect.stringContaining('index.html'),
273277
'user_agent.original': expect.stringContaining('Chrome'),
274278
'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/),
@@ -342,6 +346,8 @@ sentryTest(
342346
// Ensure the CLS span is connected to the pageload span and trace
343347
expect(spanEnvelopeItem.data?.['sentry.pageload.span_id']).toBe(pageloadSpanId);
344348
expect(spanEnvelopeItem.trace_id).toEqual(pageloadTraceId);
349+
350+
expect(spanEnvelopeItem.data?.['sentry.report_event']).toBe('pagehide');
345351
},
346352
);
347353

@@ -374,6 +380,8 @@ sentryTest('sends CLS of the initial page when soft-navigating to a new page', a
374380
expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.15);
375381
expect(spanEnvelopeItem.data?.['sentry.pageload.span_id']).toBe(pageloadEventData.contexts?.trace?.span_id);
376382
expect(spanEnvelopeItem.trace_id).toEqual(pageloadTraceId);
383+
384+
expect(spanEnvelopeItem.data?.['sentry.report_event']).toBe('navigation');
377385
});
378386

379387
sentryTest("doesn't send further CLS after the first navigation", async ({ getLocalTestUrl, page }) => {
@@ -398,6 +406,7 @@ sentryTest("doesn't send further CLS after the first navigation", async ({ getLo
398406
const spanEnvelope = (await spanEnvelopePromise)[0];
399407
const spanEnvelopeItem = spanEnvelope[1][0][1];
400408
expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0);
409+
expect(spanEnvelopeItem.data?.['sentry.report_event']).toBe('navigation');
401410

402411
getMultipleSentryEnvelopeRequests<SpanEnvelope>(page, 1, { envelopeType: 'span' }, () => {
403412
throw new Error('Unexpected span - This should not happen!');
@@ -442,6 +451,7 @@ sentryTest("doesn't send further CLS after the first page hide", async ({ getLoc
442451
const spanEnvelope = (await spanEnvelopePromise)[0];
443452
const spanEnvelopeItem = spanEnvelope[1][0][1];
444453
expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0);
454+
expect(spanEnvelopeItem.data?.['sentry.report_event']).toBe('pagehide');
445455

446456
getMultipleSentryEnvelopeRequests<SpanEnvelope>(page, 1, { envelopeType: 'span' }, () => {
447457
throw new Error('Unexpected span - This should not happen!');

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ sentryTest('captures LCP vital as a standalone span', async ({ getLocalTestUrl,
6464
'sentry.exclusive_time': 0,
6565
'sentry.op': 'ui.webvital.lcp',
6666
'sentry.origin': 'auto.http.browser.lcp',
67+
'sentry.report_event': 'pagehide',
6768
transaction: expect.stringContaining('index.html'),
6869
'user_agent.original': expect.stringContaining('Chrome'),
6970
'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/),
@@ -181,6 +182,7 @@ sentryTest('sends LCP of the initial page when soft-navigating to a new page', a
181182

182183
expect(spanEnvelopeItem.measurements?.lcp?.value).toBeGreaterThan(0);
183184
expect(spanEnvelopeItem.data?.['sentry.pageload.span_id']).toBe(pageloadEventData.contexts?.trace?.span_id);
185+
expect(spanEnvelopeItem.data?.['sentry.report_event']).toBe('navigation');
184186
expect(spanEnvelopeItem.trace_id).toBe(pageloadEventData.contexts?.trace?.trace_id);
185187
});
186188

@@ -194,10 +196,10 @@ sentryTest("doesn't send further LCP after the first navigation", async ({ getLo
194196

195197
const url = await getLocalTestUrl({ testDir: __dirname });
196198

197-
const eventData = await getFirstSentryEnvelopeRequest<SentryEvent>(page, url);
199+
const pageloadEventData = await getFirstSentryEnvelopeRequest<SentryEvent>(page, url);
198200

199-
expect(eventData.type).toBe('transaction');
200-
expect(eventData.contexts?.trace?.op).toBe('pageload');
201+
expect(pageloadEventData.type).toBe('transaction');
202+
expect(pageloadEventData.contexts?.trace?.op).toBe('pageload');
201203

202204
const spanEnvelopePromise = getMultipleSentryEnvelopeRequests<SpanEnvelope>(
203205
page,
@@ -214,6 +216,8 @@ sentryTest("doesn't send further LCP after the first navigation", async ({ getLo
214216
const spanEnvelope = (await spanEnvelopePromise)[0];
215217
const spanEnvelopeItem = spanEnvelope[1][0][1];
216218
expect(spanEnvelopeItem.measurements?.lcp?.value).toBeGreaterThan(0);
219+
expect(spanEnvelopeItem.data?.['sentry.report_event']).toBe('navigation');
220+
expect(spanEnvelopeItem.trace_id).toBe(pageloadEventData.contexts?.trace?.trace_id);
217221

218222
getMultipleSentryEnvelopeRequests<SpanEnvelope>(page, 1, { envelopeType: 'span' }, () => {
219223
throw new Error('Unexpected span - This should not happen!');
@@ -246,10 +250,10 @@ sentryTest("doesn't send further LCP after the first page hide", async ({ getLoc
246250

247251
const url = await getLocalTestUrl({ testDir: __dirname });
248252

249-
const eventData = await getFirstSentryEnvelopeRequest<SentryEvent>(page, url);
253+
const pageloadEventData = await getFirstSentryEnvelopeRequest<SentryEvent>(page, url);
250254

251-
expect(eventData.type).toBe('transaction');
252-
expect(eventData.contexts?.trace?.op).toBe('pageload');
255+
expect(pageloadEventData.type).toBe('transaction');
256+
expect(pageloadEventData.contexts?.trace?.op).toBe('pageload');
253257

254258
const spanEnvelopePromise = getMultipleSentryEnvelopeRequests<SpanEnvelope>(
255259
page,
@@ -266,6 +270,8 @@ sentryTest("doesn't send further LCP after the first page hide", async ({ getLoc
266270
const spanEnvelope = (await spanEnvelopePromise)[0];
267271
const spanEnvelopeItem = spanEnvelope[1][0][1];
268272
expect(spanEnvelopeItem.measurements?.lcp?.value).toBeGreaterThan(0);
273+
expect(spanEnvelopeItem.data?.['sentry.report_event']).toBe('pagehide');
274+
expect(spanEnvelopeItem.trace_id).toBe(pageloadEventData.contexts?.trace?.trace_id);
269275

270276
getMultipleSentryEnvelopeRequests<SpanEnvelope>(page, 1, { envelopeType: 'span' }, () => {
271277
throw new Error('Unexpected span - This should not happen!');

‎packages/browser-utils/src/metrics/cls.ts

Lines changed: 14 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,19 @@
11
import type { SpanAttributes } from '@sentry/core';
22
import {
33
browserPerformanceTimeOrigin,
4-
getActiveSpan,
5-
getClient,
64
getCurrentScope,
7-
getRootSpan,
85
htmlTreeAsString,
96
logger,
107
SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME,
118
SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT,
129
SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE,
1310
SEMANTIC_ATTRIBUTE_SENTRY_OP,
1411
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
15-
spanToJSON,
1612
} from '@sentry/core';
1713
import { DEBUG_BUILD } from '../debug-build';
1814
import { addClsInstrumentationHandler } from './instrument';
19-
import { msToSec, startStandaloneWebVitalSpan } from './utils';
20-
import { onHidden } from './web-vitals/lib/onHidden';
15+
import type { WebVitalReportEvent } from './utils';
16+
import { listenForWebVitalReportEvents, msToSec, startStandaloneWebVitalSpan, supportsWebVital } from './utils';
2117

2218
/**
2319
* Starts tracking the Cumulative Layout Shift on the current page and collects the value once
@@ -31,24 +27,11 @@ import { onHidden } from './web-vitals/lib/onHidden';
3127
export function trackClsAsStandaloneSpan(): void {
3228
let standaloneCLsValue = 0;
3329
let standaloneClsEntry: LayoutShift | undefined;
34-
let pageloadSpanId: string | undefined;
3530

36-
if (!supportsLayoutShift()) {
31+
if (!supportsWebVital('layout-shift')) {
3732
return;
3833
}
3934

40-
let sentSpan = false;
41-
function _collectClsOnce() {
42-
if (sentSpan) {
43-
return;
44-
}
45-
sentSpan = true;
46-
if (pageloadSpanId) {
47-
sendStandaloneClsSpan(standaloneCLsValue, standaloneClsEntry, pageloadSpanId);
48-
}
49-
cleanupClsHandler();
50-
}
51-
5235
const cleanupClsHandler = addClsInstrumentationHandler(({ metric }) => {
5336
const entry = metric.entries[metric.entries.length - 1] as LayoutShift | undefined;
5437
if (!entry) {
@@ -58,40 +41,18 @@ export function trackClsAsStandaloneSpan(): void {
5841
standaloneClsEntry = entry;
5942
}, true);
6043

61-
onHidden(() => {
62-
_collectClsOnce();
44+
listenForWebVitalReportEvents((reportEvent, pageloadSpanId) => {
45+
sendStandaloneClsSpan(standaloneCLsValue, standaloneClsEntry, pageloadSpanId, reportEvent);
46+
cleanupClsHandler();
6347
});
64-
65-
// Since the call chain of this function is synchronous and evaluates before the SDK client is created,
66-
// we need to wait with subscribing to a client hook until the client is created. Therefore, we defer
67-
// to the next tick after the SDK setup.
68-
setTimeout(() => {
69-
const client = getClient();
70-
71-
if (!client) {
72-
return;
73-
}
74-
75-
const unsubscribeStartNavigation = client.on('beforeStartNavigationSpan', (_, options) => {
76-
// we only want to collect LCP if we actually navigate. Redirects should be ignored.
77-
if (!options?.isRedirect) {
78-
_collectClsOnce();
79-
unsubscribeStartNavigation?.();
80-
}
81-
});
82-
83-
const activeSpan = getActiveSpan();
84-
if (activeSpan) {
85-
const rootSpan = getRootSpan(activeSpan);
86-
const spanJSON = spanToJSON(rootSpan);
87-
if (spanJSON.op === 'pageload') {
88-
pageloadSpanId = rootSpan.spanContext().spanId;
89-
}
90-
}
91-
}, 0);
9248
}
9349

94-
function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined, pageloadSpanId: string) {
50+
function sendStandaloneClsSpan(
51+
clsValue: number,
52+
entry: LayoutShift | undefined,
53+
pageloadSpanId: string,
54+
reportEvent: WebVitalReportEvent,
55+
) {
9556
DEBUG_BUILD && logger.log(`Sending CLS span (${clsValue})`);
9657

9758
const startTime = msToSec((browserPerformanceTimeOrigin() || 0) + (entry?.startTime || 0));
@@ -105,6 +66,8 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined,
10566
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0,
10667
// attach the pageload span id to the CLS span so that we can link them in the UI
10768
'sentry.pageload.span_id': pageloadSpanId,
69+
// describes what triggered the web vital to be reported
70+
'sentry.report_event': reportEvent,
10871
};
10972

11073
// Add CLS sources as span attributes to help with debugging layout shifts
@@ -133,11 +96,3 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined,
13396
span.end(startTime);
13497
}
13598
}
136-
137-
function supportsLayoutShift(): boolean {
138-
try {
139-
return PerformanceObserver.supportedEntryTypes.includes('layout-shift');
140-
} catch {
141-
return false;
142-
}
143-
}

‎packages/browser-utils/src/metrics/lcp.ts

Lines changed: 9 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,19 @@
11
import type { SpanAttributes } from '@sentry/core';
22
import {
33
browserPerformanceTimeOrigin,
4-
getActiveSpan,
5-
getClient,
64
getCurrentScope,
7-
getRootSpan,
85
htmlTreeAsString,
96
logger,
107
SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME,
118
SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT,
129
SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE,
1310
SEMANTIC_ATTRIBUTE_SENTRY_OP,
1411
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
15-
spanToJSON,
1612
} from '@sentry/core';
1713
import { DEBUG_BUILD } from '../debug-build';
1814
import { addLcpInstrumentationHandler } from './instrument';
19-
import { msToSec, startStandaloneWebVitalSpan } from './utils';
20-
import { onHidden } from './web-vitals/lib/onHidden';
15+
import type { WebVitalReportEvent } from './utils';
16+
import { listenForWebVitalReportEvents, msToSec, startStandaloneWebVitalSpan, supportsWebVital } from './utils';
2117

2218
/**
2319
* Starts tracking the Largest Contentful Paint on the current page and collects the value once
@@ -31,24 +27,11 @@ import { onHidden } from './web-vitals/lib/onHidden';
3127
export function trackLcpAsStandaloneSpan(): void {
3228
let standaloneLcpValue = 0;
3329
let standaloneLcpEntry: LargestContentfulPaint | undefined;
34-
let pageloadSpanId: string | undefined;
3530

36-
if (!supportsLargestContentfulPaint()) {
31+
if (!supportsWebVital('largest-contentful-paint')) {
3732
return;
3833
}
3934

40-
let sentSpan = false;
41-
function _collectLcpOnce() {
42-
if (sentSpan) {
43-
return;
44-
}
45-
sentSpan = true;
46-
if (pageloadSpanId) {
47-
_sendStandaloneLcpSpan(standaloneLcpValue, standaloneLcpEntry, pageloadSpanId);
48-
}
49-
cleanupLcpHandler();
50-
}
51-
5235
const cleanupLcpHandler = addLcpInstrumentationHandler(({ metric }) => {
5336
const entry = metric.entries[metric.entries.length - 1] as LargestContentfulPaint | undefined;
5437
if (!entry) {
@@ -58,37 +41,10 @@ export function trackLcpAsStandaloneSpan(): void {
5841
standaloneLcpEntry = entry;
5942
}, true);
6043

61-
onHidden(() => {
62-
_collectLcpOnce();
44+
listenForWebVitalReportEvents((reportEvent, pageloadSpanId) => {
45+
_sendStandaloneLcpSpan(standaloneLcpValue, standaloneLcpEntry, pageloadSpanId, reportEvent);
46+
cleanupLcpHandler();
6347
});
64-
65-
// Since the call chain of this function is synchronous and evaluates before the SDK client is created,
66-
// we need to wait with subscribing to a client hook until the client is created. Therefore, we defer
67-
// to the next tick after the SDK setup.
68-
setTimeout(() => {
69-
const client = getClient();
70-
71-
if (!client) {
72-
return;
73-
}
74-
75-
const unsubscribeStartNavigation = client.on('beforeStartNavigationSpan', (_, options) => {
76-
// we only want to collect LCP if we actually navigate. Redirects should be ignored.
77-
if (!options?.isRedirect) {
78-
_collectLcpOnce();
79-
unsubscribeStartNavigation?.();
80-
}
81-
});
82-
83-
const activeSpan = getActiveSpan();
84-
if (activeSpan) {
85-
const rootSpan = getRootSpan(activeSpan);
86-
const spanJSON = spanToJSON(rootSpan);
87-
if (spanJSON.op === 'pageload') {
88-
pageloadSpanId = rootSpan.spanContext().spanId;
89-
}
90-
}
91-
}, 0);
9248
}
9349

9450
/**
@@ -98,6 +54,7 @@ export function _sendStandaloneLcpSpan(
9854
lcpValue: number,
9955
entry: LargestContentfulPaint | undefined,
10056
pageloadSpanId: string,
57+
reportEvent: WebVitalReportEvent,
10158
) {
10259
DEBUG_BUILD && logger.log(`Sending LCP span (${lcpValue})`);
10360

@@ -112,6 +69,8 @@ export function _sendStandaloneLcpSpan(
11269
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: 0, // LCP is a point-in-time metric
11370
// attach the pageload span id to the LCP span so that we can link them in the UI
11471
'sentry.pageload.span_id': pageloadSpanId,
72+
// describes what triggered the web vital to be reported
73+
'sentry.report_event': reportEvent,
11574
};
11675

11776
if (entry) {
@@ -149,11 +108,3 @@ export function _sendStandaloneLcpSpan(
149108
span.end(startTime);
150109
}
151110
}
152-
153-
function supportsLargestContentfulPaint(): boolean {
154-
try {
155-
return PerformanceObserver.supportedEntryTypes.includes('largest-contentful-paint');
156-
} catch {
157-
return false;
158-
}
159-
}

0 commit comments

Comments
 (0)