fix(browser): Make sure measure spans have valid start timestamps (#12648)

Co-authored-by: Francesco Novy <francesco.novy@sentry.io>

Author: AbhiPrasad

dev-packages/browser-integration-tests/suites/tracing/metrics/pageload-measure-spans/init.js

@@ -0,0 +1,21 @@
+// Add measure before SDK initializes
+const end = performance.now();
+performance.measure('Next.js-before-hydration', {
+  duration: 1000,
+  end,
+});
+
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  debug: true,
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  integrations: [
+    Sentry.browserTracingIntegration({
+      idleTimeout: 9000,
+    }),
+  ],
+  tracesSampleRate: 1,
+});

dev-packages/browser-integration-tests/suites/tracing/metrics/pageload-measure-spans/test.ts

@@ -0,0 +1,35 @@
+import { expect } from '@playwright/test';
+import type { Event } from '@sentry/types';
+
+import { sentryTest } from '../../../../utils/fixtures';
+import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';
+
+// Validation test for https://github.com/getsentry/sentry-javascript/issues/12281
+sentryTest('should add browser-related spans to pageload transaction', async ({ getLocalTestPath, page }) => {
+  if (shouldSkipTracingTest()) {
+    sentryTest.skip();
+  }
+
+  const url = await getLocalTestPath({ testDir: __dirname });
+
+  const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+  const browserSpans = eventData.spans?.filter(({ op }) => op === 'browser');
+
+  // Spans `domContentLoadedEvent`, `connect`, `cache` and `DNS` are not
+  // always inside `pageload` transaction.
+  expect(browserSpans?.length).toBeGreaterThanOrEqual(4);
+
+  const requestSpan = browserSpans!.find(({ description }) => description === 'request');
+  expect(requestSpan).toBeDefined();
+
+  const measureSpan = eventData.spans?.find(({ op }) => op === 'measure');
+  expect(measureSpan).toBeDefined();
+
+  expect(requestSpan!.start_timestamp).toBeLessThanOrEqual(measureSpan!.start_timestamp);
+  expect(measureSpan?.data).toEqual({
+    'sentry.browser.measure_happened_before_request': true,
+    'sentry.browser.measure_start_time': expect.any(Number),
+    'sentry.op': 'measure',
+    'sentry.origin': 'auto.resource.browser.metrics',
+  });
+});

packages/browser-utils/src/metrics/browserMetrics.ts

@@ -342,15 +342,34 @@ export function _addMeasureSpans(
   duration: number,
   timeOrigin: number,
 ): number {
-  const measureStartTimestamp = timeOrigin + startTime;
-  const measureEndTimestamp = measureStartTimestamp + duration;
+  const navEntry = getNavigationEntry();
+  const requestTime = msToSec(navEntry ? navEntry.requestStart : 0);
+  // Because performance.measure accepts arbitrary timestamps it can produce
+  // spans that happen before the browser even makes a request for the page.
+  //
+  // An example of this is the automatically generated Next.js-before-hydration
+  // spans created by the Next.js framework.
+  //
+  // To prevent this we will pin the start timestamp to the request start time
+  // This does make duration inaccruate, so if this does happen, we will add
+  // an attribute to the span
+  const measureStartTimestamp = timeOrigin + Math.max(startTime, requestTime);
+  const startTimeStamp = timeOrigin + startTime;
+  const measureEndTimestamp = startTimeStamp + duration;
+
+  const attributes: SpanAttributes = {
+    [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.resource.browser.metrics',
+  };
+
+  if (measureStartTimestamp !== startTimeStamp) {
+    attributes['sentry.browser.measure_happened_before_request'] = true;
+    attributes['sentry.browser.measure_start_time'] = measureStartTimestamp;
+  }
 
   startAndEndSpan(span, measureStartTimestamp, measureEndTimestamp, {
     name: entry.name as string,
     op: entry.entryType as string,
-    attributes: {
-      [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.resource.browser.metrics',
-    },
+    attributes,
   });
 
   return measureStartTimestamp;
@@ -395,36 +414,29 @@ function _addPerformanceNavigationTiming(
 /** Create request and response related spans */
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 function _addRequest(span: Span, entry: Record<string, any>, timeOrigin: number): void {
+  const requestStartTimestamp = timeOrigin + msToSec(entry.requestStart as number);
+  const responseEndTimestamp = timeOrigin + msToSec(entry.responseEnd as number);
+  const responseStartTimestamp = timeOrigin + msToSec(entry.responseStart as number);
   if (entry.responseEnd) {
     // It is possible that we are collecting these metrics when the page hasn't finished loading yet, for example when the HTML slowly streams in.
     // In this case, ie. when the document request hasn't finished yet, `entry.responseEnd` will be 0.
     // In order not to produce faulty spans, where the end timestamp is before the start timestamp, we will only collect
     // these spans when the responseEnd value is available. The backend (Relay) would drop the entire span if it contained faulty spans.
-    startAndEndSpan(
-      span,
-      timeOrigin + msToSec(entry.requestStart as number),
-      timeOrigin + msToSec(entry.responseEnd as number),
-      {
-        op: 'browser',
-        name: 'request',
-        attributes: {
-          [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.browser.metrics',
-        },
+    startAndEndSpan(span, requestStartTimestamp, responseEndTimestamp, {
+      op: 'browser',
+      name: 'request',
+      attributes: {
+        [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.browser.metrics',
       },
-    );
+    });
 
-    startAndEndSpan(
-      span,
-      timeOrigin + msToSec(entry.responseStart as number),
-      timeOrigin + msToSec(entry.responseEnd as number),
-      {
-        op: 'browser',
-        name: 'response',
-        attributes: {
-          [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.browser.metrics',
-        },
+    startAndEndSpan(span, responseStartTimestamp, responseEndTimestamp, {
+      op: 'browser',
+      name: 'response',
+      attributes: {
+        [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.browser.metrics',
       },
-    );
+    });
   }
 }