From 5dd1f316fe72714429f00b84e6b0c70a2fa0d008 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 25 Jul 2024 18:42:37 +0200 Subject: [PATCH 01/13] feat(browser): Send CLS as standalone span (experimental) --- .../web-vitals-cls-standalone-spans/init.js | 17 + .../subject.js | 17 + .../template.html | 9 + .../web-vitals-cls-standalone-spans/test.ts | 378 ++++++++++++++++++ .../src/metrics/browserMetrics.ts | 139 ++++++- packages/browser-utils/src/metrics/inp.ts | 58 +-- .../browser-utils/src/metrics/instrument.ts | 2 +- packages/browser-utils/src/metrics/utils.ts | 90 ++++- .../src/metrics/web-vitals/README.md | 10 +- .../src/metrics/web-vitals/getCLS.ts | 20 + .../src/tracing/browserTracingIntegration.ts | 8 +- 11 files changed, 673 insertions(+), 75 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/init.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/init.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/init.js new file mode 100644 index 000000000000..32fbb07fbbae --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/init.js @@ -0,0 +1,17 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [ + Sentry.browserTracingIntegration({ + idleTimeout: 9000, + _experiments: { + enableStandaloneClsSpans: true, + }, + }), + ], + tracesSampleRate: 1, + debug: true, +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/subject.js b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/subject.js new file mode 100644 index 000000000000..ed1b9b790bb9 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/subject.js @@ -0,0 +1,17 @@ +import { simulateCLS } from '../../../../utils/web-vitals/cls.ts'; + +// Simulate Layout shift right at the beginning of the page load, depending on the URL hash +// don't run if expected CLS is NaN +const expectedCLS = Number(location.hash.slice(1)); +if (expectedCLS && expectedCLS >= 0) { + simulateCLS(expectedCLS).then(() => window.dispatchEvent(new Event('cls-done'))); +} + +// Simulate layout shift whenever the trigger-cls event is dispatched +// Cannot trigger cia a button click because expected layout shift after +// an interaction doesn't contribute to CLS. +window.addEventListener('trigger-cls', () => { + simulateCLS(0.1).then(() => { + window.dispatchEvent(new Event('cls-done')); + }); +}); diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/template.html b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/template.html new file mode 100644 index 000000000000..f525e6a94665 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/template.html @@ -0,0 +1,9 @@ + + + + + + +
+ + diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts new file mode 100644 index 000000000000..97cdd02f8236 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts @@ -0,0 +1,378 @@ +import type { Page } from '@playwright/test'; +import { expect } from '@playwright/test'; +import type { Event as SentryEvent, EventEnvelope, SpanEnvelope } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { + getFirstSentryEnvelopeRequest, + getMultipleSentryEnvelopeRequests, + properFullEnvelopeRequestParser, + shouldSkipTracingTest, +} from '../../../../utils/helpers'; + +sentryTest.beforeEach(async ({ browserName, page }) => { + if (shouldSkipTracingTest() || browserName !== 'chromium') { + sentryTest.skip(); + } + + await page.setViewportSize({ width: 800, height: 1200 }); +}); + +function waitForLayoutShift(page: Page): Promise { + return page.evaluate(() => { + return new Promise(resolve => { + window.addEventListener('cls-done', () => resolve()); + }); + }); +} + +function triggerAndWaitForLayoutShift(page: Page): Promise { + return page.evaluate(() => { + window.dispatchEvent(new CustomEvent('trigger-cls')); + return new Promise(resolve => { + window.addEventListener('cls-done', () => resolve()); + }); + }); +} + +function hidePage(page: Page): Promise { + return page.evaluate(() => { + window.dispatchEvent(new Event('pagehide')); + }); +} + +sentryTest('captures a "GOOD" CLS vital with its source as a standalone span', async ({ getLocalTestPath, page }) => { + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(`${url}#0.05`); + + await waitForLayoutShift(page); + + await hidePage(page); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + + const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.exclusive_time': 0, + 'sentry.op': 'ui.webvital.cls', + 'sentry.origin': 'auto.http.browser.cls', + transaction: expect.stringContaining('index.html'), + 'user_agent.original': expect.stringContaining('Chrome'), + }, + description: expect.stringContaining('body > div#content > p'), + exclusive_time: 0, + measurements: { + cls: { + unit: '', + value: expect.any(Number), // better check below, + }, + }, + op: 'ui.webvital.cls', + origin: 'auto.http.browser.cls', + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + segment_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: spanEnvelopeItem.start_timestamp, + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }); + + // Flakey value dependent on timings -> we check for a range + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.03); + expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.07); + + expect(spanEnvelopeHeaders).toEqual({ + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'public', + sample_rate: '1', + sampled: 'true', + trace_id: spanEnvelopeItem.trace_id, + // no transaction, because span source is URL + }, + }); +}); + +sentryTest('captures a "MEH" CLS vital with its source as a standalone span', async ({ getLocalTestPath, page }) => { + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(`${url}#0.21`); + + await waitForLayoutShift(page); + + // Page hide to trigger CLS emission + await page.evaluate(() => { + window.dispatchEvent(new Event('pagehide')); + }); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + + const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.exclusive_time': 0, + 'sentry.op': 'ui.webvital.cls', + 'sentry.origin': 'auto.http.browser.cls', + transaction: expect.stringContaining('index.html'), + 'user_agent.original': expect.stringContaining('Chrome'), + }, + description: expect.stringContaining('body > div#content > p'), + exclusive_time: 0, + measurements: { + cls: { + unit: '', + value: expect.any(Number), // better check below, + }, + }, + op: 'ui.webvital.cls', + origin: 'auto.http.browser.cls', + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + segment_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: spanEnvelopeItem.start_timestamp, + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }); + + // Flakey value dependent on timings -> we check for a range + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.18); + expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.23); + + expect(spanEnvelopeHeaders).toEqual({ + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'public', + sample_rate: '1', + sampled: 'true', + trace_id: spanEnvelopeItem.trace_id, + // no transaction, because span source is URL + }, + }); +}); + +sentryTest('captures a "POOR" CLS vital with its source as a standalone span.', async ({ getLocalTestPath, page }) => { + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(`${url}#0.35`); + + await waitForLayoutShift(page); + + // Page hide to trigger CLS emission + await hidePage(page); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + + const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.exclusive_time': 0, + 'sentry.op': 'ui.webvital.cls', + 'sentry.origin': 'auto.http.browser.cls', + transaction: expect.stringContaining('index.html'), + 'user_agent.original': expect.stringContaining('Chrome'), + }, + description: expect.stringContaining('body > div#content > p'), + exclusive_time: 0, + measurements: { + cls: { + unit: '', + value: expect.any(Number), // better check below, + }, + }, + op: 'ui.webvital.cls', + origin: 'auto.http.browser.cls', + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + segment_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: spanEnvelopeItem.start_timestamp, + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }); + + // Flakey value dependent on timings -> we check for a range + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.33); + expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.38); + + expect(spanEnvelopeHeaders).toEqual({ + sent_at: expect.any(String), + trace: { + environment: 'production', + public_key: 'public', + sample_rate: '1', + sampled: 'true', + trace_id: spanEnvelopeItem.trace_id, + // no transaction, because span source is URL + }, + }); +}); + +sentryTest( + 'captures CLS increases after the pageload span ended, when page is hidden', + async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.type).toBe('transaction'); + expect(eventData.contexts?.trace?.op).toBe('pageload'); + + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + await triggerAndWaitForLayoutShift(page); + + await hidePage(page); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + // Flakey value dependent on timings -> we check for a range + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.05); + expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.15); + }, +); + +sentryTest('sends CLS of the initial page when soft-navigating to a new page', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.type).toBe('transaction'); + expect(eventData.contexts?.trace?.op).toBe('pageload'); + + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + await triggerAndWaitForLayoutShift(page); + + await page.goto(`${url}#soft-navigation`); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + // Flakey value dependent on timings -> we check for a range + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.05); + expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.15); +}); + +sentryTest("doesn't send further CLS after the first navigation", async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.type).toBe('transaction'); + expect(eventData.contexts?.trace?.op).toBe('pageload'); + + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + await triggerAndWaitForLayoutShift(page); + + await page.goto(`${url}#soft-navigation`); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0); + + getMultipleSentryEnvelopeRequests(page, 1, { envelopeType: 'span' }, () => { + throw new Error('Unexpected span - This should not happen!'); + }); + + const navigationTxnPromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'transaction' }, + properFullEnvelopeRequestParser, + ); + + // activate both CLS emission triggers: + await page.goto(`${url}#soft-navigation-2`); + await hidePage(page); + + // assumption: If we would send another CLS span on the 2nd navigation, it would be sent before the navigation + // transaction ends. This isn't 100% safe to ensure we don't send something but otherwise we'd need to wait for + // a timeout or something similar. + await navigationTxnPromise; +}); + +sentryTest("doesn't send further CLS after the first page hide", async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.type).toBe('transaction'); + expect(eventData.contexts?.trace?.op).toBe('pageload'); + + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + await triggerAndWaitForLayoutShift(page); + + await hidePage(page); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0); + + getMultipleSentryEnvelopeRequests(page, 1, { envelopeType: 'span' }, () => { + throw new Error('Unexpected span - This should not happen!'); + }); + + const navigationTxnPromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'transaction' }, + properFullEnvelopeRequestParser, + ); + + // activate both CLS emission triggers: + await page.goto(`${url}#soft-navigation-2`); + await hidePage(page); + + // assumption: If we would send another CLS span on the 2nd navigation, it would be sent before the navigation + // transaction ends. This isn't 100% safe to ensure we don't send something but otherwise we'd need to wait for + // a timeout or something similar. + await navigationTxnPromise; +}); diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts index 43ea45dd4a08..92fe810e97eb 100644 --- a/packages/browser-utils/src/metrics/browserMetrics.ts +++ b/packages/browser-utils/src/metrics/browserMetrics.ts @@ -1,8 +1,25 @@ /* eslint-disable max-lines */ -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, getActiveSpan, startInactiveSpan } from '@sentry/core'; +import { + SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME, + SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT, + SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE, + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + getActiveSpan, + getClient, + getCurrentScope, + startInactiveSpan, +} from '@sentry/core'; import { setMeasurement } from '@sentry/core'; -import type { Measurements, Span, SpanAttributes, StartSpanOptions } from '@sentry/types'; -import { browserPerformanceTimeOrigin, getComponentName, htmlTreeAsString, logger, parseUrl } from '@sentry/utils'; +import type { Integration, Measurements, Span, SpanAttributes, StartSpanOptions } from '@sentry/types'; +import { + browserPerformanceTimeOrigin, + dropUndefinedKeys, + getComponentName, + htmlTreeAsString, + logger, + parseUrl, +} from '@sentry/utils'; import { spanToJSON } from '@sentry/core'; import { DEBUG_BUILD } from '../debug-build'; @@ -15,9 +32,17 @@ import { addPerformanceInstrumentationHandler, addTtfbInstrumentationHandler, } from './instrument'; -import { getBrowserPerformanceAPI, isMeasurementValue, msToSec, startAndEndSpan } from './utils'; +import { + getBrowserPerformanceAPI, + isMeasurementValue, + msToSec, + startAndEndSpan, + startStandaloneWebVitalSpan, +} from './utils'; import { getNavigationEntry } from './web-vitals/lib/getNavigationEntry'; import { getVisibilityWatcher } from './web-vitals/lib/getVisibilityWatcher'; +import { onFCP } from './web-vitals/onFCP'; +import type { Metric } from './web-vitals/types'; interface NavigatorNetworkInformation { readonly connection?: NetworkInformation; @@ -64,6 +89,9 @@ let _performanceCursor: number = 0; let _measurements: Measurements = {}; let _lcpEntry: LargestContentfulPaint | undefined; let _clsEntry: LayoutShift | undefined; +interface StartTrackingWebVitalsOptions { + recordClsStandaloneSpans: boolean; +} /** * Start tracking web vitals. @@ -71,23 +99,23 @@ let _clsEntry: LayoutShift | undefined; * * @returns A function that forces web vitals collection */ -export function startTrackingWebVitals(): () => void { +export function startTrackingWebVitals({ recordClsStandaloneSpans }: StartTrackingWebVitalsOptions): () => void { const performance = getBrowserPerformanceAPI(); if (performance && browserPerformanceTimeOrigin) { // @ts-expect-error we want to make sure all of these are available, even if TS is sure they are if (performance.mark) { WINDOW.performance.mark('sentry-tracing-init'); } - const fidCallback = _trackFID(); - const clsCallback = _trackCLS(); - const lcpCallback = _trackLCP(); - const ttfbCallback = _trackTtfb(); + const fidCleanupCallback = _trackFID(); + const lcpCleanupCallback = _trackLCP(); + const ttfbCleanupCallback = _trackTtfb(); + const clsCleanupCallback = _trackCLS(recordClsStandaloneSpans); return (): void => { - fidCallback(); - clsCallback(); - lcpCallback(); - ttfbCallback(); + fidCleanupCallback(); + lcpCleanupCallback(); + ttfbCleanupCallback(); + clsCleanupCallback(); }; } @@ -212,17 +240,76 @@ export function startTrackingInteractions(): void { export { startTrackingINP, registerInpInteractionListener } from './inp'; /** Starts tracking the Cumulative Layout Shift on the current page. */ -function _trackCLS(): () => void { - return addClsInstrumentationHandler(({ metric }) => { - const entry = metric.entries[metric.entries.length - 1]; +function _trackCLS(sendAsStandaloneSpan: boolean): () => void { + let _emittedFcp = false; + if (sendAsStandaloneSpan) { + onFCP( + () => { + _emittedFcp = true; + }, + { reportAllChanges: true }, + ); + } + + const cleanupClsCallback = addClsInstrumentationHandler(({ metric }) => { + const entry = metric.entries[metric.entries.length - 1] as LayoutShift | undefined; if (!entry) { return; } - DEBUG_BUILD && logger.log('[Measurements] Adding CLS'); - _measurements['cls'] = { value: metric.value, unit: '' }; - _clsEntry = entry as LayoutShift; + if (sendAsStandaloneSpan && _emittedFcp) { + sendStandaloneClsSpan(metric, entry); + // For now, we only emit once CLS span for the initial page load. + // Once we send this, we don't need to track CLS anymore. + setTimeout(() => { + cleanupClsCallback(); + }, 0); + } else if (!sendAsStandaloneSpan) { + DEBUG_BUILD && logger.log(`[Measurements] Adding CLS ${metric.value}`); + _measurements['cls'] = { value: metric.value, unit: '' }; + _clsEntry = entry as LayoutShift; + } }, true); + + return sendAsStandaloneSpan + ? () => { + /* cleanup is already taken care of when sending a standalone span, so we just return a noop */ + } + : cleanupClsCallback; +} + +function sendStandaloneClsSpan(metric: Metric, entry: LayoutShift) { + DEBUG_BUILD && logger.log(`Sending CLS span (${metric.value})`); + + const startTime = msToSec(browserPerformanceTimeOrigin as number) + entry.startTime; + const duration = msToSec(entry.duration); + const routeName = getCurrentScope().getScopeData().transactionName; + + // TODO: Is this fine / does it provide any value? Alternatively, we can + // - send the CLS source node as an attribute + // - do nothing at all and ignore the source node + const name = htmlTreeAsString(entry.sources[0]?.node); + + const attributes: SpanAttributes = dropUndefinedKeys({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls', + [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry.duration, + }); + + const span = startStandaloneWebVitalSpan({ + name, + transaction: routeName, + attributes, + startTime, + duration, + }); + + span?.addEvent('cls', { + [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT]: '', + [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE]: metric.value, + }); + + span?.end(startTime + duration); } /** Starts tracking the Largest Contentful Paint on the current page. */ @@ -267,8 +354,16 @@ function _trackTtfb(): () => void { }); } +interface AddPerformanceEntriesOptions { + /** + * Flag to determine if CLS should be recorded as a measurement on the span or + * sent as a standalone span instead. + */ + recordClsOnPageloadSpan: boolean; +} + /** Add performance related spans to a transaction */ -export function addPerformanceEntries(span: Span): void { +export function addPerformanceEntries(span: Span, options: AddPerformanceEntriesOptions): void { const performance = getBrowserPerformanceAPI(); if (!performance || !WINDOW.performance.getEntries || !browserPerformanceTimeOrigin) { // Gatekeeper if performance API not available @@ -286,7 +381,7 @@ export function addPerformanceEntries(span: Span): void { performanceEntries.slice(_performanceCursor).forEach((entry: Record) => { const startTime = msToSec(entry.startTime); const duration = msToSec( - // Inexplicibly, Chrome sometimes emits a negative duration. We need to work around this. + // Inexplicably, Chrome sometimes emits a negative duration. We need to work around this. // There is a SO post attempting to explain this, but it leaves one with open questions: https://stackoverflow.com/questions/23191918/peformance-getentries-and-negative-duration-display // The way we clamp the value is probably not accurate, since we have observed this happen for things that may take a while to load, like for example the replay worker. // TODO: Investigate why this happens and how to properly mitigate. For now, this is a workaround to prevent transactions being dropped due to negative duration spans. @@ -375,7 +470,7 @@ export function addPerformanceEntries(span: Span): void { // If FCP is not recorded we should not record the cls value // according to the new definition of CLS. - if (!('fcp' in _measurements)) { + if (!('fcp' in _measurements) || !options.recordClsOnPageloadSpan) { delete _measurements.cls; } diff --git a/packages/browser-utils/src/metrics/inp.ts b/packages/browser-utils/src/metrics/inp.ts index c4186a20f17e..07db7f0a6a1a 100644 --- a/packages/browser-utils/src/metrics/inp.ts +++ b/packages/browser-utils/src/metrics/inp.ts @@ -2,23 +2,21 @@ import { SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME, SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT, SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE, + SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, getActiveSpan, - getClient, getCurrentScope, getRootSpan, spanToJSON, - startInactiveSpan, } from '@sentry/core'; -import type { Integration, Span, SpanAttributes } from '@sentry/types'; +import type { Span, SpanAttributes } from '@sentry/types'; import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString } from '@sentry/utils'; -import { WINDOW } from '../types'; import { addInpInstrumentationHandler, addPerformanceInstrumentationHandler, isPerformanceEventTiming, } from './instrument'; -import { getBrowserPerformanceAPI, msToSec } from './utils'; +import { getBrowserPerformanceAPI, msToSec, startStandaloneWebVitalSpan } from './utils'; const LAST_INTERACTIONS: number[] = []; const INTERACTIONS_SPAN_MAP = new Map(); @@ -71,8 +69,7 @@ const INP_ENTRY_MAP: Record = { /** Starts tracking the Interaction to Next Paint on the current page. */ function _trackINP(): () => void { return addInpInstrumentationHandler(({ metric }) => { - const client = getClient(); - if (!client || metric.value == undefined) { + if (metric.value == undefined) { return; } @@ -85,11 +82,9 @@ function _trackINP(): () => void { const { interactionId } = entry; const interactionType = INP_ENTRY_MAP[entry.name]; - const options = client.getOptions(); /** Build the INP span, create an envelope from the span, and then send the envelope */ const startTime = msToSec((browserPerformanceTimeOrigin as number) + entry.startTime); const duration = msToSec(metric.value); - const scope = getCurrentScope(); const activeSpan = getActiveSpan(); const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; @@ -101,56 +96,29 @@ function _trackINP(): () => void { // Else, we try to use the active span. // Finally, we fall back to look at the transactionName on the scope - const routeName = spanToUse ? spanToJSON(spanToUse).description : scope.getScopeData().transactionName; - - const user = scope.getUser(); - - // 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 - const replay = client.getIntegrationByName string }>('Replay'); - - const replayId = replay && replay.getReplayId(); - - const userDisplay = user !== undefined ? user.email || user.id || user.ip_address : undefined; - let profileId: string | undefined = undefined; - try { - // @ts-expect-error skip optional chaining to save bundle size with try catch - profileId = scope.getScopeData().contexts.profile.profile_id; - } catch { - // do nothing - } + const routeName = spanToUse ? spanToJSON(spanToUse).description : getCurrentScope().getScopeData().transactionName; const name = htmlTreeAsString(entry.target); const attributes: SpanAttributes = dropUndefinedKeys({ - release: options.release, - 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, - // INP score calculation in the sentry backend relies on the user agent - // to account for different INP values being reported from different browsers - 'user_agent.original': WINDOW.navigator && WINDOW.navigator.userAgent, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `ui.interaction.${interactionType}`, + [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry.duration, }); - const span = startInactiveSpan({ + const span = startStandaloneWebVitalSpan({ name, - op: `ui.interaction.${interactionType}`, + transaction: routeName, attributes, - startTime: startTime, - experimental: { - standalone: true, - }, + startTime, + duration, }); - span.addEvent('inp', { + span?.addEvent('inp', { [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT]: 'millisecond', [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE]: metric.value, }); - span.end(startTime + duration); + span?.end(startTime + duration); }); } diff --git a/packages/browser-utils/src/metrics/instrument.ts b/packages/browser-utils/src/metrics/instrument.ts index 39292fb19b83..c10df607b6b1 100644 --- a/packages/browser-utils/src/metrics/instrument.ts +++ b/packages/browser-utils/src/metrics/instrument.ts @@ -228,7 +228,7 @@ function instrumentCls(): StopListening { }, // We want the callback to be called whenever the CLS value updates. // By default, the callback is only called when the tab goes to the background. - { reportAllChanges: true }, + { reportAllChanges: false }, ); } diff --git a/packages/browser-utils/src/metrics/utils.ts b/packages/browser-utils/src/metrics/utils.ts index d46cb2cfe35c..9ae3652eef07 100644 --- a/packages/browser-utils/src/metrics/utils.ts +++ b/packages/browser-utils/src/metrics/utils.ts @@ -1,6 +1,13 @@ import type { SentrySpan } from '@sentry/core'; -import { spanToJSON, startInactiveSpan, withActiveSpan } from '@sentry/core'; -import type { Span, SpanTimeInput, StartSpanOptions } from '@sentry/types'; +import { + SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME, + getClient, + getCurrentScope, + spanToJSON, + startInactiveSpan, + withActiveSpan, +} from '@sentry/core'; +import type { Integration, Span, SpanAttributes, SpanTimeInput, StartSpanOptions } from '@sentry/types'; import { WINDOW } from '../types'; /** @@ -44,6 +51,85 @@ export function startAndEndSpan( }); } +interface StandaloneWebVitalSpanOptions { + name: string; + transaction?: string; + attributes: SpanAttributes; + startTime: number; + duration: number; +} + +/** + * Starts an inactive, standalone span used to send web vital values to Sentry. + * DO NOT use this for arbitrary spans, as these spans require special handling + * during ingestion to extract metrics. + * + * This function adds a bunch of attributes and data to the span that's shared + * by all web vital standalone spans. However, you need to take care of adding + * the actual web vital value as an event to the span. Also, you need to assign + * a transaction name and some other values that are specific to the web vital. + * + * Ultimately, you also need to take care of ending the span to send it off. + * + * @param options + * + * @returns an inactive, standalone and NOT YET ended span + */ +export function startStandaloneWebVitalSpan(options: StandaloneWebVitalSpanOptions): Span | undefined { + const client = getClient(); + if (!client) { + return; + } + + const { name, transaction, attributes: passedAttributes, startTime, duration } = options; + + const { release, environment } = client.getOptions(); + // 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 + const replay = client.getIntegrationByName string }>('Replay'); + const replayId = replay && replay.getReplayId(); + + const scope = getCurrentScope(); + + const user = scope.getUser(); + const userDisplay = user !== undefined ? user.email || user.id || user.ip_address : undefined; + + let profileId: string | undefined = undefined; + try { + // @ts-expect-error skip optional chaining to save bundle size with try catch + profileId = scope.getScopeData().contexts.profile.profile_id; + } catch { + // do nothing + } + + const attributes: SpanAttributes = { + release, + environment, + + user: userDisplay || undefined, + profile_id: profileId || undefined, + replay_id: replayId || undefined, + + transaction, + + // Web vital score calculation relies on the user agent to account for different + // browsers setting different thresholds for what is considered a good/meh/bad value. + // For example: Chrome vs. Chrome Mobile + 'user_agent.original': WINDOW.navigator && WINDOW.navigator.userAgent, + + ...passedAttributes, + }; + + return startInactiveSpan({ + name, + attributes, + startTime, + experimental: { + standalone: true, + }, + }); +} + /** Get the browser performance API. */ export function getBrowserPerformanceAPI(): Performance | undefined { // @ts-expect-error we want to make sure all of these are available, even if TS is sure they are diff --git a/packages/browser-utils/src/metrics/web-vitals/README.md b/packages/browser-utils/src/metrics/web-vitals/README.md index 653ee22c7ff1..373813912075 100644 --- a/packages/browser-utils/src/metrics/web-vitals/README.md +++ b/packages/browser-utils/src/metrics/web-vitals/README.md @@ -18,8 +18,14 @@ Current vendored web vitals are: ## Notable Changes from web-vitals library This vendored web-vitals library is meant to be used in conjunction with the `@sentry/tracing` `BrowserTracing` -integration. As such, logic around `BFCache` and multiple reports were removed from the library as our web-vitals only -report once per pageload. +integration. As such, we made some changes to the original implementation: + +- logic around `BFCache` and multiple reports were removed from the library as our web-vitals only report once per + pageload. +- emission for CLS logic was changed to emit on page hide (as previously) but also, when the `_sentry_start_navigation` + event is emitted. + +Code we _added_ to the library is marged with a `// SENTRY-SPECIFIC-CHANGE` comment ## License diff --git a/packages/browser-utils/src/metrics/web-vitals/getCLS.ts b/packages/browser-utils/src/metrics/web-vitals/getCLS.ts index 380cf2e54d47..4e97d1b6c48b 100644 --- a/packages/browser-utils/src/metrics/web-vitals/getCLS.ts +++ b/packages/browser-utils/src/metrics/web-vitals/getCLS.ts @@ -14,6 +14,10 @@ * limitations under the License. */ +import { getClient } from '@sentry/core'; +import { logger } from '@sentry/utils'; +import { DEBUG_BUILD } from '../../debug-build'; +import { WINDOW } from '../../types'; import { bindReporter } from './lib/bindReporter'; import { initMetric } from './lib/initMetric'; import { observe } from './lib/observe'; @@ -45,6 +49,8 @@ export const CLSThresholds: MetricRatingThresholds = [0.1, 0.25]; * `callback` is always called when the page's visibility state changes to * hidden. As a result, the `callback` function might be called multiple times * during the same page load._ + * + * SENTRY-SPECIFIC-CHANGE: */ export const onCLS = (onReport: CLSReportCallback, opts: ReportOpts = {}): void => { // Start monitoring FCP so we can only report CLS if FCP is also reported. @@ -102,6 +108,20 @@ export const onCLS = (onReport: CLSReportCallback, opts: ReportOpts = {}): void report(true); }); + // SENTRY-SPECIFIC-CHANGE + // Add a listener to report CLS when requested. + // We need this to report CLS before starting a navigation transaction + // to only report the CLS value for the page prior to the the navigation. + // The web-vitals library does not differentiate between soft navigations + // in typical SPAs. So if we don't do this, we would report the CLS value + // for the entire lifespan of the SPA until it is first hidden, which + // potentially heavily skews CLS values of one page. + getClient()?.on('startNavigationSpan', () => { + handleEntries(po.takeRecords() as CLSMetric['entries']); + report(true); + }); + // END SENTRY-SPECIFIC-CHANGE + // Queue a task to report (if nothing else triggers a report first). // This allows CLS to be reported as soon as FCP fires when // `reportAllChanges` is true. diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts index 491c7aaae88d..ff5201878cff 100644 --- a/packages/browser/src/tracing/browserTracingIntegration.ts +++ b/packages/browser/src/tracing/browserTracingIntegration.ts @@ -146,6 +146,7 @@ export interface BrowserTracingOptions { */ _experiments: Partial<{ enableInteractions: boolean; + enableStandaloneClsSpans: boolean; }>; /** @@ -191,7 +192,7 @@ export const browserTracingIntegration = ((_options: Partial { _collectWebVitals(); - addPerformanceEntries(span); + addPerformanceEntries(span, { recordClsOnPageloadSpan: !enableStandaloneClsSpans }); }, }); @@ -298,6 +299,7 @@ export const browserTracingIntegration = ((_options: Partial Date: Thu, 25 Jul 2024 19:08:45 +0200 Subject: [PATCH 02/13] cleanup --- .../src/metrics/browserMetrics.ts | 20 +++++-------------- .../browser-utils/src/metrics/instrument.ts | 2 -- packages/browser-utils/src/metrics/utils.ts | 9 +-------- .../src/metrics/web-vitals/getCLS.ts | 5 ----- 4 files changed, 6 insertions(+), 30 deletions(-) diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts index 92fe810e97eb..bc69c8b6ddc8 100644 --- a/packages/browser-utils/src/metrics/browserMetrics.ts +++ b/packages/browser-utils/src/metrics/browserMetrics.ts @@ -6,12 +6,11 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, getActiveSpan, - getClient, getCurrentScope, startInactiveSpan, } from '@sentry/core'; import { setMeasurement } from '@sentry/core'; -import type { Integration, Measurements, Span, SpanAttributes, StartSpanOptions } from '@sentry/types'; +import type { Measurements, Span, SpanAttributes, StartSpanOptions } from '@sentry/types'; import { browserPerformanceTimeOrigin, dropUndefinedKeys, @@ -41,7 +40,6 @@ import { } from './utils'; import { getNavigationEntry } from './web-vitals/lib/getNavigationEntry'; import { getVisibilityWatcher } from './web-vitals/lib/getVisibilityWatcher'; -import { onFCP } from './web-vitals/onFCP'; import type { Metric } from './web-vitals/types'; interface NavigatorNetworkInformation { @@ -89,6 +87,7 @@ let _performanceCursor: number = 0; let _measurements: Measurements = {}; let _lcpEntry: LargestContentfulPaint | undefined; let _clsEntry: LayoutShift | undefined; + interface StartTrackingWebVitalsOptions { recordClsStandaloneSpans: boolean; } @@ -241,25 +240,15 @@ export { startTrackingINP, registerInpInteractionListener } from './inp'; /** Starts tracking the Cumulative Layout Shift on the current page. */ function _trackCLS(sendAsStandaloneSpan: boolean): () => void { - let _emittedFcp = false; - if (sendAsStandaloneSpan) { - onFCP( - () => { - _emittedFcp = true; - }, - { reportAllChanges: true }, - ); - } - const cleanupClsCallback = addClsInstrumentationHandler(({ metric }) => { const entry = metric.entries[metric.entries.length - 1] as LayoutShift | undefined; if (!entry) { return; } - if (sendAsStandaloneSpan && _emittedFcp) { + if (sendAsStandaloneSpan) { sendStandaloneClsSpan(metric, entry); - // For now, we only emit once CLS span for the initial page load. + // For now, we only emit one CLS span for the initial page load. // Once we send this, we don't need to track CLS anymore. setTimeout(() => { cleanupClsCallback(); @@ -470,6 +459,7 @@ export function addPerformanceEntries(span: Span, options: AddPerformanceEntries // If FCP is not recorded we should not record the cls value // according to the new definition of CLS. + // TODO: Check if the first condition is still necessary: `onCLS` already only fires once `onFCP` was called. if (!('fcp' in _measurements) || !options.recordClsOnPageloadSpan) { delete _measurements.cls; } diff --git a/packages/browser-utils/src/metrics/instrument.ts b/packages/browser-utils/src/metrics/instrument.ts index c10df607b6b1..8249c1ad8ba9 100644 --- a/packages/browser-utils/src/metrics/instrument.ts +++ b/packages/browser-utils/src/metrics/instrument.ts @@ -226,8 +226,6 @@ function instrumentCls(): StopListening { }); _previousCls = metric; }, - // We want the callback to be called whenever the CLS value updates. - // By default, the callback is only called when the tab goes to the background. { reportAllChanges: false }, ); } diff --git a/packages/browser-utils/src/metrics/utils.ts b/packages/browser-utils/src/metrics/utils.ts index 9ae3652eef07..17c537072b9f 100644 --- a/packages/browser-utils/src/metrics/utils.ts +++ b/packages/browser-utils/src/metrics/utils.ts @@ -1,12 +1,5 @@ import type { SentrySpan } from '@sentry/core'; -import { - SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME, - getClient, - getCurrentScope, - spanToJSON, - startInactiveSpan, - withActiveSpan, -} from '@sentry/core'; +import { getClient, getCurrentScope, spanToJSON, startInactiveSpan, withActiveSpan } from '@sentry/core'; import type { Integration, Span, SpanAttributes, SpanTimeInput, StartSpanOptions } from '@sentry/types'; import { WINDOW } from '../types'; diff --git a/packages/browser-utils/src/metrics/web-vitals/getCLS.ts b/packages/browser-utils/src/metrics/web-vitals/getCLS.ts index 4e97d1b6c48b..ef650b48a345 100644 --- a/packages/browser-utils/src/metrics/web-vitals/getCLS.ts +++ b/packages/browser-utils/src/metrics/web-vitals/getCLS.ts @@ -15,9 +15,6 @@ */ import { getClient } from '@sentry/core'; -import { logger } from '@sentry/utils'; -import { DEBUG_BUILD } from '../../debug-build'; -import { WINDOW } from '../../types'; import { bindReporter } from './lib/bindReporter'; import { initMetric } from './lib/initMetric'; import { observe } from './lib/observe'; @@ -49,8 +46,6 @@ export const CLSThresholds: MetricRatingThresholds = [0.1, 0.25]; * `callback` is always called when the page's visibility state changes to * hidden. As a result, the `callback` function might be called multiple times * during the same page load._ - * - * SENTRY-SPECIFIC-CHANGE: */ export const onCLS = (onReport: CLSReportCallback, opts: ReportOpts = {}): void => { // Start monitoring FCP so we can only report CLS if FCP is also reported. From 5322816314bbd96293ce8a4607f5de539e58a23f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 26 Jul 2024 09:38:49 +0200 Subject: [PATCH 03/13] use own perofrmance observer to decouple from replay --- .../src/metrics/browserMetrics.ts | 33 +++++++++++-------- .../browser-utils/src/metrics/instrument.ts | 4 ++- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts index bc69c8b6ddc8..6cbdd4087eea 100644 --- a/packages/browser-utils/src/metrics/browserMetrics.ts +++ b/packages/browser-utils/src/metrics/browserMetrics.ts @@ -38,6 +38,7 @@ import { startAndEndSpan, startStandaloneWebVitalSpan, } from './utils'; +import { onCLS } from './web-vitals/getCLS'; import { getNavigationEntry } from './web-vitals/lib/getNavigationEntry'; import { getVisibilityWatcher } from './web-vitals/lib/getVisibilityWatcher'; import type { Metric } from './web-vitals/types'; @@ -240,29 +241,35 @@ export { startTrackingINP, registerInpInteractionListener } from './inp'; /** Starts tracking the Cumulative Layout Shift on the current page. */ function _trackCLS(sendAsStandaloneSpan: boolean): () => void { + if (sendAsStandaloneSpan) { + let sentSpan = false; + onCLS(metric => { + if (sentSpan) { + return; + } + const entry = metric.entries[metric.entries.length - 1]; + if (!entry) { + return; + } + sendStandaloneClsSpan(metric, entry); + sentSpan = true; + }); + } + const cleanupClsCallback = addClsInstrumentationHandler(({ metric }) => { const entry = metric.entries[metric.entries.length - 1] as LayoutShift | undefined; if (!entry) { return; } - if (sendAsStandaloneSpan) { - sendStandaloneClsSpan(metric, entry); - // For now, we only emit one CLS span for the initial page load. - // Once we send this, we don't need to track CLS anymore. - setTimeout(() => { - cleanupClsCallback(); - }, 0); - } else if (!sendAsStandaloneSpan) { - DEBUG_BUILD && logger.log(`[Measurements] Adding CLS ${metric.value}`); - _measurements['cls'] = { value: metric.value, unit: '' }; - _clsEntry = entry as LayoutShift; - } + DEBUG_BUILD && logger.log(`[Measurements] Adding CLS ${metric.value}`); + _measurements['cls'] = { value: metric.value, unit: '' }; + _clsEntry = entry as LayoutShift; }, true); return sendAsStandaloneSpan ? () => { - /* cleanup is already taken care of when sending a standalone span, so we just return a noop */ + /* Cleanup for standalone span mode is handled in this function; returning a no-op */ } : cleanupClsCallback; } diff --git a/packages/browser-utils/src/metrics/instrument.ts b/packages/browser-utils/src/metrics/instrument.ts index 8249c1ad8ba9..39292fb19b83 100644 --- a/packages/browser-utils/src/metrics/instrument.ts +++ b/packages/browser-utils/src/metrics/instrument.ts @@ -226,7 +226,9 @@ function instrumentCls(): StopListening { }); _previousCls = metric; }, - { reportAllChanges: false }, + // We want the callback to be called whenever the CLS value updates. + // By default, the callback is only called when the tab goes to the background. + { reportAllChanges: true }, ); } From 53d848621c4c1af7c2ec4e88801410a776d7e6c4 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 26 Jul 2024 09:52:20 +0200 Subject: [PATCH 04/13] remove unused startStandaloneWebVitalSpan option --- packages/browser-utils/src/metrics/browserMetrics.ts | 1 - packages/browser-utils/src/metrics/inp.ts | 1 - packages/browser-utils/src/metrics/utils.ts | 3 +-- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts index 6cbdd4087eea..4088c15c33dd 100644 --- a/packages/browser-utils/src/metrics/browserMetrics.ts +++ b/packages/browser-utils/src/metrics/browserMetrics.ts @@ -297,7 +297,6 @@ function sendStandaloneClsSpan(metric: Metric, entry: LayoutShift) { transaction: routeName, attributes, startTime, - duration, }); span?.addEvent('cls', { diff --git a/packages/browser-utils/src/metrics/inp.ts b/packages/browser-utils/src/metrics/inp.ts index 07db7f0a6a1a..5814b139bd2d 100644 --- a/packages/browser-utils/src/metrics/inp.ts +++ b/packages/browser-utils/src/metrics/inp.ts @@ -110,7 +110,6 @@ function _trackINP(): () => void { transaction: routeName, attributes, startTime, - duration, }); span?.addEvent('inp', { diff --git a/packages/browser-utils/src/metrics/utils.ts b/packages/browser-utils/src/metrics/utils.ts index 17c537072b9f..5f9d0de4d4ab 100644 --- a/packages/browser-utils/src/metrics/utils.ts +++ b/packages/browser-utils/src/metrics/utils.ts @@ -49,7 +49,6 @@ interface StandaloneWebVitalSpanOptions { transaction?: string; attributes: SpanAttributes; startTime: number; - duration: number; } /** @@ -74,7 +73,7 @@ export function startStandaloneWebVitalSpan(options: StandaloneWebVitalSpanOptio return; } - const { name, transaction, attributes: passedAttributes, startTime, duration } = options; + const { name, transaction, attributes: passedAttributes, startTime } = options; const { release, environment } = client.getOptions(); // We need to get the replay, user, and activeTransaction from the current scope From 97ff097ba61df0e67665b08b5e282ad3617ecbe7 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 26 Jul 2024 11:21:12 +0200 Subject: [PATCH 05/13] use `reportAllChanges: true` - compatible approach; extract to new file --- .../src/metrics/browserMetrics.ts | 99 +++---------------- .../src/metrics/web-vitals/getCLS.ts | 14 --- 2 files changed, 12 insertions(+), 101 deletions(-) diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts index 4088c15c33dd..b3ade1e34c43 100644 --- a/packages/browser-utils/src/metrics/browserMetrics.ts +++ b/packages/browser-utils/src/metrics/browserMetrics.ts @@ -1,28 +1,13 @@ /* eslint-disable max-lines */ -import { - SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME, - SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT, - SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE, - SEMANTIC_ATTRIBUTE_SENTRY_OP, - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - getActiveSpan, - getCurrentScope, - startInactiveSpan, -} from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, getActiveSpan, startInactiveSpan } from '@sentry/core'; import { setMeasurement } from '@sentry/core'; import type { Measurements, Span, SpanAttributes, StartSpanOptions } from '@sentry/types'; -import { - browserPerformanceTimeOrigin, - dropUndefinedKeys, - getComponentName, - htmlTreeAsString, - logger, - parseUrl, -} from '@sentry/utils'; +import { browserPerformanceTimeOrigin, getComponentName, htmlTreeAsString, logger, parseUrl } from '@sentry/utils'; import { spanToJSON } from '@sentry/core'; import { DEBUG_BUILD } from '../debug-build'; import { WINDOW } from '../types'; +import { trackClsAsStandaloneSpan } from './cls'; import { type PerformanceLongAnimationFrameTiming, addClsInstrumentationHandler, @@ -31,17 +16,9 @@ import { addPerformanceInstrumentationHandler, addTtfbInstrumentationHandler, } from './instrument'; -import { - getBrowserPerformanceAPI, - isMeasurementValue, - msToSec, - startAndEndSpan, - startStandaloneWebVitalSpan, -} from './utils'; -import { onCLS } from './web-vitals/getCLS'; +import { getBrowserPerformanceAPI, isMeasurementValue, msToSec, startAndEndSpan } from './utils'; import { getNavigationEntry } from './web-vitals/lib/getNavigationEntry'; import { getVisibilityWatcher } from './web-vitals/lib/getVisibilityWatcher'; -import type { Metric } from './web-vitals/types'; interface NavigatorNetworkInformation { readonly connection?: NetworkInformation; @@ -109,7 +86,7 @@ export function startTrackingWebVitals({ recordClsStandaloneSpans }: StartTracki const fidCleanupCallback = _trackFID(); const lcpCleanupCallback = _trackLCP(); const ttfbCleanupCallback = _trackTtfb(); - const clsCleanupCallback = _trackCLS(recordClsStandaloneSpans); + const clsCleanupCallback = recordClsStandaloneSpans ? trackClsAsStandaloneSpan() : _trackCLS(); return (): void => { fidCleanupCallback(); @@ -239,72 +216,20 @@ export function startTrackingInteractions(): void { export { startTrackingINP, registerInpInteractionListener } from './inp'; -/** Starts tracking the Cumulative Layout Shift on the current page. */ -function _trackCLS(sendAsStandaloneSpan: boolean): () => void { - if (sendAsStandaloneSpan) { - let sentSpan = false; - onCLS(metric => { - if (sentSpan) { - return; - } - const entry = metric.entries[metric.entries.length - 1]; - if (!entry) { - return; - } - sendStandaloneClsSpan(metric, entry); - sentSpan = true; - }); - } - - const cleanupClsCallback = addClsInstrumentationHandler(({ metric }) => { +/** + * Starts tracking the Cumulative Layout Shift on the current page and collects the value and last entry + * to the `_measurements` object which ultimately is applied to the pageload span's measurements. + */ +function _trackCLS(): () => void { + return addClsInstrumentationHandler(({ metric }) => { const entry = metric.entries[metric.entries.length - 1] as LayoutShift | undefined; if (!entry) { return; } - DEBUG_BUILD && logger.log(`[Measurements] Adding CLS ${metric.value}`); _measurements['cls'] = { value: metric.value, unit: '' }; - _clsEntry = entry as LayoutShift; + _clsEntry = entry; }, true); - - return sendAsStandaloneSpan - ? () => { - /* Cleanup for standalone span mode is handled in this function; returning a no-op */ - } - : cleanupClsCallback; -} - -function sendStandaloneClsSpan(metric: Metric, entry: LayoutShift) { - DEBUG_BUILD && logger.log(`Sending CLS span (${metric.value})`); - - const startTime = msToSec(browserPerformanceTimeOrigin as number) + entry.startTime; - const duration = msToSec(entry.duration); - const routeName = getCurrentScope().getScopeData().transactionName; - - // TODO: Is this fine / does it provide any value? Alternatively, we can - // - send the CLS source node as an attribute - // - do nothing at all and ignore the source node - const name = htmlTreeAsString(entry.sources[0]?.node); - - const attributes: SpanAttributes = dropUndefinedKeys({ - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls', - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls', - [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry.duration, - }); - - const span = startStandaloneWebVitalSpan({ - name, - transaction: routeName, - attributes, - startTime, - }); - - span?.addEvent('cls', { - [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT]: '', - [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE]: metric.value, - }); - - span?.end(startTime + duration); } /** Starts tracking the Largest Contentful Paint on the current page. */ diff --git a/packages/browser-utils/src/metrics/web-vitals/getCLS.ts b/packages/browser-utils/src/metrics/web-vitals/getCLS.ts index ef650b48a345..c70cd8b1b9b0 100644 --- a/packages/browser-utils/src/metrics/web-vitals/getCLS.ts +++ b/packages/browser-utils/src/metrics/web-vitals/getCLS.ts @@ -103,20 +103,6 @@ export const onCLS = (onReport: CLSReportCallback, opts: ReportOpts = {}): void report(true); }); - // SENTRY-SPECIFIC-CHANGE - // Add a listener to report CLS when requested. - // We need this to report CLS before starting a navigation transaction - // to only report the CLS value for the page prior to the the navigation. - // The web-vitals library does not differentiate between soft navigations - // in typical SPAs. So if we don't do this, we would report the CLS value - // for the entire lifespan of the SPA until it is first hidden, which - // potentially heavily skews CLS values of one page. - getClient()?.on('startNavigationSpan', () => { - handleEntries(po.takeRecords() as CLSMetric['entries']); - report(true); - }); - // END SENTRY-SPECIFIC-CHANGE - // Queue a task to report (if nothing else triggers a report first). // This allows CLS to be reported as soon as FCP fires when // `reportAllChanges` is true. From be31dc12a8cf1d62f89c51d86724a3bac396ba7c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 26 Jul 2024 11:22:43 +0200 Subject: [PATCH 06/13] biome --- packages/browser-utils/src/metrics/web-vitals/getCLS.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/browser-utils/src/metrics/web-vitals/getCLS.ts b/packages/browser-utils/src/metrics/web-vitals/getCLS.ts index c70cd8b1b9b0..380cf2e54d47 100644 --- a/packages/browser-utils/src/metrics/web-vitals/getCLS.ts +++ b/packages/browser-utils/src/metrics/web-vitals/getCLS.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { getClient } from '@sentry/core'; import { bindReporter } from './lib/bindReporter'; import { initMetric } from './lib/initMetric'; import { observe } from './lib/observe'; From 98ab3bc3a235b9bb5293e6de68604db1ac755ec3 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 26 Jul 2024 11:25:44 +0200 Subject: [PATCH 07/13] remove unnecessary readme section --- .../browser-utils/src/metrics/web-vitals/README.md | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/browser-utils/src/metrics/web-vitals/README.md b/packages/browser-utils/src/metrics/web-vitals/README.md index 373813912075..d779969dbe5d 100644 --- a/packages/browser-utils/src/metrics/web-vitals/README.md +++ b/packages/browser-utils/src/metrics/web-vitals/README.md @@ -17,15 +17,9 @@ Current vendored web vitals are: ## Notable Changes from web-vitals library -This vendored web-vitals library is meant to be used in conjunction with the `@sentry/tracing` `BrowserTracing` -integration. As such, we made some changes to the original implementation: - -- logic around `BFCache` and multiple reports were removed from the library as our web-vitals only report once per - pageload. -- emission for CLS logic was changed to emit on page hide (as previously) but also, when the `_sentry_start_navigation` - event is emitted. - -Code we _added_ to the library is marged with a `// SENTRY-SPECIFIC-CHANGE` comment +This vendored web-vitals library is meant to be used in conjunction with the `@sentry/browser` +`browserTracingIntegration`. As such, logic around `BFCache` and multiple reports were removed from the library as our +web-vitals only report once per pageload. ## License From 7103fb240750bb05c78a8da67962943c4b122ac3 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 26 Jul 2024 11:34:33 +0200 Subject: [PATCH 08/13] add test for 0-CLS value --- .../template.html | 3 + .../web-vitals-cls-standalone-spans/test.ts | 62 +++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/template.html b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/template.html index f525e6a94665..487683893a7f 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/template.html +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/template.html @@ -5,5 +5,8 @@
+

+ Some content +

diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts index 97cdd02f8236..f71aea79c84b 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts @@ -234,6 +234,68 @@ sentryTest('captures a "POOR" CLS vital with its source as a standalone span.', }); }); +sentryTest( + 'captures a 0 CLS vital as a standalone span if no layout shift occurred', + async ({ getLocalTestPath, page }) => { + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( + page, + 1, + { envelopeType: 'span' }, + properFullEnvelopeRequestParser, + ); + + const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(url); + + await page.waitForTimeout(1000); + + await hidePage(page); + + const spanEnvelope = (await spanEnvelopePromise)[0]; + + const spanEnvelopeHeaders = spanEnvelope[0]; + const spanEnvelopeItem = spanEnvelope[1][0][1]; + + expect(spanEnvelopeItem).toEqual({ + data: { + 'sentry.exclusive_time': 0, + 'sentry.op': 'ui.webvital.cls', + 'sentry.origin': 'auto.http.browser.cls', + transaction: expect.stringContaining('index.html'), + 'user_agent.original': expect.stringContaining('Chrome'), + }, + description: 'Layout shift', + exclusive_time: 0, + measurements: { + cls: { + unit: '', + value: 0, + }, + }, + op: 'ui.webvital.cls', + origin: 'auto.http.browser.cls', + parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), + span_id: expect.stringMatching(/[a-f0-9]{16}/), + segment_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + timestamp: spanEnvelopeItem.start_timestamp, + trace_id: expect.stringMatching(/[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: spanEnvelopeItem.trace_id, + // no transaction, because span source is URL + }, + }); + }, +); + sentryTest( 'captures CLS increases after the pageload span ended, when page is hidden', async ({ getLocalTestPath, page }) => { From b7eac3e33d3e9f3cfad799e9635abc17afe97e59 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 26 Jul 2024 11:39:14 +0200 Subject: [PATCH 09/13] add new cls file, oops --- packages/browser-utils/src/metrics/cls.ts | 114 ++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 packages/browser-utils/src/metrics/cls.ts diff --git a/packages/browser-utils/src/metrics/cls.ts b/packages/browser-utils/src/metrics/cls.ts new file mode 100644 index 000000000000..c93705c86b1f --- /dev/null +++ b/packages/browser-utils/src/metrics/cls.ts @@ -0,0 +1,114 @@ +import { + SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME, + SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT, + SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE, + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + getClient, + getCurrentScope, +} from '@sentry/core'; +import type { SpanAttributes } from '@sentry/types'; +import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString, logger } from '@sentry/utils'; +import { DEBUG_BUILD } from '../debug-build'; +import { addClsInstrumentationHandler } from './instrument'; +import { msToSec, startStandaloneWebVitalSpan } from './utils'; +import { onHidden } from './web-vitals/lib/onHidden'; + +/** + * Starts tracking the Cumulative Layout Shift on the current page and collects the value once + * + * - the page visibility is hidden + * - a navigation span is started (to stop CLS measurement for SPA soft navigations) + * + * Once either of these events triggers, the CLS value is sent as a standalone span and we stop + * measuring CLS. + */ +export function trackClsAsStandaloneSpan(): () => void { + let standaloneCLsValue = 0; + let standaloneClsEntry: LayoutShift | undefined; + + // Cleanup for standalone span mode is handled in this function. + // Returning a no-op for API compatibility with `_trackCLS` measurement mode (saves some bytes) + const cleanupNoop = () => undefined; + + if (!supportsLayoutShift()) { + return cleanupNoop; + } + + let sentSpan = false; + function _collectClsOnce() { + if (sentSpan) { + return; + } + sendStandaloneClsSpan(standaloneCLsValue, standaloneClsEntry); + cleanupClsHandler(); + sentSpan = true; + } + + const cleanupClsHandler = addClsInstrumentationHandler(({ metric }) => { + const entry = metric.entries[metric.entries.length - 1] as LayoutShift | undefined; + if (!entry) { + return; + } + standaloneCLsValue = metric.value; + standaloneClsEntry = entry; + }, true); + + // use pagehide event from web-vitals + onHidden(() => { + _collectClsOnce(); + }); + + // Since the call chain of this function is synchronous and evaluates before the SDK client is created, + // we need to wait with subscribing to a client hook until the client is created. Therefore, we defer + // to the next tick after the SDK setup. + setTimeout(() => { + const unsubscribe = getClient()?.on('startNavigationSpan', () => { + _collectClsOnce(); + typeof unsubscribe === 'function' && unsubscribe(); + }); + }, 0); + + return cleanupNoop; +} + +function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined) { + DEBUG_BUILD && logger.log(`Sending CLS span (${clsValue})`); + + const startTime = msToSec(browserPerformanceTimeOrigin as number) + (entry?.startTime || 0); + const duration = msToSec(entry?.duration || 0); + const routeName = getCurrentScope().getScopeData().transactionName; + + // TODO: Is this fine / does it provide any value? Alternatively, we can + // - send the CLS source node as an attribute + // - do nothing at all and ignore the source node + const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift'; + + const attributes: SpanAttributes = dropUndefinedKeys({ + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls', + [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0, + }); + + const span = startStandaloneWebVitalSpan({ + name, + transaction: routeName, + attributes, + startTime, + }); + + span?.addEvent('cls', { + [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_UNIT]: '', + [SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE]: clsValue, + }); + + span?.end(startTime + duration); +} + +function supportsLayoutShift(): boolean { + try { + return PerformanceObserver.supportedEntryTypes?.includes('layout-shift'); + } catch { + return false; + } +} From 8494bc4212b18900bc22350527a11e582b571f52 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 26 Jul 2024 11:51:54 +0200 Subject: [PATCH 10/13] save a couple of bytes --- packages/browser-utils/src/metrics/browserMetrics.ts | 2 +- packages/browser-utils/src/metrics/cls.ts | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts index b3ade1e34c43..b71f80df1ff2 100644 --- a/packages/browser-utils/src/metrics/browserMetrics.ts +++ b/packages/browser-utils/src/metrics/browserMetrics.ts @@ -92,7 +92,7 @@ export function startTrackingWebVitals({ recordClsStandaloneSpans }: StartTracki fidCleanupCallback(); lcpCleanupCallback(); ttfbCleanupCallback(); - clsCleanupCallback(); + clsCleanupCallback && clsCleanupCallback(); }; } diff --git a/packages/browser-utils/src/metrics/cls.ts b/packages/browser-utils/src/metrics/cls.ts index c93705c86b1f..740d5a2735ac 100644 --- a/packages/browser-utils/src/metrics/cls.ts +++ b/packages/browser-utils/src/metrics/cls.ts @@ -23,16 +23,12 @@ import { onHidden } from './web-vitals/lib/onHidden'; * Once either of these events triggers, the CLS value is sent as a standalone span and we stop * measuring CLS. */ -export function trackClsAsStandaloneSpan(): () => void { +export function trackClsAsStandaloneSpan(): void { let standaloneCLsValue = 0; let standaloneClsEntry: LayoutShift | undefined; - // Cleanup for standalone span mode is handled in this function. - // Returning a no-op for API compatibility with `_trackCLS` measurement mode (saves some bytes) - const cleanupNoop = () => undefined; - if (!supportsLayoutShift()) { - return cleanupNoop; + return; } let sentSpan = false; @@ -68,8 +64,6 @@ export function trackClsAsStandaloneSpan(): () => void { typeof unsubscribe === 'function' && unsubscribe(); }); }, 0); - - return cleanupNoop; } function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined) { From 0d4ad0a574b61bc4d5a3549c494d3a4dd818a6d0 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 9 Aug 2024 18:11:06 +0200 Subject: [PATCH 11/13] add pageload span id --- .../web-vitals-cls-standalone-spans/test.ts | 15 +++++++++++ packages/browser-utils/src/metrics/cls.ts | 27 +++++++++++++++---- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts index f71aea79c84b..cdf1e6837ef4 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-standalone-spans/test.ts @@ -68,6 +68,7 @@ sentryTest('captures a "GOOD" CLS vital with its source as a standalone span', a 'sentry.origin': 'auto.http.browser.cls', transaction: expect.stringContaining('index.html'), 'user_agent.original': expect.stringContaining('Chrome'), + 'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/), }, description: expect.stringContaining('body > div#content > p'), exclusive_time: 0, @@ -134,6 +135,7 @@ sentryTest('captures a "MEH" CLS vital with its source as a standalone span', as 'sentry.origin': 'auto.http.browser.cls', transaction: expect.stringContaining('index.html'), 'user_agent.original': expect.stringContaining('Chrome'), + 'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/), }, description: expect.stringContaining('body > div#content > p'), exclusive_time: 0, @@ -198,6 +200,7 @@ sentryTest('captures a "POOR" CLS vital with its source as a standalone span.', 'sentry.origin': 'auto.http.browser.cls', transaction: expect.stringContaining('index.html'), 'user_agent.original': expect.stringContaining('Chrome'), + 'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/), }, description: expect.stringContaining('body > div#content > p'), exclusive_time: 0, @@ -263,6 +266,7 @@ sentryTest( 'sentry.origin': 'auto.http.browser.cls', transaction: expect.stringContaining('index.html'), 'user_agent.original': expect.stringContaining('Chrome'), + 'sentry.pageload.span_id': expect.stringMatching(/[a-f0-9]{16}/), }, description: 'Layout shift', exclusive_time: 0, @@ -306,6 +310,12 @@ sentryTest( expect(eventData.type).toBe('transaction'); expect(eventData.contexts?.trace?.op).toBe('pageload'); + const pageloadSpanId = eventData.contexts?.trace?.span_id; + const pageloadTraceId = eventData.contexts?.trace?.trace_id; + + expect(pageloadSpanId).toMatch(/[a-f0-9]{16}/); + expect(pageloadTraceId).toMatch(/[a-f0-9]{32}/); + const spanEnvelopePromise = getMultipleSentryEnvelopeRequests( page, 1, @@ -322,6 +332,10 @@ sentryTest( // Flakey value dependent on timings -> we check for a range expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.05); expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.15); + + // Ensure the CLS span is connected to the pageload span and trace + expect(spanEnvelopeItem.data?.['sentry.pageload.span_id']).toBe(pageloadSpanId); + expect(spanEnvelopeItem.trace_id).toEqual(pageloadTraceId); }, ); @@ -349,6 +363,7 @@ sentryTest('sends CLS of the initial page when soft-navigating to a new page', a // Flakey value dependent on timings -> we check for a range expect(spanEnvelopeItem.measurements?.cls?.value).toBeGreaterThan(0.05); expect(spanEnvelopeItem.measurements?.cls?.value).toBeLessThan(0.15); + expect(spanEnvelopeItem.data?.['sentry.pageload.span_id']).toMatch(/[a-f0-9]{16}/); }); sentryTest("doesn't send further CLS after the first navigation", async ({ getLocalTestPath, page }) => { diff --git a/packages/browser-utils/src/metrics/cls.ts b/packages/browser-utils/src/metrics/cls.ts index 740d5a2735ac..7227358486f4 100644 --- a/packages/browser-utils/src/metrics/cls.ts +++ b/packages/browser-utils/src/metrics/cls.ts @@ -4,8 +4,11 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + getActiveSpan, getClient, getCurrentScope, + getRootSpan, + spanToJSON, } from '@sentry/core'; import type { SpanAttributes } from '@sentry/types'; import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString, logger } from '@sentry/utils'; @@ -26,6 +29,7 @@ import { onHidden } from './web-vitals/lib/onHidden'; export function trackClsAsStandaloneSpan(): void { let standaloneCLsValue = 0; let standaloneClsEntry: LayoutShift | undefined; + let pageloadSpanId: string | undefined; if (!supportsLayoutShift()) { return; @@ -36,9 +40,11 @@ export function trackClsAsStandaloneSpan(): void { if (sentSpan) { return; } - sendStandaloneClsSpan(standaloneCLsValue, standaloneClsEntry); - cleanupClsHandler(); sentSpan = true; + if (pageloadSpanId) { + sendStandaloneClsSpan(standaloneCLsValue, standaloneClsEntry, pageloadSpanId); + } + cleanupClsHandler(); } const cleanupClsHandler = addClsInstrumentationHandler(({ metric }) => { @@ -59,14 +65,23 @@ export function trackClsAsStandaloneSpan(): void { // we need to wait with subscribing to a client hook until the client is created. Therefore, we defer // to the next tick after the SDK setup. setTimeout(() => { - const unsubscribe = getClient()?.on('startNavigationSpan', () => { + const client = getClient(); + + const unsubscribeStartNavigation = client?.on('startNavigationSpan', () => { _collectClsOnce(); - typeof unsubscribe === 'function' && unsubscribe(); + unsubscribeStartNavigation && unsubscribeStartNavigation(); }); + + const activeSpan = getActiveSpan(); + const rootSpan = activeSpan && getRootSpan(activeSpan); + const spanJSON = rootSpan && spanToJSON(rootSpan); + if (spanJSON && spanJSON.op === 'pageload') { + pageloadSpanId = rootSpan.spanContext().spanId; + } }, 0); } -function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined) { +function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined, pageloadSpanId: string) { DEBUG_BUILD && logger.log(`Sending CLS span (${clsValue})`); const startTime = msToSec(browserPerformanceTimeOrigin as number) + (entry?.startTime || 0); @@ -82,6 +97,8 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined) [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls', [SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0, + // attach the pageload span id to the CLS span so that we can link them in the UI + 'sentry.pageload.span_id': pageloadSpanId, }); const span = startStandaloneWebVitalSpan({ From 28a3028fe26e8b11e00bfbb423dd8869235b1bf8 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 9 Aug 2024 18:14:45 +0200 Subject: [PATCH 12/13] remove todo --- packages/browser-utils/src/metrics/cls.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/browser-utils/src/metrics/cls.ts b/packages/browser-utils/src/metrics/cls.ts index 7227358486f4..aa25a54754a1 100644 --- a/packages/browser-utils/src/metrics/cls.ts +++ b/packages/browser-utils/src/metrics/cls.ts @@ -88,9 +88,6 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined, const duration = msToSec(entry?.duration || 0); const routeName = getCurrentScope().getScopeData().transactionName; - // TODO: Is this fine / does it provide any value? Alternatively, we can - // - send the CLS source node as an attribute - // - do nothing at all and ignore the source node const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift'; const attributes: SpanAttributes = dropUndefinedKeys({ From 585a7ac7b248a42097ecef9e1775df049a0ddc42 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 13 Aug 2024 09:36:53 +0200 Subject: [PATCH 13/13] adjust size-limit --- .size-limit.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.size-limit.js b/.size-limit.js index 80aa4c5095ea..859ce741cc3d 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -55,7 +55,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init', 'browserTracingIntegration', 'replayIntegration', 'feedbackIntegration'), gzip: true, - limit: '90 KB', + limit: '91 KB', }, { name: '@sentry/browser (incl. Tracing, Replay, Feedback, metrics)', @@ -143,7 +143,7 @@ module.exports = [ name: 'CDN Bundle (incl. Tracing)', path: createCDNPath('bundle.tracing.min.js'), gzip: true, - limit: '37 KB', + limit: '38 KB', }, { name: 'CDN Bundle (incl. Tracing, Replay)', @@ -170,7 +170,7 @@ module.exports = [ path: createCDNPath('bundle.tracing.min.js'), gzip: false, brotli: false, - limit: '110 KB', + limit: '111 KB', }, { name: 'CDN Bundle (incl. Tracing, Replay) - uncompressed', @@ -193,7 +193,7 @@ module.exports = [ import: createImport('init'), ignore: ['next/router', 'next/constants'], gzip: true, - limit: '38.05 KB', + limit: '39 KB', }, // SvelteKit SDK (ESM) {