fix!(core): Distinguish disabled traces sampling strategy

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
 

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<TracesSampler>),
+    /// 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<Arc<TracesSampler>>,
+    /// 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<F>(self, traces_sampler: F) -> Self
     where
         F: Fn(&TransactionContext) -> f32 + Send + Sync + 'static,
     {
-        let traces_sampler = Some(Arc::new(traces_sampler) as Arc<TracesSampler>);
+        let traces_sampling_strategy =
+            TracesSamplingStrategy::Function(Arc::new(traces_sampler) as Arc<TracesSampler>);
+
         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,

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;

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<Option<bool>>) {
         self.sampled = sampled.into();
     }
@@ -606,13 +608,13 @@ type TransactionArc = Arc<Mutex<TransactionInner>>;
 /// 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<TracesSampler>);
+        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<TracesSampler>);
         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<TracesSampler>);
         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);
     }
 }