diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/assets/sentry-logo-600x179.png b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/assets/sentry-logo-600x179.png new file mode 100644 index 000000000000..353b7233d6bf Binary files /dev/null and b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/assets/sentry-logo-600x179.png differ diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/subject.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/subject.js new file mode 100644 index 000000000000..d0f8df871ee3 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/subject.js @@ -0,0 +1,18 @@ +import { addLcpInstrumentationHandler } from '@sentry-internal/tracing'; + +addLcpInstrumentationHandler(({ metric }) => { + const entry = metric.entries[metric.entries.length - 1]; + window._LCP = entry.size; +}); + +addLcpInstrumentationHandler(({ metric }) => { + const entry = metric.entries[metric.entries.length - 1]; + window._LCP2 = entry.size; +}); + +window.ADD_HANDLER = () => { + addLcpInstrumentationHandler(({ metric }) => { + const entry = metric.entries[metric.entries.length - 1]; + window._LCP3 = entry.size; + }); +}; diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/template.html b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/template.html new file mode 100644 index 000000000000..caf4b8f2deab --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/template.html @@ -0,0 +1,11 @@ + + + + + + +
+ + + + diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts new file mode 100644 index 000000000000..23cd29099a0f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts @@ -0,0 +1,51 @@ +import type { Route } from '@playwright/test'; +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; + +const bundle = process.env.PW_BUNDLE || ''; + +sentryTest( + 'should capture metrics for LCP instrumentation handlers', + async ({ browserName, getLocalTestPath, page }) => { + // This uses a utility that is not exported in CDN bundles + if (shouldSkipTracingTest() || browserName !== 'chromium' || bundle.startsWith('bundle')) { + sentryTest.skip(); + } + + await page.route('**/path/to/image.png', (route: Route) => + route.fulfill({ path: `${__dirname}/assets/sentry-logo-600x179.png` }), + ); + + const url = await getLocalTestPath({ testDir: __dirname }); + const [eventData] = await Promise.all([ + getFirstSentryEnvelopeRequest(page), + page.goto(url), + page.click('button'), + ]); + + expect(eventData.measurements).toBeDefined(); + expect(eventData.measurements?.lcp?.value).toBeDefined(); + + expect(eventData.tags?.['lcp.element']).toBe('body > img'); + expect(eventData.tags?.['lcp.size']).toBe(107400); + expect(eventData.tags?.['lcp.url']).toBe('https://example.com/path/to/image.png'); + + const lcp = await (await page.waitForFunction('window._LCP')).jsonValue(); + const lcp2 = await (await page.waitForFunction('window._LCP2')).jsonValue(); + const lcp3 = await page.evaluate('window._LCP3'); + + expect(lcp).toEqual(107400); + expect(lcp2).toEqual(107400); + // this has not been triggered yet + expect(lcp3).toEqual(undefined); + + // Adding a handler after LCP is completed still triggers the handler + await page.evaluate('window.ADD_HANDLER()'); + const lcp3_2 = await (await page.waitForFunction('window._LCP3')).jsonValue(); + + expect(lcp3_2).toEqual(107400); + }, +); diff --git a/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/fixtures/ReplayRecordingData.ts b/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/fixtures/ReplayRecordingData.ts index 698cb83b5fb4..0b454ba12214 100644 --- a/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/fixtures/ReplayRecordingData.ts +++ b/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/fixtures/ReplayRecordingData.ts @@ -215,7 +215,11 @@ export const ReplayRecordingData = [ description: 'largest-contentful-paint', startTimestamp: expect.any(Number), endTimestamp: expect.any(Number), - data: { value: expect.any(Number), size: expect.any(Number) }, + data: { + value: expect.any(Number), + size: expect.any(Number), + nodeId: 16, + }, }, }, }, diff --git a/dev-packages/e2e-tests/test-applications/react-router-6-use-routes/tests/fixtures/ReplayRecordingData.ts b/dev-packages/e2e-tests/test-applications/react-router-6-use-routes/tests/fixtures/ReplayRecordingData.ts index 698cb83b5fb4..0b454ba12214 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-6-use-routes/tests/fixtures/ReplayRecordingData.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-6-use-routes/tests/fixtures/ReplayRecordingData.ts @@ -215,7 +215,11 @@ export const ReplayRecordingData = [ description: 'largest-contentful-paint', startTimestamp: expect.any(Number), endTimestamp: expect.any(Number), - data: { value: expect.any(Number), size: expect.any(Number) }, + data: { + value: expect.any(Number), + size: expect.any(Number), + nodeId: 16, + }, }, }, }, diff --git a/dev-packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts b/dev-packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts index 698cb83b5fb4..0b454ba12214 100644 --- a/dev-packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts +++ b/dev-packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts @@ -215,7 +215,11 @@ export const ReplayRecordingData = [ description: 'largest-contentful-paint', startTimestamp: expect.any(Number), endTimestamp: expect.any(Number), - data: { value: expect.any(Number), size: expect.any(Number) }, + data: { + value: expect.any(Number), + size: expect.any(Number), + nodeId: 16, + }, }, }, }, diff --git a/packages/tracing-internal/src/browser/instrument.ts b/packages/tracing-internal/src/browser/instrument.ts index ae4532259f94..2a4e7acaf3b1 100644 --- a/packages/tracing-internal/src/browser/instrument.ts +++ b/packages/tracing-internal/src/browser/instrument.ts @@ -73,6 +73,8 @@ interface Metric { type InstrumentHandlerType = InstrumentHandlerTypeMetric | InstrumentHandlerTypePerformanceObserver; +type StopListening = undefined | void | (() => void); + // eslint-disable-next-line @typescript-eslint/no-explicit-any type InstrumentHandlerCallback = (data: any) => void; @@ -88,17 +90,29 @@ let _previousLcp: Metric | undefined; /** * Add a callback that will be triggered when a CLS metric is available. * Returns a cleanup callback which can be called to remove the instrumentation handler. + * + * Pass `stopOnCallback = true` to stop listening for CLS when the cleanup callback is called. + * This will lead to the CLS being finalized and frozen. */ -export function addClsInstrumentationHandler(callback: (data: { metric: Metric }) => void): CleanupHandlerCallback { - return addMetricObserver('cls', callback, instrumentCls, _previousCls); +export function addClsInstrumentationHandler( + callback: (data: { metric: Metric }) => void, + stopOnCallback = false, +): CleanupHandlerCallback { + return addMetricObserver('cls', callback, instrumentCls, _previousCls, stopOnCallback); } /** * Add a callback that will be triggered when a LCP metric is available. * Returns a cleanup callback which can be called to remove the instrumentation handler. + * + * Pass `stopOnCallback = true` to stop listening for LCP when the cleanup callback is called. + * This will lead to the LCP being finalized and frozen. */ -export function addLcpInstrumentationHandler(callback: (data: { metric: Metric }) => void): CleanupHandlerCallback { - return addMetricObserver('lcp', callback, instrumentLcp, _previousLcp); +export function addLcpInstrumentationHandler( + callback: (data: { metric: Metric }) => void, + stopOnCallback = false, +): CleanupHandlerCallback { + return addMetricObserver('lcp', callback, instrumentLcp, _previousLcp, stopOnCallback); } /** @@ -158,8 +172,8 @@ function triggerHandlers(type: InstrumentHandlerType, data: unknown): void { } } -function instrumentCls(): void { - onCLS(metric => { +function instrumentCls(): StopListening { + return onCLS(metric => { triggerHandlers('cls', { metric, }); @@ -168,7 +182,7 @@ function instrumentCls(): void { } function instrumentFid(): void { - onFID(metric => { + return onFID(metric => { triggerHandlers('fid', { metric, }); @@ -176,8 +190,8 @@ function instrumentFid(): void { }); } -function instrumentLcp(): void { - onLCP(metric => { +function instrumentLcp(): StopListening { + return onLCP(metric => { triggerHandlers('lcp', { metric, }); @@ -188,13 +202,16 @@ function instrumentLcp(): void { function addMetricObserver( type: InstrumentHandlerTypeMetric, callback: InstrumentHandlerCallback, - instrumentFn: () => void, + instrumentFn: () => StopListening, previousValue: Metric | undefined, + stopOnCallback = false, ): CleanupHandlerCallback { addHandler(type, callback); + let stopListening: StopListening | undefined; + if (!instrumented[type]) { - instrumentFn(); + stopListening = instrumentFn(); instrumented[type] = true; } @@ -202,7 +219,7 @@ function addMetricObserver( callback({ metric: previousValue }); } - return getCleanupCallback(type, callback); + return getCleanupCallback(type, callback, stopOnCallback ? stopListening : undefined); } function instrumentPerformanceObserver(type: InstrumentHandlerTypePerformanceObserver): void { @@ -228,8 +245,16 @@ function addHandler(type: InstrumentHandlerType, handler: InstrumentHandlerCallb } // Get a callback which can be called to remove the instrumentation handler -function getCleanupCallback(type: InstrumentHandlerType, callback: InstrumentHandlerCallback): CleanupHandlerCallback { +function getCleanupCallback( + type: InstrumentHandlerType, + callback: InstrumentHandlerCallback, + stopListening: StopListening, +): CleanupHandlerCallback { return () => { + if (stopListening) { + stopListening(); + } + const typeHandlers = handlers[type]; if (!typeHandlers) { diff --git a/packages/tracing-internal/src/browser/metrics/index.ts b/packages/tracing-internal/src/browser/metrics/index.ts index d0eb73b945d2..4c9c25111e11 100644 --- a/packages/tracing-internal/src/browser/metrics/index.ts +++ b/packages/tracing-internal/src/browser/metrics/index.ts @@ -39,7 +39,8 @@ let _lcpEntry: LargestContentfulPaint | undefined; let _clsEntry: LayoutShift | undefined; /** - * Start tracking web vitals + * Start tracking web vitals. + * The callback returned by this function can be used to stop tracking & ensure all measurements are final & captured. * * @returns A function that forces web vitals collection */ @@ -129,7 +130,7 @@ export function startTrackingInteractions(): void { /** Starts tracking the Cumulative Layout Shift on the current page. */ function _trackCLS(): () => void { return addClsInstrumentationHandler(({ metric }) => { - const entry = metric.entries.pop(); + const entry = metric.entries[metric.entries.length - 1]; if (!entry) { return; } @@ -137,13 +138,13 @@ function _trackCLS(): () => void { DEBUG_BUILD && logger.log('[Measurements] Adding CLS'); _measurements['cls'] = { value: metric.value, unit: '' }; _clsEntry = entry as LayoutShift; - }); + }, true); } /** Starts tracking the Largest Contentful Paint on the current page. */ function _trackLCP(): () => void { return addLcpInstrumentationHandler(({ metric }) => { - const entry = metric.entries.pop(); + const entry = metric.entries[metric.entries.length - 1]; if (!entry) { return; } @@ -151,13 +152,13 @@ function _trackLCP(): () => void { DEBUG_BUILD && logger.log('[Measurements] Adding LCP'); _measurements['lcp'] = { value: metric.value, unit: 'millisecond' }; _lcpEntry = entry as LargestContentfulPaint; - }); + }, true); } /** Starts tracking the First Input Delay on the current page. */ function _trackFID(): () => void { return addFidInstrumentationHandler(({ metric }) => { - const entry = metric.entries.pop(); + const entry = metric.entries[metric.entries.length - 1]; if (!entry) { return; }