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 e53f75c4a45a..e34c7cc02933 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 @@ -1,8 +1,16 @@ import * as Sentry from '@sentry/browser'; +// Tracing is opt-in per test via `?tracing=1`. The metric tests were written against a client with no +// pageload/navigation spans, and turning tracing on for all of them would put a pageload span's +// transaction name on the scope, which is the value those tests assert the metric falls back to. +const tracing = new URLSearchParams(window.location.search).get('tracing') === '1'; + Sentry.init({ dsn: process.env.E2E_TEST_DSN, - integrations: [Sentry.bfcacheMetricsIntegration()], + integrations: tracing + ? [Sentry.bfcacheMetricsIntegration(), Sentry.browserTracingIntegration()] + : [Sentry.bfcacheMetricsIntegration()], + ...(tracing && { tracesSampleRate: 1 }), release: 'e2e-test', environment: 'qa', tunnel: 'http://localhost:3031', 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 83ef9e32e87e..52d63ba21d1f 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,6 +1,7 @@ +import type { Page } from '@playwright/test'; import { expect, test } from '@playwright/test'; import type { SerializedMetric } from '@sentry/core'; -import { waitForMetric } from '@sentry-internal/test-utils'; +import { getSpanOp, waitForMetric, waitForStreamedSpan } from '@sentry-internal/test-utils'; const PROXY_SERVER_NAME = 'browser-bfcache'; const BFCACHE_ORIGIN = 'auto.browser.bfcache'; @@ -329,3 +330,69 @@ test('does not treat an ordinary forward navigation as a restore', async ({ page ); expect(restored).toBe(false); }); + +// The navigation span for a restore lives in `browserTracingIntegration`, not in the metrics +// integration above, so these run against `?tracing=1` (see `src/main.ts`). +test.describe('the navigation span for a restore', () => { + async function restoreFromBfcache(page: Page): Promise { + await page.click('#to-page-2'); + await page.waitForFunction(() => document.title === 'BFCache E2E - Page 2'); + await page.waitForTimeout(500); + + // Renderer-initiated, because Playwright's CDP `goBack` bypasses bfcache. + await page.evaluate(() => history.back()); + await page.waitForFunction( + () => (window as unknown as { __bfcacheRestored?: boolean }).__bfcacheRestored === true, + { + timeout: 5000, + }, + ); + } + + test('is a navigation segment marked as a bfcache restore', async ({ page }) => { + const restorePromise = waitForStreamedSpan( + PROXY_SERVER_NAME, + span => span.is_segment && getSpanOp(span) === 'navigation', + ); + + await page.goto('/?tracing=1'); + await page.waitForFunction(() => document.title === 'BFCache E2E - Page 1'); + + await restoreFromBfcache(page); + + expect(await restorePromise).toMatchObject({ + is_segment: true, + attributes: { + 'sentry.op': { type: 'string', value: 'navigation' }, + 'sentry.origin': { type: 'string', value: 'auto.navigation.browser.bfcache' }, + 'browser.navigation.type': { type: 'string', value: 'bfcache' }, + }, + }); + }); + + // `PerformanceNavigationTiming` is not replaced on a restore and still describes the original + // document load, so a span dated from it would start before the page was frozen. The same goes for + // the performance entries folded in when the span ends: they all predate the restore, and are only + // kept from dragging the start timestamp back by a guard keyed on the `navigation` op. + test('starts at the restore, not at the original document load', async ({ page }) => { + const pageloadPromise = waitForStreamedSpan( + PROXY_SERVER_NAME, + span => span.is_segment && getSpanOp(span) === 'pageload', + ); + const restorePromise = waitForStreamedSpan( + PROXY_SERVER_NAME, + span => span.is_segment && getSpanOp(span) === 'navigation', + ); + + await page.goto('/?tracing=1'); + await page.waitForFunction(() => document.title === 'BFCache E2E - Page 1'); + const pageload = await pageloadPromise; + + await restoreFromBfcache(page); + const restore = await restorePromise; + + // Both timestamps come from the page's own clock, so this stays free of host/browser skew. The + // 500ms spent on page 2 is the floor for the gap. + expect(restore.start_timestamp).toBeGreaterThan(pageload.start_timestamp + 0.4); + }); +}); diff --git a/packages/browser-utils/src/index.ts b/packages/browser-utils/src/index.ts index 5fa3ea37ebe5..297990dc5400 100644 --- a/packages/browser-utils/src/index.ts +++ b/packages/browser-utils/src/index.ts @@ -32,6 +32,8 @@ export { userTimingIntegration } from './performance/userTiming'; export { extractNetworkProtocol } from './performance/utils'; +export { BROWSER_NAVIGATION_TYPE_ATTRIBUTE } from './web-vitals/emitSpan'; + export { trackClsAsSpan, trackInpAsSpan, trackLcpAsSpan } from './web-vitals/spans'; export { whenIdleOrHidden } from './web-vitals/utils'; diff --git a/packages/browser-utils/src/web-vitals/emitSpan.ts b/packages/browser-utils/src/web-vitals/emitSpan.ts index d95815da3a76..b930d628829a 100644 --- a/packages/browser-utils/src/web-vitals/emitSpan.ts +++ b/packages/browser-utils/src/web-vitals/emitSpan.ts @@ -22,7 +22,7 @@ import { SOFT_NAVIGATION_ID_ATTRIBUTE } from './softNavs'; // TODO(conventions): replace with `BROWSER_NAVIGATION_TYPE` from `@sentry/conventions/attributes` // once https://github.com/getsentry/sentry-conventions/pull/600 is released. -const BROWSER_NAVIGATION_TYPE_ATTRIBUTE = 'browser.navigation.type'; +export const BROWSER_NAVIGATION_TYPE_ATTRIBUTE = 'browser.navigation.type'; // web-vitals reports a wider set of navigation types than the attribute defines. Only the states // Navigation Timing cannot express keep their own value; every ordinary document navigation folds diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 451c67dabae1..e6c89ba5c11e 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -28,6 +28,7 @@ import { import { _INTERNAL_ensureBrowserSpanStreaming, startIdleSpan, startInactiveSpan } from '@sentry/core/browser'; import { addHistoryInstrumentationHandler, + BROWSER_NAVIGATION_TYPE_ATTRIBUTE, addPerformanceEntries, getLocationHref, isBotUserAgent, @@ -94,6 +95,19 @@ export interface BrowserTracingOptions { */ instrumentNavigation: boolean; + /** + * If a navigation span should be created when the page is restored from the back/forward cache. + * + * This is deliberately independent of {@link BrowserTracingOptions.instrumentNavigation}: a restore + * is not a history change, and the framework integrations that own their own navigation spans turn + * that option off without ever handling a restore. The point of this span is trace hygiene, keeping + * everything after the restore off the trace the page had before it was frozen, so it is worth + * having even where history instrumentation is not. + * + * Default: true + */ + instrumentBfcacheRestore: boolean; + /** * Flag spans where tabs moved to background with "cancelled". Browser background tab timing is * not suited towards doing precise measurements of operations. By default, we recommend that this option @@ -263,6 +277,7 @@ export interface BrowserTracingOptions { const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = { ...TRACING_DEFAULTS, instrumentNavigation: true, + instrumentBfcacheRestore: true, instrumentPageLoad: true, markBackgroundSpan: true, enableLongTask: true, @@ -320,6 +335,7 @@ export const browserTracingIntegration = ((options: Partial { + if (!event.persisted) { + return; + } + + // A navigation has happened, so the pageload guard in the history handler above must not + // suppress the next one. + startingUrl = undefined; + + startBrowserTracingNavigationSpan( + client, + { + // Deliberately no `startTime`: the span starts now, at the restore. The + // `PerformanceNavigationTiming` entry still describes the original document load and + // would date the span to before the page was frozen. + // + // TODO(routing): resolve the parameterized route via the route provider (#23551) and set + // the source from it. No router event fires on a restore, so in a framework app this is + // the only navigation span still named from a raw pathname. + name: hasSpanStreamingEnabled(client) + ? NAVIGATION_SPAN_NAME_FALLBACK + : WINDOW.location?.pathname || '/', + attributes: { + [SENTRY_SEGMENT_NAME_SOURCE]: 'url', + [SENTRY_ORIGIN]: 'auto.navigation.browser.bfcache', + // A bfcache restore is near-instant, so these spans would otherwise drag + // navigation duration percentiles down with no way to tell them apart. + [BROWSER_NAVIGATION_TYPE_ATTRIBUTE]: 'bfcache', + }, + }, + { url: WINDOW.location?.href }, + ); + }); + } } if (markBackgroundSpan) { diff --git a/packages/browser/test/tracing/bfcacheRestoreNavigation.test.ts b/packages/browser/test/tracing/bfcacheRestoreNavigation.test.ts new file mode 100644 index 000000000000..27cc3089b309 --- /dev/null +++ b/packages/browser/test/tracing/bfcacheRestoreNavigation.test.ts @@ -0,0 +1,81 @@ +/** + * @vitest-environment jsdom + */ + +import { + browserPerformanceTimeOrigin, + getActiveSpan, + getMainCarrier, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + setCurrentClient, + spanToJSON, +} from '@sentry/core'; +import { JSDOM } from 'jsdom'; +import { TextDecoder, TextEncoder } from 'util'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { BrowserClient } from '../../src/client'; +import { WINDOW } from '../../src/helpers'; +import { browserTracingIntegration } from '../../src/tracing/browserTracingIntegration'; +import { getDefaultBrowserClientOptions } from '../helper/browser-client-options'; + +// @ts-expect-error patch the encoder on the window, else importing JSDOM fails +delete global.window.TextEncoder; +// @ts-expect-error patch the encoder on the window, else importing JSDOM fails +delete global.window.TextDecoder; +global.window.TextEncoder = TextEncoder; +// @ts-expect-error patch the encoder on the window, else importing JSDOM fails +global.window.TextDecoder = TextDecoder; + +const dom = new JSDOM(undefined, { url: 'https://example.com/' }); +Object.defineProperty(global, 'document', { value: dom.window.document, writable: true }); +Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true }); +Object.defineProperty(global, 'history', { value: dom.window.history, writable: true }); + +// This lives in its own file on purpose. The history instrumentation tracks the previous URL in +// module state, so `from` is only `undefined` on the very first history event of the module's life. +// Any earlier `pushState` in the same file sets it and makes the case under test unreachable. +describe('bfcache restore, then the first history navigation of the document', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(browserPerformanceTimeOrigin()!); + getMainCarrier().__SENTRY__ = undefined; + vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + getActiveSpan()?.end(); + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + // A page that never touched the History API before it was frozen hits the pageload guard on its + // first navigation after the restore. Clearing `startingUrl` has to actually disable that guard, + // otherwise the guard swallows the navigation and that route stays on the restore span. + it('starts a navigation span rather than leaving the route on the restore span', () => { + const client = new BrowserClient( + getDefaultBrowserClientOptions({ + tracesSampleRate: 1, + integrations: [browserTracingIntegration({ instrumentPageLoad: false })], + }), + ); + setCurrentClient(client); + client.init(); + + const event = new Event('pageshow') as PageTransitionEvent; + Object.defineProperty(event, 'persisted', { value: true }); + WINDOW.dispatchEvent(event); + + expect(spanToJSON(getActiveSpan()!).attributes).toEqual( + expect.objectContaining({ [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser.bfcache' }), + ); + + // Past the redirect threshold, so the navigation is judged on the pageload guard alone rather + // than being folded into the restore span as a redirect. + vi.advanceTimersByTime(2000); + WINDOW.history.pushState({}, '', '/after-restore'); + + expect(spanToJSON(getActiveSpan()!).attributes).toEqual( + expect.objectContaining({ [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser' }), + ); + }); +}); diff --git a/packages/browser/test/tracing/browserTracingIntegration.test.ts b/packages/browser/test/tracing/browserTracingIntegration.test.ts index 85c666421ae8..3c1cb4ffc327 100644 --- a/packages/browser/test/tracing/browserTracingIntegration.test.ts +++ b/packages/browser/test/tracing/browserTracingIntegration.test.ts @@ -8,6 +8,7 @@ import { getCurrentScope, getDynamicSamplingContextFromSpan, getMainCarrier, + metrics, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, @@ -31,6 +32,7 @@ import { startBrowserTracingPageLoadSpan, } from '../../src/tracing/browserTracingIntegration'; import { PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE } from '../../src/tracing/linkedTraces'; +import { bfcacheMetricsIntegration } from '../../src/integrations/bfcacheMetrics'; import * as webVitalsModule from '../../src/integrations/webVitals'; import { getDefaultBrowserClientOptions } from '../helper/browser-client-options'; import { SENTRY_SEGMENT_NAME_SOURCE, URL_FULL, URL_PATH } from '@sentry/conventions/attributes'; @@ -887,6 +889,114 @@ describe('browserTracingIntegration', () => { }); }); + describe('bfcache restores', () => { + function firePageShow(persisted: boolean): void { + const event = new Event('pageshow') as PageTransitionEvent; + Object.defineProperty(event, 'persisted', { value: persisted }); + WINDOW.dispatchEvent(event); + } + + function initClient(options = {}): BrowserClient { + const client = new BrowserClient( + getDefaultBrowserClientOptions({ + tracesSampleRate: 1, + integrations: [browserTracingIntegration({ instrumentPageLoad: false, ...options })], + }), + ); + setCurrentClient(client); + client.init(); + return client; + } + + it('starts a navigation span when the page is restored from the bfcache', () => { + initClient(); + + firePageShow(true); + + const span = getActiveSpan()!; + expect(span).toBeDefined(); + expect(spanToJSON(span).attributes).toEqual( + expect.objectContaining({ + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser.bfcache', + 'browser.navigation.type': 'bfcache', + }), + ); + }); + + it('ignores a pageshow that is not a bfcache restore', () => { + initClient(); + + firePageShow(false); + + expect(getActiveSpan()).toBeUndefined(); + }); + + it('starts a new trace, rather than continuing the one from before the freeze', () => { + initClient(); + + firePageShow(true); + const firstTraceId = spanToJSON(getActiveSpan()!).trace_id; + + vi.advanceTimersByTime(1600); + firePageShow(true); + const secondTraceId = spanToJSON(getActiveSpan()!).trace_id; + + expect(firstTraceId).toBeDefined(); + expect(secondTraceId).not.toBe(firstTraceId); + }); + + // The framework integrations all pass `instrumentNavigation: false` to the base integration so they + // can own history spans, and none of them handle a restore. Gating on it would ship this to plain + // `@sentry/browser` only. + it('starts a span even when history instrumentation is off', () => { + initClient({ instrumentNavigation: false }); + + firePageShow(true); + + expect(spanToJSON(getActiveSpan()!).attributes).toEqual( + expect.objectContaining({ [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser.bfcache' }), + ); + }); + + it('does not start a span when bfcache restore instrumentation is off', () => { + initClient({ instrumentBfcacheRestore: false }); + + firePageShow(true); + + expect(getActiveSpan()).toBeUndefined(); + }); + + // Pins a known ordering problem rather than endorsing it. `bfcacheMetricsIntegration` registers its + // `pageshow` listener from `setupOnce`, which core always runs before every `afterAllSetup`, + // so its hit/miss metric is emitted before this navigation span exists and lands on the trace + // the page had before it was frozen. See the note on the pageshow handler. + it('emits the bfcache metric on the pre-freeze trace, before the navigation span exists', () => { + const countSpy = vi.spyOn(metrics, 'count').mockImplementation(() => {}); + const client = new BrowserClient( + getDefaultBrowserClientOptions({ + tracesSampleRate: 1, + integrations: [browserTracingIntegration({ instrumentPageLoad: false }), bfcacheMetricsIntegration()], + }), + ); + setCurrentClient(client); + client.init(); + + const traceIdBeforeRestore = getCurrentScope().getPropagationContext().traceId; + + let traceIdAtMetricTime: string | undefined; + countSpy.mockImplementation(() => { + traceIdAtMetricTime = getCurrentScope().getPropagationContext().traceId; + }); + + firePageShow(true); + + const navigationTraceId = spanToJSON(getActiveSpan()!).trace_id; + expect(traceIdAtMetricTime).toBe(traceIdBeforeRestore); + expect(traceIdAtMetricTime).not.toBe(navigationTraceId); + }); + }); + describe('startBrowserTracingNavigationSpan', () => { it('works without integration setup', () => { const client = new BrowserClient(