feat(browser): Start a navigation span for bfcache restores

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',

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<void> {
+    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);
+  });
+});

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';

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

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<BrowserTracingOption
     ignoreResourceSpans,
     instrumentPageLoad,
     instrumentNavigation,
+    instrumentBfcacheRestore,
     detectRedirects,
     linkPreviousTrace,
     consistentTraceSampling,
@@ -645,7 +661,7 @@ export const browserTracingIntegration = ((options: Partial<BrowserTracingOption
              * only be caused in certain development environments where the usage of a hot module reloader is causing
              * errors.
              */
-            if (from === undefined && startingUrl?.indexOf(to) !== -1) {
+            if (from === undefined && startingUrl !== undefined && startingUrl.indexOf(to) !== -1) {
               startingUrl = undefined;
               return;
             }
@@ -673,6 +689,47 @@ export const browserTracingIntegration = ((options: Partial<BrowserTracingOption
             );
           });
         }
+
+        // A bfcache restore resurrects the frozen document, so there is no document load and no
+        // usable history event: `popstate` either doesn't fire or is swallowed because the URL is
+        // unchanged from when the page was frozen. Without a span of its own, everything after the
+        // restore joins the trace the page had before it was frozen, separated by however long it
+        // sat in the cache.
+        if (instrumentBfcacheRestore) {
+          WINDOW.addEventListener?.('pageshow', (event: PageTransitionEvent) => {
+            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) {

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' }),
+    );
+  });
+});