diff --git a/MIGRATION.md b/MIGRATION.md index fd236135ae68..928463f4c6d1 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -603,6 +603,20 @@ This is required for per-navigation values to be correct: `web-vitals` skips any The visible effect is in Session Replay, which records `web-vital` breadcrumbs from the same instrumentation. Replays now contain one LCP and one CLS entry per navigation instead of one per intermediate update. Where soft navigation reporting is disabled or unsupported, the previous behaviour is unchanged. +### Back/forward-cache restores report their own web vitals + +Affected SDKs: All SDKs running in the browser. + +A page restored from the back/forward cache now reports its own LCP, CLS and INP, against the navigation span `browserTracingIntegration` starts for the restore and tagged `browser.navigation.type: bfcache`. + +A restore is near-instant by construction, so these are a distinct population from page load vitals rather than more samples of the same thing. Read them through that attribute; pooling them with page loads will pull aggregates down. Set `webVitals: { bfcacheNavigations: false }` to leave restores unmeasured. + +```js +Sentry.init({ + integrations: [Sentry.browserTracingIntegration({ webVitals: { bfcacheNavigations: false } })], +}); +``` + ### `DOMException.code` is no longer set as a tag Affected SDKs: All SDKs running in the browser. diff --git a/dev-packages/e2e-tests/test-applications/browser-bfcache/index.html b/dev-packages/e2e-tests/test-applications/browser-bfcache/index.html index 6539e254cdc6..2cd4d47505da 100644 --- a/dev-packages/e2e-tests/test-applications/browser-bfcache/index.html +++ b/dev-packages/e2e-tests/test-applications/browser-bfcache/index.html @@ -9,5 +9,8 @@

BFCache E2E - Page 1

Go to page 2 + + +
diff --git a/dev-packages/e2e-tests/test-applications/browser-bfcache/src/main.ts b/dev-packages/e2e-tests/test-applications/browser-bfcache/src/main.ts index 9b242c3bfb76..8732b00c8029 100644 --- a/dev-packages/e2e-tests/test-applications/browser-bfcache/src/main.ts +++ b/dev-packages/e2e-tests/test-applications/browser-bfcache/src/main.ts @@ -16,6 +16,25 @@ Sentry.init({ tunnel: 'http://localhost:3031', }); +// INP only considers interactions whose duration clears the threshold `web-vitals` observes at +// (40ms), and a synthetic click is far quicker than that, so block long enough to be measured. +document.getElementById('slow-interaction')?.addEventListener('click', () => { + const start = performance.now(); + while (performance.now() - start < 120) { + /* block the main thread */ + } +}); + +// A same-document navigation driven by a click, which is what the browser's soft navigation +// heuristic looks for: an interaction, a URL change, and a paint. +document.getElementById('soft-nav')?.addEventListener('click', () => { + history.pushState({}, '', `/soft-${Date.now()}`); + const paragraph = document.createElement('p'); + paragraph.textContent = `soft navigation ${Math.random()}`; + paragraph.style.height = '120px'; + document.getElementById('soft-nav-content')?.appendChild(paragraph); +}); + (window as unknown as { Sentry: typeof Sentry }).Sentry = Sentry; // Test-only marker: lets the test distinguish a genuine bfcache restore (environment working) from a diff --git a/dev-packages/e2e-tests/test-applications/browser-bfcache/tests/bfcache.test.ts b/dev-packages/e2e-tests/test-applications/browser-bfcache/tests/bfcache.test.ts index 01e8991a398a..4d72591e9249 100644 --- a/dev-packages/e2e-tests/test-applications/browser-bfcache/tests/bfcache.test.ts +++ b/dev-packages/e2e-tests/test-applications/browser-bfcache/tests/bfcache.test.ts @@ -1,7 +1,13 @@ import type { Page } from '@playwright/test'; import { expect, test } from '@playwright/test'; import type { SerializedMetric } from '@sentry/core'; -import { getSpanOp, waitForMetric, waitForStreamedSpan } from '@sentry-internal/test-utils'; +import { + collectStreamedSpans, + getSpanOp, + hidePage, + waitForMetric, + waitForStreamedSpan, +} from '@sentry-internal/test-utils'; const PROXY_SERVER_NAME = 'browser-bfcache'; const BFCACHE_ORIGIN = 'auto.browser.bfcache'; @@ -396,4 +402,81 @@ test.describe('the navigation span for a restore', () => { // 500ms spent on page 2 is the floor for the gap. expect(restore.start_timestamp).toBeGreaterThan(pageload.start_timestamp + 0.4); }); + + // Redirect detection turns a navigation that follows another one closely into a child + // `navigation.redirect` span rather than a root navigation, and a restore starts a navigation + // span like any other. A soft navigation is driven by a click, which is exactly what the redirect + // heuristic treats as proof that a navigation was user-initiated, so the two must not collide. + test('a soft navigation right after a restore is not treated as a redirect', async ({ page }) => { + const spansPromise = collectStreamedSpans(PROXY_SERVER_NAME, spansOfTrace => + spansOfTrace.some( + span => + span.is_segment && + getSpanOp(span) === 'navigation' && + span.attributes?.['sentry.origin']?.value === 'auto.navigation.browser', + ), + ); + + await page.goto('/?tracing=1'); + await page.waitForFunction(() => document.title === 'BFCache E2E - Page 1'); + + await restoreFromBfcache(page); + // No wait: the restore's idle span has to still be open, otherwise redirect detection never + // engages and this would pass without exercising anything. + await page.click('#soft-nav'); + + const spans = await spansPromise; + + const softNavSpan = spans.find( + span => + span.is_segment && + getSpanOp(span) === 'navigation' && + span.attributes?.['sentry.origin']?.value === 'auto.navigation.browser', + )!; + expect(softNavSpan.attributes?.['browser.navigation.type']).toBeUndefined(); + + expect(spans.filter(span => getSpanOp(span) === 'navigation.redirect')).toEqual([]); + }); + + // `webVitals.bfcacheNavigations` is on by default, so the app opts into nothing for this. + test('carries the vitals measured on the restore', async ({ page }) => { + const spansPromise = collectStreamedSpans( + PROXY_SERVER_NAME, + spansOfTrace => + spansOfTrace.some(span => span.is_segment && getSpanOp(span) === 'navigation') && + spansOfTrace.some(span => getSpanOp(span) === 'ui.webvital.lcp') && + spansOfTrace.some(span => getSpanOp(span) === 'ui.webvital.cls') && + spansOfTrace.some(span => getSpanOp(span) === 'ui.interaction.click'), + ); + + await page.goto('/?tracing=1'); + await page.waitForFunction(() => document.title === 'BFCache E2E - Page 1'); + + await restoreFromBfcache(page); + + // Interacting after the restore is what gives it an INP to report. + await page.click('#slow-interaction'); + + // CLS is only finalized on pagehide, unlike LCP which reports as soon as the restore paints. + await hidePage(page); + + const spans = await spansPromise; + const restoreSpan = spans.find(span => span.is_segment && getSpanOp(span) === 'navigation')!; + const lcpSpan = spans.find(span => getSpanOp(span) === 'ui.webvital.lcp')!; + const clsSpan = spans.find(span => getSpanOp(span) === 'ui.webvital.cls')!; + const inpSpan = spans.find(span => getSpanOp(span) === 'ui.interaction.click')!; + + expect(restoreSpan.attributes).toMatchObject({ + 'browser.navigation.type': { type: 'string', value: 'bfcache' }, + }); + + // All three hang off the restore itself. They also carry the `bfcache` navigation type, so the + // one reported first must not become the parent of the ones reported later. + for (const vital of [lcpSpan, clsSpan, inpSpan]) { + expect(vital.parent_span_id).toBe(restoreSpan.span_id); + expect(vital.attributes).toMatchObject({ + 'browser.navigation.type': { type: 'string', value: 'bfcache' }, + }); + } + }); }); diff --git a/packages/browser-utils/src/web-vitals/spans.ts b/packages/browser-utils/src/web-vitals/spans.ts index a3b1387f49b6..fd5d6ebbd6ac 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, + SEMANTIC_ATTRIBUTE_SENTRY_OP, spanToJSON, timestampInSeconds, } from '@sentry/core'; @@ -29,6 +30,7 @@ import { getNavigationSpanForMetric } from './softNavs'; import { getBrowserPerformanceAPI, msToSec, supportsWebVital } from '../performance/utils'; import type { PerformanceEventTiming } from '../instrumentation/performanceObserver'; import { + NAVIGATION, UI_INTERACTION_CLICK, UI_INTERACTION_DRAG, UI_INTERACTION_HOVER, @@ -70,7 +72,14 @@ function trackWebVitalPerNavigation( // 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] === 'bfcache') { + // The op has to be checked too: the web vital spans emitted for a restore carry the same + // `bfcache` navigation type, so matching on that alone lets the first of them replace the + // navigation span, and every later vital then hangs off a sibling vital instead. + const attributes = spanToJSON(span).attributes; + if ( + attributes?.[SEMANTIC_ATTRIBUTE_SENTRY_OP] === NAVIGATION && + attributes[BROWSER_NAVIGATION_TYPE] === 'bfcache' + ) { bfcacheNavigationSpan = span; } }); diff --git a/packages/browser-utils/test/web-vitals/spans.test.ts b/packages/browser-utils/test/web-vitals/spans.test.ts index f7d15147103e..b148faa2758a 100644 --- a/packages/browser-utils/test/web-vitals/spans.test.ts +++ b/packages/browser-utils/test/web-vitals/spans.test.ts @@ -79,7 +79,7 @@ describe('_emitWebVitalSpan', () => { vi.mocked(SentryCore.spanToJSON).mockImplementation( (span: any) => (span === bfcacheNavigationSpan - ? { attributes: { 'browser.navigation.type': 'bfcache' } } + ? { attributes: { 'sentry.op': 'navigation', 'browser.navigation.type': 'bfcache' } } : { attributes: {} }) as any, ); // A root span is its own root, which is what the web vital spans are parented to. @@ -593,7 +593,7 @@ describe('_sendInpSpan', () => { vi.mocked(SentryCore.spanToJSON).mockImplementation( (span: any) => (span === bfcacheNavigationSpan - ? { attributes: { 'browser.navigation.type': 'bfcache' } } + ? { attributes: { 'sentry.op': 'navigation', 'browser.navigation.type': 'bfcache' } } : { attributes: {} }) as any, ); // A root span is its own root, which is what the web vital spans are parented to. @@ -717,7 +717,7 @@ describe('trackInpAsSpan', () => { vi.mocked(SentryCore.spanToJSON).mockImplementation( (span: any) => (span === bfcacheNavigationSpan - ? { attributes: { 'browser.navigation.type': 'bfcache' } } + ? { attributes: { 'sentry.op': 'navigation', 'browser.navigation.type': 'bfcache' } } : { attributes: {} }) as any, ); // A root span is its own root, which is what the web vital spans are parented to. @@ -789,10 +789,12 @@ describe('soft navigation web vitals', () => { const navigationSpan = { spanContext: () => ({ spanId: 'nav-1' }) } as any; const pageloadSpan = createMockPageloadSpan('pageload-1'); const bfcacheNavigationSpan = { spanContext: () => ({ spanId: 'bfcache-nav' }) } as any; + const bfcacheVitalSpan = { spanContext: () => ({ spanId: 'bfcache-vital' }) } as any; let lcpCallback: (arg: { metric: any }) => void; let clsCallback: (arg: { metric: any }) => void; let client: any; + let startSpan: (span: unknown) => void; function lcpMetric(navigationId: number, value: number, navigationType = 'soft-navigation') { return { value, navigationId, navigationType, entries: [{ startTime: value, element: {} }] }; @@ -808,8 +810,10 @@ describe('soft navigation web vitals', () => { vi.mocked(SentryCore.spanToJSON).mockImplementation( (span: any) => (span === bfcacheNavigationSpan - ? { attributes: { 'browser.navigation.type': 'bfcache' } } - : { attributes: {} }) as any, + ? { attributes: { 'sentry.op': 'navigation', 'browser.navigation.type': 'bfcache' } } + : span === bfcacheVitalSpan + ? { attributes: { 'sentry.op': 'ui.webvital.lcp', 'browser.navigation.type': 'bfcache' } } + : { attributes: {} }) as any, ); vi.mocked(htmlTreeAsString).mockReturnValue('
'); vi.spyOn(softNavs, 'getNavigationSpanForMetric').mockImplementation((metric: any) => @@ -830,6 +834,7 @@ describe('soft navigation web vitals', () => { cb(pageloadSpan); } if (hook === 'spanStart') { + startSpan = cb; cb(bfcacheNavigationSpan); } }), @@ -955,6 +960,34 @@ describe('soft navigation web vitals', () => { ); }); + it("does not let a restore's own vital span become the parent of the next one", () => { + // Web vital spans for a restore carry the same `bfcache` navigation type as the navigation span + // they hang off, so the second vital would otherwise be parented to the first. + vi.mocked(SentryCore.getActiveSpan).mockReturnValue(undefined); + + trackLcpAsSpan(client, true); + trackClsAsSpan(client, true); + + lcpCallback({ + metric: { + value: 40, + navigationId: 9, + navigationType: 'back-forward-cache', + entries: [{ startTime: 40, element: {} }], + }, + }); + + startSpan(bfcacheVitalSpan); + + clsCallback({ + metric: { value: 0.05, navigationId: 9, navigationType: 'back-forward-cache', entries: [] }, + }); + + expect(SentryCoreBrowser.startInactiveSpan).toHaveBeenLastCalledWith( + expect.objectContaining({ parentSpan: bfcacheNavigationSpan }), + ); + }); + 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 03356ff86740..7c1b3d14a9fe 100644 --- a/packages/browser/src/integrations/webVitals.ts +++ b/packages/browser/src/integrations/webVitals.ts @@ -45,20 +45,20 @@ export interface WebVitalsOptions { softNavigations?: boolean; /** - * Report a fresh set of LCP, CLS and INP after the page is restored from the back/forward cache. + * Give each back/forward-cache restore its own set of LCP, CLS and INP. * * 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. + * and tagged `browser.navigation.type: bfcache`. A restore is near-instant by construction, so + * these are a distinct population from page load vitals and are meant to be read through that + * attribute rather than pooled with them. Set this to `false` to leave restores unmeasured. * * Requires span streaming (`traceLifecycle: 'stream'`, the default) and * `browserTracingIntegration`, which supplies the navigation span these attach to. * - * Default: `false` + * Default: `true` */ - bfcache?: boolean; + bfcacheNavigations?: boolean; } /** @@ -69,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, bfcache = false } = options; + const { ignore = [], softNavigations = true, bfcacheNavigations = true } = options; const ignored = new Set(ignore); return { @@ -80,7 +80,7 @@ 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; + const reportBfcache = bfcacheNavigations && 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. diff --git a/packages/browser/test/integrations/webVitals.test.ts b/packages/browser/test/integrations/webVitals.test.ts index 6c3f9b7445c9..19a7df2c99ef 100644 --- a/packages/browser/test/integrations/webVitals.test.ts +++ b/packages/browser/test/integrations/webVitals.test.ts @@ -90,8 +90,8 @@ describe('webVitalsIntegration', () => { trackLcp: false, client, }); - expect(mockTrackLcpAsSpan).toHaveBeenCalledWith(client, false); - expect(mockTrackClsAsSpan).toHaveBeenCalledWith(client, false); + expect(mockTrackLcpAsSpan).toHaveBeenCalledWith(client, true); + expect(mockTrackClsAsSpan).toHaveBeenCalledWith(client, true); expect(mockTrackInpAsSpan).toHaveBeenCalledTimes(1); expect(mockRegisterInpInteractionListener).toHaveBeenCalledTimes(1); }); @@ -104,7 +104,7 @@ describe('webVitalsIntegration', () => { integration.afterAllSetup?.(client as never); expect(mockTrackLcpAsSpan).not.toHaveBeenCalled(); - expect(mockTrackClsAsSpan).toHaveBeenCalledWith(client, false); + expect(mockTrackClsAsSpan).toHaveBeenCalledWith(client, true); expect(mockTrackInpAsSpan).toHaveBeenCalledTimes(1); }); @@ -131,9 +131,10 @@ describe('webVitalsIntegration', () => { expect(mockEnableSoftNavigationReporting).not.toHaveBeenCalled(); expect(mockStartSoftNavigationCorrelation).not.toHaveBeenCalled(); - expect(mockTrackLcpAsSpan).toHaveBeenCalledWith(client, false); - expect(mockTrackClsAsSpan).toHaveBeenCalledWith(client, false); - expect(mockTrackInpAsSpan).toHaveBeenCalledWith(client, false); + // Restores still select the per-navigation path, independently of soft navigations. + expect(mockTrackLcpAsSpan).toHaveBeenCalledWith(client, true); + expect(mockTrackClsAsSpan).toHaveBeenCalledWith(client, true); + expect(mockTrackInpAsSpan).toHaveBeenCalledWith(client, true); }); it('does not report soft navigation web vitals without span streaming', () => { @@ -148,29 +149,29 @@ describe('webVitalsIntegration', () => { expect(mockTrackInpAsSpan).toHaveBeenCalledWith(client, false); }); - it('does not report bfcache web vitals by default', () => { + it('reports bfcache web vitals by default', () => { const client = getMockClient({ traceLifecycle: 'stream' }); const integration = webVitalsIntegration(); integration.setup?.(client as never); - expect(mockEnableBfcacheReporting).not.toHaveBeenCalled(); + expect(mockEnableBfcacheReporting).toHaveBeenCalledTimes(1); }); - it('reports bfcache web vitals when opted in', () => { + it('does not report bfcache web vitals when opted out', () => { const client = getMockClient({ traceLifecycle: 'stream' }); - const integration = webVitalsIntegration({ bfcache: true }); + const integration = webVitalsIntegration({ bfcacheNavigations: false }); integration.setup?.(client as never); - expect(mockEnableBfcacheReporting).toHaveBeenCalledTimes(1); + expect(mockEnableBfcacheReporting).not.toHaveBeenCalled(); }); 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. + // Soft navigations are unsupported here, so bfcache restores are the only thing that can select it. mockSupportsSoftNavigations.mockReturnValue(false); const client = getMockClient({ traceLifecycle: 'stream' }); - const integration = webVitalsIntegration({ bfcache: true }); + const integration = webVitalsIntegration(); integration.setup?.(client as never); @@ -181,7 +182,7 @@ describe('webVitalsIntegration', () => { it('does not report bfcache web vitals without span streaming', () => { const client = getMockClient(); - const integration = webVitalsIntegration({ bfcache: true }); + const integration = webVitalsIntegration(); integration.setup?.(client as never); @@ -196,7 +197,8 @@ describe('webVitalsIntegration', () => { integration.setup?.(client as never); expect(mockEnableSoftNavigationReporting).not.toHaveBeenCalled(); - expect(mockTrackLcpAsSpan).toHaveBeenCalledWith(client, false); + // Restores still select the per-navigation path, independently of soft navigations. + expect(mockTrackLcpAsSpan).toHaveBeenCalledWith(client, true); }); it('supports ignoring selected web vitals', () => {