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..bc1ff93e0002 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,63 @@ export function serializeMetricBuckets(metricBucketItems: MetricBucketItem[]): s return out; } +/** + * 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 + * + * 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'], + ['\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 +117,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); + }); });