Skip to content
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.

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

fix(webvitals): Fix interactionIdToRouteNameMapping not being maintained properly and sometimes not sending INP spans #11183

Merged
merged 3 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
const delay = e => {
const createDelayFunction = (delay = 70) => e => {
const startTime = Date.now();

function getElasped() {
const time = Date.now();
return time - startTime;
}

while (getElasped() < 70) {
while (getElasped() < delay) {
//
}

e.target.classList.add('clicked');
};

document.querySelector('[data-test-id=interaction-button]').addEventListener('click', delay);
document.querySelector('[data-test-id=annotated-button]').addEventListener('click', delay);
document.querySelector('[data-test-id=interaction-button]').addEventListener('click', createDelayFunction());
document.querySelector('[data-test-id=annotated-button]').addEventListener('click', createDelayFunction());
document.querySelector('[data-test-id=slow-interaction-button]').addEventListener('click', createDelayFunction(200));
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<body>
<div>Rendered Before Long Task</div>
<button data-test-id="interaction-button">Click Me</button>
<button data-test-id="slow-interaction-button">Also Click Me</button>
<button data-test-id="annotated-button" data-sentry-component="AnnotatedButton">Click Me</button>
<script src="https://example.com/path/to/script.js"></script>
</body>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ sentryTest(
},
);

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

if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) {
sentryTest.skip();
Expand Down Expand Up @@ -160,3 +160,62 @@ sentryTest('should capture an INP click event span. @firefox', async ({ browserN
expect(spanEnvelopes[0].measurements.inp.value).toBeGreaterThan(0);
expect(spanEnvelopes[0].measurements.inp.unit).toBe('millisecond');
});

sentryTest(
'should choose the slowest interaction click event when INP is triggered.',
async ({ browserName, getLocalTestPath, page }) => {
const supportedBrowsers = ['chromium'];

if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) {
sentryTest.skip();
}

await page.route('**/path/to/script.js', (route: Route) =>
route.fulfill({ path: `${__dirname}/assets/script.js` }),
);
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
return route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ id: 'test-id' }),
});
});

const url = await getLocalTestPath({ testDir: __dirname });

await page.goto(url);
await getFirstSentryEnvelopeRequest<SentryEvent>(page);

await page.locator('[data-test-id=interaction-button]').click();
await page.locator('.clicked[data-test-id=interaction-button]').isVisible();

// Wait for the interaction transaction from the enableInteractions experiment
await getMultipleSentryEnvelopeRequests<TransactionJSON>(page, 1);

await page.locator('[data-test-id=slow-interaction-button]').click();
await page.locator('.clicked[data-test-id=slow-interaction-button]').isVisible();

// Wait for the interaction transaction from the enableInteractions experiment
await getMultipleSentryEnvelopeRequests<TransactionJSON>(page, 1);

const spanEnvelopesPromise = getMultipleSentryEnvelopeRequests<
SpanJSON & { exclusive_time: number; measurements: Measurements }
>(page, 1, {
envelopeType: 'span',
});
// Page hide to trigger INP
await page.evaluate(() => {
window.dispatchEvent(new Event('pagehide'));
});

// Get the INP span envelope
const spanEnvelopes = await spanEnvelopesPromise;

expect(spanEnvelopes).toHaveLength(1);
expect(spanEnvelopes[0].op).toBe('ui.interaction.click');
expect(spanEnvelopes[0].description).toBe('body > button.clicked');
expect(spanEnvelopes[0].exclusive_time).toBeGreaterThan(150);
expect(spanEnvelopes[0].measurements.inp.value).toBeGreaterThan(150);
expect(spanEnvelopes[0].measurements.inp.unit).toBe('millisecond');
},
);
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
const delay = e => {
const createDelayFunction = (delay = 70) => e => {
const startTime = Date.now();

function getElasped() {
const time = Date.now();
return time - startTime;
}

while (getElasped() < 70) {
while (getElasped() < delay) {
//
}

e.target.classList.add('clicked');
};

document.querySelector('[data-test-id=interaction-button]').addEventListener('click', delay);
document.querySelector('[data-test-id=annotated-button]').addEventListener('click', delay);
document.querySelector('[data-test-id=interaction-button]').addEventListener('click', createDelayFunction());
document.querySelector('[data-test-id=annotated-button]').addEventListener('click', createDelayFunction());
document.querySelector('[data-test-id=slow-interaction-button]').addEventListener('click', createDelayFunction(200));
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
<body>
<div>Rendered Before Long Task</div>
<button data-test-id="interaction-button">Click Me</button>
<button data-test-id="slow-interaction-button">Also Click Me</button>
<button data-test-id="annotated-button" data-sentry-component="AnnotatedButton">Click Me</button>
<script src="https://example.com/path/to/script.js"></script>
</body>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
import type { Route } from '@playwright/test';
import { expect } from '@playwright/test';
import type { Measurements, SerializedEvent, Span, SpanContext, SpanJSON, Transaction } from '@sentry/types';
import type {
Event as SentryEvent,
Measurements,
SerializedEvent,
Span,
SpanContext,
SpanJSON,
Transaction,
} from '@sentry/types';

import { sentryTest } from '../../../../utils/fixtures';
import {
Expand Down Expand Up @@ -113,8 +121,8 @@ sentryTest(
},
);

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

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

await page.goto(url);
await getFirstSentryEnvelopeRequest<Event>(page);
await getFirstSentryEnvelopeRequest<SentryEvent>(page);

await page.locator('[data-test-id=interaction-button]').click();
await page.locator('.clicked[data-test-id=interaction-button]').isVisible();
Expand Down Expand Up @@ -160,3 +168,62 @@ sentryTest('should capture an INP click event span. @firefox', async ({ browserN
expect(spanEnvelopes[0].measurements.inp.value).toBeGreaterThan(0);
expect(spanEnvelopes[0].measurements.inp.unit).toBe('millisecond');
});

sentryTest(
'should choose the slowest interaction click event when INP is triggered.',
async ({ browserName, getLocalTestPath, page }) => {
const supportedBrowsers = ['chromium'];

if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) {
sentryTest.skip();
}

await page.route('**/path/to/script.js', (route: Route) =>
route.fulfill({ path: `${__dirname}/assets/script.js` }),
);
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
return route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ id: 'test-id' }),
});
});

const url = await getLocalTestPath({ testDir: __dirname });

await page.goto(url);
await getFirstSentryEnvelopeRequest<SentryEvent>(page);

await page.locator('[data-test-id=interaction-button]').click();
await page.locator('.clicked[data-test-id=interaction-button]').isVisible();

// Wait for the interaction transaction from the enableInteractions experiment
await getMultipleSentryEnvelopeRequests<TransactionJSON>(page, 1);

await page.locator('[data-test-id=slow-interaction-button]').click();
await page.locator('.clicked[data-test-id=slow-interaction-button]').isVisible();

// Wait for the interaction transaction from the enableInteractions experiment
await getMultipleSentryEnvelopeRequests<TransactionJSON>(page, 1);

const spanEnvelopesPromise = getMultipleSentryEnvelopeRequests<
SpanJSON & { exclusive_time: number; measurements: Measurements }
>(page, 1, {
envelopeType: 'span',
});
// Page hide to trigger INP
await page.evaluate(() => {
window.dispatchEvent(new Event('pagehide'));
});

// Get the INP span envelope
const spanEnvelopes = await spanEnvelopesPromise;

expect(spanEnvelopes).toHaveLength(1);
expect(spanEnvelopes[0].op).toBe('ui.interaction.click');
expect(spanEnvelopes[0].description).toBe('body > button.clicked');
expect(spanEnvelopes[0].exclusive_time).toBeGreaterThan(150);
expect(spanEnvelopes[0].measurements.inp.value).toBeGreaterThan(150);
expect(spanEnvelopes[0].measurements.inp.unit).toBe('millisecond');
},
);
64 changes: 48 additions & 16 deletions packages/tracing-internal/src/browser/browserTracingIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,9 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
const _collectWebVitals = startTrackingWebVitals();

/** Stores a mapping of interactionIds from PerformanceEventTimings to the origin interaction path */
const interactionIdtoRouteNameMapping: InteractionRouteNameMapping = {};
const interactionIdToRouteNameMapping: InteractionRouteNameMapping = {};
if (options.enableInp) {
startTrackingINP(interactionIdtoRouteNameMapping);
startTrackingINP(interactionIdToRouteNameMapping);
}

if (options.enableLongTask) {
Expand Down Expand Up @@ -411,7 +411,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
}

if (options.enableInp) {
registerInpInteractionListener(interactionIdtoRouteNameMapping, latestRoute);
registerInpInteractionListener(interactionIdToRouteNameMapping, latestRoute);
}

instrumentOutgoingRequests({
Expand Down Expand Up @@ -541,13 +541,13 @@ const MAX_INTERACTIONS = 10;

/** Creates a listener on interaction entries, and maps interactionIds to the origin path of the interaction */
function registerInpInteractionListener(
interactionIdtoRouteNameMapping: InteractionRouteNameMapping,
interactionIdToRouteNameMapping: InteractionRouteNameMapping,
latestRoute: {
name: string | undefined;
context: TransactionContext | undefined;
},
): void {
addPerformanceInstrumentationHandler('event', ({ entries }) => {
const handleEntries = ({ entries }: { entries: PerformanceEntry[] }): void => {
const client = getClient();
// We need to get the replay, user, and activeTransaction from the current scope
// so that we can associate replay id, profile id, and a user display to the span
Expand All @@ -560,40 +560,72 @@ function registerInpInteractionListener(
const activeTransaction = getActiveTransaction();
const currentScope = getCurrentScope();
const user = currentScope !== undefined ? currentScope.getUser() : undefined;
for (const entry of entries) {
entries.forEach(entry => {
if (isPerformanceEventTiming(entry)) {
const interactionId = entry.interactionId;
if (interactionId === undefined) {
return;
}
const existingInteraction = interactionIdToRouteNameMapping[interactionId];
const duration = entry.duration;
const keys = Object.keys(interactionIdtoRouteNameMapping);
const startTime = entry.startTime;
const keys = Object.keys(interactionIdToRouteNameMapping);
const minInteractionId =
keys.length > 0
? keys.reduce((a, b) => {
return interactionIdtoRouteNameMapping[a].duration < interactionIdtoRouteNameMapping[b].duration
return interactionIdToRouteNameMapping[a].duration < interactionIdToRouteNameMapping[b].duration
? a
: b;
})
: undefined;
if (minInteractionId === undefined || duration > interactionIdtoRouteNameMapping[minInteractionId].duration) {
const interactionId = entry.interactionId;
// 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.
// This is also checked in the web-vitals library.
if (entry.entryType === 'first-input') {
const matchingEntry = keys
.map(key => interactionIdToRouteNameMapping[key])
.some(interaction => {
return interaction.duration === duration && interaction.startTime === startTime;
});
if (matchingEntry) {
return;
}
}
// Interactions with an id of 0 and are not first-input are not valid.
if (!interactionId) {
return;
}
// If the interaction already exists, we want to use the duration of the longest entry, since that is what the INP metric uses.
if (existingInteraction) {
existingInteraction.duration = Math.max(existingInteraction.duration, duration);
} else if (
keys.length < MAX_INTERACTIONS ||
minInteractionId === undefined ||
duration > interactionIdToRouteNameMapping[minInteractionId].duration
) {
// 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.
const routeName = latestRoute.name;
const parentContext = latestRoute.context;
if (interactionId && routeName && parentContext) {
if (minInteractionId && Object.keys(interactionIdtoRouteNameMapping).length >= MAX_INTERACTIONS) {
if (routeName && parentContext) {
if (minInteractionId && Object.keys(interactionIdToRouteNameMapping).length >= MAX_INTERACTIONS) {
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
delete interactionIdtoRouteNameMapping[minInteractionId];
delete interactionIdToRouteNameMapping[minInteractionId];
}
interactionIdtoRouteNameMapping[interactionId] = {
interactionIdToRouteNameMapping[interactionId] = {
routeName,
duration,
parentContext,
user,
activeTransaction,
replayId,
startTime,
};
}
}
}
}
});
});
};
addPerformanceInstrumentationHandler('event', handleEntries);
addPerformanceInstrumentationHandler('first-input', handleEntries);
}

function getSource(context: TransactionContext): TransactionSource | undefined {
Expand Down