From c04de6e3471976f35ae841a1989c5a394c6efe1d Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 6 Mar 2024 15:57:30 +0000 Subject: [PATCH 1/3] feat(core): Allow metrics aggregator per client --- packages/core/src/baseclient.ts | 4 +- packages/core/src/metrics/aggregator.ts | 10 +-- .../core/src/metrics/browser-aggregator.ts | 5 +- packages/core/src/metrics/envelope.ts | 15 +--- packages/core/src/metrics/exports-default.ts | 12 +++ packages/core/src/metrics/exports.ts | 88 ++++++++++++------- packages/types/src/client.ts | 3 + 7 files changed, 79 insertions(+), 58 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 4180dde1e389..811c6466e612 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -534,7 +534,7 @@ export abstract class BaseClient implements Client { /** * @inheritdoc */ - public sendEnvelope(envelope: Envelope): PromiseLike | void { + public sendEnvelope(envelope: Envelope): PromiseLike { this.emit('beforeEnvelope', envelope); if (this._isEnabled() && this._transport) { @@ -545,6 +545,8 @@ export abstract class BaseClient implements Client { } DEBUG_BUILD && logger.error('Transport disabled'); + + return resolvedSyncPromise({}); } /* eslint-enable @typescript-eslint/unified-signatures */ diff --git a/packages/core/src/metrics/aggregator.ts b/packages/core/src/metrics/aggregator.ts index 5f0337e804f3..9116370efa52 100644 --- a/packages/core/src/metrics/aggregator.ts +++ b/packages/core/src/metrics/aggregator.ts @@ -1,11 +1,5 @@ -import type { - ClientOptions, - MeasurementUnit, - MetricsAggregator as MetricsAggregatorBase, - Primitive, -} from '@sentry/types'; +import type { Client, MeasurementUnit, MetricsAggregator as MetricsAggregatorBase, Primitive } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; -import type { BaseClient } from '../baseclient'; import { DEFAULT_FLUSH_INTERVAL, MAX_WEIGHT, NAME_AND_TAG_KEY_NORMALIZATION_REGEX, SET_METRIC_TYPE } from './constants'; import { captureAggregateMetrics } from './envelope'; import { METRIC_MAP } from './instance'; @@ -40,7 +34,7 @@ export class MetricsAggregator implements MetricsAggregatorBase { // Force flush is used on either shutdown, flush() or when we exceed the max weight. private _forceFlush: boolean; - public constructor(private readonly _client: BaseClient) { + public constructor(private readonly _client: Client) { this._buckets = new Map(); this._bucketsTotalWeight = 0; this._interval = setInterval(() => this._flush(), DEFAULT_FLUSH_INTERVAL); diff --git a/packages/core/src/metrics/browser-aggregator.ts b/packages/core/src/metrics/browser-aggregator.ts index d19aa441aef3..c88611a32d82 100644 --- a/packages/core/src/metrics/browser-aggregator.ts +++ b/packages/core/src/metrics/browser-aggregator.ts @@ -1,6 +1,5 @@ -import type { ClientOptions, MeasurementUnit, MetricsAggregator, Primitive } from '@sentry/types'; +import type { Client, MeasurementUnit, MetricsAggregator, Primitive } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; -import type { BaseClient } from '../baseclient'; import { DEFAULT_BROWSER_FLUSH_INTERVAL, NAME_AND_TAG_KEY_NORMALIZATION_REGEX, SET_METRIC_TYPE } from './constants'; import { captureAggregateMetrics } from './envelope'; import { METRIC_MAP } from './instance'; @@ -21,7 +20,7 @@ export class BrowserMetricsAggregator implements MetricsAggregator { private _buckets: MetricBucket; private readonly _interval: ReturnType; - public constructor(private readonly _client: BaseClient) { + public constructor(private readonly _client: Client) { this._buckets = new Map(); this._interval = setInterval(() => this.flush(), DEFAULT_BROWSER_FLUSH_INTERVAL); } diff --git a/packages/core/src/metrics/envelope.ts b/packages/core/src/metrics/envelope.ts index 47ccc2740834..f3bd67b27ec6 100644 --- a/packages/core/src/metrics/envelope.ts +++ b/packages/core/src/metrics/envelope.ts @@ -1,22 +1,11 @@ -import type { - ClientOptions, - DsnComponents, - MetricBucketItem, - SdkMetadata, - StatsdEnvelope, - StatsdItem, -} from '@sentry/types'; +import type { Client, DsnComponents, MetricBucketItem, SdkMetadata, StatsdEnvelope, StatsdItem } from '@sentry/types'; import { createEnvelope, dsnToString, logger } from '@sentry/utils'; -import type { BaseClient } from '../baseclient'; import { serializeMetricBuckets } from './utils'; /** * Captures aggregated metrics to the supplied client. */ -export function captureAggregateMetrics( - client: BaseClient, - metricBucketItems: Array, -): void { +export function captureAggregateMetrics(client: Client, metricBucketItems: Array): void { logger.log(`Flushing aggregated metrics, number of metrics: ${metricBucketItems.length}`); const dsn = client.getDsn(); const metadata = client.getSdkMetadata(); diff --git a/packages/core/src/metrics/exports-default.ts b/packages/core/src/metrics/exports-default.ts index f331bd3de3f1..280d1d619bea 100644 --- a/packages/core/src/metrics/exports-default.ts +++ b/packages/core/src/metrics/exports-default.ts @@ -1,3 +1,4 @@ +import type { Client, MetricsAggregator as MetricsAggregatorInterface } from '@sentry/types'; import { MetricsAggregator } from './aggregator'; import type { MetricData } from './exports'; import { metrics as metricsCore } from './exports'; @@ -38,9 +39,20 @@ 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 = { increment, distribution, set, gauge, + /** + * @ignore This is for internal use only. + */ + getMetricsAggregatorForClient, }; diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index 4587c26a8510..51517d27170c 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -1,11 +1,10 @@ import type { - ClientOptions, + Client, MeasurementUnit, MetricsAggregator as MetricsAggregatorInterface, Primitive, } from '@sentry/types'; import { logger } from '@sentry/utils'; -import type { BaseClient } from '../baseclient'; import { getCurrentScope } from '../currentScopes'; import { getClient } from '../currentScopes'; import { DEBUG_BUILD } from '../debug-build'; @@ -17,18 +16,43 @@ export interface MetricData { unit?: MeasurementUnit; tags?: Record; timestamp?: number; + client?: Client; } type MetricsAggregatorConstructor = { - new (client: BaseClient): MetricsAggregatorInterface; + new (client: Client): MetricsAggregatorInterface; }; /** - * Global metrics aggregator instance. - * - * This is initialized on the first call to any `Sentry.metric.*` method. + * Global metrics aggregators instances oper Client. */ -let globalMetricsAggregator: MetricsAggregatorInterface | undefined; +let globalMetricsAggregators: Map | undefined; + +/** + * Gets the metrics aggregator for a given client. + * @param client The client for which to get the metrics aggregator. + * @param Aggregator Optional metrics aggregator class to use to create an aggregator if one does not exist. + */ +function getMetricsAggregatorForClient( + client: Client, + Aggregator: MetricsAggregatorConstructor, +): MetricsAggregatorInterface { + if (!globalMetricsAggregators) { + globalMetricsAggregators = new Map(); + } + + const aggregator = globalMetricsAggregators.get(client); + if (aggregator) { + return aggregator; + } + + const newAggregator = new Aggregator(client); + client.on('flush', () => newAggregator.flush()); + client.on('close', () => newAggregator.close()); + globalMetricsAggregators.set(client, newAggregator); + + return newAggregator; +} function addToMetricsAggregator( Aggregator: MetricsAggregatorConstructor, @@ -37,38 +61,32 @@ function addToMetricsAggregator( value: number | string, data: MetricData | undefined = {}, ): void { - const client = getClient>(); + const client = data.client || getClient(); + if (!client) { return; } - if (!globalMetricsAggregator) { - const aggregator = (globalMetricsAggregator = new Aggregator(client)); - - client.on('flush', () => aggregator.flush()); - client.on('close', () => aggregator.close()); + const scope = getCurrentScope(); + const { unit, tags, timestamp } = data; + const { release, environment } = client.getOptions(); + // eslint-disable-next-line deprecation/deprecation + const transaction = scope.getTransaction(); + const metricTags: Record = {}; + if (release) { + metricTags.release = release; } - - if (client) { - const scope = getCurrentScope(); - const { unit, tags, timestamp } = data; - const { release, environment } = client.getOptions(); - // eslint-disable-next-line deprecation/deprecation - const transaction = scope.getTransaction(); - const metricTags: Record = {}; - if (release) { - metricTags.release = release; - } - if (environment) { - metricTags.environment = environment; - } - if (transaction) { - metricTags.transaction = spanToJSON(transaction).description || ''; - } - - DEBUG_BUILD && logger.log(`Adding value of ${value} to ${metricType} metric ${name}`); - globalMetricsAggregator.add(metricType, name, value, unit, { ...metricTags, ...tags }, timestamp); + if (environment) { + metricTags.environment = environment; + } + if (transaction) { + metricTags.transaction = spanToJSON(transaction).description || ''; } + + DEBUG_BUILD && logger.log(`Adding value of ${value} to ${metricType} metric ${name}`); + + const aggregator = getMetricsAggregatorForClient(client, Aggregator); + aggregator.add(metricType, name, value, unit, { ...metricTags, ...tags }, timestamp); } /** @@ -112,4 +130,8 @@ export const metrics = { distribution, set, gauge, + /** + * @ignore This is for internal use only. + */ + getMetricsAggregatorForClient, }; diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index 23eede42d1e3..2bee7d3047ad 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -170,6 +170,9 @@ export interface Client { /** Submits the session to Sentry */ sendSession(session: Session | SessionAggregates): void; + /** Sends an envelope to Sentry */ + sendEnvelope(envelope: Envelope): PromiseLike; + /** * Record on the client that an event got dropped (ie, an event that will not be sent to sentry). * From d5e729baf8e0f98832306ba95bc934467b0f0853 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 6 Mar 2024 16:04:02 +0000 Subject: [PATCH 2/3] spell --- packages/core/src/metrics/exports.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index 51517d27170c..defcbf4ad7e4 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -24,7 +24,7 @@ type MetricsAggregatorConstructor = { }; /** - * Global metrics aggregators instances oper Client. + * Global metrics aggregator instance per Client. */ let globalMetricsAggregators: Map | undefined; From 5d9bbdb7fec72775366f2721afb6b2108216675e Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 6 Mar 2024 16:45:34 +0000 Subject: [PATCH 3/3] Use `WeakMap` --- packages/core/src/metrics/exports.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index defcbf4ad7e4..c946f3780f8b 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -24,9 +24,9 @@ type MetricsAggregatorConstructor = { }; /** - * Global metrics aggregator instance per Client. + * A metrics aggregator instance per Client. */ -let globalMetricsAggregators: Map | undefined; +let globalMetricsAggregators: WeakMap | undefined; /** * Gets the metrics aggregator for a given client. @@ -38,7 +38,7 @@ function getMetricsAggregatorForClient( Aggregator: MetricsAggregatorConstructor, ): MetricsAggregatorInterface { if (!globalMetricsAggregators) { - globalMetricsAggregators = new Map(); + globalMetricsAggregators = new WeakMap(); } const aggregator = globalMetricsAggregators.get(client);