From 5d95eca02f21e816d3bc3d7396496c2f1baacb92 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 7 Dec 2023 22:27:39 -0500 Subject: [PATCH 01/20] feat(browser): Add brower metrics sdk --- packages/browser/src/exports.ts | 5 + packages/core/src/baseclient.ts | 11 ++ packages/core/src/index.ts | 7 ++ packages/core/src/metrics/constants.ts | 7 ++ .../src/metrics/{index.ts => envelope.ts} | 12 +- packages/core/src/metrics/exports.ts | 79 ++++++++++++ packages/core/src/metrics/instance.ts | 106 ++++++++++++++++ packages/core/src/metrics/integration.ts | 38 ++++++ packages/core/src/metrics/simpleaggregator.ts | 119 ++++++++++++++++++ packages/core/src/metrics/types.ts | 31 +++++ packages/core/src/metrics/utils.ts | 16 +++ .../test/lib/metrics/simpleaggregator.test.ts | 57 +++++++++ packages/core/test/lib/metrics/utils.test.ts | 21 ++++ packages/types/src/envelope.ts | 2 +- packages/types/src/index.ts | 1 - packages/types/src/metrics.ts | 32 ----- 16 files changed, 502 insertions(+), 42 deletions(-) create mode 100644 packages/core/src/metrics/constants.ts rename packages/core/src/metrics/{index.ts => envelope.ts} (65%) create mode 100644 packages/core/src/metrics/exports.ts create mode 100644 packages/core/src/metrics/instance.ts create mode 100644 packages/core/src/metrics/integration.ts create mode 100644 packages/core/src/metrics/simpleaggregator.ts create mode 100644 packages/core/src/metrics/types.ts create mode 100644 packages/core/src/metrics/utils.ts create mode 100644 packages/core/test/lib/metrics/simpleaggregator.test.ts create mode 100644 packages/core/test/lib/metrics/utils.test.ts delete mode 100644 packages/types/src/metrics.ts diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 0804dbe49ac0..d7edf9d7f389 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -56,6 +56,11 @@ export { withScope, FunctionToString, InboundFilters, + incr, + distribution, + set, + gauge, + Metrics, } from '@sentry/core'; export { WINDOW } from './helpers'; diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 0ea80e229c58..3246e5955549 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -49,6 +49,7 @@ import { createEventEnvelope, createSessionEnvelope } from './envelope'; import { getCurrentHub } from './hub'; import type { IntegrationIndex } from './integration'; import { setupIntegration, setupIntegrations } from './integration'; +import type { MetricsAggregator } from './metrics/types'; import type { Scope } from './scope'; import { updateSession } from './session'; import { getDynamicSamplingContextFromClient } from './tracing/dynamicSamplingContext'; @@ -88,6 +89,13 @@ const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been ca * } */ export abstract class BaseClient implements Client { + /** + * A reference to a metrics aggregator + * + * @experimental Note this is alpha API. It may experience breaking changes in the future. + */ + public metricsAggregator: MetricsAggregator | undefined; + /** Options passed to the SDK. */ protected readonly _options: O; @@ -264,6 +272,9 @@ export abstract class BaseClient implements Client { public flush(timeout?: number): PromiseLike { const transport = this._transport; if (transport) { + if (this.metricsAggregator) { + this.metricsAggregator.flush(); + } return this._isClientDoneProcessing(timeout).then(clientFinished => { return transport.flush(timeout).then(transportFlushed => clientFinished && transportFlushed); }); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 07871633f16c..3e75c6bad530 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -63,5 +63,12 @@ export { DEFAULT_ENVIRONMENT } from './constants'; export { ModuleMetadata } from './integrations/metadata'; export { RequestData } from './integrations/requestdata'; import * as Integrations from './integrations'; +export { + incr, + distribution, + set, + gauge, +} from './metrics/exports'; +export { Metrics } from './metrics/integration'; export { Integrations }; diff --git a/packages/core/src/metrics/constants.ts b/packages/core/src/metrics/constants.ts new file mode 100644 index 000000000000..b5871a7ee7ef --- /dev/null +++ b/packages/core/src/metrics/constants.ts @@ -0,0 +1,7 @@ +export const COUNTER_METRIC_TYPE = 'c'; +export const GAUGE_METRIC_TYPE = 'g'; +export const SET_METRIC_TYPE = 's'; +export const DISTRIBUTION_METRIC_TYPE = 'd'; + +export const NAME_AND_TAG_KEY_REGEX = /[^a-zA-Z0-9_/.-]+"/g; +export const TAG_VALUE_REGEX = /[^\w\d_:/@.{}[\]$-]+/g; diff --git a/packages/core/src/metrics/index.ts b/packages/core/src/metrics/envelope.ts similarity index 65% rename from packages/core/src/metrics/index.ts rename to packages/core/src/metrics/envelope.ts index 3cfeae37e03f..5f4d0895bdd5 100644 --- a/packages/core/src/metrics/index.ts +++ b/packages/core/src/metrics/envelope.ts @@ -1,13 +1,11 @@ -import type { DsnComponents, DynamicSamplingContext, SdkMetadata, StatsdEnvelope, StatsdItem } from '@sentry/types'; -import { createEnvelope, dropUndefinedKeys, dsnToString } from '@sentry/utils'; +import type { DsnComponents, SdkMetadata, StatsdEnvelope, StatsdItem } from '@sentry/types'; +import { createEnvelope, dsnToString } from '@sentry/utils'; /** * Create envelope from a metric aggregate. */ export function createMetricEnvelope( - // TODO(abhi): Add type for this metricAggregate: string, - dynamicSamplingContext?: Partial, metadata?: SdkMetadata, tunnel?: string, dsn?: DsnComponents, @@ -27,10 +25,6 @@ export function createMetricEnvelope( headers.dsn = dsnToString(dsn); } - if (dynamicSamplingContext) { - headers.trace = dropUndefinedKeys(dynamicSamplingContext) as DynamicSamplingContext; - } - const item = createMetricEnvelopeItem(metricAggregate); return createEnvelope(headers, [item]); } @@ -38,6 +32,8 @@ export function createMetricEnvelope( function createMetricEnvelopeItem(metricAggregate: string): StatsdItem { const metricHeaders: StatsdItem[0] = { type: 'statsd', + content_type: 'application/octet-stream', + length: metricAggregate.length, }; return [metricHeaders, metricAggregate]; } diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts new file mode 100644 index 000000000000..aae6151fd748 --- /dev/null +++ b/packages/core/src/metrics/exports.ts @@ -0,0 +1,79 @@ +import type { ClientOptions, MeasurementUnit, Primitive } from '@sentry/types'; +import { logger } from '@sentry/utils'; +import type { BaseClient } from '../baseclient'; +import { DEBUG_BUILD } from '../debug-build'; +import { getCurrentHub } from '../hub'; +import type { MetricType } from './types'; + +interface MetricData { + unit?: MeasurementUnit; + tags?: { [key: string]: Primitive }; + timestamp?: number; +} + +function addToMetricsAggregator(metricType: MetricType, name: string, value: number, data: MetricData = {}): void { + const hub = getCurrentHub(); + const client = hub.getClient() as BaseClient; + const scope = hub.getScope(); + if (client) { + if (!client.metricsAggregator) { + DEBUG_BUILD && + logger.warn('No metrics aggregator enabled. Please add the Metrics integration to use metrics APIs'); + return; + } + const { unit, tags, timestamp } = data; + const { release, environment } = client.getOptions(); + const transaction = scope.getTransaction(); + const metricTags = { + ...tags, + }; + if (release) { + metricTags.release = release; + } + if (environment) { + metricTags.environment = environment; + } + if (transaction) { + metricTags.transaction = transaction.name; + } + + DEBUG_BUILD && logger.log(`Adding value of ${value} to ${metricType} metric ${name}`); + client.metricsAggregator.add(metricType, name, value, unit, metricTags, timestamp); + } +} + +/** + * Adds a value to a counter metric + * + * @experimental This API is experimental and might having breaking changes in the future. + */ +export function incr(name: string, value: number, data?: MetricData): void { + addToMetricsAggregator('c', name, value, data); +} + +/** + * Adds a value to a distribution metric + * + * @experimental This API is experimental and might having breaking changes in the future. + */ +export function distribution(name: string, value: number, data?: MetricData): void { + addToMetricsAggregator('d', name, value, data); +} + +/** + * Adds a value to a set metric + * + * @experimental This API is experimental and might having breaking changes in the future. + */ +export function set(name: string, value: number, data?: MetricData): void { + addToMetricsAggregator('s', name, value, data); +} + +/** + * Adds a value to a gauge metric + * + * @experimental This API is experimental and might having breaking changes in the future. + */ +export function gauge(name: string, value: number, data?: MetricData): void { + addToMetricsAggregator('s', name, value, data); +} diff --git a/packages/core/src/metrics/instance.ts b/packages/core/src/metrics/instance.ts new file mode 100644 index 000000000000..4856b4a4e54c --- /dev/null +++ b/packages/core/src/metrics/instance.ts @@ -0,0 +1,106 @@ +interface MetricInstance { + add(value: number): void; + toString(): string; +} + +/** + * A metric instance representing a counter. + */ +export class CounterMetric implements MetricInstance { + public constructor(public value: number) {} + + /** JSDoc */ + public add(value: number): void { + this.value += value; + } + + /** JSDoc */ + public toString(): string { + return `${this.value}`; + } +} + +/** + * A metric instance representing a gauge. + */ +export class GaugeMetric implements MetricInstance { + public last: number; + public min: number; + public max: number; + public sum: number; + public count: number; + + public constructor(public value: number) { + this.last = value; + this.min = value; + this.max = value; + this.sum = value; + this.count = 1; + } + + /** JSDoc */ + public add(value: number): void { + this.value = value; + this.last = value; + this.min = Math.min(this.min, value); + this.max = Math.max(this.max, value); + this.sum += value; + this.count += 1; + } + + /** JSDoc */ + public toString(): string { + return `${this.last}:${this.min}:${this.max}:${this.sum}:${this.count}`; + } +} + +/** + * A metric instance representing a distribution. + */ +export class DistributionMetric implements MetricInstance { + public value: number[]; + + public constructor(first: number) { + this.value = [first]; + } + + /** JSDoc */ + public add(value: number): void { + this.value.push(value); + } + + /** JSDoc */ + public toString(): string { + return this.value.join(':'); + } +} + +/** + * A metric instance representing a set. + */ +export class SetMetric implements MetricInstance { + public value: Set; + + public constructor(public first: number) { + this.value = new Set([first]); + } + + /** JSDoc */ + public add(value: number): void { + this.value.add(value); + } + + /** JSDoc */ + public toString(): string { + return `${Array.from(this.value).join(':')}`; + } +} + +export type Metric = CounterMetric | GaugeMetric | DistributionMetric | SetMetric; + +export const METRIC_MAP = { + c: CounterMetric, + g: GaugeMetric, + d: DistributionMetric, + s: SetMetric, +}; diff --git a/packages/core/src/metrics/integration.ts b/packages/core/src/metrics/integration.ts new file mode 100644 index 000000000000..250ac28862d7 --- /dev/null +++ b/packages/core/src/metrics/integration.ts @@ -0,0 +1,38 @@ +import type { ClientOptions, Integration } from '@sentry/types'; +import type { BaseClient } from '../baseclient'; +import { SimpleMetricsAggregator } from './simpleaggregator'; + +/** + * Enables Sentry metrics monitoring. + * + * @experimental This API is experimental and might having breaking changes in the future. + */ +export class Metrics implements Integration { + /** + * @inheritDoc + */ + public static id: string = 'Metrics'; + + /** + * @inheritDoc + */ + public name: string; + + public constructor() { + this.name = Metrics.id; + } + + /** + * @inheritDoc + */ + public setupOnce(): void { + // Do nothing + } + + /** + * @inheritDoc + */ + public setup(client: BaseClient): void { + client.metricsAggregator = new SimpleMetricsAggregator(client); + } +} diff --git a/packages/core/src/metrics/simpleaggregator.ts b/packages/core/src/metrics/simpleaggregator.ts new file mode 100644 index 000000000000..46bfc533374f --- /dev/null +++ b/packages/core/src/metrics/simpleaggregator.ts @@ -0,0 +1,119 @@ +import type { ClientOptions, MeasurementUnit, Primitive } from '@sentry/types'; +import { timestampInSeconds } from '@sentry/utils'; +import type { BaseClient } from '../baseclient'; +import { NAME_AND_TAG_KEY_REGEX, TAG_VALUE_REGEX } from './constants'; +import { createMetricEnvelope } from './envelope'; +import type { Metric } from './instance'; +import { METRIC_MAP } from './instance'; +import type { MetricType, MetricsAggregator } from './types'; +import { getBucketKey } from './utils'; + +type SimpleMetricBucket = Map< + string, + [ + metric: Metric, + timestamp: number, + metricType: MetricType, + name: string, + unit: MeasurementUnit, + tags: { [key: string]: string }, + ] +>; + +/** + * A simple metrics aggregator that aggregates metrics in memory and flushes them periodically. + * + * @experimental This API is experimental and might change in the future. + */ +export class SimpleMetricsAggregator implements MetricsAggregator { + private _buckets: SimpleMetricBucket; + + public constructor(private readonly _client: BaseClient) { + this._buckets = new Map(); + } + + /** + * @inheritDoc + */ + public add( + metricType: MetricType, + unsanitizedName: string, + value: number, + unit: MeasurementUnit = 'none', + unsanitizedTags: { [key: string]: Primitive } = {}, + maybeFloatTimestamp = timestampInSeconds(), + ): void { + const timestamp = Math.floor(maybeFloatTimestamp); + const name = unsanitizedName.replace(NAME_AND_TAG_KEY_REGEX, '_'); + const tags = sanitizeTags(unsanitizedTags); + + const bucketKey = getBucketKey(metricType, name, unit, tags); + const bucketItem = this._buckets.get(bucketKey); + if (bucketItem) { + const [bucketMetric, bucketTimestamp] = bucketItem; + bucketMetric.add(value); + // TODO(abhi): Do we need this check? + if (bucketTimestamp < timestamp) { + bucketItem[1] = timestamp; + } + } else { + const newMetric = new METRIC_MAP[metricType](value); + this._buckets.set(bucketKey, [newMetric, timestamp, metricType, name, unit, tags]); + } + } + + /** + * @inheritDoc + */ + public flush(): void { + // short circuit if buckets are empty. + if (this._buckets.size === 0) { + return; + } + + // Allow of this logic should be in the client, but we want to + const metrics = serializeBuckets(this._buckets); + const sdkMetadata = this._client.getSdkMetadata && this._client.getSdkMetadata(); + const metricsEnvelope = createMetricEnvelope( + metrics, + sdkMetadata, + this._client.getOptions().tunnel, + this._client.getDsn(), + ); + + // TODO(abhi): Remove this hack - only here until we decide on final API for metrics on client + void this._client['_sendEnvelope'](metricsEnvelope); + this._buckets.clear(); + } +} + +/** + * Serialize metrics buckets into a string based on statsd format. + */ +export function serializeBuckets(buckets: SimpleMetricBucket): string { + let out = ''; + buckets.forEach(([metric, timestamp, metricType, name, unit, tags]) => { + out += `${name}@${unit}:${metric}|${metricType}`; + if (Object.keys(tags).length) { + out += '|#'; + out += Object.entries(tags) + .map(([key, value]) => `${key}:${String(value)}`) + .join(','); + } + // timestamp must be an integer + out += `|T${timestamp}\n`; + }); + + return out; +} + +function sanitizeTags(unsanitizedTags: { [key: string]: Primitive }): { [key: string]: string } { + const tags: { [key: string]: string } = {}; + for (const key in unsanitizedTags) { + if (Object.prototype.hasOwnProperty.call(unsanitizedTags, key)) { + const sanitizedKey = key.replace(NAME_AND_TAG_KEY_REGEX, '_'); + tags[sanitizedKey] = String(unsanitizedTags[key]).replace(TAG_VALUE_REGEX, '_'); + } + } + return tags; +} diff --git a/packages/core/src/metrics/types.ts b/packages/core/src/metrics/types.ts new file mode 100644 index 000000000000..fcf87d35fecf --- /dev/null +++ b/packages/core/src/metrics/types.ts @@ -0,0 +1,31 @@ +import type { MeasurementUnit, Primitive } from '@sentry/types'; +import type { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants'; +import { Metric } from './instance'; + +export type MetricType = + | typeof COUNTER_METRIC_TYPE + | typeof GAUGE_METRIC_TYPE + | typeof SET_METRIC_TYPE + | typeof DISTRIBUTION_METRIC_TYPE; + +/** + * A metrics aggregator that aggregates metrics in memory and flushes them periodically. + */ +export interface MetricsAggregator { + /** + * Add a metric to the aggregator. + */ + add( + metricType: MetricType, + name: string, + value: number, + unit?: MeasurementUnit, + tags?: { [key: string]: Primitive }, + timestamp?: number, + ): void; + + /** + * Flushes the current metrics to the transport via the transport. + */ + flush(): void; +} diff --git a/packages/core/src/metrics/utils.ts b/packages/core/src/metrics/utils.ts new file mode 100644 index 000000000000..8c4eeae8ef19 --- /dev/null +++ b/packages/core/src/metrics/utils.ts @@ -0,0 +1,16 @@ +import type { MeasurementUnit, Primitive } from '@sentry/types'; +import { dropUndefinedKeys } from '@sentry/utils'; +import type { MetricType } from './types'; + +/** + * Generate bucket key from metric properties. + */ +export function getBucketKey( + metricType: MetricType, + name: string, + unit: MeasurementUnit, + tags: { [key: string]: string }, +): string { + const stringifiedTags = Object.entries(dropUndefinedKeys(tags)).sort() as unknown as string[]; + return [metricType, name, unit].concat(stringifiedTags).join(''); +} diff --git a/packages/core/test/lib/metrics/simpleaggregator.test.ts b/packages/core/test/lib/metrics/simpleaggregator.test.ts new file mode 100644 index 000000000000..7998f547d09a --- /dev/null +++ b/packages/core/test/lib/metrics/simpleaggregator.test.ts @@ -0,0 +1,57 @@ +import { CounterMetric } from '../../../src/metrics/instance'; +import { SimpleMetricsAggregator, serializeBuckets } from '../../../src/metrics/simpleaggregator'; +import { TestClient, getDefaultTestClientOptions } from '../../mocks/client'; + +describe('SimpleMetricsAggregator', () => { + const options = getDefaultTestClientOptions({ tracesSampleRate: 0.0 }); + const testClient = new TestClient(options); + + it('adds items to buckets', () => { + const aggregator = new SimpleMetricsAggregator(testClient); + aggregator.add('c', 'requests', 1); + expect(aggregator['_buckets'].size).toEqual(1); + + const firstValue = aggregator['_buckets'].values().next().value; + expect(firstValue).toEqual([expect.any(CounterMetric), expect.any(Number), 'c', 'requests', 'none', {}]); + }); + + it('groups same items together', () => { + const aggregator = new SimpleMetricsAggregator(testClient); + aggregator.add('c', 'requests', 1); + expect(aggregator['_buckets'].size).toEqual(1); + aggregator.add('c', 'requests', 1); + expect(aggregator['_buckets'].size).toEqual(1); + + const firstValue = aggregator['_buckets'].values().next().value; + expect(firstValue).toEqual([expect.any(CounterMetric), expect.any(Number), 'c', 'requests', 'none', {}]); + + expect(firstValue[0].value).toEqual(2); + }); + + it('differentiates based on tag value', () => { + const aggregator = new SimpleMetricsAggregator(testClient); + aggregator.add('g', 'cpu', 50); + expect(aggregator['_buckets'].size).toEqual(1); + aggregator.add('g', 'cpu', 55, undefined, { a: 'value' }); + expect(aggregator['_buckets'].size).toEqual(2); + }); + + describe('serializeBuckets', () => { + it('serializes ', () => { + const aggregator = new SimpleMetricsAggregator(testClient); + aggregator.add('c', 'requests', 8); + aggregator.add('g', 'cpu', 50); + aggregator.add('g', 'cpu', 55); + aggregator.add('g', 'cpu', 52); + aggregator.add('d', 'lcp', 1, 'second', { a: 'value', b: 'anothervalue' }); + aggregator.add('d', 'lcp', 1.2, 'second', { a: 'value', b: 'anothervalue' }); + aggregator.add('s', 'important_org_ids', 1, 'none', { numericKey: 2 }); + aggregator.add('s', 'important_org_ids', 2, 'none', { numericKey: 2 }); + + expect(serializeBuckets(aggregator['_buckets'])).toContain('requests@none:8|c|T'); + expect(serializeBuckets(aggregator['_buckets'])).toContain('cpu@none:52:50:55:157:3|g|T'); + expect(serializeBuckets(aggregator['_buckets'])).toContain('lcp@second:1:1.2|d|#a:value,b:anothervalue|T'); + expect(serializeBuckets(aggregator['_buckets'])).toContain('important_org_ids@none:1:2|s|#numericKey:2|T'); + }); + }); +}); diff --git a/packages/core/test/lib/metrics/utils.test.ts b/packages/core/test/lib/metrics/utils.test.ts new file mode 100644 index 000000000000..63a8fe697b1d --- /dev/null +++ b/packages/core/test/lib/metrics/utils.test.ts @@ -0,0 +1,21 @@ +import { getBucketKey } from '../../../src/metrics/utils'; + +describe('getBucketKey', () => { + it.each([ + ['c' as const, 'requests', 'none', {}, 'crequestsnone'], + ['g' as const, 'cpu', 'none', {}, 'gcpunone'], + ['d' as const, 'lcp', 'second', { a: 'value', b: 'anothervalue' }, 'dlcpseconda,valueb,anothervalue'], + ['d' as const, 'lcp', 'second', { a: 'value', b: 'anothervalue' }, 'dlcpseconda,valueb,anothervalue'], + ['d' as const, 'lcp', 'second', { numericKey: 2 }, 'dlcpsecondnumericKey,2'], + ['d' as const, 'lcp', 'second', { undefinedKey: undefined, numericKey: 2 }, 'dlcpsecondnumericKey,2'], + [ + 's' as const, + 'important_org_ids', + 'none', + { undefinedKey: undefined, numericKey: 2 }, + 'simportant_org_idsnonenumericKey,2', + ], + ])('should return', (metricType, name, unit, tags, expected) => { + expect(getBucketKey(metricType, name, unit, tags)).toEqual(expected); + }); +}); diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts index fbe593dd21f9..f0173c64f45c 100644 --- a/packages/types/src/envelope.ts +++ b/packages/types/src/envelope.ts @@ -76,7 +76,7 @@ type ClientReportItemHeaders = { type: 'client_report' }; type ReplayEventItemHeaders = { type: 'replay_event' }; type ReplayRecordingItemHeaders = { type: 'replay_recording'; length: number }; type CheckInItemHeaders = { type: 'check_in' }; -type StatsdItemHeaders = { type: 'statsd' }; +type StatsdItemHeaders = { type: 'statsd'; content_type: 'application/octet-stream'; length: number }; export type EventItem = BaseEnvelopeItem; export type AttachmentItem = BaseEnvelopeItem; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 615bf71ff785..bc1cdd10de60 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -138,4 +138,3 @@ export type { export type { BrowserClientReplayOptions, BrowserClientProfilingOptions } from './browseroptions'; export type { CheckIn, MonitorConfig, FinishedCheckIn, InProgressCheckIn, SerializedCheckIn } from './checkin'; -export type { Metric, CounterMetric, GaugeMetric, DistributionMetric, SetMetric } from './metrics'; diff --git a/packages/types/src/metrics.ts b/packages/types/src/metrics.ts deleted file mode 100644 index b55096950b2f..000000000000 --- a/packages/types/src/metrics.ts +++ /dev/null @@ -1,32 +0,0 @@ -import type { MeasurementUnit } from './measurement'; -import type { Primitive } from './misc'; - -export interface BaseMetric { - name: string; - timestamp: number; - unit?: MeasurementUnit; - tags?: { [key: string]: Primitive }; -} - -export interface CounterMetric extends BaseMetric { - value: number; -} - -export interface GaugeMetric extends BaseMetric { - value: number; - first: number; - min: number; - max: number; - sum: number; - count: number; -} - -export interface DistributionMetric extends BaseMetric { - value: number[]; -} - -export interface SetMetric extends BaseMetric { - value: Set; -} - -export type Metric = CounterMetric | GaugeMetric | DistributionMetric | SetMetric; From f1cd3118594520b4e3bfb5544950074cc1e46dbc Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 11 Dec 2023 08:52:23 -0500 Subject: [PATCH 02/20] refactor client logic --- packages/core/src/baseclient.ts | 17 ++++++++++ packages/core/src/metrics/constants.ts | 2 ++ packages/core/src/metrics/envelope.ts | 2 +- packages/core/src/metrics/simpleaggregator.ts | 33 ++++++++++--------- packages/core/src/metrics/types.ts | 5 +++ packages/types/src/client.ts | 7 ++++ 6 files changed, 49 insertions(+), 17 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 3246e5955549..2f6161763ed0 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -49,6 +49,7 @@ import { createEventEnvelope, createSessionEnvelope } from './envelope'; import { getCurrentHub } from './hub'; import type { IntegrationIndex } from './integration'; import { setupIntegration, setupIntegrations } from './integration'; +import { createMetricEnvelope } from './metrics/envelope'; import type { MetricsAggregator } from './metrics/types'; import type { Scope } from './scope'; import { updateSession } from './session'; @@ -289,6 +290,9 @@ export abstract class BaseClient implements Client { public close(timeout?: number): PromiseLike { return this.flush(timeout).then(result => { this.getOptions().enabled = false; + if (this.metricsAggregator) { + this.metricsAggregator.close(); + } return result; }); } @@ -394,6 +398,19 @@ export abstract class BaseClient implements Client { } } + /** + * @inheritDoc + */ + public captureSerializedMetrics(serializedMetrics: string): void { + const metricsEnvelope = createMetricEnvelope( + serializedMetrics, + this._dsn, + this._options._metadata, + this._options.tunnel, + ); + void this._sendEnvelope(metricsEnvelope); + } + // Keep on() & emit() signatures in sync with types' client.ts interface /* eslint-disable @typescript-eslint/unified-signatures */ diff --git a/packages/core/src/metrics/constants.ts b/packages/core/src/metrics/constants.ts index b5871a7ee7ef..64173ba5a4fc 100644 --- a/packages/core/src/metrics/constants.ts +++ b/packages/core/src/metrics/constants.ts @@ -5,3 +5,5 @@ export const DISTRIBUTION_METRIC_TYPE = 'd'; export const NAME_AND_TAG_KEY_REGEX = /[^a-zA-Z0-9_/.-]+"/g; export const TAG_VALUE_REGEX = /[^\w\d_:/@.{}[\]$-]+/g; + +export const DEFAULT_FLUSH_INTERVAL = 5000; diff --git a/packages/core/src/metrics/envelope.ts b/packages/core/src/metrics/envelope.ts index 5f4d0895bdd5..b50cfef67b33 100644 --- a/packages/core/src/metrics/envelope.ts +++ b/packages/core/src/metrics/envelope.ts @@ -6,9 +6,9 @@ import { createEnvelope, dsnToString } from '@sentry/utils'; */ export function createMetricEnvelope( metricAggregate: string, + dsn?: DsnComponents, metadata?: SdkMetadata, tunnel?: string, - dsn?: DsnComponents, ): StatsdEnvelope { const headers: StatsdEnvelope[0] = { sent_at: new Date().toISOString(), diff --git a/packages/core/src/metrics/simpleaggregator.ts b/packages/core/src/metrics/simpleaggregator.ts index 46bfc533374f..d2d977994f0c 100644 --- a/packages/core/src/metrics/simpleaggregator.ts +++ b/packages/core/src/metrics/simpleaggregator.ts @@ -1,8 +1,6 @@ -import type { ClientOptions, MeasurementUnit, Primitive } from '@sentry/types'; +import type { Client, ClientOptions, MeasurementUnit, Primitive } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; -import type { BaseClient } from '../baseclient'; -import { NAME_AND_TAG_KEY_REGEX, TAG_VALUE_REGEX } from './constants'; -import { createMetricEnvelope } from './envelope'; +import { DEFAULT_FLUSH_INTERVAL, NAME_AND_TAG_KEY_REGEX, TAG_VALUE_REGEX } from './constants'; import type { Metric } from './instance'; import { METRIC_MAP } from './instance'; import type { MetricType, MetricsAggregator } from './types'; @@ -22,14 +20,17 @@ type SimpleMetricBucket = Map< /** * A simple metrics aggregator that aggregates metrics in memory and flushes them periodically. + * Default flush interval is 5 seconds. * * @experimental This API is experimental and might change in the future. */ export class SimpleMetricsAggregator implements MetricsAggregator { private _buckets: SimpleMetricBucket; + 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_FLUSH_INTERVAL); } /** @@ -71,18 +72,18 @@ export class SimpleMetricsAggregator implements MetricsAggregator { return; } - // Allow of this logic should be in the client, but we want to - const metrics = serializeBuckets(this._buckets); - const sdkMetadata = this._client.getSdkMetadata && this._client.getSdkMetadata(); - const metricsEnvelope = createMetricEnvelope( - metrics, - sdkMetadata, - this._client.getOptions().tunnel, - this._client.getDsn(), - ); + if (this._client.captureSerializedMetrics) { + this._client.captureSerializedMetrics(serializeBuckets(this._buckets)); + } + this._buckets.clear(); + } - // TODO(abhi): Remove this hack - only here until we decide on final API for metrics on client - void this._client['_sendEnvelope'](metricsEnvelope); + /** + * @inheritDoc + */ + public close(): void { + clearInterval(this._interval); + this.flush(); this._buckets.clear(); } } diff --git a/packages/core/src/metrics/types.ts b/packages/core/src/metrics/types.ts index fcf87d35fecf..696d96186d11 100644 --- a/packages/core/src/metrics/types.ts +++ b/packages/core/src/metrics/types.ts @@ -28,4 +28,9 @@ export interface MetricsAggregator { * Flushes the current metrics to the transport via the transport. */ flush(): void; + + /** + * Shuts down metrics aggregator and clears all metrics. + */ + close(): void; } diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index 33fa749eb379..b556f15ec2c2 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -179,6 +179,13 @@ export interface Client { */ recordDroppedEvent(reason: EventDropReason, dataCategory: DataCategory, event?: Event): void; + /** + * Captures serialized metrics and sends them to Sentry. + * + * @experimental This API is experimental and might experience breaking changes + */ + captureSerializedMetrics?(serializedMetrics: string): void; + // HOOKS // TODO(v8): Make the hooks non-optional. /* eslint-disable @typescript-eslint/unified-signatures */ From 33395d6a1b8d17f3cd1d97efe684dac4b3aa1928 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 11 Dec 2023 12:37:13 -0500 Subject: [PATCH 03/20] change aggregator type --- packages/core/src/baseclient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 2f6161763ed0..06a0722a20cd 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -95,7 +95,7 @@ export abstract class BaseClient implements Client { * * @experimental Note this is alpha API. It may experience breaking changes in the future. */ - public metricsAggregator: MetricsAggregator | undefined; + public metricsAggregator?: MetricsAggregator; /** Options passed to the SDK. */ protected readonly _options: O; From b48096f6bf2e39c9cf890cc27eed51ee15cdd4eb Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 11 Dec 2023 12:43:58 -0500 Subject: [PATCH 04/20] add details about regex and improve name --- packages/core/src/metrics/constants.ts | 21 +++++++++++++++++-- packages/core/src/metrics/simpleaggregator.ts | 12 +++++++---- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/packages/core/src/metrics/constants.ts b/packages/core/src/metrics/constants.ts index 64173ba5a4fc..419b7be382ef 100644 --- a/packages/core/src/metrics/constants.ts +++ b/packages/core/src/metrics/constants.ts @@ -3,7 +3,24 @@ export const GAUGE_METRIC_TYPE = 'g'; export const SET_METRIC_TYPE = 's'; export const DISTRIBUTION_METRIC_TYPE = 'd'; -export const NAME_AND_TAG_KEY_REGEX = /[^a-zA-Z0-9_/.-]+"/g; -export const TAG_VALUE_REGEX = /[^\w\d_:/@.{}[\]$-]+/g; +/** + * Normalization regex for metric names and metric tag names. + * + * This enforces that names and tag keys only contain alphanumeric characters, + * underscores, forward slashes, periods, and dashes. + * + * See: https://develop.sentry.dev/sdk/metrics/#normalization + */ +export const NAME_AND_TAG_KEY_NORMALIZATION_REGEX = /[^a-zA-Z0-9_/.-]+/g; + +/** + * Normalization regex for metric tag balues. + * + * This enforces that values only contain words, digits, or the following + * special characters: _:/@.{}[\]$- + * + * See: https://develop.sentry.dev/sdk/metrics/#normalization + */ +export const TAG_VALUE_NORMALIZATION_REGEX = /[^\w\d_:/@.{}[\]$-]+/g; export const DEFAULT_FLUSH_INTERVAL = 5000; diff --git a/packages/core/src/metrics/simpleaggregator.ts b/packages/core/src/metrics/simpleaggregator.ts index d2d977994f0c..d372db9cca7d 100644 --- a/packages/core/src/metrics/simpleaggregator.ts +++ b/packages/core/src/metrics/simpleaggregator.ts @@ -1,6 +1,10 @@ import type { Client, ClientOptions, MeasurementUnit, Primitive } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; -import { DEFAULT_FLUSH_INTERVAL, NAME_AND_TAG_KEY_REGEX, TAG_VALUE_REGEX } from './constants'; +import { + DEFAULT_FLUSH_INTERVAL, + NAME_AND_TAG_KEY_NORMALIZATION_REGEX, + TAG_VALUE_NORMALIZATION_REGEX, +} from './constants'; import type { Metric } from './instance'; import { METRIC_MAP } from './instance'; import type { MetricType, MetricsAggregator } from './types'; @@ -45,7 +49,7 @@ export class SimpleMetricsAggregator implements MetricsAggregator { maybeFloatTimestamp = timestampInSeconds(), ): void { const timestamp = Math.floor(maybeFloatTimestamp); - const name = unsanitizedName.replace(NAME_AND_TAG_KEY_REGEX, '_'); + const name = unsanitizedName.replace(NAME_AND_TAG_KEY_NORMALIZATION_REGEX, '_'); const tags = sanitizeTags(unsanitizedTags); const bucketKey = getBucketKey(metricType, name, unit, tags); @@ -112,8 +116,8 @@ function sanitizeTags(unsanitizedTags: { [key: string]: Primitive }): { [key: st const tags: { [key: string]: string } = {}; for (const key in unsanitizedTags) { if (Object.prototype.hasOwnProperty.call(unsanitizedTags, key)) { - const sanitizedKey = key.replace(NAME_AND_TAG_KEY_REGEX, '_'); - tags[sanitizedKey] = String(unsanitizedTags[key]).replace(TAG_VALUE_REGEX, '_'); + const sanitizedKey = key.replace(NAME_AND_TAG_KEY_NORMALIZATION_REGEX, '_'); + tags[sanitizedKey] = String(unsanitizedTags[key]).replace(TAG_VALUE_NORMALIZATION_REGEX, '_'); } } return tags; From e698dfacd1ff84823f53763962cffb30784db7e2 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 11 Dec 2023 12:45:53 -0500 Subject: [PATCH 05/20] clarify flush interval --- packages/core/src/metrics/constants.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/core/src/metrics/constants.ts b/packages/core/src/metrics/constants.ts index 419b7be382ef..3f4a53fcacb7 100644 --- a/packages/core/src/metrics/constants.ts +++ b/packages/core/src/metrics/constants.ts @@ -23,4 +23,8 @@ export const NAME_AND_TAG_KEY_NORMALIZATION_REGEX = /[^a-zA-Z0-9_/.-]+/g; */ export const TAG_VALUE_NORMALIZATION_REGEX = /[^\w\d_:/@.{}[\]$-]+/g; +/** + * This does not match spec in https://develop.sentry.dev/sdk/metrics + * but was chosen to optimize for the most common case in browser environments. + */ export const DEFAULT_FLUSH_INTERVAL = 5000; From f5d633325e67620fb8ce080a1cae885ec2bed6fd Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 11 Dec 2023 12:48:12 -0500 Subject: [PATCH 06/20] use metric constants --- packages/core/src/metrics/exports.ts | 9 +++++---- packages/core/src/metrics/instance.ts | 10 ++++++---- packages/core/src/metrics/types.ts | 1 - 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index aae6151fd748..640f12a820ff 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -3,6 +3,7 @@ import { logger } from '@sentry/utils'; import type { BaseClient } from '../baseclient'; import { DEBUG_BUILD } from '../debug-build'; import { getCurrentHub } from '../hub'; +import { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants'; import type { MetricType } from './types'; interface MetricData { @@ -48,7 +49,7 @@ function addToMetricsAggregator(metricType: MetricType, name: string, value: num * @experimental This API is experimental and might having breaking changes in the future. */ export function incr(name: string, value: number, data?: MetricData): void { - addToMetricsAggregator('c', name, value, data); + addToMetricsAggregator(COUNTER_METRIC_TYPE, name, value, data); } /** @@ -57,7 +58,7 @@ export function incr(name: string, value: number, data?: MetricData): void { * @experimental This API is experimental and might having breaking changes in the future. */ export function distribution(name: string, value: number, data?: MetricData): void { - addToMetricsAggregator('d', name, value, data); + addToMetricsAggregator(DISTRIBUTION_METRIC_TYPE, name, value, data); } /** @@ -66,7 +67,7 @@ export function distribution(name: string, value: number, data?: MetricData): vo * @experimental This API is experimental and might having breaking changes in the future. */ export function set(name: string, value: number, data?: MetricData): void { - addToMetricsAggregator('s', name, value, data); + addToMetricsAggregator(SET_METRIC_TYPE, name, value, data); } /** @@ -75,5 +76,5 @@ export function set(name: string, value: number, data?: MetricData): void { * @experimental This API is experimental and might having breaking changes in the future. */ export function gauge(name: string, value: number, data?: MetricData): void { - addToMetricsAggregator('s', name, value, data); + addToMetricsAggregator(GAUGE_METRIC_TYPE, name, value, data); } diff --git a/packages/core/src/metrics/instance.ts b/packages/core/src/metrics/instance.ts index 4856b4a4e54c..fe4410cda32b 100644 --- a/packages/core/src/metrics/instance.ts +++ b/packages/core/src/metrics/instance.ts @@ -1,3 +1,5 @@ +import { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants'; + interface MetricInstance { add(value: number): void; toString(): string; @@ -99,8 +101,8 @@ export class SetMetric implements MetricInstance { export type Metric = CounterMetric | GaugeMetric | DistributionMetric | SetMetric; export const METRIC_MAP = { - c: CounterMetric, - g: GaugeMetric, - d: DistributionMetric, - s: SetMetric, + [COUNTER_METRIC_TYPE]: CounterMetric, + [GAUGE_METRIC_TYPE]: GaugeMetric, + [DISTRIBUTION_METRIC_TYPE]: DistributionMetric, + [SET_METRIC_TYPE]: SetMetric, }; diff --git a/packages/core/src/metrics/types.ts b/packages/core/src/metrics/types.ts index 696d96186d11..6734d89ef477 100644 --- a/packages/core/src/metrics/types.ts +++ b/packages/core/src/metrics/types.ts @@ -1,6 +1,5 @@ import type { MeasurementUnit, Primitive } from '@sentry/types'; import type { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants'; -import { Metric } from './instance'; export type MetricType = | typeof COUNTER_METRIC_TYPE From 43ca4150bb03cc0818c5e110b4fdbb56f32ac1bc Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 11 Dec 2023 12:58:40 -0500 Subject: [PATCH 07/20] allow strings in set metric --- packages/core/src/metrics/exports.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index 640f12a820ff..58e75e1438ef 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -48,7 +48,7 @@ function addToMetricsAggregator(metricType: MetricType, name: string, value: num * * @experimental This API is experimental and might having breaking changes in the future. */ -export function incr(name: string, value: number, data?: MetricData): void { +export function incr(name: string, value: number = 1, data?: MetricData): void { addToMetricsAggregator(COUNTER_METRIC_TYPE, name, value, data); } @@ -62,11 +62,12 @@ export function distribution(name: string, value: number, data?: MetricData): vo } /** - * Adds a value to a set metric + * Adds a value to a set metric. Value must be a string or integer. * * @experimental This API is experimental and might having breaking changes in the future. */ -export function set(name: string, value: number, data?: MetricData): void { +export function set(name: string, incomingValue: number | string, data?: MetricData): void { + const value = typeof incomingValue === 'string' ? parseInt(incomingValue) : Math.floor(incomingValue); addToMetricsAggregator(SET_METRIC_TYPE, name, value, data); } From 9f19dec87385aa765c7ebbe7ab38802b6869f40c Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 11 Dec 2023 13:01:01 -0500 Subject: [PATCH 08/20] make value private --- packages/core/src/metrics/instance.ts | 60 +++++++++++++-------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/packages/core/src/metrics/instance.ts b/packages/core/src/metrics/instance.ts index fe4410cda32b..fcd6db65845a 100644 --- a/packages/core/src/metrics/instance.ts +++ b/packages/core/src/metrics/instance.ts @@ -9,16 +9,16 @@ interface MetricInstance { * A metric instance representing a counter. */ export class CounterMetric implements MetricInstance { - public constructor(public value: number) {} + public constructor(private _value: number) {} /** JSDoc */ public add(value: number): void { - this.value += value; + this._value += value; } /** JSDoc */ public toString(): string { - return `${this.value}`; + return `${this._value}`; } } @@ -26,33 +26,33 @@ export class CounterMetric implements MetricInstance { * A metric instance representing a gauge. */ export class GaugeMetric implements MetricInstance { - public last: number; - public min: number; - public max: number; - public sum: number; - public count: number; - - public constructor(public value: number) { - this.last = value; - this.min = value; - this.max = value; - this.sum = value; - this.count = 1; + private _last: number; + private _min: number; + private _max: number; + private _sum: number; + private _count: number; + + public constructor(private _value: number) { + this._last = _value; + this._min = _value; + this._max = _value; + this._sum = _value; + this._count = 1; } /** JSDoc */ public add(value: number): void { - this.value = value; - this.last = value; - this.min = Math.min(this.min, value); - this.max = Math.max(this.max, value); - this.sum += value; - this.count += 1; + this._value = value; + this._value = value; + this._min = Math.min(this._min, value); + this._max = Math.max(this._max, value); + this._sum += value; + this._count += 1; } /** JSDoc */ public toString(): string { - return `${this.last}:${this.min}:${this.max}:${this.sum}:${this.count}`; + return `${this._last}:${this._min}:${this._max}:${this._sum}:${this._count}`; } } @@ -60,20 +60,20 @@ export class GaugeMetric implements MetricInstance { * A metric instance representing a distribution. */ export class DistributionMetric implements MetricInstance { - public value: number[]; + private _value: number[]; public constructor(first: number) { - this.value = [first]; + this._value = [first]; } /** JSDoc */ public add(value: number): void { - this.value.push(value); + this._value.push(value); } /** JSDoc */ public toString(): string { - return this.value.join(':'); + return this._value.join(':'); } } @@ -81,20 +81,20 @@ export class DistributionMetric implements MetricInstance { * A metric instance representing a set. */ export class SetMetric implements MetricInstance { - public value: Set; + private _value: Set; public constructor(public first: number) { - this.value = new Set([first]); + this._value = new Set([first]); } /** JSDoc */ public add(value: number): void { - this.value.add(value); + this._value.add(value); } /** JSDoc */ public toString(): string { - return `${Array.from(this.value).join(':')}`; + return `${Array.from(this._value).join(':')}`; } } From 979f51af8de7c2dc924bc877bc2ebe8944a20ebc Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 11 Dec 2023 13:47:05 -0500 Subject: [PATCH 09/20] switch to record --- packages/core/src/metrics/exports.ts | 2 +- packages/core/src/metrics/simpleaggregator.ts | 6 +++--- packages/core/src/metrics/types.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index 58e75e1438ef..a4ba09433389 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -8,7 +8,7 @@ import type { MetricType } from './types'; interface MetricData { unit?: MeasurementUnit; - tags?: { [key: string]: Primitive }; + tags?: Record; timestamp?: number; } diff --git a/packages/core/src/metrics/simpleaggregator.ts b/packages/core/src/metrics/simpleaggregator.ts index d372db9cca7d..e9d1e1382c4b 100644 --- a/packages/core/src/metrics/simpleaggregator.ts +++ b/packages/core/src/metrics/simpleaggregator.ts @@ -45,7 +45,7 @@ export class SimpleMetricsAggregator implements MetricsAggregator { unsanitizedName: string, value: number, unit: MeasurementUnit = 'none', - unsanitizedTags: { [key: string]: Primitive } = {}, + unsanitizedTags: Record = {}, maybeFloatTimestamp = timestampInSeconds(), ): void { const timestamp = Math.floor(maybeFloatTimestamp); @@ -112,8 +112,8 @@ export function serializeBuckets(buckets: SimpleMetricBucket): string { return out; } -function sanitizeTags(unsanitizedTags: { [key: string]: Primitive }): { [key: string]: string } { - const tags: { [key: string]: string } = {}; +function sanitizeTags(unsanitizedTags: Record): Record { + const tags: Record = {}; for (const key in unsanitizedTags) { if (Object.prototype.hasOwnProperty.call(unsanitizedTags, key)) { const sanitizedKey = key.replace(NAME_AND_TAG_KEY_NORMALIZATION_REGEX, '_'); diff --git a/packages/core/src/metrics/types.ts b/packages/core/src/metrics/types.ts index 6734d89ef477..9da2cc0db371 100644 --- a/packages/core/src/metrics/types.ts +++ b/packages/core/src/metrics/types.ts @@ -19,7 +19,7 @@ export interface MetricsAggregator { name: string, value: number, unit?: MeasurementUnit, - tags?: { [key: string]: Primitive }, + tags?: Record, timestamp?: number, ): void; From b05729edeb2b72276dc161707134ef6ef61e0248 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 11 Dec 2023 14:10:34 -0500 Subject: [PATCH 10/20] add comment to serializeBuckets --- packages/core/src/metrics/simpleaggregator.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/core/src/metrics/simpleaggregator.ts b/packages/core/src/metrics/simpleaggregator.ts index e9d1e1382c4b..fc666571f0f0 100644 --- a/packages/core/src/metrics/simpleaggregator.ts +++ b/packages/core/src/metrics/simpleaggregator.ts @@ -94,6 +94,16 @@ export class SimpleMetricsAggregator implements MetricsAggregator { /** * Serialize metrics buckets into a string based on statsd format. + * + * Example of format: + * metric.name@second:1:1.2|d|#a:value,b:anothervalue|T12345677 + * Segments: + * name: metric.name + * unit: second + * value: [1, 1.2] + * type of metric: d (distribution) + * tags: { a: value, b: anothervalue } + * timestamp: 12345677 */ export function serializeBuckets(buckets: SimpleMetricBucket): string { let out = ''; From 4e4cb4247525eea7506e506e377be9a4cb4cbf63 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 11 Dec 2023 14:54:30 -0500 Subject: [PATCH 11/20] use inheritdoc --- packages/core/src/metrics/instance.ts | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/core/src/metrics/instance.ts b/packages/core/src/metrics/instance.ts index fcd6db65845a..afc60130fd6d 100644 --- a/packages/core/src/metrics/instance.ts +++ b/packages/core/src/metrics/instance.ts @@ -1,7 +1,13 @@ import { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants'; interface MetricInstance { + /** + * Adds a value to a metric. + */ add(value: number): void; + /** + * Serializes the metric into a statsd format string. + */ toString(): string; } @@ -11,12 +17,12 @@ interface MetricInstance { export class CounterMetric implements MetricInstance { public constructor(private _value: number) {} - /** JSDoc */ + /** @inheritdoc */ public add(value: number): void { this._value += value; } - /** JSDoc */ + /** @inheritdoc */ public toString(): string { return `${this._value}`; } @@ -40,7 +46,7 @@ export class GaugeMetric implements MetricInstance { this._count = 1; } - /** JSDoc */ + /** @inheritdoc */ public add(value: number): void { this._value = value; this._value = value; @@ -50,7 +56,7 @@ export class GaugeMetric implements MetricInstance { this._count += 1; } - /** JSDoc */ + /** @inheritdoc */ public toString(): string { return `${this._last}:${this._min}:${this._max}:${this._sum}:${this._count}`; } @@ -66,12 +72,12 @@ export class DistributionMetric implements MetricInstance { this._value = [first]; } - /** JSDoc */ + /** @inheritdoc */ public add(value: number): void { this._value.push(value); } - /** JSDoc */ + /** @inheritdoc */ public toString(): string { return this._value.join(':'); } @@ -87,12 +93,12 @@ export class SetMetric implements MetricInstance { this._value = new Set([first]); } - /** JSDoc */ + /** @inheritdoc */ public add(value: number): void { this._value.add(value); } - /** JSDoc */ + /** @inheritdoc */ public toString(): string { return `${Array.from(this._value).join(':')}`; } From 2c2b715dd00329ae060394f2f59e02cee81dc428 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 12 Dec 2023 09:35:11 -0500 Subject: [PATCH 12/20] fix gauge adder --- packages/core/src/metrics/instance.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/core/src/metrics/instance.ts b/packages/core/src/metrics/instance.ts index afc60130fd6d..7299a8d76134 100644 --- a/packages/core/src/metrics/instance.ts +++ b/packages/core/src/metrics/instance.ts @@ -38,18 +38,17 @@ export class GaugeMetric implements MetricInstance { private _sum: number; private _count: number; - public constructor(private _value: number) { - this._last = _value; - this._min = _value; - this._max = _value; - this._sum = _value; + public constructor(value: number) { + this._last = value; + this._min = value; + this._max = value; + this._sum = value; this._count = 1; } /** @inheritdoc */ public add(value: number): void { - this._value = value; - this._value = value; + this._last = value; this._min = Math.min(this._min, value); this._max = Math.max(this._max, value); this._sum += value; From 5ead644cbfe129dd181a575fbb9d6bcaaaa47dbd Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 12 Dec 2023 10:12:13 -0500 Subject: [PATCH 13/20] dont overwrite user set tags --- packages/core/src/metrics/exports.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index a4ba09433389..6a0e971ef789 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -25,9 +25,7 @@ function addToMetricsAggregator(metricType: MetricType, name: string, value: num const { unit, tags, timestamp } = data; const { release, environment } = client.getOptions(); const transaction = scope.getTransaction(); - const metricTags = { - ...tags, - }; + const metricTags: Record = {}; if (release) { metricTags.release = release; } @@ -39,7 +37,7 @@ function addToMetricsAggregator(metricType: MetricType, name: string, value: num } DEBUG_BUILD && logger.log(`Adding value of ${value} to ${metricType} metric ${name}`); - client.metricsAggregator.add(metricType, name, value, unit, metricTags, timestamp); + client.metricsAggregator.add(metricType, name, value, unit, { ...metricTags, ...tags }, timestamp); } } From 261cd272bac0e96e9f1792e11dbbeb6625ef2aaf Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 12 Dec 2023 10:35:12 -0500 Subject: [PATCH 14/20] make set use string and introduce hash --- packages/core/src/metrics/exports.ts | 10 +++++++--- packages/core/src/metrics/instance.ts | 13 ++++++++----- packages/core/src/metrics/simpleaggregator.ts | 15 ++++++--------- packages/core/src/metrics/types.ts | 2 +- packages/core/src/metrics/utils.ts | 17 ++++++++++++++++- 5 files changed, 38 insertions(+), 19 deletions(-) diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index 6a0e971ef789..5fbe049446d8 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -12,7 +12,12 @@ interface MetricData { timestamp?: number; } -function addToMetricsAggregator(metricType: MetricType, name: string, value: number, data: MetricData = {}): void { +function addToMetricsAggregator( + metricType: MetricType, + name: string, + value: number | string, + data: MetricData = {}, +): void { const hub = getCurrentHub(); const client = hub.getClient() as BaseClient; const scope = hub.getScope(); @@ -64,8 +69,7 @@ export function distribution(name: string, value: number, data?: MetricData): vo * * @experimental This API is experimental and might having breaking changes in the future. */ -export function set(name: string, incomingValue: number | string, data?: MetricData): void { - const value = typeof incomingValue === 'string' ? parseInt(incomingValue) : Math.floor(incomingValue); +export function set(name: string, value: number | string, data?: MetricData): void { addToMetricsAggregator(SET_METRIC_TYPE, name, value, data); } diff --git a/packages/core/src/metrics/instance.ts b/packages/core/src/metrics/instance.ts index 7299a8d76134..fac699e04218 100644 --- a/packages/core/src/metrics/instance.ts +++ b/packages/core/src/metrics/instance.ts @@ -1,10 +1,11 @@ import { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants'; +import { simpleHash } from './utils'; interface MetricInstance { /** * Adds a value to a metric. */ - add(value: number): void; + add(value: number | string): void; /** * Serializes the metric into a statsd format string. */ @@ -86,20 +87,22 @@ export class DistributionMetric implements MetricInstance { * A metric instance representing a set. */ export class SetMetric implements MetricInstance { - private _value: Set; + private _value: Set; - public constructor(public first: number) { + public constructor(public first: number | string) { this._value = new Set([first]); } /** @inheritdoc */ - public add(value: number): void { + public add(value: number | string): void { this._value.add(value); } /** @inheritdoc */ public toString(): string { - return `${Array.from(this._value).join(':')}`; + return `${Array.from(this._value) + .map(val => (typeof val === 'string' ? simpleHash(val) : val)) + .join(':')}`; } } diff --git a/packages/core/src/metrics/simpleaggregator.ts b/packages/core/src/metrics/simpleaggregator.ts index fc666571f0f0..ab7c96994118 100644 --- a/packages/core/src/metrics/simpleaggregator.ts +++ b/packages/core/src/metrics/simpleaggregator.ts @@ -108,15 +108,12 @@ export class SimpleMetricsAggregator implements MetricsAggregator { export function serializeBuckets(buckets: SimpleMetricBucket): string { let out = ''; buckets.forEach(([metric, timestamp, metricType, name, unit, tags]) => { - out += `${name}@${unit}:${metric}|${metricType}`; - if (Object.keys(tags).length) { - out += '|#'; - out += Object.entries(tags) - .map(([key, value]) => `${key}:${String(value)}`) - .join(','); - } - // timestamp must be an integer - out += `|T${timestamp}\n`; + const maybeTags = Object.keys(tags).length + ? `|#${Object.entries(tags) + .map(([key, value]) => `${key}:${String(value)}`) + .join(',')}` + : ''; + out += `${name}@${unit}:${metric}|${metricType}${maybeTags}|T${timestamp}\n`; }); return out; diff --git a/packages/core/src/metrics/types.ts b/packages/core/src/metrics/types.ts index 9da2cc0db371..8bdc5dd56826 100644 --- a/packages/core/src/metrics/types.ts +++ b/packages/core/src/metrics/types.ts @@ -17,7 +17,7 @@ export interface MetricsAggregator { add( metricType: MetricType, name: string, - value: number, + value: number | string, unit?: MeasurementUnit, tags?: Record, timestamp?: number, diff --git a/packages/core/src/metrics/utils.ts b/packages/core/src/metrics/utils.ts index 8c4eeae8ef19..3a0b8188596b 100644 --- a/packages/core/src/metrics/utils.ts +++ b/packages/core/src/metrics/utils.ts @@ -1,4 +1,4 @@ -import type { MeasurementUnit, Primitive } from '@sentry/types'; +import type { MeasurementUnit } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; import type { MetricType } from './types'; @@ -14,3 +14,18 @@ export function getBucketKey( const stringifiedTags = Object.entries(dropUndefinedKeys(tags)).sort() as unknown as string[]; return [metricType, name, unit].concat(stringifiedTags).join(''); } + +/* eslint-disable no-bitwise */ +/** + * Simple hash function for strings. + */ +export function simpleHash(s: string): number { + let rv = 0; + for (let i = 0; i < s.length; i++) { + const c = s.charCodeAt(i); + rv = (rv << 5) - rv + c; + rv &= rv; + } + return rv >>> 0; +} +/* eslint-enable no-bitwise */ From 682fec377b74706177fae4fecb8d21eb78fad574 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 12 Dec 2023 10:39:18 -0500 Subject: [PATCH 15/20] export as metrics namespace --- packages/browser/src/exports.ts | 5 +---- packages/core/src/index.ts | 7 +------ packages/core/src/metrics/exports.ts | 7 +++++++ 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index d7edf9d7f389..9cfbd0ae7aad 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -56,10 +56,7 @@ export { withScope, FunctionToString, InboundFilters, - incr, - distribution, - set, - gauge, + metrics, Metrics, } from '@sentry/core'; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 3e75c6bad530..78b153f645e2 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -63,12 +63,7 @@ export { DEFAULT_ENVIRONMENT } from './constants'; export { ModuleMetadata } from './integrations/metadata'; export { RequestData } from './integrations/requestdata'; import * as Integrations from './integrations'; -export { - incr, - distribution, - set, - gauge, -} from './metrics/exports'; +export { metrics } from './metrics/exports'; export { Metrics } from './metrics/integration'; export { Integrations }; diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index 5fbe049446d8..3b7a51d8df84 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -81,3 +81,10 @@ export function set(name: string, value: number | string, data?: MetricData): vo export function gauge(name: string, value: number, data?: MetricData): void { addToMetricsAggregator(GAUGE_METRIC_TYPE, name, value, data); } + +export const metrics = { + incr, + distribution, + set, + gauge, +}; From 60e8989386c4b287928c0a1aa4cc7fd3f17a1141 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 12 Dec 2023 10:39:43 -0500 Subject: [PATCH 16/20] rename from incr -> increment --- packages/core/src/metrics/exports.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index 3b7a51d8df84..806779463b0c 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -51,7 +51,7 @@ function addToMetricsAggregator( * * @experimental This API is experimental and might having breaking changes in the future. */ -export function incr(name: string, value: number = 1, data?: MetricData): void { +export function increment(name: string, value: number = 1, data?: MetricData): void { addToMetricsAggregator(COUNTER_METRIC_TYPE, name, value, data); } @@ -83,7 +83,7 @@ export function gauge(name: string, value: number, data?: MetricData): void { } export const metrics = { - incr, + increment, distribution, set, gauge, From 365cd32ee3750a7cfc749b0303c44a0869967b4f Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 12 Dec 2023 10:46:00 -0500 Subject: [PATCH 17/20] remove content type --- packages/core/src/metrics/envelope.ts | 1 - packages/types/src/envelope.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/src/metrics/envelope.ts b/packages/core/src/metrics/envelope.ts index b50cfef67b33..d7a61466c8f6 100644 --- a/packages/core/src/metrics/envelope.ts +++ b/packages/core/src/metrics/envelope.ts @@ -32,7 +32,6 @@ export function createMetricEnvelope( function createMetricEnvelopeItem(metricAggregate: string): StatsdItem { const metricHeaders: StatsdItem[0] = { type: 'statsd', - content_type: 'application/octet-stream', length: metricAggregate.length, }; return [metricHeaders, metricAggregate]; diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts index f0173c64f45c..ec54c7fd868c 100644 --- a/packages/types/src/envelope.ts +++ b/packages/types/src/envelope.ts @@ -76,7 +76,7 @@ type ClientReportItemHeaders = { type: 'client_report' }; type ReplayEventItemHeaders = { type: 'replay_event' }; type ReplayRecordingItemHeaders = { type: 'replay_recording'; length: number }; type CheckInItemHeaders = { type: 'check_in' }; -type StatsdItemHeaders = { type: 'statsd'; content_type: 'application/octet-stream'; length: number }; +type StatsdItemHeaders = { type: 'statsd'; length: number }; export type EventItem = BaseEnvelopeItem; export type AttachmentItem = BaseEnvelopeItem; From ecd54ec99ffe81c8623b25b801a88d9d1cdeaf38 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 12 Dec 2023 11:48:52 -0500 Subject: [PATCH 18/20] refactor client serialization logic --- packages/core/src/baseclient.ts | 7 +-- packages/core/src/metrics/constants.ts | 8 +-- packages/core/src/metrics/envelope.ts | 14 ++--- packages/core/src/metrics/instance.ts | 12 +---- packages/core/src/metrics/simpleaggregator.ts | 53 +++--------------- packages/core/src/metrics/types.ts | 29 +--------- packages/core/src/metrics/utils.ts | 32 +++++++++-- .../test/lib/metrics/simpleaggregator.test.ts | 20 ++++--- packages/core/test/lib/metrics/utils.test.ts | 25 +++++---- packages/types/src/client.ts | 3 +- packages/types/src/index.ts | 1 + packages/types/src/metrics.ts | 54 +++++++++++++++++++ 12 files changed, 136 insertions(+), 122 deletions(-) create mode 100644 packages/types/src/metrics.ts diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 06a0722a20cd..a6e0575a2e67 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -16,6 +16,8 @@ import type { FeedbackEvent, Integration, IntegrationClass, + MetricBucketItem, + MetricsAggregator, Outcome, PropagationContext, SdkMetadata, @@ -50,7 +52,6 @@ import { getCurrentHub } from './hub'; import type { IntegrationIndex } from './integration'; import { setupIntegration, setupIntegrations } from './integration'; import { createMetricEnvelope } from './metrics/envelope'; -import type { MetricsAggregator } from './metrics/types'; import type { Scope } from './scope'; import { updateSession } from './session'; import { getDynamicSamplingContextFromClient } from './tracing/dynamicSamplingContext'; @@ -401,9 +402,9 @@ export abstract class BaseClient implements Client { /** * @inheritDoc */ - public captureSerializedMetrics(serializedMetrics: string): void { + public captureAggregateMetrics(metricBucketItems: Array): void { const metricsEnvelope = createMetricEnvelope( - serializedMetrics, + metricBucketItems, this._dsn, this._options._metadata, this._options.tunnel, diff --git a/packages/core/src/metrics/constants.ts b/packages/core/src/metrics/constants.ts index 3f4a53fcacb7..65a2c19d93d0 100644 --- a/packages/core/src/metrics/constants.ts +++ b/packages/core/src/metrics/constants.ts @@ -1,7 +1,7 @@ -export const COUNTER_METRIC_TYPE = 'c'; -export const GAUGE_METRIC_TYPE = 'g'; -export const SET_METRIC_TYPE = 's'; -export const DISTRIBUTION_METRIC_TYPE = 'd'; +export const COUNTER_METRIC_TYPE = 'c' as const; +export const GAUGE_METRIC_TYPE = 'g' as const; +export const SET_METRIC_TYPE = 's' as const; +export const DISTRIBUTION_METRIC_TYPE = 'd' as const; /** * Normalization regex for metric names and metric tag names. diff --git a/packages/core/src/metrics/envelope.ts b/packages/core/src/metrics/envelope.ts index d7a61466c8f6..af9790fd6534 100644 --- a/packages/core/src/metrics/envelope.ts +++ b/packages/core/src/metrics/envelope.ts @@ -1,11 +1,12 @@ -import type { DsnComponents, SdkMetadata, StatsdEnvelope, StatsdItem } from '@sentry/types'; +import type { DsnComponents, MetricBucketItem, SdkMetadata, StatsdEnvelope, StatsdItem } from '@sentry/types'; import { createEnvelope, dsnToString } from '@sentry/utils'; +import { serializeMetricBuckets } from './utils'; /** * Create envelope from a metric aggregate. */ export function createMetricEnvelope( - metricAggregate: string, + metricBucketItems: Array, dsn?: DsnComponents, metadata?: SdkMetadata, tunnel?: string, @@ -25,14 +26,15 @@ export function createMetricEnvelope( headers.dsn = dsnToString(dsn); } - const item = createMetricEnvelopeItem(metricAggregate); + const item = createMetricEnvelopeItem(metricBucketItems); return createEnvelope(headers, [item]); } -function createMetricEnvelopeItem(metricAggregate: string): StatsdItem { +function createMetricEnvelopeItem(metricBucketItems: Array): StatsdItem { + const payload = serializeMetricBuckets(metricBucketItems); const metricHeaders: StatsdItem[0] = { type: 'statsd', - length: metricAggregate.length, + length: payload.length, }; - return [metricHeaders, metricAggregate]; + return [metricHeaders, payload]; } diff --git a/packages/core/src/metrics/instance.ts b/packages/core/src/metrics/instance.ts index fac699e04218..532425f2f489 100644 --- a/packages/core/src/metrics/instance.ts +++ b/packages/core/src/metrics/instance.ts @@ -1,17 +1,7 @@ +import type { MetricInstance } from '@sentry/types'; import { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants'; import { simpleHash } from './utils'; -interface MetricInstance { - /** - * Adds a value to a metric. - */ - add(value: number | string): void; - /** - * Serializes the metric into a statsd format string. - */ - toString(): string; -} - /** * A metric instance representing a counter. */ diff --git a/packages/core/src/metrics/simpleaggregator.ts b/packages/core/src/metrics/simpleaggregator.ts index ab7c96994118..d481485d25c2 100644 --- a/packages/core/src/metrics/simpleaggregator.ts +++ b/packages/core/src/metrics/simpleaggregator.ts @@ -1,27 +1,14 @@ -import type { Client, ClientOptions, MeasurementUnit, Primitive } from '@sentry/types'; +import type { Client, ClientOptions, MeasurementUnit, MetricsAggregator, Primitive } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; import { DEFAULT_FLUSH_INTERVAL, NAME_AND_TAG_KEY_NORMALIZATION_REGEX, TAG_VALUE_NORMALIZATION_REGEX, } from './constants'; -import type { Metric } from './instance'; import { METRIC_MAP } from './instance'; -import type { MetricType, MetricsAggregator } from './types'; +import type { MetricType, SimpleMetricBucket } from './types'; import { getBucketKey } from './utils'; -type SimpleMetricBucket = Map< - string, - [ - metric: Metric, - timestamp: number, - metricType: MetricType, - name: string, - unit: MeasurementUnit, - tags: { [key: string]: string }, - ] ->; - /** * A simple metrics aggregator that aggregates metrics in memory and flushes them periodically. * Default flush interval is 5 seconds. @@ -43,7 +30,7 @@ export class SimpleMetricsAggregator implements MetricsAggregator { public add( metricType: MetricType, unsanitizedName: string, - value: number, + value: number | string, unit: MeasurementUnit = 'none', unsanitizedTags: Record = {}, maybeFloatTimestamp = timestampInSeconds(), @@ -62,6 +49,7 @@ export class SimpleMetricsAggregator implements MetricsAggregator { bucketItem[1] = timestamp; } } else { + // @ts-expect-error we don't need to narrow down the type of value here, saves bundle size. const newMetric = new METRIC_MAP[metricType](value); this._buckets.set(bucketKey, [newMetric, timestamp, metricType, name, unit, tags]); } @@ -75,9 +63,9 @@ export class SimpleMetricsAggregator implements MetricsAggregator { if (this._buckets.size === 0) { return; } - - if (this._client.captureSerializedMetrics) { - this._client.captureSerializedMetrics(serializeBuckets(this._buckets)); + if (this._client.captureAggregateMetrics) { + const metricBuckets = Array.from(this._buckets).map(([, bucketItem]) => bucketItem); + this._client.captureAggregateMetrics(metricBuckets); } this._buckets.clear(); } @@ -92,33 +80,6 @@ export class SimpleMetricsAggregator implements MetricsAggregator { } } -/** - * Serialize metrics buckets into a string based on statsd format. - * - * Example of format: - * metric.name@second:1:1.2|d|#a:value,b:anothervalue|T12345677 - * Segments: - * name: metric.name - * unit: second - * value: [1, 1.2] - * type of metric: d (distribution) - * tags: { a: value, b: anothervalue } - * timestamp: 12345677 - */ -export function serializeBuckets(buckets: SimpleMetricBucket): string { - let out = ''; - buckets.forEach(([metric, timestamp, metricType, name, unit, tags]) => { - const maybeTags = Object.keys(tags).length - ? `|#${Object.entries(tags) - .map(([key, value]) => `${key}:${String(value)}`) - .join(',')}` - : ''; - out += `${name}@${unit}:${metric}|${metricType}${maybeTags}|T${timestamp}\n`; - }); - - return out; -} - function sanitizeTags(unsanitizedTags: Record): Record { const tags: Record = {}; for (const key in unsanitizedTags) { diff --git a/packages/core/src/metrics/types.ts b/packages/core/src/metrics/types.ts index 8bdc5dd56826..de6032f811b8 100644 --- a/packages/core/src/metrics/types.ts +++ b/packages/core/src/metrics/types.ts @@ -1,4 +1,4 @@ -import type { MeasurementUnit, Primitive } from '@sentry/types'; +import type { MetricBucketItem } from '@sentry/types'; import type { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants'; export type MetricType = @@ -7,29 +7,4 @@ export type MetricType = | typeof SET_METRIC_TYPE | typeof DISTRIBUTION_METRIC_TYPE; -/** - * A metrics aggregator that aggregates metrics in memory and flushes them periodically. - */ -export interface MetricsAggregator { - /** - * Add a metric to the aggregator. - */ - add( - metricType: MetricType, - name: string, - value: number | string, - unit?: MeasurementUnit, - tags?: Record, - timestamp?: number, - ): void; - - /** - * Flushes the current metrics to the transport via the transport. - */ - flush(): void; - - /** - * Shuts down metrics aggregator and clears all metrics. - */ - close(): void; -} +export type SimpleMetricBucket = Map; diff --git a/packages/core/src/metrics/utils.ts b/packages/core/src/metrics/utils.ts index 3a0b8188596b..59039c42f96a 100644 --- a/packages/core/src/metrics/utils.ts +++ b/packages/core/src/metrics/utils.ts @@ -1,6 +1,6 @@ -import type { MeasurementUnit } from '@sentry/types'; +import type { MeasurementUnit, MetricBucketItem } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; -import type { MetricType } from './types'; +import type { MetricType, SimpleMetricBucket } from './types'; /** * Generate bucket key from metric properties. @@ -9,7 +9,7 @@ export function getBucketKey( metricType: MetricType, name: string, unit: MeasurementUnit, - tags: { [key: string]: string }, + tags: Record, ): string { const stringifiedTags = Object.entries(dropUndefinedKeys(tags)).sort() as unknown as string[]; return [metricType, name, unit].concat(stringifiedTags).join(''); @@ -29,3 +29,29 @@ export function simpleHash(s: string): number { return rv >>> 0; } /* eslint-enable no-bitwise */ + +/** + * Serialize metrics buckets into a string based on statsd format. + * + * Example of format: + * metric.name@second:1:1.2|d|#a:value,b:anothervalue|T12345677 + * Segments: + * name: metric.name + * unit: second + * value: [1, 1.2] + * type of metric: d (distribution) + * tags: { a: value, b: anothervalue } + * timestamp: 12345677 + */ +export function serializeMetricBuckets(metricBucketItems: Array): string { + let out = ''; + for (const [metric, timestamp, metricType, name, unit, tags] of metricBucketItems) { + const maybeTags = Object.keys(tags).length + ? `|#${Object.entries(tags) + .map(([key, value]) => `${key}:${String(value)}`) + .join(',')}` + : ''; + out += `${name}@${unit}:${metric}|${metricType}${maybeTags}|T${timestamp}\n`; + } + return out; +} diff --git a/packages/core/test/lib/metrics/simpleaggregator.test.ts b/packages/core/test/lib/metrics/simpleaggregator.test.ts index 7998f547d09a..cafc78d1e018 100644 --- a/packages/core/test/lib/metrics/simpleaggregator.test.ts +++ b/packages/core/test/lib/metrics/simpleaggregator.test.ts @@ -1,5 +1,6 @@ import { CounterMetric } from '../../../src/metrics/instance'; -import { SimpleMetricsAggregator, serializeBuckets } from '../../../src/metrics/simpleaggregator'; +import { SimpleMetricsAggregator } from '../../../src/metrics/simpleaggregator'; +import { serializeMetricBuckets } from '../../../src/metrics/utils'; import { TestClient, getDefaultTestClientOptions } from '../../mocks/client'; describe('SimpleMetricsAggregator', () => { @@ -25,7 +26,7 @@ describe('SimpleMetricsAggregator', () => { const firstValue = aggregator['_buckets'].values().next().value; expect(firstValue).toEqual([expect.any(CounterMetric), expect.any(Number), 'c', 'requests', 'none', {}]); - expect(firstValue[0].value).toEqual(2); + expect(firstValue[0]._value).toEqual(2); }); it('differentiates based on tag value', () => { @@ -45,13 +46,16 @@ describe('SimpleMetricsAggregator', () => { aggregator.add('g', 'cpu', 52); aggregator.add('d', 'lcp', 1, 'second', { a: 'value', b: 'anothervalue' }); aggregator.add('d', 'lcp', 1.2, 'second', { a: 'value', b: 'anothervalue' }); - aggregator.add('s', 'important_org_ids', 1, 'none', { numericKey: 2 }); - aggregator.add('s', 'important_org_ids', 2, 'none', { numericKey: 2 }); + aggregator.add('s', 'important_people', 'a', 'none', { numericKey: 2 }); + aggregator.add('s', 'important_people', 'b', 'none', { numericKey: 2 }); - expect(serializeBuckets(aggregator['_buckets'])).toContain('requests@none:8|c|T'); - expect(serializeBuckets(aggregator['_buckets'])).toContain('cpu@none:52:50:55:157:3|g|T'); - expect(serializeBuckets(aggregator['_buckets'])).toContain('lcp@second:1:1.2|d|#a:value,b:anothervalue|T'); - expect(serializeBuckets(aggregator['_buckets'])).toContain('important_org_ids@none:1:2|s|#numericKey:2|T'); + const metricBuckets = Array.from(aggregator['_buckets']).map(([, bucketItem]) => bucketItem); + const serializedBuckets = serializeMetricBuckets(metricBuckets); + + expect(serializedBuckets).toContain('requests@none:8|c|T'); + expect(serializedBuckets).toContain('cpu@none:52:50:55:157:3|g|T'); + expect(serializedBuckets).toContain('lcp@second:1:1.2|d|#a:value,b:anothervalue|T'); + expect(serializedBuckets).toContain('important_people@none:97:98|s|#numericKey:2|T'); }); }); }); diff --git a/packages/core/test/lib/metrics/utils.test.ts b/packages/core/test/lib/metrics/utils.test.ts index 63a8fe697b1d..61db9c0f30d1 100644 --- a/packages/core/test/lib/metrics/utils.test.ts +++ b/packages/core/test/lib/metrics/utils.test.ts @@ -1,20 +1,19 @@ +import { + COUNTER_METRIC_TYPE, + DISTRIBUTION_METRIC_TYPE, + GAUGE_METRIC_TYPE, + SET_METRIC_TYPE, +} from '../../../src/metrics/constants'; import { getBucketKey } from '../../../src/metrics/utils'; describe('getBucketKey', () => { it.each([ - ['c' as const, 'requests', 'none', {}, 'crequestsnone'], - ['g' as const, 'cpu', 'none', {}, 'gcpunone'], - ['d' as const, 'lcp', 'second', { a: 'value', b: 'anothervalue' }, 'dlcpseconda,valueb,anothervalue'], - ['d' as const, 'lcp', 'second', { a: 'value', b: 'anothervalue' }, 'dlcpseconda,valueb,anothervalue'], - ['d' as const, 'lcp', 'second', { numericKey: 2 }, 'dlcpsecondnumericKey,2'], - ['d' as const, 'lcp', 'second', { undefinedKey: undefined, numericKey: 2 }, 'dlcpsecondnumericKey,2'], - [ - 's' as const, - 'important_org_ids', - 'none', - { undefinedKey: undefined, numericKey: 2 }, - 'simportant_org_idsnonenumericKey,2', - ], + [COUNTER_METRIC_TYPE, 'requests', 'none', {}, 'crequestsnone'], + [GAUGE_METRIC_TYPE, 'cpu', 'none', {}, 'gcpunone'], + [DISTRIBUTION_METRIC_TYPE, 'lcp', 'second', { a: 'value', b: 'anothervalue' }, 'dlcpseconda,valueb,anothervalue'], + [DISTRIBUTION_METRIC_TYPE, 'lcp', 'second', { a: 'value', b: 'anothervalue' }, 'dlcpseconda,valueb,anothervalue'], + [DISTRIBUTION_METRIC_TYPE, 'lcp', 'second', { numericKey: '2' }, 'dlcpsecondnumericKey,2'], + [SET_METRIC_TYPE, 'important_org_ids', 'none', { numericKey: '2' }, 'simportant_org_idsnonenumericKey,2'], ])('should return', (metricType, name, unit, tags, expected) => { expect(getBucketKey(metricType, name, unit, tags)).toEqual(expected); }); diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index b556f15ec2c2..bfb657d135fa 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -8,6 +8,7 @@ import type { Event, EventHint } from './event'; import type { EventProcessor } from './eventprocessor'; import type { FeedbackEvent } from './feedback'; import type { Integration, IntegrationClass } from './integration'; +import type { MetricBucketItem } from './metrics'; import type { ClientOptions } from './options'; import type { Scope } from './scope'; import type { SdkMetadata } from './sdkmetadata'; @@ -184,7 +185,7 @@ export interface Client { * * @experimental This API is experimental and might experience breaking changes */ - captureSerializedMetrics?(serializedMetrics: string): void; + captureAggregateMetrics?(metricBucketItems: Array): void; // HOOKS // TODO(v8): Make the hooks non-optional. diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index bc1cdd10de60..72d5a50020c9 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -138,3 +138,4 @@ export type { export type { BrowserClientReplayOptions, BrowserClientProfilingOptions } from './browseroptions'; export type { CheckIn, MonitorConfig, FinishedCheckIn, InProgressCheckIn, SerializedCheckIn } from './checkin'; +export type { MetricsAggregator, MetricBucketItem, MetricInstance } from './metrics'; diff --git a/packages/types/src/metrics.ts b/packages/types/src/metrics.ts new file mode 100644 index 000000000000..18943ee3997e --- /dev/null +++ b/packages/types/src/metrics.ts @@ -0,0 +1,54 @@ +import type { MeasurementUnit } from './measurement'; +import type { Primitive } from './misc'; + +export interface MetricInstance { + /** + * Adds a value to a metric. + */ + add(value: number | string): void; + /** + * Serializes the metric into a statsd format string. + */ + toString(): string; +} + +export type MetricBucketItem = [ + metric: MetricInstance, + timestamp: number, + metricType: 'c' | 'g' | 's' | 'd', + name: string, + unit: MeasurementUnit, + tags: { [key: string]: string }, +]; + +/** + * A metrics aggregator that aggregates metrics in memory and flushes them periodically. + */ +export interface MetricsAggregator { + /** + * Add a metric to the aggregator. + */ + add( + metricType: 'c' | 'g' | 's' | 'd', + name: string, + value: number | string, + unit?: MeasurementUnit, + tags?: Record, + timestamp?: number, + ): void; + + /** + * Flushes the current metrics to the transport via the transport. + */ + flush(): void; + + /** + * Shuts down metrics aggregator and clears all metrics. + */ + close(): void; + + /** + * Returns a string representation of the aggregator. + */ + toString(): string; +} From e693382a79f74b70c6812f7160af8f79f773eb42 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 14 Dec 2023 08:22:25 -0500 Subject: [PATCH 19/20] quick pr review --- packages/core/src/metrics/constants.ts | 2 +- packages/core/src/metrics/envelope.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/metrics/constants.ts b/packages/core/src/metrics/constants.ts index 65a2c19d93d0..f29ac323c2ee 100644 --- a/packages/core/src/metrics/constants.ts +++ b/packages/core/src/metrics/constants.ts @@ -14,7 +14,7 @@ export const DISTRIBUTION_METRIC_TYPE = 'd' as const; export const NAME_AND_TAG_KEY_NORMALIZATION_REGEX = /[^a-zA-Z0-9_/.-]+/g; /** - * Normalization regex for metric tag balues. + * Normalization regex for metric tag values. * * This enforces that values only contain words, digits, or the following * special characters: _:/@.{}[\]$- diff --git a/packages/core/src/metrics/envelope.ts b/packages/core/src/metrics/envelope.ts index af9790fd6534..c7c65674b736 100644 --- a/packages/core/src/metrics/envelope.ts +++ b/packages/core/src/metrics/envelope.ts @@ -22,7 +22,7 @@ export function createMetricEnvelope( }; } - if (!!tunnel && !!dsn) { + if (!!tunnel && dsn) { headers.dsn = dsnToString(dsn); } From f6c8bbf55fdcf07d9ffd1d8c4e34fd09ec838b91 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 14 Dec 2023 10:05:34 -0500 Subject: [PATCH 20/20] address Yagiz PR review and metrics aggregator rename --- packages/browser/src/exports.ts | 1 - packages/core/src/index.ts | 1 - packages/core/src/metrics/exports.ts | 4 +++- packages/core/src/metrics/instance.ts | 10 +++++++--- packages/core/src/metrics/integration.ts | 6 +++--- packages/core/src/metrics/simpleaggregator.ts | 1 - packages/core/src/metrics/utils.ts | 4 ++-- packages/core/test/lib/metrics/utils.test.ts | 5 +++-- 8 files changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 9cfbd0ae7aad..499bb12c5fc9 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -57,7 +57,6 @@ export { FunctionToString, InboundFilters, metrics, - Metrics, } from '@sentry/core'; export { WINDOW } from './helpers'; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 78b153f645e2..48ba2b4a362a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -64,6 +64,5 @@ export { ModuleMetadata } from './integrations/metadata'; export { RequestData } from './integrations/requestdata'; import * as Integrations from './integrations'; export { metrics } from './metrics/exports'; -export { Metrics } from './metrics/integration'; export { Integrations }; diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index 806779463b0c..c27e76cf79b1 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -4,6 +4,7 @@ import type { BaseClient } from '../baseclient'; import { DEBUG_BUILD } from '../debug-build'; import { getCurrentHub } from '../hub'; import { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants'; +import { MetricsAggregator } from './integration'; import type { MetricType } from './types'; interface MetricData { @@ -24,7 +25,7 @@ function addToMetricsAggregator( if (client) { if (!client.metricsAggregator) { DEBUG_BUILD && - logger.warn('No metrics aggregator enabled. Please add the Metrics integration to use metrics APIs'); + logger.warn('No metrics aggregator enabled. Please add the MetricsAggregator integration to use metrics APIs'); return; } const { unit, tags, timestamp } = data; @@ -87,4 +88,5 @@ export const metrics = { distribution, set, gauge, + MetricsAggregator, }; diff --git a/packages/core/src/metrics/instance.ts b/packages/core/src/metrics/instance.ts index 532425f2f489..f071006c96ca 100644 --- a/packages/core/src/metrics/instance.ts +++ b/packages/core/src/metrics/instance.ts @@ -40,10 +40,14 @@ export class GaugeMetric implements MetricInstance { /** @inheritdoc */ public add(value: number): void { this._last = value; - this._min = Math.min(this._min, value); - this._max = Math.max(this._max, value); + if (value < this._min) { + this._min = value; + } + if (value > this._max) { + this._max = value; + } this._sum += value; - this._count += 1; + this._count++; } /** @inheritdoc */ diff --git a/packages/core/src/metrics/integration.ts b/packages/core/src/metrics/integration.ts index 250ac28862d7..9ce7f836ca8c 100644 --- a/packages/core/src/metrics/integration.ts +++ b/packages/core/src/metrics/integration.ts @@ -7,11 +7,11 @@ import { SimpleMetricsAggregator } from './simpleaggregator'; * * @experimental This API is experimental and might having breaking changes in the future. */ -export class Metrics implements Integration { +export class MetricsAggregator implements Integration { /** * @inheritDoc */ - public static id: string = 'Metrics'; + public static id: string = 'MetricsAggregator'; /** * @inheritDoc @@ -19,7 +19,7 @@ export class Metrics implements Integration { public name: string; public constructor() { - this.name = Metrics.id; + this.name = MetricsAggregator.id; } /** diff --git a/packages/core/src/metrics/simpleaggregator.ts b/packages/core/src/metrics/simpleaggregator.ts index d481485d25c2..a628a3b5a406 100644 --- a/packages/core/src/metrics/simpleaggregator.ts +++ b/packages/core/src/metrics/simpleaggregator.ts @@ -76,7 +76,6 @@ export class SimpleMetricsAggregator implements MetricsAggregator { public close(): void { clearInterval(this._interval); this.flush(); - this._buckets.clear(); } } diff --git a/packages/core/src/metrics/utils.ts b/packages/core/src/metrics/utils.ts index 59039c42f96a..27c49d144523 100644 --- a/packages/core/src/metrics/utils.ts +++ b/packages/core/src/metrics/utils.ts @@ -11,8 +11,8 @@ export function getBucketKey( unit: MeasurementUnit, tags: Record, ): string { - const stringifiedTags = Object.entries(dropUndefinedKeys(tags)).sort() as unknown as string[]; - return [metricType, name, unit].concat(stringifiedTags).join(''); + const stringifiedTags = Object.entries(dropUndefinedKeys(tags)).sort((a, b) => a[0].localeCompare(b[0])); + return `${metricType}${name}${unit}${stringifiedTags}`; } /* eslint-disable no-bitwise */ diff --git a/packages/core/test/lib/metrics/utils.test.ts b/packages/core/test/lib/metrics/utils.test.ts index 61db9c0f30d1..fe96404b72ea 100644 --- a/packages/core/test/lib/metrics/utils.test.ts +++ b/packages/core/test/lib/metrics/utils.test.ts @@ -10,8 +10,9 @@ describe('getBucketKey', () => { it.each([ [COUNTER_METRIC_TYPE, 'requests', 'none', {}, 'crequestsnone'], [GAUGE_METRIC_TYPE, 'cpu', 'none', {}, 'gcpunone'], - [DISTRIBUTION_METRIC_TYPE, 'lcp', 'second', { a: 'value', b: 'anothervalue' }, 'dlcpseconda,valueb,anothervalue'], - [DISTRIBUTION_METRIC_TYPE, 'lcp', 'second', { a: 'value', b: 'anothervalue' }, 'dlcpseconda,valueb,anothervalue'], + [DISTRIBUTION_METRIC_TYPE, 'lcp', 'second', { a: 'value', b: 'anothervalue' }, 'dlcpseconda,value,b,anothervalue'], + [DISTRIBUTION_METRIC_TYPE, 'lcp', 'second', { b: 'anothervalue', a: 'value' }, 'dlcpseconda,value,b,anothervalue'], + [DISTRIBUTION_METRIC_TYPE, 'lcp', 'second', { a: '1', b: '2', c: '3' }, 'dlcpseconda,1,b,2,c,3'], [DISTRIBUTION_METRIC_TYPE, 'lcp', 'second', { numericKey: '2' }, 'dlcpsecondnumericKey,2'], [SET_METRIC_TYPE, 'important_org_ids', 'none', { numericKey: '2' }, 'simportant_org_idsnonenumericKey,2'], ])('should return', (metricType, name, unit, tags, expected) => {