Skip to content

Commit 00a507c

Browse files
committed
feat(browser): Make bfcache web vitals configurable instead of always dropped
`withoutBfcache` dropped every metric web-vitals reported after a back/forward-cache restore. That was the right call while there was nothing to attach them to: a restore reuses the frozen document, so the values would have landed on the span the page had before it was frozen. Now that a restore gets its own navigation span, they have a correct parent, so the drop becomes an option rather than a hard rule: webVitals: { bfcache: true } Off by default. A restore is near-instant, so its vitals are a different population from page load vitals, and the earlier concern about skewing aggregates still applies to anyone who has not decided how to treat them. `browser.navigation.type: bfcache` makes them separable once enabled. Reporting per navigation rather than per page load is now what the tracker flag means, since bfcache restores need it for the same reason soft navigations do. `reportAllChanges` is switched off for either, since the per-navigation path relies on each reported value already being final for its navigation. Verified end to end in Chrome 152: a restore emits LCP and CLS parented to the bfcache navigation span on the restore's own trace, and a bfcache-ineligible back navigation still falls back to a page load.
1 parent 08b4500 commit 00a507c

6 files changed

Lines changed: 150 additions & 32 deletions

File tree

packages/browser-utils/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export {
55
addLcpInstrumentationHandler,
66
addInpInstrumentationHandler,
77
addFcpInstrumentationHandler,
8+
enableBfcacheReporting,
89
enableSoftNavigationReporting,
910
} from './instrumentation/performanceObserver';
1011

packages/browser-utils/src/instrumentation/performanceObserver.ts

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ let _previousInp: Metric | undefined;
164164
let _previousFcp: Metric | undefined;
165165

166166
let _reportSoftNavs = false;
167+
let _reportBfcache = false;
167168

168169
/**
169170
* Opt the CLS, LCP and INP observers into reporting metrics for soft navigations.
@@ -186,6 +187,21 @@ export function enableSoftNavigationReporting(): void {
186187
_reportSoftNavs = true;
187188
}
188189

190+
/**
191+
* Opt the CLS, LCP and INP observers into reporting metrics for back/forward-cache restores.
192+
*
193+
* web-vitals re-reports each metric after a restore, tagged with a `back-forward-cache` navigation
194+
* type. A restore is a new page view measured against a document that was never reloaded, so the
195+
* values only mean anything if there is a fresh root span for them to belong to. Without one they
196+
* would attach to the span the page had before it was frozen, which is why this is off by default.
197+
*
198+
* Like `enableSoftNavigationReporting`, this only affects observers instrumented after it is
199+
* called.
200+
*/
201+
export function enableBfcacheReporting(): void {
202+
_reportBfcache = true;
203+
}
204+
189205
/**
190206
* Add a callback that will be triggered when a CLS metric is available.
191207
* Returns a cleanup callback which can be called to remove the instrumentation handler.
@@ -292,16 +308,12 @@ function triggerHandlers(type: InstrumentHandlerType, data: unknown): void {
292308
}
293309

294310
/**
295-
* Wraps a metric callback so that metrics reported after a back/forward-cache restore are ignored.
296-
*
297-
* web-vitals re-reports each metric after a bfcache restore (tagged with a `back-forward-cache`
298-
* navigation type). We intentionally drop those for now: our reporting assumes one set of vitals
299-
* per page load, so surfacing bfcache re-reports would skew the data until we're ready to model
300-
* and communicate them.
311+
* Wraps a metric callback so that metrics reported after a back/forward-cache restore are dropped
312+
* unless `enableBfcacheReporting` was called. See there for why they are off by default.
301313
*/
302-
function withoutBfcache(callback: (metric: Metric) => void): (metric: Metric) => void {
314+
function unlessBfcacheDisabled(callback: (metric: Metric) => void): (metric: Metric) => void {
303315
return metric => {
304-
if (metric.navigationType === 'back-forward-cache') {
316+
if (!_reportBfcache && metric.navigationType === 'back-forward-cache') {
305317
return;
306318
}
307319
callback(metric);
@@ -310,35 +322,35 @@ function withoutBfcache(callback: (metric: Metric) => void): (metric: Metric) =>
310322

311323
function instrumentCls(): StopListening {
312324
return onCLS(
313-
withoutBfcache(metric => {
325+
unlessBfcacheDisabled(metric => {
314326
triggerHandlers('cls', {
315327
metric,
316328
});
317329
_previousCls = metric;
318330
}),
319331
// We want the callback to be called whenever the CLS value updates.
320332
// By default, the callback is only called when the tab goes to the background.
321-
{ reportAllChanges: !_reportSoftNavs, reportSoftNavs: _reportSoftNavs },
333+
{ reportAllChanges: !_reportSoftNavs && !_reportBfcache, reportSoftNavs: _reportSoftNavs },
322334
);
323335
}
324336

325337
function instrumentLcp(): StopListening {
326338
return onLCP(
327-
withoutBfcache(metric => {
339+
unlessBfcacheDisabled(metric => {
328340
triggerHandlers('lcp', {
329341
metric,
330342
});
331343
_previousLcp = metric;
332344
}),
333345
// We want the callback to be called whenever the LCP value updates.
334346
// By default, the callback is only called when the tab goes to the background.
335-
{ reportAllChanges: !_reportSoftNavs, reportSoftNavs: _reportSoftNavs },
347+
{ reportAllChanges: !_reportSoftNavs && !_reportBfcache, reportSoftNavs: _reportSoftNavs },
336348
);
337349
}
338350

339351
function instrumentTtfb(): StopListening {
340352
return onTTFB(
341-
withoutBfcache(metric => {
353+
unlessBfcacheDisabled(metric => {
342354
triggerHandlers('ttfb', {
343355
metric,
344356
});
@@ -349,7 +361,7 @@ function instrumentTtfb(): StopListening {
349361

350362
function instrumentFcp(): StopListening {
351363
return onFCP(
352-
withoutBfcache(metric => {
364+
unlessBfcacheDisabled(metric => {
353365
triggerHandlers('fcp', {
354366
metric,
355367
});
@@ -360,7 +372,7 @@ function instrumentFcp(): StopListening {
360372

361373
function instrumentInp(): StopListening {
362374
return onINP(
363-
withoutBfcache(metric => {
375+
unlessBfcacheDisabled(metric => {
364376
triggerHandlers('inp', {
365377
metric,
366378
});

packages/browser-utils/src/web-vitals/spans.ts

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,12 @@ type WebVitalMetric = Parameters<Parameters<typeof addLcpInstrumentationHandler>
4646
type InpMetric = Parameters<InstrumentationHandlerCallback>[0]['metric'];
4747

4848
/**
49-
* Reports a web vital once per navigation, for browsers reporting soft navigations.
49+
* Reports a web vital once per navigation, rather than once per page load.
5050
*
51-
* With `reportSoftNavs`, web-vitals restarts the metric on every soft navigation and force-reports
52-
* the previous one just before it does (and again on pagehide). Since we also drop
53-
* `reportAllChanges` in this mode, every value we're handed is already the final one for its
54-
* navigation, so there is nothing to accumulate: each report is a span.
51+
* web-vitals restarts the metric on every soft navigation and force-reports the previous one just
52+
* before it does (and again on pagehide), and re-reports every metric after a bfcache restore.
53+
* Since `reportAllChanges` is off in this mode, every value we're handed is already the final one
54+
* for its navigation, so there is nothing to accumulate: each report is a span.
5555
*/
5656
function trackWebVitalPerNavigation<M extends WebVitalMetric>(
5757
client: Client,
@@ -77,19 +77,28 @@ function trackWebVitalPerNavigation<M extends WebVitalMetric>(
7777
return;
7878
}
7979

80+
if (metric.navigationType === 'back-forward-cache') {
81+
// A restore reuses the frozen document, so the pageload span above belongs to the page view
82+
// from before the freeze. The active root span is the navigation span started for the
83+
// restore, which is the page view these values were actually measured on.
84+
const activeSpan = getActiveSpan();
85+
send(metric, activeSpan ? getRootSpan(activeSpan) : undefined, undefined);
86+
return;
87+
}
88+
8089
send(metric, pageloadSpan, undefined);
8190
});
8291
}
8392

8493
/**
8594
* Tracks LCP as a streamed span.
8695
*/
87-
export function trackLcpAsSpan(client: Client, reportSoftNavs = false): void {
96+
export function trackLcpAsSpan(client: Client, perNavigation = false): void {
8897
if (!supportsWebVital('largest-contentful-paint')) {
8998
return;
9099
}
91100

92-
if (reportSoftNavs) {
101+
if (perNavigation) {
93102
trackWebVitalPerNavigation(client, addLcpInstrumentationHandler, (metric, parentSpan, softNavigationId) => {
94103
const entry = metric.entries[metric.entries.length - 1] as LargestContentfulPaint | undefined;
95104
_sendLcpSpan(metric.value, entry, parentSpan, undefined, softNavigationId, metric.navigationType);
@@ -170,12 +179,12 @@ export function _sendLcpSpan(
170179
/**
171180
* Tracks CLS as a streamed span.
172181
*/
173-
export function trackClsAsSpan(client: Client, reportSoftNavs = false): void {
182+
export function trackClsAsSpan(client: Client, perNavigation = false): void {
174183
if (!supportsWebVital('layout-shift')) {
175184
return;
176185
}
177186

178-
if (reportSoftNavs) {
187+
if (perNavigation) {
179188
trackWebVitalPerNavigation(client, addClsInstrumentationHandler, (metric, parentSpan, softNavigationId) => {
180189
const entry = metric.entries[metric.entries.length - 1] as LayoutShift | undefined;
181190
_sendClsSpan(metric.value, entry, parentSpan, undefined, softNavigationId, metric.navigationType);
@@ -250,7 +259,7 @@ export function _sendClsSpan(
250259
* Requires `registerInpInteractionListener()` to be called separately for cached element names and
251260
* root spans per interaction.
252261
*/
253-
export function trackInpAsSpan(client: Client, reportSoftNavs = false): void {
262+
export function trackInpAsSpan(client: Client, perNavigation = false): void {
254263
const performance = getBrowserPerformanceAPI();
255264
if (!performance || !browserPerformanceTimeOrigin()) {
256265
return;
@@ -263,7 +272,7 @@ export function trackInpAsSpan(client: Client, reportSoftNavs = false): void {
263272
// TODO(standalone): once the static trace lifecycle is dropped, INP always streams; drop this flag.
264273
const standalone = !hasSpanStreamingEnabled(client);
265274

266-
if (reportSoftNavs) {
275+
if (perNavigation) {
267276
// INP restarts per navigation and reports once that navigation is over, by which point the
268277
// navigation span has ended and the interaction cache no longer knows about it. The metric
269278
// says which navigation it belongs to, so INP is attributed exactly like LCP and CLS.

packages/browser-utils/test/web-vitals/spans.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,33 @@ describe('soft navigation web vitals', () => {
817817
expect(calls[1]![0].parentSpan).toBe(navigationSpan);
818818
});
819819

820+
it('reports a bfcache restore against the restore navigation span, not the frozen pageload', () => {
821+
const bfcacheNavigationSpan = { spanContext: () => ({ spanId: 'bfcache-nav' }) } as any;
822+
vi.mocked(SentryCore.getActiveSpan).mockReturnValue(bfcacheNavigationSpan);
823+
vi.mocked(SentryCore.getRootSpan).mockReturnValue(bfcacheNavigationSpan);
824+
825+
trackLcpAsSpan(client, true);
826+
827+
lcpCallback({
828+
metric: {
829+
value: 40,
830+
navigationId: 9,
831+
navigationType: 'back-forward-cache',
832+
entries: [{ startTime: 40, element: {} }],
833+
},
834+
});
835+
836+
expect(SentryCoreBrowser.startInactiveSpan).toHaveBeenCalledWith(
837+
expect.objectContaining({
838+
parentSpan: bfcacheNavigationSpan,
839+
attributes: expect.objectContaining({ 'browser.navigation.type': 'bfcache' }),
840+
}),
841+
);
842+
expect(SentryCoreBrowser.startInactiveSpan).not.toHaveBeenCalledWith(
843+
expect.objectContaining({ parentSpan: pageloadSpan }),
844+
);
845+
});
846+
820847
it('drops soft navigation vitals that could not be correlated', () => {
821848
vi.spyOn(softNavs, 'getNavigationSpanForMetric').mockReturnValue(undefined);
822849

packages/browser/src/integrations/webVitals.ts

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { IntegrationFn, Span } from '@sentry/core';
22
import { defineIntegration, hasSpanStreamingEnabled } from '@sentry/core';
33
import {
44
addWebVitalsToSpan,
5+
enableBfcacheReporting,
56
enableSoftNavigationReporting,
67
registerInpInteractionListener,
78
startSoftNavigationCorrelation,
@@ -42,6 +43,22 @@ export interface WebVitalsOptions {
4243
* Default: `true`
4344
*/
4445
softNavigations?: boolean;
46+
47+
/**
48+
* Report a fresh set of LCP, CLS and INP after the page is restored from the back/forward cache.
49+
*
50+
* A restore is a new page view measured against a document that was never reloaded, so its vitals
51+
* are reported against the navigation span `browserTracingIntegration` starts for the restore,
52+
* and tagged `browser.navigation.type: bfcache`. They measure a near-instant restore rather than
53+
* a document load, so they are a distinct population from page load vitals and are off by
54+
* default.
55+
*
56+
* Requires span streaming (`traceLifecycle: 'stream'`, the default) and
57+
* `browserTracingIntegration`, which supplies the navigation span these attach to.
58+
*
59+
* Default: `false`
60+
*/
61+
bfcache?: boolean;
4562
}
4663

4764
/**
@@ -52,7 +69,7 @@ export interface WebVitalsOptions {
5269
* needed to customize options or to use it without `browserTracingIntegration`.
5370
*/
5471
export const webVitalsIntegration = defineIntegration((options: WebVitalsOptions = {}) => {
55-
const { ignore = [], softNavigations = true } = options;
72+
const { ignore = [], softNavigations = true, bfcache = false } = options;
5673
const ignored = new Set(ignore);
5774

5875
return {
@@ -63,14 +80,23 @@ export const webVitalsIntegration = defineIntegration((options: WebVitalsOptions
6380
// Soft navigation vitals are finalized at the next soft navigation or on pagehide, long after
6481
// the navigation span they belong to has ended. Only span streaming can still send them.
6582
const reportSoftNavs = softNavigations && spanStreamingEnabled && supportsSoftNavigations();
83+
const reportBfcache = bfcache && spanStreamingEnabled;
84+
85+
// Both attribute a vital to the page view it was measured on rather than to the page load, so
86+
// either one puts the trackers on the per-navigation path.
87+
const perNavigation = reportSoftNavs || reportBfcache;
6688

89+
// These have to run before any web vital observer is instrumented, since web-vitals only
90+
// reads its options when the observer is set up.
6791
if (reportSoftNavs) {
68-
// Has to run before any web vital observer is instrumented, since web-vitals only reads its
69-
// options when the observer is set up.
7092
enableSoftNavigationReporting();
7193
startSoftNavigationCorrelation(client);
7294
}
7395

96+
if (reportBfcache) {
97+
enableBfcacheReporting();
98+
}
99+
74100
// With span streaming enabled, CLS and LCP are tracked as standalone v2 spans (like INP).
75101
// Otherwise, they're recorded as measurements on the pageload span.
76102
const trackClsOnPageloadSpan = !spanStreamingEnabled && !ignored.has('cls');
@@ -103,17 +129,17 @@ export const webVitalsIntegration = defineIntegration((options: WebVitalsOptions
103129

104130
if (spanStreamingEnabled) {
105131
if (!ignored.has('lcp')) {
106-
trackLcpAsSpan(client, reportSoftNavs);
132+
trackLcpAsSpan(client, perNavigation);
107133
}
108134
if (!ignored.has('cls')) {
109-
trackClsAsSpan(client, reportSoftNavs);
135+
trackClsAsSpan(client, perNavigation);
110136
}
111137
}
112138

113139
// INP is always sent as a streamed web vital span. When span streaming is disabled, INP still
114140
// streams (it overrides the static trace lifecycle for INP only), see `trackInpAsSpan`.
115141
if (!ignored.has('inp')) {
116-
trackInpAsSpan(client, reportSoftNavs);
142+
trackInpAsSpan(client, perNavigation);
117143
}
118144
},
119145
afterAllSetup() {

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ const mockTrackClsAsSpan = vi.hoisted(() => vi.fn());
88
const mockTrackInpAsSpan = vi.hoisted(() => vi.fn());
99
const mockTrackLcpAsSpan = vi.hoisted(() => vi.fn());
1010
const mockEnableSoftNavigationReporting = vi.hoisted(() => vi.fn());
11+
const mockEnableBfcacheReporting = vi.hoisted(() => vi.fn());
1112
const mockStartSoftNavigationCorrelation = vi.hoisted(() => vi.fn());
1213
const mockSupportsSoftNavigations = vi.hoisted(() => vi.fn());
1314

1415
vi.mock('@sentry/browser-utils', () => ({
1516
addWebVitalsToSpan: mockAddWebVitalsToSpan,
17+
enableBfcacheReporting: mockEnableBfcacheReporting,
1618
enableSoftNavigationReporting: mockEnableSoftNavigationReporting,
1719
registerInpInteractionListener: mockRegisterInpInteractionListener,
1820
startSoftNavigationCorrelation: mockStartSoftNavigationCorrelation,
@@ -146,6 +148,47 @@ describe('webVitalsIntegration', () => {
146148
expect(mockTrackInpAsSpan).toHaveBeenCalledWith(client, false);
147149
});
148150

151+
it('does not report bfcache web vitals by default', () => {
152+
const client = getMockClient({ traceLifecycle: 'stream' });
153+
const integration = webVitalsIntegration();
154+
155+
integration.setup?.(client as never);
156+
157+
expect(mockEnableBfcacheReporting).not.toHaveBeenCalled();
158+
});
159+
160+
it('reports bfcache web vitals when opted in', () => {
161+
const client = getMockClient({ traceLifecycle: 'stream' });
162+
const integration = webVitalsIntegration({ bfcache: true });
163+
164+
integration.setup?.(client as never);
165+
166+
expect(mockEnableBfcacheReporting).toHaveBeenCalledTimes(1);
167+
});
168+
169+
it('puts the trackers on the per-navigation path for bfcache alone', () => {
170+
// Soft navigations are unsupported here, so `bfcache` is the only thing that can select it.
171+
mockSupportsSoftNavigations.mockReturnValue(false);
172+
const client = getMockClient({ traceLifecycle: 'stream' });
173+
const integration = webVitalsIntegration({ bfcache: true });
174+
175+
integration.setup?.(client as never);
176+
177+
expect(mockTrackLcpAsSpan).toHaveBeenCalledWith(client, true);
178+
expect(mockTrackClsAsSpan).toHaveBeenCalledWith(client, true);
179+
expect(mockTrackInpAsSpan).toHaveBeenCalledWith(client, true);
180+
});
181+
182+
it('does not report bfcache web vitals without span streaming', () => {
183+
const client = getMockClient();
184+
const integration = webVitalsIntegration({ bfcache: true });
185+
186+
integration.setup?.(client as never);
187+
188+
expect(mockEnableBfcacheReporting).not.toHaveBeenCalled();
189+
expect(mockTrackInpAsSpan).toHaveBeenCalledWith(client, false);
190+
});
191+
149192
it('does not report soft navigation web vitals in unsupporting browsers', () => {
150193
const client = getMockClient({ traceLifecycle: 'stream' });
151194
const integration = webVitalsIntegration();

0 commit comments

Comments
 (0)