diff --git a/CHANGELOG.md b/CHANGELOG.md index a7f1292a..8f8d7750 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ .debug(true) .release("my-app@1.0.0"); ``` +- Removed the public `ClientOptions::traces_sample_rate` and `ClientOptions::traces_sampler` fields. Use `ClientOptions::traces_sampling_strategy` to inspect the configured traces sampling strategy, and use the existing `ClientOptions::traces_sample_rate(...)` and `ClientOptions::traces_sampler(...)` builder setters to configure fixed-rate and callback-based sampling ([#1227](https://github.com/getsentry/sentry-rust/pull/1227)). ## 0.48.4 diff --git a/sentry-core/src/clientoptions.rs b/sentry-core/src/clientoptions.rs index 98cc5444..7bc86233 100644 --- a/sentry-core/src/clientoptions.rs +++ b/sentry-core/src/clientoptions.rs @@ -79,6 +79,37 @@ impl MaxRequestBodySize { } } +/// Defines how traces should be sampled. +/// +/// Leaving this at [`Disabled`](Self::Disabled) is distinct from explicitly configuring +/// [`FixedRate`](Self::FixedRate) with a rate of `0.0`. Both disable local transaction sampling +/// when there is no parent sampling decision, but an explicit fixed-rate strategy can still honor +/// an inherited sampling decision. +#[derive(Clone, Default)] +#[non_exhaustive] +pub enum TracesSamplingStrategy { + /// Sample the trace at a fixed sample rate. The rate should be between 0.0 and 1.0, inclusive. + FixedRate(f32), + /// Sample the traces using a [`TracesSampler`] function. + Function(Arc), + /// Disable tracing. + #[default] + Disabled, +} + +impl fmt::Debug for TracesSamplingStrategy { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::FixedRate(rate) => f.debug_tuple("FixedRate").field(rate).finish(), + Self::Function(callback) => f + .debug_tuple("Function") + .field(&format_args!("{:p}", Arc::as_ptr(callback))) + .finish(), + Self::Disabled => f.write_str("Disabled"), + } + } +} + /// Configuration settings for the client. /// /// These options are explained in more detail in the general @@ -114,14 +145,13 @@ pub struct ClientOptions { /// /// See [`sample_rate`](method@ClientOptions::sample_rate) for details. pub sample_rate: f32, - /// The sample rate for tracing transactions. + /// The traces sampling strategy. /// - /// See [`traces_sample_rate`](method@ClientOptions::traces_sample_rate) for details. - pub traces_sample_rate: f32, - /// The sampler callback for tracing transactions. - /// - /// See [`traces_sampler`](method@ClientOptions::traces_sampler) for details. - pub traces_sampler: Option>, + /// This can be set to a fixed rate with + /// [`traces_sample_rate`](method@ClientOptions::traces_sample_rate), a function with + /// [`traces_sampler`](method@ClientOptions::traces_sampler), or can be left at the default + /// disabled value. + pub traces_sampling_strategy: TracesSamplingStrategy, /// Maximum number of breadcrumbs. /// /// See [`max_breadcrumbs`](method@ClientOptions::max_breadcrumbs) for details. @@ -308,9 +338,14 @@ impl ClientOptions { } } - /// Sets the [sample rate](field@ClientOptions::traces_sample_rate) for tracing transactions. + /// Sets the [traces sampling strategy](field@ClientOptions::traces_sampling_strategy) to a + /// fixed sample rate for tracing transactions. + /// + /// Must be between `0.0` and `1.0`. /// - /// Must be between `0.0` and `1.0`. Defaults to `0.0`. + /// Calling this method stores an explicit fixed-rate traces sampling strategy, even when the + /// rate is `0.0`. That is distinct from leaving traces sampling unset, which uses + /// [`TracesSamplingStrategy::Disabled`]. /// /// # Panics /// @@ -323,24 +358,30 @@ impl ClientOptions { ) } + let traces_sampling_strategy = TracesSamplingStrategy::FixedRate(traces_sample_rate); + Self { - traces_sample_rate, + traces_sampling_strategy, ..self } } - /// Sets the [sampler callback](field@ClientOptions::traces_sampler) for tracing transactions. + /// Sets the [traces sampling strategy](field@ClientOptions::traces_sampling_strategy) to a + /// sampler callback for tracing transactions. /// - /// Return a sample rate between `0.0` and `1.0` for the transaction in question. Takes - /// priority over [`traces_sample_rate`](method@ClientOptions::traces_sample_rate). + /// Return a sample rate between `0.0` and `1.0` for the transaction in question. This replaces + /// any fixed-rate strategy configured with [`Self::traces_sample_rate`] and is distinct from + /// leaving traces sampling unset. #[inline] pub fn traces_sampler(self, traces_sampler: F) -> Self where F: Fn(&TransactionContext) -> f32 + Send + Sync + 'static, { - let traces_sampler = Some(Arc::new(traces_sampler) as Arc); + let traces_sampling_strategy = + TracesSamplingStrategy::Function(Arc::new(traces_sampler) as Arc); + Self { - traces_sampler, + traces_sampling_strategy, ..self } } @@ -696,14 +737,7 @@ impl fmt::Debug for ClientOptions { .field("release", &self.release) .field("environment", &self.environment) .field("sample_rate", &self.sample_rate) - .field("traces_sample_rate", &self.traces_sample_rate) - .field( - "traces_sampler", - &self - .traces_sampler - .as_ref() - .map(|arc| std::ptr::addr_of!(**arc)), - ) + .field("traces_sampling_strategy", &self.traces_sampling_strategy) .field("max_breadcrumbs", &self.max_breadcrumbs) .field("attach_stacktrace", &self.attach_stacktrace) .field("send_default_pii", &self.send_default_pii) @@ -738,8 +772,7 @@ impl Default for ClientOptions { release: None, environment: None, sample_rate: 1.0, - traces_sample_rate: 0.0, - traces_sampler: None, + traces_sampling_strategy: Default::default(), max_breadcrumbs: 100, attach_stacktrace: false, send_default_pii: false, diff --git a/sentry-core/src/lib.rs b/sentry-core/src/lib.rs index 53490873..45f7c528 100644 --- a/sentry-core/src/lib.rs +++ b/sentry-core/src/lib.rs @@ -123,7 +123,9 @@ mod transport; // public api or exports from this crate pub use crate::api::*; pub use crate::breadcrumbs::IntoBreadcrumbs; -pub use crate::clientoptions::{BeforeCallback, ClientOptions, SessionMode}; +pub use crate::clientoptions::{ + BeforeCallback, ClientOptions, SessionMode, TracesSamplingStrategy, +}; pub use crate::error::{capture_error, event_from_error, parse_type_from_debug}; pub use crate::futures::{SentryFuture, SentryFutureExt}; pub use crate::hub::Hub; diff --git a/sentry-core/src/performance.rs b/sentry-core/src/performance.rs index a5d405e3..ae6056e0 100644 --- a/sentry-core/src/performance.rs +++ b/sentry-core/src/performance.rs @@ -8,6 +8,8 @@ use std::time::SystemTime; use sentry_types::protocol::v7::client_report::Reason as ClientReportReason; use sentry_types::protocol::v7::SpanId; +#[cfg(feature = "client")] +use crate::clientoptions::TracesSamplingStrategy; use crate::{protocol, Hub}; #[cfg(feature = "client")] @@ -252,8 +254,8 @@ impl TransactionContext { /// Set the sampling decision for this Transaction. /// - /// This can be either an explicit boolean flag, or [`None`], which will fall - /// back to use the configured `traces_sample_rate` option. + /// This can be either an explicit boolean flag, or [`None`], which leaves + /// the decision to the configured traces sampling strategy. pub fn set_sampled(&mut self, sampled: impl Into>) { self.sampled = sampled.into(); } @@ -606,13 +608,13 @@ type TransactionArc = Arc>; /// Split out from `Client.is_transaction_sampled` for testing. #[cfg(feature = "client")] fn transaction_sample_rate( - traces_sampler: Option<&TracesSampler>, + traces_sampling_strategy: &TracesSamplingStrategy, ctx: &TransactionContext, - traces_sample_rate: f32, ) -> f32 { - match (traces_sampler, traces_sample_rate) { - (Some(traces_sampler), _) => traces_sampler(ctx), - (None, traces_sample_rate) => ctx.sampled.map(f32::from).unwrap_or(traces_sample_rate), + match traces_sampling_strategy { + &TracesSamplingStrategy::FixedRate(rate) => ctx.sampled.map_or(rate, f32::from), + TracesSamplingStrategy::Function(traces_sampler) => traces_sampler(ctx), + TracesSamplingStrategy::Disabled => 0.0, } } @@ -621,11 +623,7 @@ fn transaction_sample_rate( impl Client { fn determine_sampling_decision(&self, ctx: &TransactionContext) -> (bool, f32) { let client_options = self.options(); - let sample_rate = transaction_sample_rate( - client_options.traces_sampler.as_deref(), - ctx, - client_options.traces_sample_rate, - ); + let sample_rate = transaction_sample_rate(&client_options.traces_sampling_strategy, ctx); let sampled = self.sample_should_send(sample_rate); (sampled, sample_rate) } @@ -1319,6 +1317,7 @@ impl std::fmt::Display for SentryTrace { #[cfg(test)] mod tests { use std::str::FromStr; + use std::sync::Arc; use super::*; @@ -1373,35 +1372,65 @@ mod tests { #[cfg(feature = "client")] #[test] fn compute_transaction_sample_rate() { - // Global rate used as fallback. let ctx = TransactionContext::new("noop", "noop"); - assert_eq!(transaction_sample_rate(None, &ctx, 0.3), 0.3); - assert_eq!(transaction_sample_rate(None, &ctx, 0.7), 0.7); + assert_eq!( + transaction_sample_rate(&TracesSamplingStrategy::FixedRate(0.3), &ctx), + 0.3 + ); + assert_eq!( + transaction_sample_rate(&TracesSamplingStrategy::FixedRate(0.7), &ctx), + 0.7 + ); + + let mut ctx = TransactionContext::new("noop", "noop"); + ctx.set_sampled(true); + assert_eq!( + transaction_sample_rate(&TracesSamplingStrategy::FixedRate(0.3), &ctx), + 1.0 + ); + ctx.set_sampled(false); + assert_eq!( + transaction_sample_rate(&TracesSamplingStrategy::FixedRate(0.3), &ctx), + 0.0 + ); - // If only global rate, setting sampled overrides it + let ctx = TransactionContext::new("noop", "noop"); + assert_eq!( + transaction_sample_rate(&TracesSamplingStrategy::Disabled, &ctx), + 0.0 + ); let mut ctx = TransactionContext::new("noop", "noop"); ctx.set_sampled(true); - assert_eq!(transaction_sample_rate(None, &ctx, 0.3), 1.0); + assert_eq!( + transaction_sample_rate(&TracesSamplingStrategy::Disabled, &ctx), + 0.0 + ); ctx.set_sampled(false); - assert_eq!(transaction_sample_rate(None, &ctx, 0.3), 0.0); + assert_eq!( + transaction_sample_rate(&TracesSamplingStrategy::Disabled, &ctx), + 0.0 + ); - // If given, sampler function overrides everything else. + // Function and FixedRate are mutually exclusive strategy variants. A function + // strategy can ignore parent sampling or choose to inspect it. let mut ctx = TransactionContext::new("noop", "noop"); - assert_eq!(transaction_sample_rate(Some(&|_| { 0.7 }), &ctx, 0.3), 0.7); + let sampler = |_: &TransactionContext| 0.7_f32; + let strategy = TracesSamplingStrategy::Function(Arc::new(sampler) as Arc); + assert_eq!(transaction_sample_rate(&strategy, &ctx), 0.7); ctx.set_sampled(false); - assert_eq!(transaction_sample_rate(Some(&|_| { 0.7 }), &ctx, 0.3), 0.7); - // But the sampler may choose to inspect parent sampling + assert_eq!(transaction_sample_rate(&strategy, &ctx), 0.7); + let sampler = |ctx: &TransactionContext| match ctx.sampled() { - Some(true) => 0.8, - Some(false) => 0.4, - None => 0.6, + Some(true) => 0.8_f32, + Some(false) => 0.4_f32, + None => 0.6_f32, }; + let strategy = TracesSamplingStrategy::Function(Arc::new(sampler) as Arc); ctx.set_sampled(true); - assert_eq!(transaction_sample_rate(Some(&sampler), &ctx, 0.3), 0.8); + assert_eq!(transaction_sample_rate(&strategy, &ctx), 0.8); ctx.set_sampled(None); - assert_eq!(transaction_sample_rate(Some(&sampler), &ctx, 0.3), 0.6); + assert_eq!(transaction_sample_rate(&strategy, &ctx), 0.6); - // Can use first-class and custom attributes of the context. let sampler = |ctx: &TransactionContext| { if ctx.name() == "must-name" || ctx.operation() == "must-operation" { return 1.0; @@ -1417,14 +1446,13 @@ mod tests { 0.1 }; - // First class attributes + let strategy = TracesSamplingStrategy::Function(Arc::new(sampler) as Arc); let ctx = TransactionContext::new("noop", "must-operation"); - assert_eq!(transaction_sample_rate(Some(&sampler), &ctx, 0.3), 1.0); + assert_eq!(transaction_sample_rate(&strategy, &ctx), 1.0); let ctx = TransactionContext::new("must-name", "noop"); - assert_eq!(transaction_sample_rate(Some(&sampler), &ctx, 0.3), 1.0); - // Custom data payload + assert_eq!(transaction_sample_rate(&strategy, &ctx), 1.0); let mut ctx = TransactionContext::new("noop", "noop"); ctx.custom_insert("rate".to_owned(), serde_json::json!(0.7)); - assert_eq!(transaction_sample_rate(Some(&sampler), &ctx, 0.3), 0.7); + assert_eq!(transaction_sample_rate(&strategy, &ctx), 0.7); } }