From d43ba7ce1584a4ad00fee3bfe69542ac704a0d31 Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Mon, 27 May 2024 11:33:30 +0200
Subject: [PATCH 1/4] feat(browser): Do not include metrics in base CDN bundle

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.
---
 packages/browser/src/exports.ts               |  2 -
 packages/browser/src/index.bundle.feedback.ts |  3 +-
 packages/browser/src/index.bundle.replay.ts   |  7 +++-
 .../index.bundle.tracing.replay.feedback.ts   |  2 +
 .../src/index.bundle.tracing.replay.ts        |  2 +
 packages/browser/src/index.bundle.tracing.ts  |  2 +
 packages/browser/src/index.bundle.ts          |  2 +
 packages/browser/src/index.ts                 |  2 +
 packages/browser/src/metrics.ts               |  4 +-
 packages/core/src/index.ts                    |  2 +-
 packages/core/src/metrics/exports-default.ts  |  3 +-
 packages/core/src/metrics/exports.ts          | 14 +------
 packages/integration-shims/src/index.ts       |  1 +
 packages/integration-shims/src/metrics.ts     | 10 +++++
 packages/types/src/index.ts                   |  2 +
 packages/types/src/metrics.ts                 | 38 +++++++++++++++++++
 16 files changed, 74 insertions(+), 22 deletions(-)
 create mode 100644 packages/integration-shims/src/metrics.ts

diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts
index 6b8edb66541a..94b4bd0b3c2a 100644
--- 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
index 957583d79eeb..c6f75c03d9d1 100644
--- 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
index 1a538a97162f..1f1d4441346b 100644
--- 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
index b9dc457640be..29438387ee5b 100644
--- 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
index a0e4d4736384..6520aa185b2f 100644
--- 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
index d540ff0bd6f9..8115e628aa89 100644
--- 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
index 5004b376cd46..38787264f9b0 100644
--- 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
index 245eaa966859..9f8ea02e5822 100644
--- 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
index 6a7792a16c67..267fe90d03c9 100644
--- 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
index f193f75bfe60..03dfa8e63aa3 100644
--- 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
index 280d1d619bea..6557ba935179 100644
--- 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, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types';
 import { MetricsAggregator } from './aggregator';
-import type { MetricData } from './exports';
 import { metrics as metricsCore } from './exports';
 
 /**
diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts
index f062b65f72d9..665ac9c12816 100644
--- a/packages/core/src/metrics/exports.ts
+++ b/packages/core/src/metrics/exports.ts
@@ -1,9 +1,4 @@
-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';
@@ -11,13 +6,6 @@ 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
index 510b26ddbb76..616f9910cf35 100644
--- 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
new file mode 100644
index 000000000000..48542b3b359c
--- /dev/null
+++ b/packages/integration-shims/src/metrics.ts
@@ -0,0 +1,10 @@
+import type { Metrics } from '@sentry/types';
+
+export const metricsShim: Metrics = {
+  /* eslint-disable @typescript-eslint/no-empty-function */
+  increment: () => {},
+  distribution: () => {},
+  set: () => {},
+  gauge: () => {},
+  /* eslint-enable @typescript-eslint/no-empty-function */
+};
diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
index bb4230ea3e5c..c90b7841f9ff 100644
--- 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
index 0f8dc4f53435..843068db0aef 100644
--- 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;
+}

From fd6c502ab791504b5583320cf39116fbc124c8a9 Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Mon, 27 May 2024 13:07:12 +0200
Subject: [PATCH 2/4] do not re-export metrics aggrgator?

---
 packages/core/src/metrics/exports-default.ts | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/packages/core/src/metrics/exports-default.ts b/packages/core/src/metrics/exports-default.ts
index 6557ba935179..30b204dd2cb7 100644
--- a/packages/core/src/metrics/exports-default.ts
+++ b/packages/core/src/metrics/exports-default.ts
@@ -1,4 +1,4 @@
-import type { Client, MetricData, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types';
+import type { MetricData, Metrics } from '@sentry/types';
 import { MetricsAggregator } from './aggregator';
 import { metrics as metricsCore } from './exports';
 
@@ -38,20 +38,9 @@ function gauge(name: string, value: number, data?: MetricData): void {
   metricsCore.gauge(MetricsAggregator, name, value, data);
 }
 
-/**
- * Returns the metrics aggregator for a given client.
- */
-function getMetricsAggregatorForClient(client: Client): MetricsAggregatorInterface {
-  return metricsCore.getMetricsAggregatorForClient(client, MetricsAggregator);
-}
-
-export const metricsDefault = {
+export const metricsDefault: Metrics = {
   increment,
   distribution,
   set,
   gauge,
-  /**
-   * @ignore This is for internal use only.
-   */
-  getMetricsAggregatorForClient,
 };

From f8592a219c964836d22998125e55bafb50d7b689 Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Mon, 27 May 2024 13:33:51 +0200
Subject: [PATCH 3/4] fix tests

---
 .../suites/metrics/{ => metricsEvent}/init.js |  0
 .../suites/metrics/{ => metricsEvent}/test.ts | 12 +++++--
 .../suites/metrics/metricsShim/init.js        | 13 +++++++
 .../suites/metrics/metricsShim/test.ts        | 36 +++++++++++++++++++
 .../utils/helpers.ts                          | 14 +++++++-
 packages/integration-shims/src/metrics.ts     | 18 ++++++----
 6 files changed, 84 insertions(+), 9 deletions(-)
 rename dev-packages/browser-integration-tests/suites/metrics/{ => metricsEvent}/init.js (100%)
 rename dev-packages/browser-integration-tests/suites/metrics/{ => metricsEvent}/test.ts (73%)
 create mode 100644 dev-packages/browser-integration-tests/suites/metrics/metricsShim/init.js
 create mode 100644 dev-packages/browser-integration-tests/suites/metrics/metricsShim/test.ts

diff --git a/dev-packages/browser-integration-tests/suites/metrics/init.js b/dev-packages/browser-integration-tests/suites/metrics/metricsEvent/init.js
similarity index 100%
rename from dev-packages/browser-integration-tests/suites/metrics/init.js
rename to dev-packages/browser-integration-tests/suites/metrics/metricsEvent/init.js
diff --git a/dev-packages/browser-integration-tests/suites/metrics/test.ts b/dev-packages/browser-integration-tests/suites/metrics/metricsEvent/test.ts
similarity index 73%
rename from dev-packages/browser-integration-tests/suites/metrics/test.ts
rename to dev-packages/browser-integration-tests/suites/metrics/metricsEvent/test.ts
index 5c6ff8bb13a4..05a6d238be93 100644
--- a/dev-packages/browser-integration-tests/suites/metrics/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
new file mode 100644
index 000000000000..93c639cbdff9
--- /dev/null
+++ 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
new file mode 100644
index 000000000000..ba86f0a991f5
--- /dev/null
+++ 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
index 0e888a708f00..ab55da449ad2 100644
--- 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/integration-shims/src/metrics.ts b/packages/integration-shims/src/metrics.ts
index 48542b3b359c..9af425e0f629 100644
--- a/packages/integration-shims/src/metrics.ts
+++ b/packages/integration-shims/src/metrics.ts
@@ -1,10 +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 = {
-  /* eslint-disable @typescript-eslint/no-empty-function */
-  increment: () => {},
-  distribution: () => {},
-  set: () => {},
-  gauge: () => {},
-  /* eslint-enable @typescript-eslint/no-empty-function */
+  increment: warn,
+  distribution: warn,
+  set: warn,
+  gauge: warn,
 };

From 246f1599907753d3ecbda8e7d76211c70225668c Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Mon, 27 May 2024 15:46:21 +0200
Subject: [PATCH 4/4] re-add export

---
 packages/core/src/metrics/exports-default.ts | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/packages/core/src/metrics/exports-default.ts b/packages/core/src/metrics/exports-default.ts
index 30b204dd2cb7..86d294d059d8 100644
--- a/packages/core/src/metrics/exports-default.ts
+++ b/packages/core/src/metrics/exports-default.ts
@@ -1,4 +1,4 @@
-import type { MetricData, Metrics } from '@sentry/types';
+import type { Client, MetricData, Metrics, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types';
 import { MetricsAggregator } from './aggregator';
 import { metrics as metricsCore } from './exports';
 
@@ -38,9 +38,23 @@ function gauge(name: string, value: number, data?: MetricData): void {
   metricsCore.gauge(MetricsAggregator, name, value, data);
 }
 
-export const metricsDefault: Metrics = {
+/**
+ * Returns the metrics aggregator for a given client.
+ */
+function getMetricsAggregatorForClient(client: Client): MetricsAggregatorInterface {
+  return metricsCore.getMetricsAggregatorForClient(client, MetricsAggregator);
+}
+
+export const metricsDefault: Metrics & {
+  getMetricsAggregatorForClient: typeof getMetricsAggregatorForClient;
+} = {
   increment,
   distribution,
   set,
   gauge,
+
+  /**
+   * @ignore This is for internal use only.
+   */
+  getMetricsAggregatorForClient,
 };