From a351f9137b41dd62e6adf29eac51469e4561e989 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 30 Jan 2024 19:40:36 -0400 Subject: [PATCH 1/8] feat(core): Add metric summaries to spans --- .../tracing/metric-summaries/scenario.js | 35 ++++++++++++ .../suites/tracing/metric-summaries/test.ts | 27 +++++++++ packages/core/src/metric-summary.ts | 56 +++++++++++++++++++ packages/core/src/metrics/exports.ts | 10 +++- packages/core/src/tracing/span.ts | 13 +++++ packages/types/src/index.ts | 3 +- packages/types/src/metrics.ts | 15 +++++ packages/types/src/span.ts | 15 +++++ 8 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts create mode 100644 packages/core/src/metric-summary.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js new file mode 100644 index 000000000000..9d84234ea44e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js @@ -0,0 +1,35 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/node'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, + _experiments: { + metricsAggregator: true, + }, +}); + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +Sentry.startSpan( + { + name: 'Test Transaction', + op: 'transaction', + }, + () => { + Sentry.startSpan( + { + name: 'Some other span', + op: 'transaction', + }, + () => { + Sentry.metrics.increment('root-counter'); + Sentry.metrics.increment('root-counter'); + Sentry.metrics.increment('root-counter', 2); + }, + ); + }, +); diff --git a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts new file mode 100644 index 000000000000..feb19a331595 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts @@ -0,0 +1,27 @@ +import { createRunner } from '../../../utils/runner'; + +const EXPECTED_TRANSACTION = { + transaction: 'Test Transaction', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: 'Some other span', + op: 'transaction', + _metrics_summary: { + 'c:root-counter@undefined': { + min: 1, + max: 2, + count: 3, + sum: 4, + tags: { + release: '1.0', + transaction: 'Test Transaction', + }, + }, + }, + }), + ]), +}; + +test('Should add metric summaries to spans', done => { + createRunner(__dirname, 'scenario.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); +}); diff --git a/packages/core/src/metric-summary.ts b/packages/core/src/metric-summary.ts new file mode 100644 index 000000000000..18aedb9e5fe4 --- /dev/null +++ b/packages/core/src/metric-summary.ts @@ -0,0 +1,56 @@ +import type { MeasurementUnit, Primitive, SpanMetricSummaryAggregator } from '@sentry/types'; +import type { MetricSpanSummary } from '@sentry/types'; +import { dropUndefinedKeys } from '@sentry/utils'; + +/** */ +export class MetricSummaryAggregator implements SpanMetricSummaryAggregator { + private readonly _measurements: Map; + + public constructor() { + this._measurements = new Map(); + } + + /** @inheritdoc */ + public add( + metricType: 'c' | 'g' | 's' | 'd', + name: string, + value: number, + unit?: MeasurementUnit | undefined, + tags?: Record | undefined, + ): void { + const exportKey = `${metricType}:${name}@${unit}`; + const bucketKey = `${exportKey}\n${JSON.stringify(tags)}`; + + const summary = this._measurements.get(bucketKey); + + if (summary) { + this._measurements.set(bucketKey, { + min: Math.min(summary.min, value), + max: Math.max(summary.max, value), + count: (summary.count += 1), + sum: (summary.sum += value), + tags: summary.tags, + }); + } else { + this._measurements.set(bucketKey, { + min: value, + max: value, + count: 1, + sum: value, + tags: tags, + }); + } + } + + /** @inheritdoc */ + public getSummaryJson(): Record { + const output: Record = {}; + + for (const [bucketKey, summary] of this._measurements) { + const [exportKey] = bucketKey.split('\n'); + output[exportKey] = dropUndefinedKeys(summary); + } + + return output; + } +} diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index 20d63a2ca119..02b9314884ef 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -43,8 +43,16 @@ function addToMetricsAggregator( metricTags.transaction = spanToJSON(transaction).description || ''; } + const combinedTags = { ...metricTags, ...tags }; + + // eslint-disable-next-line deprecation/deprecation + const span = scope.getSpan(); + if (span && typeof value === 'number') { + span.getMetricSummaryAggregator().add(metricType, name, value, unit, combinedTags); + } + DEBUG_BUILD && logger.log(`Adding value of ${value} to ${metricType} metric ${name}`); - client.metricsAggregator.add(metricType, name, value, unit, { ...metricTags, ...tags }, timestamp); + client.metricsAggregator.add(metricType, name, value, unit, combinedTags, timestamp); } } diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index 165677455d7f..16b9442acc63 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -8,6 +8,7 @@ import type { SpanContext, SpanContextData, SpanJSON, + SpanMetricSummaryAggregator, SpanOrigin, SpanTimeInput, TraceContext, @@ -17,6 +18,7 @@ import { dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/ut import { DEBUG_BUILD } from '../debug-build'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes'; +import { MetricSummaryAggregator } from '../metric-summary'; import { getRootSpan } from '../utils/getRootSpan'; import { TRACE_FLAG_NONE, @@ -114,6 +116,7 @@ export class Span implements SpanInterface { protected _endTime?: number; /** Internal keeper of the status */ protected _status?: SpanStatusType | string; + protected _metricSummary: MetricSummaryAggregator | undefined; private _logMessage?: string; @@ -602,6 +605,15 @@ export class Span implements SpanInterface { return spanToTraceContext(this); } + /** @inheritdoc */ + public getMetricSummaryAggregator(): SpanMetricSummaryAggregator { + if (!this._metricSummary) { + this._metricSummary = new MetricSummaryAggregator(); + } + + return this._metricSummary; + } + /** * Get JSON representation of this span. * @@ -624,6 +636,7 @@ export class Span implements SpanInterface { timestamp: this._endTime, trace_id: this._traceId, origin: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] as SpanOrigin | undefined, + _metrics_summary: this._metricSummary ? this._metricSummary.getSummaryJson() : undefined, }); } diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 5970383febc3..d43379062191 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -99,6 +99,7 @@ export type { SpanJSON, SpanContextData, TraceFlag, + MetricSummary as MetricSpanSummary, } from './span'; export type { StackFrame } from './stackframe'; export type { Stacktrace, StackParser, StackLineParser, StackLineParserFn } from './stacktrace'; @@ -150,5 +151,5 @@ export type { export type { BrowserClientReplayOptions, BrowserClientProfilingOptions } from './browseroptions'; export type { CheckIn, MonitorConfig, FinishedCheckIn, InProgressCheckIn, SerializedCheckIn } from './checkin'; -export type { MetricsAggregator, MetricBucketItem, MetricInstance } from './metrics'; +export type { MetricsAggregator, MetricBucketItem, MetricInstance, SpanMetricSummaryAggregator } from './metrics'; export type { ParameterizedString } from './parameterize'; diff --git a/packages/types/src/metrics.ts b/packages/types/src/metrics.ts index 0f8dc4f53435..2eacfe58b33a 100644 --- a/packages/types/src/metrics.ts +++ b/packages/types/src/metrics.ts @@ -1,5 +1,6 @@ import type { MeasurementUnit } from './measurement'; import type { Primitive } from './misc'; +import type { MetricSummary } from './span'; /** * An abstract definition of the minimum required API @@ -62,3 +63,17 @@ export interface MetricsAggregator { */ toString(): string; } + +export interface SpanMetricSummaryAggregator { + /** Adds a metric to the summary */ + add( + metricType: 'c' | 'g' | 's' | 'd', + name: string, + value: number | string, + unit?: MeasurementUnit, + tags?: Record, + ): void; + + /** Gets the JSON representation of the metric summary */ + getSummaryJson(): Record; +} diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index 73c2fbdaaaa8..cc27bcc8e5f2 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -1,5 +1,6 @@ import type { TraceContext } from './context'; import type { Instrumenter } from './instrumenter'; +import type { SpanMetricSummaryAggregator } from './metrics'; import type { Primitive } from './misc'; import type { HrTime } from './opentelemetry'; import type { Transaction } from './transaction'; @@ -31,6 +32,14 @@ export type SpanAttributes = Partial<{ }> & Record; +export type MetricSummary = { + min: number; + max: number; + count: number; + sum: number; + tags?: Record | undefined; +}; + /** This type is aligned with the OpenTelemetry TimeInput type. */ export type SpanTimeInput = HrTime | number | Date; @@ -47,6 +56,7 @@ export interface SpanJSON { timestamp?: number; trace_id: string; origin?: SpanOrigin; + _metrics_summary?: Record; } // These are aligned with OpenTelemetry trace flags @@ -387,6 +397,11 @@ export interface Span extends Omit { */ getTraceContext(): TraceContext; + /** + * Gets the metric summary aggregator for this span + */ + getMetricSummaryAggregator(): SpanMetricSummaryAggregator; + /** * Convert the object to JSON. * @deprecated Use `spanToJSON(span)` instead. From 1d5fbd8b2d02509130763e0946b9841e6cc53d4f Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 30 Jan 2024 20:07:17 -0400 Subject: [PATCH 2/8] Fix lint --- packages/core/src/tracing/span.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index 16b9442acc63..bad8db0d8cb7 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -17,8 +17,8 @@ import type { import { dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; -import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes'; import { MetricSummaryAggregator } from '../metric-summary'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes'; import { getRootSpan } from '../utils/getRootSpan'; import { TRACE_FLAG_NONE, From 25dcfe673f91d5c4e26d2939caec5ff8b9122da6 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 31 Jan 2024 20:01:49 -0400 Subject: [PATCH 3/8] Don't add to bundle size and fix all issues --- .../tracing/metric-summaries/scenario.js | 3 + .../suites/tracing/metric-summaries/test.ts | 14 ++- packages/core/src/metric-summary.ts | 56 ------------ packages/core/src/metrics/aggregator.ts | 4 + .../core/src/metrics/browser-aggregator.ts | 4 + packages/core/src/metrics/exports.ts | 10 +-- packages/core/src/metrics/metric-summary.ts | 90 +++++++++++++++++++ packages/core/src/tracing/span.ts | 16 ++-- packages/core/src/tracing/transaction.ts | 4 + packages/types/src/event.ts | 3 +- packages/types/src/index.ts | 7 +- packages/types/src/metrics.ts | 11 +-- packages/types/src/span.ts | 9 +- 13 files changed, 145 insertions(+), 86 deletions(-) delete mode 100644 packages/core/src/metric-summary.ts create mode 100644 packages/core/src/metrics/metric-summary.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js index 9d84234ea44e..657e4f789cb8 100644 --- a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js @@ -20,6 +20,9 @@ Sentry.startSpan( op: 'transaction', }, () => { + Sentry.metrics.increment('root-counter'); + Sentry.metrics.increment('root-counter'); + Sentry.startSpan( { name: 'Some other span', diff --git a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts index feb19a331595..c7eb7c753766 100644 --- a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts @@ -2,12 +2,24 @@ import { createRunner } from '../../../utils/runner'; const EXPECTED_TRANSACTION = { transaction: 'Test Transaction', + _metrics_summary: { + 'c:root-counter@none': { + min: 1, + max: 1, + count: 2, + sum: 2, + tags: { + release: '1.0', + transaction: 'Test Transaction', + }, + }, + }, spans: expect.arrayContaining([ expect.objectContaining({ description: 'Some other span', op: 'transaction', _metrics_summary: { - 'c:root-counter@undefined': { + 'c:root-counter@none': { min: 1, max: 2, count: 3, diff --git a/packages/core/src/metric-summary.ts b/packages/core/src/metric-summary.ts deleted file mode 100644 index 18aedb9e5fe4..000000000000 --- a/packages/core/src/metric-summary.ts +++ /dev/null @@ -1,56 +0,0 @@ -import type { MeasurementUnit, Primitive, SpanMetricSummaryAggregator } from '@sentry/types'; -import type { MetricSpanSummary } from '@sentry/types'; -import { dropUndefinedKeys } from '@sentry/utils'; - -/** */ -export class MetricSummaryAggregator implements SpanMetricSummaryAggregator { - private readonly _measurements: Map; - - public constructor() { - this._measurements = new Map(); - } - - /** @inheritdoc */ - public add( - metricType: 'c' | 'g' | 's' | 'd', - name: string, - value: number, - unit?: MeasurementUnit | undefined, - tags?: Record | undefined, - ): void { - const exportKey = `${metricType}:${name}@${unit}`; - const bucketKey = `${exportKey}\n${JSON.stringify(tags)}`; - - const summary = this._measurements.get(bucketKey); - - if (summary) { - this._measurements.set(bucketKey, { - min: Math.min(summary.min, value), - max: Math.max(summary.max, value), - count: (summary.count += 1), - sum: (summary.sum += value), - tags: summary.tags, - }); - } else { - this._measurements.set(bucketKey, { - min: value, - max: value, - count: 1, - sum: value, - tags: tags, - }); - } - } - - /** @inheritdoc */ - public getSummaryJson(): Record { - const output: Record = {}; - - for (const [bucketKey, summary] of this._measurements) { - const [exportKey] = bucketKey.split('\n'); - output[exportKey] = dropUndefinedKeys(summary); - } - - return output; - } -} diff --git a/packages/core/src/metrics/aggregator.ts b/packages/core/src/metrics/aggregator.ts index 6a49fda5918b..826b8e9ae0fe 100644 --- a/packages/core/src/metrics/aggregator.ts +++ b/packages/core/src/metrics/aggregator.ts @@ -8,6 +8,7 @@ import type { import { timestampInSeconds } from '@sentry/utils'; import { DEFAULT_FLUSH_INTERVAL, MAX_WEIGHT, NAME_AND_TAG_KEY_NORMALIZATION_REGEX } from './constants'; import { METRIC_MAP } from './instance'; +import { updateMetricSummaryOnActiveSpan } from './metric-summary'; import type { MetricBucket, MetricType } from './types'; import { getBucketKey, sanitizeTags } from './utils'; @@ -62,6 +63,9 @@ export class MetricsAggregator implements MetricsAggregatorBase { const tags = sanitizeTags(unsanitizedTags); const bucketKey = getBucketKey(metricType, name, unit, tags); + + updateMetricSummaryOnActiveSpan(metricType, name, value, unit, unsanitizedTags, bucketKey); + let bucketItem = this._buckets.get(bucketKey); if (bucketItem) { bucketItem.metric.add(value); diff --git a/packages/core/src/metrics/browser-aggregator.ts b/packages/core/src/metrics/browser-aggregator.ts index 5b5c81353024..782bee5577a0 100644 --- a/packages/core/src/metrics/browser-aggregator.ts +++ b/packages/core/src/metrics/browser-aggregator.ts @@ -9,6 +9,7 @@ import type { import { timestampInSeconds } from '@sentry/utils'; import { DEFAULT_BROWSER_FLUSH_INTERVAL, NAME_AND_TAG_KEY_NORMALIZATION_REGEX } from './constants'; import { METRIC_MAP } from './instance'; +import { updateMetricSummaryOnActiveSpan } from './metric-summary'; import type { MetricBucket, MetricType } from './types'; import { getBucketKey, sanitizeTags } from './utils'; @@ -46,6 +47,9 @@ export class BrowserMetricsAggregator implements MetricsAggregator { const tags = sanitizeTags(unsanitizedTags); const bucketKey = getBucketKey(metricType, name, unit, tags); + + updateMetricSummaryOnActiveSpan(metricType, name, value, unit, unsanitizedTags, bucketKey); + const bucketItem: MetricBucketItem | undefined = this._buckets.get(bucketKey); if (bucketItem) { bucketItem.metric.add(value); diff --git a/packages/core/src/metrics/exports.ts b/packages/core/src/metrics/exports.ts index 02b9314884ef..20d63a2ca119 100644 --- a/packages/core/src/metrics/exports.ts +++ b/packages/core/src/metrics/exports.ts @@ -43,16 +43,8 @@ function addToMetricsAggregator( metricTags.transaction = spanToJSON(transaction).description || ''; } - const combinedTags = { ...metricTags, ...tags }; - - // eslint-disable-next-line deprecation/deprecation - const span = scope.getSpan(); - if (span && typeof value === 'number') { - span.getMetricSummaryAggregator().add(metricType, name, value, unit, combinedTags); - } - DEBUG_BUILD && logger.log(`Adding value of ${value} to ${metricType} metric ${name}`); - client.metricsAggregator.add(metricType, name, value, unit, combinedTags, timestamp); + client.metricsAggregator.add(metricType, name, value, unit, { ...metricTags, ...tags }, timestamp); } } diff --git a/packages/core/src/metrics/metric-summary.ts b/packages/core/src/metrics/metric-summary.ts new file mode 100644 index 000000000000..c74c81b5bb07 --- /dev/null +++ b/packages/core/src/metrics/metric-summary.ts @@ -0,0 +1,90 @@ +import type { MeasurementUnit, MetricSummaryAggregator as MetricSummaryAggregatorInterface } from '@sentry/types'; +import type { MetricSpanSummary } from '@sentry/types'; +import type { Primitive } from '@sentry/types'; +import { dropUndefinedKeys } from '@sentry/utils'; +import { getActiveSpan } from '../tracing'; + +/** + * Updates the metric summary on the currently active span + */ +export function updateMetricSummaryOnActiveSpan( + metricType: 'c' | 'g' | 's' | 'd', + sanitizedName: string, + value: number | string, + unit: MeasurementUnit, + tags: Record, + bucketKey: string, +): void { + const span = getActiveSpan(); + if (span) { + const summary = span.getMetricSummary() || new MetricSummaryAggregator(); + summary.add(metricType, sanitizedName, value, unit, tags, bucketKey); + span.setMetricSummary(summary); + } +} + +/** + * Summaries metrics for spans + */ +class MetricSummaryAggregator implements MetricSummaryAggregatorInterface { + /** + * key: bucketKey + * value: [exportKey, MetricSpanSummary] + */ + private readonly _measurements: Map; + + public constructor() { + this._measurements = new Map(); + } + + /** @inheritdoc */ + public add( + metricType: 'c' | 'g' | 's' | 'd', + sanitizedName: string, + value: number | string, + unit: MeasurementUnit, + sanitizedTags: Record, + bucketKey: string, + ): void { + const exportKey = `${metricType}:${sanitizedName}@${unit}`; + const bucketItem = this._measurements.get(bucketKey); + + if (bucketItem) { + const val = typeof value === 'string' ? 1 : value; + const [, summary] = bucketItem; + this._measurements.set(bucketKey, [ + exportKey, + { + min: Math.min(summary.min, val), + max: Math.max(summary.max, val), + count: (summary.count += 1), + sum: (summary.sum += val), + tags: summary.tags, + }, + ]); + } else { + const val = typeof value === 'string' ? 0 : value; + this._measurements.set(bucketKey, [ + exportKey, + { + min: val, + max: val, + count: 1, + sum: val, + tags: sanitizedTags, + }, + ]); + } + } + + /** @inheritdoc */ + public getJson(): Record { + const output: Record = {}; + + for (const [, [exportKey, summary]] of this._measurements) { + output[exportKey] = dropUndefinedKeys(summary); + } + + return output; + } +} diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index bad8db0d8cb7..0487a5e92074 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -1,6 +1,7 @@ /* eslint-disable max-lines */ import type { Instrumenter, + MetricSummaryAggregator, Primitive, Span as SpanInterface, SpanAttributeValue, @@ -8,7 +9,6 @@ import type { SpanContext, SpanContextData, SpanJSON, - SpanMetricSummaryAggregator, SpanOrigin, SpanTimeInput, TraceContext, @@ -17,7 +17,6 @@ import type { import { dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; -import { MetricSummaryAggregator } from '../metric-summary'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes'; import { getRootSpan } from '../utils/getRootSpan'; import { @@ -606,14 +605,15 @@ export class Span implements SpanInterface { } /** @inheritdoc */ - public getMetricSummaryAggregator(): SpanMetricSummaryAggregator { - if (!this._metricSummary) { - this._metricSummary = new MetricSummaryAggregator(); - } - + public getMetricSummary(): MetricSummaryAggregator | undefined { return this._metricSummary; } + /** @inheritdoc */ + public setMetricSummary(metricSummary: MetricSummaryAggregator): void { + this._metricSummary = metricSummary; + } + /** * Get JSON representation of this span. * @@ -636,7 +636,7 @@ export class Span implements SpanInterface { timestamp: this._endTime, trace_id: this._traceId, origin: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] as SpanOrigin | undefined, - _metrics_summary: this._metricSummary ? this._metricSummary.getSummaryJson() : undefined, + _metrics_summary: this._metricSummary ? this._metricSummary.getJson() : undefined, }); } diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index 490714636fe9..baedb5ec6601 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/packages/core/src/tracing/transaction.ts @@ -333,6 +333,10 @@ export class Transaction extends SpanClass implements TransactionInterface { }), }; + if (this._metricSummary) { + transaction._metrics_summary = this._metricSummary.getJson(); + } + const hasMeasurements = Object.keys(this._measurements).length > 0; if (hasMeasurements) { diff --git a/packages/types/src/event.ts b/packages/types/src/event.ts index 50322f18fbc6..9e16100bbb1b 100644 --- a/packages/types/src/event.ts +++ b/packages/types/src/event.ts @@ -11,7 +11,7 @@ import type { Request } from './request'; import type { CaptureContext } from './scope'; import type { SdkInfo } from './sdkinfo'; import type { Severity, SeverityLevel } from './severity'; -import type { Span, SpanJSON } from './span'; +import type { MetricSummary, Span, SpanJSON } from './span'; import type { Thread } from './thread'; import type { TransactionSource } from './transaction'; import type { User } from './user'; @@ -73,6 +73,7 @@ export interface ErrorEvent extends Event { } export interface TransactionEvent extends Event { type: 'transaction'; + _metrics_summary?: Record; } /** JSDoc */ diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index d43379062191..2be8ca9c6d26 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -151,5 +151,10 @@ export type { export type { BrowserClientReplayOptions, BrowserClientProfilingOptions } from './browseroptions'; export type { CheckIn, MonitorConfig, FinishedCheckIn, InProgressCheckIn, SerializedCheckIn } from './checkin'; -export type { MetricsAggregator, MetricBucketItem, MetricInstance, SpanMetricSummaryAggregator } from './metrics'; +export type { + MetricsAggregator, + MetricBucketItem, + MetricInstance, + MetricSummaryAggregator, +} from './metrics'; export type { ParameterizedString } from './parameterize'; diff --git a/packages/types/src/metrics.ts b/packages/types/src/metrics.ts index 2eacfe58b33a..46bb240e8eee 100644 --- a/packages/types/src/metrics.ts +++ b/packages/types/src/metrics.ts @@ -64,16 +64,17 @@ export interface MetricsAggregator { toString(): string; } -export interface SpanMetricSummaryAggregator { +export interface MetricSummaryAggregator { /** Adds a metric to the summary */ add( metricType: 'c' | 'g' | 's' | 'd', - name: string, + sanitizedName: string, value: number | string, - unit?: MeasurementUnit, - tags?: Record, + unit: MeasurementUnit, + tags: Record, + bucketKey: string, ): void; /** Gets the JSON representation of the metric summary */ - getSummaryJson(): Record; + getJson(): Record; } diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index cc27bcc8e5f2..cab02d5f43bc 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -1,6 +1,6 @@ import type { TraceContext } from './context'; import type { Instrumenter } from './instrumenter'; -import type { SpanMetricSummaryAggregator } from './metrics'; +import type { MetricSummaryAggregator } from './metrics'; import type { Primitive } from './misc'; import type { HrTime } from './opentelemetry'; import type { Transaction } from './transaction'; @@ -397,10 +397,9 @@ export interface Span extends Omit { */ getTraceContext(): TraceContext; - /** - * Gets the metric summary aggregator for this span - */ - getMetricSummaryAggregator(): SpanMetricSummaryAggregator; + getMetricSummary(): MetricSummaryAggregator | undefined; + + setMetricSummary(summary: MetricSummaryAggregator): void; /** * Convert the object to JSON. From 9c4f3c1ed91d28d808ef5efd0563a5217acf25d3 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 31 Jan 2024 20:04:31 -0400 Subject: [PATCH 4/8] Don't rename MetricSummary on export --- packages/core/src/metrics/metric-summary.ts | 10 +++++----- packages/types/src/index.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/core/src/metrics/metric-summary.ts b/packages/core/src/metrics/metric-summary.ts index c74c81b5bb07..c780da99ce4d 100644 --- a/packages/core/src/metrics/metric-summary.ts +++ b/packages/core/src/metrics/metric-summary.ts @@ -1,5 +1,5 @@ import type { MeasurementUnit, MetricSummaryAggregator as MetricSummaryAggregatorInterface } from '@sentry/types'; -import type { MetricSpanSummary } from '@sentry/types'; +import type { MetricSummary } from '@sentry/types'; import type { Primitive } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; import { getActiveSpan } from '../tracing'; @@ -31,10 +31,10 @@ class MetricSummaryAggregator implements MetricSummaryAggregatorInterface { * key: bucketKey * value: [exportKey, MetricSpanSummary] */ - private readonly _measurements: Map; + private readonly _measurements: Map; public constructor() { - this._measurements = new Map(); + this._measurements = new Map(); } /** @inheritdoc */ @@ -78,8 +78,8 @@ class MetricSummaryAggregator implements MetricSummaryAggregatorInterface { } /** @inheritdoc */ - public getJson(): Record { - const output: Record = {}; + public getJson(): Record { + const output: Record = {}; for (const [, [exportKey, summary]] of this._measurements) { output[exportKey] = dropUndefinedKeys(summary); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 2be8ca9c6d26..3242190dc506 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -99,7 +99,7 @@ export type { SpanJSON, SpanContextData, TraceFlag, - MetricSummary as MetricSpanSummary, + MetricSummary, } from './span'; export type { StackFrame } from './stackframe'; export type { Stacktrace, StackParser, StackLineParser, StackLineParserFn } from './stacktrace'; From 8b16e4d65ff9bc15469b109ab6a76e50ce69d50f Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 31 Jan 2024 20:08:22 -0400 Subject: [PATCH 5/8] minor --- packages/core/src/metrics/metric-summary.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/core/src/metrics/metric-summary.ts b/packages/core/src/metrics/metric-summary.ts index c780da99ce4d..c466f6783fcd 100644 --- a/packages/core/src/metrics/metric-summary.ts +++ b/packages/core/src/metrics/metric-summary.ts @@ -43,13 +43,14 @@ class MetricSummaryAggregator implements MetricSummaryAggregatorInterface { sanitizedName: string, value: number | string, unit: MeasurementUnit, - sanitizedTags: Record, + tags: Record, bucketKey: string, ): void { const exportKey = `${metricType}:${sanitizedName}@${unit}`; const bucketItem = this._measurements.get(bucketKey); if (bucketItem) { + // if value is string, this was a set, so value should be 1 const val = typeof value === 'string' ? 1 : value; const [, summary] = bucketItem; this._measurements.set(bucketKey, [ @@ -63,6 +64,7 @@ class MetricSummaryAggregator implements MetricSummaryAggregatorInterface { }, ]); } else { + // if value is string, this was a set, so value should be 0 const val = typeof value === 'string' ? 0 : value; this._measurements.set(bucketKey, [ exportKey, @@ -71,7 +73,7 @@ class MetricSummaryAggregator implements MetricSummaryAggregatorInterface { max: val, count: 1, sum: val, - tags: sanitizedTags, + tags, }, ]); } From d3f52df108f2b06a35a92b13a65b57f53d74ba3f Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 31 Jan 2024 20:46:25 -0400 Subject: [PATCH 6/8] correctly handle set metrics --- packages/core/src/metrics/aggregator.ts | 9 ++++++-- .../core/src/metrics/browser-aggregator.ts | 23 +++++++++---------- packages/core/src/metrics/metric-summary.ts | 20 +++++++--------- packages/types/src/metrics.ts | 2 +- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/packages/core/src/metrics/aggregator.ts b/packages/core/src/metrics/aggregator.ts index 826b8e9ae0fe..bb0d7333f104 100644 --- a/packages/core/src/metrics/aggregator.ts +++ b/packages/core/src/metrics/aggregator.ts @@ -64,9 +64,10 @@ export class MetricsAggregator implements MetricsAggregatorBase { const bucketKey = getBucketKey(metricType, name, unit, tags); - updateMetricSummaryOnActiveSpan(metricType, name, value, unit, unsanitizedTags, bucketKey); - let bucketItem = this._buckets.get(bucketKey); + // If this is a set metric, we need to calculate the delta from the previous weight. + const previousWeight = bucketItem && metricType === 's' ? bucketItem.metric.weight : 0; + if (bucketItem) { bucketItem.metric.add(value); // TODO(abhi): Do we need this check? @@ -86,6 +87,10 @@ export class MetricsAggregator implements MetricsAggregatorBase { this._buckets.set(bucketKey, bucketItem); } + // If value is a string, it's a set metric so calculate the delta from the previous weight. + const val = typeof value === 'string' ? bucketItem.metric.weight - previousWeight : value; + updateMetricSummaryOnActiveSpan(metricType, name, val, unit, unsanitizedTags, bucketKey); + // We need to keep track of the total weight of the buckets so that we can // flush them when we exceed the max weight. this._bucketsTotalWeight += bucketItem.metric.weight; diff --git a/packages/core/src/metrics/browser-aggregator.ts b/packages/core/src/metrics/browser-aggregator.ts index 782bee5577a0..7c4c23757edc 100644 --- a/packages/core/src/metrics/browser-aggregator.ts +++ b/packages/core/src/metrics/browser-aggregator.ts @@ -1,11 +1,4 @@ -import type { - Client, - ClientOptions, - MeasurementUnit, - MetricBucketItem, - MetricsAggregator, - Primitive, -} from '@sentry/types'; +import type { Client, ClientOptions, MeasurementUnit, MetricsAggregator, Primitive } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; import { DEFAULT_BROWSER_FLUSH_INTERVAL, NAME_AND_TAG_KEY_NORMALIZATION_REGEX } from './constants'; import { METRIC_MAP } from './instance'; @@ -48,9 +41,10 @@ export class BrowserMetricsAggregator implements MetricsAggregator { const bucketKey = getBucketKey(metricType, name, unit, tags); - updateMetricSummaryOnActiveSpan(metricType, name, value, unit, unsanitizedTags, bucketKey); + let bucketItem = this._buckets.get(bucketKey); + // If this is a set metric, we need to calculate the delta from the previous weight. + const previousWeight = bucketItem && metricType === 's' ? bucketItem.metric.weight : 0; - const bucketItem: MetricBucketItem | undefined = this._buckets.get(bucketKey); if (bucketItem) { bucketItem.metric.add(value); // TODO(abhi): Do we need this check? @@ -58,7 +52,7 @@ export class BrowserMetricsAggregator implements MetricsAggregator { bucketItem.timestamp = timestamp; } } else { - this._buckets.set(bucketKey, { + bucketItem = { // @ts-expect-error we don't need to narrow down the type of value here, saves bundle size. metric: new METRIC_MAP[metricType](value), timestamp, @@ -66,8 +60,13 @@ export class BrowserMetricsAggregator implements MetricsAggregator { name, unit, tags, - }); + }; + this._buckets.set(bucketKey, bucketItem); } + + // If value is a string, it's a set metric so calculate the delta from the previous weight. + const val = typeof value === 'string' ? bucketItem.metric.weight - previousWeight : value; + updateMetricSummaryOnActiveSpan(metricType, name, val, unit, unsanitizedTags, bucketKey); } /** diff --git a/packages/core/src/metrics/metric-summary.ts b/packages/core/src/metrics/metric-summary.ts index c466f6783fcd..e15cc1e108c0 100644 --- a/packages/core/src/metrics/metric-summary.ts +++ b/packages/core/src/metrics/metric-summary.ts @@ -10,7 +10,7 @@ import { getActiveSpan } from '../tracing'; export function updateMetricSummaryOnActiveSpan( metricType: 'c' | 'g' | 's' | 'd', sanitizedName: string, - value: number | string, + value: number, unit: MeasurementUnit, tags: Record, bucketKey: string, @@ -41,7 +41,7 @@ class MetricSummaryAggregator implements MetricSummaryAggregatorInterface { public add( metricType: 'c' | 'g' | 's' | 'd', sanitizedName: string, - value: number | string, + value: number, unit: MeasurementUnit, tags: Record, bucketKey: string, @@ -50,29 +50,25 @@ class MetricSummaryAggregator implements MetricSummaryAggregatorInterface { const bucketItem = this._measurements.get(bucketKey); if (bucketItem) { - // if value is string, this was a set, so value should be 1 - const val = typeof value === 'string' ? 1 : value; const [, summary] = bucketItem; this._measurements.set(bucketKey, [ exportKey, { - min: Math.min(summary.min, val), - max: Math.max(summary.max, val), + min: Math.min(summary.min, value), + max: Math.max(summary.max, value), count: (summary.count += 1), - sum: (summary.sum += val), + sum: (summary.sum += value), tags: summary.tags, }, ]); } else { - // if value is string, this was a set, so value should be 0 - const val = typeof value === 'string' ? 0 : value; this._measurements.set(bucketKey, [ exportKey, { - min: val, - max: val, + min: value, + max: value, count: 1, - sum: val, + sum: value, tags, }, ]); diff --git a/packages/types/src/metrics.ts b/packages/types/src/metrics.ts index 46bb240e8eee..eada1b97801a 100644 --- a/packages/types/src/metrics.ts +++ b/packages/types/src/metrics.ts @@ -69,7 +69,7 @@ export interface MetricSummaryAggregator { add( metricType: 'c' | 'g' | 's' | 'd', sanitizedName: string, - value: number | string, + value: number, unit: MeasurementUnit, tags: Record, bucketKey: string, From 1a485c2478d8fb7999d4b30c9f6d9c9a24246d6d Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sat, 3 Feb 2024 17:39:09 -0400 Subject: [PATCH 7/8] Fix issues! --- packages/core/src/metrics/aggregator.ts | 4 +- .../core/src/metrics/browser-aggregator.ts | 4 +- packages/core/src/metrics/metric-summary.ts | 81 +++++++++---------- packages/core/src/tracing/span.ts | 15 +--- packages/core/src/tracing/transaction.ts | 6 +- packages/types/src/index.ts | 1 - packages/types/src/metrics.ts | 16 ---- packages/types/src/span.ts | 5 -- 8 files changed, 48 insertions(+), 84 deletions(-) diff --git a/packages/core/src/metrics/aggregator.ts b/packages/core/src/metrics/aggregator.ts index bb0d7333f104..2b331082ab3e 100644 --- a/packages/core/src/metrics/aggregator.ts +++ b/packages/core/src/metrics/aggregator.ts @@ -6,7 +6,7 @@ import type { Primitive, } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; -import { DEFAULT_FLUSH_INTERVAL, MAX_WEIGHT, NAME_AND_TAG_KEY_NORMALIZATION_REGEX } from './constants'; +import { DEFAULT_FLUSH_INTERVAL, MAX_WEIGHT, NAME_AND_TAG_KEY_NORMALIZATION_REGEX, SET_METRIC_TYPE } from './constants'; import { METRIC_MAP } from './instance'; import { updateMetricSummaryOnActiveSpan } from './metric-summary'; import type { MetricBucket, MetricType } from './types'; @@ -66,7 +66,7 @@ export class MetricsAggregator implements MetricsAggregatorBase { let bucketItem = this._buckets.get(bucketKey); // If this is a set metric, we need to calculate the delta from the previous weight. - const previousWeight = bucketItem && metricType === 's' ? bucketItem.metric.weight : 0; + const previousWeight = bucketItem && metricType === SET_METRIC_TYPE ? bucketItem.metric.weight : 0; if (bucketItem) { bucketItem.metric.add(value); diff --git a/packages/core/src/metrics/browser-aggregator.ts b/packages/core/src/metrics/browser-aggregator.ts index 7c4c23757edc..40cfa1d404ab 100644 --- a/packages/core/src/metrics/browser-aggregator.ts +++ b/packages/core/src/metrics/browser-aggregator.ts @@ -1,6 +1,6 @@ import type { Client, ClientOptions, MeasurementUnit, MetricsAggregator, Primitive } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; -import { DEFAULT_BROWSER_FLUSH_INTERVAL, NAME_AND_TAG_KEY_NORMALIZATION_REGEX } from './constants'; +import { DEFAULT_BROWSER_FLUSH_INTERVAL, NAME_AND_TAG_KEY_NORMALIZATION_REGEX, SET_METRIC_TYPE } from './constants'; import { METRIC_MAP } from './instance'; import { updateMetricSummaryOnActiveSpan } from './metric-summary'; import type { MetricBucket, MetricType } from './types'; @@ -43,7 +43,7 @@ export class BrowserMetricsAggregator implements MetricsAggregator { let bucketItem = this._buckets.get(bucketKey); // If this is a set metric, we need to calculate the delta from the previous weight. - const previousWeight = bucketItem && metricType === 's' ? bucketItem.metric.weight : 0; + const previousWeight = bucketItem && metricType === SET_METRIC_TYPE ? bucketItem.metric.weight : 0; if (bucketItem) { bucketItem.metric.add(value); diff --git a/packages/core/src/metrics/metric-summary.ts b/packages/core/src/metrics/metric-summary.ts index e15cc1e108c0..dff610574b2c 100644 --- a/packages/core/src/metrics/metric-summary.ts +++ b/packages/core/src/metrics/metric-summary.ts @@ -1,14 +1,45 @@ -import type { MeasurementUnit, MetricSummaryAggregator as MetricSummaryAggregatorInterface } from '@sentry/types'; +import type { MeasurementUnit, Span } from '@sentry/types'; import type { MetricSummary } from '@sentry/types'; import type { Primitive } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; import { getActiveSpan } from '../tracing'; +import type { MetricType } from './types'; + +/** + * key: bucketKey + * value: [exportKey, MetricSummary] + */ +type MetricSummaryStorage = Map; + +let SPAN_METRIC_SUMMARY: WeakMap | undefined; + +function getMetricStorageForSpan(span: Span): MetricSummaryStorage | undefined { + return SPAN_METRIC_SUMMARY ? SPAN_METRIC_SUMMARY.get(span) : undefined; +} + +/** + * Fetches the metric summary if it exists for the passed span + */ +export function getMetricSummaryJsonForSpan(span: Span): Record | undefined { + const storage = getMetricStorageForSpan(span); + + if (!storage) { + return undefined; + } + const output: Record = {}; + + for (const [, [exportKey, summary]] of storage) { + output[exportKey] = dropUndefinedKeys(summary); + } + + return output; +} /** * Updates the metric summary on the currently active span */ export function updateMetricSummaryOnActiveSpan( - metricType: 'c' | 'g' | 's' | 'd', + metricType: MetricType, sanitizedName: string, value: number, unit: MeasurementUnit, @@ -17,41 +48,14 @@ export function updateMetricSummaryOnActiveSpan( ): void { const span = getActiveSpan(); if (span) { - const summary = span.getMetricSummary() || new MetricSummaryAggregator(); - summary.add(metricType, sanitizedName, value, unit, tags, bucketKey); - span.setMetricSummary(summary); - } -} - -/** - * Summaries metrics for spans - */ -class MetricSummaryAggregator implements MetricSummaryAggregatorInterface { - /** - * key: bucketKey - * value: [exportKey, MetricSpanSummary] - */ - private readonly _measurements: Map; - - public constructor() { - this._measurements = new Map(); - } + const storage = getMetricStorageForSpan(span) || new Map(); - /** @inheritdoc */ - public add( - metricType: 'c' | 'g' | 's' | 'd', - sanitizedName: string, - value: number, - unit: MeasurementUnit, - tags: Record, - bucketKey: string, - ): void { const exportKey = `${metricType}:${sanitizedName}@${unit}`; - const bucketItem = this._measurements.get(bucketKey); + const bucketItem = storage.get(bucketKey); if (bucketItem) { const [, summary] = bucketItem; - this._measurements.set(bucketKey, [ + storage.set(bucketKey, [ exportKey, { min: Math.min(summary.min, value), @@ -62,7 +66,7 @@ class MetricSummaryAggregator implements MetricSummaryAggregatorInterface { }, ]); } else { - this._measurements.set(bucketKey, [ + storage.set(bucketKey, [ exportKey, { min: value, @@ -73,16 +77,11 @@ class MetricSummaryAggregator implements MetricSummaryAggregatorInterface { }, ]); } - } - - /** @inheritdoc */ - public getJson(): Record { - const output: Record = {}; - for (const [, [exportKey, summary]] of this._measurements) { - output[exportKey] = dropUndefinedKeys(summary); + if (!SPAN_METRIC_SUMMARY) { + SPAN_METRIC_SUMMARY = new WeakMap(); } - return output; + SPAN_METRIC_SUMMARY.set(span, storage); } } diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index 0487a5e92074..1e12628ae2a1 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -1,7 +1,6 @@ /* eslint-disable max-lines */ import type { Instrumenter, - MetricSummaryAggregator, Primitive, Span as SpanInterface, SpanAttributeValue, @@ -17,6 +16,7 @@ import type { import { dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; +import { getMetricSummaryJsonForSpan } from '../metrics/metric-summary'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes'; import { getRootSpan } from '../utils/getRootSpan'; import { @@ -115,7 +115,6 @@ export class Span implements SpanInterface { protected _endTime?: number; /** Internal keeper of the status */ protected _status?: SpanStatusType | string; - protected _metricSummary: MetricSummaryAggregator | undefined; private _logMessage?: string; @@ -604,16 +603,6 @@ export class Span implements SpanInterface { return spanToTraceContext(this); } - /** @inheritdoc */ - public getMetricSummary(): MetricSummaryAggregator | undefined { - return this._metricSummary; - } - - /** @inheritdoc */ - public setMetricSummary(metricSummary: MetricSummaryAggregator): void { - this._metricSummary = metricSummary; - } - /** * Get JSON representation of this span. * @@ -636,7 +625,7 @@ export class Span implements SpanInterface { timestamp: this._endTime, trace_id: this._traceId, origin: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] as SpanOrigin | undefined, - _metrics_summary: this._metricSummary ? this._metricSummary.getJson() : undefined, + _metrics_summary: getMetricSummaryJsonForSpan(this), }); } diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index baedb5ec6601..08dd8c5e9ca0 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/packages/core/src/tracing/transaction.ts @@ -15,6 +15,7 @@ import { dropUndefinedKeys, logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; import type { Hub } from '../hub'; import { getCurrentHub } from '../hub'; +import { getMetricSummaryJsonForSpan } from '../metrics/metric-summary'; import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; import { spanTimeInputToSeconds, spanToJSON, spanToTraceContext } from '../utils/spanUtils'; import { getDynamicSamplingContextFromSpan } from './dynamicSamplingContext'; @@ -326,6 +327,7 @@ export class Transaction extends SpanClass implements TransactionInterface { ...metadata, dynamicSamplingContext: getDynamicSamplingContextFromSpan(this), }, + _metrics_summary: getMetricSummaryJsonForSpan(this), ...(source && { transaction_info: { source, @@ -333,10 +335,6 @@ export class Transaction extends SpanClass implements TransactionInterface { }), }; - if (this._metricSummary) { - transaction._metrics_summary = this._metricSummary.getJson(); - } - const hasMeasurements = Object.keys(this._measurements).length > 0; if (hasMeasurements) { diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 3242190dc506..d4fcd439ae4a 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -155,6 +155,5 @@ export type { MetricsAggregator, MetricBucketItem, MetricInstance, - MetricSummaryAggregator, } from './metrics'; export type { ParameterizedString } from './parameterize'; diff --git a/packages/types/src/metrics.ts b/packages/types/src/metrics.ts index eada1b97801a..0f8dc4f53435 100644 --- a/packages/types/src/metrics.ts +++ b/packages/types/src/metrics.ts @@ -1,6 +1,5 @@ import type { MeasurementUnit } from './measurement'; import type { Primitive } from './misc'; -import type { MetricSummary } from './span'; /** * An abstract definition of the minimum required API @@ -63,18 +62,3 @@ export interface MetricsAggregator { */ toString(): string; } - -export interface MetricSummaryAggregator { - /** Adds a metric to the summary */ - add( - metricType: 'c' | 'g' | 's' | 'd', - sanitizedName: string, - value: number, - unit: MeasurementUnit, - tags: Record, - bucketKey: string, - ): void; - - /** Gets the JSON representation of the metric summary */ - getJson(): Record; -} diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index cab02d5f43bc..0743497f1411 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -1,6 +1,5 @@ import type { TraceContext } from './context'; import type { Instrumenter } from './instrumenter'; -import type { MetricSummaryAggregator } from './metrics'; import type { Primitive } from './misc'; import type { HrTime } from './opentelemetry'; import type { Transaction } from './transaction'; @@ -397,10 +396,6 @@ export interface Span extends Omit { */ getTraceContext(): TraceContext; - getMetricSummary(): MetricSummaryAggregator | undefined; - - setMetricSummary(summary: MetricSummaryAggregator): void; - /** * Convert the object to JSON. * @deprecated Use `spanToJSON(span)` instead. From 1e8fed74f06bd4a2854889e938c6d03c1c378579 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sat, 3 Feb 2024 19:36:12 -0400 Subject: [PATCH 8/8] Test all types --- .../tracing/metric-summaries/scenario.js | 10 +++++++ .../suites/tracing/metric-summaries/test.ts | 30 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js index 657e4f789cb8..8a7dbabe0dec 100644 --- a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js @@ -32,6 +32,16 @@ Sentry.startSpan( Sentry.metrics.increment('root-counter'); Sentry.metrics.increment('root-counter'); Sentry.metrics.increment('root-counter', 2); + + Sentry.metrics.set('root-set', 'some-value'); + Sentry.metrics.set('root-set', 'another-value'); + Sentry.metrics.set('root-set', 'another-value'); + + Sentry.metrics.gauge('root-gauge', 42); + Sentry.metrics.gauge('root-gauge', 20); + + Sentry.metrics.distribution('root-distribution', 42); + Sentry.metrics.distribution('root-distribution', 20); }, ); }, diff --git a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts index c7eb7c753766..98ed58a75c57 100644 --- a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts @@ -29,6 +29,36 @@ const EXPECTED_TRANSACTION = { transaction: 'Test Transaction', }, }, + 's:root-set@none': { + min: 0, + max: 1, + count: 3, + sum: 2, + tags: { + release: '1.0', + transaction: 'Test Transaction', + }, + }, + 'g:root-gauge@none': { + min: 20, + max: 42, + count: 2, + sum: 62, + tags: { + release: '1.0', + transaction: 'Test Transaction', + }, + }, + 'd:root-distribution@none': { + min: 20, + max: 42, + count: 2, + sum: 62, + tags: { + release: '1.0', + transaction: 'Test Transaction', + }, + }, }, }), ]),