From 6aedc4f3b1e864e651ece914cf55f03db2145bc9 Mon Sep 17 00:00:00 2001
From: Jan Michael Auer <mail@jauer.org>
Date: Tue, 14 Nov 2023 12:00:46 +0100
Subject: [PATCH 01/10] feat(protocol): Support comparisons in rule conditions
 on strings

---
 relay-protocol/src/condition.rs | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/relay-protocol/src/condition.rs b/relay-protocol/src/condition.rs
index 0f10fe652b..b35ede0933 100644
--- a/relay-protocol/src/condition.rs
+++ b/relay-protocol/src/condition.rs
@@ -4,7 +4,7 @@
 
 use relay_common::glob3::GlobPatterns;
 use serde::{Deserialize, Serialize};
-use serde_json::{Number, Value};
+use serde_json::Value;
 
 use crate::{Getter, Val};
 
@@ -106,12 +106,12 @@ macro_rules! impl_cmp_condition {
             /// Path of the field that should match the value.
             pub name: String,
             /// The numeric value to check against.
-            pub value: Number,
+            pub value: Value,
         }
 
         impl $struct_name {
             /// Creates a new condition that comparison condition.
-            pub fn new(field: impl Into<String>, value: impl Into<Number>) -> Self {
+            pub fn new(field: impl Into<String>, value: impl Into<Value>) -> Self {
                 Self {
                     name: field.into(),
                     value: value.into(),
@@ -136,6 +136,8 @@ macro_rules! impl_cmp_condition {
                     a $operator b
                 } else if let (Some(a), Some(b)) = (value.as_f64(), self.value.as_f64()) {
                     a $operator b
+                } else if let (Some(a), Some(b)) = (value.as_str(), self.value.as_str()) {
+                    a $operator b
                 } else {
                     false
                 }
@@ -527,7 +529,7 @@ impl RuleCondition {
     ///
     /// let condition = RuleCondition::gt("obj.length", 10);
     /// ```
-    pub fn gt(field: impl Into<String>, value: impl Into<Number>) -> Self {
+    pub fn gt(field: impl Into<String>, value: impl Into<Value>) -> Self {
         Self::Gt(GtCondition::new(field, value))
     }
 
@@ -540,7 +542,7 @@ impl RuleCondition {
     ///
     /// let condition = RuleCondition::gte("obj.length", 10);
     /// ```
-    pub fn gte(field: impl Into<String>, value: impl Into<Number>) -> Self {
+    pub fn gte(field: impl Into<String>, value: impl Into<Value>) -> Self {
         Self::Gte(GteCondition::new(field, value))
     }
 
@@ -553,7 +555,7 @@ impl RuleCondition {
     ///
     /// let condition = RuleCondition::lt("obj.length", 10);
     /// ```
-    pub fn lt(field: impl Into<String>, value: impl Into<Number>) -> Self {
+    pub fn lt(field: impl Into<String>, value: impl Into<Value>) -> Self {
         Self::Lt(LtCondition::new(field, value))
     }
 
@@ -566,7 +568,7 @@ impl RuleCondition {
     ///
     /// let condition = RuleCondition::lte("obj.length", 10);
     /// ```
-    pub fn lte(field: impl Into<String>, value: impl Into<Number>) -> Self {
+    pub fn lte(field: impl Into<String>, value: impl Into<Value>) -> Self {
         Self::Lte(LteCondition::new(field, value))
     }
 
@@ -934,6 +936,7 @@ mod tests {
                     & RuleCondition::eq_ignore_case("trace.user.segment", "vip"),
             ),
             ("match no conditions", RuleCondition::all()),
+            ("string cmp", RuleCondition::gt("trace.transaction", "t")),
         ];
 
         let dsc = mock_dsc();

From 70df1d4527d404820c726978caaa8a9f1735ea9c Mon Sep 17 00:00:00 2001
From: Jan Michael Auer <mail@jauer.org>
Date: Tue, 14 Nov 2023 16:21:35 +0100
Subject: [PATCH 02/10] ref: Introduce versioning for dynamic sampling

---
 py/tests/test_processing.py                   |  28 ++++-
 relay-dynamic-config/src/project.rs           |  12 ++-
 relay-sampling/src/config.rs                  |  80 +++++++++-----
 .../tests/fixtures/sampling_config.json       |   6 +-
 relay-server/src/actors/processor.rs          | 102 ++++++++++--------
 relay-server/src/actors/project.rs            |   4 +-
 relay-server/src/testutils.rs                 |  18 ++--
 relay-server/src/utils/dynamic_sampling.rs    |  51 +++------
 tests/integration/test_dynamic_sampling.py    |   7 +-
 tests/integration/test_healthchecks.py        |   4 +-
 tests/integration/test_metrics.py             |  12 +--
 tests/integration/test_outcome.py             |  16 +--
 tests/integration/test_projectconfigs.py      |  12 +--
 13 files changed, 197 insertions(+), 155 deletions(-)

diff --git a/py/tests/test_processing.py b/py/tests/test_processing.py
index aff0690389..9b32da1a13 100644
--- a/py/tests/test_processing.py
+++ b/py/tests/test_processing.py
@@ -248,7 +248,7 @@ def test_invalid_sampling_condition():
         sentry_relay.validate_rule_condition(condition)
 
 
-def test_validate_sampling_configuration():
+def test_validate_legacy_sampling_configuration():
     """
     Tests that a valid sampling rule configuration passes
     """
@@ -288,6 +288,32 @@ def test_validate_sampling_configuration():
     sentry_relay.validate_sampling_configuration(config)
 
 
+def test_validate_legacy_sampling_configuration():
+    """
+    Tests that a valid sampling rule configuration passes
+    """
+    config = """{
+        "version": 2,
+        "rules": [
+            {
+                "type": "trace",
+                "samplingValue": {
+                    "type": "sampleRate",
+                    "value": 0.7
+                },
+                "condition": {
+                    "op": "custom",
+                    "name": "event.legacy_browser",
+                    "value":["ie10"]
+                },
+                "id":1
+            }
+        ]
+    }"""
+    # Should NOT throw
+    sentry_relay.validate_sampling_configuration(config)
+
+
 def test_validate_project_config():
     config = {"allowedDomains": ["*"], "trustedRelays": [], "piiConfig": None}
     # Does not raise:
diff --git a/relay-dynamic-config/src/project.rs b/relay-dynamic-config/src/project.rs
index 00a6a6f3ec..4cbfc8e0cb 100644
--- a/relay-dynamic-config/src/project.rs
+++ b/relay-dynamic-config/src/project.rs
@@ -46,8 +46,8 @@ pub struct ProjectConfig {
     #[serde(skip_serializing_if = "Vec::is_empty")]
     pub quotas: Vec<Quota>,
     /// Configuration for sampling traces, if not present there will be no sampling.
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub dynamic_sampling: Option<SamplingConfig>,
+    #[serde(alias = "dynamicSampling", skip_serializing_if = "Option::is_none")]
+    pub sampling: Option<ErrorBoundary<SamplingConfig>>,
     /// Configuration for measurements.
     /// NOTE: do not access directly, use [`relay_event_normalization::DynamicMeasurementsConfig`].
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -97,6 +97,10 @@ impl ProjectConfig {
 
         metrics::convert_conditional_tagging(self);
         defaults::add_span_metrics(self);
+
+        if let Some(ErrorBoundary::Ok(ref mut sampling_config)) = self.sampling {
+            sampling_config.normalize();
+        }
     }
 }
 
@@ -111,7 +115,7 @@ impl Default for ProjectConfig {
             datascrubbing_settings: DataScrubbingConfig::default(),
             event_retention: None,
             quotas: Vec::new(),
-            dynamic_sampling: None,
+            sampling: None,
             measurements: None,
             breakdowns_v2: None,
             performance_score: Default::default(),
@@ -150,7 +154,7 @@ pub struct LimitedProjectConfig {
     #[serde(skip_serializing_if = "DataScrubbingConfig::is_disabled")]
     pub datascrubbing_settings: DataScrubbingConfig,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub dynamic_sampling: Option<SamplingConfig>,
+    pub sampling: Option<ErrorBoundary<SamplingConfig>>,
     #[serde(skip_serializing_if = "SessionMetricsConfig::is_disabled")]
     pub session_metrics: SessionMetricsConfig,
     #[serde(skip_serializing_if = "Option::is_none")]
diff --git a/relay-sampling/src/config.rs b/relay-sampling/src/config.rs
index 44376137a3..35f7a8586f 100644
--- a/relay-sampling/src/config.rs
+++ b/relay-sampling/src/config.rs
@@ -7,22 +7,36 @@ use serde::{Deserialize, Serialize};
 
 use relay_protocol::RuleCondition;
 
+/// Maximum supported version of dynamic sampling.
+///
+/// The version is an integer scalar, incremented by one on each new version:
+///  - 1: Initial version that uses `rules_v2`.
+///  - 2: Breaking change to `RuleConfig` and moves back to `rules`.
+const SAMPLING_CONFIG_VERSION: u16 = 2;
+
 /// Represents the dynamic sampling configuration available to a project.
 ///
 /// Note: This comes from the organization data
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct SamplingConfig {
+    /// The required version to run dynamic sampling.
+    ///
+    /// Defaults to legacy version (`1`) when missing.
+    #[serde(default = "SamplingConfig::legacy_version")]
+    pub version: u16,
+
     /// The ordered sampling rules for the project.
     ///
     /// This field will remain here to serve only for old customer Relays to which we will
     /// forward the sampling config. The idea is that those Relays will get the old rules as
     /// empty array, which will result in them not sampling and forwarding sampling decisions to
     /// upstream Relays.
-    #[serde(default, skip_deserializing)]
+    #[serde(default)]
     pub rules: Vec<SamplingRule>,
 
     /// The ordered sampling rules v2 for the project.
+    #[serde(default, skip_serializing)]
     pub rules_v2: Vec<SamplingRule>,
 
     /// Defines which population of items a dynamic sample rate applies to.
@@ -31,16 +45,42 @@ pub struct SamplingConfig {
 }
 
 impl SamplingConfig {
+    /// Creates an enabled configuration with empty defaults and the latest version.
+    pub fn new() -> Self {
+        Self::default()
+    }
+
     /// Returns `true` if any of the rules in this configuration is unsupported.
     pub fn unsupported(&self) -> bool {
-        !self.rules_v2.iter().all(SamplingRule::supported)
+        self.version > SAMPLING_CONFIG_VERSION || !self.rules.iter().all(SamplingRule::supported)
     }
 
     /// Filters the sampling rules by the given [`RuleType`].
     pub fn filter_rules(&self, rule_type: RuleType) -> impl Iterator<Item = &SamplingRule> {
-        self.rules_v2
-            .iter()
-            .filter(move |rule| rule.ty == rule_type)
+        self.rules.iter().filter(move |rule| rule.ty == rule_type)
+    }
+
+    /// Upgrades legacy sampling configs into the latest format.
+    pub fn normalize(&mut self) {
+        if self.version == Self::legacy_version() {
+            self.rules.append(&mut self.rules_v2);
+            self.version = SAMPLING_CONFIG_VERSION;
+        }
+    }
+
+    const fn legacy_version() -> u16 {
+        1
+    }
+}
+
+impl Default for SamplingConfig {
+    fn default() -> Self {
+        Self {
+            version: SAMPLING_CONFIG_VERSION,
+            rules: vec![],
+            rules_v2: vec![],
+            mode: SamplingMode::default(),
+        }
     }
 }
 
@@ -422,20 +462,9 @@ mod tests {
     }
 
     #[test]
-    fn test_sampling_config_with_rules_and_rules_v2_deserialization() {
+    fn test_legacy_deserialization() {
         let serialized_rule = r#"{
-               "rules": [
-                  {
-                     "sampleRate": 0.5,
-                     "type": "trace",
-                     "active": true,
-                     "condition": {
-                        "op": "and",
-                        "inner": []
-                     },
-                     "id": 1000
-                 }
-               ],
+               "rules": [],
                "rulesV2": [
                   {
                      "samplingValue":{
@@ -453,13 +482,15 @@ mod tests {
                ],
                "mode": "received"
         }"#;
-        let config: SamplingConfig = serde_json::from_str(serialized_rule).unwrap();
+        let mut config: SamplingConfig = serde_json::from_str(serialized_rule).unwrap();
+        config.normalize();
 
         // We want to make sure that we serialize an empty array of rule, irrespectively of the
         // received payload.
         assert!(config.rules.is_empty());
+        assert_eq!(config.version, SAMPLING_CONFIG_VERSION);
         assert_eq!(
-            config.rules_v2[0].sampling_value,
+            config.rules[0].sampling_value,
             SamplingValue::SampleRate { value: 0.5 }
         );
     }
@@ -467,8 +498,7 @@ mod tests {
     #[test]
     fn test_sampling_config_with_rules_and_rules_v2_serialization() {
         let config = SamplingConfig {
-            rules: vec![],
-            rules_v2: vec![SamplingRule {
+            rules: vec![SamplingRule {
                 condition: RuleCondition::all(),
                 sampling_value: SamplingValue::Factor { value: 2.0 },
                 ty: RuleType::Transaction,
@@ -476,13 +506,13 @@ mod tests {
                 time_range: Default::default(),
                 decaying_fn: Default::default(),
             }],
-            mode: SamplingMode::Received,
+            ..SamplingConfig::new()
         };
 
         let serialized_config = serde_json::to_string_pretty(&config).unwrap();
         let expected_serialized_config = r#"{
-  "rules": [],
-  "rulesV2": [
+  "version": 2,
+  "rules": [
     {
       "condition": {
         "op": "and",
diff --git a/relay-sampling/tests/fixtures/sampling_config.json b/relay-sampling/tests/fixtures/sampling_config.json
index e9b8b01f2e..a0d707148c 100644
--- a/relay-sampling/tests/fixtures/sampling_config.json
+++ b/relay-sampling/tests/fixtures/sampling_config.json
@@ -1,6 +1,6 @@
 {
-  "rules": [],
-  "rulesV2": [
+  "version": 2,
+  "rules": [
     {
       "condition": {
         "op": "and",
@@ -46,4 +46,4 @@
     }
   ],
   "mode": "total"
-}
\ No newline at end of file
+}
diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs
index a066494348..1687ac8225 100644
--- a/relay-server/src/actors/processor.rs
+++ b/relay-server/src/actors/processor.rs
@@ -2508,27 +2508,35 @@ impl EnvelopeProcessorService {
         // - Tagging whether an incoming error has a sampled trace connected to it.
         // - Computing the actual sampling decision on an incoming transaction.
         match state.event_type().unwrap_or_default() {
-            EventType::Default | EventType::Error => self.tag_error_with_sampling_decision(state),
+            EventType::Default | EventType::Error => {
+                self.tag_error_with_sampling_decision(state);
+            }
             EventType::Transaction => {
-                if let Some(ErrorBoundary::Ok(config)) =
-                    &state.project_state.config.transaction_metrics
-                {
-                    if config.is_enabled() {
-                        state.sampling_result = Self::compute_sampling_decision(
-                            self.inner.config.processing_enabled(),
-                            &state.reservoir,
-                            state.project_state.config.dynamic_sampling.as_ref(),
-                            state.event.value(),
-                            state
-                                .sampling_project_state
-                                .as_ref()
-                                .and_then(|state| state.config.dynamic_sampling.as_ref()),
-                            state.envelope().dsc(),
-                        );
-                    }
+                match state.project_state.config.transaction_metrics {
+                    Some(ErrorBoundary::Ok(ref c)) if c.is_enabled() => (),
+                    _ => return,
                 }
-            }
 
+                let sampling_config = match state.project_state.config.sampling {
+                    Some(ErrorBoundary::Ok(ref config)) if !config.unsupported() => Some(config),
+                    _ => None,
+                };
+
+                let root_state = state.sampling_project_state.as_ref();
+                let root_config = match root_state.and_then(|s| s.config.sampling.as_ref()) {
+                    Some(ErrorBoundary::Ok(ref config)) if !config.unsupported() => Some(config),
+                    _ => None,
+                };
+
+                state.sampling_result = Self::compute_sampling_decision(
+                    self.inner.config.processing_enabled(),
+                    &state.reservoir,
+                    sampling_config,
+                    state.event.value(),
+                    root_config,
+                    state.envelope().dsc(),
+                );
+            }
             _ => {}
         }
     }
@@ -2606,24 +2614,28 @@ impl EnvelopeProcessorService {
     /// This execution of dynamic sampling is technically a "simulation" since we will use the result
     /// only for tagging errors and not for actually sampling incoming events.
     fn tag_error_with_sampling_decision(&self, state: &mut ProcessEnvelopeState) {
-        if state.event.is_empty() {
-            return;
-        }
-
-        let (Some(config), Some(dsc)) = (
-            state
-                .sampling_project_state
-                .as_deref()
-                .and_then(|state| state.config.dynamic_sampling.as_ref()),
-            state.envelope().dsc(),
+        let (Some(dsc), Some(event)) = (
+            state.managed_envelope.envelope().dsc(),
+            state.event.value_mut(),
         ) else {
             return;
         };
 
-        let sampled =
-            utils::is_trace_fully_sampled(self.inner.config.processing_enabled(), config, dsc);
+        let root_state = state.sampling_project_state.as_ref();
+        let config = match root_state.and_then(|s| s.config.sampling.as_ref()) {
+            Some(ErrorBoundary::Ok(ref config)) => config,
+            _ => return,
+        };
+
+        if config.unsupported() {
+            if self.inner.config.processing_enabled() {
+                relay_log::error!("found unsupported rules even as processing relay");
+            }
 
-        let (Some(event), Some(sampled)) = (state.event.value_mut(), sampled) else {
+            return;
+        }
+
+        let Some(sampled) = utils::is_trace_fully_sampled(config, dsc) else {
             return;
         };
 
@@ -3401,8 +3413,7 @@ mod tests {
 
         for (sample_rate, should_keep) in [(0.0, false), (1.0, true)] {
             let sampling_config = SamplingConfig {
-                rules: vec![],
-                rules_v2: vec![SamplingRule {
+                rules: vec![SamplingRule {
                     condition: RuleCondition::all(),
                     sampling_value: SamplingValue::SampleRate { value: sample_rate },
                     ty: RuleType::Transaction,
@@ -3410,7 +3421,7 @@ mod tests {
                     time_range: Default::default(),
                     decaying_fn: DecayingFunction::Constant,
                 }],
-                mode: SamplingMode::Received,
+                ..SamplingConfig::new()
             };
 
             // TODO: This does not test if the sampling decision is actually applied. This should be
@@ -3668,8 +3679,7 @@ mod tests {
 
     fn project_state_with_single_rule(sample_rate: f64) -> ProjectState {
         let sampling_config = SamplingConfig {
-            rules: vec![],
-            rules_v2: vec![SamplingRule {
+            rules: vec![SamplingRule {
                 condition: RuleCondition::all(),
                 sampling_value: SamplingValue::SampleRate { value: sample_rate },
                 ty: RuleType::Trace,
@@ -3677,10 +3687,11 @@ mod tests {
                 time_range: Default::default(),
                 decaying_fn: Default::default(),
             }],
-            mode: SamplingMode::Received,
+            ..SamplingConfig::new()
         };
+
         let mut sampling_project_state = ProjectState::allowed();
-        sampling_project_state.config.dynamic_sampling = Some(sampling_config);
+        sampling_project_state.config.sampling = Some(ErrorBoundary::Ok(sampling_config));
         sampling_project_state
     }
 
@@ -4319,9 +4330,8 @@ mod tests {
         };
 
         let sampling_config = SamplingConfig {
-            rules: vec![],
-            rules_v2: vec![rule],
-            mode: SamplingMode::Received,
+            rules: vec![rule],
+            ..SamplingConfig::new()
         };
 
         let res = EnvelopeProcessorService::compute_sampling_decision(
@@ -4357,9 +4367,8 @@ mod tests {
         };
 
         let sampling_config = SamplingConfig {
-            rules: vec![],
-            rules_v2: vec![rule, unsupported_rule],
-            mode: SamplingMode::Received,
+            rules: vec![rule, unsupported_rule],
+            ..SamplingConfig::new()
         };
 
         // Unsupported rule should result in no match if processing is not enabled.
@@ -4410,9 +4419,8 @@ mod tests {
         };
 
         let mut sampling_config = SamplingConfig {
-            rules: vec![],
-            rules_v2: vec![rule],
-            mode: SamplingMode::Received,
+            rules: vec![rule],
+            ..SamplingConfig::new()
         };
 
         let res = EnvelopeProcessorService::compute_sampling_decision(
diff --git a/relay-server/src/actors/project.rs b/relay-server/src/actors/project.rs
index 2df5eeb6f6..63499b6767 100644
--- a/relay-server/src/actors/project.rs
+++ b/relay-server/src/actors/project.rs
@@ -483,13 +483,13 @@ impl Project {
             return;
         };
 
-        let Some(config) = state.config.dynamic_sampling.as_ref() else {
+        let Some(ErrorBoundary::Ok(config)) = state.config.sampling.as_ref() else {
             return;
         };
 
         // Using try_lock to not slow down the project cache service.
         if let Ok(mut guard) = self.reservoir_counters.try_lock() {
-            guard.retain(|key, _| config.rules_v2.iter().any(|rule| rule.id == *key));
+            guard.retain(|key, _| config.rules.iter().any(|rule| rule.id == *key));
         }
     }
 
diff --git a/relay-server/src/testutils.rs b/relay-server/src/testutils.rs
index 8c4de7ba08..0808826285 100644
--- a/relay-server/src/testutils.rs
+++ b/relay-server/src/testutils.rs
@@ -1,9 +1,8 @@
 use bytes::Bytes;
+use relay_dynamic_config::ErrorBoundary;
 use relay_event_schema::protocol::EventId;
 use relay_protocol::RuleCondition;
-use relay_sampling::config::{
-    DecayingFunction, RuleId, RuleType, SamplingMode, SamplingRule, SamplingValue,
-};
+use relay_sampling::config::{DecayingFunction, RuleId, RuleType, SamplingRule, SamplingValue};
 use relay_sampling::{DynamicSamplingContext, SamplingConfig};
 
 use crate::actors::project::ProjectState;
@@ -27,16 +26,11 @@ pub fn state_with_rule_and_condition(
         None => Vec::new(),
     };
 
-    project_state_with_config(SamplingConfig {
-        rules: vec![],
-        rules_v2: rules,
-        mode: SamplingMode::Received,
-    })
-}
-
-pub fn project_state_with_config(sampling_config: SamplingConfig) -> ProjectState {
     let mut state = ProjectState::allowed();
-    state.config.dynamic_sampling = Some(sampling_config);
+    state.config.sampling = Some(ErrorBoundary::Ok(SamplingConfig {
+        rules,
+        ..SamplingConfig::new()
+    }));
     state
 }
 
diff --git a/relay-server/src/utils/dynamic_sampling.rs b/relay-server/src/utils/dynamic_sampling.rs
index ead0b83b25..ad43ecfdd6 100644
--- a/relay-server/src/utils/dynamic_sampling.rs
+++ b/relay-server/src/utils/dynamic_sampling.rs
@@ -65,7 +65,6 @@ impl From<ControlFlow<SamplingMatch, SamplingEvaluator<'_>>> for SamplingResult
 /// transactions received with such dsc and project state would be kept or dropped by dynamic
 /// sampling.
 pub fn is_trace_fully_sampled(
-    processing_enabled: bool,
     root_project_config: &SamplingConfig,
     dsc: &DynamicSamplingContext,
 ) -> Option<bool> {
@@ -77,14 +76,6 @@ pub fn is_trace_fully_sampled(
         return Some(false);
     }
 
-    if root_project_config.unsupported() {
-        if processing_enabled {
-            relay_log::error!("found unsupported rules even as processing relay");
-        } else {
-            return Some(true);
-        }
-    }
-
     let adjustment_rate = match root_project_config.mode {
         SamplingMode::Total => dsc.sample_rate,
         _ => None,
@@ -160,9 +151,7 @@ mod tests {
     use relay_event_schema::protocol::{Event, EventId, LenientString};
     use relay_protocol::Annotated;
     use relay_protocol::RuleCondition;
-    use relay_sampling::config::{
-        RuleId, RuleType, SamplingConfig, SamplingMode, SamplingRule, SamplingValue,
-    };
+    use relay_sampling::config::{RuleId, RuleType, SamplingConfig, SamplingRule, SamplingValue};
     use uuid::Uuid;
 
     fn mocked_event(event_type: EventType, transaction: &str, release: &str) -> Event {
@@ -278,21 +267,17 @@ mod tests {
     #[test]
     fn test_is_trace_fully_sampled_return_true_with_unsupported_rules() {
         let config = SamplingConfig {
-            rules: vec![],
-            rules_v2: vec![
+            rules: vec![
                 mocked_sampling_rule(1, RuleType::Unsupported, 1.0),
                 mocked_sampling_rule(1, RuleType::Trace, 0.0),
             ],
-            mode: SamplingMode::Received,
+            ..SamplingConfig::new()
         };
 
         let dsc = mocked_simple_dynamic_sampling_context(None, None, None, None, Some(true));
 
-        // Return true if any unsupported rules.
-        assert_eq!(is_trace_fully_sampled(false, &config, &dsc), Some(true));
-
         // If processing is enabled, we simply log an error and otherwise proceed as usual.
-        assert_eq!(is_trace_fully_sampled(true, &config, &dsc), Some(false));
+        assert_eq!(is_trace_fully_sampled(&config, &dsc), Some(false));
     }
 
     #[test]
@@ -301,41 +286,38 @@ mod tests {
         // We test with `sampled = true` and 100% rule.
 
         let config = SamplingConfig {
-            rules: vec![],
-            rules_v2: vec![mocked_sampling_rule(1, RuleType::Trace, 1.0)],
-            mode: SamplingMode::Received,
+            rules: vec![mocked_sampling_rule(1, RuleType::Trace, 1.0)],
+            ..SamplingConfig::new()
         };
 
         let dsc =
             mocked_simple_dynamic_sampling_context(Some(1.0), Some("3.0"), None, None, Some(true));
 
-        let result = is_trace_fully_sampled(false, &config, &dsc).unwrap();
+        let result = is_trace_fully_sampled(&config, &dsc).unwrap();
         assert!(result);
 
         // We test with `sampled = true` and 0% rule.
         let config = SamplingConfig {
-            rules: vec![],
-            rules_v2: vec![mocked_sampling_rule(1, RuleType::Trace, 0.0)],
-            mode: SamplingMode::Received,
+            rules: vec![mocked_sampling_rule(1, RuleType::Trace, 0.0)],
+            ..SamplingConfig::new()
         };
 
         let dsc =
             mocked_simple_dynamic_sampling_context(Some(1.0), Some("3.0"), None, None, Some(true));
 
-        let result = is_trace_fully_sampled(false, &config, &dsc).unwrap();
+        let result = is_trace_fully_sampled(&config, &dsc).unwrap();
         assert!(!result);
 
         // We test with `sampled = false` and 100% rule.
         let config = SamplingConfig {
-            rules: vec![],
-            rules_v2: vec![mocked_sampling_rule(1, RuleType::Trace, 1.0)],
-            mode: SamplingMode::Received,
+            rules: vec![mocked_sampling_rule(1, RuleType::Trace, 1.0)],
+            ..SamplingConfig::new()
         };
 
         let dsc =
             mocked_simple_dynamic_sampling_context(Some(1.0), Some("3.0"), None, None, Some(false));
 
-        let result = is_trace_fully_sampled(false, &config, &dsc).unwrap();
+        let result = is_trace_fully_sampled(&config, &dsc).unwrap();
         assert!(!result);
     }
 
@@ -344,13 +326,12 @@ mod tests {
     fn test_is_trace_fully_sampled_with_invalid_inputs() {
         // We test with missing `sampled`.
         let config = SamplingConfig {
-            rules: vec![],
-            rules_v2: vec![mocked_sampling_rule(1, RuleType::Trace, 1.0)],
-            mode: SamplingMode::Received,
+            rules: vec![mocked_sampling_rule(1, RuleType::Trace, 1.0)],
+            ..SamplingConfig::new()
         };
         let dsc = mocked_simple_dynamic_sampling_context(Some(1.0), Some("3.0"), None, None, None);
 
-        let result = is_trace_fully_sampled(false, &config, &dsc);
+        let result = is_trace_fully_sampled(&config, &dsc);
         assert!(result.is_none());
     }
 }
diff --git a/tests/integration/test_dynamic_sampling.py b/tests/integration/test_dynamic_sampling.py
index 1bb337f361..e962f0002d 100644
--- a/tests/integration/test_dynamic_sampling.py
+++ b/tests/integration/test_dynamic_sampling.py
@@ -89,10 +89,9 @@ def _add_sampling_config(
     Adds a sampling configuration rule to a project configuration
     """
     ds = config["config"].setdefault("dynamicSampling", {})
-    # We set the old rules v1 as empty array.
-    ds.setdefault("rules", [])
-    # We set the new rules v2 as empty array, and we add rules to it.
-    rules = ds.setdefault("rulesV2", [])
+    ds.setdefault("version", 2)
+    # We set the rules as empty array, and we add rules to it.
+    rules = ds.setdefault("rules", [])
 
     if rule_type is None:
         rule_type = "trace"
diff --git a/tests/integration/test_healthchecks.py b/tests/integration/test_healthchecks.py
index 2f2c07b6d1..d9938158eb 100644
--- a/tests/integration/test_healthchecks.py
+++ b/tests/integration/test_healthchecks.py
@@ -115,8 +115,8 @@ def store_internal_error_event():
         # Set the broken config, so we won't be able to dequeue the envelopes.
         config = mini_sentry.project_configs[project_key]["config"]
         ds = config.setdefault("dynamicSampling", {})
-        ds.setdefault("rules", [])
-        ds.setdefault("rulesV2", []).append(
+        ds.setdefault("version", 2)
+        ds.setdefault("rules", []).append(
             {
                 "condition": {
                     "op": "and",
diff --git a/tests/integration/test_metrics.py b/tests/integration/test_metrics.py
index e0ebaa4ffb..e16419df1a 100644
--- a/tests/integration/test_metrics.py
+++ b/tests/integration/test_metrics.py
@@ -590,8 +590,8 @@ def test_transaction_metrics(
     if discard_data:
         # Make sure Relay drops the transaction
         ds = config.setdefault("dynamicSampling", {})
-        ds.setdefault("rules", [])
-        ds.setdefault("rulesV2", []).append(
+        ds.setdefault("version", 2)
+        ds.setdefault("rules", []).append(
             {
                 "samplingValue": {"type": "sampleRate", "value": 0.0},
                 "type": discard_data,
@@ -807,8 +807,8 @@ def test_transaction_metrics_extraction_external_relays(
     config = mini_sentry.project_configs[project_id]["config"]
     config["transactionMetrics"] = {"version": 3}
     config["dynamicSampling"] = {
-        "rules": [],
-        "rulesV2": [
+        "version": 2,
+        "rules": [
             {
                 "id": 1,
                 "samplingValue": {"type": "sampleRate", "value": 0.0},
@@ -1247,8 +1247,8 @@ def test_generic_metric_extraction(mini_sentry, relay):
     }
     config["transactionMetrics"] = {"version": 3}
     config["dynamicSampling"] = {
-        "rules": [],
-        "rulesV2": [
+        "version": 2,
+        "rules": [
             {
                 "id": 1,
                 "samplingValue": {"type": "sampleRate", "value": 0.0},
diff --git a/tests/integration/test_outcome.py b/tests/integration/test_outcome.py
index a3083e0811..dca6674670 100644
--- a/tests/integration/test_outcome.py
+++ b/tests/integration/test_outcome.py
@@ -733,8 +733,8 @@ def test_outcome_to_client_report(relay, mini_sentry):
     project_config = mini_sentry.add_full_project_config(project_id)
     project_config["config"]["transactionMetrics"] = {"version": 1}
     project_config["config"]["dynamicSampling"] = {
-        "rules": [],
-        "rulesV2": [
+        "version": 2,
+        "rules": [
             {
                 "id": 1,
                 "samplingValue": {"type": "sampleRate", "value": 0.0},
@@ -900,8 +900,8 @@ def test_outcomes_aggregate_dynamic_sampling(relay, mini_sentry):
     project_id = 42
     project_config = mini_sentry.add_full_project_config(project_id)
     project_config["config"]["dynamicSampling"] = {
-        "rules": [],
-        "rulesV2": [
+        "version": 2,
+        "rules": [
             {
                 "id": 1,
                 "samplingValue": {"type": "sampleRate", "value": 0.0},
@@ -1024,8 +1024,8 @@ def test_graceful_shutdown(relay, mini_sentry):
     project_config = mini_sentry.add_full_project_config(project_id)
     project_config["config"]["transactionMetrics"] = {"version": 1}
     project_config["config"]["dynamicSampling"] = {
-        "rules": [],
-        "rulesV2": [
+        "version": 2,
+        "rules": [
             {
                 "id": 1,
                 "samplingValue": {"type": "sampleRate", "value": 0.0},
@@ -1112,8 +1112,8 @@ def test_profile_outcomes(
         "version": 1,
     }
     project_config["dynamicSampling"] = {
-        "rules": [],
-        "rulesV2": [
+        "version": 2,
+        "rules": [
             {
                 "id": 1,
                 "samplingValue": {"type": "sampleRate", "value": 0.0},
diff --git a/tests/integration/test_projectconfigs.py b/tests/integration/test_projectconfigs.py
index 943e4637c4..1240250105 100644
--- a/tests/integration/test_projectconfigs.py
+++ b/tests/integration/test_projectconfigs.py
@@ -270,8 +270,8 @@ def test_unparsable_project_config(buffer_config, mini_sentry, relay):
     # Config is broken and will produce the invalid project state.
     config = mini_sentry.project_configs[project_key]["config"]
     ds = config.setdefault("dynamicSampling", {})
-    ds.setdefault("rules", [])
-    ds.setdefault("rulesV2", []).append(
+    ds.setdefault("version", 2)
+    ds.setdefault("rules", []).append(
         {
             "condition": {
                 "op": "and",
@@ -325,8 +325,8 @@ def test_unparsable_project_config(buffer_config, mini_sentry, relay):
 
     # Fix the config.
     config = mini_sentry.project_configs[project_key]["config"]
-    config["dynamicSampling"]["rules"] = []
-    config["dynamicSampling"]["rulesV2"] = [
+    config["dynamicSampling"]["version"] = 2
+    config["dynamicSampling"]["rules"] = [
         {
             "condition": {
                 "op": "and",
@@ -404,8 +404,8 @@ def test_cached_project_config(mini_sentry, relay):
     # Introduce unparsable config.
     config = mini_sentry.project_configs[project_key]["config"]
     ds = config.setdefault("dynamicSampling", {})
-    ds.setdefault("rules", [])
-    ds.setdefault("rulesV2", []).append(
+    ds.setdefault("version", 2)
+    ds.setdefault("rules", []).append(
         {
             "condition": {
                 "op": "and",

From 9bff63639edf5dccf3eea5e76739c5a108e7da75 Mon Sep 17 00:00:00 2001
From: Jan Michael Auer <mail@jauer.org>
Date: Tue, 14 Nov 2023 16:23:15 +0100
Subject: [PATCH 03/10] ref(dynamic-config): Bump metrics and sampling config
 versions

---
 relay-dynamic-config/src/metrics.rs | 3 ++-
 relay-sampling/src/config.rs        | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/relay-dynamic-config/src/metrics.rs b/relay-dynamic-config/src/metrics.rs
index 2d863fecd4..b95e4beea7 100644
--- a/relay-dynamic-config/src/metrics.rs
+++ b/relay-dynamic-config/src/metrics.rs
@@ -84,7 +84,8 @@ pub struct CustomMeasurementConfig {
 ///  - 3:
 ///      - Emit a `usage` metric and use it for rate limiting.
 ///      - Delay metrics extraction for indexed transactions.
-const TRANSACTION_EXTRACT_VERSION: u16 = 3;
+///  - 4: Adds support for `RuleConfigs` with string comparisons.
+const TRANSACTION_EXTRACT_VERSION: u16 = 4;
 
 /// Deprecated. Defines whether URL transactions should be considered low cardinality.
 #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq)]
diff --git a/relay-sampling/src/config.rs b/relay-sampling/src/config.rs
index 35f7a8586f..d9a2d6c112 100644
--- a/relay-sampling/src/config.rs
+++ b/relay-sampling/src/config.rs
@@ -11,7 +11,7 @@ use relay_protocol::RuleCondition;
 ///
 /// The version is an integer scalar, incremented by one on each new version:
 ///  - 1: Initial version that uses `rules_v2`.
-///  - 2: Breaking change to `RuleConfig` and moves back to `rules`.
+///  - 2: Moves back to `rules` and adds support for `RuleConfigs` with string comparisons.
 const SAMPLING_CONFIG_VERSION: u16 = 2;
 
 /// Represents the dynamic sampling configuration available to a project.

From 761e366a83e8f843c35037609eb633ffc2bbdaba Mon Sep 17 00:00:00 2001
From: Jan Michael Auer <mail@jauer.org>
Date: Tue, 14 Nov 2023 16:33:02 +0100
Subject: [PATCH 04/10] fix(py): Test name

---
 py/tests/test_processing.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/py/tests/test_processing.py b/py/tests/test_processing.py
index 9b32da1a13..ad0c928b1c 100644
--- a/py/tests/test_processing.py
+++ b/py/tests/test_processing.py
@@ -288,7 +288,7 @@ def test_validate_legacy_sampling_configuration():
     sentry_relay.validate_sampling_configuration(config)
 
 
-def test_validate_legacy_sampling_configuration():
+def test_validate_sampling_configuration():
     """
     Tests that a valid sampling rule configuration passes
     """

From 636d382281963b935d6953eb225ab146be3e2839 Mon Sep 17 00:00:00 2001
From: Jan Michael Auer <mail@jauer.org>
Date: Tue, 14 Nov 2023 16:44:15 +0100
Subject: [PATCH 05/10] fix: Broken tests

---
 py/tests/test_processing.py  | 11 ++++++-----
 relay-sampling/src/config.rs |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/py/tests/test_processing.py b/py/tests/test_processing.py
index ad0c928b1c..4352b7aaf2 100644
--- a/py/tests/test_processing.py
+++ b/py/tests/test_processing.py
@@ -299,14 +299,15 @@ def test_validate_sampling_configuration():
                 "type": "trace",
                 "samplingValue": {
                     "type": "sampleRate",
-                    "value": 0.7
+                    "value": 0.9
                 },
                 "condition": {
-                    "op": "custom",
-                    "name": "event.legacy_browser",
-                    "value":["ie10"]
+                    "op": "eq",
+                    "name": "event.release",
+                    "value":["1.1.*"],
+                    "options": {"ignoreCase": true}
                 },
-                "id":1
+                "id":2
             }
         ]
     }"""
diff --git a/relay-sampling/src/config.rs b/relay-sampling/src/config.rs
index d9a2d6c112..43f2e35bc8 100644
--- a/relay-sampling/src/config.rs
+++ b/relay-sampling/src/config.rs
@@ -487,12 +487,12 @@ mod tests {
 
         // We want to make sure that we serialize an empty array of rule, irrespectively of the
         // received payload.
-        assert!(config.rules.is_empty());
         assert_eq!(config.version, SAMPLING_CONFIG_VERSION);
         assert_eq!(
             config.rules[0].sampling_value,
             SamplingValue::SampleRate { value: 0.5 }
         );
+        assert!(config.rules_v2.is_empty());
     }
 
     #[test]

From b94bfe7f16e0e505a33fe0915e724afee3698266 Mon Sep 17 00:00:00 2001
From: Jan Michael Auer <mail@jauer.org>
Date: Wed, 15 Nov 2023 09:53:57 +0100
Subject: [PATCH 06/10] test(integration): Move sampling config

---
 tests/integration/test_dynamic_sampling.py | 2 +-
 tests/integration/test_healthchecks.py     | 2 +-
 tests/integration/test_metrics.py          | 6 +++---
 tests/integration/test_outcome.py          | 8 ++++----
 tests/integration/test_projectconfigs.py   | 8 ++++----
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/tests/integration/test_dynamic_sampling.py b/tests/integration/test_dynamic_sampling.py
index e962f0002d..db8d81a91c 100644
--- a/tests/integration/test_dynamic_sampling.py
+++ b/tests/integration/test_dynamic_sampling.py
@@ -88,7 +88,7 @@ def _add_sampling_config(
     """
     Adds a sampling configuration rule to a project configuration
     """
-    ds = config["config"].setdefault("dynamicSampling", {})
+    ds = config["config"].setdefault("sampling", {})
     ds.setdefault("version", 2)
     # We set the rules as empty array, and we add rules to it.
     rules = ds.setdefault("rules", [])
diff --git a/tests/integration/test_healthchecks.py b/tests/integration/test_healthchecks.py
index d9938158eb..07a597be48 100644
--- a/tests/integration/test_healthchecks.py
+++ b/tests/integration/test_healthchecks.py
@@ -114,7 +114,7 @@ def store_internal_error_event():
         mini_sentry.add_full_project_config(project_key)
         # Set the broken config, so we won't be able to dequeue the envelopes.
         config = mini_sentry.project_configs[project_key]["config"]
-        ds = config.setdefault("dynamicSampling", {})
+        ds = config.setdefault("sampling", {})
         ds.setdefault("version", 2)
         ds.setdefault("rules", []).append(
             {
diff --git a/tests/integration/test_metrics.py b/tests/integration/test_metrics.py
index e16419df1a..a81c16425c 100644
--- a/tests/integration/test_metrics.py
+++ b/tests/integration/test_metrics.py
@@ -589,7 +589,7 @@ def test_transaction_metrics(
 
     if discard_data:
         # Make sure Relay drops the transaction
-        ds = config.setdefault("dynamicSampling", {})
+        ds = config.setdefault("sampling", {})
         ds.setdefault("version", 2)
         ds.setdefault("rules", []).append(
             {
@@ -806,7 +806,7 @@ def test_transaction_metrics_extraction_external_relays(
     mini_sentry.add_full_project_config(project_id)
     config = mini_sentry.project_configs[project_id]["config"]
     config["transactionMetrics"] = {"version": 3}
-    config["dynamicSampling"] = {
+    config["sampling"] = {
         "version": 2,
         "rules": [
             {
@@ -1246,7 +1246,7 @@ def test_generic_metric_extraction(mini_sentry, relay):
         ],
     }
     config["transactionMetrics"] = {"version": 3}
-    config["dynamicSampling"] = {
+    config["sampling"] = {
         "version": 2,
         "rules": [
             {
diff --git a/tests/integration/test_outcome.py b/tests/integration/test_outcome.py
index dca6674670..d06c98161e 100644
--- a/tests/integration/test_outcome.py
+++ b/tests/integration/test_outcome.py
@@ -732,7 +732,7 @@ def test_outcome_to_client_report(relay, mini_sentry):
     project_id = 42
     project_config = mini_sentry.add_full_project_config(project_id)
     project_config["config"]["transactionMetrics"] = {"version": 1}
-    project_config["config"]["dynamicSampling"] = {
+    project_config["config"]["sampling"] = {
         "version": 2,
         "rules": [
             {
@@ -899,7 +899,7 @@ def test_outcomes_aggregate_dynamic_sampling(relay, mini_sentry):
     # Create project config
     project_id = 42
     project_config = mini_sentry.add_full_project_config(project_id)
-    project_config["config"]["dynamicSampling"] = {
+    project_config["config"]["sampling"] = {
         "version": 2,
         "rules": [
             {
@@ -1023,7 +1023,7 @@ def test_graceful_shutdown(relay, mini_sentry):
     project_id = 42
     project_config = mini_sentry.add_full_project_config(project_id)
     project_config["config"]["transactionMetrics"] = {"version": 1}
-    project_config["config"]["dynamicSampling"] = {
+    project_config["config"]["sampling"] = {
         "version": 2,
         "rules": [
             {
@@ -1111,7 +1111,7 @@ def test_profile_outcomes(
     project_config["transactionMetrics"] = {
         "version": 1,
     }
-    project_config["dynamicSampling"] = {
+    project_config["sampling"] = {
         "version": 2,
         "rules": [
             {
diff --git a/tests/integration/test_projectconfigs.py b/tests/integration/test_projectconfigs.py
index 1240250105..e5fd4624bd 100644
--- a/tests/integration/test_projectconfigs.py
+++ b/tests/integration/test_projectconfigs.py
@@ -269,7 +269,7 @@ def test_unparsable_project_config(buffer_config, mini_sentry, relay):
 
     # Config is broken and will produce the invalid project state.
     config = mini_sentry.project_configs[project_key]["config"]
-    ds = config.setdefault("dynamicSampling", {})
+    ds = config.setdefault("sampling", {})
     ds.setdefault("version", 2)
     ds.setdefault("rules", []).append(
         {
@@ -325,8 +325,8 @@ def test_unparsable_project_config(buffer_config, mini_sentry, relay):
 
     # Fix the config.
     config = mini_sentry.project_configs[project_key]["config"]
-    config["dynamicSampling"]["version"] = 2
-    config["dynamicSampling"]["rules"] = [
+    config["sampling"]["version"] = 2
+    config["sampling"]["rules"] = [
         {
             "condition": {
                 "op": "and",
@@ -403,7 +403,7 @@ def test_cached_project_config(mini_sentry, relay):
 
     # Introduce unparsable config.
     config = mini_sentry.project_configs[project_key]["config"]
-    ds = config.setdefault("dynamicSampling", {})
+    ds = config.setdefault("sampling", {})
     ds.setdefault("version", 2)
     ds.setdefault("rules", []).append(
         {

From a910d4bdf38f1be263aae0ae5b14194775d883e4 Mon Sep 17 00:00:00 2001
From: Jan Michael Auer <mail@jauer.org>
Date: Wed, 15 Nov 2023 09:55:43 +0100
Subject: [PATCH 07/10] meta: Changelog

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 66e0ba6dba..7f2373b3fa 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -33,6 +33,7 @@
 - Scrub all DB Core Data spans differently. ([#2686](https://github.com/getsentry/relay/pull/2686))
 - Support generic metrics extraction version 2. ([#2692](https://github.com/getsentry/relay/pull/2692))
 - Emit error on continued project config fetch failures after a time interval. ([#2700](https://github.com/getsentry/relay/pull/2700))
+- Support comparison operators (`>`, `>=`, `<`, `<=`) for strings in dynamic sampling and metric extraction rules. Previously, these comparisons were only possible on numbers. ([#2721](https://github.com/getsentry/relay/pull/2721))
 
 ## 23.10.1
 

From 55912c36db4926aa5dcf2d275e864ae06a3a47fb Mon Sep 17 00:00:00 2001
From: Jan Michael Auer <mail@jauer.org>
Date: Wed, 15 Nov 2023 12:54:42 +0100
Subject: [PATCH 08/10] ref: Address review feedback

---
 py/tests/test_processing.py  | 12 ++++++------
 relay-cabi/include/relay.h   | 24 ++++++++++++++++--------
 relay-sampling/src/config.rs | 13 +++++++------
 3 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/py/tests/test_processing.py b/py/tests/test_processing.py
index 4352b7aaf2..926df40e52 100644
--- a/py/tests/test_processing.py
+++ b/py/tests/test_processing.py
@@ -264,9 +264,9 @@ def test_validate_legacy_sampling_configuration():
                 "condition": {
                     "op": "custom",
                     "name": "event.legacy_browser",
-                    "value":["ie10"]
+                    "value": ["ie10"]
                 },
-                "id":1
+                "id": 1
             },
             {
                 "type": "trace",
@@ -277,10 +277,10 @@ def test_validate_legacy_sampling_configuration():
                 "condition": {
                     "op": "eq",
                     "name": "event.release",
-                    "value":["1.1.*"],
+                    "value": ["1.1.*"],
                     "options": {"ignoreCase": true}
                 },
-                "id":2
+                "id": 2
             }
         ]
     }"""
@@ -304,10 +304,10 @@ def test_validate_sampling_configuration():
                 "condition": {
                     "op": "eq",
                     "name": "event.release",
-                    "value":["1.1.*"],
+                    "value": ["1.1.*"],
                     "options": {"ignoreCase": true}
                 },
-                "id":2
+                "id": 2
             }
         ]
     }"""
diff --git a/relay-cabi/include/relay.h b/relay-cabi/include/relay.h
index 132d27d0d9..ba149a5e09 100644
--- a/relay-cabi/include/relay.h
+++ b/relay-cabi/include/relay.h
@@ -15,7 +15,8 @@
 /**
  * Classifies the type of data that is being ingested.
  */
-enum RelayDataCategory {
+enum RelayDataCategory
+{
   /**
    * Reserved and unused.
    */
@@ -106,7 +107,8 @@ typedef int8_t RelayDataCategory;
 /**
  * Controls the globbing behaviors.
  */
-enum GlobFlags {
+enum GlobFlags
+{
   /**
    * When enabled `**` matches over path separators and `*` does not.
    */
@@ -129,7 +131,8 @@ typedef uint32_t GlobFlags;
 /**
  * Represents all possible error codes.
  */
-enum RelayErrorCode {
+enum RelayErrorCode
+{
   RELAY_ERROR_CODE_NO_ERROR = 0,
   RELAY_ERROR_CODE_PANIC = 1,
   RELAY_ERROR_CODE_UNKNOWN = 2,
@@ -154,7 +157,8 @@ typedef uint32_t RelayErrorCode;
  * Values from <https://github.com/open-telemetry/opentelemetry-specification/blob/8fb6c14e4709e75a9aaa64b0dbbdf02a6067682a/specification/api-tracing.md#status>
  * Mapping to HTTP from <https://github.com/open-telemetry/opentelemetry-specification/blob/8fb6c14e4709e75a9aaa64b0dbbdf02a6067682a/specification/data-http.md#status>
  */
-enum RelaySpanStatus {
+enum RelaySpanStatus
+{
   /**
    * The operation completed successfully.
    *
@@ -279,7 +283,8 @@ typedef struct RelayStoreNormalizer RelayStoreNormalizer;
  *  - When obtained as instance through return values, always free the string.
  *  - When obtained as pointer through field access, never free the string.
  */
-typedef struct RelayStr {
+typedef struct RelayStr
+{
   /**
    * Pointer to the UTF-8 encoded string data.
    */
@@ -303,7 +308,8 @@ typedef struct RelayStr {
  *  - When obtained as instance through return values, always free the buffer.
  *  - When obtained as pointer through field access, never free the buffer.
  */
-typedef struct RelayBuf {
+typedef struct RelayBuf
+{
   /**
    * Pointer to the raw data.
    */
@@ -321,7 +327,8 @@ typedef struct RelayBuf {
 /**
  * Represents a key pair from key generation.
  */
-typedef struct RelayKeyPair {
+typedef struct RelayKeyPair
+{
   /**
    * The public key used for verifying Relay signatures.
    */
@@ -335,7 +342,8 @@ typedef struct RelayKeyPair {
 /**
  * A 16-byte UUID.
  */
-typedef struct RelayUuid {
+typedef struct RelayUuid
+{
   /**
    * UUID bytes in network byte order (big endian).
    */
diff --git a/relay-sampling/src/config.rs b/relay-sampling/src/config.rs
index 43f2e35bc8..dcb27be8b4 100644
--- a/relay-sampling/src/config.rs
+++ b/relay-sampling/src/config.rs
@@ -27,15 +27,15 @@ pub struct SamplingConfig {
     pub version: u16,
 
     /// The ordered sampling rules for the project.
-    ///
-    /// This field will remain here to serve only for old customer Relays to which we will
-    /// forward the sampling config. The idea is that those Relays will get the old rules as
-    /// empty array, which will result in them not sampling and forwarding sampling decisions to
-    /// upstream Relays.
     #[serde(default)]
     pub rules: Vec<SamplingRule>,
 
-    /// The ordered sampling rules v2 for the project.
+    /// **Deprecated**. The ordered sampling rules for the project in legacy format.
+    ///
+    /// Removed in favor of `Self::rules` in version `2`. This field remains here to parse rules
+    /// from old Sentry instances and convert them into the new format. The legacy format contained
+    /// both an empty `rules` as well as the actual rules in `rules_v2`. During normalization, these
+    /// two arrays are merged together.
     #[serde(default, skip_serializing)]
     pub rules_v2: Vec<SamplingRule>,
 
@@ -52,6 +52,7 @@ impl SamplingConfig {
 
     /// Returns `true` if any of the rules in this configuration is unsupported.
     pub fn unsupported(&self) -> bool {
+        debug_assert!(self.version > 1, "SamplingConfig not normalized");
         self.version > SAMPLING_CONFIG_VERSION || !self.rules.iter().all(SamplingRule::supported)
     }
 

From cb7db8e8a0db78ba5464204e33474ace73b62d0d Mon Sep 17 00:00:00 2001
From: Jan Michael Auer <mail@jauer.org>
Date: Wed, 15 Nov 2023 13:59:37 +0100
Subject: [PATCH 09/10] fix(integration): Update tests with simpler assertion

---
 tests/integration/test_projectconfigs.py | 74 +++---------------------
 1 file changed, 9 insertions(+), 65 deletions(-)

diff --git a/tests/integration/test_projectconfigs.py b/tests/integration/test_projectconfigs.py
index e5fd4624bd..068db7c0f1 100644
--- a/tests/integration/test_projectconfigs.py
+++ b/tests/integration/test_projectconfigs.py
@@ -268,27 +268,8 @@ def test_unparsable_project_config(buffer_config, mini_sentry, relay):
     public_key = mini_sentry.get_dsn_public_key(project_key)
 
     # Config is broken and will produce the invalid project state.
-    config = mini_sentry.project_configs[project_key]["config"]
-    ds = config.setdefault("sampling", {})
-    ds.setdefault("version", 2)
-    ds.setdefault("rules", []).append(
-        {
-            "condition": {
-                "op": "and",
-                "inner": [
-                    {"op": "glob", "name": "releases", "value": ["1.1.1", "1.1.2"]}
-                ],
-            },
-            "samplingValue": {"strategy": "sampleRate", "value": 0.7},
-            "type": "trace",
-            "id": 1,
-            "timeRange": {
-                "start": "2022-10-10T00:00:00.000000Z",
-                "end": "2022-10-20T00:00:00.000000Z",
-            },
-            "decayingFn": {"function": "linear", "decayedSampleRate": 0.9},
-        }
-    )
+    config = mini_sentry.project_configs[project_key]
+    config["slug"] = 99  # ERROR: Number not allowed
 
     body = {"publicKeys": [public_key]}
     packed, signature = SecretKey.parse(relay.secret_key).pack(body)
@@ -318,32 +299,14 @@ def test_unparsable_project_config(buffer_config, mini_sentry, relay):
         relay.send_event(project_key)
         assert mini_sentry.captured_events.empty()
         assert {str(e) for _, e in mini_sentry.test_failures} == {
-            f"Relay sent us event: error fetching project state {public_key}: missing field `type`",
+            f"Relay sent us event: error fetching project state {public_key}: invalid type: integer `99`, expected a string",
         }
     finally:
         mini_sentry.test_failures.clear()
 
     # Fix the config.
-    config = mini_sentry.project_configs[project_key]["config"]
-    config["sampling"]["version"] = 2
-    config["sampling"]["rules"] = [
-        {
-            "condition": {
-                "op": "and",
-                "inner": [
-                    {"op": "glob", "name": "releases", "value": ["1.1.1", "1.1.2"]}
-                ],
-            },
-            "samplingValue": {"type": "sampleRate", "value": 0.7},
-            "type": "trace",
-            "id": 1,
-            "timeRange": {
-                "start": "2022-10-10T00:00:00.000000Z",
-                "end": "2022-10-20T00:00:00.000000Z",
-            },
-            "decayingFn": {"type": "linear", "decayedValue": 0.9},
-        }
-    ]
+    config = mini_sentry.project_configs[project_key]
+    config["slug"] = "some-slug"
 
     try:
         relay.send_event(project_key)
@@ -353,7 +316,7 @@ def test_unparsable_project_config(buffer_config, mini_sentry, relay):
         assert not mini_sentry.test_failures or {
             str(e) for _, e in mini_sentry.test_failures
         } == {
-            f"Relay sent us event: error fetching project state {public_key}: missing field `type`",
+            f"Relay sent us event: error fetching project state {public_key}: invalid type: integer `99`, expected a string",
         }
     finally:
         mini_sentry.test_failures.clear()
@@ -402,27 +365,8 @@ def test_cached_project_config(mini_sentry, relay):
     assert not data["configs"][public_key]["disabled"]
 
     # Introduce unparsable config.
-    config = mini_sentry.project_configs[project_key]["config"]
-    ds = config.setdefault("sampling", {})
-    ds.setdefault("version", 2)
-    ds.setdefault("rules", []).append(
-        {
-            "condition": {
-                "op": "and",
-                "inner": [
-                    {"op": "glob", "name": "releases", "value": ["1.1.1", "1.1.2"]}
-                ],
-            },
-            "samplingValue": {"type": "sampleRate", "value": 0.7},
-            "type": "trace",
-            "id": 1,
-            "timeRange": {
-                "start": "2022-10-10T00:00:00.000000Z",
-                "end": "2022-10-20T00:00:00.000000Z",
-            },
-            "decayingFn": {"function": "linear", "decayedSampleRate": 0.9},
-        }
-    )
+    config = mini_sentry.project_configs[project_key]
+    config["slug"] = 99  # ERROR: Number not allowed
 
     # Give it a bit time for update to go through.
     time.sleep(1)
@@ -437,7 +381,7 @@ def test_cached_project_config(mini_sentry, relay):
         relay.send_event(project_key)
         time.sleep(0.5)
         assert {str(e) for _, e in mini_sentry.test_failures} == {
-            f"Relay sent us event: error fetching project state {public_key}: missing field `type`",
+            f"Relay sent us event: error fetching project state {public_key}: invalid type: integer `99`, expected a string",
         }
     finally:
         mini_sentry.test_failures.clear()

From 730328d8f17e65e06ea784dfdf549674ed12fee3 Mon Sep 17 00:00:00 2001
From: Jan Michael Auer <mail@jauer.org>
Date: Thu, 16 Nov 2023 12:59:03 +0100
Subject: [PATCH 10/10] meta: Changelog

---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 558619c903..6473e95458 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@
 **Internal**:
 
 - Support `device.model` in dynamic sampling and metric extraction. ([#2728](https://github.com/getsentry/relay/pull/2728))
+- Support comparison operators (`>`, `>=`, `<`, `<=`) for strings in dynamic sampling and metric extraction rules. Previously, these comparisons were only possible on numbers. ([#2730](https://github.com/getsentry/relay/pull/2730))
 
 ## 23.11.0
 
@@ -39,7 +40,6 @@
 - Scrub all DB Core Data spans differently. ([#2686](https://github.com/getsentry/relay/pull/2686))
 - Support generic metrics extraction version 2. ([#2692](https://github.com/getsentry/relay/pull/2692))
 - Emit error on continued project config fetch failures after a time interval. ([#2700](https://github.com/getsentry/relay/pull/2700))
-- Support comparison operators (`>`, `>=`, `<`, `<=`) for strings in dynamic sampling and metric extraction rules. Previously, these comparisons were only possible on numbers. ([#2721](https://github.com/getsentry/relay/pull/2721))
 
 ## 23.10.1