From 61e9a77a6f3aa56abda172da86993494b5ca8e25 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 7 Feb 2024 17:08:46 +0100 Subject: [PATCH 01/10] Revert "Revert "feat(core): Add metric summaries to spans (#10432)" (#10495)" This reverts commit 60a7d65605d7db4de854e4071896abf168e6bdc5. # Conflicts: # packages/types/src/event.ts --- .../tracing/metric-summaries/scenario.js | 48 ++++++++++ .../suites/tracing/metric-summaries/test.ts | 69 +++++++++++++++ packages/core/src/metrics/aggregator.ts | 11 ++- .../core/src/metrics/browser-aggregator.ts | 27 +++--- packages/core/src/metrics/metric-summary.ts | 87 +++++++++++++++++++ packages/core/src/tracing/span.ts | 2 + packages/core/src/tracing/transaction.ts | 2 + packages/types/src/event.ts | 3 +- packages/types/src/index.ts | 7 +- packages/types/src/span.ts | 9 ++ 10 files changed, 250 insertions(+), 15 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/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 new file mode 100644 index 000000000000..8a7dbabe0dec --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js @@ -0,0 +1,48 @@ +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.metrics.increment('root-counter'); + Sentry.metrics.increment('root-counter'); + + Sentry.startSpan( + { + name: 'Some other span', + op: 'transaction', + }, + () => { + Sentry.metrics.increment('root-counter'); + Sentry.metrics.increment('root-counter'); + Sentry.metrics.increment('root-counter', 2); + + Sentry.metrics.set('root-set', 'some-value'); + Sentry.metrics.set('root-set', 'another-value'); + Sentry.metrics.set('root-set', 'another-value'); + + Sentry.metrics.gauge('root-gauge', 42); + Sentry.metrics.gauge('root-gauge', 20); + + Sentry.metrics.distribution('root-distribution', 42); + Sentry.metrics.distribution('root-distribution', 20); + }, + ); + }, +); 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..98ed58a75c57 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/test.ts @@ -0,0 +1,69 @@ +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@none': { + min: 1, + max: 2, + count: 3, + sum: 4, + tags: { + release: '1.0', + transaction: 'Test Transaction', + }, + }, + 's:root-set@none': { + min: 0, + max: 1, + count: 3, + sum: 2, + tags: { + release: '1.0', + transaction: 'Test Transaction', + }, + }, + 'g:root-gauge@none': { + min: 20, + max: 42, + count: 2, + sum: 62, + tags: { + release: '1.0', + transaction: 'Test Transaction', + }, + }, + 'd:root-distribution@none': { + min: 20, + max: 42, + count: 2, + sum: 62, + tags: { + release: '1.0', + transaction: 'Test Transaction', + }, + }, + }, + }), + ]), +}; + +test('Should add metric summaries to spans', done => { + createRunner(__dirname, 'scenario.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); +}); diff --git a/packages/core/src/metrics/aggregator.ts b/packages/core/src/metrics/aggregator.ts index 6a49fda5918b..2b331082ab3e 100644 --- a/packages/core/src/metrics/aggregator.ts +++ b/packages/core/src/metrics/aggregator.ts @@ -6,8 +6,9 @@ import type { Primitive, } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; -import { DEFAULT_FLUSH_INTERVAL, MAX_WEIGHT, NAME_AND_TAG_KEY_NORMALIZATION_REGEX } from './constants'; +import { DEFAULT_FLUSH_INTERVAL, MAX_WEIGHT, NAME_AND_TAG_KEY_NORMALIZATION_REGEX, SET_METRIC_TYPE } from './constants'; import { METRIC_MAP } from './instance'; +import { updateMetricSummaryOnActiveSpan } from './metric-summary'; import type { MetricBucket, MetricType } from './types'; import { getBucketKey, sanitizeTags } from './utils'; @@ -62,7 +63,11 @@ export class MetricsAggregator implements MetricsAggregatorBase { const tags = sanitizeTags(unsanitizedTags); const bucketKey = getBucketKey(metricType, name, unit, tags); + let bucketItem = this._buckets.get(bucketKey); + // If this is a set metric, we need to calculate the delta from the previous weight. + const previousWeight = bucketItem && metricType === SET_METRIC_TYPE ? bucketItem.metric.weight : 0; + if (bucketItem) { bucketItem.metric.add(value); // TODO(abhi): Do we need this check? @@ -82,6 +87,10 @@ export class MetricsAggregator implements MetricsAggregatorBase { this._buckets.set(bucketKey, bucketItem); } + // If value is a string, it's a set metric so calculate the delta from the previous weight. + const val = typeof value === 'string' ? bucketItem.metric.weight - previousWeight : value; + updateMetricSummaryOnActiveSpan(metricType, name, val, unit, unsanitizedTags, bucketKey); + // We need to keep track of the total weight of the buckets so that we can // flush them when we exceed the max weight. this._bucketsTotalWeight += bucketItem.metric.weight; diff --git a/packages/core/src/metrics/browser-aggregator.ts b/packages/core/src/metrics/browser-aggregator.ts index 5b5c81353024..40cfa1d404ab 100644 --- a/packages/core/src/metrics/browser-aggregator.ts +++ b/packages/core/src/metrics/browser-aggregator.ts @@ -1,14 +1,8 @@ -import type { - Client, - ClientOptions, - MeasurementUnit, - MetricBucketItem, - MetricsAggregator, - Primitive, -} from '@sentry/types'; +import type { Client, ClientOptions, MeasurementUnit, MetricsAggregator, Primitive } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; -import { DEFAULT_BROWSER_FLUSH_INTERVAL, NAME_AND_TAG_KEY_NORMALIZATION_REGEX } from './constants'; +import { DEFAULT_BROWSER_FLUSH_INTERVAL, NAME_AND_TAG_KEY_NORMALIZATION_REGEX, SET_METRIC_TYPE } from './constants'; import { METRIC_MAP } from './instance'; +import { updateMetricSummaryOnActiveSpan } from './metric-summary'; import type { MetricBucket, MetricType } from './types'; import { getBucketKey, sanitizeTags } from './utils'; @@ -46,7 +40,11 @@ export class BrowserMetricsAggregator implements MetricsAggregator { const tags = sanitizeTags(unsanitizedTags); const bucketKey = getBucketKey(metricType, name, unit, tags); - const bucketItem: MetricBucketItem | undefined = this._buckets.get(bucketKey); + + let bucketItem = this._buckets.get(bucketKey); + // If this is a set metric, we need to calculate the delta from the previous weight. + const previousWeight = bucketItem && metricType === SET_METRIC_TYPE ? bucketItem.metric.weight : 0; + if (bucketItem) { bucketItem.metric.add(value); // TODO(abhi): Do we need this check? @@ -54,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, @@ -62,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 new file mode 100644 index 000000000000..dff610574b2c --- /dev/null +++ b/packages/core/src/metrics/metric-summary.ts @@ -0,0 +1,87 @@ +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: MetricType, + sanitizedName: string, + value: number, + unit: MeasurementUnit, + tags: Record, + bucketKey: string, +): void { + const span = getActiveSpan(); + if (span) { + const storage = getMetricStorageForSpan(span) || new Map(); + + const exportKey = `${metricType}:${sanitizedName}@${unit}`; + const bucketItem = storage.get(bucketKey); + + if (bucketItem) { + const [, summary] = bucketItem; + storage.set(bucketKey, [ + exportKey, + { + min: Math.min(summary.min, value), + max: Math.max(summary.max, value), + count: (summary.count += 1), + sum: (summary.sum += value), + tags: summary.tags, + }, + ]); + } else { + storage.set(bucketKey, [ + exportKey, + { + min: value, + max: value, + count: 1, + sum: value, + tags, + }, + ]); + } + + if (!SPAN_METRIC_SUMMARY) { + SPAN_METRIC_SUMMARY = new WeakMap(); + } + + SPAN_METRIC_SUMMARY.set(span, storage); + } +} diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index 165677455d7f..1e12628ae2a1 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -16,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 { @@ -624,6 +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: getMetricSummaryJsonForSpan(this), }); } diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index 026723929471..709aa628f42e 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'; @@ -331,6 +332,7 @@ export class Transaction extends SpanClass implements TransactionInterface { capturedSpanIsolationScope, dynamicSamplingContext: getDynamicSamplingContextFromSpan(this), }, + _metrics_summary: getMetricSummaryJsonForSpan(this), ...(source && { transaction_info: { source, diff --git a/packages/types/src/event.ts b/packages/types/src/event.ts index cfddd6bef471..40ca2e88be65 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 { 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'; @@ -72,6 +72,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 0504803c49a1..abdf076cf033 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -98,6 +98,7 @@ export type { SpanJSON, SpanContextData, TraceFlag, + MetricSummary, } from './span'; export type { StackFrame } from './stackframe'; export type { Stacktrace, StackParser, StackLineParser, StackLineParserFn } from './stacktrace'; @@ -149,5 +150,9 @@ 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, +} from './metrics'; export type { ParameterizedString } from './parameterize'; diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index 73c2fbdaaaa8..0743497f1411 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -31,6 +31,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 +55,7 @@ export interface SpanJSON { timestamp?: number; trace_id: string; origin?: SpanOrigin; + _metrics_summary?: Record; } // These are aligned with OpenTelemetry trace flags From c694cf3410535049dc6ebee02613e3e3e6588d04 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Wed, 7 Feb 2024 17:29:02 +0100 Subject: [PATCH 02/10] Update types and tests --- .../suites/tracing/metric-summaries/test.ts | 102 ++++++++++-------- packages/types/src/event.ts | 2 +- packages/types/src/span.ts | 2 +- 3 files changed, 58 insertions(+), 48 deletions(-) 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 98ed58a75c57..99d9b762b537 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 @@ -3,62 +3,72 @@ 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', + '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@none': { - min: 1, - max: 2, - count: 3, - sum: 4, - tags: { - release: '1.0', - transaction: 'Test Transaction', - }, - }, - 's:root-set@none': { - min: 0, - max: 1, - count: 3, - sum: 2, - tags: { - release: '1.0', - transaction: 'Test Transaction', + 'c:root-counter@none': [ + { + min: 1, + max: 2, + count: 3, + sum: 4, + 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', + ], + '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', + ] + 'd:root-distribution@none': [ + { + min: 20, + max: 42, + count: 2, + sum: 62, + tags: { + release: '1.0', + transaction: 'Test Transaction', + }, }, - }, + ], }, }), ]), diff --git a/packages/types/src/event.ts b/packages/types/src/event.ts index 40ca2e88be65..b64a9e4b1ef4 100644 --- a/packages/types/src/event.ts +++ b/packages/types/src/event.ts @@ -72,7 +72,7 @@ export interface ErrorEvent extends Event { } export interface TransactionEvent extends Event { type: 'transaction'; - _metrics_summary?: Record; + _metrics_summary?: Record>; } /** JSDoc */ diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index 0743497f1411..c254bee08aad 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -55,7 +55,7 @@ export interface SpanJSON { timestamp?: number; trace_id: string; origin?: SpanOrigin; - _metrics_summary?: Record; + _metrics_summary?: Record>; } // These are aligned with OpenTelemetry trace flags From c88ba57da51f62abdf47e3f7de9af5f244c92f63 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 8 Feb 2024 13:14:11 +0100 Subject: [PATCH 03/10] Push new summaries instead of overwriting them --- packages/core/src/metrics/metric-summary.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/src/metrics/metric-summary.ts b/packages/core/src/metrics/metric-summary.ts index dff610574b2c..947152d4d5d4 100644 --- a/packages/core/src/metrics/metric-summary.ts +++ b/packages/core/src/metrics/metric-summary.ts @@ -20,16 +20,16 @@ function getMetricStorageForSpan(span: Span): MetricSummaryStorage | undefined { /** * Fetches the metric summary if it exists for the passed span */ -export function getMetricSummaryJsonForSpan(span: Span): Record | undefined { +export function getMetricSummaryJsonForSpan(span: Span): Record> | undefined { const storage = getMetricStorageForSpan(span); if (!storage) { return undefined; } - const output: Record = {}; + const output: Record> = {}; for (const [, [exportKey, summary]] of storage) { - output[exportKey] = dropUndefinedKeys(summary); + output[exportKey].push(dropUndefinedKeys(summary)); } return output; From 639406149aa443d4b6afa35229271a675c585cae Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 8 Feb 2024 13:36:48 +0100 Subject: [PATCH 04/10] Do as Lukas told you --- packages/core/src/metrics/metric-summary.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/metrics/metric-summary.ts b/packages/core/src/metrics/metric-summary.ts index 947152d4d5d4..1230b3f27217 100644 --- a/packages/core/src/metrics/metric-summary.ts +++ b/packages/core/src/metrics/metric-summary.ts @@ -29,7 +29,7 @@ export function getMetricSummaryJsonForSpan(span: Span): Record> = {}; for (const [, [exportKey, summary]] of storage) { - output[exportKey].push(dropUndefinedKeys(summary)); + output[exportKey] = [...(output[exportKey] || []), dropUndefinedKeys(summary)]; } return output; From f231c8a20c1a6493f48557965a19b884724eb3b2 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 8 Feb 2024 13:42:35 +0100 Subject: [PATCH 05/10] CS --- .../suites/tracing/metric-summaries/test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 99d9b762b537..9a5f9c2826f3 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 @@ -43,7 +43,7 @@ const EXPECTED_TRANSACTION = { release: '1.0', transaction: 'Test Transaction', }, - } + }, ], 'g:root-gauge@none': [ { @@ -56,7 +56,7 @@ const EXPECTED_TRANSACTION = { transaction: 'Test Transaction', }, }, - ] + ], 'd:root-distribution@none': [ { min: 20, From dff3a8fe8c450379fcadc25fb6bd7d0997ff14e4 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 8 Feb 2024 14:04:54 +0100 Subject: [PATCH 06/10] Allow whitespace in tag values and remove tag value replacement character --- packages/core/src/metrics/constants.ts | 2 +- packages/core/src/metrics/utils.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 e89e0fd1562b..a5f3a87f57d5 100644 --- a/packages/core/src/metrics/constants.ts +++ b/packages/core/src/metrics/constants.ts @@ -21,7 +21,7 @@ export const NAME_AND_TAG_KEY_NORMALIZATION_REGEX = /[^a-zA-Z0-9_/.-]+/g; * * See: https://develop.sentry.dev/sdk/metrics/#normalization */ -export const TAG_VALUE_NORMALIZATION_REGEX = /[^\w\d_:/@.{}[\]$-]+/g; +export const TAG_VALUE_NORMALIZATION_REGEX = /[^\w\d\s_:/@.{}[\]$-]+/g; /** * This does not match spec in https://develop.sentry.dev/sdk/metrics diff --git a/packages/core/src/metrics/utils.ts b/packages/core/src/metrics/utils.ts index a6674bcf30e1..7b1cf96a8462 100644 --- a/packages/core/src/metrics/utils.ts +++ b/packages/core/src/metrics/utils.ts @@ -62,7 +62,7 @@ export function sanitizeTags(unsanitizedTags: Record): Record for (const key in unsanitizedTags) { if (Object.prototype.hasOwnProperty.call(unsanitizedTags, key)) { const sanitizedKey = key.replace(NAME_AND_TAG_KEY_NORMALIZATION_REGEX, '_'); - tags[sanitizedKey] = String(unsanitizedTags[key]).replace(TAG_VALUE_NORMALIZATION_REGEX, '_'); + tags[sanitizedKey] = String(unsanitizedTags[key]).replace(TAG_VALUE_NORMALIZATION_REGEX, ''); } } return tags; From 858b9bd0792c8d92951d33609a1a8df96f28aaa2 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 8 Feb 2024 14:49:00 +0100 Subject: [PATCH 07/10] Change some test --- .../suites/tracing/metric-summaries/scenario.js | 12 ++++++++++-- .../suites/tracing/metric-summaries/test.ts | 16 ++++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) 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 8a7dbabe0dec..2b4e26970c0f 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,8 +20,16 @@ Sentry.startSpan( op: 'transaction', }, () => { - Sentry.metrics.increment('root-counter'); - Sentry.metrics.increment('root-counter'); + Sentry.metrics.increment('root-counter', 1, { + tags: { + email: 'jon.doe@example.com', + } + }); + Sentry.metrics.increment('root-counter', 1, { + tags: { + email: 'jane.doe@example.com', + } + }); Sentry.startSpan( { 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 9a5f9c2826f3..94f5fdc30c70 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 @@ -7,11 +7,23 @@ const EXPECTED_TRANSACTION = { { min: 1, max: 1, - count: 2, - sum: 2, + count: 1, + sum: 1, tags: { release: '1.0', transaction: 'Test Transaction', + email: 'jon.doe@example.com', + }, + }, + { + min: 1, + max: 1, + count: 1, + sum: 1, + tags: { + release: '1.0', + transaction: 'Test Transaction', + email: 'jane.doe@example.com', }, }, ], From 9fe0359c5394b02c9b8ddd3c2a093e3cf570348e Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 8 Feb 2024 15:15:10 +0100 Subject: [PATCH 08/10] CS --- .../suites/tracing/metric-summaries/scenario.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 2b4e26970c0f..ef68afb06576 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 @@ -23,12 +23,12 @@ Sentry.startSpan( Sentry.metrics.increment('root-counter', 1, { tags: { email: 'jon.doe@example.com', - } + }, }); Sentry.metrics.increment('root-counter', 1, { tags: { email: 'jane.doe@example.com', - } + }, }); Sentry.startSpan( From 34c62198b73da08cf92513daab53b528889256be Mon Sep 17 00:00:00 2001 From: Michi Hoffmann Date: Thu, 8 Feb 2024 16:10:31 +0100 Subject: [PATCH 09/10] Update packages/core/src/metrics/metric-summary.ts Co-authored-by: Abhijeet Prasad --- packages/core/src/metrics/metric-summary.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/core/src/metrics/metric-summary.ts b/packages/core/src/metrics/metric-summary.ts index 1230b3f27217..ede2330bffcf 100644 --- a/packages/core/src/metrics/metric-summary.ts +++ b/packages/core/src/metrics/metric-summary.ts @@ -29,7 +29,11 @@ export function getMetricSummaryJsonForSpan(span: Span): Record> = {}; for (const [, [exportKey, summary]] of storage) { - output[exportKey] = [...(output[exportKey] || []), dropUndefinedKeys(summary)]; + if (!output[exportKey]) { + output[exportKey] = []; + } + + output[exportKey].push(dropUndefinedKeys(summary)); } return output; From 1b32bc54082746853c95c07678145cc9c4b9b540 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 12 Feb 2024 10:01:32 -0500 Subject: [PATCH 10/10] formatting fix for metric-summary --- packages/core/src/metrics/metric-summary.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/metrics/metric-summary.ts b/packages/core/src/metrics/metric-summary.ts index ede2330bffcf..bf2e828dae1b 100644 --- a/packages/core/src/metrics/metric-summary.ts +++ b/packages/core/src/metrics/metric-summary.ts @@ -32,7 +32,7 @@ export function getMetricSummaryJsonForSpan(span: Span): Record