From bc4e1db042fe3912da99e24865242ec3b2304d90 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Apr 2024 23:25:48 +0200 Subject: [PATCH] feat(core): Update metric normalization (v7) --- packages/core/src/metrics/aggregator.ts | 10 +++-- .../core/src/metrics/browser-aggregator.ts | 11 +++-- packages/core/src/metrics/constants.ts | 20 --------- packages/core/src/metrics/utils.ts | 42 +++++++++++++++++-- packages/core/test/lib/metrics/utils.test.ts | 24 ++++++++++- 5 files changed, 75 insertions(+), 32 deletions(-) diff --git a/packages/core/src/metrics/aggregator.ts b/packages/core/src/metrics/aggregator.ts index 4bf66f099887..746f3a33ce31 100644 --- a/packages/core/src/metrics/aggregator.ts +++ b/packages/core/src/metrics/aggregator.ts @@ -6,11 +6,11 @@ import type { Primitive, } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; -import { DEFAULT_FLUSH_INTERVAL, MAX_WEIGHT, NAME_AND_TAG_KEY_NORMALIZATION_REGEX, SET_METRIC_TYPE } from './constants'; +import { DEFAULT_FLUSH_INTERVAL, MAX_WEIGHT, 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'; +import { getBucketKey, sanitizeMetricKey, sanitizeTags, sanitizeUnit } from './utils'; /** * A metrics aggregator that aggregates metrics in memory and flushes them periodically. @@ -51,6 +51,7 @@ export class MetricsAggregator implements MetricsAggregatorBase { // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access this._interval.unref(); } + this._flushShift = Math.floor((Math.random() * DEFAULT_FLUSH_INTERVAL) / 1000); this._forceFlush = false; } @@ -62,13 +63,14 @@ export class MetricsAggregator implements MetricsAggregatorBase { metricType: MetricType, unsanitizedName: string, value: number | string, - unit: MeasurementUnit = 'none', + unsanitizedUnit: MeasurementUnit = 'none', unsanitizedTags: Record = {}, maybeFloatTimestamp = timestampInSeconds(), ): void { const timestamp = Math.floor(maybeFloatTimestamp); - const name = unsanitizedName.replace(NAME_AND_TAG_KEY_NORMALIZATION_REGEX, '_'); + const name = sanitizeMetricKey(unsanitizedName); const tags = sanitizeTags(unsanitizedTags); + const unit = sanitizeUnit(unsanitizedUnit as string); const bucketKey = getBucketKey(metricType, name, unit, tags); diff --git a/packages/core/src/metrics/browser-aggregator.ts b/packages/core/src/metrics/browser-aggregator.ts index 40cfa1d404ab..89202746572f 100644 --- a/packages/core/src/metrics/browser-aggregator.ts +++ b/packages/core/src/metrics/browser-aggregator.ts @@ -1,10 +1,10 @@ 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, SET_METRIC_TYPE } from './constants'; +import { DEFAULT_BROWSER_FLUSH_INTERVAL, 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'; +import { getBucketKey, sanitizeMetricKey, sanitizeTags, sanitizeUnit } from './utils'; /** * A simple metrics aggregator that aggregates metrics in memory and flushes them periodically. @@ -31,13 +31,14 @@ export class BrowserMetricsAggregator implements MetricsAggregator { metricType: MetricType, unsanitizedName: string, value: number | string, - unit: MeasurementUnit | undefined = 'none', + unsanitizedUnit: MeasurementUnit | undefined = 'none', unsanitizedTags: Record | undefined = {}, maybeFloatTimestamp: number | undefined = timestampInSeconds(), ): void { const timestamp = Math.floor(maybeFloatTimestamp); - const name = unsanitizedName.replace(NAME_AND_TAG_KEY_NORMALIZATION_REGEX, '_'); + const name = sanitizeMetricKey(unsanitizedName); const tags = sanitizeTags(unsanitizedTags); + const unit = sanitizeUnit(unsanitizedUnit as string); const bucketKey = getBucketKey(metricType, name, unit, tags); @@ -77,11 +78,13 @@ export class BrowserMetricsAggregator implements MetricsAggregator { if (this._buckets.size === 0) { return; } + if (this._client.captureAggregateMetrics) { // TODO(@anonrig): Use Object.values() when we support ES6+ const metricBuckets = Array.from(this._buckets).map(([, bucketItem]) => bucketItem); this._client.captureAggregateMetrics(metricBuckets); } + this._buckets.clear(); } diff --git a/packages/core/src/metrics/constants.ts b/packages/core/src/metrics/constants.ts index a5f3a87f57d5..ae1cd968723c 100644 --- a/packages/core/src/metrics/constants.ts +++ b/packages/core/src/metrics/constants.ts @@ -3,26 +3,6 @@ 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. - * - * 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 values. - * - * 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\s_:/@.{}[\]$-]+/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. diff --git a/packages/core/src/metrics/utils.ts b/packages/core/src/metrics/utils.ts index 7b1cf96a8462..f45313c3b068 100644 --- a/packages/core/src/metrics/utils.ts +++ b/packages/core/src/metrics/utils.ts @@ -1,6 +1,5 @@ import type { MeasurementUnit, MetricBucketItem, Primitive } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; -import { NAME_AND_TAG_KEY_NORMALIZATION_REGEX, TAG_VALUE_NORMALIZATION_REGEX } from './constants'; import type { MetricType } from './types'; /** @@ -54,6 +53,43 @@ export function serializeMetricBuckets(metricBucketItems: MetricBucketItem[]): s return out; } +/** Sanitizes units */ +export function sanitizeUnit(unit: string): string { + return unit.replace(/[^\w]+/gi, '_'); +} + +/** Sanitizes metric keys */ +export function sanitizeMetricKey(key: string): string { + return key.replace(/[^\w\-.]+/gi, '_'); +} + +function sanitizeTagKey(key: string): string { + return key.replace(/[^\w\-./]+/gi, ''); +} + +const tagValueReplacements: [string, string][] = [ + ['\n', '\\n'], + ['\r', '\\r'], + ['\t', '\\t'], + ['\\', '\\\\'], + ['|', '\\u{7c}'], + [',', '\\u{2c}'], +]; + +function getCharOrReplacement(input: string): string { + for (const [search, replacement] of tagValueReplacements) { + if (input === search) { + return replacement; + } + } + + return input; +} + +function sanitizeTagValue(value: string): string { + return [...value].reduce((acc, char) => acc + getCharOrReplacement(char), ''); +} + /** * Sanitizes tags. */ @@ -61,8 +97,8 @@ export 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, '_'); - tags[sanitizedKey] = String(unsanitizedTags[key]).replace(TAG_VALUE_NORMALIZATION_REGEX, ''); + const sanitizedKey = sanitizeTagKey(key); + tags[sanitizedKey] = sanitizeTagValue(String(unsanitizedTags[key])); } } return tags; diff --git a/packages/core/test/lib/metrics/utils.test.ts b/packages/core/test/lib/metrics/utils.test.ts index fe96404b72ea..e25014715748 100644 --- a/packages/core/test/lib/metrics/utils.test.ts +++ b/packages/core/test/lib/metrics/utils.test.ts @@ -4,7 +4,7 @@ import { GAUGE_METRIC_TYPE, SET_METRIC_TYPE, } from '../../../src/metrics/constants'; -import { getBucketKey } from '../../../src/metrics/utils'; +import { getBucketKey, sanitizeTags } from '../../../src/metrics/utils'; describe('getBucketKey', () => { it.each([ @@ -18,4 +18,26 @@ describe('getBucketKey', () => { ])('should return', (metricType, name, unit, tags, expected) => { expect(getBucketKey(metricType, name, unit, tags)).toEqual(expected); }); + + it('should sanitize tags', () => { + const inputTags = { + 'f-oo|bar': '%$foo/', + 'foo$.$.$bar': 'blah{}', + 'foö-bar': 'snöwmän', + route: 'GET /foo', + __bar__: 'this | or , that', + 'foo/': 'hello!\n\r\t\\', + }; + + const outputTags = { + 'f-oobar': '%$foo/', + 'foo..bar': 'blah{}', + 'fo-bar': 'snöwmän', + route: 'GET /foo', + __bar__: 'this \\u{7c} or \\u{2c} that', + 'foo/': 'hello!\\n\\r\\t\\\\', + }; + + expect(sanitizeTags(inputTags)).toEqual(outputTags); + }); });