Merge branch 'develop' into sig/nuxt-external-init

Author: s1gr1d

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://[email protected]/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',
+  });
+});

dev-packages/rollup-utils/plugins/npmPlugins.mjs

@@ -10,7 +10,6 @@
 import * as fs from 'fs';
 import * as path from 'path';
 
-import { codecovRollupPlugin } from '@codecov/rollup-plugin';
 import json from '@rollup/plugin-json';
 import replace from '@rollup/plugin-replace';
 import cleanup from 'rollup-plugin-cleanup';

package.json

@@ -92,7 +92,6 @@
   ],
   "devDependencies": {
     "@biomejs/biome": "^1.4.0",
-    "@codecov/rollup-plugin": "0.0.1-beta.5",
     "@rollup/plugin-commonjs": "^25.0.7",
     "@rollup/plugin-esm-shim": "^0.1.5",
     "@rollup/plugin-json": "^6.1.0",
@@ -112,7 +111,6 @@
     "codecov": "^3.6.5",
     "deepmerge": "^4.2.2",
     "downlevel-dts": "~0.11.0",
-    "es-check": "7.1.0",
     "eslint": "7.32.0",
     "jest": "^27.5.1",
     "jest-environment-node": "^27.5.1",

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',
       },
-    );
+    });
   }
 }
 

packages/nextjs/src/config/types.ts

@@ -404,6 +404,22 @@ export type NextConfigFunction = (
  * Webpack config
  */
 
+// Note: The interface for `ignoreWarnings` is larger but we only need this. See https://webpack.js.org/configuration/other-options/#ignorewarnings
+export type IgnoreWarningsOption = (
+  | { module?: RegExp; message?: RegExp }
+  | ((
+      webpackError: {
+        module?: {
+          readableIdentifier: (requestShortener: unknown) => string;
+        };
+        message: string;
+      },
+      compilation: {
+        requestShortener: unknown;
+      },
+    ) => boolean)
+)[];
+
 // The two possible formats for providing custom webpack config in `next.config.js`
 export type WebpackConfigFunction = (config: WebpackConfigObject, options: BuildContext) => WebpackConfigObject;
 export type WebpackConfigObject = {
@@ -413,7 +429,7 @@ export type WebpackConfigObject = {
   output: { filename: string; path: string };
   target: string;
   context: string;
-  ignoreWarnings?: { module?: RegExp }[]; // Note: The interface for `ignoreWarnings` is larger but we only need this. See https://webpack.js.org/configuration/other-options/#ignorewarnings
+  ignoreWarnings?: IgnoreWarningsOption;
   resolve?: {
     modules?: string[];
     alias?: { [key: string]: string | boolean };

packages/nextjs/src/config/webpack.ts

@@ -14,6 +14,7 @@ import type { VercelCronsConfig } from '../common/types';
 import type {
   BuildContext,
   EntryPropertyObject,
+  IgnoreWarningsOption,
   NextConfigObject,
   SentryBuildOptions,
   WebpackConfigFunction,
@@ -72,9 +73,7 @@ export function constructWebpackConfigFunction(
     // Add a loader which will inject code that sets global values
     addValueInjectionLoader(newConfig, userNextConfig, userSentryOptions, buildContext);
 
-    if (isServer) {
-      addOtelWarningIgnoreRule(newConfig);
-    }
+    addOtelWarningIgnoreRule(newConfig);
 
     let pagesDirPath: string | undefined;
     const maybePagesDirPath = path.join(projectDir, 'pages');
@@ -668,9 +667,28 @@ function getRequestAsyncStorageModuleLocation(
 
 function addOtelWarningIgnoreRule(newConfig: WebpackConfigObjectWithModuleRules): void {
   const ignoreRules = [
+    // Inspired by @matmannion: https://github.com/getsentry/sentry-javascript/issues/12077#issuecomment-2180307072
+    (warning, compilation) => {
+      // This is wapped in try-catch because we are vendoring types for this hook and we can't be 100% sure that we are accessing API that is there
+      try {
+        if (!warning.module) {
+          return false;
+        }
+
+        const isDependencyThatMayRaiseCriticalDependencyMessage =
+          /@opentelemetry\/instrumentation/.test(warning.module.readableIdentifier(compilation.requestShortener)) ||
+          /@prisma\/instrumentation/.test(warning.module.readableIdentifier(compilation.requestShortener));
+        const isCriticalDependencyMessage = /Critical dependency/.test(warning.message);
+
+        return isDependencyThatMayRaiseCriticalDependencyMessage && isCriticalDependencyMessage;
+      } catch {
+        return false;
+      }
+    },
+    // We provide these objects in addition to the hook above to provide redundancy in case the hook fails.
     { module: /@opentelemetry\/instrumentation/, message: /Critical dependency/ },
     { module: /@prisma\/instrumentation/, message: /Critical dependency/ },
-  ];
+  ] satisfies IgnoreWarningsOption;
 
   if (newConfig.ignoreWarnings === undefined) {
     newConfig.ignoreWarnings = ignoreRules;