From 65b0dc2fb350248f8dde471d24f0de03b3160304 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 5 Jun 2024 13:14:20 +0200 Subject: [PATCH 1/3] fix(browser): Fix INP span creation & transaction tagging Instead of https://github.com/getsentry/sentry-javascript/pull/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) --- .../metrics/web-vitals-inp-late/init.js | 27 ++++ .../metrics/web-vitals-inp-late/subject.js | 18 +++ .../metrics/web-vitals-inp-late/template.html | 12 ++ .../metrics/web-vitals-inp-late/test.ts | 98 +++++++++++ .../web-vitals-inp-parametrized-late/init.js | 27 ++++ .../subject.js | 18 +++ .../template.html | 12 ++ .../web-vitals-inp-parametrized-late/test.ts | 102 ++++++++++++ .../web-vitals-inp-parametrized/init.js | 27 ++++ .../web-vitals-inp-parametrized/subject.js | 18 +++ .../web-vitals-inp-parametrized/template.html | 12 ++ .../web-vitals-inp-parametrized/test.ts | 99 ++++++++++++ .../tracing/metrics/web-vitals-inp/init.js | 14 +- .../tracing/metrics/web-vitals-inp/test.ts | 153 +++++++++--------- packages/browser-utils/src/metrics/inp.ts | 6 +- 15 files changed, 566 insertions(+), 77 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/test.ts diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/init.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/init.js new file mode 100644 index 000000000000..1044a4b68bda --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/init.js @@ -0,0 +1,27 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [ + Sentry.browserTracingIntegration({ + idleTimeout: 1000, + enableLongTask: false, + enableInp: true, + instrumentPageLoad: false, + instrumentNavigation: false, + }), + ], + tracesSampleRate: 1, +}); + +const client = Sentry.getClient(); + +// Force page load transaction name to a testable value +Sentry.startBrowserTracingPageLoadSpan(client, { + name: 'test-url', + attributes: { + [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + }, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/subject.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/subject.js new file mode 100644 index 000000000000..ed6db5b5afe2 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/subject.js @@ -0,0 +1,18 @@ +const blockUI = (delay = 70) => e => { + const startTime = Date.now(); + + function getElasped() { + const time = Date.now(); + return time - startTime; + } + + while (getElasped() < delay) { + // + } + + e.target.classList.add('clicked'); +}; + +document.querySelector('[data-test-id=not-so-slow-button]').addEventListener('click', blockUI(300)); +document.querySelector('[data-test-id=slow-button]').addEventListener('click', blockUI(450)); +document.querySelector('[data-test-id=normal-button]').addEventListener('click', blockUI()); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/template.html b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/template.html new file mode 100644 index 000000000000..25c6920f07e2 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/template.html @@ -0,0 +1,12 @@ + + + + + + +
Rendered Before Long Task
+ + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts new file mode 100644 index 000000000000..1ec7ec50998a --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-late/test.ts @@ -0,0 +1,98 @@ +import { expect } from '@playwright/test'; +import type { Event as SentryEvent, SpanEnvelope } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { + getFirstSentryEnvelopeRequest, + getMultipleSentryEnvelopeRequests, + properFullEnvelopeRequestParser, + shouldSkipTracingTest, +} from '../../../../utils/helpers'; + +sentryTest('should capture an INP click event span after pageload', async ({ browserName, getLocalTestPath, page }) => { + const supportedBrowsers = ['chromium']; + + if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) { + sentryTest.skip(); + } + + 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(page); // wait for page load + + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + await page.locator('[data-test-id=normal-button]').click(); + await page.locator('.clicked[data-test-id=normal-button]').isVisible(); + + await page.waitForTimeout(500); + + // Page hide to trigger INP + await page.evaluate(() => { + window.dispatchEvent(new Event('pagehide')); + }); + + // Get the INP span envelope + const spanEnvelope = (await spanEnvelopePromise)[0]; + + const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + const traceId = spanEnvelopeHeaders.trace!.trace_id; + expect(traceId).toMatch(/[a-f0-9]{32}/); + + expect(spanEnvelopeHeaders).toEqual({ + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'public', + sample_rate: '1', + sampled: 'true', + trace_id: traceId, + }, + }); + + const inpValue = spanEnvelopeItem.measurements?.inp.value; + expect(inpValue).toBeGreaterThan(0); + + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.exclusive_time': inpValue, + 'sentry.op': 'ui.interaction.click', + 'sentry.origin': 'auto.http.browser.inp', + 'sentry.sample_rate': 1, + 'sentry.source': 'custom', + transaction: 'test-url', + }, + measurements: { + inp: { + unit: 'millisecond', + value: inpValue, + }, + }, + description: 'body > NormalButton', + exclusive_time: inpValue, + op: 'ui.interaction.click', + origin: 'auto.http.browser.inp', + is_segment: true, + segment_id: spanEnvelopeItem.span_id, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: traceId, + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/init.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/init.js new file mode 100644 index 000000000000..895e6f60ff42 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/init.js @@ -0,0 +1,27 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [ + Sentry.browserTracingIntegration({ + idleTimeout: 1000, + enableLongTask: false, + enableInp: true, + instrumentPageLoad: false, + instrumentNavigation: false, + }), + ], + tracesSampleRate: 1, +}); + +const client = Sentry.getClient(); + +// Force page load transaction name to a testable value +Sentry.startBrowserTracingPageLoadSpan(client, { + name: 'test-route', + attributes: { + [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + }, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/subject.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/subject.js new file mode 100644 index 000000000000..ed6db5b5afe2 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/subject.js @@ -0,0 +1,18 @@ +const blockUI = (delay = 70) => e => { + const startTime = Date.now(); + + function getElasped() { + const time = Date.now(); + return time - startTime; + } + + while (getElasped() < delay) { + // + } + + e.target.classList.add('clicked'); +}; + +document.querySelector('[data-test-id=not-so-slow-button]').addEventListener('click', blockUI(300)); +document.querySelector('[data-test-id=slow-button]').addEventListener('click', blockUI(450)); +document.querySelector('[data-test-id=normal-button]').addEventListener('click', blockUI()); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/template.html b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/template.html new file mode 100644 index 000000000000..25c6920f07e2 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/template.html @@ -0,0 +1,12 @@ + + + + + + +
Rendered Before Long Task
+ + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts new file mode 100644 index 000000000000..1354c373253e --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized-late/test.ts @@ -0,0 +1,102 @@ +import { expect } from '@playwright/test'; +import type { Event as SentryEvent, SpanEnvelope } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { + getFirstSentryEnvelopeRequest, + getMultipleSentryEnvelopeRequests, + properFullEnvelopeRequestParser, + shouldSkipTracingTest, +} from '../../../../utils/helpers'; + +sentryTest( + 'should capture an INP click event span after pageload for a parametrized transaction', + async ({ browserName, getLocalTestPath, page }) => { + const supportedBrowsers = ['chromium']; + + if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) { + sentryTest.skip(); + } + + 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(page); // wait for page load + + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + await page.locator('[data-test-id=normal-button]').click(); + await page.locator('.clicked[data-test-id=normal-button]').isVisible(); + + await page.waitForTimeout(500); + + // Page hide to trigger INP + await page.evaluate(() => { + window.dispatchEvent(new Event('pagehide')); + }); + + // Get the INP span envelope + const spanEnvelope = (await spanEnvelopePromise)[0]; + + const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + const traceId = spanEnvelopeHeaders.trace!.trace_id; + expect(traceId).toMatch(/[a-f0-9]{32}/); + + expect(spanEnvelopeHeaders).toEqual({ + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'public', + sample_rate: '1', + sampled: 'true', + trace_id: traceId, + transaction: 'test-route', + }, + }); + + const inpValue = spanEnvelopeItem.measurements?.inp.value; + expect(inpValue).toBeGreaterThan(0); + + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.exclusive_time': inpValue, + 'sentry.op': 'ui.interaction.click', + 'sentry.origin': 'auto.http.browser.inp', + 'sentry.sample_rate': 1, + 'sentry.source': 'custom', + transaction: 'test-route', + }, + measurements: { + inp: { + unit: 'millisecond', + value: inpValue, + }, + }, + description: 'body > NormalButton', + exclusive_time: inpValue, + op: 'ui.interaction.click', + origin: 'auto.http.browser.inp', + is_segment: true, + segment_id: spanEnvelopeItem.span_id, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: traceId, + }); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/init.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/init.js new file mode 100644 index 000000000000..fa9619209dfe --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/init.js @@ -0,0 +1,27 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [ + Sentry.browserTracingIntegration({ + idleTimeout: 4000, + enableLongTask: false, + enableInp: true, + instrumentPageLoad: false, + instrumentNavigation: false, + }), + ], + tracesSampleRate: 1, +}); + +const client = Sentry.getClient(); + +// Force page load transaction name to a testable value +Sentry.startBrowserTracingPageLoadSpan(client, { + name: 'test-route', + attributes: { + [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + }, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/subject.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/subject.js new file mode 100644 index 000000000000..ed6db5b5afe2 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/subject.js @@ -0,0 +1,18 @@ +const blockUI = (delay = 70) => e => { + const startTime = Date.now(); + + function getElasped() { + const time = Date.now(); + return time - startTime; + } + + while (getElasped() < delay) { + // + } + + e.target.classList.add('clicked'); +}; + +document.querySelector('[data-test-id=not-so-slow-button]').addEventListener('click', blockUI(300)); +document.querySelector('[data-test-id=slow-button]').addEventListener('click', blockUI(450)); +document.querySelector('[data-test-id=normal-button]').addEventListener('click', blockUI()); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/template.html b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/template.html new file mode 100644 index 000000000000..25c6920f07e2 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/template.html @@ -0,0 +1,12 @@ + + + + + + +
Rendered Before Long Task
+ + + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/test.ts new file mode 100644 index 000000000000..248cb7d1e510 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp-parametrized/test.ts @@ -0,0 +1,99 @@ +import { expect } from '@playwright/test'; +import type { SpanEnvelope } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { + getMultipleSentryEnvelopeRequests, + properFullEnvelopeRequestParser, + shouldSkipTracingTest, +} from '../../../../utils/helpers'; + +sentryTest( + 'should capture an INP click event span during pageload for a parametrized transaction', + async ({ browserName, getLocalTestPath, page }) => { + const supportedBrowsers = ['chromium']; + + if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) { + sentryTest.skip(); + } + + 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); + + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + await page.locator('[data-test-id=normal-button]').click(); + await page.locator('.clicked[data-test-id=normal-button]').isVisible(); + + await page.waitForTimeout(500); + + // Page hide to trigger INP + await page.evaluate(() => { + window.dispatchEvent(new Event('pagehide')); + }); + + // Get the INP span envelope + const spanEnvelope = (await spanEnvelopePromise)[0]; + + const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + const traceId = spanEnvelopeHeaders.trace!.trace_id; + expect(traceId).toMatch(/[a-f0-9]{32}/); + + expect(spanEnvelopeHeaders).toEqual({ + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'public', + sample_rate: '1', + sampled: 'true', + trace_id: traceId, + transaction: 'test-route', + }, + }); + + const inpValue = spanEnvelopeItem.measurements?.inp.value; + expect(inpValue).toBeGreaterThan(0); + + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.exclusive_time': inpValue, + 'sentry.op': 'ui.interaction.click', + 'sentry.origin': 'auto.http.browser.inp', + transaction: 'test-route', + }, + measurements: { + inp: { + unit: 'millisecond', + value: inpValue, + }, + }, + description: 'body > NormalButton', + exclusive_time: inpValue, + op: 'ui.interaction.click', + origin: 'auto.http.browser.inp', + segment_id: expect.not.stringMatching(spanEnvelopeItem.span_id!), + // parent is the pageload span + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: traceId, + }); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/init.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/init.js index b558562e4cd4..a941877ff88e 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/init.js +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/init.js @@ -6,10 +6,22 @@ Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', integrations: [ Sentry.browserTracingIntegration({ - idleTimeout: 1000, + idleTimeout: 4000, enableLongTask: false, enableInp: true, + instrumentPageLoad: false, + instrumentNavigation: false, }), ], tracesSampleRate: 1, }); + +const client = Sentry.getClient(); + +// Force page load transaction name to a testable value +Sentry.startBrowserTracingPageLoadSpan(client, { + name: 'test-url', + attributes: { + [Sentry.SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + }, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts index 582508f7a584..3f9684cf7f2a 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-inp/test.ts @@ -9,92 +9,95 @@ import { shouldSkipTracingTest, } from '../../../../utils/helpers'; -sentryTest('should capture an INP click event span.', async ({ browserName, getLocalTestPath, page }) => { - const supportedBrowsers = ['chromium']; - - if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) { - sentryTest.skip(); - } - - await page.route('https://dsn.ingest.sentry.io/**/*', route => { - return route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ id: 'test-id' }), - }); - }); +sentryTest( + 'should capture an INP click event span during pageload', + async ({ browserName, getLocalTestPath, page }) => { + const supportedBrowsers = ['chromium']; - const url = await getLocalTestPath({ testDir: __dirname }); + if (shouldSkipTracingTest() || !supportedBrowsers.includes(browserName)) { + sentryTest.skip(); + } - await page.goto(url); - await getFirstSentryEnvelopeRequest(page); // wait for page load + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); - const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( - page, - 1, - { envelopeType: 'span' }, - properFullEnvelopeRequestParser, - ); + const url = await getLocalTestPath({ testDir: __dirname }); - await page.locator('[data-test-id=normal-button]').click(); - await page.locator('.clicked[data-test-id=normal-button]').isVisible(); + await page.goto(url); - await page.waitForTimeout(500); + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); - // Page hide to trigger INP - await page.evaluate(() => { - window.dispatchEvent(new Event('pagehide')); - }); + await page.locator('[data-test-id=normal-button]').click(); + await page.locator('.clicked[data-test-id=normal-button]').isVisible(); + + await page.waitForTimeout(500); - // Get the INP span envelope - const spanEnvelope = (await spanEnvelopePromise)[0]; + // Page hide to trigger INP + await page.evaluate(() => { + window.dispatchEvent(new Event('pagehide')); + }); - const spanEnvelopeHeaders = spanEnvelope[0]; - const spanEnvelopeItem = spanEnvelope[1][0][1]; + // Get the INP span envelope + const spanEnvelope = (await spanEnvelopePromise)[0]; + + const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + const traceId = spanEnvelopeHeaders.trace!.trace_id; + expect(traceId).toMatch(/[a-f0-9]{32}/); + + expect(spanEnvelopeHeaders).toEqual({ + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'public', + sample_rate: '1', + sampled: 'true', + trace_id: traceId, + // no transaction, because span source is URL + }, + }); - const traceId = spanEnvelopeHeaders.trace!.trace_id; - expect(traceId).toMatch(/[a-f0-9]{32}/); + const inpValue = spanEnvelopeItem.measurements?.inp.value; + expect(inpValue).toBeGreaterThan(0); - expect(spanEnvelopeHeaders).toEqual({ - sent_at: expect.any(String), - trace: { - environment: 'production', - public_key: 'public', - sample_rate: '1', - sampled: 'true', - trace_id: traceId, - }, - }); - - const inpValue = spanEnvelopeItem.measurements?.inp.value; - expect(inpValue).toBeGreaterThan(0); - - expect(spanEnvelopeItem).toEqual({ - data: { - 'sentry.exclusive_time': inpValue, - 'sentry.op': 'ui.interaction.click', - 'sentry.origin': 'manual', - 'sentry.sample_rate': 1, - 'sentry.source': 'custom', - }, - measurements: { - inp: { - unit: 'millisecond', - value: inpValue, + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.exclusive_time': inpValue, + 'sentry.op': 'ui.interaction.click', + 'sentry.origin': 'auto.http.browser.inp', + transaction: 'test-url', + }, + measurements: { + inp: { + unit: 'millisecond', + value: inpValue, + }, }, - }, - description: 'body > NormalButton', - exclusive_time: inpValue, - op: 'ui.interaction.click', - origin: 'manual', - is_segment: true, - segment_id: spanEnvelopeItem.span_id, - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - trace_id: traceId, - }); -}); + description: 'body > NormalButton', + exclusive_time: inpValue, + op: 'ui.interaction.click', + origin: 'auto.http.browser.inp', + segment_id: expect.not.stringMatching(spanEnvelopeItem.span_id!), + // Parent is the pageload span + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + trace_id: traceId, + }); + }, +); sentryTest( 'should choose the slowest interaction click event when INP is triggered.', diff --git a/packages/browser-utils/src/metrics/inp.ts b/packages/browser-utils/src/metrics/inp.ts index c6c0113d6be3..232fe3ad39cb 100644 --- a/packages/browser-utils/src/metrics/inp.ts +++ b/packages/browser-utils/src/metrics/inp.ts @@ -2,6 +2,7 @@ import { SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME, SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT, SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, getActiveSpan, getClient, getCurrentScope, @@ -83,7 +84,9 @@ function _trackINP(): () => void { const activeSpan = getActiveSpan(); const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; - const routeName = rootSpan ? spanToJSON(rootSpan).description : undefined; + // If there is no active span, we fall back to look at the transactionName on the scope + // This is set if the pageload/navigation span is already finished, + const routeName = rootSpan ? spanToJSON(rootSpan).description : scope.getScopeData().transactionName; const user = scope.getUser(); // We need to get the replay, user, and activeTransaction from the current scope @@ -107,6 +110,7 @@ function _trackINP(): () => void { environment: options.environment, transaction: routeName, [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: metric.value, + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.inp', user: userDisplay || undefined, profile_id: profileId || undefined, replay_id: replayId || undefined, From 4a7cb6f1c98a0fe9c85bf52e34fa0bf772a613cd Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 6 Jun 2024 16:33:30 +0200 Subject: [PATCH 2/3] fix(browser): Handle more edge cases with INP (#12378) This is a PR adjusting https://github.com/getsentry/sentry-javascript/pull/12372 to keep transaction name for INP even if the route has changed in the meanwhile. Now, we keep a map of the last 10 interactionId <> route names, which we use instead (if possible). --- packages/browser-utils/src/index.ts | 1 + .../src/metrics/browserMetrics.ts | 2 +- packages/browser-utils/src/metrics/inp.ts | 63 +++++++++++++++++-- .../browser-utils/src/metrics/instrument.ts | 15 ++++- .../src/tracing/browserTracingIntegration.ts | 14 ++++- 5 files changed, 87 insertions(+), 8 deletions(-) diff --git a/packages/browser-utils/src/index.ts b/packages/browser-utils/src/index.ts index 7e7c4d0a387a..8e48e0988db9 100644 --- a/packages/browser-utils/src/index.ts +++ b/packages/browser-utils/src/index.ts @@ -12,6 +12,7 @@ export { startTrackingLongTasks, startTrackingWebVitals, startTrackingINP, + registerInpInteractionListener, } from './metrics/browserMetrics'; export { addClickKeypressInstrumentationHandler } from './instrument/dom'; diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts index 877b0612fb06..bb9969f1fde6 100644 --- a/packages/browser-utils/src/metrics/browserMetrics.ts +++ b/packages/browser-utils/src/metrics/browserMetrics.ts @@ -157,7 +157,7 @@ export function startTrackingInteractions(): void { }); } -export { startTrackingINP } from './inp'; +export { startTrackingINP, registerInpInteractionListener } from './inp'; /** Starts tracking the Cumulative Layout Shift on the current page. */ function _trackCLS(): () => void { diff --git a/packages/browser-utils/src/metrics/inp.ts b/packages/browser-utils/src/metrics/inp.ts index 232fe3ad39cb..00e524c048b6 100644 --- a/packages/browser-utils/src/metrics/inp.ts +++ b/packages/browser-utils/src/metrics/inp.ts @@ -12,9 +12,21 @@ import { } from '@sentry/core'; import type { Integration, SpanAttributes } from '@sentry/types'; import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString } from '@sentry/utils'; -import { addInpInstrumentationHandler } from './instrument'; +import { + addInpInstrumentationHandler, + addPerformanceInstrumentationHandler, + isPerformanceEventTiming, +} from './instrument'; import { getBrowserPerformanceAPI, msToSec } from './utils'; +// We only care about name here +interface PartialRouteInfo { + name: string | undefined; +} + +const LAST_INTERACTIONS: number[] = []; +const INTERACTIONS_ROUTE_MAP = new Map(); + /** * Start tracking INP webvital events. */ @@ -74,6 +86,7 @@ function _trackINP(): () => void { return; } + const { interactionId } = entry; const interactionType = INP_ENTRY_MAP[entry.name]; const options = client.getOptions(); @@ -84,9 +97,15 @@ function _trackINP(): () => void { const activeSpan = getActiveSpan(); const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; - // If there is no active span, we fall back to look at the transactionName on the scope - // This is set if the pageload/navigation span is already finished, - const routeName = rootSpan ? spanToJSON(rootSpan).description : scope.getScopeData().transactionName; + // We first try to lookup the route name from our INTERACTIONS_ROUTE_MAP, + // where we cache the route per interactionId + const cachedRouteName = interactionId != null ? INTERACTIONS_ROUTE_MAP.get(interactionId) : undefined; + + // Else, we try to use the active span. + // Finally, we fall back to look at the transactionName on the scope + const routeName = + cachedRouteName || (rootSpan ? spanToJSON(rootSpan).description : scope.getScopeData().transactionName); + const user = scope.getUser(); // We need to get the replay, user, and activeTransaction from the current scope @@ -134,3 +153,39 @@ function _trackINP(): () => void { span.end(startTime + duration); }); } + +/** Register a listener to cache route information for INP interactions. */ +export function registerInpInteractionListener(latestRoute: PartialRouteInfo): void { + const handleEntries = ({ entries }: { entries: PerformanceEntry[] }): void => { + entries.forEach(entry => { + if (!isPerformanceEventTiming(entry) || !latestRoute.name) { + return; + } + + const interactionId = entry.interactionId; + if (interactionId == null) { + return; + } + + // If the interaction was already recorded before, nothing more to do + if (INTERACTIONS_ROUTE_MAP.has(interactionId)) { + return; + } + + // We keep max. 10 interactions in the list, then remove the oldest one & clean up + if (LAST_INTERACTIONS.length > 10) { + const last = LAST_INTERACTIONS.shift() as number; + INTERACTIONS_ROUTE_MAP.delete(last); + } + + // We add the interaction to the list of recorded interactions + // and store the route information for this interaction + // (we clone the object because it is mutated when it changes) + LAST_INTERACTIONS.push(interactionId); + INTERACTIONS_ROUTE_MAP.set(interactionId, latestRoute.name); + }); + }; + + addPerformanceInstrumentationHandler('event', handleEntries); + addPerformanceInstrumentationHandler('first-input', handleEntries); +} diff --git a/packages/browser-utils/src/metrics/instrument.ts b/packages/browser-utils/src/metrics/instrument.ts index 06a27c4225ff..e22a345e3116 100644 --- a/packages/browser-utils/src/metrics/instrument.ts +++ b/packages/browser-utils/src/metrics/instrument.ts @@ -8,7 +8,13 @@ import { onLCP } from './web-vitals/getLCP'; import { observe } from './web-vitals/lib/observe'; import { onTTFB } from './web-vitals/onTTFB'; -type InstrumentHandlerTypePerformanceObserver = 'longtask' | 'event' | 'navigation' | 'paint' | 'resource'; +type InstrumentHandlerTypePerformanceObserver = + | 'longtask' + | 'event' + | 'navigation' + | 'paint' + | 'resource' + | 'first-input'; type InstrumentHandlerTypeMetric = 'cls' | 'lcp' | 'fid' | 'ttfb' | 'inp'; @@ -324,3 +330,10 @@ function getCleanupCallback( } }; } + +/** + * Check if a PerformanceEntry is a PerformanceEventTiming by checking for the `duration` property. + */ +export function isPerformanceEventTiming(entry: PerformanceEntry): entry is PerformanceEventTiming { + return 'duration' in entry; +} diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index f6528e4d155d..c058b1930928 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -2,6 +2,7 @@ import { addHistoryInstrumentationHandler, addPerformanceEntries, + registerInpInteractionListener, startTrackingINP, startTrackingInteractions, startTrackingLongTasks, @@ -40,6 +41,11 @@ import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from export const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing'; +interface RouteInfo { + name: string | undefined; + source: TransactionSource | undefined; +} + /** Options for Browser Tracing integration */ export interface BrowserTracingOptions { /** @@ -204,7 +210,7 @@ export const browserTracingIntegration = ((_options: Partial { From 615340a3cf28b35d5611946d88a9b6ed5da90fc1 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 7 Jun 2024 09:16:34 +0200 Subject: [PATCH 3/3] update size limits --- .size-limit.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.size-limit.js b/.size-limit.js index f9b62e7198e9..ac2b8591254a 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -15,14 +15,14 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration'), gzip: true, - limit: '34 KB', + limit: '35 KB', }, { name: '@sentry/browser (incl. Tracing, Replay)', path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration'), gzip: true, - limit: '71 KB', + limit: '72 KB', }, { name: '@sentry/browser (incl. Tracing, Replay) - with treeshaking flags', @@ -48,14 +48,14 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'replayCanvasIntegration'), gzip: true, - limit: '75 KB', + limit: '76 KB', }, { name: '@sentry/browser (incl. Tracing, Replay, Feedback)', path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'feedbackIntegration'), gzip: true, - limit: '87 KB', + limit: '89 KB', }, { name: '@sentry/browser (incl. Tracing, Replay, Feedback, metrics)', @@ -69,21 +69,21 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'metrics'), gzip: true, - limit: '40 KB', + limit: '30 KB', }, { name: '@sentry/browser (incl. Feedback)', path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'feedbackIntegration'), gzip: true, - limit: '40 KB', + limit: '41 KB', }, { name: '@sentry/browser (incl. sendFeedback)', path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'sendFeedback'), gzip: true, - limit: '28 KB', + limit: '29 KB', }, { name: '@sentry/browser (incl. FeedbackAsync)', @@ -107,7 +107,7 @@ module.exports = [ import: createImport('init', 'ErrorBoundary', 'reactRouterV6BrowserTracingIntegration'), ignore: ['react/jsx-runtime'], gzip: true, - limit: '37 KB', + limit: '38 KB', }, // Vue SDK (ESM) { @@ -143,7 +143,7 @@ module.exports = [ name: 'CDN Bundle (incl. Tracing)', path: createCDNPath('bundle.tracing.min.js'), gzip: true, - limit: '36 KB', + limit: '37 KB', }, { name: 'CDN Bundle (incl. Tracing, Replay)', @@ -193,7 +193,7 @@ module.exports = [ import: createImport('init'), ignore: ['next/router', 'next/constants'], gzip: true, - limit: '37 KB', + limit: '38 KB', }, // SvelteKit SDK (ESM) {