From b56eea4c77cf3ef8905b0bac61b3e37e25c022ca Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 4 Jun 2025 15:20:31 +0200 Subject: [PATCH 1/9] fix(browser): Ignore unrealistically long INP values --- packages/browser-utils/src/metrics/inp.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/browser-utils/src/metrics/inp.ts b/packages/browser-utils/src/metrics/inp.ts index 05b7d7ed17a8..37f36d28bec1 100644 --- a/packages/browser-utils/src/metrics/inp.ts +++ b/packages/browser-utils/src/metrics/inp.ts @@ -22,6 +22,11 @@ import { getBrowserPerformanceAPI, msToSec, startStandaloneWebVitalSpan } from ' const LAST_INTERACTIONS: number[] = []; const INTERACTIONS_SPAN_MAP = new Map(); +/** + * 60 seconds is the maximum for a plausible INP value. + * (source: Me) + */ +const MAX_PLAUSIBLE_INP_VALUE = 60; /** * Start tracking INP webvital events. */ @@ -74,6 +79,11 @@ function _trackINP(): () => void { return; } + const duration = msToSec(metric.value); + if (duration > MAX_PLAUSIBLE_INP_VALUE) { + return; + } + const entry = metric.entries.find(entry => entry.duration === metric.value && INP_ENTRY_MAP[entry.name]); if (!entry) { @@ -85,7 +95,6 @@ function _trackINP(): () => void { /** Build the INP span, create an envelope from the span, and then send the envelope */ const startTime = msToSec((browserPerformanceTimeOrigin() as number) + entry.startTime); - const duration = msToSec(metric.value); const activeSpan = getActiveSpan(); const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; From 015620950e6b69488b75ba88a35da07b6a627358 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 4 Jun 2025 15:54:37 +0200 Subject: [PATCH 2/9] add tests --- packages/browser-utils/src/metrics/inp.ts | 101 +++++++++--------- .../browser-utils/src/metrics/instrument.ts | 10 +- .../test/instrument/metrics/inpt.test.ts | 47 ++++++++ 3 files changed, 107 insertions(+), 51 deletions(-) create mode 100644 packages/browser-utils/test/instrument/metrics/inpt.test.ts diff --git a/packages/browser-utils/src/metrics/inp.ts b/packages/browser-utils/src/metrics/inp.ts index 37f36d28bec1..2a765481e569 100644 --- a/packages/browser-utils/src/metrics/inp.ts +++ b/packages/browser-utils/src/metrics/inp.ts @@ -13,6 +13,7 @@ import { spanToJSON, } from '@sentry/core'; import { + type InstrumentationHandlerCallback, addInpInstrumentationHandler, addPerformanceInstrumentationHandler, isPerformanceEventTiming, @@ -72,66 +73,70 @@ const INP_ENTRY_MAP: Record = { input: 'press', }; -/** Starts tracking the Interaction to Next Paint on the current page. */ -function _trackINP(): () => void { - return addInpInstrumentationHandler(({ metric }) => { - if (metric.value == undefined) { - return; - } +/** Starts tracking the Interaction to Next Paint on the current page. # + * exported only for testing + */ +export function _trackINP(): () => void { + return addInpInstrumentationHandler(_onInp); +} - const duration = msToSec(metric.value); - if (duration > MAX_PLAUSIBLE_INP_VALUE) { - return; - } +export const _onInp = (({ metric }) => { + if (metric.value == undefined) { + return; + } - const entry = metric.entries.find(entry => entry.duration === metric.value && INP_ENTRY_MAP[entry.name]); + const duration = msToSec(metric.value); + if (duration > MAX_PLAUSIBLE_INP_VALUE) { + return; + } - if (!entry) { - return; - } + const entry = metric.entries.find(entry => entry.duration === metric.value && INP_ENTRY_MAP[entry.name]); - const { interactionId } = entry; - const interactionType = INP_ENTRY_MAP[entry.name]; + if (!entry) { + return; + } - /** Build the INP span, create an envelope from the span, and then send the envelope */ - const startTime = msToSec((browserPerformanceTimeOrigin() as number) + entry.startTime); - const activeSpan = getActiveSpan(); - const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; + const { interactionId } = entry; + const interactionType = INP_ENTRY_MAP[entry.name]; - // We first try to lookup the span from our INTERACTIONS_SPAN_MAP, - // where we cache the route per interactionId - const cachedSpan = interactionId != null ? INTERACTIONS_SPAN_MAP.get(interactionId) : undefined; + /** Build the INP span, create an envelope from the span, and then send the envelope */ + const startTime = msToSec((browserPerformanceTimeOrigin() as number) + entry.startTime); + const activeSpan = getActiveSpan(); + const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; - const spanToUse = cachedSpan || rootSpan; + // We first try to lookup the span from our INTERACTIONS_SPAN_MAP, + // where we cache the route per interactionId + const cachedSpan = interactionId != null ? INTERACTIONS_SPAN_MAP.get(interactionId) : undefined; - // Else, we try to use the active span. - // Finally, we fall back to look at the transactionName on the scope - const routeName = spanToUse ? spanToJSON(spanToUse).description : getCurrentScope().getScopeData().transactionName; + const spanToUse = cachedSpan || rootSpan; - const name = htmlTreeAsString(entry.target); - const attributes: SpanAttributes = { - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.inp', - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `ui.interaction.${interactionType}`, - [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry.duration, - }; + // Else, we try to use the active span. + // Finally, we fall back to look at the transactionName on the scope + const routeName = spanToUse ? spanToJSON(spanToUse).description : getCurrentScope().getScopeData().transactionName; - const span = startStandaloneWebVitalSpan({ - name, - transaction: routeName, - attributes, - startTime, - }); - - if (span) { - span.addEvent('inp', { - [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT]: 'millisecond', - [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE]: metric.value, - }); + const name = htmlTreeAsString(entry.target); + const attributes: SpanAttributes = { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.inp', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `ui.interaction.${interactionType}`, + [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry.duration, + }; - span.end(startTime + duration); - } + const span = startStandaloneWebVitalSpan({ + name, + transaction: routeName, + attributes, + startTime, }); -} + + if (span) { + span.addEvent('inp', { + [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT]: 'millisecond', + [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE]: metric.value, + }); + + span.end(startTime + duration); + } +}) satisfies InstrumentationHandlerCallback; /** * Register a listener to cache route information for INP interactions. diff --git a/packages/browser-utils/src/metrics/instrument.ts b/packages/browser-utils/src/metrics/instrument.ts index 7b9d7e562f37..cb84908ce55b 100644 --- a/packages/browser-utils/src/metrics/instrument.ts +++ b/packages/browser-utils/src/metrics/instrument.ts @@ -158,13 +158,17 @@ export function addTtfbInstrumentationHandler(callback: (data: { metric: Metric return addMetricObserver('ttfb', callback, instrumentTtfb, _previousTtfb); } +export type InstrumentationHandlerCallback = (data: { + metric: Omit & { + entries: PerformanceEventTiming[]; + }; +}) => void; + /** * Add a callback that will be triggered when a INP metric is available. * Returns a cleanup callback which can be called to remove the instrumentation handler. */ -export function addInpInstrumentationHandler( - callback: (data: { metric: Omit & { entries: PerformanceEventTiming[] } }) => void, -): CleanupHandlerCallback { +export function addInpInstrumentationHandler(callback: InstrumentationHandlerCallback): CleanupHandlerCallback { return addMetricObserver('inp', callback, instrumentInp, _previousInp); } diff --git a/packages/browser-utils/test/instrument/metrics/inpt.test.ts b/packages/browser-utils/test/instrument/metrics/inpt.test.ts new file mode 100644 index 000000000000..456052fd2454 --- /dev/null +++ b/packages/browser-utils/test/instrument/metrics/inpt.test.ts @@ -0,0 +1,47 @@ +import { afterEach } from 'node:test'; +import { describe, expect, it, vi } from 'vitest'; +import { _onInp, _trackINP } from '../../../src/metrics/inp'; +import * as instrument from '../../../src/metrics/instrument'; +import * as utils from '../../../src/metrics/utils'; + +describe('_trackINP', () => { + const addInpInstrumentationHandler = vi.spyOn(instrument, 'addInpInstrumentationHandler'); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('adds an instrumentation handler', () => { + _trackINP(); + expect(addInpInstrumentationHandler).toHaveBeenCalledOnce(); + }); + + it('returns an unsubscribe dunction', () => { + const handler = _trackINP(); + expect(typeof handler).toBe('function'); + }); +}); + +describe('_onInp', () => { + const startStandaloneWebVitalSpan = vi.spyOn(utils, 'startStandaloneWebVitalSpan'); + + it('early-returns if the INP metric entry has no value', () => { + const metric = { + value: undefined, + entries: [], + }; + // @ts-expect-error - incomplete metric object + _onInp({ metric }); + expect(startStandaloneWebVitalSpan).not.toHaveBeenCalled(); + }); + + it('early-returns if the INP metric value is greater than 60 seconds', () => { + const metric = { + value: 60_001, + entries: [], + }; + // @ts-expect-error - incomplete metric object + _onInp({ metric }); + expect(startStandaloneWebVitalSpan).not.toHaveBeenCalled(); + }); +}); From 1cc4a8ae745d589ea09814f47a2f281bc99f991f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 4 Jun 2025 19:04:34 +0200 Subject: [PATCH 3/9] tests --- .../test/instrument/metrics/inpt.test.ts | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/packages/browser-utils/test/instrument/metrics/inpt.test.ts b/packages/browser-utils/test/instrument/metrics/inpt.test.ts index 456052fd2454..3cdba893965c 100644 --- a/packages/browser-utils/test/instrument/metrics/inpt.test.ts +++ b/packages/browser-utils/test/instrument/metrics/inpt.test.ts @@ -1,4 +1,4 @@ -import { afterEach } from 'node:test'; +import { afterEach, beforeEach } from 'node:test'; import { describe, expect, it, vi } from 'vitest'; import { _onInp, _trackINP } from '../../../src/metrics/inp'; import * as instrument from '../../../src/metrics/instrument'; @@ -25,6 +25,10 @@ describe('_trackINP', () => { describe('_onInp', () => { const startStandaloneWebVitalSpan = vi.spyOn(utils, 'startStandaloneWebVitalSpan'); + beforeEach(() => { + vi.clearAllMocks(); + }); + it('early-returns if the INP metric entry has no value', () => { const metric = { value: undefined, @@ -38,10 +42,39 @@ describe('_onInp', () => { it('early-returns if the INP metric value is greater than 60 seconds', () => { const metric = { value: 60_001, - entries: [], + entries: [{ name: 'click', duration: 60_001, interactionId: 1 }], }; // @ts-expect-error - incomplete metric object _onInp({ metric }); expect(startStandaloneWebVitalSpan).not.toHaveBeenCalled(); }); + + it('early-returns if the inp metric has an unknown interaction type', () => { + const metric = { + value: 10, + entries: [{ name: 'unknown', duration: 10, interactionId: 1 }], + }; + // @ts-expect-error - incomplete metric object + _onInp({ metric }); + expect(startStandaloneWebVitalSpan).not.toHaveBeenCalled(); + }); + + it('starts a span for a valid INP metric entry', () => { + const metric = { + value: 10, + entries: [{ name: 'click', duration: 10, interactionId: 1 }], + }; + // @ts-expect-error - incomplete metric object + _onInp({ metric }); + expect(startStandaloneWebVitalSpan).toHaveBeenCalledWith({ + attributes: { + 'sentry.exclusive_time': 10, + 'sentry.op': 'ui.interaction.click', + 'sentry.origin': 'auto.http.browser.inp', + }, + name: '', + startTime: NaN, + transaction: undefined, + }); + }); }); From 2993326fe0eccb33e1b410cd60722b85b7719dec Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 4 Jun 2025 20:00:30 +0200 Subject: [PATCH 4/9] fix build error --- packages/browser-utils/src/metrics/inp.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/browser-utils/src/metrics/inp.ts b/packages/browser-utils/src/metrics/inp.ts index 2a765481e569..2c74f40c2ddb 100644 --- a/packages/browser-utils/src/metrics/inp.ts +++ b/packages/browser-utils/src/metrics/inp.ts @@ -12,8 +12,8 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, spanToJSON, } from '@sentry/core'; +import type { InstrumentationHandlerCallback } from './instrument'; import { - type InstrumentationHandlerCallback, addInpInstrumentationHandler, addPerformanceInstrumentationHandler, isPerformanceEventTiming, @@ -80,7 +80,10 @@ export function _trackINP(): () => void { return addInpInstrumentationHandler(_onInp); } -export const _onInp = (({ metric }) => { +/** + * exported only for testing + */ +export const _onInp: InstrumentationHandlerCallback = ({ metric }) => { if (metric.value == undefined) { return; } @@ -136,7 +139,7 @@ export const _onInp = (({ metric }) => { span.end(startTime + duration); } -}) satisfies InstrumentationHandlerCallback; +}; /** * Register a listener to cache route information for INP interactions. From d37b462fc55ca05fe22ad7785350975eafa1cd27 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 5 Jun 2025 10:58:26 +0200 Subject: [PATCH 5/9] size limit --- .size-limit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.js b/.size-limit.js index 10efb849a582..c3105a772987 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -206,7 +206,7 @@ module.exports = [ import: createImport('init'), ignore: ['next/router', 'next/constants'], gzip: true, - limit: '42 KB', + limit: '43 KB', }, // SvelteKit SDK (ESM) { From 565f54dd5dae32dc7ad9e5ea5b68e82e25bff760 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 5 Jun 2025 11:07:49 +0200 Subject: [PATCH 6/9] improve tests --- .../test/instrument/metrics/inpt.test.ts | 58 +++++++++++++++---- 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/packages/browser-utils/test/instrument/metrics/inpt.test.ts b/packages/browser-utils/test/instrument/metrics/inpt.test.ts index 3cdba893965c..267b8874ac7f 100644 --- a/packages/browser-utils/test/instrument/metrics/inpt.test.ts +++ b/packages/browser-utils/test/instrument/metrics/inpt.test.ts @@ -23,50 +23,86 @@ describe('_trackINP', () => { }); describe('_onInp', () => { - const startStandaloneWebVitalSpan = vi.spyOn(utils, 'startStandaloneWebVitalSpan'); - - beforeEach(() => { - vi.clearAllMocks(); - }); - it('early-returns if the INP metric entry has no value', () => { + const startStandaloneWebVitalSpanSpy = vi.spyOn(utils, 'startStandaloneWebVitalSpan'); + const metric = { value: undefined, entries: [], }; // @ts-expect-error - incomplete metric object _onInp({ metric }); - expect(startStandaloneWebVitalSpan).not.toHaveBeenCalled(); + + expect(startStandaloneWebVitalSpanSpy).not.toHaveBeenCalled(); }); it('early-returns if the INP metric value is greater than 60 seconds', () => { + const startStandaloneWebVitalSpanSpy = vi.spyOn(utils, 'startStandaloneWebVitalSpan'); + const metric = { value: 60_001, - entries: [{ name: 'click', duration: 60_001, interactionId: 1 }], + entries: [ + { name: 'click', duration: 60_001, interactionId: 1 }, + { name: 'click', duration: 60_000, interactionId: 2 }, + ], }; // @ts-expect-error - incomplete metric object _onInp({ metric }); - expect(startStandaloneWebVitalSpan).not.toHaveBeenCalled(); + + expect(startStandaloneWebVitalSpanSpy).not.toHaveBeenCalled(); }); it('early-returns if the inp metric has an unknown interaction type', () => { + const startStandaloneWebVitalSpanSpy = vi.spyOn(utils, 'startStandaloneWebVitalSpan'); + const metric = { value: 10, entries: [{ name: 'unknown', duration: 10, interactionId: 1 }], }; // @ts-expect-error - incomplete metric object _onInp({ metric }); - expect(startStandaloneWebVitalSpan).not.toHaveBeenCalled(); + + expect(startStandaloneWebVitalSpanSpy).not.toHaveBeenCalled(); }); it('starts a span for a valid INP metric entry', () => { + const startStandaloneWebVitalSpanSpy = vi.spyOn(utils, 'startStandaloneWebVitalSpan'); + const metric = { value: 10, entries: [{ name: 'click', duration: 10, interactionId: 1 }], }; // @ts-expect-error - incomplete metric object _onInp({ metric }); - expect(startStandaloneWebVitalSpan).toHaveBeenCalledWith({ + + expect(startStandaloneWebVitalSpanSpy).toHaveBeenCalledTimes(1); + expect(startStandaloneWebVitalSpanSpy).toHaveBeenCalledWith({ + attributes: { + 'sentry.exclusive_time': 10, + 'sentry.op': 'ui.interaction.click', + 'sentry.origin': 'auto.http.browser.inp', + }, + name: '', + startTime: NaN, + transaction: undefined, + }); + }); + + it('takes the correct entry based on the metric value', () => { + const startStandaloneWebVitalSpanSpy = vi.spyOn(utils, 'startStandaloneWebVitalSpan'); + + const metric = { + value: 10, + entries: [ + { name: 'click', duration: 10, interactionId: 1 }, + { name: 'click', duration: 9, interactionId: 2 }, + ], + }; + // @ts-expect-error - incomplete metric object + _onInp({ metric }); + + expect(startStandaloneWebVitalSpanSpy).toHaveBeenCalledTimes(1); + expect(startStandaloneWebVitalSpanSpy).toHaveBeenCalledWith({ attributes: { 'sentry.exclusive_time': 10, 'sentry.op': 'ui.interaction.click', From 5138f3310753831187781d629269dae22c51d89d Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 5 Jun 2025 11:54:23 +0200 Subject: [PATCH 7/9] lint --- packages/browser-utils/test/instrument/metrics/inpt.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser-utils/test/instrument/metrics/inpt.test.ts b/packages/browser-utils/test/instrument/metrics/inpt.test.ts index 267b8874ac7f..437ae650d0fe 100644 --- a/packages/browser-utils/test/instrument/metrics/inpt.test.ts +++ b/packages/browser-utils/test/instrument/metrics/inpt.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach } from 'node:test'; +import { afterEach } from 'node:test'; import { describe, expect, it, vi } from 'vitest'; import { _onInp, _trackINP } from '../../../src/metrics/inp'; import * as instrument from '../../../src/metrics/instrument'; From 70ee6d5f3fe7252e94c9350d49fd2bfcd8bdbbf3 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 5 Jun 2025 12:12:52 +0200 Subject: [PATCH 8/9] add comment --- packages/browser-utils/src/metrics/inp.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/browser-utils/src/metrics/inp.ts b/packages/browser-utils/src/metrics/inp.ts index 2c74f40c2ddb..e936c08c825d 100644 --- a/packages/browser-utils/src/metrics/inp.ts +++ b/packages/browser-utils/src/metrics/inp.ts @@ -24,10 +24,10 @@ const LAST_INTERACTIONS: number[] = []; const INTERACTIONS_SPAN_MAP = new Map(); /** - * 60 seconds is the maximum for a plausible INP value. + * 60 seconds is the maximum for a plausible INP value * (source: Me) */ -const MAX_PLAUSIBLE_INP_VALUE = 60; +const MAX_PLAUSIBLE_INP_DURATION = 60; /** * Start tracking INP webvital events. */ @@ -89,7 +89,11 @@ export const _onInp: InstrumentationHandlerCallback = ({ metric }) => { } const duration = msToSec(metric.value); - if (duration > MAX_PLAUSIBLE_INP_VALUE) { + + // we received occasional reports of hour-long INP values. + // Therefore, we add a sanity check to avoid creating spans for + // unrealistically long INP durations. + if (duration > MAX_PLAUSIBLE_INP_DURATION) { return; } From b888d024b77daf28bcd02ac3148d43d9c320e1f4 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 5 Jun 2025 12:13:24 +0200 Subject: [PATCH 9/9] . --- packages/browser-utils/src/metrics/inp.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser-utils/src/metrics/inp.ts b/packages/browser-utils/src/metrics/inp.ts index e936c08c825d..30a628b5997f 100644 --- a/packages/browser-utils/src/metrics/inp.ts +++ b/packages/browser-utils/src/metrics/inp.ts @@ -90,7 +90,7 @@ export const _onInp: InstrumentationHandlerCallback = ({ metric }) => { const duration = msToSec(metric.value); - // we received occasional reports of hour-long INP values. + // We received occasional reports of hour-long INP values. // Therefore, we add a sanity check to avoid creating spans for // unrealistically long INP durations. if (duration > MAX_PLAUSIBLE_INP_DURATION) {