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 5c2b761b742e..7bdefc299c20 100644 --- a/packages/browser-utils/src/instrumentation/performanceObserver.ts +++ b/packages/browser-utils/src/instrumentation/performanceObserver.ts @@ -166,6 +166,7 @@ let _previousFcp: Metric | undefined; const stopListeners: Partial> = {}; let _reportSoftNavs = false; +let _reportBfcache = false; /** * Opt the CLS, LCP and INP observers into reporting metrics for soft navigations. @@ -188,6 +189,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. @@ -294,16 +310,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); @@ -312,7 +324,7 @@ function withoutBfcache(callback: (metric: Metric) => void): (metric: Metric) => function instrumentCls(): StopListening { return onCLS( - withoutBfcache(metric => { + unlessBfcacheDisabled(metric => { triggerHandlers('cls', { metric, }); @@ -320,13 +332,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, }); @@ -334,13 +346,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, }); @@ -351,7 +363,7 @@ function instrumentTtfb(): StopListening { function instrumentFcp(): StopListening { return onFCP( - withoutBfcache(metric => { + unlessBfcacheDisabled(metric => { triggerHandlers('fcp', { metric, }); @@ -362,7 +374,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 4a0f22e95cf5..95ef5a14978d 100644 --- a/packages/browser-utils/src/web-vitals/spans.ts +++ b/packages/browser-utils/src/web-vitals/spans.ts @@ -6,6 +6,7 @@ import { getRootSpan, hasSpanStreamingEnabled, SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME, + spanToJSON, timestampInSeconds, } from '@sentry/core'; import { DEBUG_BUILD } from '../debug-build'; @@ -19,7 +20,7 @@ import { addLcpInstrumentationHandler, } from '../instrumentation/performanceObserver'; import type { LargestContentfulPaint, LayoutShift } from './emitSpan'; -import { _emitWebVitalSpan } from './emitSpan'; +import { BROWSER_NAVIGATION_TYPE_ATTRIBUTE, _emitWebVitalSpan } from './emitSpan'; import { isValidLcpMetric } from './lcp'; import type { WebVitalReportEvent } from './reportEvents'; import { listenForWebVitalReportEvents } from './reportEvents'; @@ -46,12 +47,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, @@ -63,6 +64,16 @@ function trackWebVitalPerNavigation( pageloadSpan = span; }); + // Remembered when the restore happens rather than read back at report time: the restore + // navigation span is an idle span, and CLS and INP are only finalized on pagehide, by which point + // it has long ended and is no longer what is active. + let bfcacheNavigationSpan: Span | undefined; + client.on('spanStart', span => { + if (spanToJSON(span).attributes?.[BROWSER_NAVIGATION_TYPE_ATTRIBUTE] === 'bfcache') { + bfcacheNavigationSpan = span; + } + }); + addInstrumentationHandler(({ metric }) => { const navigationSpan = getNavigationSpanForMetric(metric); if (metric.navigationType === 'soft-navigation') { @@ -77,6 +88,14 @@ 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 navigation span started for the restore is the page view these + // values were actually measured on. + send(metric, bfcacheNavigationSpan, undefined); + return; + } + send(metric, pageloadSpan, undefined); }); } @@ -84,12 +103,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( @@ -184,12 +203,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( @@ -278,7 +297,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; @@ -291,7 +310,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 f48077199cb7..1fd0b5f2e529 100644 --- a/packages/browser-utils/test/web-vitals/spans.test.ts +++ b/packages/browser-utils/test/web-vitals/spans.test.ts @@ -76,7 +76,12 @@ describe('_emitWebVitalSpan', () => { beforeEach(() => { vi.mocked(SentryCore.getCurrentScope).mockReturnValue(mockScope as any); vi.mocked(SentryCoreBrowser.startInactiveSpan).mockReturnValue(mockSpan as any); - vi.mocked(SentryCore.spanToJSON).mockReturnValue({ attributes: {} } as any); + vi.mocked(SentryCore.spanToJSON).mockImplementation( + (span: any) => + (span === bfcacheNavigationSpan + ? { attributes: { 'browser.navigation.type': 'bfcache' } } + : { attributes: {} }) as any, + ); // A root span is its own root, which is what the web vital spans are parented to. vi.mocked(SentryCore.getRootSpan).mockImplementation(span => span); vi.mocked(SentryCore.getClient).mockReturnValue({ getIntegrationByName: () => undefined } as any); @@ -585,7 +590,12 @@ describe('_sendInpSpan', () => { vi.mocked(htmlTreeAsString).mockReturnValue('