From b172bce75f7b549a23a7d3ea1d2a21b1a00395d1 Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Fri, 28 Aug 2026 13:31:10 -0400 Subject: [PATCH] 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. --- packages/browser-utils/src/index.ts | 1 + .../instrumentation/performanceObserver.ts | 42 +++++++++++------- .../browser-utils/src/web-vitals/spans.ts | 31 ++++++++----- .../test/web-vitals/spans.test.ts | 27 ++++++++++++ .../browser/src/integrations/webVitals.ts | 38 +++++++++++++--- .../test/integrations/webVitals.test.ts | 43 +++++++++++++++++++ 6 files changed, 150 insertions(+), 32 deletions(-) diff --git a/packages/browser-utils/src/index.ts b/packages/browser-utils/src/index.ts index 297990dc5400..733129d52f5f 100644 --- a/packages/browser-utils/src/index.ts +++ b/packages/browser-utils/src/index.ts @@ -5,6 +5,7 @@ export { addLcpInstrumentationHandler, addInpInstrumentationHandler, addFcpInstrumentationHandler, + enableBfcacheReporting, enableSoftNavigationReporting, } from './instrumentation/performanceObserver'; diff --git a/packages/browser-utils/src/instrumentation/performanceObserver.ts b/packages/browser-utils/src/instrumentation/performanceObserver.ts index 5e19aa21d840..8c5a1d33d3c7 100644 --- a/packages/browser-utils/src/instrumentation/performanceObserver.ts +++ b/packages/browser-utils/src/instrumentation/performanceObserver.ts @@ -164,6 +164,7 @@ let _previousInp: Metric | undefined; let _previousFcp: Metric | undefined; let _reportSoftNavs = false; +let _reportBfcache = false; /** * Opt the CLS, LCP and INP observers into reporting metrics for soft navigations. @@ -186,6 +187,21 @@ export function enableSoftNavigationReporting(): void { _reportSoftNavs = true; } +/** + * Opt the CLS, LCP and INP observers into reporting metrics for back/forward-cache restores. + * + * web-vitals re-reports each metric after a restore, tagged with a `back-forward-cache` navigation + * type. A restore is a new page view measured against a document that was never reloaded, so the + * values only mean anything if there is a fresh root span for them to belong to. Without one they + * would attach to the span the page had before it was frozen, which is why this is off by default. + * + * Like `enableSoftNavigationReporting`, this only affects observers instrumented after it is + * called. + */ +export function enableBfcacheReporting(): void { + _reportBfcache = true; +} + /** * Add a callback that will be triggered when a CLS metric is available. * Returns a cleanup callback which can be called to remove the instrumentation handler. @@ -292,16 +308,12 @@ function triggerHandlers(type: InstrumentHandlerType, data: unknown): void { } /** - * Wraps a metric callback so that metrics reported after a back/forward-cache restore are ignored. - * - * web-vitals re-reports each metric after a bfcache restore (tagged with a `back-forward-cache` - * navigation type). We intentionally drop those for now: our reporting assumes one set of vitals - * per page load, so surfacing bfcache re-reports would skew the data until we're ready to model - * and communicate them. + * Wraps a metric callback so that metrics reported after a back/forward-cache restore are dropped + * unless `enableBfcacheReporting` was called. See there for why they are off by default. */ -function withoutBfcache(callback: (metric: Metric) => void): (metric: Metric) => void { +function unlessBfcacheDisabled(callback: (metric: Metric) => void): (metric: Metric) => void { return metric => { - if (metric.navigationType === 'back-forward-cache') { + if (!_reportBfcache && metric.navigationType === 'back-forward-cache') { return; } callback(metric); @@ -310,7 +322,7 @@ function withoutBfcache(callback: (metric: Metric) => void): (metric: Metric) => function instrumentCls(): StopListening { return onCLS( - withoutBfcache(metric => { + unlessBfcacheDisabled(metric => { triggerHandlers('cls', { metric, }); @@ -318,13 +330,13 @@ function instrumentCls(): StopListening { }), // We want the callback to be called whenever the CLS value updates. // By default, the callback is only called when the tab goes to the background. - { reportAllChanges: !_reportSoftNavs, reportSoftNavs: _reportSoftNavs }, + { reportAllChanges: !_reportSoftNavs && !_reportBfcache, reportSoftNavs: _reportSoftNavs }, ); } function instrumentLcp(): StopListening { return onLCP( - withoutBfcache(metric => { + unlessBfcacheDisabled(metric => { triggerHandlers('lcp', { metric, }); @@ -332,13 +344,13 @@ function instrumentLcp(): StopListening { }), // We want the callback to be called whenever the LCP value updates. // By default, the callback is only called when the tab goes to the background. - { reportAllChanges: !_reportSoftNavs, reportSoftNavs: _reportSoftNavs }, + { reportAllChanges: !_reportSoftNavs && !_reportBfcache, reportSoftNavs: _reportSoftNavs }, ); } function instrumentTtfb(): StopListening { return onTTFB( - withoutBfcache(metric => { + unlessBfcacheDisabled(metric => { triggerHandlers('ttfb', { metric, }); @@ -349,7 +361,7 @@ function instrumentTtfb(): StopListening { function instrumentFcp(): StopListening { return onFCP( - withoutBfcache(metric => { + unlessBfcacheDisabled(metric => { triggerHandlers('fcp', { metric, }); @@ -360,7 +372,7 @@ function instrumentFcp(): StopListening { function instrumentInp(): StopListening { return onINP( - withoutBfcache(metric => { + unlessBfcacheDisabled(metric => { triggerHandlers('inp', { metric, }); diff --git a/packages/browser-utils/src/web-vitals/spans.ts b/packages/browser-utils/src/web-vitals/spans.ts index cba119f2e4e3..89963c17dd14 100644 --- a/packages/browser-utils/src/web-vitals/spans.ts +++ b/packages/browser-utils/src/web-vitals/spans.ts @@ -46,12 +46,12 @@ type WebVitalMetric = Parameters type InpMetric = Parameters[0]['metric']; /** - * Reports a web vital once per navigation, for browsers reporting soft navigations. + * Reports a web vital once per navigation, rather than once per page load. * - * With `reportSoftNavs`, web-vitals restarts the metric on every soft navigation and force-reports - * the previous one just before it does (and again on pagehide). Since we also drop - * `reportAllChanges` in this mode, every value we're handed is already the final one for its - * navigation, so there is nothing to accumulate: each report is a span. + * web-vitals restarts the metric on every soft navigation and force-reports the previous one just + * before it does (and again on pagehide), and re-reports every metric after a bfcache restore. + * Since `reportAllChanges` is off in this mode, every value we're handed is already the final one + * for its navigation, so there is nothing to accumulate: each report is a span. */ function trackWebVitalPerNavigation( client: Client, @@ -77,6 +77,15 @@ function trackWebVitalPerNavigation( return; } + if (metric.navigationType === 'back-forward-cache') { + // A restore reuses the frozen document, so the pageload span above belongs to the page view + // from before the freeze. The active root span is the navigation span started for the + // restore, which is the page view these values were actually measured on. + const activeSpan = getActiveSpan(); + send(metric, activeSpan ? getRootSpan(activeSpan) : undefined, undefined); + return; + } + send(metric, pageloadSpan, undefined); }); } @@ -84,12 +93,12 @@ function trackWebVitalPerNavigation( /** * Tracks LCP as a streamed span. */ -export function trackLcpAsSpan(client: Client, reportSoftNavs = false): void { +export function trackLcpAsSpan(client: Client, perNavigation = false): void { if (!supportsWebVital('largest-contentful-paint')) { return; } - if (reportSoftNavs) { + if (perNavigation) { trackWebVitalPerNavigation(client, addLcpInstrumentationHandler, (metric, parentSpan, softNavigationId) => { const entry = metric.entries[metric.entries.length - 1] as LargestContentfulPaint | undefined; _sendLcpSpan(metric.value, entry, parentSpan, undefined, softNavigationId, metric.navigationType); @@ -170,12 +179,12 @@ export function _sendLcpSpan( /** * Tracks CLS as a streamed span. */ -export function trackClsAsSpan(client: Client, reportSoftNavs = false): void { +export function trackClsAsSpan(client: Client, perNavigation = false): void { if (!supportsWebVital('layout-shift')) { return; } - if (reportSoftNavs) { + if (perNavigation) { trackWebVitalPerNavigation(client, addClsInstrumentationHandler, (metric, parentSpan, softNavigationId) => { const entry = metric.entries[metric.entries.length - 1] as LayoutShift | undefined; _sendClsSpan(metric.value, entry, parentSpan, undefined, softNavigationId, metric.navigationType); @@ -250,7 +259,7 @@ export function _sendClsSpan( * Requires `registerInpInteractionListener()` to be called separately for cached element names and * root spans per interaction. */ -export function trackInpAsSpan(client: Client, reportSoftNavs = false): void { +export function trackInpAsSpan(client: Client, perNavigation = false): void { const performance = getBrowserPerformanceAPI(); if (!performance || !browserPerformanceTimeOrigin()) { return; @@ -263,7 +272,7 @@ export function trackInpAsSpan(client: Client, reportSoftNavs = false): void { // TODO(standalone): once the static trace lifecycle is dropped, INP always streams; drop this flag. const standalone = !hasSpanStreamingEnabled(client); - if (reportSoftNavs) { + if (perNavigation) { // INP restarts per navigation and reports once that navigation is over, by which point the // navigation span has ended and the interaction cache no longer knows about it. The metric // says which navigation it belongs to, so INP is attributed exactly like LCP and CLS. diff --git a/packages/browser-utils/test/web-vitals/spans.test.ts b/packages/browser-utils/test/web-vitals/spans.test.ts index 7b0613b26ca7..be233df8f190 100644 --- a/packages/browser-utils/test/web-vitals/spans.test.ts +++ b/packages/browser-utils/test/web-vitals/spans.test.ts @@ -817,6 +817,33 @@ describe('soft navigation web vitals', () => { expect(calls[1]![0].parentSpan).toBe(navigationSpan); }); + it('reports a bfcache restore against the restore navigation span, not the frozen pageload', () => { + const bfcacheNavigationSpan = { spanContext: () => ({ spanId: 'bfcache-nav' }) } as any; + vi.mocked(SentryCore.getActiveSpan).mockReturnValue(bfcacheNavigationSpan); + vi.mocked(SentryCore.getRootSpan).mockReturnValue(bfcacheNavigationSpan); + + trackLcpAsSpan(client, true); + + lcpCallback({ + metric: { + value: 40, + navigationId: 9, + navigationType: 'back-forward-cache', + entries: [{ startTime: 40, element: {} }], + }, + }); + + expect(SentryCoreBrowser.startInactiveSpan).toHaveBeenCalledWith( + expect.objectContaining({ + parentSpan: bfcacheNavigationSpan, + attributes: expect.objectContaining({ 'browser.navigation.type': 'bfcache' }), + }), + ); + expect(SentryCoreBrowser.startInactiveSpan).not.toHaveBeenCalledWith( + expect.objectContaining({ parentSpan: pageloadSpan }), + ); + }); + it('drops soft navigation vitals that could not be correlated', () => { vi.spyOn(softNavs, 'getNavigationSpanForMetric').mockReturnValue(undefined); diff --git a/packages/browser/src/integrations/webVitals.ts b/packages/browser/src/integrations/webVitals.ts index 36ffad2e5905..5b8054ddaa1d 100644 --- a/packages/browser/src/integrations/webVitals.ts +++ b/packages/browser/src/integrations/webVitals.ts @@ -2,6 +2,7 @@ import type { IntegrationFn, Span } from '@sentry/core/browser'; import { defineIntegration, hasSpanStreamingEnabled } from '@sentry/core/browser'; import { addWebVitalsToSpan, + enableBfcacheReporting, enableSoftNavigationReporting, registerInpInteractionListener, startSoftNavigationCorrelation, @@ -42,6 +43,22 @@ export interface WebVitalsOptions { * Default: `true` */ softNavigations?: boolean; + + /** + * Report a fresh set of LCP, CLS and INP after the page is restored from the back/forward cache. + * + * A restore is a new page view measured against a document that was never reloaded, so its vitals + * are reported against the navigation span `browserTracingIntegration` starts for the restore, + * and tagged `browser.navigation.type: bfcache`. They measure a near-instant restore rather than + * a document load, so they are a distinct population from page load vitals and are off by + * default. + * + * Requires span streaming (`traceLifecycle: 'stream'`, the default) and + * `browserTracingIntegration`, which supplies the navigation span these attach to. + * + * Default: `false` + */ + bfcache?: boolean; } /** @@ -52,7 +69,7 @@ export interface WebVitalsOptions { * needed to customize options or to use it without `browserTracingIntegration`. */ export const webVitalsIntegration = defineIntegration((options: WebVitalsOptions = {}) => { - const { ignore = [], softNavigations = true } = options; + const { ignore = [], softNavigations = true, bfcache = false } = options; const ignored = new Set(ignore); return { @@ -63,14 +80,23 @@ export const webVitalsIntegration = defineIntegration((options: WebVitalsOptions // Soft navigation vitals are finalized at the next soft navigation or on pagehide, long after // the navigation span they belong to has ended. Only span streaming can still send them. const reportSoftNavs = softNavigations && spanStreamingEnabled && supportsSoftNavigations(); + const reportBfcache = bfcache && spanStreamingEnabled; + + // Both attribute a vital to the page view it was measured on rather than to the page load, so + // either one puts the trackers on the per-navigation path. + const perNavigation = reportSoftNavs || reportBfcache; + // These have to run before any web vital observer is instrumented, since web-vitals only + // reads its options when the observer is set up. if (reportSoftNavs) { - // Has to run before any web vital observer is instrumented, since web-vitals only reads its - // options when the observer is set up. enableSoftNavigationReporting(); startSoftNavigationCorrelation(client); } + if (reportBfcache) { + enableBfcacheReporting(); + } + // With span streaming enabled, CLS and LCP are tracked as standalone v2 spans (like INP). // Otherwise, they're recorded as measurements on the pageload span. const trackClsOnPageloadSpan = !spanStreamingEnabled && !ignored.has('cls'); @@ -103,17 +129,17 @@ export const webVitalsIntegration = defineIntegration((options: WebVitalsOptions if (spanStreamingEnabled) { if (!ignored.has('lcp')) { - trackLcpAsSpan(client, reportSoftNavs); + trackLcpAsSpan(client, perNavigation); } if (!ignored.has('cls')) { - trackClsAsSpan(client, reportSoftNavs); + trackClsAsSpan(client, perNavigation); } } // INP is always sent as a streamed web vital span. When span streaming is disabled, INP still // streams (it overrides the static trace lifecycle for INP only), see `trackInpAsSpan`. if (!ignored.has('inp')) { - trackInpAsSpan(client, reportSoftNavs); + trackInpAsSpan(client, perNavigation); } }, afterAllSetup() { diff --git a/packages/browser/test/integrations/webVitals.test.ts b/packages/browser/test/integrations/webVitals.test.ts index eff0b68dde79..6c3f9b7445c9 100644 --- a/packages/browser/test/integrations/webVitals.test.ts +++ b/packages/browser/test/integrations/webVitals.test.ts @@ -8,11 +8,13 @@ const mockTrackClsAsSpan = vi.hoisted(() => vi.fn()); const mockTrackInpAsSpan = vi.hoisted(() => vi.fn()); const mockTrackLcpAsSpan = vi.hoisted(() => vi.fn()); const mockEnableSoftNavigationReporting = vi.hoisted(() => vi.fn()); +const mockEnableBfcacheReporting = vi.hoisted(() => vi.fn()); const mockStartSoftNavigationCorrelation = vi.hoisted(() => vi.fn()); const mockSupportsSoftNavigations = vi.hoisted(() => vi.fn()); vi.mock('@sentry/browser-utils', () => ({ addWebVitalsToSpan: mockAddWebVitalsToSpan, + enableBfcacheReporting: mockEnableBfcacheReporting, enableSoftNavigationReporting: mockEnableSoftNavigationReporting, registerInpInteractionListener: mockRegisterInpInteractionListener, startSoftNavigationCorrelation: mockStartSoftNavigationCorrelation, @@ -146,6 +148,47 @@ describe('webVitalsIntegration', () => { expect(mockTrackInpAsSpan).toHaveBeenCalledWith(client, false); }); + it('does not report bfcache web vitals by default', () => { + const client = getMockClient({ traceLifecycle: 'stream' }); + const integration = webVitalsIntegration(); + + integration.setup?.(client as never); + + expect(mockEnableBfcacheReporting).not.toHaveBeenCalled(); + }); + + it('reports bfcache web vitals when opted in', () => { + const client = getMockClient({ traceLifecycle: 'stream' }); + const integration = webVitalsIntegration({ bfcache: true }); + + integration.setup?.(client as never); + + expect(mockEnableBfcacheReporting).toHaveBeenCalledTimes(1); + }); + + it('puts the trackers on the per-navigation path for bfcache alone', () => { + // Soft navigations are unsupported here, so `bfcache` is the only thing that can select it. + mockSupportsSoftNavigations.mockReturnValue(false); + const client = getMockClient({ traceLifecycle: 'stream' }); + const integration = webVitalsIntegration({ bfcache: true }); + + integration.setup?.(client as never); + + expect(mockTrackLcpAsSpan).toHaveBeenCalledWith(client, true); + expect(mockTrackClsAsSpan).toHaveBeenCalledWith(client, true); + expect(mockTrackInpAsSpan).toHaveBeenCalledWith(client, true); + }); + + it('does not report bfcache web vitals without span streaming', () => { + const client = getMockClient(); + const integration = webVitalsIntegration({ bfcache: true }); + + integration.setup?.(client as never); + + expect(mockEnableBfcacheReporting).not.toHaveBeenCalled(); + expect(mockTrackInpAsSpan).toHaveBeenCalledWith(client, false); + }); + it('does not report soft navigation web vitals in unsupporting browsers', () => { const client = getMockClient({ traceLifecycle: 'stream' }); const integration = webVitalsIntegration();