Skip to content

Comments

feat: Add trace metrics support#997

Open
jjbayer wants to merge 13 commits intomasterfrom
feat/trace-metrics
Open

feat: Add trace metrics support#997
jjbayer wants to merge 13 commits intomasterfrom
feat/trace-metrics

Conversation

@jjbayer
Copy link
Member

@jjbayer jjbayer commented Feb 17, 2026

This PR introduces logic for emiting & batching trace metrics, largely copy-pasted from logs.

Usage example:

sentry::metric_count!("api.requests", 1, route = "/users", method = "GET");
sentry::metric_gauge!("memory.usage", 1024.0, unit = "byte");
sentry::metric_distribution!("response.time", 150.0, unit = "millisecond", route = "/users");

Resolves #938.

Implement trace metrics per the Sentry SDK telemetry spec, enabling
SDKs to capture and transmit quantitative performance data (counters,
gauges, distributions) associated with traces.

The implementation mirrors the existing structured logs architecture:
- TraceMetric/TraceMetricType protocol types in sentry-types
- TraceMetrics envelope item container with serialization/deserialization
- MetricsBatcher with background worker thread (100-item / 5s flush)
- Feature-gated behind `metrics` feature flag
- Public API: metrics_count(), metrics_gauge(), metrics_distribution()
- before_send_metric callback and enable_metrics client option
- Trace context (trace_id, span_id) and user attributes from scope

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Feb 17, 2026

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 343a63d

jjbayer and others added 9 commits February 17, 2026 15:22
Replace the duplicate LogsBatcher and MetricsBatcher with a single
generic Batcher<T> that handles both log and metric batching using
a function pointer for envelope item conversion.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Only the configured fraction of metrics is submitted. Sampled metrics
are annotated with a `sentry.client_sample_rate` attribute so the
server can extrapolate.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
A sample rate of 0.0 now disables metrics entirely, removing the
need for a separate enable_metrics boolean.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Changes the public API from free functions with options to a builder:

  sentry::metrics::count("api.requests").emit(1);
  sentry::metrics::distribution("response.time")
      .with_attribute("route", "my_route")
      .with_unit("millisecond")
      .emit(0.123);

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Metrics are now opt-in: users must explicitly set a sample rate to
enable trace metrics collection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@@ -0,0 +1,337 @@
//! Generic batching for Sentry envelope items (logs, metrics, etc.).
Copy link
Member Author

Choose a reason for hiding this comment

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

This was moved & generalized from logs.rs.

Copy link
Member Author

Choose a reason for hiding this comment

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

This has moved to batcher.rs.

/// Metrics are disabled by default. Sampled metrics are annotated with a
/// `sentry.client_sample_rate` attribute so the server can extrapolate.
#[cfg(feature = "metrics")]
pub metrics_sample_rate: f32,
Copy link
Member Author

Choose a reason for hiding this comment

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

The develop docs state that this should be a boolean flag, and that it should go into _experimental: https://develop.sentry.dev/sdk/telemetry/metrics/#initialization-options

I'm willing to change this but I still think it makes sense to have a sample rate.

@jjbayer jjbayer marked this pull request as ready for review February 19, 2026 13:00
}
},
crate::ClientOptions {
metrics_sample_rate: 1.0,
Copy link
Member

Choose a reason for hiding this comment

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

Spec doesn't mention any metrics_sample_rate, I don't think we should have it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will revert to a boolean flag. I need sampling for the relay use case (I wouldn't dare to send the entire volume at once), but I can add the sentry.client_sample_rate attribute there manually.

What about what the specs say about an _experiments section, should we move the flag there like the spec suggests?

Copy link
Member

@lcian lcian Feb 19, 2026

Choose a reason for hiding this comment

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

I guess we could also keep the sample rate, if this is going to be used by internal services we're gonna need it at some point anyways.

Another approach which I prefer for Rust is to feature flag it and make sure we state that it's clear that the flag is experimental and could be removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Keeping the enable_metrics flag for now to keep it simple & spec-conformant. The spec now seems to allow putting it in the stable section.

Switch from MetricBuilder to declarative macros (metric_count!,
metric_gauge!, metric_distribution!) matching the pattern used by
the logging macros. Support `unit = "..."` for measurement units
and quoted `"key" = value` syntax for attribute keys.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

Per the spec, use a simple boolean flag (default: false) instead of a
sample rate float. Removes client-side sampling and the
sentry.client_sample_rate attribute annotation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@lcian
Copy link
Member

lcian commented Feb 19, 2026

Haven't done an in depth review but I noticed that we're missing the rate limiting data category.
I assume you've already done it, but it might be worth it to ask an agent to compare the implementation with the spec (https://develop.sentry.dev/sdk/telemetry/metrics/) and identify any gaps. The data category is in the spec so I assume an agent should find descrepancies like this.

- Default enable_metrics to true (spec requirement)
- Gate user PII attributes (user.id, user.name, user.email) on
  send_default_pii
- Add trace_metric rate limiting category so server-sent rate limits
  for metrics are properly respected

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jjbayer added a commit to getsentry/relay that referenced this pull request Feb 20, 2026
…client_sample_rate

Adapt to the updated sentry-rust trace metrics API (getsentry/sentry-rust#997):
- Rename `metrics.trace_sample_rate` config to `metrics.send_to_sentry`
- Enable `metrics_enabled` on the Sentry SDK when `send_to_sentry > 0.0`
- Use sentry-core macros instead of removed free functions
- Set `sentry.client_sample_rate` attribute on each trace metric so
  Sentry can extrapolate from the sampled data

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jjbayer
Copy link
Member Author

jjbayer commented Feb 20, 2026

Haven't done an in depth review but I noticed that we're missing the rate limiting data category. I assume you've already done it, but it might be worth it to ask an agent to compare the implementation with the spec (https://develop.sentry.dev/sdk/telemetry/metrics/) and identify any gaps. The data category is in the spec so I assume an agent should find descrepancies like this.

@lcian good catch! I gave the agent the spec when it came up with the initial plan, but it skipped rate limiting. I asked it to check the spec again (it also seems to have changed since then) and it made some improvements, see 343a63d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sentry Metrics for Rust

2 participants