ref(server): Return pick/sampling decisions as enum instead of bool (#4799)

Because I get eternally confused by whether sampled means to keep
something or to drop it, let's just avoid the confusion with an enum
instead.

It's called `PickResult` to not collide with a (dynamic)
`SamplingResult`.

Author: Dav1dde

relay-server/src/metrics/metric_stats.rs

@@ -119,7 +119,7 @@ impl MetricStats {
             .options
             .metric_stats_rollout_rate;
 
-        is_rolled_out(organization_id.value(), rate)
+        is_rolled_out(organization_id.value(), rate).is_keep()
     }
 
     fn to_volume_metric(&self, bucket: impl TrackableBucket, outcome: &Outcome) -> Option<Bucket> {

relay-server/src/services/processor.rs

@@ -1389,7 +1389,7 @@ impl EnvelopeProcessorService {
         // If spans were already extracted for an event, we rely on span processing to extract metrics.
         let extract_spans = !spans_extracted.0
             && project_info.config.features.produces_spans()
-            && utils::sample(global.options.span_extraction_sample_rate.unwrap_or(1.0));
+            && utils::sample(global.options.span_extraction_sample_rate.unwrap_or(1.0)).is_keep();
 
         let metrics = crate::metrics_extraction::event::extract_metrics(
             event,
@@ -2933,7 +2933,7 @@ impl EnvelopeProcessorService {
         };
 
         let error_sample_rate = global_config.options.cardinality_limiter_error_sample_rate;
-        if !limits.exceeded_limits().is_empty() && utils::sample(error_sample_rate) {
+        if !limits.exceeded_limits().is_empty() && utils::sample(error_sample_rate).is_keep() {
             for limit in limits.exceeded_limits() {
                 relay_log::with_scope(
                     |scope| {

relay-server/src/services/processor/ourlog.rs

@@ -25,9 +25,9 @@ pub fn filter(
         .options
         .ourlogs_ingestion_sample_rate
         .map(sample)
-        .unwrap_or(true);
+        .unwrap_or_default();
 
-    let action = match logging_disabled || !logs_sampled {
+    let action = match logging_disabled || logs_sampled.is_discard() {
         true => ItemAction::DropSilently,
         false => ItemAction::Keep,
     };

relay-server/src/services/processor/replay.rs

@@ -261,7 +261,7 @@ fn handle_replay_recording_item(
         match &error {
             relay_replays::recording::ParseRecordingError::Compression(e) => {
                 // 20k errors per day at 0.1% sample rate == 20 logs per day
-                if sample(0.001) {
+                if sample(0.001).is_keep() {
                     relay_log::with_scope(
                         move |scope| {
                             scope.add_attachment(relay_log::protocol::Attachment {

relay-server/src/services/processor/span/processing.rs

@@ -293,7 +293,7 @@ pub fn extract_from_event(
     }
 
     if let Some(sample_rate) = global_config.options.span_extraction_sample_rate {
-        if !sample(sample_rate) {
+        if sample(sample_rate).is_discard() {
             return spans_extracted;
         }
     }

relay-server/src/utils/pick.rs

@@ -1,16 +1,44 @@
-/// Returns `true` if `id` is rolled out with a rollout rate `rate`.
+/// Result of a sampling operation, whether to keep or discard something.
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
+pub enum PickResult {
+    /// The item should be kept.
+    #[default]
+    Keep,
+    /// The item should be dropped.
+    Discard,
+}
+
+/// Returns [`PickResult::Keep`] if `id` is rolled out with a rollout rate `rate`.
 ///
 /// Deterministically makes a rollout decision for an id, usually organization id,
 /// and rate.
-pub fn is_rolled_out(id: u64, rate: f32) -> bool {
-    ((id % 100000) as f32 / 100000.0f32) < rate
+pub fn is_rolled_out(id: u64, rate: f32) -> PickResult {
+    match ((id % 100000) as f32 / 100000.0f32) < rate {
+        true => PickResult::Keep,
+        false => PickResult::Discard,
+    }
 }
 
-/// Returns `true` if the current item should be sampled.
+impl PickResult {
+    /// Returns `true` if the sampling result is [`PickResult::Keep`].
+    pub fn is_keep(self) -> bool {
+        matches!(self, PickResult::Keep)
+    }
+
+    /// Returns `true` if the sampling result is [`PickResult::Discard`].
+    pub fn is_discard(self) -> bool {
+        !self.is_keep()
+    }
+}
+
+/// Returns [`PickResult::Keep`] if the current item should be sampled.
 ///
 /// The passed `rate` is expected to be `0 <= rate <= 1`.
-pub fn sample(rate: f32) -> bool {
-    (rate >= 1.0) || (rate > 0.0 && rand::random::<f32>() < rate)
+pub fn sample(rate: f32) -> PickResult {
+    match (rate >= 1.0) || (rate > 0.0 && rand::random::<f32>() < rate) {
+        true => PickResult::Keep,
+        false => PickResult::Discard,
+    }
 }
 
 #[cfg(test)]
@@ -19,31 +47,31 @@ mod test {
 
     #[test]
     fn test_rollout() {
-        assert!(!is_rolled_out(1, 0.0));
-        assert!(is_rolled_out(1, 0.0001)); // Id 1 should always be rolled out
-        assert!(is_rolled_out(1, 0.1));
-        assert!(is_rolled_out(1, 0.9));
-        assert!(is_rolled_out(1, 1.0));
-
-        assert!(!is_rolled_out(0, 0.0));
-        assert!(is_rolled_out(0, 1.0));
-        assert!(!is_rolled_out(100000, 0.0));
-        assert!(is_rolled_out(100000, 1.0));
-        assert!(!is_rolled_out(100001, 0.0));
-        assert!(is_rolled_out(100001, 1.0));
-
-        assert!(!is_rolled_out(42, -100.0));
-        assert!(is_rolled_out(42, 100.0));
+        assert_eq!(is_rolled_out(1, 0.0), PickResult::Discard);
+        assert_eq!(is_rolled_out(1, 0.0001), PickResult::Keep); // Id 1 should always be rolled out
+        assert_eq!(is_rolled_out(1, 0.1), PickResult::Keep);
+        assert_eq!(is_rolled_out(1, 0.9), PickResult::Keep);
+        assert_eq!(is_rolled_out(1, 1.0), PickResult::Keep);
+
+        assert_eq!(is_rolled_out(0, 0.0), PickResult::Discard);
+        assert_eq!(is_rolled_out(0, 1.0), PickResult::Keep);
+        assert_eq!(is_rolled_out(100000, 0.0), PickResult::Discard);
+        assert_eq!(is_rolled_out(100000, 1.0), PickResult::Keep);
+        assert_eq!(is_rolled_out(100001, 0.0), PickResult::Discard);
+        assert_eq!(is_rolled_out(100001, 1.0), PickResult::Keep);
+
+        assert_eq!(is_rolled_out(42, -100.0), PickResult::Discard);
+        assert_eq!(is_rolled_out(42, 100.0), PickResult::Keep);
     }
 
     #[test]
     fn test_sample() {
-        assert!(sample(1.0));
-        assert!(!sample(0.0));
+        assert_eq!(sample(1.0), PickResult::Keep);
+        assert_eq!(sample(0.0), PickResult::Discard);
 
         let mut r: i64 = 0;
         for _ in 0..10000 {
-            if sample(0.5) {
+            if sample(0.5).is_keep() {
                 r += 1;
             } else {
                 r -= 1;