Skip to content

Commit 343a63d

Browse files
jjbayerclaude
andcommitted
fix: Align metrics implementation with spec
- 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>
1 parent 20216fc commit 343a63d

File tree

6 files changed

+46
-27
lines changed

6 files changed

+46
-27
lines changed

sentry-core/src/batcher.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,14 +268,17 @@ mod tests {
268268
}
269269

270270
#[test]
271-
fn test_metrics_disabled_by_default() {
271+
fn test_metrics_disabled_explicitly() {
272272
let envelopes = test::with_captured_envelopes_options(
273273
|| {
274274
for i in 0..10 {
275275
crate::Hub::current().capture_metric(test_metric(&format!("metric.{i}")));
276276
}
277277
},
278-
crate::ClientOptions::default(),
278+
crate::ClientOptions {
279+
enable_metrics: false,
280+
..Default::default()
281+
},
279282
);
280283

281284
assert_eq!(0, envelopes.len());

sentry-core/src/client.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,7 @@ impl Client {
579579
/// processing it through `before_send_metric`.
580580
#[cfg(feature = "metrics")]
581581
fn prepare_metric(&self, mut metric: TraceMetric, scope: &Scope) -> Option<TraceMetric> {
582-
scope.apply_to_metric(&mut metric);
582+
scope.apply_to_metric(&mut metric, self.options.send_default_pii);
583583

584584
if let Some(default_attributes) = self.default_metric_attributes.as_ref() {
585585
for (key, val) in default_attributes.iter() {

sentry-core/src/clientoptions.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ pub struct ClientOptions {
174174
/// Determines whether captured structured logs should be sent to Sentry (defaults to false).
175175
#[cfg(feature = "logs")]
176176
pub enable_logs: bool,
177-
/// Determines whether captured trace metrics should be sent to Sentry (defaults to false).
177+
/// Determines whether captured trace metrics should be sent to Sentry (defaults to true).
178178
#[cfg(feature = "metrics")]
179179
pub enable_metrics: bool,
180180
/// Callback that is executed for each TraceMetric being added.
@@ -337,7 +337,7 @@ impl Default for ClientOptions {
337337
#[cfg(feature = "logs")]
338338
before_send_log: None,
339339
#[cfg(feature = "metrics")]
340-
enable_metrics: false,
340+
enable_metrics: true,
341341
#[cfg(feature = "metrics")]
342342
before_send_metric: None,
343343
}

sentry-core/src/scope/noop.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,9 @@ impl Scope {
123123

124124
/// Applies the contained scoped data to fill a trace metric.
125125
#[cfg(feature = "metrics")]
126-
pub fn apply_to_metric(&self, metric: &mut TraceMetric) {
126+
pub fn apply_to_metric(&self, metric: &mut TraceMetric, send_default_pii: bool) {
127127
let _metric = metric;
128+
let _send_default_pii = send_default_pii;
128129
minimal_unreachable!();
129130
}
130131

sentry-core/src/scope/real.rs

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -405,9 +405,10 @@ impl Scope {
405405
}
406406

407407
/// Applies the contained scoped data to a trace metric, setting the `trace_id`, `span_id`,
408-
/// and certain default attributes.
408+
/// and certain default attributes. User PII attributes are only attached when
409+
/// `send_default_pii` is `true`.
409410
#[cfg(feature = "metrics")]
410-
pub fn apply_to_metric(&self, metric: &mut TraceMetric) {
411+
pub fn apply_to_metric(&self, metric: &mut TraceMetric, send_default_pii: bool) {
411412
if let Some(span) = self.span.as_ref() {
412413
metric.trace_id = span.get_trace_context().trace_id;
413414
} else {
@@ -426,29 +427,31 @@ impl Scope {
426427
}
427428
}
428429

429-
if let Some(user) = self.user.as_ref() {
430-
if !metric.attributes.contains_key("user.id") {
431-
if let Some(id) = user.id.as_ref() {
432-
metric
433-
.attributes
434-
.insert("user.id".to_owned(), LogAttribute(id.to_owned().into()));
430+
if send_default_pii {
431+
if let Some(user) = self.user.as_ref() {
432+
if !metric.attributes.contains_key("user.id") {
433+
if let Some(id) = user.id.as_ref() {
434+
metric
435+
.attributes
436+
.insert("user.id".to_owned(), LogAttribute(id.to_owned().into()));
437+
}
435438
}
436-
}
437439

438-
if !metric.attributes.contains_key("user.name") {
439-
if let Some(name) = user.username.as_ref() {
440-
metric
441-
.attributes
442-
.insert("user.name".to_owned(), LogAttribute(name.to_owned().into()));
440+
if !metric.attributes.contains_key("user.name") {
441+
if let Some(name) = user.username.as_ref() {
442+
metric
443+
.attributes
444+
.insert("user.name".to_owned(), LogAttribute(name.to_owned().into()));
445+
}
443446
}
444-
}
445447

446-
if !metric.attributes.contains_key("user.email") {
447-
if let Some(email) = user.email.as_ref() {
448-
metric.attributes.insert(
449-
"user.email".to_owned(),
450-
LogAttribute(email.to_owned().into()),
451-
);
448+
if !metric.attributes.contains_key("user.email") {
449+
if let Some(email) = user.email.as_ref() {
450+
metric.attributes.insert(
451+
"user.email".to_owned(),
452+
LogAttribute(email.to_owned().into()),
453+
);
454+
}
452455
}
453456
}
454457
}

sentry/src/transports/ratelimit.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ pub struct RateLimiter {
1414
transaction: Option<SystemTime>,
1515
attachment: Option<SystemTime>,
1616
log_item: Option<SystemTime>,
17+
trace_metric: Option<SystemTime>,
1718
}
1819

1920
impl RateLimiter {
@@ -59,6 +60,7 @@ impl RateLimiter {
5960
"transaction" => self.transaction = new_time,
6061
"attachment" => self.attachment = new_time,
6162
"log_item" => self.log_item = new_time,
63+
"trace_metric" => self.trace_metric = new_time,
6264
_ => {}
6365
}
6466
}
@@ -93,6 +95,7 @@ impl RateLimiter {
9395
RateLimitingCategory::Transaction => self.transaction,
9496
RateLimitingCategory::Attachment => self.attachment,
9597
RateLimitingCategory::LogItem => self.log_item,
98+
RateLimitingCategory::TraceMetric => self.trace_metric,
9699
}?;
97100
time_left.duration_since(SystemTime::now()).ok()
98101
}
@@ -119,6 +122,9 @@ impl RateLimiter {
119122
EnvelopeItem::ItemContainer(ItemContainer::Logs(_)) => {
120123
RateLimitingCategory::LogItem
121124
}
125+
EnvelopeItem::ItemContainer(ItemContainer::TraceMetrics(_)) => {
126+
RateLimitingCategory::TraceMetric
127+
}
122128
_ => RateLimitingCategory::Any,
123129
})
124130
})
@@ -140,6 +146,8 @@ pub enum RateLimitingCategory {
140146
Attachment,
141147
/// Rate Limit pertaining to Log Items.
142148
LogItem,
149+
/// Rate Limit pertaining to Trace Metrics.
150+
TraceMetric,
143151
}
144152

145153
#[cfg(test)]
@@ -155,6 +163,7 @@ mod tests {
155163
assert!(rl.is_disabled(RateLimitingCategory::Session).unwrap() <= Duration::from_secs(60));
156164
assert!(rl.is_disabled(RateLimitingCategory::Transaction).is_none());
157165
assert!(rl.is_disabled(RateLimitingCategory::LogItem).is_none());
166+
assert!(rl.is_disabled(RateLimitingCategory::TraceMetric).is_none());
158167
assert!(rl.is_disabled(RateLimitingCategory::Any).is_none());
159168

160169
rl.update_from_sentry_header(
@@ -185,6 +194,9 @@ mod tests {
185194
assert!(
186195
rl.is_disabled(RateLimitingCategory::Attachment).unwrap() <= Duration::from_secs(120)
187196
);
197+
assert!(
198+
rl.is_disabled(RateLimitingCategory::TraceMetric).unwrap() <= Duration::from_secs(120)
199+
);
188200
assert!(rl.is_disabled(RateLimitingCategory::Any).unwrap() <= Duration::from_secs(120));
189201
}
190202

0 commit comments

Comments
 (0)