Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions packages/core/src/metrics/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,36 @@ function setMetricAttribute(
}
}

/**
* Validates and processes the sample_rate for a metric.
*
* @param metric - The metric containing the sample_rate to validate.
* @param client - The client to record dropped events with.
* @returns true if the sample_rate is valid, false if the metric should be dropped.
*/
function validateAndProcessSampleRate(metric: Metric, client: Client): boolean {
if (metric.sample_rate !== undefined) {
if (metric.sample_rate <= 0 || metric.sample_rate > 1.0) {
// Invalid sample rate - drop the metric entirely and record the lost event
client.recordDroppedEvent('invalid_sample_rate', 'metric');
return false;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Invalid NaN Rejection in Sample Rate Validation

The sample_rate validation doesn't properly reject NaN values. When sample_rate is NaN, the condition metric.sample_rate <= 0 || metric.sample_rate > 1.0 evaluates to false (since all comparisons with NaN return false), allowing NaN to pass validation. This causes the metric to be silently dropped during sampling (since sampleRand < NaN is always false) without recording a dropped event. The validation should explicitly check for NaN and reject it as invalid.

Fix in Cursor Fix in Web

}
return true;
}

/**
* Adds the sample_rate attribute to the metric attributes if needed.
*
* @param metric - The metric containing the sample_rate.
* @param attributes - The attributes object to modify.
*/
function addSampleRateAttribute(metric: Metric, attributes: Record<string, unknown>): void {
if (metric.sample_rate !== undefined && metric.sample_rate !== 1.0) {
setMetricAttribute(attributes, 'sentry.client_sample_rate', metric.sample_rate);
}
}

/**
* Captures a serialized metric event and adds it to the metric buffer for the given client.
*
Expand Down Expand Up @@ -133,6 +163,10 @@ export function _INTERNAL_captureMetric(beforeMetric: Metric, options?: Internal
return;
}

if (!validateAndProcessSampleRate(beforeMetric, client)) {
return;
}

const { release, environment, _experiments } = client.getOptions();
if (!_experiments?.enableMetrics) {
DEBUG_BUILD && debug.warn('metrics option not enabled, metric will not be captured.');
Expand All @@ -145,6 +179,9 @@ export function _INTERNAL_captureMetric(beforeMetric: Metric, options?: Internal
...beforeMetric.attributes,
};


addSampleRateAttribute(beforeMetric, processedMetricAttributes);

const {
user: { id, email, username },
} = getMergedScopeData(currentScope);
Expand Down
25 changes: 18 additions & 7 deletions packages/core/src/metrics/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ export interface MetricOptions {
* The scope to capture the metric with.
*/
scope?: Scope;

/**
* The sample rate for the metric. Must be a float between 0 (exclusive) and 1 (inclusive).
*/
sample_rate?: number;
}

/**
Expand All @@ -32,7 +37,7 @@ export interface MetricOptions {
*/
function captureMetric(type: MetricType, name: string, value: number | string, options?: MetricOptions): void {
_INTERNAL_captureMetric(
{ type, name, value, unit: options?.unit, attributes: options?.attributes },
{ type, name, value, unit: options?.unit, attributes: options?.attributes, sample_rate: options?.sample_rate },
{ scope: options?.scope },
);
}
Expand All @@ -43,6 +48,7 @@ function captureMetric(type: MetricType, name: string, value: number | string, o
* @param name - The name of the counter metric.
* @param value - The value to increment by (defaults to 1).
* @param options - Options for capturing the metric.
* @param options.sample_rate - Sample rate for the metric (0 < sample_rate <= 1.0).
*
* @example
*
Expand All @@ -56,14 +62,15 @@ function captureMetric(type: MetricType, name: string, value: number | string, o
* });
* ```
*
* @example With custom value
* @example With custom value and sample rate
*
* ```
* Sentry.metrics.count('items.processed', 5, {
* attributes: {
* processor: 'batch-processor',
* queue: 'high-priority'
* }
* },
* sample_rate: 0.1
* });
* ```
*/
Expand All @@ -77,6 +84,7 @@ export function count(name: string, value: number = 1, options?: MetricOptions):
* @param name - The name of the gauge metric.
* @param value - The current value of the gauge.
* @param options - Options for capturing the metric.
* @param options.sample_rate - Sample rate for the metric (0 < sample_rate <= 1.0).
*
* @example
*
Expand All @@ -90,14 +98,15 @@ export function count(name: string, value: number = 1, options?: MetricOptions):
* });
* ```
*
* @example Without unit
* @example With sample rate
*
* ```
* Sentry.metrics.gauge('active.connections', 42, {
* attributes: {
* server: 'api-1',
* protocol: 'websocket'
* }
* },
* sample_rate: 0.5
* });
* ```
*/
Expand All @@ -111,6 +120,7 @@ export function gauge(name: string, value: number, options?: MetricOptions): voi
* @param name - The name of the distribution metric.
* @param value - The value to record in the distribution.
* @param options - Options for capturing the metric.
* @param options.sample_rate - Sample rate for the metric (0 < sample_rate <= 1.0).
*
* @example
*
Expand All @@ -124,14 +134,15 @@ export function gauge(name: string, value: number, options?: MetricOptions): voi
* });
* ```
*
* @example Without unit
* @example With sample rate
*
* ```
* Sentry.metrics.distribution('batch.size', 100, {
* attributes: {
* processor: 'batch-1',
* type: 'async'
* }
* },
* sample_rate: 0.25
* });
* ```
*/
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/types-hoist/clientreport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ export type EventDropReason =
| 'sample_rate'
| 'send_error'
| 'internal_sdk_error'
| 'buffer_overflow';
| 'buffer_overflow'
| 'invalid_sample_rate';

export type Outcome = {
reason: EventDropReason;
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/types-hoist/metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ export interface Metric {
* Arbitrary structured data that stores information about the metric.
*/
attributes?: Record<string, unknown>;

/**
* The sample rate for the metric. Must be a float between 0 (exclusive) and 1 (inclusive).
*/
sample_rate?: number;
}

export type SerializedMetricAttributeValue =
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils/lru.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class LRUMap<K, V> {
if (this._cache.size >= this._maxSize) {
// keys() returns an iterator in insertion order so keys().next() gives us the oldest key
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const nextKey = this._cache.keys().next().value!;
const nextKey = this._cache.keys().next().value;
this._cache.delete(nextKey);
}
this._cache.set(key, value);
Expand Down
175 changes: 174 additions & 1 deletion packages/core/test/lib/metrics/public-api.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, expect, it } from 'vitest';
import { describe, expect, it, vi } from 'vitest';
import { Scope } from '../../../src';
import { _INTERNAL_getMetricBuffer } from '../../../src/metrics/internal';
import { count, distribution, gauge } from '../../../src/metrics/public-api';
Expand Down Expand Up @@ -119,6 +119,123 @@ describe('Metrics Public API', () => {

expect(_INTERNAL_getMetricBuffer(client)).toBeUndefined();
});

it('captures a counter metric with sample_rate', () => {
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableMetrics: true } });
const client = new TestClient(options);
const scope = new Scope();
scope.setClient(client);

count('api.requests', 1, {
scope,
sample_rate: 0.5,
});

const metricBuffer = _INTERNAL_getMetricBuffer(client);
expect(metricBuffer).toHaveLength(1);
expect(metricBuffer?.[0]).toEqual(
expect.objectContaining({
name: 'api.requests',
type: 'counter',
value: 1,
attributes: {
'sentry.client_sample_rate': {
value: 0.5,
type: 'double',
},
},
}),
);
});

it('captures a counter metric with sample_rate and existing attributes', () => {
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableMetrics: true } });
const client = new TestClient(options);
const scope = new Scope();
scope.setClient(client);

count('api.requests', 1, {
scope,
sample_rate: 0.25,
attributes: {
endpoint: '/api/users',
method: 'GET',
},
});

const metricBuffer = _INTERNAL_getMetricBuffer(client);
expect(metricBuffer).toHaveLength(1);
expect(metricBuffer?.[0]).toEqual(
expect.objectContaining({
name: 'api.requests',
type: 'counter',
value: 1,
attributes: {
endpoint: {
value: '/api/users',
type: 'string',
},
method: {
value: 'GET',
type: 'string',
},
'sentry.client_sample_rate': {
value: 0.25,
type: 'double',
},
},
}),
);
});

it('drops metrics with sample_rate above 1', () => {
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableMetrics: true } });
const client = new TestClient(options);
const scope = new Scope();
scope.setClient(client);

count('api.requests', 1, {
scope,
sample_rate: 1.5,
});

const metricBuffer = _INTERNAL_getMetricBuffer(client);
expect(metricBuffer).toBeUndefined();
});

it('drops metrics with sample_rate at or below 0', () => {
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableMetrics: true } });
const client = new TestClient(options);
const scope = new Scope();
scope.setClient(client);

count('api.requests', 1, {
scope,
sample_rate: 0,
});

const metricBuffer = _INTERNAL_getMetricBuffer(client);
expect(metricBuffer).toBeUndefined();
});

it('records dropped event for invalid sample_rate values', () => {
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableMetrics: true } });
const client = new TestClient(options);
const scope = new Scope();
scope.setClient(client);

const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent');

count('api.requests', 1, { scope, sample_rate: 0 });
count('api.requests', 1, { scope, sample_rate: -0.5 });
count('api.requests', 1, { scope, sample_rate: 1.5 });

expect(recordDroppedEventSpy).toHaveBeenCalledTimes(3);
expect(recordDroppedEventSpy).toHaveBeenCalledWith('invalid_sample_rate', 'metric');

const metricBuffer = _INTERNAL_getMetricBuffer(client);
expect(metricBuffer).toBeUndefined();
});
});

describe('gauge', () => {
Expand Down Expand Up @@ -209,6 +326,34 @@ describe('Metrics Public API', () => {

expect(_INTERNAL_getMetricBuffer(client)).toBeUndefined();
});

it('captures a gauge metric with sample_rate', () => {
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableMetrics: true } });
const client = new TestClient(options);
const scope = new Scope();
scope.setClient(client);

gauge('memory.usage', 1024, {
scope,
sample_rate: 0.75,
});

const metricBuffer = _INTERNAL_getMetricBuffer(client);
expect(metricBuffer).toHaveLength(1);
expect(metricBuffer?.[0]).toEqual(
expect.objectContaining({
name: 'memory.usage',
type: 'gauge',
value: 1024,
attributes: {
'sentry.client_sample_rate': {
value: 0.75,
type: 'double',
},
},
}),
);
});
});

describe('distribution', () => {
Expand Down Expand Up @@ -299,6 +444,34 @@ describe('Metrics Public API', () => {

expect(_INTERNAL_getMetricBuffer(client)).toBeUndefined();
});

it('captures a distribution metric with sample_rate', () => {
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableMetrics: true } });
const client = new TestClient(options);
const scope = new Scope();
scope.setClient(client);

distribution('task.duration', 500, {
scope,
sample_rate: 0.1,
});

const metricBuffer = _INTERNAL_getMetricBuffer(client);
expect(metricBuffer).toHaveLength(1);
expect(metricBuffer?.[0]).toEqual(
expect.objectContaining({
name: 'task.duration',
type: 'distribution',
value: 500,
attributes: {
'sentry.client_sample_rate': {
value: 0.1,
type: 'double',
},
},
}),
);
});
});

describe('mixed metric types', () => {
Expand Down
Loading