From a16e498dd112ba5afa4cca485f68cbad29c15271 Mon Sep 17 00:00:00 2001 From: "Daniel Szoke (via Pi Coding Agent)" Date: Thu, 5 Mar 2026 15:16:48 +0000 Subject: [PATCH] feat(sentry-core): Implement trace metric capture batching (#1026) Basic metrics capture functionality. Follow-up PR will implement the rest. Closes #1023 Closes [RUST-168](https://linear.app/getsentry/issue/RUST-168/implement-trace-metric-capture-and-batching-in-sentry-core) --- CHANGELOG.md | 1 + sentry-core/Cargo.toml | 1 + sentry-core/src/batcher.rs | 9 +- sentry-core/src/client.rs | 44 +++++- sentry-core/src/clientoptions.rs | 8 + sentry-core/src/hub.rs | 12 ++ sentry-core/src/lib.rs | 2 +- sentry-core/tests/metrics.rs | 262 +++++++++++++++++++++++++++++++ sentry-types/src/protocol/v7.rs | 20 +++ 9 files changed, 356 insertions(+), 3 deletions(-) create mode 100644 sentry-core/tests/metrics.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 78515c7cf..22cda106d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Add SDK protocol support for sending `trace_metric` envelope items ([#1022](https://github.com/getsentry/sentry-rust/pull/1022)). - Add `Metric` and `MetricType` types representing [trace metrics](https://develop.sentry.dev/sdk/telemetry/metrics/) ([#1026](https://github.com/getsentry/sentry-rust/pull/1026)). +- Add metric capture and batching in `sentry-core`. Metrics can be captured via `Hub::capture_metric` and are batched and sent as `trace_metric` envelope items. Controlled by the `metrics` feature flag and `ClientOptions::enable_metrics` ([#1026](https://github.com/getsentry/sentry-rust/pull/1026)). ## 0.47.0 diff --git a/sentry-core/Cargo.toml b/sentry-core/Cargo.toml index 5336781a7..39f2f7853 100644 --- a/sentry-core/Cargo.toml +++ b/sentry-core/Cargo.toml @@ -25,6 +25,7 @@ client = ["rand"] test = ["client", "release-health"] release-health = [] logs = [] +metrics = [] [dependencies] log = { version = "0.4.8", optional = true, features = ["std"] } diff --git a/sentry-core/src/batcher.rs b/sentry-core/src/batcher.rs index dfc28bf5e..5a714588f 100644 --- a/sentry-core/src/batcher.rs +++ b/sentry-core/src/batcher.rs @@ -8,6 +8,8 @@ use crate::client::TransportArc; use crate::protocol::EnvelopeItem; use crate::Envelope; use sentry_types::protocol::v7::Log; +#[cfg(feature = "metrics")] +use sentry_types::protocol::v7::Metric; // Flush when there's 100 items in the buffer const MAX_ITEMS: usize = 100; @@ -40,6 +42,11 @@ impl Batch for Log { const TYPE_NAME: &str = "logs"; } +#[cfg(feature = "metrics")] +impl Batch for Metric { + const TYPE_NAME: &str = "metrics"; +} + /// Accumulates items in the queue and submits them through the transport when one of the flushing /// conditions is met. pub(crate) struct Batcher { @@ -154,7 +161,7 @@ impl Drop for Batcher { } } -#[cfg(all(test, feature = "test"))] +#[cfg(all(test, feature = "test", feature = "logs"))] mod tests { use crate::logger_info; use crate::test; diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs index 003d3edc5..de9f4be32 100644 --- a/sentry-core/src/client.rs +++ b/sentry-core/src/client.rs @@ -12,7 +12,7 @@ use crate::protocol::SessionUpdate; use rand::random; use sentry_types::random_uuid; -#[cfg(feature = "logs")] +#[cfg(any(feature = "logs", feature = "metrics"))] use crate::batcher::Batcher; use crate::constants::SDK_INFO; use crate::protocol::{ClientSdkInfo, Event}; @@ -24,6 +24,8 @@ use crate::SessionMode; use crate::{ClientOptions, Envelope, Hub, Integration, Scope, Transport}; #[cfg(feature = "logs")] use sentry_types::protocol::v7::Context; +#[cfg(feature = "metrics")] +use sentry_types::protocol::v7::Metric; #[cfg(feature = "logs")] use sentry_types::protocol::v7::{Log, LogAttribute}; @@ -59,6 +61,8 @@ pub struct Client { session_flusher: RwLock>, #[cfg(feature = "logs")] logs_batcher: RwLock>>, + #[cfg(feature = "metrics")] + metrics_batcher: RwLock>>, #[cfg(feature = "logs")] default_log_attributes: Option>, integrations: Vec<(TypeId, Arc)>, @@ -91,6 +95,13 @@ impl Clone for Client { None }); + #[cfg(feature = "metrics")] + let metrics_batcher = RwLock::new(if self.options.enable_metrics { + Some(Batcher::new(transport.clone())) + } else { + None + }); + Client { options: self.options.clone(), transport, @@ -98,6 +109,8 @@ impl Clone for Client { session_flusher, #[cfg(feature = "logs")] logs_batcher, + #[cfg(feature = "metrics")] + metrics_batcher, #[cfg(feature = "logs")] default_log_attributes: self.default_log_attributes.clone(), integrations: self.integrations.clone(), @@ -176,6 +189,13 @@ impl Client { None }); + #[cfg(feature = "metrics")] + let metrics_batcher = RwLock::new(if options.enable_metrics { + Some(Batcher::new(transport.clone())) + } else { + None + }); + #[allow(unused_mut)] let mut client = Client { options, @@ -184,6 +204,8 @@ impl Client { session_flusher, #[cfg(feature = "logs")] logs_batcher, + #[cfg(feature = "metrics")] + metrics_batcher, #[cfg(feature = "logs")] default_log_attributes: None, integrations, @@ -420,6 +442,10 @@ impl Client { if let Some(ref batcher) = *self.logs_batcher.read().unwrap() { batcher.flush(); } + #[cfg(feature = "metrics")] + if let Some(ref batcher) = *self.metrics_batcher.read().unwrap() { + batcher.flush(); + } if let Some(ref transport) = *self.transport.read().unwrap() { transport.flush(timeout.unwrap_or(self.options.shutdown_timeout)) } else { @@ -439,6 +465,8 @@ impl Client { drop(self.session_flusher.write().unwrap().take()); #[cfg(feature = "logs")] drop(self.logs_batcher.write().unwrap().take()); + #[cfg(feature = "metrics")] + drop(self.metrics_batcher.write().unwrap().take()); let transport_opt = self.transport.write().unwrap().take(); if let Some(transport) = transport_opt { sentry_debug!("client close; request transport to shut down"); @@ -493,6 +521,20 @@ impl Client { Some(log) } + + /// Captures a metric and sends it to Sentry. + #[cfg(feature = "metrics")] + pub fn capture_metric(&self, metric: Metric, _: &Scope) { + // TODO: Read scope + if let Some(batcher) = self + .metrics_batcher + .read() + .expect("metrics batcher lock could not be acquired") + .as_ref() + { + batcher.enqueue(metric); + } + } } // Make this unwind safe. It's not out of the box because of the diff --git a/sentry-core/src/clientoptions.rs b/sentry-core/src/clientoptions.rs index 02d5d5a98..3bd552e2f 100644 --- a/sentry-core/src/clientoptions.rs +++ b/sentry-core/src/clientoptions.rs @@ -172,6 +172,9 @@ pub struct ClientOptions { /// Determines whether captured structured logs should be sent to Sentry (defaults to false). #[cfg(feature = "logs")] pub enable_logs: bool, + /// Determines whether captured metrics should be sent to Sentry (defaults to true). + #[cfg(feature = "metrics")] + pub enable_metrics: bool, // Other options not documented in Unified API /// Disable SSL verification. /// @@ -278,6 +281,9 @@ impl fmt::Debug for ClientOptions { .field("enable_logs", &self.enable_logs) .field("before_send_log", &before_send_log); + #[cfg(feature = "metrics")] + debug_struct.field("enable_metrics", &self.enable_metrics); + debug_struct.field("user_agent", &self.user_agent).finish() } } @@ -317,6 +323,8 @@ impl Default for ClientOptions { enable_logs: true, #[cfg(feature = "logs")] before_send_log: None, + #[cfg(feature = "metrics")] + enable_metrics: false, } } } diff --git a/sentry-core/src/hub.rs b/sentry-core/src/hub.rs index ebd70b0ca..68f705629 100644 --- a/sentry-core/src/hub.rs +++ b/sentry-core/src/hub.rs @@ -4,6 +4,8 @@ use std::sync::{Arc, RwLock}; +#[cfg(feature = "metrics")] +use crate::protocol::Metric; use crate::protocol::{Event, Level, Log, LogAttribute, LogLevel, Map, SessionStatus}; use crate::types::Uuid; use crate::{Integration, IntoBreadcrumbs, Scope, ScopeGuard}; @@ -255,4 +257,14 @@ impl Hub { client.capture_log(log, &top.scope); }} } + + /// Captures a metric. + #[cfg(feature = "metrics")] + pub fn capture_metric(&self, metric: Metric) { + with_client_impl! {{ + let top = self.inner.with(|stack| stack.top().clone()); + let Some(ref client) = top.client else { return }; + client.capture_metric(metric, &top.scope); + }} + } } diff --git a/sentry-core/src/lib.rs b/sentry-core/src/lib.rs index 6a2a1b5dd..6a6c7c899 100644 --- a/sentry-core/src/lib.rs +++ b/sentry-core/src/lib.rs @@ -136,7 +136,7 @@ pub use crate::transport::{Transport, TransportFactory}; mod logger; // structured logging macros exported with `#[macro_export]` // client feature -#[cfg(all(feature = "client", feature = "logs"))] +#[cfg(all(feature = "client", any(feature = "logs", feature = "metrics")))] mod batcher; #[cfg(feature = "client")] mod client; diff --git a/sentry-core/tests/metrics.rs b/sentry-core/tests/metrics.rs new file mode 100644 index 000000000..819f0ad2d --- /dev/null +++ b/sentry-core/tests/metrics.rs @@ -0,0 +1,262 @@ +#![cfg(all(feature = "test", feature = "metrics"))] + +use std::collections::HashSet; + +use anyhow::{Context, Result}; + +use sentry::protocol::MetricType; +use sentry_core::protocol::{EnvelopeItem, ItemContainer}; +use sentry_core::test; +use sentry_core::{ClientOptions, Hub}; +use sentry_types::protocol::v7::Metric; + +/// Test that metrics are sent when metrics are enabled. +#[test] +fn sent_when_enabled() { + let options = ClientOptions { + enable_metrics: true, + ..Default::default() + }; + + let mut envelopes = + test::with_captured_envelopes_options(|| capture_test_metric("test"), options); + + assert_eq!(envelopes.len(), 1, "expected exactly one envelope"); + + let envelope = envelopes.pop().unwrap(); + + let mut items = envelope.into_items(); + let Some(item) = items.next() else { + panic!("Expected at least one item"); + }; + + assert!(items.next().is_none(), "Expected only one item"); + + let EnvelopeItem::ItemContainer(ItemContainer::Metrics(mut metrics)) = item else { + panic!("Envelope item has unexpected structure"); + }; + + assert_eq!(metrics.len(), 1, "Expected exactly one metric"); + + let metric = metrics.pop().unwrap(); + assert!(matches!(metric, Metric { + r#type: MetricType::Counter, + name, + value: 1.0, + .. + } if name == "test")); +} + +/// Test that metrics are disabled (not sent) when disabled in the +/// [`ClientOptions`]. +#[test] +fn metrics_disabled_by_default() { + // Metrics are disabled by default. + let options: ClientOptions = Default::default(); + + let envelopes = test::with_captured_envelopes_options(|| capture_test_metric("test"), options); + assert!( + envelopes.is_empty(), + "no envelopes should be captured when metrics disabled" + ) +} + +/// Test that no metrics are captured by a no-op call with +/// metrics enabled +#[test] +fn noop_sends_nothing() { + let options = ClientOptions { + enable_metrics: true, + ..Default::default() + }; + + let envelopes = test::with_captured_envelopes_options(|| (), options); + + assert!(envelopes.is_empty(), "no-op should not capture metrics"); +} + +/// Test that 100 metrics are sent in a single envelope. +#[test] +fn test_metrics_batching_at_limit() { + let options = ClientOptions { + enable_metrics: true, + ..Default::default() + }; + + let envelopes = test::with_captured_envelopes_options( + || { + (0..100) + .map(|i| format!("metric.{i}")) + .for_each(capture_test_metric); + }, + options, + ); + + let envelope = envelopes + .try_into_only_item() + .expect("expected exactly one envelope"); + let item = envelope + .into_items() + .try_into_only_item() + .expect("expected exactly one item"); + let metrics = item + .into_metrics() + .expect("the envelope item is not a metrics item"); + + assert_eq!(metrics.len(), 100, "expected 100 metrics"); + + let metric_names: HashSet<_> = metrics + .into_iter() + .inspect(|metric| assert_eq!(metric.value, 1.0, "metric had unexpected value")) + .inspect(|metric| { + assert_eq!( + metric.r#type, + MetricType::Counter, + "metric had unexpected type" + ) + }) + .map(|metric| metric.name) + .collect(); + + (0..100) + .map(|i| format!("metric.{i}")) + .for_each(|metric_name| { + assert!( + metric_names.contains(&metric_name), + "expected metric {metric_name} was not captured" + ) + }); +} + +/// Test that 101 envelopes are sent in two separate envelopes +#[test] +fn test_metrics_batching_over_limit() { + let options = ClientOptions { + enable_metrics: true, + ..Default::default() + }; + + let mut envelopes = test::with_captured_envelopes_options( + || { + (0..101) + .map(|i| format!("metric.{i}")) + .for_each(capture_test_metric); + }, + options, + ) + .into_iter(); + let envelope1 = envelopes.next().expect("expected a first envelope"); + let envelope2 = envelopes.next().expect("expected a second envelope"); + assert!(envelopes.next().is_none(), "expected exactly two envelopes"); + + let item1 = envelope1 + .into_items() + .try_into_only_item() + .expect("expected exactly one item in the first envelope"); + let metrics1 = item1 + .into_metrics() + .expect("the first envelope item is not a metrics item"); + + assert_eq!(metrics1.len(), 100, "expected 100 metrics"); + + let first_metric_names: HashSet<_> = metrics1 + .into_iter() + .inspect(|metric| assert_eq!(metric.value, 1.0, "metric had unexpected value")) + .inspect(|metric| { + assert_eq!( + metric.r#type, + MetricType::Counter, + "metric had unexpected type" + ) + }) + .map(|metric| metric.name) + .collect(); + + (0..100) + .map(|i| format!("metric.{i}")) + .for_each(|metric_name| { + assert!( + first_metric_names.contains(&metric_name), + "expected metric {metric_name} was not captured in the first envelope" + ) + }); + + let item2 = envelope2 + .into_items() + .try_into_only_item() + .expect("expected exactly one item in the second envelope"); + let metrics2 = item2 + .into_metrics() + .expect("the second envelope item is not a metrics item"); + let metric2 = metrics2 + .try_into_only_item() + .expect("expected exactly one metric in the second envelope"); + + assert!( + matches!(metric2, Metric { + r#type: MetricType::Counter, + name, + value: 1.0, + .. + } if name == "metric.100"), + "unexpected metric captured" + ) +} + +/// Returns a new [`Metric`] with [type `Counter`](MetricType), +/// the provided name, and a value of `1.0`. The other fields are unspecified. +fn test_metric(name: S) -> Metric +where + S: Into, +{ + Metric::new(MetricType::Counter, name, 1.0, Default::default()) +} + +/// Helper function to capture a metric, returned by `test_metric` on the current Hub. +fn capture_test_metric(name: S) +where + S: Into, +{ + Hub::current().capture_metric(test_metric(name)) +} + +/// Extension trait for iterators allowing conversion to only item. +trait TryIntoOnlyElementExt { + type Item; + + /// Convert the iterator to the only item, erroring if the + /// iterator does not contain exactly one item. + fn try_into_only_item(self) -> Result; +} + +impl TryIntoOnlyElementExt for I +where + I: IntoIterator, +{ + type Item = I::Item; + + fn try_into_only_item(self) -> Result { + let mut iter = self.into_iter(); + let rv = iter.next().context("iterator was empty")?; + + match iter.next() { + Some(_) => anyhow::bail!("iterator had more than one item"), + None => Ok(rv), + } + } +} + +trait IntoMetricsExt { + /// Attempt to convert the provided value to a trace metric, + /// returning None if the conversion is not possible. + fn into_metrics(self) -> Option>; +} + +impl IntoMetricsExt for EnvelopeItem { + fn into_metrics(self) -> Option> { + match self { + EnvelopeItem::ItemContainer(ItemContainer::Metrics(metrics)) => Some(metrics), + _ => None, + } + } +} diff --git a/sentry-types/src/protocol/v7.rs b/sentry-types/src/protocol/v7.rs index a63c97b4e..47395185f 100644 --- a/sentry-types/src/protocol/v7.rs +++ b/sentry-types/src/protocol/v7.rs @@ -2407,6 +2407,26 @@ pub struct Metric { pub attributes: Map, } +impl Metric { + /// Create a new [`Metric`] with provided values and the current time + /// as the timestamp. Other values set to their [`Default`] values. + pub fn new(r#type: MetricType, name: S, value: f64, trace_id: TraceId) -> Self + where + S: Into, + { + Self { + r#type, + name: name.into(), + value, + timestamp: SystemTime::now(), + trace_id, + span_id: Default::default(), + unit: Default::default(), + attributes: Default::default(), + } + } +} + /// An ID that identifies an organization in the Sentry backend. #[derive(Clone, Copy, Debug, Serialize, Deserialize, PartialEq)] pub struct OrganizationId(u64);