feat(browser): Do not include metrics in base CDN bundle (#12230)

mydea · web-flow · commit 0fbe39adc029 · 2024-05-27T17:19:10.000+02:00
Metrics are only included when performance is included, reducing the base bundle size. We always expose a shim so there is no API breakage, it just does nothing. I noticed this while working on #12226.
diff --git a/dev-packages/browser-integration-tests/suites/metrics/metricsEvent/init.js b/dev-packages/browser-integration-tests/suites/metrics/metricsEvent/init.js
diff --git a/dev-packages/browser-integration-tests/suites/metrics/metricsEvent/test.ts b/dev-packages/browser-integration-tests/suites/metrics/metricsEvent/test.ts
@@ -1,9 +1,17 @@
 import { expect } from '@playwright/test';
 
-import { sentryTest } from '../../utils/fixtures';
-import { getFirstSentryEnvelopeRequest, properEnvelopeRequestParser } from '../../utils/helpers';
+import { sentryTest } from '../../../utils/fixtures';
+import {
+  getFirstSentryEnvelopeRequest,
+  properEnvelopeRequestParser,
+  shouldSkipMetricsTest,
+} from '../../../utils/helpers';
 
 sentryTest('collects metrics', async ({ getLocalTestUrl, page }) => {
+  if (shouldSkipMetricsTest()) {
+    sentryTest.skip();
+  }
+
   const url = await getLocalTestUrl({ testDir: __dirname });
 
   const statsdBuffer = await getFirstSentryEnvelopeRequest<Uint8Array>(page, url, properEnvelopeRequestParser);
diff --git a/dev-packages/browser-integration-tests/suites/metrics/metricsShim/init.js b/dev-packages/browser-integration-tests/suites/metrics/metricsShim/init.js
@@ -0,0 +1,13 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+});
+
+// This should not fail
+Sentry.metrics.increment('increment');
+Sentry.metrics.distribution('distribution', 42);
+Sentry.metrics.gauge('gauge', 5);
+Sentry.metrics.set('set', 'nope');
diff --git a/dev-packages/browser-integration-tests/suites/metrics/metricsShim/test.ts b/dev-packages/browser-integration-tests/suites/metrics/metricsShim/test.ts
@@ -0,0 +1,36 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../utils/fixtures';
+import { shouldSkipMetricsTest } from '../../../utils/helpers';
+
+sentryTest('exports shim metrics integration for non-tracing bundles', async ({ getLocalTestPath, page }) => {
+  // Skip in tracing tests
+  if (!shouldSkipMetricsTest()) {
+    sentryTest.skip();
+  }
+
+  const consoleMessages: string[] = [];
+  page.on('console', msg => consoleMessages.push(msg.text()));
+
+  let requestCount = 0;
+  await page.route('https://dsn.ingest.sentry.io/**/*', route => {
+    requestCount++;
+    return route.fulfill({
+      status: 200,
+      contentType: 'application/json',
+      body: JSON.stringify({ id: 'test-id' }),
+    });
+  });
+
+  const url = await getLocalTestPath({ testDir: __dirname });
+
+  await page.goto(url);
+
+  expect(requestCount).toBe(0);
+  expect(consoleMessages).toEqual([
+    'You are using metrics even though this bundle does not include tracing.',
+    'You are using metrics even though this bundle does not include tracing.',
+    'You are using metrics even though this bundle does not include tracing.',
+    'You are using metrics even though this bundle does not include tracing.',
+  ]);
+});
diff --git a/dev-packages/browser-integration-tests/utils/helpers.ts b/dev-packages/browser-integration-tests/utils/helpers.ts
@@ -241,7 +241,7 @@ export function shouldSkipTracingTest(): boolean {
 }
 
 /**
- * We can only test replay tests in certain bundles/packages:
+ * We can only test feedback tests in certain bundles/packages:
  * - NPM (ESM, CJS)
  * - CDN bundles that contain the Replay integration
  *
@@ -252,6 +252,18 @@ export function shouldSkipFeedbackTest(): boolean {
   return bundle != null && !bundle.includes('feedback') && !bundle.includes('esm') && !bundle.includes('cjs');
 }
 
+/**
+ * We can only test metrics tests in certain bundles/packages:
+ * - NPM (ESM, CJS)
+ * - CDN bundles that include tracing
+ *
+ * @returns `true` if we should skip the metrics test
+ */
+export function shouldSkipMetricsTest(): boolean {
+  const bundle = process.env.PW_BUNDLE as string | undefined;
+  return bundle != null && !bundle.includes('tracing') && !bundle.includes('esm') && !bundle.includes('cjs');
+}
+
 /**
  * Waits until a number of requests matching urlRgx at the given URL arrive.
  * If the timout option is configured, this function will abort waiting, even if it hasn't reveived the configured
diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts
@@ -68,8 +68,6 @@ export {
   SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
 } from '@sentry/core';
 
-export * from './metrics';
-
 export { WINDOW } from './helpers';
 export { BrowserClient } from './client';
 export { makeFetchTransport } from './transports/fetch';
diff --git a/packages/browser/src/index.bundle.feedback.ts b/packages/browser/src/index.bundle.feedback.ts
@@ -1,4 +1,4 @@
-import { browserTracingIntegrationShim, replayIntegrationShim } from '@sentry-internal/integration-shims';
+import { browserTracingIntegrationShim, metricsShim, replayIntegrationShim } from '@sentry-internal/integration-shims';
 import { feedbackAsyncIntegration } from './feedbackAsync';
 
 export * from './index.bundle.base';
@@ -10,6 +10,7 @@ export {
   feedbackAsyncIntegration as feedbackAsyncIntegration,
   feedbackAsyncIntegration as feedbackIntegration,
   replayIntegrationShim as replayIntegration,
+  metricsShim as metrics,
 };
 
 export { captureFeedback } from '@sentry/core';
diff --git a/packages/browser/src/index.bundle.replay.ts b/packages/browser/src/index.bundle.replay.ts
@@ -1,4 +1,8 @@
-import { browserTracingIntegrationShim, feedbackIntegrationShim } from '@sentry-internal/integration-shims';
+import {
+  browserTracingIntegrationShim,
+  feedbackIntegrationShim,
+  metricsShim,
+} from '@sentry-internal/integration-shims';
 
 export * from './index.bundle.base';
 
@@ -8,4 +12,5 @@ export {
   browserTracingIntegrationShim as browserTracingIntegration,
   feedbackIntegrationShim as feedbackAsyncIntegration,
   feedbackIntegrationShim as feedbackIntegration,
+  metricsShim as metrics,
 };
diff --git a/packages/browser/src/index.bundle.tracing.replay.feedback.ts b/packages/browser/src/index.bundle.tracing.replay.feedback.ts
@@ -4,6 +4,8 @@ registerSpanErrorInstrumentation();
 
 export * from './index.bundle.base';
 
+export * from './metrics';
+
 export {
   getActiveSpan,
   getRootSpan,
diff --git a/packages/browser/src/index.bundle.tracing.replay.ts b/packages/browser/src/index.bundle.tracing.replay.ts
@@ -4,6 +4,8 @@ registerSpanErrorInstrumentation();
 
 export * from './index.bundle.base';
 
+export * from './metrics';
+
 export {
   getActiveSpan,
   getRootSpan,
diff --git a/packages/browser/src/index.bundle.tracing.ts b/packages/browser/src/index.bundle.tracing.ts
@@ -5,6 +5,8 @@ registerSpanErrorInstrumentation();
 
 export * from './index.bundle.base';
 
+export * from './metrics';
+
 export {
   getActiveSpan,
   getRootSpan,
diff --git a/packages/browser/src/index.bundle.ts b/packages/browser/src/index.bundle.ts
@@ -1,6 +1,7 @@
 import {
   browserTracingIntegrationShim,
   feedbackIntegrationShim,
+  metricsShim,
   replayIntegrationShim,
 } from '@sentry-internal/integration-shims';
 
@@ -11,4 +12,5 @@ export {
   feedbackIntegrationShim as feedbackAsyncIntegration,
   feedbackIntegrationShim as feedbackIntegration,
   replayIntegrationShim as replayIntegration,
+  metricsShim as metrics,
 };
diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts
@@ -39,6 +39,8 @@ export {
   sendFeedback,
 } from '@sentry-internal/feedback';
 
+export * from './metrics';
+
 export {
   defaultRequestInstrumentationOptions,
   instrumentOutgoingRequests,
diff --git a/packages/browser/src/metrics.ts b/packages/browser/src/metrics.ts
@@ -1,5 +1,5 @@
-import type { MetricData } from '@sentry/core';
 import { BrowserMetricsAggregator, metrics as metricsCore } from '@sentry/core';
+import type { MetricData, Metrics } from '@sentry/types';
 
 /**
  * Adds a value to a counter metric
@@ -37,7 +37,7 @@ function gauge(name: string, value: number, data?: MetricData): void {
   metricsCore.gauge(BrowserMetricsAggregator, name, value, data);
 }
 
-export const metrics = {
+export const metrics: Metrics = {
   increment,
   distribution,
   set,
diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
@@ -97,7 +97,7 @@ export { rewriteFramesIntegration } from './integrations/rewriteframes';
 export { sessionTimingIntegration } from './integrations/sessiontiming';
 export { zodErrorsIntegration } from './integrations/zoderrors';
 export { metrics } from './metrics/exports';
-export type { MetricData } from './metrics/exports';
+export type { MetricData } from '@sentry/types';
 export { metricsDefault } from './metrics/exports-default';
 export { BrowserMetricsAggregator } from './metrics/browser-aggregator';
 export { getMetricSummaryJsonForSpan } from './metrics/metric-summary';
diff --git a/packages/core/src/metrics/exports-default.ts b/packages/core/src/metrics/exports-default.ts
@@ -1,6 +1,5 @@
-import type { Client, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types';
+import type { Client, MetricData, Metrics, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types';
 import { MetricsAggregator } from './aggregator';
-import type { MetricData } from './exports';
 import { metrics as metricsCore } from './exports';
 
 /**
@@ -46,11 +45,14 @@ function getMetricsAggregatorForClient(client: Client): MetricsAggregatorInterfa
   return metricsCore.getMetricsAggregatorForClient(client, MetricsAggregator);
 }
 
-export const metricsDefault = {
+export const metricsDefault: Metrics & {
+  getMetricsAggregatorForClient: typeof getMetricsAggregatorForClient;
+} = {
   increment,
   distribution,
   set,
   gauge,
+
   /**
    * @ignore This is for internal use only.
    */
diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts
@@ -1,23 +1,11 @@
-import type {
-  Client,
-  MeasurementUnit,
-  MetricsAggregator as MetricsAggregatorInterface,
-  Primitive,
-} from '@sentry/types';
+import type { Client, MetricData, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types';
 import { getGlobalSingleton, logger } from '@sentry/utils';
 import { getClient } from '../currentScopes';
 import { DEBUG_BUILD } from '../debug-build';
 import { getActiveSpan, getRootSpan, spanToJSON } from '../utils/spanUtils';
 import { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants';
 import type { MetricType } from './types';
 
-export interface MetricData {
-  unit?: MeasurementUnit;
-  tags?: Record<string, Primitive>;
-  timestamp?: number;
-  client?: Client;
-}
-
 type MetricsAggregatorConstructor = {
   new (client: Client): MetricsAggregatorInterface;
 };
diff --git a/packages/integration-shims/src/index.ts b/packages/integration-shims/src/index.ts
@@ -1,3 +1,4 @@
 export { feedbackIntegrationShim } from './Feedback';
 export { replayIntegrationShim } from './Replay';
 export { browserTracingIntegrationShim } from './BrowserTracing';
+export { metricsShim } from './metrics';
diff --git a/packages/integration-shims/src/metrics.ts b/packages/integration-shims/src/metrics.ts
@@ -0,0 +1,16 @@
+import type { Metrics } from '@sentry/types';
+import { consoleSandbox } from '@sentry/utils';
+
+function warn(): void {
+  consoleSandbox(() => {
+    // eslint-disable-next-line no-console
+    console.warn('You are using metrics even though this bundle does not include tracing.');
+  });
+}
+
+export const metricsShim: Metrics = {
+  increment: warn,
+  distribution: warn,
+  set: warn,
+  gauge: warn,
+};
diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
@@ -164,6 +164,8 @@ export type {
   MetricsAggregator,
   MetricBucketItem,
   MetricInstance,
+  MetricData,
+  Metrics,
 } from './metrics';
 export type { ParameterizedString } from './parameterize';
 export type { ViewHierarchyData, ViewHierarchyWindow } from './view-hierarchy';
diff --git a/packages/types/src/metrics.ts b/packages/types/src/metrics.ts
@@ -1,6 +1,14 @@
+import type { Client } from './client';
 import type { MeasurementUnit } from './measurement';
 import type { Primitive } from './misc';
 
+export interface MetricData {
+  unit?: MeasurementUnit;
+  tags?: Record<string, Primitive>;
+  timestamp?: number;
+  client?: Client;
+}
+
 /**
  * An abstract definition of the minimum required API
  * for a metric instance.
@@ -62,3 +70,33 @@ export interface MetricsAggregator {
    */
   toString(): string;
 }
+
+export interface Metrics {
+  /**
+   * Adds a value to a counter metric
+   *
+   * @experimental This API is experimental and might have breaking changes in the future.
+   */
+  increment(name: string, value?: number, data?: MetricData): void;
+
+  /**
+   * Adds a value to a distribution metric
+   *
+   * @experimental This API is experimental and might have breaking changes in the future.
+   */
+  distribution(name: string, value: number, data?: MetricData): void;
+
+  /**
+   * Adds a value to a set metric. Value must be a string or integer.
+   *
+   * @experimental This API is experimental and might have breaking changes in the future.
+   */
+  set(name: string, value: number | string, data?: MetricData): void;
+
+  /**
+   * Adds a value to a gauge metric
+   *
+   * @experimental This API is experimental and might have breaking changes in the future.
+   */
+  gauge(name: string, value: number, data?: MetricData): void;
+}
