Skip to content

Commit f5449d2

Browse files
mydeabillyvg
authored andcommitted
fix(browser): Fix INP span creation & transaction tagging (#12372)
Instead of #12358, this is a simpler change which ensures we pick the transaction from the scope instead. I also added tests for the various different scenarios, to ensure we see how they behave: 1. INP is emitted _during_ pageload (span is active) 2. INP is emitted _after_ pageload a. Pageload is parametrized (route) b. Pageload is unparametrized (URL) When the pageload is unparametrized (default browser SDK), the transaction is not added to the DSC envelope header (which is correct and also what we do in other places). it is _always_ added to the span attributes now, though. If no span is active, it will use transactionName from the last active pageload/navigation span. There may be edge cases where this is not 100% correct (e.g. if the INP span is only emitted once the pageload is done but another navigation already started) but IMHO these are more edge cases and this change is probably fine for now..? (While at it, I also added an origin to the INP spans)
1 parent cc3be00 commit f5449d2

File tree

20 files changed

+660
-92
lines changed

20 files changed

+660
-92
lines changed

.size-limit.js

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ module.exports = [
1515
path: 'packages/browser/build/npm/esm/index.js',
1616
import: createImport('init', 'browserTracingIntegration'),
1717
gzip: true,
18-
limit: '34 KB',
18+
limit: '35 KB',
1919
},
2020
{
2121
name: '@sentry/browser (incl. Tracing, Replay)',
2222
path: 'packages/browser/build/npm/esm/index.js',
2323
import: createImport('init', 'browserTracingIntegration', 'replayIntegration'),
2424
gzip: true,
25-
limit: '71 KB',
25+
limit: '72 KB',
2626
},
2727
{
2828
name: '@sentry/browser (incl. Tracing, Replay) - with treeshaking flags',
@@ -48,14 +48,14 @@ module.exports = [
4848
path: 'packages/browser/build/npm/esm/index.js',
4949
import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'),
5050
gzip: true,
51-
limit: '75 KB',
51+
limit: '76 KB',
5252
},
5353
{
5454
name: '@sentry/browser (incl. Tracing, Replay, Feedback)',
5555
path: 'packages/browser/build/npm/esm/index.js',
5656
import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'feedbackIntegration'),
5757
gzip: true,
58-
limit: '87 KB',
58+
limit: '89 KB',
5959
},
6060
{
6161
name: '@sentry/browser (incl. Tracing, Replay, Feedback, metrics)',
@@ -69,21 +69,21 @@ module.exports = [
6969
path: 'packages/browser/build/npm/esm/index.js',
7070
import: createImport('init', 'metrics'),
7171
gzip: true,
72-
limit: '40 KB',
72+
limit: '30 KB',
7373
},
7474
{
7575
name: '@sentry/browser (incl. Feedback)',
7676
path: 'packages/browser/build/npm/esm/index.js',
7777
import: createImport('init', 'feedbackIntegration'),
7878
gzip: true,
79-
limit: '40 KB',
79+
limit: '41 KB',
8080
},
8181
{
8282
name: '@sentry/browser (incl. sendFeedback)',
8383
path: 'packages/browser/build/npm/esm/index.js',
8484
import: createImport('init', 'sendFeedback'),
8585
gzip: true,
86-
limit: '28 KB',
86+
limit: '29 KB',
8787
},
8888
{
8989
name: '@sentry/browser (incl. FeedbackAsync)',
@@ -107,7 +107,7 @@ module.exports = [
107107
import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'),
108108
ignore: ['react/jsx-runtime'],
109109
gzip: true,
110-
limit: '37 KB',
110+
limit: '38 KB',
111111
},
112112
// Vue SDK (ESM)
113113
{
@@ -143,7 +143,7 @@ module.exports = [
143143
name: 'CDN Bundle (incl. Tracing)',
144144
path: createCDNPath('bundle.tracing.min.js'),
145145
gzip: true,
146-
limit: '36 KB',
146+
limit: '37 KB',
147147
},
148148
{
149149
name: 'CDN Bundle (incl. Tracing, Replay)',
@@ -193,7 +193,7 @@ module.exports = [
193193
import: createImport('init'),
194194
ignore: ['next/router', 'next/constants'],
195195
gzip: true,
196-
limit: '37 KB',
196+
limit: '38 KB',
197197
},
198198
// SvelteKit SDK (ESM)
199199
{
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://[email protected]/1337',
7+
integrations: [
8+
Sentry.browserTracingIntegration({
9+
idleTimeout: 1000,
10+
enableLongTask: false,
11+
enableInp: true,
12+
instrumentPageLoad: false,
13+
instrumentNavigation: false,
14+
}),
15+
],
16+
tracesSampleRate: 1,
17+
});
18+
19+
const client = Sentry.getClient();
20+
21+
// Force page load transaction name to a testable value
22+
Sentry.startBrowserTracingPageLoadSpan(client, {
23+
name: 'test-url',
24+
attributes: {
25+
[Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
26+
},
27+
});
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
const blockUI = (delay = 70) => e => {
2+
const startTime = Date.now();
3+
4+
function getElasped() {
5+
const time = Date.now();
6+
return time - startTime;
7+
}
8+
9+
while (getElasped() < delay) {
10+
//
11+
}
12+
13+
e.target.classList.add('clicked');
14+
};
15+
16+
document.querySelector('[data-test-id=not-so-slow-button]').addEventListener('click', blockUI(300));
17+
document.querySelector('[data-test-id=slow-button]').addEventListener('click', blockUI(450));
18+
document.querySelector('[data-test-id=normal-button]').addEventListener('click', blockUI());
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<div>Rendered Before Long Task</div>
8+
<button data-test-id="slow-button" data-sentry-element="SlowButton">Slow</button>
9+
<button data-test-id="not-so-slow-button" data-sentry-element="NotSoSlowButton">Not so slow</button>
10+
<button data-test-id="normal-button" data-sentry-element="NormalButton">Click Me</button>
11+
</body>
12+
</html>
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import { expect } from '@playwright/test';
2+
import type { Event as SentryEvent, SpanEnvelope } from '@sentry/types';
3+
4+
import { sentryTest } from '../../../../utils/fixtures';
5+
import {
6+
getFirstSentryEnvelopeRequest,
7+
getMultipleSentryEnvelopeRequests,
8+
properFullEnvelopeRequestParser,
9+
shouldSkipTracingTest,
10+
} from '../../../../utils/helpers';
11+
12+
sentryTest('should capture an INP click event span after pageload', async ({ browserName, getLocalTestPath, page }) => {
13+
const supportedBrowsers = ['chromium'];
14+
15+
if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) {
16+
sentryTest.skip();
17+
}
18+
19+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
20+
return route.fulfill({
21+
status: 200,
22+
contentType: 'application/json',
23+
body: JSON.stringify({ id: 'test-id' }),
24+
});
25+
});
26+
27+
const url = await getLocalTestPath({ testDir: __dirname });
28+
29+
await page.goto(url);
30+
await getFirstSentryEnvelopeRequest<SentryEvent>(page); // wait for page load
31+
32+
const spanEnvelopePromise = getMultipleSentryEnvelopeRequests<SpanEnvelope>(
33+
page,
34+
1,
35+
{ envelopeType: 'span' },
36+
properFullEnvelopeRequestParser,
37+
);
38+
39+
await page.locator('[data-test-id=normal-button]').click();
40+
await page.locator('.clicked[data-test-id=normal-button]').isVisible();
41+
42+
await page.waitForTimeout(500);
43+
44+
// Page hide to trigger INP
45+
await page.evaluate(() => {
46+
window.dispatchEvent(new Event('pagehide'));
47+
});
48+
49+
// Get the INP span envelope
50+
const spanEnvelope = (await spanEnvelopePromise)[0];
51+
52+
const spanEnvelopeHeaders = spanEnvelope[0];
53+
const spanEnvelopeItem = spanEnvelope[1][0][1];
54+
55+
const traceId = spanEnvelopeHeaders.trace!.trace_id;
56+
expect(traceId).toMatch(/[a-f0-9]{32}/);
57+
58+
expect(spanEnvelopeHeaders).toEqual({
59+
sent_at: expect.any(String),
60+
trace: {
61+
environment: 'production',
62+
public_key: 'public',
63+
sample_rate: '1',
64+
sampled: 'true',
65+
trace_id: traceId,
66+
},
67+
});
68+
69+
const inpValue = spanEnvelopeItem.measurements?.inp.value;
70+
expect(inpValue).toBeGreaterThan(0);
71+
72+
expect(spanEnvelopeItem).toEqual({
73+
data: {
74+
'sentry.exclusive_time': inpValue,
75+
'sentry.op': 'ui.interaction.click',
76+
'sentry.origin': 'auto.http.browser.inp',
77+
'sentry.sample_rate': 1,
78+
'sentry.source': 'custom',
79+
transaction: 'test-url',
80+
},
81+
measurements: {
82+
inp: {
83+
unit: 'millisecond',
84+
value: inpValue,
85+
},
86+
},
87+
description: 'body > NormalButton',
88+
exclusive_time: inpValue,
89+
op: 'ui.interaction.click',
90+
origin: 'auto.http.browser.inp',
91+
is_segment: true,
92+
segment_id: spanEnvelopeItem.span_id,
93+
span_id: expect.stringMatching(/[a-f0-9]{16}/),
94+
start_timestamp: expect.any(Number),
95+
timestamp: expect.any(Number),
96+
trace_id: traceId,
97+
});
98+
});
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://[email protected]/1337',
7+
integrations: [
8+
Sentry.browserTracingIntegration({
9+
idleTimeout: 1000,
10+
enableLongTask: false,
11+
enableInp: true,
12+
instrumentPageLoad: false,
13+
instrumentNavigation: false,
14+
}),
15+
],
16+
tracesSampleRate: 1,
17+
});
18+
19+
const client = Sentry.getClient();
20+
21+
// Force page load transaction name to a testable value
22+
Sentry.startBrowserTracingPageLoadSpan(client, {
23+
name: 'test-route',
24+
attributes: {
25+
[Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
26+
},
27+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
const blockUI = (delay = 70) => e => {
2+
const startTime = Date.now();
3+
4+
function getElasped() {
5+
const time = Date.now();
6+
return time - startTime;
7+
}
8+
9+
while (getElasped() < delay) {
10+
//
11+
}
12+
13+
e.target.classList.add('clicked');
14+
};
15+
16+
document.querySelector('[data-test-id=not-so-slow-button]').addEventListener('click', blockUI(300));
17+
document.querySelector('[data-test-id=slow-button]').addEventListener('click', blockUI(450));
18+
document.querySelector('[data-test-id=normal-button]').addEventListener('click', blockUI());
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<div>Rendered Before Long Task</div>
8+
<button data-test-id="slow-button" data-sentry-element="SlowButton">Slow</button>
9+
<button data-test-id="not-so-slow-button" data-sentry-element="NotSoSlowButton">Not so slow</button>
10+
<button data-test-id="normal-button" data-sentry-element="NormalButton">Click Me</button>
11+
</body>
12+
</html>

0 commit comments

Comments
 (0)