feat(core): Update metric normalization

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<string, Primitive> = {},
     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);
 

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<string, Primitive> | 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();

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.

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,15 +53,72 @@ 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.
  */
 export function sanitizeTags(unsanitizedTags: Record<string, Primitive>): Record<string, string> {
   const tags: Record<string, string> = {};
   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;

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);
+  });
 });