Skip to content

Commit 330c75d

Browse files
logaretmcodex
andcommitted
ref(browser): let web vitals own pageload flush
Co-Authored-By: OpenAI GPT-5 <codex@openai.com>
1 parent fe17013 commit 330c75d

3 files changed

Lines changed: 67 additions & 42 deletions

File tree

packages/browser/src/integrations/webVitals.ts

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Client, IntegrationFn, Span } from '@sentry/core/browser';
1+
import type { IntegrationFn, Span } from '@sentry/core/browser';
22
import { defineIntegration, hasSpanStreamingEnabled } from '@sentry/core/browser';
33
import {
44
addWebVitalsToSpan,
@@ -29,16 +29,6 @@ export interface WebVitalsOptions {
2929
}>;
3030
}
3131

32-
const collectWebVitalsCallbacks = new WeakMap<Client, (span: Span) => void>();
33-
34-
/**
35-
* Finalizes the collected web vitals and writes them onto the given (pageload) span.
36-
* Called by `browserTracingIntegration` when the pageload/navigation span ends.
37-
*/
38-
export function collectWebVitalsForClient(client: Client, span: Span): void {
39-
collectWebVitalsCallbacks.get(client)?.(span);
40-
}
41-
4232
/**
4333
* Captures Core Web Vitals (LCP, CLS, INP) and related pageload vitals.
4434
*
@@ -66,14 +56,16 @@ export const webVitalsIntegration = defineIntegration((options: WebVitalsOptions
6656
client,
6757
});
6858

69-
collectWebVitalsCallbacks.set(client, span => {
70-
finalizeWebVitals();
71-
addWebVitalsToSpan(span, {
72-
// CLS/LCP are recorded as pageload span measurements only when they're neither
73-
// tracked as standalone spans nor handled by span streaming (and not ignored).
74-
recordClsOnPageloadSpan: recordClsStandaloneSpans === false,
75-
recordLcpOnPageloadSpan: recordLcpStandaloneSpans === false,
76-
spanStreamingEnabled,
59+
client.on('afterStartPageLoadSpan', span => {
60+
wrapPageloadSpanEnd(span, () => {
61+
finalizeWebVitals();
62+
addWebVitalsToSpan(span, {
63+
// CLS/LCP are recorded as pageload span measurements only when they're neither
64+
// tracked as standalone spans nor handled by span streaming (and not ignored).
65+
recordClsOnPageloadSpan: recordClsStandaloneSpans === false,
66+
recordLcpOnPageloadSpan: recordLcpStandaloneSpans === false,
67+
spanStreamingEnabled,
68+
});
7769
});
7870
});
7971

@@ -98,3 +90,17 @@ export const webVitalsIntegration = defineIntegration((options: WebVitalsOptions
9890
},
9991
};
10092
}) satisfies IntegrationFn;
93+
94+
function wrapPageloadSpanEnd(span: Span, beforeEnd: () => void): void {
95+
let hasEnded = false;
96+
const originalEnd = span.end.bind(span);
97+
98+
span.end = (...args: Parameters<Span['end']>): void => {
99+
if (!hasEnded) {
100+
hasEnded = true;
101+
beforeEnd();
102+
}
103+
104+
originalEnd(...args);
105+
};
106+
}

packages/browser/src/tracing/browserTracingIntegration.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,7 @@ import {
4747
import { DEBUG_BUILD } from '../debug-build';
4848
import { getHttpRequestData, WINDOW } from '../helpers';
4949
import { fetchStreamPerformanceIntegration } from '../integrations/fetchStreamPerformance';
50-
import {
51-
collectWebVitalsForClient,
52-
WEB_VITALS_INTEGRATION_NAME,
53-
webVitalsIntegration,
54-
} from '../integrations/webVitals';
50+
import { WEB_VITALS_INTEGRATION_NAME, webVitalsIntegration } from '../integrations/webVitals';
5551
import { registerBackgroundTabDetection } from './backgroundtab';
5652
import { linkTraces } from './linkedTraces';
5753
import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request';
@@ -456,9 +452,6 @@ export const browserTracingIntegration = ((options: Partial<BrowserTracingOption
456452
// should wait for finish signal if it's a pageload transaction
457453
disableAutoFinish: isPageloadSpan,
458454
beforeSpanEnd: span => {
459-
// The webVitalsIntegration owns all web vital logic; it finalizes the collected
460-
// web vitals and writes them onto the pageload span.
461-
collectWebVitalsForClient(client, span);
462455
addPerformanceEntries(span, {
463456
ignoreResourceSpans,
464457
ignorePerformanceApiSpans,

packages/browser/test/integrations/webVitals.test.ts

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2-
import { collectWebVitalsForClient, webVitalsIntegration } from '../../src/integrations/webVitals';
2+
import { webVitalsIntegration } from '../../src/integrations/webVitals';
33

44
const mockAddWebVitalsToSpan = vi.hoisted(() => vi.fn());
55
const mockRegisterInpInteractionListener = vi.hoisted(() => vi.fn());
@@ -19,6 +19,29 @@ vi.mock('@sentry-internal/browser-utils', () => ({
1919
trackLcpAsSpan: mockTrackLcpAsSpan,
2020
}));
2121

22+
function getMockClient(options: Record<string, unknown> = {}) {
23+
const listeners = new Map<string, Array<(...args: unknown[]) => void>>();
24+
25+
return {
26+
getOptions: () => options,
27+
on: vi.fn((hook: string, callback: (...args: unknown[]) => void) => {
28+
const callbacks = listeners.get(hook) ?? [];
29+
callbacks.push(callback);
30+
listeners.set(hook, callbacks);
31+
32+
return () => {
33+
const updatedCallbacks = listeners.get(hook)?.filter(cb => cb !== callback) ?? [];
34+
listeners.set(hook, updatedCallbacks);
35+
};
36+
}),
37+
emit: (hook: string, ...args: unknown[]) => {
38+
listeners.get(hook)?.forEach(callback => {
39+
callback(...args);
40+
});
41+
},
42+
};
43+
}
44+
2245
describe('webVitalsIntegration', () => {
2346
beforeEach(() => {
2447
vi.clearAllMocks();
@@ -30,7 +53,7 @@ describe('webVitalsIntegration', () => {
3053
});
3154

3255
it('tracks web vitals with the existing non-streaming behavior by default', () => {
33-
const client = { getOptions: () => ({}) };
56+
const client = getMockClient();
3457
const integration = webVitalsIntegration();
3558

3659
integration.setup?.(client as never);
@@ -49,7 +72,7 @@ describe('webVitalsIntegration', () => {
4972
});
5073

5174
it('keeps standalone LCP and CLS experiments working', () => {
52-
const client = { getOptions: () => ({}) };
75+
const client = getMockClient();
5376
const integration = webVitalsIntegration({
5477
_experiments: {
5578
enableStandaloneClsSpans: true,
@@ -67,7 +90,7 @@ describe('webVitalsIntegration', () => {
6790
});
6891

6992
it('tracks LCP, CLS and INP as streamed spans when span streaming is enabled', () => {
70-
const client = { getOptions: () => ({ traceLifecycle: 'stream' }) };
93+
const client = getMockClient({ traceLifecycle: 'stream' });
7194
const integration = webVitalsIntegration();
7295

7396
integration.setup?.(client as never);
@@ -86,7 +109,7 @@ describe('webVitalsIntegration', () => {
86109
});
87110

88111
it('supports ignoring selected web vitals for browserTracingIntegration compatibility', () => {
89-
const client = { getOptions: () => ({}) };
112+
const client = getMockClient();
90113
const integration = webVitalsIntegration({ ignore: ['cls', 'inp', 'lcp'] });
91114

92115
integration.setup?.(client as never);
@@ -102,14 +125,15 @@ describe('webVitalsIntegration', () => {
102125
expect(mockRegisterInpInteractionListener).not.toHaveBeenCalled();
103126
});
104127

105-
it('finalizes web vitals and writes them onto the span when collected for the client', () => {
128+
it('finalizes web vitals and writes them onto the pageload span before it ends', () => {
106129
const finalizeWebVitals = vi.fn();
107-
const client = { getOptions: () => ({}) };
108-
const span = {};
130+
const client = getMockClient();
131+
const span = { end: vi.fn() };
109132
mockStartTrackingWebVitals.mockReturnValue(finalizeWebVitals);
110133

111134
webVitalsIntegration().setup?.(client as never);
112-
collectWebVitalsForClient(client as never, span as never);
135+
client.emit('afterStartPageLoadSpan', span);
136+
span.end();
113137

114138
expect(finalizeWebVitals).toHaveBeenCalledTimes(1);
115139
expect(mockAddWebVitalsToSpan).toHaveBeenCalledWith(span, {
@@ -120,11 +144,12 @@ describe('webVitalsIntegration', () => {
120144
});
121145

122146
it('does not record CLS/LCP on the pageload span when span streaming is enabled', () => {
123-
const client = { getOptions: () => ({ traceLifecycle: 'stream' }) };
124-
const span = {};
147+
const client = getMockClient({ traceLifecycle: 'stream' });
148+
const span = { end: vi.fn() };
125149

126150
webVitalsIntegration().setup?.(client as never);
127-
collectWebVitalsForClient(client as never, span as never);
151+
client.emit('afterStartPageLoadSpan', span);
152+
span.end();
128153

129154
expect(mockAddWebVitalsToSpan).toHaveBeenCalledWith(span, {
130155
recordClsOnPageloadSpan: false,
@@ -134,14 +159,15 @@ describe('webVitalsIntegration', () => {
134159
});
135160

136161
it('does not record CLS/LCP on the pageload span when standalone spans are enabled', () => {
137-
const client = { getOptions: () => ({}) };
138-
const span = {};
162+
const client = getMockClient();
163+
const span = { end: vi.fn() };
139164
const integration = webVitalsIntegration({
140165
_experiments: { enableStandaloneClsSpans: true, enableStandaloneLcpSpans: true },
141166
});
142167

143168
integration.setup?.(client as never);
144-
collectWebVitalsForClient(client as never, span as never);
169+
client.emit('afterStartPageLoadSpan', span);
170+
span.end();
145171

146172
expect(mockAddWebVitalsToSpan).toHaveBeenCalledWith(span, {
147173
recordClsOnPageloadSpan: false,

0 commit comments

Comments
 (0)