Skip to content

Commit 6d61be0

Browse files
Lms24Lms24
authored
fix(browser): Ignore unrealistically long INP values (#16484)
We shouldn't send values INP spans if the reported value is unrealistically long. I decided to draw the line at 60 seconds for now but if anyone has concerns or wants a different upper bound, happy to change it.
1 parent 0a7b915 commit 6d61be0

File tree

4 files changed

+189
-48
lines changed

4 files changed

+189
-48
lines changed

‎.size-limit.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ module.exports = [
206206
import: createImport('init'),
207207
ignore: ['next/router', 'next/constants'],
208208
gzip: true,
209-
limit: '42 KB',
209+
limit: '43 KB',
210210
},
211211
// SvelteKit SDK (ESM)
212212
{

‎packages/browser-utils/src/metrics/inp.ts

Lines changed: 65 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
1313
spanToJSON,
1414
} from '@sentry/core';
15+
import type { InstrumentationHandlerCallback } from './instrument';
1516
import {
1617
addInpInstrumentationHandler,
1718
addPerformanceInstrumentationHandler,
@@ -22,6 +23,11 @@ import { getBrowserPerformanceAPI, msToSec, startStandaloneWebVitalSpan } from '
2223
const LAST_INTERACTIONS: number[] = [];
2324
const INTERACTIONS_SPAN_MAP = new Map<number, Span>();
2425

26+
/**
27+
* 60 seconds is the maximum for a plausible INP value
28+
* (source: Me)
29+
*/
30+
const MAX_PLAUSIBLE_INP_DURATION = 60;
2531
/**
2632
* Start tracking INP webvital events.
2733
*/
@@ -67,62 +73,77 @@ const INP_ENTRY_MAP: Record<string, 'click' | 'hover' | 'drag' | 'press'> = {
6773
input: 'press',
6874
};
6975

70-
/** Starts tracking the Interaction to Next Paint on the current page. */
71-
function _trackINP(): () => void {
72-
return addInpInstrumentationHandler(({ metric }) => {
73-
if (metric.value == undefined) {
74-
return;
75-
}
76+
/** Starts tracking the Interaction to Next Paint on the current page. #
77+
* exported only for testing
78+
*/
79+
export function _trackINP(): () => void {
80+
return addInpInstrumentationHandler(_onInp);
81+
}
82+
83+
/**
84+
* exported only for testing
85+
*/
86+
export const _onInp: InstrumentationHandlerCallback = ({ metric }) => {
87+
if (metric.value == undefined) {
88+
return;
89+
}
7690

77-
const entry = metric.entries.find(entry => entry.duration === metric.value && INP_ENTRY_MAP[entry.name]);
91+
const duration = msToSec(metric.value);
7892

79-
if (!entry) {
80-
return;
81-
}
93+
// We received occasional reports of hour-long INP values.
94+
// Therefore, we add a sanity check to avoid creating spans for
95+
// unrealistically long INP durations.
96+
if (duration > MAX_PLAUSIBLE_INP_DURATION) {
97+
return;
98+
}
8299

83-
const { interactionId } = entry;
84-
const interactionType = INP_ENTRY_MAP[entry.name];
100+
const entry = metric.entries.find(entry => entry.duration === metric.value && INP_ENTRY_MAP[entry.name]);
85101

86-
/** Build the INP span, create an envelope from the span, and then send the envelope */
87-
const startTime = msToSec((browserPerformanceTimeOrigin() as number) + entry.startTime);
88-
const duration = msToSec(metric.value);
89-
const activeSpan = getActiveSpan();
90-
const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined;
102+
if (!entry) {
103+
return;
104+
}
91105

92-
// We first try to lookup the span from our INTERACTIONS_SPAN_MAP,
93-
// where we cache the route per interactionId
94-
const cachedSpan = interactionId != null ? INTERACTIONS_SPAN_MAP.get(interactionId) : undefined;
106+
const { interactionId } = entry;
107+
const interactionType = INP_ENTRY_MAP[entry.name];
95108

96-
const spanToUse = cachedSpan || rootSpan;
109+
/** Build the INP span, create an envelope from the span, and then send the envelope */
110+
const startTime = msToSec((browserPerformanceTimeOrigin() as number) + entry.startTime);
111+
const activeSpan = getActiveSpan();
112+
const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined;
97113

98-
// Else, we try to use the active span.
99-
// Finally, we fall back to look at the transactionName on the scope
100-
const routeName = spanToUse ? spanToJSON(spanToUse).description : getCurrentScope().getScopeData().transactionName;
114+
// We first try to lookup the span from our INTERACTIONS_SPAN_MAP,
115+
// where we cache the route per interactionId
116+
const cachedSpan = interactionId != null ? INTERACTIONS_SPAN_MAP.get(interactionId) : undefined;
101117

102-
const name = htmlTreeAsString(entry.target);
103-
const attributes: SpanAttributes = {
104-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.inp',
105-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: `ui.interaction.${interactionType}`,
106-
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry.duration,
107-
};
118+
const spanToUse = cachedSpan || rootSpan;
108119

109-
const span = startStandaloneWebVitalSpan({
110-
name,
111-
transaction: routeName,
112-
attributes,
113-
startTime,
114-
});
120+
// Else, we try to use the active span.
121+
// Finally, we fall back to look at the transactionName on the scope
122+
const routeName = spanToUse ? spanToJSON(spanToUse).description : getCurrentScope().getScopeData().transactionName;
115123

116-
if (span) {
117-
span.addEvent('inp', {
118-
[SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT]: 'millisecond',
119-
[SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE]: metric.value,
120-
});
124+
const name = htmlTreeAsString(entry.target);
125+
const attributes: SpanAttributes = {
126+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.inp',
127+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: `ui.interaction.${interactionType}`,
128+
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry.duration,
129+
};
121130

122-
span.end(startTime + duration);
123-
}
131+
const span = startStandaloneWebVitalSpan({
132+
name,
133+
transaction: routeName,
134+
attributes,
135+
startTime,
124136
});
125-
}
137+
138+
if (span) {
139+
span.addEvent('inp', {
140+
[SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT]: 'millisecond',
141+
[SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE]: metric.value,
142+
});
143+
144+
span.end(startTime + duration);
145+
}
146+
};
126147

127148
/**
128149
* Register a listener to cache route information for INP interactions.

‎packages/browser-utils/src/metrics/instrument.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,13 +158,17 @@ export function addTtfbInstrumentationHandler(callback: (data: { metric: Metric
158158
return addMetricObserver('ttfb', callback, instrumentTtfb, _previousTtfb);
159159
}
160160

161+
export type InstrumentationHandlerCallback = (data: {
162+
metric: Omit<Metric, 'entries'> & {
163+
entries: PerformanceEventTiming[];
164+
};
165+
}) => void;
166+
161167
/**
162168
* Add a callback that will be triggered when a INP metric is available.
163169
* Returns a cleanup callback which can be called to remove the instrumentation handler.
164170
*/
165-
export function addInpInstrumentationHandler(
166-
callback: (data: { metric: Omit<Metric, 'entries'> & { entries: PerformanceEventTiming[] } }) => void,
167-
): CleanupHandlerCallback {
171+
export function addInpInstrumentationHandler(callback: InstrumentationHandlerCallback): CleanupHandlerCallback {
168172
return addMetricObserver('inp', callback, instrumentInp, _previousInp);
169173
}
170174

‎packages/browser-utils/test/instrument/metrics/inpt.test.ts

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
import { afterEach } from 'node:test';
2+
import { describe, expect, it, vi } from 'vitest';
3+
import { _onInp, _trackINP } from '../../../src/metrics/inp';
4+
import * as instrument from '../../../src/metrics/instrument';
5+
import * as utils from '../../../src/metrics/utils';
6+
7+
describe('_trackINP', () => {
8+
const addInpInstrumentationHandler = vi.spyOn(instrument, 'addInpInstrumentationHandler');
9+
10+
afterEach(() => {
11+
vi.clearAllMocks();
12+
});
13+
14+
it('adds an instrumentation handler', () => {
15+
_trackINP();
16+
expect(addInpInstrumentationHandler).toHaveBeenCalledOnce();
17+
});
18+
19+
it('returns an unsubscribe dunction', () => {
20+
const handler = _trackINP();
21+
expect(typeof handler).toBe('function');
22+
});
23+
});
24+
25+
describe('_onInp', () => {
26+
it('early-returns if the INP metric entry has no value', () => {
27+
const startStandaloneWebVitalSpanSpy = vi.spyOn(utils, 'startStandaloneWebVitalSpan');
28+
29+
const metric = {
30+
value: undefined,
31+
entries: [],
32+
};
33+
// @ts-expect-error - incomplete metric object
34+
_onInp({ metric });
35+
36+
expect(startStandaloneWebVitalSpanSpy).not.toHaveBeenCalled();
37+
});
38+
39+
it('early-returns if the INP metric value is greater than 60 seconds', () => {
40+
const startStandaloneWebVitalSpanSpy = vi.spyOn(utils, 'startStandaloneWebVitalSpan');
41+
42+
const metric = {
43+
value: 60_001,
44+
entries: [
45+
{ name: 'click', duration: 60_001, interactionId: 1 },
46+
{ name: 'click', duration: 60_000, interactionId: 2 },
47+
],
48+
};
49+
// @ts-expect-error - incomplete metric object
50+
_onInp({ metric });
51+
52+
expect(startStandaloneWebVitalSpanSpy).not.toHaveBeenCalled();
53+
});
54+
55+
it('early-returns if the inp metric has an unknown interaction type', () => {
56+
const startStandaloneWebVitalSpanSpy = vi.spyOn(utils, 'startStandaloneWebVitalSpan');
57+
58+
const metric = {
59+
value: 10,
60+
entries: [{ name: 'unknown', duration: 10, interactionId: 1 }],
61+
};
62+
// @ts-expect-error - incomplete metric object
63+
_onInp({ metric });
64+
65+
expect(startStandaloneWebVitalSpanSpy).not.toHaveBeenCalled();
66+
});
67+
68+
it('starts a span for a valid INP metric entry', () => {
69+
const startStandaloneWebVitalSpanSpy = vi.spyOn(utils, 'startStandaloneWebVitalSpan');
70+
71+
const metric = {
72+
value: 10,
73+
entries: [{ name: 'click', duration: 10, interactionId: 1 }],
74+
};
75+
// @ts-expect-error - incomplete metric object
76+
_onInp({ metric });
77+
78+
expect(startStandaloneWebVitalSpanSpy).toHaveBeenCalledTimes(1);
79+
expect(startStandaloneWebVitalSpanSpy).toHaveBeenCalledWith({
80+
attributes: {
81+
'sentry.exclusive_time': 10,
82+
'sentry.op': 'ui.interaction.click',
83+
'sentry.origin': 'auto.http.browser.inp',
84+
},
85+
name: '<unknown>',
86+
startTime: NaN,
87+
transaction: undefined,
88+
});
89+
});
90+
91+
it('takes the correct entry based on the metric value', () => {
92+
const startStandaloneWebVitalSpanSpy = vi.spyOn(utils, 'startStandaloneWebVitalSpan');
93+
94+
const metric = {
95+
value: 10,
96+
entries: [
97+
{ name: 'click', duration: 10, interactionId: 1 },
98+
{ name: 'click', duration: 9, interactionId: 2 },
99+
],
100+
};
101+
// @ts-expect-error - incomplete metric object
102+
_onInp({ metric });
103+
104+
expect(startStandaloneWebVitalSpanSpy).toHaveBeenCalledTimes(1);
105+
expect(startStandaloneWebVitalSpanSpy).toHaveBeenCalledWith({
106+
attributes: {
107+
'sentry.exclusive_time': 10,
108+
'sentry.op': 'ui.interaction.click',
109+
'sentry.origin': 'auto.http.browser.inp',
110+
},
111+
name: '<unknown>',
112+
startTime: NaN,
113+
transaction: undefined,
114+
});
115+
});
116+
});

0 commit comments

Comments
 (0)