feat(core): Decouple metrics aggregation from client

dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js

@@ -0,0 +1,53 @@
+const { loggingTransport } = require('@sentry-internal/node-integration-tests');
+const Sentry = require('@sentry/node');
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  tracesSampleRate: 1.0,
+  transport: loggingTransport,
+});
+
+// Stop the process from exiting before the transaction is sent
+setInterval(() => {}, 1000);
+
+Sentry.startSpan(
+  {
+    name: 'Test Transaction',
+    op: 'transaction',
+  },
+  () => {
+    Sentry.metrics.increment('root-counter', 1, {
+      tags: {
+        email: '[email protected]',
+      },
+    });
+    Sentry.metrics.increment('root-counter', 1, {
+      tags: {
+        email: '[email protected]',
+      },
+    });
+
+    Sentry.startSpan(
+      {
+        name: 'Some other span',
+        op: 'transaction',
+      },
+      () => {
+        Sentry.metrics.increment('root-counter');
+        Sentry.metrics.increment('root-counter');
+        Sentry.metrics.increment('root-counter', 2);
+
+        Sentry.metrics.set('root-set', 'some-value');
+        Sentry.metrics.set('root-set', 'another-value');
+        Sentry.metrics.set('root-set', 'another-value');
+
+        Sentry.metrics.gauge('root-gauge', 42);
+        Sentry.metrics.gauge('root-gauge', 20);
+
+        Sentry.metrics.distribution('root-distribution', 42);
+        Sentry.metrics.distribution('root-distribution', 20);
+      },
+    );
+  },
+);

dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts

@@ -0,0 +1,91 @@
+import { createRunner } from '../../../utils/runner';
+
+const EXPECTED_TRANSACTION = {
+  transaction: 'Test Transaction',
+  _metrics_summary: {
+    'c:root-counter@none': [
+      {
+        min: 1,
+        max: 1,
+        count: 1,
+        sum: 1,
+        tags: {
+          release: '1.0',
+          transaction: 'Test Transaction',
+          email: '[email protected]',
+        },
+      },
+      {
+        min: 1,
+        max: 1,
+        count: 1,
+        sum: 1,
+        tags: {
+          release: '1.0',
+          transaction: 'Test Transaction',
+          email: '[email protected]',
+        },
+      },
+    ],
+  },
+  spans: expect.arrayContaining([
+    expect.objectContaining({
+      description: 'Some other span',
+      op: 'transaction',
+      _metrics_summary: {
+        'c:root-counter@none': [
+          {
+            min: 1,
+            max: 2,
+            count: 3,
+            sum: 4,
+            tags: {
+              release: '1.0',
+              transaction: 'Test Transaction',
+            },
+          },
+        ],
+        's:root-set@none': [
+          {
+            min: 0,
+            max: 1,
+            count: 3,
+            sum: 2,
+            tags: {
+              release: '1.0',
+              transaction: 'Test Transaction',
+            },
+          },
+        ],
+        'g:root-gauge@none': [
+          {
+            min: 20,
+            max: 42,
+            count: 2,
+            sum: 62,
+            tags: {
+              release: '1.0',
+              transaction: 'Test Transaction',
+            },
+          },
+        ],
+        'd:root-distribution@none': [
+          {
+            min: 20,
+            max: 42,
+            count: 2,
+            sum: 62,
+            tags: {
+              release: '1.0',
+              transaction: 'Test Transaction',
+            },
+          },
+        ],
+      },
+    }),
+  ]),
+};
+
+test('Should add metric summaries to spans', done => {
+  createRunner(__dirname, 'scenario.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done);
+});

packages/astro/src/index.types.ts

@@ -26,6 +26,8 @@ export declare const defaultStackParser: StackParser;
 export declare function close(timeout?: number | undefined): PromiseLike<boolean>;
 export declare function flush(timeout?: number | undefined): PromiseLike<boolean>;
 
+export declare const metrics: typeof clientSdk.metrics & typeof serverSdk.metrics;
+
 /**
  * @deprecated This function will be removed in the next major version of the Sentry SDK.
  */

packages/browser/src/exports.ts

@@ -68,12 +68,12 @@ export {
   FunctionToString,
   // eslint-disable-next-line deprecation/deprecation
   InboundFilters,
-  metrics,
   functionToStringIntegration,
   inboundFiltersIntegration,
   parameterize,
 } from '@sentry/core';
 
+export * from './metrics';
 export { WINDOW } from './helpers';
 export { BrowserClient } from './client';
 export { makeFetchTransport, makeXHRTransport } from './transports';

packages/browser/src/metrics.ts

@@ -0,0 +1,51 @@
+import type { MetricData } from '@sentry/core';
+import { BrowserMetricsAggregator, metrics as metricsCore } from '@sentry/core';
+
+/**
+ * Adds a value to a counter metric
+ *
+ * @experimental This API is experimental and might have breaking changes in the future.
+ */
+function increment(name: string, value: number = 1, data?: MetricData): void {
+  metricsCore.increment(BrowserMetricsAggregator, name, value, data);
+}
+
+/**
+ * Adds a value to a distribution metric
+ *
+ * @experimental This API is experimental and might have breaking changes in the future.
+ */
+function distribution(name: string, value: number, data?: MetricData): void {
+  metricsCore.distribution(BrowserMetricsAggregator, name, value, data);
+}
+
+/**
+ * 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.
+ */
+function set(name: string, value: number | string, data?: MetricData): void {
+  metricsCore.set(BrowserMetricsAggregator, name, value, data);
+}
+
+/**
+ * Adds a value to a gauge metric
+ *
+ * @experimental This API is experimental and might have breaking changes in the future.
+ */
+function gauge(name: string, value: number, data?: MetricData): void {
+  metricsCore.gauge(BrowserMetricsAggregator, name, value, data);
+}
+
+export const metrics = {
+  increment,
+  distribution,
+  set,
+  gauge,
+  /** @deprecated An integration is no longer required to use the metrics feature */
+  // eslint-disable-next-line deprecation/deprecation
+  MetricsAggregator: metricsCore.MetricsAggregator,
+  /** @deprecated An integration is no longer required to use the metrics feature */
+  // eslint-disable-next-line deprecation/deprecation
+  metricsAggregatorIntegration: metricsCore.metricsAggregatorIntegration,
+};

packages/bun/src/index.ts

@@ -78,7 +78,7 @@ export {
   startInactiveSpan,
   startSpanManual,
   continueTrace,
-  metrics,
+  metricsNode as metrics,
   functionToStringIntegration,
   inboundFiltersIntegration,
   linkedErrorsIntegration,

packages/core/src/baseclient.ts

@@ -95,9 +95,9 @@ const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been ca
  */
 export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   /**
-   * A reference to a metrics aggregator
+   * TODO (v8): Remove
    *
-   * @experimental Note this is alpha API. It may experience breaking changes in the future.
+   * @deprecated The metricsAggregator is no longer referenced on the client.
    */
   public metricsAggregator?: MetricsAggregator;
 
@@ -281,9 +281,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   public flush(timeout?: number): PromiseLike<boolean> {
     const transport = this._transport;
     if (transport) {
-      if (this.metricsAggregator) {
-        this.metricsAggregator.flush();
-      }
+      this.emit('flush');
       return this._isClientDoneProcessing(timeout).then(clientFinished => {
         return transport.flush(timeout).then(transportFlushed => clientFinished && transportFlushed);
       });
@@ -298,9 +296,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   public close(timeout?: number): PromiseLike<boolean> {
     return this.flush(timeout).then(result => {
       this.getOptions().enabled = false;
-      if (this.metricsAggregator) {
-        this.metricsAggregator.close();
-      }
+      this.emit('close');
       return result;
     });
   }
@@ -496,6 +492,10 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   /** @inheritdoc */
   public on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): void;
 
+  public on(hook: 'flush', callback: () => void): void;
+
+  public on(hook: 'close', callback: () => void): void;
+
   /** @inheritdoc */
   public on(hook: string, callback: unknown): void {
     if (!this._hooks[hook]) {
@@ -542,6 +542,12 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   /** @inheritdoc */
   public emit(hook: 'startNavigationSpan', options: StartSpanOptions): void;
 
+  /** @inheritdoc */
+  public emit(hook: 'flush'): void;
+
+  /** @inheritdoc */
+  public emit(hook: 'close'): void;
+
   /** @inheritdoc */
   public emit(hook: string, ...rest: unknown[]): void {
     if (this._hooks[hook]) {

packages/core/src/index.ts

@@ -106,6 +106,9 @@ export { linkedErrorsIntegration } from './integrations/linkederrors';
 export { moduleMetadataIntegration } from './integrations/metadata';
 export { requestDataIntegration } from './integrations/requestdata';
 export { metrics } from './metrics/exports';
+export type { MetricData } from './metrics/exports';
+export { metricsNode } from './metrics/exports-node';
+export { BrowserMetricsAggregator } from './metrics/browser-aggregator';
 
 /** @deprecated Import the integration function directly, e.g. `inboundFiltersIntegration()` instead of `new Integrations.InboundFilter(). */
 const Integrations = INTEGRATIONS;

packages/core/src/metrics/aggregator.ts

@@ -6,8 +6,9 @@ import type {
   Primitive,
 } from '@sentry/types';
 import { timestampInSeconds } from '@sentry/utils';
-import { DEFAULT_FLUSH_INTERVAL, MAX_WEIGHT, NAME_AND_TAG_KEY_NORMALIZATION_REGEX } from './constants';
+import { DEFAULT_FLUSH_INTERVAL, MAX_WEIGHT, NAME_AND_TAG_KEY_NORMALIZATION_REGEX, SET_METRIC_TYPE } from './constants';
 import { METRIC_MAP } from './instance';
+import { updateMetricSummaryOnActiveSpan } from './metric-summary';
 import type { MetricBucket, MetricType } from './types';
 import { getBucketKey, sanitizeTags } from './utils';
 
@@ -62,7 +63,11 @@ export class MetricsAggregator implements MetricsAggregatorBase {
     const tags = sanitizeTags(unsanitizedTags);
 
     const bucketKey = getBucketKey(metricType, name, unit, tags);
+
     let bucketItem = this._buckets.get(bucketKey);
+    // If this is a set metric, we need to calculate the delta from the previous weight.
+    const previousWeight = bucketItem && metricType === SET_METRIC_TYPE ? bucketItem.metric.weight : 0;
+
     if (bucketItem) {
       bucketItem.metric.add(value);
       // TODO(abhi): Do we need this check?
@@ -82,6 +87,10 @@ export class MetricsAggregator implements MetricsAggregatorBase {
       this._buckets.set(bucketKey, bucketItem);
     }
 
+    // If value is a string, it's a set metric so calculate the delta from the previous weight.
+    const val = typeof value === 'string' ? bucketItem.metric.weight - previousWeight : value;
+    updateMetricSummaryOnActiveSpan(metricType, name, val, unit, unsanitizedTags, bucketKey);
+
     // We need to keep track of the total weight of the buckets so that we can
     // flush them when we exceed the max weight.
     this._bucketsTotalWeight += bucketItem.metric.weight;

packages/core/src/metrics/browser-aggregator.ts

@@ -1,14 +1,8 @@
-import type {
-  Client,
-  ClientOptions,
-  MeasurementUnit,
-  MetricBucketItem,
-  MetricsAggregator,
-  Primitive,
-} from '@sentry/types';
+import type { Client, ClientOptions, MeasurementUnit, MetricsAggregator, Primitive } from '@sentry/types';
 import { timestampInSeconds } from '@sentry/utils';
-import { DEFAULT_BROWSER_FLUSH_INTERVAL, NAME_AND_TAG_KEY_NORMALIZATION_REGEX } from './constants';
+import { DEFAULT_BROWSER_FLUSH_INTERVAL, NAME_AND_TAG_KEY_NORMALIZATION_REGEX, SET_METRIC_TYPE } from './constants';
 import { METRIC_MAP } from './instance';
+import { updateMetricSummaryOnActiveSpan } from './metric-summary';
 import type { MetricBucket, MetricType } from './types';
 import { getBucketKey, sanitizeTags } from './utils';
 
@@ -46,24 +40,33 @@ export class BrowserMetricsAggregator implements MetricsAggregator {
     const tags = sanitizeTags(unsanitizedTags);
 
     const bucketKey = getBucketKey(metricType, name, unit, tags);
-    const bucketItem: MetricBucketItem | undefined = this._buckets.get(bucketKey);
+
+    let bucketItem = this._buckets.get(bucketKey);
+    // If this is a set metric, we need to calculate the delta from the previous weight.
+    const previousWeight = bucketItem && metricType === SET_METRIC_TYPE ? bucketItem.metric.weight : 0;
+
     if (bucketItem) {
       bucketItem.metric.add(value);
       // TODO(abhi): Do we need this check?
       if (bucketItem.timestamp < timestamp) {
         bucketItem.timestamp = timestamp;
       }
     } else {
-      this._buckets.set(bucketKey, {
+      bucketItem = {
         // @ts-expect-error we don't need to narrow down the type of value here, saves bundle size.
         metric: new METRIC_MAP[metricType](value),
         timestamp,
         metricType,
         name,
         unit,
         tags,
-      });
+      };
+      this._buckets.set(bucketKey, bucketItem);
     }
+
+    // If value is a string, it's a set metric so calculate the delta from the previous weight.
+    const val = typeof value === 'string' ? bucketItem.metric.weight - previousWeight : value;
+    updateMetricSummaryOnActiveSpan(metricType, name, val, unit, unsanitizedTags, bucketKey);
   }
 
   /**

packages/core/src/metrics/constants.ts

@@ -21,7 +21,7 @@ export const NAME_AND_TAG_KEY_NORMALIZATION_REGEX = /[^a-zA-Z0-9_/.-]+/g;
  *
  * See: https://develop.sentry.dev/sdk/metrics/#normalization
  */
-export const TAG_VALUE_NORMALIZATION_REGEX = /[^\w\d_:/@.{}[\]$-]+/g;
+export const TAG_VALUE_NORMALIZATION_REGEX = /[^\w\d\s_:/@.{}[\]$-]+/g;
 
 /**
  * This does not match spec in https://develop.sentry.dev/sdk/metrics