From 0a1417d18c54d6b5ab37ab55894e87274d400660 Mon Sep 17 00:00:00 2001 From: "Daniel Szoke (via Pi Coding Agent)" Date: Mon, 9 Mar 2026 16:19:57 +0000 Subject: [PATCH 1/2] feat(metrics): Add trace metric enrichment with default and user attributes (RUST-169) Add metric enrichment in sentry-core so emitted trace metrics include: - Trace/span association: trace_id from active span or propagation context, span_id from active span when not explicitly set. - Default SDK attributes: sentry.environment, sentry.release, sentry.sdk.name, sentry.sdk.version, and server.address. - User PII attributes (user.id, user.name, user.email) gated by send_default_pii. - before_send_metric callback for filtering/modifying metrics. Attribute merges use or_insert to preserve explicitly set metric attributes, matching the behavior specified in https://develop.sentry.dev/sdk/telemetry/metrics/#default-attributes and https://develop.sentry.dev/sdk/telemetry/metrics/#user-attributes. Scope and client enrichment logic is based on the approach from #997. Co-authored-by: Joris Bayer Closes #1024 Closes [RUST-169](https://linear.app/getsentry/issue/RUST-169/add-trace-metric-default-and-user-attribute-enrichment-in-sentry-core) --- sentry-core/src/client.rs | 81 ++++++++-- sentry-core/src/clientoptions.rs | 18 ++- sentry-core/src/scope/noop.rs | 10 ++ sentry-core/src/scope/real.rs | 59 ++++++- sentry-core/tests/metrics.rs | 266 ++++++++++++++++++++++++++++++- 5 files changed, 420 insertions(+), 14 deletions(-) diff --git a/sentry-core/src/client.rs b/sentry-core/src/client.rs index de9f4be32..df209aca5 100644 --- a/sentry-core/src/client.rs +++ b/sentry-core/src/client.rs @@ -1,6 +1,6 @@ use std::any::TypeId; use std::borrow::Cow; -#[cfg(feature = "logs")] +#[cfg(any(feature = "logs", feature = "metrics"))] use std::collections::BTreeMap; use std::fmt; use std::panic::RefUnwindSafe; @@ -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(all(feature = "metrics", not(feature = "logs")))] +use sentry_types::protocol::v7::LogAttribute; #[cfg(feature = "metrics")] use sentry_types::protocol::v7::Metric; #[cfg(feature = "logs")] @@ -65,6 +67,8 @@ pub struct Client { metrics_batcher: RwLock>>, #[cfg(feature = "logs")] default_log_attributes: Option>, + #[cfg(feature = "metrics")] + default_metric_attributes: Option>, integrations: Vec<(TypeId, Arc)>, pub(crate) sdk_info: ClientSdkInfo, } @@ -113,6 +117,8 @@ impl Clone for Client { metrics_batcher, #[cfg(feature = "logs")] default_log_attributes: self.default_log_attributes.clone(), + #[cfg(feature = "metrics")] + default_metric_attributes: self.default_metric_attributes.clone(), integrations: self.integrations.clone(), sdk_info: self.sdk_info.clone(), } @@ -208,6 +214,8 @@ impl Client { metrics_batcher, #[cfg(feature = "logs")] default_log_attributes: None, + #[cfg(feature = "metrics")] + default_metric_attributes: None, integrations, sdk_info, }; @@ -215,6 +223,9 @@ impl Client { #[cfg(feature = "logs")] client.cache_default_log_attributes(); + #[cfg(feature = "metrics")] + client.cache_default_metric_attributes(); + client } @@ -269,6 +280,35 @@ impl Client { self.default_log_attributes = Some(attributes); } + #[cfg(feature = "metrics")] + fn cache_default_metric_attributes(&mut self) { + let mut attributes = BTreeMap::new(); + + if let Some(environment) = self.options.environment.as_ref() { + attributes.insert("sentry.environment".to_owned(), environment.clone().into()); + } + + if let Some(release) = self.options.release.as_ref() { + attributes.insert("sentry.release".to_owned(), release.clone().into()); + } + + attributes.insert( + "sentry.sdk.name".to_owned(), + self.sdk_info.name.to_owned().into(), + ); + + attributes.insert( + "sentry.sdk.version".to_owned(), + self.sdk_info.version.to_owned().into(), + ); + + if let Some(server) = &self.options.server_name { + attributes.insert("server.address".to_owned(), server.clone().into()); + } + + self.default_metric_attributes = Some(attributes); + } + pub(crate) fn get_integration(&self) -> Option<&I> where I: Integration, @@ -524,16 +564,37 @@ impl Client { /// 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); + pub fn capture_metric(&self, metric: Metric, scope: &Scope) { + if !self.options.enable_metrics { + return; } + if let Some(metric) = self.prepare_metric(metric, scope) { + if let Some(ref batcher) = *self.metrics_batcher.read().unwrap() { + batcher.enqueue(metric); + } + } + } + + /// Prepares a metric to be sent, setting the `trace_id` and other default attributes, and + /// processing it through `before_send_metric`. + #[cfg(feature = "metrics")] + fn prepare_metric(&self, mut metric: Metric, scope: &Scope) -> Option { + scope.apply_to_metric(&mut metric, self.options.send_default_pii); + + if let Some(default_attributes) = self.default_metric_attributes.as_ref() { + for (key, val) in default_attributes.iter() { + metric + .attributes + .entry(key.to_owned()) + .or_insert(val.clone()); + } + } + + if let Some(ref func) = self.options.before_send_metric { + metric = func(metric)?; + } + + Some(metric) } } diff --git a/sentry-core/src/clientoptions.rs b/sentry-core/src/clientoptions.rs index 3bd552e2f..33dd09585 100644 --- a/sentry-core/src/clientoptions.rs +++ b/sentry-core/src/clientoptions.rs @@ -7,6 +7,8 @@ use crate::constants::USER_AGENT; use crate::performance::TracesSampler; #[cfg(feature = "logs")] use crate::protocol::Log; +#[cfg(feature = "metrics")] +use crate::protocol::Metric; use crate::protocol::{Breadcrumb, Event}; use crate::types::Dsn; use crate::{Integration, IntoDsn, TransportFactory}; @@ -175,6 +177,9 @@ pub struct ClientOptions { /// Determines whether captured metrics should be sent to Sentry (defaults to true). #[cfg(feature = "metrics")] pub enable_metrics: bool, + /// Callback that is executed for each Metric before sending. + #[cfg(feature = "metrics")] + pub before_send_metric: Option>, // Other options not documented in Unified API /// Disable SSL verification. /// @@ -282,7 +287,16 @@ impl fmt::Debug for ClientOptions { .field("before_send_log", &before_send_log); #[cfg(feature = "metrics")] - debug_struct.field("enable_metrics", &self.enable_metrics); + { + let before_send_metric = { + #[derive(Debug)] + struct BeforeSendMetric; + self.before_send_metric.as_ref().map(|_| BeforeSendMetric) + }; + debug_struct + .field("enable_metrics", &self.enable_metrics) + .field("before_send_metric", &before_send_metric); + } debug_struct.field("user_agent", &self.user_agent).finish() } @@ -325,6 +339,8 @@ impl Default for ClientOptions { before_send_log: None, #[cfg(feature = "metrics")] enable_metrics: false, + #[cfg(feature = "metrics")] + before_send_metric: None, } } } diff --git a/sentry-core/src/scope/noop.rs b/sentry-core/src/scope/noop.rs index fc62120b9..e8bad34f3 100644 --- a/sentry-core/src/scope/noop.rs +++ b/sentry-core/src/scope/noop.rs @@ -2,6 +2,8 @@ use std::fmt; #[cfg(feature = "logs")] use crate::protocol::Log; +#[cfg(feature = "metrics")] +use crate::protocol::Metric; use crate::protocol::{Context, Event, Level, User, Value}; use crate::TransactionOrSpan; @@ -119,6 +121,14 @@ impl Scope { minimal_unreachable!(); } + /// Applies the contained scoped data to fill a trace metric. + #[cfg(feature = "metrics")] + pub fn apply_to_metric(&self, metric: &mut Metric, send_default_pii: bool) { + let _metric = metric; + let _send_default_pii = send_default_pii; + minimal_unreachable!(); + } + /// Set the given [`TransactionOrSpan`] as the active span for this scope. pub fn set_span(&mut self, span: Option) { let _ = span; diff --git a/sentry-core/src/scope/real.rs b/sentry-core/src/scope/real.rs index 590f39214..9889ea0df 100644 --- a/sentry-core/src/scope/real.rs +++ b/sentry-core/src/scope/real.rs @@ -10,7 +10,11 @@ use crate::protocol::{ Attachment, Breadcrumb, Context, Event, Level, TraceContext, Transaction, User, Value, }; #[cfg(feature = "logs")] -use crate::protocol::{Log, LogAttribute}; +use crate::protocol::Log; +#[cfg(any(feature = "logs", feature = "metrics"))] +use crate::protocol::LogAttribute; +#[cfg(feature = "metrics")] +use crate::protocol::Metric; #[cfg(feature = "release-health")] use crate::session::Session; use crate::{Client, SentryTrace, TraceHeader, TraceHeadersIter}; @@ -399,6 +403,59 @@ impl Scope { } } + /// Applies the contained scoped data to a trace metric, setting the `trace_id`, `span_id`, + /// and certain default attributes. User PII attributes are only attached when + /// `send_default_pii` is `true`. + #[cfg(feature = "metrics")] + pub fn apply_to_metric(&self, metric: &mut Metric, send_default_pii: bool) { + if let Some(span) = self.span.as_ref() { + metric.trace_id = span.get_trace_context().trace_id; + } else { + metric.trace_id = self.propagation_context.trace_id; + } + + if metric.span_id.is_none() { + if let Some(span) = self.get_span() { + let span_id = match span { + crate::TransactionOrSpan::Transaction(transaction) => { + transaction.get_trace_context().span_id + } + crate::TransactionOrSpan::Span(span) => span.get_span_id(), + }; + metric.span_id = Some(span_id); + } + } + + if send_default_pii { + if let Some(user) = self.user.as_ref() { + if !metric.attributes.contains_key("user.id") { + if let Some(id) = user.id.as_ref() { + metric + .attributes + .insert("user.id".to_owned(), LogAttribute(id.to_owned().into())); + } + } + + if !metric.attributes.contains_key("user.name") { + if let Some(name) = user.username.as_ref() { + metric + .attributes + .insert("user.name".to_owned(), LogAttribute(name.to_owned().into())); + } + } + + if !metric.attributes.contains_key("user.email") { + if let Some(email) = user.email.as_ref() { + metric.attributes.insert( + "user.email".to_owned(), + LogAttribute(email.to_owned().into()), + ); + } + } + } + } + } + /// Set the given [`TransactionOrSpan`] as the active span for this scope. pub fn set_span(&mut self, span: Option) { self.span = Arc::new(span); diff --git a/sentry-core/tests/metrics.rs b/sentry-core/tests/metrics.rs index 819f0ad2d..496962a8c 100644 --- a/sentry-core/tests/metrics.rs +++ b/sentry-core/tests/metrics.rs @@ -1,11 +1,12 @@ #![cfg(all(feature = "test", feature = "metrics"))] use std::collections::HashSet; +use std::sync::Arc; use anyhow::{Context, Result}; -use sentry::protocol::MetricType; -use sentry_core::protocol::{EnvelopeItem, ItemContainer}; +use sentry::protocol::{LogAttribute, MetricType, User}; +use sentry_core::protocol::{EnvelopeItem, ItemContainer, Value}; use sentry_core::test; use sentry_core::{ClientOptions, Hub}; use sentry_types::protocol::v7::Metric; @@ -260,3 +261,264 @@ impl IntoMetricsExt for EnvelopeItem { } } } + +/// Helper to extract the single metric from captured envelopes. +fn extract_single_metric(envelopes: Vec) -> Metric { + let envelope = envelopes.into_only_item().expect("expected one envelope"); + let item = envelope + .into_items() + .into_only_item() + .expect("expected one item"); + let mut metrics = item.into_metrics().expect("expected metrics item"); + assert_eq!(metrics.len(), 1, "expected exactly one metric"); + metrics.pop().unwrap() +} + +/// Test that trace_id is set from the propagation context when no span is active. +#[test] +fn trace_id_from_propagation_context() { + let options = ClientOptions { + enable_metrics: true, + ..Default::default() + }; + + let envelopes = test::with_captured_envelopes_options(|| capture_test_metric("test"), options); + let metric = extract_single_metric(envelopes); + + // trace_id should be non-zero (set from propagation context) + assert_ne!( + metric.trace_id, + Default::default(), + "trace_id should be set from propagation context" + ); +} + +/// Test that default SDK attributes are attached to metrics. +#[test] +fn default_attributes_attached() { + let options = ClientOptions { + enable_metrics: true, + environment: Some("test-env".into()), + release: Some("1.0.0".into()), + server_name: Some("test-server".into()), + ..Default::default() + }; + + let envelopes = test::with_captured_envelopes_options(|| capture_test_metric("test"), options); + let metric = extract_single_metric(envelopes); + + assert_eq!( + metric.attributes.get("sentry.environment"), + Some(&LogAttribute(Value::from("test-env"))), + ); + assert_eq!( + metric.attributes.get("sentry.release"), + Some(&LogAttribute(Value::from("1.0.0"))), + ); + assert!( + metric.attributes.contains_key("sentry.sdk.name"), + "sentry.sdk.name should be present" + ); + assert!( + metric.attributes.contains_key("sentry.sdk.version"), + "sentry.sdk.version should be present" + ); + assert_eq!( + metric.attributes.get("server.address"), + Some(&LogAttribute(Value::from("test-server"))), + ); +} + +/// Test that explicitly set metric attributes are not overwritten by defaults. +#[test] +fn default_attributes_do_not_overwrite_explicit() { + let options = ClientOptions { + enable_metrics: true, + environment: Some("default-env".into()), + ..Default::default() + }; + + let envelopes = test::with_captured_envelopes_options( + || { + let mut metric = test_metric("test"); + metric.attributes.insert( + "sentry.environment".to_owned(), + LogAttribute(Value::from("custom-env")), + ); + Hub::current().capture_metric(metric); + }, + options, + ); + let metric = extract_single_metric(envelopes); + + assert_eq!( + metric.attributes.get("sentry.environment"), + Some(&LogAttribute(Value::from("custom-env"))), + "explicitly set attribute should not be overwritten" + ); +} + +/// Test that user attributes are NOT attached when `send_default_pii` is false. +#[test] +fn user_attributes_absent_without_send_default_pii() { + let options = ClientOptions { + enable_metrics: true, + send_default_pii: false, + ..Default::default() + }; + + let envelopes = test::with_captured_envelopes_options( + || { + sentry_core::configure_scope(|scope| { + scope.set_user(Some(User { + id: Some("uid-123".into()), + username: Some("testuser".into()), + email: Some("test@example.com".into()), + ..Default::default() + })); + }); + capture_test_metric("test"); + }, + options, + ); + let metric = extract_single_metric(envelopes); + + assert!( + !metric.attributes.contains_key("user.id"), + "user.id should not be set when send_default_pii is false" + ); + assert!( + !metric.attributes.contains_key("user.name"), + "user.name should not be set when send_default_pii is false" + ); + assert!( + !metric.attributes.contains_key("user.email"), + "user.email should not be set when send_default_pii is false" + ); +} + +/// Test that user attributes ARE attached when `send_default_pii` is true. +#[test] +fn user_attributes_present_with_send_default_pii() { + let options = ClientOptions { + enable_metrics: true, + send_default_pii: true, + ..Default::default() + }; + + let envelopes = test::with_captured_envelopes_options( + || { + sentry_core::configure_scope(|scope| { + scope.set_user(Some(User { + id: Some("uid-123".into()), + username: Some("testuser".into()), + email: Some("test@example.com".into()), + ..Default::default() + })); + }); + capture_test_metric("test"); + }, + options, + ); + let metric = extract_single_metric(envelopes); + + assert_eq!( + metric.attributes.get("user.id"), + Some(&LogAttribute(Value::from("uid-123"))), + ); + assert_eq!( + metric.attributes.get("user.name"), + Some(&LogAttribute(Value::from("testuser"))), + ); + assert_eq!( + metric.attributes.get("user.email"), + Some(&LogAttribute(Value::from("test@example.com"))), + ); +} + +/// Test that explicitly set user attributes on the metric are not overwritten +/// by scope user data, even when `send_default_pii` is true. +#[test] +fn user_attributes_do_not_overwrite_explicit() { + let options = ClientOptions { + enable_metrics: true, + send_default_pii: true, + ..Default::default() + }; + + let envelopes = test::with_captured_envelopes_options( + || { + sentry_core::configure_scope(|scope| { + scope.set_user(Some(User { + id: Some("scope-uid".into()), + username: Some("scope-user".into()), + email: Some("scope@example.com".into()), + ..Default::default() + })); + }); + let mut metric = test_metric("test"); + metric.attributes.insert( + "user.id".to_owned(), + LogAttribute(Value::from("explicit-uid")), + ); + Hub::current().capture_metric(metric); + }, + options, + ); + let metric = extract_single_metric(envelopes); + + assert_eq!( + metric.attributes.get("user.id"), + Some(&LogAttribute(Value::from("explicit-uid"))), + "explicitly set user.id should not be overwritten" + ); + // Non-explicit user attributes should still come from scope + assert_eq!( + metric.attributes.get("user.name"), + Some(&LogAttribute(Value::from("scope-user"))), + ); + assert_eq!( + metric.attributes.get("user.email"), + Some(&LogAttribute(Value::from("scope@example.com"))), + ); +} + +/// Test that `before_send_metric` can filter out metrics. +#[test] +fn before_send_metric_can_drop() { + let options = ClientOptions { + enable_metrics: true, + before_send_metric: Some(Arc::new(|_| None)), + ..Default::default() + }; + + let envelopes = test::with_captured_envelopes_options(|| capture_test_metric("test"), options); + assert!( + envelopes.is_empty(), + "metric should be dropped by before_send_metric" + ); +} + +/// Test that `before_send_metric` can modify metrics. +#[test] +fn before_send_metric_can_modify() { + let options = ClientOptions { + enable_metrics: true, + before_send_metric: Some(Arc::new(|mut metric| { + metric.attributes.insert( + "added_by_callback".to_owned(), + LogAttribute(Value::from("yes")), + ); + Some(metric) + })), + ..Default::default() + }; + + let envelopes = test::with_captured_envelopes_options(|| capture_test_metric("test"), options); + let metric = extract_single_metric(envelopes); + + assert_eq!( + metric.attributes.get("added_by_callback"), + Some(&LogAttribute(Value::from("yes"))), + ); +} From 4f8e195db465962ff32316cecff5332f6b7974f5 Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Tue, 10 Mar 2026 13:55:45 +0100 Subject: [PATCH 2/2] fixup! some improvements --- sentry-core/src/performance.rs | 7 +++ sentry-core/src/scope/real.rs | 83 ++++++++++++++++------------------ 2 files changed, 45 insertions(+), 45 deletions(-) diff --git a/sentry-core/src/performance.rs b/sentry-core/src/performance.rs index 5416746ea..303b3b531 100644 --- a/sentry-core/src/performance.rs +++ b/sentry-core/src/performance.rs @@ -586,6 +586,13 @@ impl TransactionOrSpan { TransactionOrSpan::Span(span) => span.finish(), } } + + pub(crate) fn span_id(&self) -> SpanId { + match self { + TransactionOrSpan::Transaction(transaction) => transaction.get_trace_context().span_id, + TransactionOrSpan::Span(span) => span.get_span_id(), + } + } } #[derive(Debug)] diff --git a/sentry-core/src/scope/real.rs b/sentry-core/src/scope/real.rs index 9889ea0df..9033cbd45 100644 --- a/sentry-core/src/scope/real.rs +++ b/sentry-core/src/scope/real.rs @@ -6,15 +6,15 @@ use std::sync::Mutex; use std::sync::{Arc, PoisonError, RwLock}; use crate::performance::TransactionOrSpan; -use crate::protocol::{ - Attachment, Breadcrumb, Context, Event, Level, TraceContext, Transaction, User, Value, -}; #[cfg(feature = "logs")] use crate::protocol::Log; #[cfg(any(feature = "logs", feature = "metrics"))] use crate::protocol::LogAttribute; #[cfg(feature = "metrics")] use crate::protocol::Metric; +use crate::protocol::{ + Attachment, Breadcrumb, Context, Event, Level, TraceContext, Transaction, User, Value, +}; #[cfg(feature = "release-health")] use crate::session::Session; use crate::{Client, SentryTrace, TraceHeader, TraceHeadersIter}; @@ -408,52 +408,26 @@ impl Scope { /// `send_default_pii` is `true`. #[cfg(feature = "metrics")] pub fn apply_to_metric(&self, metric: &mut Metric, send_default_pii: bool) { - if let Some(span) = self.span.as_ref() { - metric.trace_id = span.get_trace_context().trace_id; - } else { - metric.trace_id = self.propagation_context.trace_id; - } + metric.trace_id = match self.span.as_ref().as_ref() { + Some(span) => span.get_trace_context().trace_id, + None => self.propagation_context.trace_id, + }; - if metric.span_id.is_none() { - if let Some(span) = self.get_span() { - let span_id = match span { - crate::TransactionOrSpan::Transaction(transaction) => { - transaction.get_trace_context().span_id - } - crate::TransactionOrSpan::Span(span) => span.get_span_id(), - }; - metric.span_id = Some(span_id); - } - } + metric.span_id = metric + .span_id + .or_else(|| self.get_span().map(|ts| ts.span_id())); - if send_default_pii { - if let Some(user) = self.user.as_ref() { - if !metric.attributes.contains_key("user.id") { - if let Some(id) = user.id.as_ref() { - metric - .attributes - .insert("user.id".to_owned(), LogAttribute(id.to_owned().into())); - } - } + if !send_default_pii { + return; + } - if !metric.attributes.contains_key("user.name") { - if let Some(name) = user.username.as_ref() { - metric - .attributes - .insert("user.name".to_owned(), LogAttribute(name.to_owned().into())); - } - } + let Some(user) = self.user.as_ref() else { + return; + }; - if !metric.attributes.contains_key("user.email") { - if let Some(email) = user.email.as_ref() { - metric.attributes.insert( - "user.email".to_owned(), - LogAttribute(email.to_owned().into()), - ); - } - } - } - } + metric.insert_attribute("user.id", user.id.as_deref()); + metric.insert_attribute("user.name", user.username.as_deref()); + metric.insert_attribute("user.email", user.email.as_deref()); } /// Set the given [`TransactionOrSpan`] as the active span for this scope. @@ -501,3 +475,22 @@ impl Scope { } } } + +#[cfg(feature = "metrics")] +trait MetricExt { + fn insert_attribute(&mut self, key: K, value: V) + where + K: Into, + V: Into; +} + +#[cfg(feature = "metrics")] +impl MetricExt for Metric { + fn insert_attribute(&mut self, key: K, value: V) + where + K: Into, + V: Into, + { + self.attributes.insert(key, LogAttribute(value.into())); + } +}