Skip to content

Commit 0e9cddf

Browse files
edwardgou-sentryedwardgou-sentry
authored
fix(webvitals): Fix mapping not being maintained properly and sometimes not sending INP spans (#11183)
Fixes scenarios where interaction tracking wasn't properly recording up to 10 candidate INP interactions. Fixes `first-input` interaction not always tracked as INP candidates. Fixes INP candidates not always using the longest latency even in their interaction id grouping. Also fixes: #11156
1 parent 17a0e0f commit 0e9cddf

File tree

11 files changed

+254
-60
lines changed

11 files changed

+254
-60
lines changed

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions/assets/script.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
1-
const delay = e => {
1+
const createDelayFunction = (delay = 70) => e => {
22
const startTime = Date.now();
33

44
function getElasped() {
55
const time = Date.now();
66
return time - startTime;
77
}
88

9-
while (getElasped() < 70) {
9+
while (getElasped() < delay) {
1010
//
1111
}
1212

1313
e.target.classList.add('clicked');
1414
};
1515

16-
document.querySelector('[data-test-id=interaction-button]').addEventListener('click', delay);
17-
document.querySelector('[data-test-id=annotated-button]').addEventListener('click', delay);
16+
document.querySelector('[data-test-id=interaction-button]').addEventListener('click', createDelayFunction());
17+
document.querySelector('[data-test-id=annotated-button]').addEventListener('click', createDelayFunction());
18+
document.querySelector('[data-test-id=slow-interaction-button]').addEventListener('click', createDelayFunction(200));

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions/template.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
<body>
77
<div>Rendered Before Long Task</div>
88
<button data-test-id="interaction-button">Click Me</button>
9+
<button data-test-id="slow-interaction-button">Also Click Me</button>
910
<button data-test-id="annotated-button" data-sentry-component="AnnotatedButton">Click Me</button>
1011
<script src="https://example.com/path/to/script.js"></script>
1112
</body>

‎dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions/test.ts

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ sentryTest(
113113
},
114114
);
115115

116-
sentryTest('should capture an INP click event span. @firefox', async ({ browserName, getLocalTestPath, page }) => {
117-
const supportedBrowsers = ['chromium', 'firefox'];
116+
sentryTest('should capture an INP click event span.', async ({ browserName, getLocalTestPath, page }) => {
117+
const supportedBrowsers = ['chromium'];
118118

119119
if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) {
120120
sentryTest.skip();
@@ -160,3 +160,62 @@ sentryTest('should capture an INP click event span. @firefox', async ({ browserN
160160
expect(spanEnvelopes[0].measurements.inp.value).toBeGreaterThan(0);
161161
expect(spanEnvelopes[0].measurements.inp.unit).toBe('millisecond');
162162
});
163+
164+
sentryTest(
165+
'should choose the slowest interaction click event when INP is triggered.',
166+
async ({ browserName, getLocalTestPath, page }) => {
167+
const supportedBrowsers = ['chromium'];
168+
169+
if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) {
170+
sentryTest.skip();
171+
}
172+
173+
await page.route('**/path/to/script.js', (route: Route) =>
174+
route.fulfill({ path: `${__dirname}/assets/script.js` }),
175+
);
176+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
177+
return route.fulfill({
178+
status: 200,
179+
contentType: 'application/json',
180+
body: JSON.stringify({ id: 'test-id' }),
181+
});
182+
});
183+
184+
const url = await getLocalTestPath({ testDir: __dirname });
185+
186+
await page.goto(url);
187+
await getFirstSentryEnvelopeRequest<SentryEvent>(page);
188+
189+
await page.locator('[data-test-id=interaction-button]').click();
190+
await page.locator('.clicked[data-test-id=interaction-button]').isVisible();
191+
192+
// Wait for the interaction transaction from the enableInteractions experiment
193+
await getMultipleSentryEnvelopeRequests<TransactionJSON>(page, 1);
194+
195+
await page.locator('[data-test-id=slow-interaction-button]').click();
196+
await page.locator('.clicked[data-test-id=slow-interaction-button]').isVisible();
197+
198+
// Wait for the interaction transaction from the enableInteractions experiment
199+
await getMultipleSentryEnvelopeRequests<TransactionJSON>(page, 1);
200+
201+
const spanEnvelopesPromise = getMultipleSentryEnvelopeRequests<
202+
SpanJSON & { exclusive_time: number; measurements: Measurements }
203+
>(page, 1, {
204+
envelopeType: 'span',
205+
});
206+
// Page hide to trigger INP
207+
await page.evaluate(() => {
208+
window.dispatchEvent(new Event('pagehide'));
209+
});
210+
211+
// Get the INP span envelope
212+
const spanEnvelopes = await spanEnvelopesPromise;
213+
214+
expect(spanEnvelopes).toHaveLength(1);
215+
expect(spanEnvelopes[0].op).toBe('ui.interaction.click');
216+
expect(spanEnvelopes[0].description).toBe('body > button.clicked');
217+
expect(spanEnvelopes[0].exclusive_time).toBeGreaterThan(150);
218+
expect(spanEnvelopes[0].measurements.inp.value).toBeGreaterThan(150);
219+
expect(spanEnvelopes[0].measurements.inp.unit).toBe('millisecond');
220+
},
221+
);

‎dev-packages/browser-integration-tests/suites/tracing/browsertracing/interactions/assets/script.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
1-
const delay = e => {
1+
const createDelayFunction = (delay = 70) => e => {
22
const startTime = Date.now();
33

44
function getElasped() {
55
const time = Date.now();
66
return time - startTime;
77
}
88

9-
while (getElasped() < 70) {
9+
while (getElasped() < delay) {
1010
//
1111
}
1212

1313
e.target.classList.add('clicked');
1414
};
1515

16-
document.querySelector('[data-test-id=interaction-button]').addEventListener('click', delay);
17-
document.querySelector('[data-test-id=annotated-button]').addEventListener('click', delay);
16+
document.querySelector('[data-test-id=interaction-button]').addEventListener('click', createDelayFunction());
17+
document.querySelector('[data-test-id=annotated-button]').addEventListener('click', createDelayFunction());
18+
document.querySelector('[data-test-id=slow-interaction-button]').addEventListener('click', createDelayFunction(200));

‎dev-packages/browser-integration-tests/suites/tracing/browsertracing/interactions/template.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
<body>
77
<div>Rendered Before Long Task</div>
88
<button data-test-id="interaction-button">Click Me</button>
9+
<button data-test-id="slow-interaction-button">Also Click Me</button>
910
<button data-test-id="annotated-button" data-sentry-component="AnnotatedButton">Click Me</button>
1011
<script src="https://example.com/path/to/script.js"></script>
1112
</body>

‎dev-packages/browser-integration-tests/suites/tracing/browsertracing/interactions/test.ts

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
import type { Route } from '@playwright/test';
22
import { expect } from '@playwright/test';
3-
import type { Measurements, SerializedEvent, Span, SpanContext, SpanJSON, Transaction } from '@sentry/types';
3+
import type {
4+
Event as SentryEvent,
5+
Measurements,
6+
SerializedEvent,
7+
Span,
8+
SpanContext,
9+
SpanJSON,
10+
Transaction,
11+
} from '@sentry/types';
412

513
import { sentryTest } from '../../../../utils/fixtures';
614
import {
@@ -113,8 +121,8 @@ sentryTest(
113121
},
114122
);
115123

116-
sentryTest('should capture an INP click event span. @firefox', async ({ browserName, getLocalTestPath, page }) => {
117-
const supportedBrowsers = ['chromium', 'firefox'];
124+
sentryTest('should capture an INP click event span.', async ({ browserName, getLocalTestPath, page }) => {
125+
const supportedBrowsers = ['chromium'];
118126

119127
if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) {
120128
sentryTest.skip();
@@ -132,7 +140,7 @@ sentryTest('should capture an INP click event span. @firefox', async ({ browserN
132140
const url = await getLocalTestPath({ testDir: __dirname });
133141

134142
await page.goto(url);
135-
await getFirstSentryEnvelopeRequest<Event>(page);
143+
await getFirstSentryEnvelopeRequest<SentryEvent>(page);
136144

137145
await page.locator('[data-test-id=interaction-button]').click();
138146
await page.locator('.clicked[data-test-id=interaction-button]').isVisible();
@@ -160,3 +168,62 @@ sentryTest('should capture an INP click event span. @firefox', async ({ browserN
160168
expect(spanEnvelopes[0].measurements.inp.value).toBeGreaterThan(0);
161169
expect(spanEnvelopes[0].measurements.inp.unit).toBe('millisecond');
162170
});
171+
172+
sentryTest(
173+
'should choose the slowest interaction click event when INP is triggered.',
174+
async ({ browserName, getLocalTestPath, page }) => {
175+
const supportedBrowsers = ['chromium'];
176+
177+
if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) {
178+
sentryTest.skip();
179+
}
180+
181+
await page.route('**/path/to/script.js', (route: Route) =>
182+
route.fulfill({ path: `${__dirname}/assets/script.js` }),
183+
);
184+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
185+
return route.fulfill({
186+
status: 200,
187+
contentType: 'application/json',
188+
body: JSON.stringify({ id: 'test-id' }),
189+
});
190+
});
191+
192+
const url = await getLocalTestPath({ testDir: __dirname });
193+
194+
await page.goto(url);
195+
await getFirstSentryEnvelopeRequest<SentryEvent>(page);
196+
197+
await page.locator('[data-test-id=interaction-button]').click();
198+
await page.locator('.clicked[data-test-id=interaction-button]').isVisible();
199+
200+
// Wait for the interaction transaction from the enableInteractions experiment
201+
await getMultipleSentryEnvelopeRequests<TransactionJSON>(page, 1);
202+
203+
await page.locator('[data-test-id=slow-interaction-button]').click();
204+
await page.locator('.clicked[data-test-id=slow-interaction-button]').isVisible();
205+
206+
// Wait for the interaction transaction from the enableInteractions experiment
207+
await getMultipleSentryEnvelopeRequests<TransactionJSON>(page, 1);
208+
209+
const spanEnvelopesPromise = getMultipleSentryEnvelopeRequests<
210+
SpanJSON & { exclusive_time: number; measurements: Measurements }
211+
>(page, 1, {
212+
envelopeType: 'span',
213+
});
214+
// Page hide to trigger INP
215+
await page.evaluate(() => {
216+
window.dispatchEvent(new Event('pagehide'));
217+
});
218+
219+
// Get the INP span envelope
220+
const spanEnvelopes = await spanEnvelopesPromise;
221+
222+
expect(spanEnvelopes).toHaveLength(1);
223+
expect(spanEnvelopes[0].op).toBe('ui.interaction.click');
224+
expect(spanEnvelopes[0].description).toBe('body > button.clicked');
225+
expect(spanEnvelopes[0].exclusive_time).toBeGreaterThan(150);
226+
expect(spanEnvelopes[0].measurements.inp.value).toBeGreaterThan(150);
227+
expect(spanEnvelopes[0].measurements.inp.unit).toBe('millisecond');
228+
},
229+
);

‎packages/tracing-internal/src/browser/browserTracingIntegration.ts

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,9 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
194194
const _collectWebVitals = startTrackingWebVitals();
195195

196196
/** Stores a mapping of interactionIds from PerformanceEventTimings to the origin interaction path */
197-
const interactionIdtoRouteNameMapping: InteractionRouteNameMapping = {};
197+
const interactionIdToRouteNameMapping: InteractionRouteNameMapping = {};
198198
if (options.enableInp) {
199-
startTrackingINP(interactionIdtoRouteNameMapping);
199+
startTrackingINP(interactionIdToRouteNameMapping);
200200
}
201201

202202
if (options.enableLongTask) {
@@ -411,7 +411,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
411411
}
412412

413413
if (options.enableInp) {
414-
registerInpInteractionListener(interactionIdtoRouteNameMapping, latestRoute);
414+
registerInpInteractionListener(interactionIdToRouteNameMapping, latestRoute);
415415
}
416416

417417
instrumentOutgoingRequests({
@@ -541,13 +541,13 @@ const MAX_INTERACTIONS = 10;
541541

542542
/** Creates a listener on interaction entries, and maps interactionIds to the origin path of the interaction */
543543
function registerInpInteractionListener(
544-
interactionIdtoRouteNameMapping: InteractionRouteNameMapping,
544+
interactionIdToRouteNameMapping: InteractionRouteNameMapping,
545545
latestRoute: {
546546
name: string | undefined;
547547
context: TransactionContext | undefined;
548548
},
549549
): void {
550-
addPerformanceInstrumentationHandler('event', ({ entries }) => {
550+
const handleEntries = ({ entries }: { entries: PerformanceEntry[] }): void => {
551551
const client = getClient();
552552
// We need to get the replay, user, and activeTransaction from the current scope
553553
// so that we can associate replay id, profile id, and a user display to the span
@@ -560,40 +560,72 @@ function registerInpInteractionListener(
560560
const activeTransaction = getActiveTransaction();
561561
const currentScope = getCurrentScope();
562562
const user = currentScope !== undefined ? currentScope.getUser() : undefined;
563-
for (const entry of entries) {
563+
entries.forEach(entry => {
564564
if (isPerformanceEventTiming(entry)) {
565+
const interactionId = entry.interactionId;
566+
if (interactionId === undefined) {
567+
return;
568+
}
569+
const existingInteraction = interactionIdToRouteNameMapping[interactionId];
565570
const duration = entry.duration;
566-
const keys = Object.keys(interactionIdtoRouteNameMapping);
571+
const startTime = entry.startTime;
572+
const keys = Object.keys(interactionIdToRouteNameMapping);
567573
const minInteractionId =
568574
keys.length > 0
569575
? keys.reduce((a, b) => {
570-
return interactionIdtoRouteNameMapping[a].duration < interactionIdtoRouteNameMapping[b].duration
576+
return interactionIdToRouteNameMapping[a].duration < interactionIdToRouteNameMapping[b].duration
571577
? a
572578
: b;
573579
})
574580
: undefined;
575-
if (minInteractionId === undefined || duration > interactionIdtoRouteNameMapping[minInteractionId].duration) {
576-
const interactionId = entry.interactionId;
581+
// For a first input event to be considered, we must check that an interaction event does not already exist with the same duration and start time.
582+
// This is also checked in the web-vitals library.
583+
if (entry.entryType === 'first-input') {
584+
const matchingEntry = keys
585+
.map(key => interactionIdToRouteNameMapping[key])
586+
.some(interaction => {
587+
return interaction.duration === duration && interaction.startTime === startTime;
588+
});
589+
if (matchingEntry) {
590+
return;
591+
}
592+
}
593+
// Interactions with an id of 0 and are not first-input are not valid.
594+
if (!interactionId) {
595+
return;
596+
}
597+
// If the interaction already exists, we want to use the duration of the longest entry, since that is what the INP metric uses.
598+
if (existingInteraction) {
599+
existingInteraction.duration = Math.max(existingInteraction.duration, duration);
600+
} else if (
601+
keys.length < MAX_INTERACTIONS ||
602+
minInteractionId === undefined ||
603+
duration > interactionIdToRouteNameMapping[minInteractionId].duration
604+
) {
605+
// If the interaction does not exist, we want to add it to the mapping if there is space, or if the duration is longer than the shortest entry.
577606
const routeName = latestRoute.name;
578607
const parentContext = latestRoute.context;
579-
if (interactionId && routeName && parentContext) {
580-
if (minInteractionId && Object.keys(interactionIdtoRouteNameMapping).length >= MAX_INTERACTIONS) {
608+
if (routeName && parentContext) {
609+
if (minInteractionId && Object.keys(interactionIdToRouteNameMapping).length >= MAX_INTERACTIONS) {
581610
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
582-
delete interactionIdtoRouteNameMapping[minInteractionId];
611+
delete interactionIdToRouteNameMapping[minInteractionId];
583612
}
584-
interactionIdtoRouteNameMapping[interactionId] = {
613+
interactionIdToRouteNameMapping[interactionId] = {
585614
routeName,
586615
duration,
587616
parentContext,
588617
user,
589618
activeTransaction,
590619
replayId,
620+
startTime,
591621
};
592622
}
593623
}
594624
}
595-
}
596-
});
625+
});
626+
};
627+
addPerformanceInstrumentationHandler('event', handleEntries);
628+
addPerformanceInstrumentationHandler('first-input', handleEntries);
597629
}
598630

599631
function getSource(context: TransactionContext): TransactionSource | undefined {

0 commit comments

Comments
 (0)