Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): Update metric normalization (v7) #11519

Merged
merged 1 commit into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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);
});
});