From 5ed59368a58d97c6e629e0248a2747599088ac55 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Apr 2024 23:14:21 +0200 Subject: [PATCH 1/3] feat(core): Metric normalization --- packages/core/src/metrics/aggregator.ts | 10 +++-- .../core/src/metrics/browser-aggregator.ts | 12 +++--- 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, 74 insertions(+), 34 deletions(-) diff --git a/packages/core/src/metrics/aggregator.ts b/packages/core/src/metrics/aggregator.ts index 8b56d190b88a..8752d2a10df7 100644 --- a/packages/core/src/metrics/aggregator.ts +++ b/packages/core/src/metrics/aggregator.ts @@ -1,11 +1,11 @@ import type { Client, MeasurementUnit, MetricsAggregator as MetricsAggregatorBase, Primitive } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; import { updateMetricSummaryOnActiveSpan } from '../utils/spanUtils'; -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 { captureAggregateMetrics } from './envelope'; import { METRIC_MAP } from './instance'; 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. @@ -46,6 +46,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; } @@ -57,13 +58,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 7d599f5aeba8..e087b45ca010 100644 --- a/packages/core/src/metrics/browser-aggregator.ts +++ b/packages/core/src/metrics/browser-aggregator.ts @@ -1,11 +1,11 @@ import type { Client, MeasurementUnit, MetricsAggregator, Primitive } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; import { updateMetricSummaryOnActiveSpan } from '../utils/spanUtils'; -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 { captureAggregateMetrics } from './envelope'; import { METRIC_MAP } from './instance'; 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. @@ -32,13 +32,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); @@ -79,8 +80,7 @@ export class BrowserMetricsAggregator implements MetricsAggregator { return; } - // TODO(@anonrig): Use Object.values() when we support ES6+ - const metricBuckets = Array.from(this._buckets).map(([, bucketItem]) => bucketItem); + const metricBuckets = Array.from(this._buckets.values()); captureAggregateMetrics(this._client, 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..4eed3e4df523 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); + }) }); From 141575bba215eee20282c89c2cf9238708bdc361 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 9 Apr 2024 23:26:10 +0200 Subject: [PATCH 2/3] Fix lint --- packages/core/test/lib/metrics/utils.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/core/test/lib/metrics/utils.test.ts b/packages/core/test/lib/metrics/utils.test.ts index 4eed3e4df523..e25014715748 100644 --- a/packages/core/test/lib/metrics/utils.test.ts +++ b/packages/core/test/lib/metrics/utils.test.ts @@ -24,8 +24,8 @@ describe('getBucketKey', () => { 'f-oo|bar': '%$foo/', 'foo$.$.$bar': 'blah{}', 'foö-bar': 'snöwmän', - 'route': 'GET /foo', - '__bar__': 'this | or , that', + route: 'GET /foo', + __bar__: 'this | or , that', 'foo/': 'hello!\n\r\t\\', }; @@ -33,11 +33,11 @@ describe('getBucketKey', () => { 'f-oobar': '%$foo/', 'foo..bar': 'blah{}', 'fo-bar': 'snöwmän', - 'route': 'GET /foo', - '__bar__': 'this \\u{7c} or \\u{2c} that', + route: 'GET /foo', + __bar__: 'this \\u{7c} or \\u{2c} that', 'foo/': 'hello!\\n\\r\\t\\\\', }; expect(sanitizeTags(inputTags)).toEqual(outputTags); - }) + }); }); From 2d87c8f3fc1e6b5b4d57cf38fb3e4155c2f5326c Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Wed, 10 Apr 2024 17:16:00 +0200 Subject: [PATCH 3/3] Add links to docs --- packages/core/src/metrics/utils.ts | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/packages/core/src/metrics/utils.ts b/packages/core/src/metrics/utils.ts index f45313c3b068..bc1ff93e0002 100644 --- a/packages/core/src/metrics/utils.ts +++ b/packages/core/src/metrics/utils.ts @@ -53,20 +53,40 @@ export function serializeMetricBuckets(metricBucketItems: MetricBucketItem[]): s return out; } -/** Sanitizes units */ +/** + * Sanitizes units + * + * These Regex's are straight from the normalisation docs: + * https://develop.sentry.dev/sdk/metrics/#normalization + */ export function sanitizeUnit(unit: string): string { return unit.replace(/[^\w]+/gi, '_'); } -/** Sanitizes metric keys */ +/** + * Sanitizes metric keys + * + * These Regex's are straight from the normalisation docs: + * https://develop.sentry.dev/sdk/metrics/#normalization + */ export function sanitizeMetricKey(key: string): string { return key.replace(/[^\w\-.]+/gi, '_'); } +/** + * Sanitizes metric keys + * + * These Regex's are straight from the normalisation docs: + * https://develop.sentry.dev/sdk/metrics/#normalization + */ function sanitizeTagKey(key: string): string { return key.replace(/[^\w\-./]+/gi, ''); } +/** + * These Regex's are straight from the normalisation docs: + * https://develop.sentry.dev/sdk/metrics/#normalization + */ const tagValueReplacements: [string, string][] = [ ['\n', '\\n'], ['\r', '\\r'],