Skip to content

Commit

Permalink
feat(core): Update metric normalization (getsentry#11518)
Browse files Browse the repository at this point in the history
  • Loading branch information
timfish authored and cadesalaberry committed Apr 19, 2024
1 parent 6c8dd8b commit 44ff826
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 34 deletions.
10 changes: 6 additions & 4 deletions packages/core/src/metrics/aggregator.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);

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

Expand Down Expand Up @@ -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();
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
62 changes: 59 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,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;
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 44ff826

Please sign in to comment.