Skip to content

Commit

Permalink
feat(core): Update metric normalization (v7) (#11519)
Browse files Browse the repository at this point in the history
Backport of #11518 for v7
  • Loading branch information
timfish committed Apr 10, 2024
1 parent 5c35031 commit 1c0194a
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 32 deletions.
10 changes: 6 additions & 4 deletions packages/core/src/metrics/aggregator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
Expand All @@ -62,13 +63,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);

Expand Down
11 changes: 7 additions & 4 deletions packages/core/src/metrics/browser-aggregator.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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<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);

Expand Down Expand Up @@ -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();
}

Expand Down
20 changes: 0 additions & 20 deletions packages/core/src/metrics/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
42 changes: 39 additions & 3 deletions packages/core/src/metrics/utils.ts
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand Down Expand Up @@ -54,15 +53,52 @@ 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.
*/
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;
Expand Down
24 changes: 23 additions & 1 deletion packages/core/test/lib/metrics/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand All @@ -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);
});
});

0 comments on commit 1c0194a

Please sign in to comment.