-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(tracemetrics): Add sample_rate parameter to metric API calls #18103
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
base: develop
Are you sure you want to change the base?
Changes from all commits
cec2081
1d11dca
d219b74
f86899a
a08460d
77ce587
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,6 +77,105 @@ 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; | ||
| } | ||
| } | ||
| 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 An object with sampled (boolean) and samplingPerformed (boolean) flags. | ||
| */ | ||
| function shouldSampleMetric(metric: Metric, scope: Scope): { sampled: boolean; samplingPerformed: boolean } { | ||
| if (metric.sample_rate === undefined) { | ||
| return { sampled: true, samplingPerformed: false }; | ||
| } | ||
|
|
||
| const propagationContext = scope.getPropagationContext(); | ||
| if (!propagationContext || propagationContext.sampleRand === undefined) { | ||
| return { sampled: true, samplingPerformed: false }; | ||
| } | ||
|
|
||
| const sampleRand = propagationContext.sampleRand; | ||
| return { sampled: sampleRand < metric.sample_rate, samplingPerformed: true }; | ||
| } | ||
|
|
||
| /** | ||
| * Adds the sample_rate attribute to the metric attributes if sampling was actually performed. | ||
| * | ||
| * @param metric - The metric containing the sample_rate. | ||
| * @param attributes - The attributes object to modify. | ||
| * @param samplingPerformed - Whether sampling was actually performed. | ||
| */ | ||
| function addSampleRateAttribute(metric: Metric, attributes: Record<string, unknown>, samplingPerformed: boolean): void { | ||
| if (metric.sample_rate !== undefined && metric.sample_rate !== 1.0 && samplingPerformed) { | ||
| 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. | ||
| * @param samplingPerformed - Whether sampling was actually performed. | ||
| * @returns The processed metric attributes. | ||
| */ | ||
| function processMetricAttributes(beforeMetric: Metric, currentScope: Scope, client: Client, samplingPerformed: boolean): Record<string, unknown> { | ||
| const processedMetricAttributes = { | ||
| ...beforeMetric.attributes, | ||
| }; | ||
|
|
||
| addSampleRateAttribute(beforeMetric, processedMetricAttributes, samplingPerformed); | ||
|
|
||
| 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. | ||
| * | ||
|
|
@@ -133,48 +232,24 @@ 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); | ||
| const { sampled, samplingPerformed } = shouldSampleMetric(beforeMetric, currentScope); | ||
| if (!sampled) { | ||
| return; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Missed Tracking of Sampled-Out Metrics DropsWhen a metric is dropped due to sampling (when |
||
| } | ||
|
|
||
| const [, traceContext] = _getTraceInfoFromScope(client, currentScope); | ||
| const processedMetricAttributes = processMetricAttributes(beforeMetric, currentScope, client, samplingPerformed); | ||
|
|
||
| const metric: Metric = { | ||
| ...beforeMetric, | ||
| attributes: processedMetricAttributes, | ||
|
|
||
There was a problem hiding this comment.
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.0evaluates to false (since all comparisons with NaN return false), allowing NaN to pass validation. This causes the metric to be silently dropped during sampling (sincesampleRand < NaNis always false) without recording a dropped event. The validation should explicitly check for NaN and reject it as invalid.