Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
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
134 changes: 100 additions & 34 deletions packages/core/src/metrics/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,97 @@ 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) {
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;
}

/**
* Checks if a metric should be sampled based on the sample rate and scope's sampleRand (random based on trace).
*
* @param metric - The metric containing the sample_rate.
* @param scope - The scope containing the sampleRand.
* @returns true if the metric should be sampled, false if it should be dropped.
*/
function shouldSampleMetric(metric: Metric, scope: Scope): boolean {
if (metric.sample_rate === undefined) {
return true;
}
const sampleRand = scope.getPropagationContext().sampleRand;
return sampleRand < metric.sample_rate;
}

/**
* 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);
}
}

/**
* Processes and enriches metric attributes with context data.
*
* @param beforeMetric - The original metric.
* @param currentScope - The current scope.
* @param client - The client.
* @returns The processed metric attributes.
*/
function processMetricAttributes(beforeMetric: Metric, currentScope: Scope, client: Client): Record<string, unknown> {
const processedMetricAttributes = {
...beforeMetric.attributes,
};

addSampleRateAttribute(beforeMetric, processedMetricAttributes);

const {
user: { id, email, username },
} = getMergedScopeData(currentScope);
setMetricAttribute(processedMetricAttributes, 'user.id', id, false);
setMetricAttribute(processedMetricAttributes, 'user.email', email, false);
setMetricAttribute(processedMetricAttributes, 'user.name', username, false);

const { release, environment } = client.getOptions();
setMetricAttribute(processedMetricAttributes, 'sentry.release', release);
setMetricAttribute(processedMetricAttributes, 'sentry.environment', environment);

const { name, version } = client.getSdkMetadata()?.sdk ?? {};
setMetricAttribute(processedMetricAttributes, 'sentry.sdk.name', name);
setMetricAttribute(processedMetricAttributes, 'sentry.sdk.version', version);

const replay = client.getIntegrationByName<
Integration & {
getReplayId: (onlyIfSampled?: boolean) => string;
getRecordingMode: () => 'session' | 'buffer' | undefined;
}
>('Replay');

const replayId = replay?.getReplayId(true);
setMetricAttribute(processedMetricAttributes, 'sentry.replay_id', replayId);

if (replayId && replay?.getRecordingMode() === 'buffer') {
setMetricAttribute(processedMetricAttributes, 'sentry._internal.replay_is_buffering', true);
}

return processedMetricAttributes;
}

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

const { release, environment, _experiments } = client.getOptions();
const { _experiments } = client.getOptions();
if (!_experiments?.enableMetrics) {
DEBUG_BUILD && debug.warn('metrics option not enabled, metric will not be captured.');
return;
}

const [, traceContext] = _getTraceInfoFromScope(client, currentScope);

const processedMetricAttributes = {
...beforeMetric.attributes,
};

const {
user: { id, email, username },
} = getMergedScopeData(currentScope);
setMetricAttribute(processedMetricAttributes, 'user.id', id, false);
setMetricAttribute(processedMetricAttributes, 'user.email', email, false);
setMetricAttribute(processedMetricAttributes, 'user.name', username, false);

setMetricAttribute(processedMetricAttributes, 'sentry.release', release);
setMetricAttribute(processedMetricAttributes, 'sentry.environment', environment);

const { name, version } = client.getSdkMetadata()?.sdk ?? {};
setMetricAttribute(processedMetricAttributes, 'sentry.sdk.name', name);
setMetricAttribute(processedMetricAttributes, 'sentry.sdk.version', version);

const replay = client.getIntegrationByName<
Integration & {
getReplayId: (onlyIfSampled?: boolean) => string;
getRecordingMode: () => 'session' | 'buffer' | undefined;
}
>('Replay');

const replayId = replay?.getReplayId(true);

setMetricAttribute(processedMetricAttributes, 'sentry.replay_id', replayId);
if (!validateAndProcessSampleRate(beforeMetric, client)) {
return;
}

if (replayId && replay?.getRecordingMode() === 'buffer') {
// We send this so we can identify cases where the replayId is attached but the replay itself might not have been sent to Sentry
setMetricAttribute(processedMetricAttributes, 'sentry._internal.replay_is_buffering', true);
if (!shouldSampleMetric(beforeMetric, currentScope)) {
return;
Copy link

Choose a reason for hiding this comment

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

Bug: Missed Tracking of Sampled-Out Metrics Drops

When a metric is dropped due to sampling (when sampled is false), the code does not record this as a dropped event using client.recordDroppedEvent('sample_rate', 'metric'). The EventDropReason type already includes 'sample_rate' as a valid drop reason, and the code properly records drops for invalid sample rates at line 90, but fails to record drops for valid sampling decisions. This means sampled-out metrics won't be tracked in client reports, making it impossible to monitor how many metrics are being dropped due to sampling.

Fix in Cursor Fix in Web

}

const [, traceContext] = _getTraceInfoFromScope(client, currentScope);
const processedMetricAttributes = processMetricAttributes(beforeMetric, currentScope, client);

const metric: Metric = {
...beforeMetric,
attributes: processedMetricAttributes,
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
Loading
Loading