sentry-core: Add traces_sampler client option (getsentry#510)

* sentry-core: add traces_sampler client option

Author: tommilligan

CHANGELOG.md

@@ -5,6 +5,7 @@
 **Breaking Changes**:
 
 - The minimum supported Rust version was bumped to **1.60.0** due to requirements from dependencies. ([#498](https://github.com/getsentry/sentry-rust/pull/498))
+- Added the `traces_sampler` option to `ClientOptions`. This allows the user to customise sampling rates on a per-transaction basis. ([#510](https://github.com/getsentry/sentry-rust/pull/510))
 
 **Features**:
 

sentry-contexts/src/utils.rs

@@ -26,7 +26,7 @@ mod model_support {
                 return None;
             }
 
-            let mut buf = vec![0u8; size as usize];
+            let mut buf = vec![0u8; size];
             let res = libc::sysctlbyname(
                 c_name.as_ptr() as _,
                 buf.as_mut_ptr() as *mut c_void,

sentry-core/src/clientoptions.rs

@@ -4,6 +4,7 @@ use std::sync::Arc;
 use std::time::Duration;
 
 use crate::constants::USER_AGENT;
+use crate::performance::TracesSampler;
 use crate::protocol::{Breadcrumb, Event};
 use crate::types::Dsn;
 use crate::{Integration, IntoDsn, TransportFactory};
@@ -74,6 +75,11 @@ pub struct ClientOptions {
     pub sample_rate: f32,
     /// The sample rate for tracing transactions. (0.0 - 1.0, defaults to 0.0)
     pub traces_sample_rate: f32,
+    /// If given, called with a SamplingContext for each transaction to determine the sampling rate.
+    ///
+    /// Return a sample rate between 0.0 and 1.0 for the transaction in question.
+    /// Takes priority over the `sample_rate`.
+    pub traces_sampler: Option<Arc<TracesSampler>>,
     /// Enables profiling
     pub enable_profiling: bool,
     /// The sample rate for profiling a transactions. (0.0 - 1.0, defaults to 0.0)
@@ -196,6 +202,13 @@ impl fmt::Debug for ClientOptions {
             .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("enable_profiling", &self.enable_profiling)
             .field("profiles_sample_rate", &self.profiles_sample_rate)
             .field("max_breadcrumbs", &self.max_breadcrumbs)
@@ -231,6 +244,7 @@ impl Default for ClientOptions {
             environment: None,
             sample_rate: 1.0,
             traces_sample_rate: 0.0,
+            traces_sampler: None,
             enable_profiling: false,
             profiles_sample_rate: 0.0,
             max_breadcrumbs: 100,

sentry-core/src/hub.rs

@@ -42,12 +42,12 @@ pub(crate) struct HubImpl {
 impl HubImpl {
     pub(crate) fn with<F: FnOnce(&Stack) -> R, R>(&self, f: F) -> R {
         let guard = self.stack.read().unwrap_or_else(PoisonError::into_inner);
-        f(&*guard)
+        f(&guard)
     }
 
     fn with_mut<F: FnOnce(&mut Stack) -> R, R>(&self, f: F) -> R {
         let mut guard = self.stack.write().unwrap_or_else(PoisonError::into_inner);
-        f(&mut *guard)
+        f(&mut guard)
     }
 
     fn is_active_and_usage_safe(&self) -> bool {

sentry-core/src/performance.rs

@@ -156,6 +156,13 @@ impl TransactionContext {
     }
 }
 
+/// A function to be run for each new transaction, to determine the rate at which
+/// it should be sampled.
+///
+/// This function may choose to respect the sampling of the parent transaction (`ctx.sampled`)
+/// or ignore it.
+pub type TracesSampler = dyn Fn(&TransactionContext) -> f32 + Send + Sync;
+
 // global API types:
 
 /// A wrapper that groups a [`Transaction`] and a [`Span`] together.
@@ -279,6 +286,37 @@ pub(crate) struct TransactionInner {
 
 type TransactionArc = Arc<Mutex<TransactionInner>>;
 
+/// Functional implementation of how a new transation's sample rate is chosen.
+///
+/// Split out from `Client.is_transaction_sampled` for testing.
+#[cfg(feature = "client")]
+fn transaction_sample_rate(
+    traces_sampler: Option<&TracesSampler>,
+    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(|sampled| if sampled { 1.0 } else { 0.0 })
+            .unwrap_or(traces_sample_rate),
+    }
+}
+
+/// Determine whether the new transaction should be sampled.
+#[cfg(feature = "client")]
+impl Client {
+    fn is_transaction_sampled(&self, ctx: &TransactionContext) -> bool {
+        let client_options = self.options();
+        self.sample_should_send(transaction_sample_rate(
+            client_options.traces_sampler.as_deref(),
+            ctx,
+            client_options.traces_sample_rate,
+        ))
+    }
+}
+
 /// A running Performance Monitoring Transaction.
 ///
 /// The transaction needs to be explicitly finished via [`Transaction::finish`],
@@ -292,18 +330,9 @@ pub struct Transaction {
 impl Transaction {
     #[cfg(feature = "client")]
     fn new(mut client: Option<Arc<Client>>, ctx: TransactionContext) -> Self {
-        let context = protocol::TraceContext {
-            trace_id: ctx.trace_id,
-            parent_span_id: ctx.parent_span_id,
-            op: Some(ctx.op),
-            ..Default::default()
-        };
-
         let (sampled, mut transaction) = match client.as_ref() {
             Some(client) => (
-                ctx.sampled.unwrap_or_else(|| {
-                    client.sample_should_send(client.options().traces_sample_rate)
-                }),
+                client.is_transaction_sampled(&ctx),
                 Some(protocol::Transaction {
                     name: Some(ctx.name),
                     #[cfg(all(feature = "profiling", target_family = "unix"))]
@@ -314,6 +343,13 @@ impl Transaction {
             None => (ctx.sampled.unwrap_or(false), None),
         };
 
+        let context = protocol::TraceContext {
+            trace_id: ctx.trace_id,
+            parent_span_id: ctx.parent_span_id,
+            op: Some(ctx.op),
+            ..Default::default()
+        };
+
         // throw away the transaction here, which means there is nothing to send
         // on `finish`.
         if !sampled {
@@ -679,4 +715,36 @@ mod tests {
         assert_eq!(&parsed.0.to_string(), "09e04486820349518ac7b5d2adbf6ba5");
         assert_eq!(parsed.2, Some(true));
     }
+
+    #[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);
+
+        // If only global rate, setting sampled overrides it
+        let mut ctx = TransactionContext::new("noop", "noop");
+        ctx.set_sampled(true);
+        assert_eq!(transaction_sample_rate(None, &ctx, 0.3), 1.0);
+        ctx.set_sampled(false);
+        assert_eq!(transaction_sample_rate(None, &ctx, 0.3), 0.0);
+
+        // If given, sampler function overrides everything else.
+        let mut ctx = TransactionContext::new("noop", "noop");
+        assert_eq!(transaction_sample_rate(Some(&|_| { 0.7 }), &ctx, 0.3), 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
+        let sampler = |ctx: &TransactionContext| match ctx.sampled {
+            Some(true) => 0.8,
+            Some(false) => 0.4,
+            None => 0.6,
+        };
+        ctx.set_sampled(true);
+        assert_eq!(transaction_sample_rate(Some(&sampler), &ctx, 0.3), 0.8);
+        ctx.set_sampled(None);
+        assert_eq!(transaction_sample_rate(Some(&sampler), &ctx, 0.3), 0.6);
+    }
 }

sentry-core/src/profiling.rs

@@ -241,7 +241,7 @@ fn get_profile_from_report(
         timestamp: rep.timing.start_time,
         transactions: vec![TransactionMetadata {
             id: transaction.event_id,
-            name: transaction.name.clone().unwrap_or_else(|| "".to_string()),
+            name: transaction.name.clone().unwrap_or_default(),
             trace_id,
             relative_start_ns: 0,
             relative_end_ns: transaction

sentry-panic/src/lib.rs

@@ -77,7 +77,7 @@ impl Integration for PanicIntegration {
 /// Extract the message of a panic.
 pub fn message_from_panic_info<'a>(info: &'a PanicInfo<'_>) -> &'a str {
     match info.payload().downcast_ref::<&'static str>() {
-        Some(s) => *s,
+        Some(s) => s,
         None => match info.payload().downcast_ref::<String>() {
             Some(s) => &s[..],
             None => "Box<Any>",

sentry-types/src/protocol/envelope.rs

@@ -388,7 +388,7 @@ impl Envelope {
         let payload_start = std::cmp::min(header_end + 1, slice.len());
         let payload_end = match header.length {
             Some(len) => {
-                let payload_end = payload_start + len as usize;
+                let payload_end = payload_start + len;
                 if slice.len() < payload_end {
                     return Err(EnvelopeError::UnexpectedEof);
                 }

sentry-types/src/protocol/v7.rs

@@ -1360,7 +1360,7 @@ impl Default for SpanId {
 
 impl fmt::Display for SpanId {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        write!(fmt, "{}", hex::encode(&self.0))
+        write!(fmt, "{}", hex::encode(self.0))
     }
 }
 
@@ -1406,7 +1406,7 @@ impl Default for TraceId {
 
 impl fmt::Display for TraceId {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        write!(fmt, "{}", hex::encode(&self.0))
+        write!(fmt, "{}", hex::encode(self.0))
     }
 }
 

sentry/src/transports/curl.rs

@@ -31,7 +31,7 @@ impl CurlHttpTransport {
         let http_proxy = options.http_proxy.as_ref().map(ToString::to_string);
         let https_proxy = options.https_proxy.as_ref().map(ToString::to_string);
         let dsn = options.dsn.as_ref().unwrap();
-        let user_agent = options.user_agent.to_owned();
+        let user_agent = options.user_agent.clone();
         let auth = dsn.to_auth(Some(&user_agent)).to_string();
         let url = dsn.envelope_api_url().to_string();
         let scheme = dsn.scheme();

sentry/src/transports/reqwest.rs

@@ -56,7 +56,7 @@ impl ReqwestHttpTransport {
             builder.build().unwrap()
         });
         let dsn = options.dsn.as_ref().unwrap();
-        let user_agent = options.user_agent.to_owned();
+        let user_agent = options.user_agent.clone();
         let auth = dsn.to_auth(Some(&user_agent)).to_string();
         let url = dsn.envelope_api_url().to_string();
 

sentry/src/transports/surf.rs

@@ -43,7 +43,7 @@ impl SurfHttpTransport {
         }
 
         let dsn = options.dsn.as_ref().unwrap();
-        let user_agent = options.user_agent.to_owned();
+        let user_agent = options.user_agent.clone();
         let auth = dsn.to_auth(Some(&user_agent)).to_string();
         let url = dsn.envelope_api_url().to_string();
 

sentry/src/transports/ureq.rs

@@ -112,7 +112,7 @@ impl UreqHttpTransport {
 
             builder.build()
         });
-        let user_agent = options.user_agent.to_owned();
+        let user_agent = options.user_agent.clone();
         let auth = dsn.to_auth(Some(&user_agent)).to_string();
         let url = dsn.envelope_api_url().to_string();
 

sentry/tests/test_tracing.rs

@@ -106,7 +106,7 @@ fn test_tracing() {
 
 #[tracing::instrument(fields(span_field))]
 fn function() {
-    tracing::Span::current().record("span_field", &"some data");
+    tracing::Span::current().record("span_field", "some data");
 }
 
 #[test]