Skip to content

Commit 268dcd7

Browse files
committed
feat(browser): Start a navigation span when the page is restored from bfcache
Prototype. 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. Two independent guards in the existing path suppress it, neither written with bfcache in mind, so there is no small nudge that gets a span out of it. Without one, everything after the restore joins the trace the page had before it was frozen, separated by however long it sat in the cache. That misattributes errors, breadcrumbs, clicks and fetches, not just the web vitals that prompted this. The span is started from a `pageshow` listener in `browserTracingIntegration` rather than `bfcacheIntegration`, so it does not depend on an opt-in integration that is about hit/miss diagnostics. It is gated on `instrumentNavigation` and on by default. It carries `browser.navigation.type: bfcache`. A restore is near-instant, so without a way to filter these out they would drag navigation duration percentiles down exactly the way bfcache vitals would have dragged LCP. The span deliberately starts at the `pageshow` event rather than from `PerformanceNavigationTiming`, which is not replaced on restore and still describes the original document load. Known gap, pinned by a test: `bfcacheIntegration` registers its own `pageshow` listener from `setupOnce`, which core always runs before every `afterAllSetup`, so its hit/miss metric is emitted before this span exists and still lands on the pre-freeze trace.
1 parent ad4d1c5 commit 268dcd7

4 files changed

Lines changed: 136 additions & 1 deletion

File tree

packages/browser-utils/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ export { userTimingIntegration } from './performance/userTiming';
3232

3333
export { extractNetworkProtocol } from './performance/utils';
3434

35+
export { BROWSER_NAVIGATION_TYPE_ATTRIBUTE } from './web-vitals/emitSpan';
36+
3537
export { trackClsAsSpan, trackInpAsSpan, trackLcpAsSpan } from './web-vitals/spans';
3638

3739
export { whenIdleOrHidden } from './web-vitals/utils';

packages/browser-utils/src/web-vitals/emitSpan.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import { SOFT_NAVIGATION_ID_ATTRIBUTE } from './softNavs';
2222

2323
// TODO(conventions): replace with `BROWSER_NAVIGATION_TYPE` from `@sentry/conventions/attributes`
2424
// once https://github.com/getsentry/sentry-conventions/pull/600 is released.
25-
const BROWSER_NAVIGATION_TYPE_ATTRIBUTE = 'browser.navigation.type';
25+
export const BROWSER_NAVIGATION_TYPE_ATTRIBUTE = 'browser.navigation.type';
2626

2727
// web-vitals reports a wider set of navigation types than the attribute defines. Only the states
2828
// Navigation Timing cannot express keep their own value; every ordinary document navigation folds

packages/browser/src/tracing/browserTracingIntegration.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
import { _INTERNAL_ensureBrowserSpanStreaming, startIdleSpan, startInactiveSpan } from '@sentry/core/browser';
2929
import {
3030
addHistoryInstrumentationHandler,
31+
BROWSER_NAVIGATION_TYPE_ATTRIBUTE,
3132
addPerformanceEntries,
3233
enableSoftNavigationReporting,
3334
getLocationHref,
@@ -688,6 +689,41 @@ export const browserTracingIntegration = ((options: Partial<BrowserTracingOption
688689
{ url: to, isRedirect: navigationIsRedirect },
689690
);
690691
});
692+
693+
// A bfcache restore resurrects the frozen document, so there is no document load and no
694+
// usable history event: `popstate` either doesn't fire or is swallowed because the URL is
695+
// unchanged from when the page was frozen. Without a span of its own, everything after the
696+
// restore joins the trace the page had before it was frozen, separated by however long it
697+
// sat in the cache.
698+
WINDOW.addEventListener?.('pageshow', (event: PageTransitionEvent) => {
699+
if (!event.persisted) {
700+
return;
701+
}
702+
703+
// A navigation has happened, so the pageload guard in the history handler above must not
704+
// suppress the next one.
705+
startingUrl = undefined;
706+
707+
startBrowserTracingNavigationSpan(
708+
client,
709+
{
710+
// Deliberately no `startTime`: the span starts now, at the restore. The
711+
// `PerformanceNavigationTiming` entry still describes the original document load and
712+
// would date the span to before the page was frozen.
713+
name: hasSpanStreamingEnabled(client)
714+
? NAVIGATION_SPAN_NAME_FALLBACK
715+
: WINDOW.location?.pathname || '/',
716+
attributes: {
717+
[SENTRY_SEGMENT_NAME_SOURCE]: 'url',
718+
[SENTRY_ORIGIN]: 'auto.navigation.browser.bfcache',
719+
// A bfcache restore is near-instant, so these spans would otherwise drag
720+
// navigation duration percentiles down with no way to tell them apart.
721+
[BROWSER_NAVIGATION_TYPE_ATTRIBUTE]: 'bfcache',
722+
},
723+
},
724+
{ url: WINDOW.location?.href },
725+
);
726+
});
691727
}
692728
}
693729

packages/browser/test/tracing/browserTracingIntegration.test.ts

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
getCurrentScope,
99
getDynamicSamplingContextFromSpan,
1010
getMainCarrier,
11+
metrics,
1112
SEMANTIC_ATTRIBUTE_SENTRY_OP,
1213
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
1314
SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
@@ -32,6 +33,7 @@ import {
3233
} from '../../src/tracing/browserTracingIntegration';
3334
import { PREVIOUS_TRACE_TMP_SPAN_ATTRIBUTE } from '../../src/tracing/linkedTraces';
3435
import * as browserUtils from '@sentry/browser-utils';
36+
import { bfcacheMetricsIntegration } from '../../src/integrations/bfcacheMetrics';
3537
import * as webVitalsModule from '../../src/integrations/webVitals';
3638
import { getDefaultBrowserClientOptions } from '../helper/browser-client-options';
3739
import { SENTRY_SEGMENT_NAME_SOURCE, URL_FULL, URL_PATH } from '@sentry/conventions/attributes';
@@ -923,6 +925,101 @@ describe('browserTracingIntegration', () => {
923925
});
924926
});
925927

928+
describe('bfcache restores', () => {
929+
function firePageShow(persisted: boolean): void {
930+
const event = new Event('pageshow') as PageTransitionEvent;
931+
Object.defineProperty(event, 'persisted', { value: persisted });
932+
WINDOW.dispatchEvent(event);
933+
}
934+
935+
function initClient(options = {}): BrowserClient {
936+
const client = new BrowserClient(
937+
getDefaultBrowserClientOptions({
938+
tracesSampleRate: 1,
939+
integrations: [browserTracingIntegration({ instrumentPageLoad: false, ...options })],
940+
}),
941+
);
942+
setCurrentClient(client);
943+
client.init();
944+
return client;
945+
}
946+
947+
it('starts a navigation span when the page is restored from the bfcache', () => {
948+
initClient();
949+
950+
firePageShow(true);
951+
952+
const span = getActiveSpan()!;
953+
expect(span).toBeDefined();
954+
expect(spanToJSON(span).attributes).toEqual(
955+
expect.objectContaining({
956+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
957+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser.bfcache',
958+
'browser.navigation.type': 'bfcache',
959+
}),
960+
);
961+
});
962+
963+
it('ignores a pageshow that is not a bfcache restore', () => {
964+
initClient();
965+
966+
firePageShow(false);
967+
968+
expect(getActiveSpan()).toBeUndefined();
969+
});
970+
971+
it('starts a new trace, rather than continuing the one from before the freeze', () => {
972+
initClient();
973+
974+
firePageShow(true);
975+
const firstTraceId = spanToJSON(getActiveSpan()!).trace_id;
976+
977+
vi.advanceTimersByTime(1600);
978+
firePageShow(true);
979+
const secondTraceId = spanToJSON(getActiveSpan()!).trace_id;
980+
981+
expect(firstTraceId).toBeDefined();
982+
expect(secondTraceId).not.toBe(firstTraceId);
983+
});
984+
985+
it('does not start a span when navigation instrumentation is off', () => {
986+
initClient({ instrumentNavigation: false });
987+
988+
firePageShow(true);
989+
990+
expect(getActiveSpan()).toBeUndefined();
991+
});
992+
993+
// Pins a known ordering problem rather than endorsing it. `bfcacheMetricsIntegration` registers its
994+
// `pageshow` listener from `setupOnce`, which core always runs before every `afterAllSetup`,
995+
// so its hit/miss metric is emitted before this navigation span exists and lands on the trace
996+
// the page had before it was frozen. See the note on the pageshow handler.
997+
it('emits the bfcache metric on the pre-freeze trace, before the navigation span exists', () => {
998+
const countSpy = vi.spyOn(metrics, 'count').mockImplementation(() => {});
999+
const client = new BrowserClient(
1000+
getDefaultBrowserClientOptions({
1001+
tracesSampleRate: 1,
1002+
integrations: [browserTracingIntegration({ instrumentPageLoad: false }), bfcacheMetricsIntegration()],
1003+
}),
1004+
);
1005+
setCurrentClient(client);
1006+
client.init();
1007+
1008+
const traceIdBeforeRestore = getCurrentScope().getPropagationContext().traceId;
1009+
1010+
let traceIdAtMetricTime: string | undefined;
1011+
countSpy.mockImplementation(() => {
1012+
traceIdAtMetricTime = getCurrentScope().getPropagationContext().traceId;
1013+
});
1014+
1015+
firePageShow(true);
1016+
1017+
const navigationTraceId = spanToJSON(getActiveSpan()!).trace_id;
1018+
expect(traceIdAtMetricTime).toBe(traceIdBeforeRestore);
1019+
expect(traceIdAtMetricTime).not.toBe(navigationTraceId);
1020+
});
1021+
});
1022+
9261023
describe('startBrowserTracingNavigationSpan', () => {
9271024
it('works without integration setup', () => {
9281025
const client = new BrowserClient(

0 commit comments

Comments
 (0)