From 2c4c631dd944f1841d377a671234e682e5432cec Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 19 Jul 2024 15:57:26 +0200 Subject: [PATCH] fix(browser): Set artificial zero-value CLS measurement to report no layout shift --- .../template.html | 11 +++++ .../web-vitals-cls-no-layout-shift/test.ts | 41 +++++++++++++++++++ .../src/metrics/browserMetrics.ts | 37 ++++++++++++++++- 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-no-layout-shift/template.html create mode 100644 dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-no-layout-shift/test.ts diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-no-layout-shift/template.html b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-no-layout-shift/template.html new file mode 100644 index 000000000000..4245edf74abb --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-no-layout-shift/template.html @@ -0,0 +1,11 @@ + + + + + + +
+ Some content +
+ + diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-no-layout-shift/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-no-layout-shift/test.ts new file mode 100644 index 000000000000..c1b83ce6e447 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-no-layout-shift/test.ts @@ -0,0 +1,41 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers'; + +sentryTest.beforeEach(async ({ page }) => { + await page.setViewportSize({ width: 800, height: 1200 }); +}); + +sentryTest('captures 0 CLS if the browser supports reporting CLS', async ({ getLocalTestPath, page, browserName }) => { + if (shouldSkipTracingTest() || browserName !== 'chromium') { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + const transactionEvent = await getFirstSentryEnvelopeRequest(page, url); + + expect(transactionEvent.measurements).toBeDefined(); + expect(transactionEvent.measurements?.cls?.value).toBe(0); + + // but no source entry (no source if there is no layout shift) + expect(transactionEvent.contexts?.trace?.data?.['cls.source.1']).toBeUndefined(); +}); + +sentryTest( + "doesn't capture 0 CLS if the browser doesn't support reporting CLS", + async ({ getLocalTestPath, page, browserName }) => { + if (shouldSkipTracingTest() || browserName === 'chromium') { + sentryTest.skip(); + } + + const url = await getLocalTestPath({ testDir: __dirname }); + const transactionEvent = await getFirstSentryEnvelopeRequest(page, `${url}#no-cls`); + + expect(transactionEvent.measurements).toBeDefined(); + expect(transactionEvent.measurements?.cls).toBeUndefined(); + + expect(transactionEvent.contexts?.trace?.data?.['cls.source.1']).toBeUndefined(); + }, +); diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts index 02c044322bd3..af51d038028e 100644 --- a/packages/browser-utils/src/metrics/browserMetrics.ts +++ b/packages/browser-utils/src/metrics/browserMetrics.ts @@ -2,7 +2,14 @@ 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, getComponentName, htmlTreeAsString, logger, parseUrl } from '@sentry/utils'; +import { + browserPerformanceTimeOrigin, + consoleSandbox, + getComponentName, + htmlTreeAsString, + logger, + parseUrl, +} from '@sentry/utils'; import { spanToJSON } from '@sentry/core'; import { DEBUG_BUILD } from '../debug-build'; @@ -215,6 +222,8 @@ export { startTrackingINP, registerInpInteractionListener } from './inp'; /** Starts tracking the Cumulative Layout Shift on the current page. */ function _trackCLS(): () => void { + trySetZeroClsValue(); + return addClsInstrumentationHandler(({ metric }) => { const entry = metric.entries[metric.entries.length - 1]; if (!entry) { @@ -227,6 +236,32 @@ function _trackCLS(): () => void { }, true); } +/** + * Why does this function exist? A very good question! + * + * The `PerformanceObserver` emits `LayoutShift` entries whenever a layout shift occurs. + * If none occurs (which is great!), the observer will never emit any entries. Makes sense so far! + * + * This is problematic for the Sentry product though. We can't differentiate between a CLS of 0 and not having received + * CLS data at all. So in both cases, we'd show users that the CLS score simply is not available. When in fact, it can + * be 0, which is a very good score. This function is a workaround to emit a CLS of 0 right at the start of + * listening to CLS events. This way, we can differentiate between a CLS of 0 and no CLS at all. If a layout shift + * occurs later, the real CLS value will be emitted and the 0 value will be ignored. + * We also only send this artificial 0 value if the browser supports reporting the `layout-shift` entry type. + */ +function trySetZeroClsValue(): void { + try { + const canReportLayoutShift = PerformanceObserver.supportedEntryTypes.includes('layout-shift'); + if (canReportLayoutShift) { + DEBUG_BUILD && logger.log('[Measurements] Adding CLS 0'); + _measurements['cls'] = { value: 0, unit: '' }; + // TODO: Do we have to set _clsEntry here as well? If so, what attribution should we give it? + } + } catch { + // catching and ignoring access errors for bundle size reduction + } +} + /** Starts tracking the Largest Contentful Paint on the current page. */ function _trackLCP(): () => void { return addLcpInstrumentationHandler(({ metric }) => {