Skip to content

feat(browser): Report web vitals for bfcache restores #23750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
logaretm wants to merge 1 commit into from
Open

feat(browser): Report web vitals for bfcache restores #23750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/browser-utils/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export {
addLcpInstrumentationHandler,
addInpInstrumentationHandler,
addFcpInstrumentationHandler,
enableBfcacheReporting,
enableSoftNavigationReporting,
} from './instrumentation/performanceObserver';

Expand Down
42 changes: 27 additions & 15 deletions packages/browser-utils/src/instrumentation/performanceObserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ let _previousFcp: Metric | undefined;
const stopListeners: Partial<Record<InstrumentHandlerType, StopListening>> = {};

let _reportSoftNavs = false;
let _reportBfcache = false;

/**
* Opt the CLS, LCP and INP observers into reporting metrics for soft navigations.
Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand All @@ -312,35 +324,35 @@ function withoutBfcache(callback: (metric: Metric) => void): (metric: Metric) =>

function instrumentCls(): StopListening {
return onCLS(
withoutBfcache(metric => {
unlessBfcacheDisabled(metric => {
triggerHandlers('cls', {
metric,
});
_previousCls = metric;
}),
// 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,
});
_previousLcp = metric;
}),
// 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,
});
Expand All @@ -351,7 +363,7 @@ function instrumentTtfb(): StopListening {

function instrumentFcp(): StopListening {
return onFCP(
withoutBfcache(metric => {
unlessBfcacheDisabled(metric => {
triggerHandlers('fcp', {
metric,
});
Expand All @@ -362,7 +374,7 @@ function instrumentFcp(): StopListening {

function instrumentInp(): StopListening {
return onINP(
withoutBfcache(metric => {
unlessBfcacheDisabled(metric => {
triggerHandlers('inp', {
metric,
});
Expand Down
43 changes: 31 additions & 12 deletions packages/browser-utils/src/web-vitals/spans.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
getRootSpan,
hasSpanStreamingEnabled,
SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME,
spanToJSON,
timestampInSeconds,
} from '@sentry/core';
import { DEBUG_BUILD } from '../debug-build';
Expand All @@ -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';
Expand All @@ -46,12 +47,12 @@ type WebVitalMetric = Parameters<Parameters<typeof addLcpInstrumentationHandler>
type InpMetric = Parameters<InstrumentationHandlerCallback>[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<M extends WebVitalMetric>(
client: Client,
Expand All @@ -63,6 +64,16 @@ function trackWebVitalPerNavigation<M extends WebVitalMetric>(
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;
}
});
Comment thread
cursor[bot] marked this conversation as resolved.

addInstrumentationHandler(({ metric }) => {
const navigationSpan = getNavigationSpanForMetric(metric);
if (metric.navigationType === 'soft-navigation') {
Expand All @@ -77,19 +88,27 @@ function trackWebVitalPerNavigation<M extends WebVitalMetric>(
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;
}
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.

send(metric, pageloadSpan, undefined);
});
}

/**
* 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) {
Comment thread
cursor[bot] marked this conversation as resolved.
trackWebVitalPerNavigation(client, addLcpInstrumentationHandler, (metric, parentSpan, softNavigationId) => {
const entry = metric.entries[metric.entries.length - 1] as LargestContentfulPaint | undefined;
_sendLcpSpan(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Expand All @@ -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.
Expand Down
77 changes: 73 additions & 4 deletions packages/browser-utils/test/web-vitals/spans.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -585,7 +590,12 @@ describe('_sendInpSpan', () => {
vi.mocked(htmlTreeAsString).mockReturnValue('<button>');
vi.mocked(SentryCoreBrowser.startInactiveSpan).mockReturnValue(mockSpan as any);
vi.mocked(SentryCore.getActiveSpan).mockReturnValue(undefined);
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);
});
Expand Down Expand Up @@ -704,7 +714,12 @@ describe('trackInpAsSpan', () => {
vi.mocked(SentryCore.getCurrentScope).mockReturnValue(mockScope as any);
vi.mocked(SentryCore.getActiveSpan).mockReturnValue(undefined);
vi.mocked(SentryCoreBrowser.startInactiveSpan).mockReturnValue({ end: vi.fn() } 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(htmlTreeAsString).mockReturnValue('<button>');
Expand Down Expand Up @@ -773,6 +788,7 @@ 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;

let lcpCallback: (arg: { metric: any }) => void;
let clsCallback: (arg: { metric: any }) => void;
Expand All @@ -789,7 +805,12 @@ describe('soft navigation web vitals', () => {
vi.mocked(SentryCore.browserPerformanceTimeOrigin).mockReturnValue(1000);
vi.mocked(SentryCore.getCurrentScope).mockReturnValue(mockScope as any);
vi.mocked(SentryCoreBrowser.startInactiveSpan).mockReturnValue({ end: vi.fn() } 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,
);
vi.mocked(htmlTreeAsString).mockReturnValue('<div>');
vi.spyOn(softNavs, 'getNavigationSpanForMetric').mockImplementation((metric: any) =>
metric.navigationType === 'soft-navigation' ? navigationSpan : undefined,
Expand All @@ -808,6 +829,9 @@ describe('soft navigation web vitals', () => {
if (hook === 'afterStartPageLoadSpan') {
cb(pageloadSpan);
}
if (hook === 'spanStart') {
cb(bfcacheNavigationSpan);
}
}),
};
});
Expand Down Expand Up @@ -886,6 +910,51 @@ describe('soft navigation web vitals', () => {
expect(SentryCoreBrowser.startInactiveSpan).toHaveBeenCalledWith(expect.objectContaining({ startTime: 1 }));
});

it('reports a bfcache restore against the restore navigation span, not the frozen pageload', () => {
trackLcpAsSpan(client, true);

lcpCallback({
metric: {
value: 40,
navigationId: 9,
navigationType: 'back-forward-cache',
entries: [{ startTime: 40, element: {} }],
},
});

expect(SentryCoreBrowser.startInactiveSpan).toHaveBeenCalledWith(
expect.objectContaining({
parentSpan: bfcacheNavigationSpan,
attributes: expect.objectContaining({ 'browser.navigation.type': 'bfcache' }),
}),
);
expect(SentryCoreBrowser.startInactiveSpan).not.toHaveBeenCalledWith(
expect.objectContaining({ parentSpan: pageloadSpan }),
);
});

it('keeps the restore span as the parent once it has ended', () => {
// CLS is only finalized on pagehide, long after the restore navigation span's idle timeout, so
// there is no active span left to read it back from.
vi.mocked(SentryCore.getActiveSpan).mockReturnValue(undefined);

trackClsAsSpan(client, true);

clsCallback({
metric: {
value: 0.05,
navigationId: 9,
navigationType: 'back-forward-cache',
navigationStartTime: 5000,
entries: [],
},
});

expect(SentryCoreBrowser.startInactiveSpan).toHaveBeenCalledWith(
expect.objectContaining({ parentSpan: bfcacheNavigationSpan }),
);
});

it('drops soft navigation vitals that could not be correlated', () => {
vi.spyOn(softNavs, 'getNavigationSpanForMetric').mockReturnValue(undefined);

Expand Down
Loading
Loading