Skip to content

Commit

Permalink
fix(webvitals): Fix mapping not being maintained properly and sometim…
Browse files Browse the repository at this point in the history 
…es 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
  • Loading branch information
@edwardgou-sentry
edwardgou-sentry committed Mar 19, 2024
1 parent 17a0e0f commit 0e9cddf
Show file tree
Hide file tree
Showing 11 changed files with 254 additions and 60 deletions.
9 changes: 5 additions & 4 deletions ...-integration-tests/suites/tracing/browserTracingIntegration/interactions/assets/script.js
View file
@@ -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));
1 change: 1 addition & 0 deletions ...ser-integration-tests/suites/tracing/browserTracingIntegration/interactions/template.html
View file
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
63 changes: 61 additions & 2 deletions ...s/browser-integration-tests/suites/tracing/browserTracingIntegration/interactions/test.ts
View file
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');
},
);
9 changes: 5 additions & 4 deletions ...ges/browser-integration-tests/suites/tracing/browsertracing/interactions/assets/script.js
View file
@@ -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));
1 change: 1 addition & 0 deletions ...ckages/browser-integration-tests/suites/tracing/browsertracing/interactions/template.html
View file
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
75 changes: 71 additions & 4 deletions dev-packages/browser-integration-tests/suites/tracing/browsertracing/interactions/test.ts
View file
@@ -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
View file
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

0 comments on commit 0e9cddf

Please sign in to comment.