fix(browser): Set artificial zero-value CLS measurement to report no …

…layout shift
getsentry · Jul 19, 2024 · 2c4c631 · 2c4c631
1 parent eb23dc4
commit 2c4c631
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 1 deletion.
diff --git a/...ser-integration-tests/suites/tracing/metrics/web-vitals-cls-no-layout-shift/template.html b/...ser-integration-tests/suites/tracing/metrics/web-vitals-cls-no-layout-shift/template.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+  </head>
+  <body>
+    <div id="content">
+      Some content
+    </div>
+  </body>
+</html>
diff --git a/...s/browser-integration-tests/suites/tracing/metrics/web-vitals-cls-no-layout-shift/test.ts b/...s/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<Event>(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<Event>(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
@@ -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 }) => {