diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b2d787311..688fdc213b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Apply all configured data scrubbing rules on Replays. ([#1731](https://github.com/getsentry/relay/pull/1731)) - Add count transactions toward root project. ([#1734](https://github.com/getsentry/relay/pull/1734)) - Add or remove the profile ID on the transaction's profiling context. ([#1801](https://github.com/getsentry/relay/pull/1801)) +- Implement a new sampling algorithm with factors and multi-matching. ([#1790](https://github.com/getsentry/relay/pull/1790) **Bug Fixes**: diff --git a/Cargo.lock b/Cargo.lock index 02f4e61c16..b859fce924 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3598,6 +3598,7 @@ dependencies = [ "relay-log", "serde", "serde_json", + "similar-asserts", "unicase", ] @@ -3651,6 +3652,7 @@ dependencies = [ "serde", "serde_json", "serde_urlencoded 0.7.1", + "similar-asserts", "smallvec 1.10.0", "symbolic-common", "symbolic-unreal", diff --git a/py/tests/test_processing.py b/py/tests/test_processing.py index 6d69cf4479..130eaa5cb6 100644 --- a/py/tests/test_processing.py +++ b/py/tests/test_processing.py @@ -239,10 +239,14 @@ def test_validate_sampling_configuration(): Tests that a valid sampling rule configuration passes """ config = """{ - "rules": [ + "rules": [], + "rulesV2": [ { "type": "trace", - "sampleRate": 0.7, + "samplingValue": { + "type": "sampleRate", + "value": 0.7 + }, "condition": { "op": "custom", "name": "event.legacy_browser", @@ -252,7 +256,10 @@ def test_validate_sampling_configuration(): }, { "type": "trace", - "sampleRate": 0.9, + "samplingValue": { + "type": "sampleRate", + "value": 0.9 + }, "condition": { "op": "eq", "name": "event.release", diff --git a/relay-sampling/Cargo.toml b/relay-sampling/Cargo.toml index 2f39b62b6d..f745b85ac9 100644 --- a/relay-sampling/Cargo.toml +++ b/relay-sampling/Cargo.toml @@ -20,6 +20,7 @@ rand = "0.6.5" rand_pcg = "0.1.2" unicase = "2.6.0" chrono = "0.4.11" +similar-asserts = "1.4.2" [dev-dependencies] insta = { version = "1.19.0", features = ["ron"] } diff --git a/relay-sampling/src/lib.rs b/relay-sampling/src/lib.rs index cc7c35b691..667c0d9b79 100644 --- a/relay-sampling/src/lib.rs +++ b/relay-sampling/src/lib.rs @@ -10,6 +10,7 @@ use std::borrow::Cow; use std::collections::{BTreeMap, HashMap}; use std::fmt::{self, Display, Formatter}; use std::net::IpAddr; +use std::num::ParseIntError; use chrono::{DateTime, Utc}; use rand::{distributions::Uniform, Rng}; @@ -357,6 +358,31 @@ impl TimeRange { } } +/// A sampling strategy definition. +/// +/// A sampling strategy refers to the strategy that we want to use for sampling a specific rule. +#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +#[serde(tag = "type")] +pub enum SamplingValue { + /// A rule with a sample rate will be matched and the final sample rate will be computed by + /// multiplying its sample rate with the accumulated factors from previous rules. + SampleRate { value: f64 }, + /// A rule with a factor will be matched and the matching will continue onto the next rules until + /// a sample rate rule is found. The matched rule's factor will be multiplied with the accumulated + /// factors before moving onto the next possible match. + Factor { value: f64 }, +} + +impl SamplingValue { + fn value(&self) -> f64 { + *match self { + SamplingValue::SampleRate { value: sample_rate } => sample_rate, + SamplingValue::Factor { value: factor } => factor, + } + } +} + /// A decaying function definition. /// /// A decaying function is responsible of decaying the sample rate from a value to another following @@ -366,34 +392,34 @@ impl TimeRange { #[serde(tag = "type")] pub enum DecayingFunction { #[serde(rename_all = "camelCase")] - Linear { decayed_sample_rate: f64 }, + Linear { decayed_value: f64 }, #[default] Constant, } /// A struct representing the evaluation context of a sample rate. #[derive(Debug, Clone, Copy)] -enum SampleRateEvaluator { +enum SamplingValueEvaluator { Linear { start: DateTime, end: DateTime, - sample_rate: f64, - decayed_sample_rate: f64, + initial_value: f64, + decayed_value: f64, }, Constant { - sample_rate: f64, + initial_value: f64, }, } -impl SampleRateEvaluator { - /// Evaluates the sample rate given the decaying function and the current time. +impl SamplingValueEvaluator { + /// Evaluates the value of the sampling strategy given a the current time. fn evaluate(&self, now: DateTime) -> f64 { match self { - SampleRateEvaluator::Linear { + SamplingValueEvaluator::Linear { start, end, - sample_rate, - decayed_sample_rate, + initial_value, + decayed_value, } => { let now_timestamp = now.timestamp() as f64; let start_timestamp = start.timestamp() as f64; @@ -402,10 +428,11 @@ impl SampleRateEvaluator { / (end_timestamp - start_timestamp)) .clamp(0.0, 1.0); - let interval = decayed_sample_rate - sample_rate; - sample_rate + (interval * progress_ratio) + // This interval will always be < 0. + let interval = decayed_value - initial_value; + initial_value + (interval * progress_ratio) } - SampleRateEvaluator::Constant { sample_rate } => *sample_rate, + SamplingValueEvaluator::Constant { initial_value } => *initial_value, } } } @@ -415,12 +442,12 @@ impl SampleRateEvaluator { #[derive(Debug, Clone, Copy)] pub struct ActiveRule { pub id: RuleId, - evaluator: SampleRateEvaluator, + evaluator: SamplingValueEvaluator, } impl ActiveRule { /// Gets the sample rate for the specific rule. - pub fn get_sample_rate(&self, now: DateTime) -> f64 { + pub fn sampling_value(&self, now: DateTime) -> f64 { self.evaluator.evaluate(now) } } @@ -430,7 +457,7 @@ impl ActiveRule { #[serde(rename_all = "camelCase")] pub struct SamplingRule { pub condition: RuleCondition, - pub sample_rate: f64, + pub sampling_value: SamplingValue, #[serde(rename = "type")] pub ty: RuleType, pub id: RuleId, @@ -454,24 +481,24 @@ impl SamplingRule { /// The checking of the "active" state of a SamplingRule is performed independently /// based on the specified DecayingFunction, which defaults to constant. fn is_active(&self, now: DateTime) -> Option { + let sampling_base_value = self.sampling_value.value(); + match self.decaying_fn { - DecayingFunction::Linear { - decayed_sample_rate, - } => { + DecayingFunction::Linear { decayed_value } => { if let TimeRange { start: Some(start), end: Some(end), } = self.time_range { // As in the TimeRange::contains method we use a right non-inclusive time bound. - if self.sample_rate > decayed_sample_rate && start <= now && now < end { + if sampling_base_value > decayed_value && start <= now && now < end { return Some(ActiveRule { id: self.id, - evaluator: SampleRateEvaluator::Linear { + evaluator: SamplingValueEvaluator::Linear { start, end, - sample_rate: self.sample_rate, - decayed_sample_rate, + initial_value: sampling_base_value, + decayed_value, }, }); } @@ -481,8 +508,8 @@ impl SamplingRule { if self.time_range.contains(now) { return Some(ActiveRule { id: self.id, - evaluator: SampleRateEvaluator::Constant { - sample_rate: self.sample_rate, + evaluator: SamplingValueEvaluator::Constant { + initial_value: sampling_base_value, }, }); } @@ -491,6 +518,10 @@ impl SamplingRule { None } + + fn is_sample_rate_rule(&self) -> bool { + matches!(self.sampling_value, SamplingValue::SampleRate { .. }) + } } /// Trait implemented by providers of fields (Events and Trace Contexts). @@ -809,79 +840,199 @@ impl Default for SamplingMode { } } -/// 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 ordered sampling rules for the project from highest to lowest priority. - pub rules: Vec, - /// Defines which population of items a dynamic sample rate applies to. - #[serde(default)] - pub mode: SamplingMode, - /// The unique identifier for the next new rule to be added. - #[serde(default)] - pub next_id: Option, +/// Represents a list of rule ids which is used for outcomes. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct MatchedRuleIds(pub Vec); + +impl MatchedRuleIds { + /// Creates a MatchedRuleIds struct from a string formatted with the following format: + /// rule_id_1,rule_id_2,... + pub fn from_string(value: &str) -> Result { + let mut rule_ids = vec![]; + + for rule_id in value.split(',') { + let int_rule_id = rule_id.parse()?; + rule_ids.push(RuleId(int_rule_id)); + } + + Ok(MatchedRuleIds(rule_ids)) + } } -impl SamplingConfig { - pub fn has_unsupported_rules(&self) -> bool { - !self.rules.iter().all(SamplingRule::supported) +impl Display for MatchedRuleIds { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!( + f, + "{}", + self.0 + .iter() + .map(|rule_id| format!("{rule_id}")) + .collect::>() + .join(",") + ) } +} - /// Get the first rule of type [`RuleType::Trace`] whose conditions match on the given sampling - /// context. - /// - /// This is a function separate from `get_matching_event_rule` because trace rules can - /// (theoretically) be applied even if there's no event. Also we expect that trace rules are - /// executed before event rules. - pub fn get_matching_trace_rule( - &self, - sampling_context: &DynamicSamplingContext, - ip_addr: Option, - now: DateTime, - ) -> Option { - self.rules.iter().find_map(|rule| { - if rule.ty != RuleType::Trace { - return None; - } +/// Returns an iterator of references that chains together and merges rules. +/// +/// The chaining logic will take all the non-trace rules from the project and all the trace/unsupported +/// rules from the root project and concatenate them. +pub fn merge_rules_from_configs<'a>( + sampling_config: &'a SamplingConfig, + root_sampling_config: Option<&'a SamplingConfig>, +) -> impl Iterator { + let event_rules = sampling_config + .rules_v2 + .iter() + .filter(|&rule| rule.ty == RuleType::Transaction || rule.ty == RuleType::Error); + + let parent_rules = root_sampling_config + .into_iter() + .flat_map(|config| config.rules_v2.iter()) + .filter(|&rule| rule.ty == RuleType::Trace); + + event_rules.chain(parent_rules) +} - if !rule.condition.matches(sampling_context, ip_addr) { - return None; - } +/// Represents the specification for sampling an incoming event. +#[derive(Clone, Debug, PartialEq)] +pub struct SamplingMatch { + /// The sample rate to use for the incoming event. + pub sample_rate: f64, + /// The seed to feed to the random number generator which allows the same number to be + /// generated given the same seed. + /// + /// This is especially important for trace sampling, even though we can have inconsistent + /// traces due to multi-matching. + pub seed: Uuid, + /// The list of rule ids that have matched the incoming event and/or dynamic sampling context. + pub matched_rule_ids: MatchedRuleIds, +} - rule.is_active(now) - }) +impl SamplingMatch { + /// Setter for `sample_rate`. + pub fn set_sample_rate(&mut self, new_sample_rate: f64) { + self.sample_rate = new_sample_rate; } - /// Get the first rule of type [`RuleType::Transaction`] or [`RuleType::Error`] whose conditions - /// match the given event. + /// Matches an event and/or dynamic sampling context against the rules of the sampling configuration. + /// + /// The multi-matching algorithm used iterates by collecting and multiplying factor rules until + /// it finds a sample rate rule. Once a sample rate rule is found, the final sample rate is + /// computed by multiplying it with the previously accumulated factors. + /// + /// The default accumulated factors equal to 1 because it is the identity of the multiplication + /// operation, thus in case no factor rules are matched, the final result will just be the + /// sample rate of the matching rule. /// - /// The rule type to filter by is inferred from the event's type. - pub fn get_matching_event_rule( - &self, + /// In case no sample rate rule is matched, we are going to return a None, signaling that no + /// match has been found. + pub fn match_against_rules<'a, I>( + rules: I, event: &Event, + dsc: Option<&DynamicSamplingContext>, ip_addr: Option, now: DateTime, - ) -> Option { - let ty = if let Some(EventType::Transaction) = &event.ty.0 { - RuleType::Transaction - } else { - RuleType::Error - }; + ) -> Option + where + I: Iterator, + { + let mut matched_rule_ids = vec![]; + // Even though this seed is changed based on whether we match event or trace rules, we will + // still incur in inconsistent trace sampling because of multi-matching of rules across event + // and trace rules. + // + // An example of inconsistent trace sampling could be: + // /hello -> /world -> /transaction belong to trace_id = abc + // * /hello has uniform rule with 0.2 sample rate which will match all the transactions of the trace + // * each project has a single transaction rule with different factors (2, 3, 4) + // + // 1. /hello is matched with a transaction rule with a factor of 2 and uses as seed abc -> 0.2 * 2 = 0.4 sample rate + // 2. /world is matched with a transaction rule with a factor of 3 and uses as seed abc -> 0.2 * 3 = 0.6 sample rate + // 3. /transaction is matched with a transaction rule with a factor of 4 and uses as seed abc -> 0.2 * 4 = 0.8 sample rate + // + // We can see that we have 3 different samples rates but given the same seed, the random number generated will be the same. + let mut seed = event.id.value().map(|id| id.0); + let mut accumulated_factors = 1.0; + + for rule in rules { + let matches = match rule.ty { + RuleType::Trace => match dsc { + Some(dsc) => rule.condition.matches(dsc, ip_addr), + _ => false, + }, + RuleType::Transaction => match event.ty.0 { + Some(EventType::Transaction) => rule.condition.matches(event, ip_addr), + _ => false, + }, + RuleType::Error => { + if let Some(EventType::Transaction) = event.ty.0 { + false + } else { + rule.condition.matches(event, ip_addr) + } + } + _ => false, + }; - self.rules.iter().find_map(|rule| { - if rule.ty != ty { - return None; - } + if matches { + if let Some(active_rule) = rule.is_active(now) { + matched_rule_ids.push(rule.id); + + if rule.ty == RuleType::Trace { + if let Some(dsc) = dsc { + seed = Some(dsc.trace_id); + } + } - if !rule.condition.matches(event, ip_addr) { - return None; + let value = active_rule.sampling_value(now); + if rule.is_sample_rate_rule() { + return Some(SamplingMatch { + sample_rate: (value * accumulated_factors).clamp(0.0, 1.0), + seed: match seed { + Some(seed) => seed, + // In case we are not able to generate a seed, we will return a no + // match. + None => return None, + }, + matched_rule_ids: MatchedRuleIds(matched_rule_ids), + }); + } else { + accumulated_factors *= value + } + } } + } - rule.is_active(now) - }) + // In case no match is available, we won't return any specification. + None + } +} + +/// 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 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)] + pub rules: Vec, + /// The ordered sampling rules v2 for the project. + pub rules_v2: Vec, + /// Defines which population of items a dynamic sample rate applies to. + #[serde(default)] + pub mode: SamplingMode, +} + +impl SamplingConfig { + pub fn has_unsupported_rules(&self) -> bool { + !self.rules_v2.iter().all(SamplingRule::supported) } } @@ -1095,19 +1246,59 @@ pub fn pseudo_random_from_uuid(id: Uuid) -> f64 { #[cfg(test)] mod tests { + use similar_asserts::assert_eq; + use std::net::{IpAddr as NetIpAddr, Ipv4Addr}; use std::str::FromStr; use chrono::{TimeZone, Utc}; use relay_general::protocol::{ - Contexts, Csp, DeviceContext, Exception, Headers, IpAddr, JsonLenientString, LenientString, - LogEntry, OsContext, PairList, Request, TagEntry, Tags, User, Values, + Contexts, Csp, DeviceContext, EventId, Exception, Headers, IpAddr, JsonLenientString, + LenientString, LogEntry, OsContext, PairList, Request, TagEntry, Tags, User, Values, }; use relay_general::types::Annotated; use super::*; + macro_rules! assert_transaction_match { + ($res:expr, $sr:expr, $sd:expr, $( $id:expr ),*) => { + assert_eq!( + $res, + Some(SamplingMatch { + sample_rate: $sr, + seed: $sd.id.value().unwrap().0, + matched_rule_ids: MatchedRuleIds(vec![$(RuleId($id),)*]) + }) + ) + } + } + + macro_rules! assert_trace_match { + ($res:expr, $sr:expr, $sd:expr, $( $id:expr ),*) => { + assert_eq!( + $res, + Some(SamplingMatch { + sample_rate: $sr, + seed: $sd.trace_id, + matched_rule_ids: MatchedRuleIds(vec![$(RuleId($id),)*]) + }) + ) + } + } + + macro_rules! assert_rule_ids_eq { + ($exc:expr, $res:expr) => { + if ($exc.len() != $res.len()) { + panic!("The rule ids don't match.") + } + + for (index, rule) in $res.iter().enumerate() { + assert_eq!(rule.id.0, $exc[index]) + } + }; + } + fn default_sampling_context() -> DynamicSamplingContext { DynamicSamplingContext { trace_id: Uuid::default(), @@ -1166,6 +1357,84 @@ mod tests { }) } + fn mocked_sampling_config(rules: Vec) -> SamplingConfig { + SamplingConfig { + rules: vec![], + rules_v2: rules, + mode: SamplingMode::Received, + } + } + + fn mocked_sampling_rule(id: u32, ty: RuleType, sample_rate: f64) -> SamplingRule { + SamplingRule { + condition: RuleCondition::all(), + sampling_value: SamplingValue::SampleRate { value: sample_rate }, + ty, + id: RuleId(id), + time_range: Default::default(), + decaying_fn: Default::default(), + } + } + + fn mocked_event( + event_type: EventType, + transaction: &str, + release: &str, + environment: &str, + ) -> Event { + Event { + id: Annotated::new(EventId::new()), + ty: Annotated::new(event_type), + transaction: Annotated::new(transaction.to_string()), + release: Annotated::new(LenientString(release.to_string())), + environment: Annotated::new(environment.to_string()), + ..Event::default() + } + } + + fn mocked_dynamic_sampling_context( + transaction: &str, + release: &str, + environment: &str, + user_segment: &str, + user_id: &str, + ) -> DynamicSamplingContext { + DynamicSamplingContext { + trace_id: Uuid::new_v4(), + public_key: "12345678901234567890123456789012".parse().unwrap(), + release: Some(release.to_string()), + environment: Some(environment.to_string()), + transaction: Some(transaction.to_string()), + sample_rate: Some(1.0), + user: TraceUserContext { + user_segment: user_segment.to_string(), + user_id: user_id.to_string(), + }, + other: Default::default(), + } + } + + fn match_against_rules( + config: &SamplingConfig, + event: &Event, + dsc: &DynamicSamplingContext, + now: DateTime, + ) -> Option { + SamplingMatch::match_against_rules(config.rules_v2.iter(), event, Some(dsc), None, now) + } + + fn merge_root_and_non_root_configs_with( + rules: Vec, + root_rules: Vec, + ) -> Vec { + let sampling_config = mocked_sampling_config(rules); + let root_sampling_config = mocked_sampling_config(root_rules); + + merge_rules_from_configs(&sampling_config, Some(&root_sampling_config)) + .cloned() + .collect() + } + #[test] fn test_unmatching_json_rule_is_unsupported() { let bad_json = r#"{ @@ -1763,7 +2032,7 @@ mod tests { } #[test] - // /// test various rules that do not match + /// test various rules that do not match fn test_does_not_match() { let conditions = [ ( @@ -1990,8 +2259,7 @@ mod tests { } #[test] - ///Test SamplingRule deserialization - fn test_nondecaying_sampling_rule_deserialization() { + fn test_non_decaying_sampling_rule_deserialization() { let serialized_rule = r#"{ "condition":{ "op":"and", @@ -1999,20 +2267,40 @@ mod tests { { "op" : "glob", "name": "releases", "value":["1.1.1", "1.1.2"]} ] }, - "sampleRate": 0.7, + "samplingValue": {"type": "sampleRate", "value": 0.7}, "type": "trace", "id": 1 }"#; - let rule: Result = serde_json::from_str(serialized_rule); - assert!(rule.is_ok()); - let rule = rule.unwrap(); - assert!(approx_eq(rule.sample_rate, 0.7f64)); + let rule: SamplingRule = serde_json::from_str(serialized_rule).unwrap(); + assert_eq!( + rule.sampling_value, + SamplingValue::SampleRate { value: 0.7f64 } + ); + assert_eq!(rule.ty, RuleType::Trace); + } + + #[test] + fn test_non_decaying_sampling_rule_deserialization_with_factor() { + let serialized_rule = r#"{ + "condition":{ + "op":"and", + "inner": [ + { "op" : "glob", "name": "releases", "value":["1.1.1", "1.1.2"]} + ] + }, + "samplingValue": {"type": "factor", "value": 5.0}, + "type": "trace", + "id": 1 + }"#; + + let rule: SamplingRule = serde_json::from_str(serialized_rule).unwrap(); + assert_eq!(rule.sampling_value, SamplingValue::Factor { value: 5.0 }); assert_eq!(rule.ty, RuleType::Trace); } #[test] - fn test_sampling_rule_with_none_decaying_function_deserialization() { + fn test_sampling_rule_with_constant_decaying_function_deserialization() { let serialized_rule = r#"{ "condition":{ "op":"and", @@ -2020,7 +2308,7 @@ mod tests { { "op" : "glob", "name": "releases", "value":["1.1.1", "1.1.2"]} ] }, - "sampleRate": 0.7, + "samplingValue": {"type": "factor", "value": 5.0}, "type": "trace", "id": 1, "timeRange": { @@ -2053,7 +2341,7 @@ mod tests { { "op" : "glob", "name": "releases", "value":["1.1.1", "1.1.2"]} ] }, - "sampleRate": 0.7, + "samplingValue": {"type": "sampleRate", "value": 1.0}, "type": "trace", "id": 1, "timeRange": { @@ -2062,7 +2350,7 @@ mod tests { }, "decayingFn": { "type": "linear", - "decayedSampleRate": 0.9 + "decayedValue": 0.9 } }"#; let rule: Result = serde_json::from_str(serialized_rule); @@ -2071,12 +2359,105 @@ mod tests { assert_eq!( decaying_function, - DecayingFunction::Linear { - decayed_sample_rate: 0.9 - } + DecayingFunction::Linear { decayed_value: 0.9 } ); } + #[test] + fn test_sampling_config_with_rules_and_rules_v2_deserialization() { + let serialized_rule = r#"{ + "rules": [ + { + "sampleRate": 0.5, + "type": "trace", + "active": true, + "condition": { + "op": "and", + "inner": [] + }, + "id": 1000 + } + ], + "rulesV2": [ + { + "samplingValue":{ + "type": "sampleRate", + "value": 0.5 + }, + "type": "trace", + "active": true, + "condition": { + "op": "and", + "inner": [] + }, + "id": 1000 + } + ], + "mode": "received" + }"#; + let config: SamplingConfig = serde_json::from_str(serialized_rule).unwrap(); + + // 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.rules_v2[0].sampling_value, + SamplingValue::SampleRate { value: 0.5 } + ); + } + + #[test] + fn test_sampling_config_with_rules_and_rules_v2_serialization() { + let config = SamplingConfig { + rules: vec![], + rules_v2: vec![SamplingRule { + condition: and(vec![eq("event.transaction", &["foo"], true)]), + sampling_value: SamplingValue::Factor { value: 2.0 }, + ty: RuleType::Transaction, + id: RuleId(1), + time_range: Default::default(), + decaying_fn: Default::default(), + }], + mode: SamplingMode::Received, + }; + + let serialized_config = serde_json::to_string_pretty(&config).unwrap(); + let expected_serialized_config = r#"{ + "rules": [], + "rulesV2": [ + { + "condition": { + "op": "and", + "inner": [ + { + "op": "eq", + "name": "event.transaction", + "value": [ + "foo" + ], + "options": { + "ignoreCase": true + } + } + ] + }, + "samplingValue": { + "type": "factor", + "value": 2.0 + }, + "type": "transaction", + "id": 1, + "decayingFn": { + "type": "constant" + } + } + ], + "mode": "received" +}"#; + + assert_eq!(serialized_config, expected_serialized_config) + } + #[test] fn test_partial_trace_matches() { let condition = and(vec![ @@ -2185,408 +2566,288 @@ mod tests { ); } - fn approx_eq(left: f64, right: f64) -> bool { - let diff = left - right; - diff < 0.001 && diff > -0.001 - } - #[test] - /// test that the first rule that matches is selected - fn test_nondecaying_rule_precedence() { - let rules = SamplingConfig { - rules: vec![ - //everything specified - SamplingRule { - condition: and(vec![ - glob("trace.release", &["1.1.1"]), - eq("trace.environment", &["debug"], true), - eq("trace.user.segment", &["vip"], true), - ]), - sample_rate: 0.1, - ty: RuleType::Trace, - id: RuleId(1), - time_range: Default::default(), - decaying_fn: Default::default(), - }, - // no user segments - SamplingRule { - condition: and(vec![ - glob("trace.release", &["1.1.2"]), - eq("trace.environment", &["debug"], true), - ]), - sample_rate: 0.2, - ty: RuleType::Trace, - id: RuleId(2), - time_range: Default::default(), - decaying_fn: Default::default(), - }, - // no releases - SamplingRule { - condition: and(vec![ - eq("trace.environment", &["debug"], true), - eq("trace.user.segment", &["vip"], true), - ]), - sample_rate: 0.3, - ty: RuleType::Trace, - id: RuleId(3), - time_range: Default::default(), - decaying_fn: Default::default(), - }, - // no environments - SamplingRule { - condition: and(vec![ - glob("trace.release", &["1.1.1"]), - eq("trace.user.segment", &["vip"], true), - ]), - sample_rate: 0.4, - ty: RuleType::Trace, - id: RuleId(4), - time_range: Default::default(), - decaying_fn: Default::default(), - }, - // no user segments releases or environments - SamplingRule { - condition: RuleCondition::And(AndCondition { inner: vec![] }), - sample_rate: 0.5, - ty: RuleType::Trace, - id: RuleId(5), - time_range: Default::default(), - decaying_fn: Default::default(), - }, - ], - mode: SamplingMode::Received, - next_id: None, - }; - - let trace_context = DynamicSamplingContext { - trace_id: Uuid::new_v4(), - public_key: ProjectKey::parse("abd0f232775f45feab79864e580d160b").unwrap(), - release: Some("1.1.1".to_string()), - user: TraceUserContext { - user_segment: "vip".to_owned(), - user_id: "user-id".to_owned(), - }, - environment: Some("debug".to_string()), - transaction: Some("transaction1".into()), - sample_rate: None, - other: BTreeMap::new(), - }; + /// Tests if the MatchedRuleIds struct is displayed correctly as string. + fn test_matched_rule_ids_to_string() { + let matched_rule_ids = MatchedRuleIds(vec![RuleId(123), RuleId(456)]); + assert_eq!(format!("{matched_rule_ids}"), "123,456"); - let result = rules.get_matching_trace_rule(&trace_context, None, Utc::now()); - // complete match with first rule - assert_eq!( - result.unwrap().id, - RuleId(1), - "did not match the expected first rule" - ); + let matched_rule_ids = MatchedRuleIds(vec![RuleId(123)]); + assert_eq!(format!("{matched_rule_ids}"), "123"); - let trace_context = DynamicSamplingContext { - trace_id: Uuid::new_v4(), - public_key: ProjectKey::parse("abd0f232775f45feab79864e580d160b").unwrap(), - release: Some("1.1.2".to_string()), - user: TraceUserContext { - user_segment: "vip".to_owned(), - user_id: "user-id".to_owned(), - }, - environment: Some("debug".to_string()), - transaction: Some("transaction1".into()), - sample_rate: None, - other: BTreeMap::new(), - }; + let matched_rule_ids = MatchedRuleIds(vec![]); + assert_eq!(format!("{matched_rule_ids}"), "") + } - let result = rules.get_matching_trace_rule(&trace_context, None, Utc::now()); - // should mach the second rule because of the release + #[test] + /// Tests if the MatchRuleIds struct is created correctly from its string representation. + fn test_matched_rule_ids_from_string() { assert_eq!( - result.unwrap().id, - RuleId(2), - "did not match the expected second rule" + MatchedRuleIds::from_string("123,456"), + Ok(MatchedRuleIds(vec![RuleId(123), RuleId(456)])) ); - let trace_context = DynamicSamplingContext { - trace_id: Uuid::new_v4(), - public_key: ProjectKey::parse("abd0f232775f45feab79864e580d160b").unwrap(), - release: Some("1.1.3".to_string()), - user: TraceUserContext { - user_segment: "vip".to_owned(), - user_id: "user-id".to_owned(), - }, - environment: Some("debug".to_string()), - transaction: Some("transaction1".into()), - sample_rate: None, - other: BTreeMap::new(), - }; - - let result = rules.get_matching_trace_rule(&trace_context, None, Utc::now()); - // should match the third rule because of the unknown release assert_eq!( - result.unwrap().id, - RuleId(3), - "did not match the expected third rule" + MatchedRuleIds::from_string("123"), + Ok(MatchedRuleIds(vec![RuleId(123)])) ); - let trace_context = DynamicSamplingContext { - trace_id: Uuid::new_v4(), - public_key: ProjectKey::parse("abd0f232775f45feab79864e580d160b").unwrap(), - release: Some("1.1.1".to_string()), - user: TraceUserContext { - user_segment: "vip".to_owned(), - user_id: "user-id".to_owned(), - }, - environment: Some("production".to_string()), - transaction: Some("transaction1".into()), - sample_rate: None, - other: BTreeMap::new(), - }; + assert!(matches!(MatchedRuleIds::from_string(""), Err(_))); - let result = rules.get_matching_trace_rule(&trace_context, None, Utc::now()); - // should match the fourth rule because of the unknown environment - assert_eq!( - result.unwrap().id, - RuleId(4), - "did not match the expected fourth rule" - ); + assert!(matches!(MatchedRuleIds::from_string(","), Err(_))); - let trace_context = DynamicSamplingContext { - trace_id: Uuid::new_v4(), - public_key: ProjectKey::parse("abd0f232775f45feab79864e580d160b").unwrap(), - release: Some("1.1.1".to_string()), - user: TraceUserContext { - user_segment: "all".to_owned(), - user_id: "user-id".to_owned(), - }, - environment: Some("debug".to_string()), - transaction: Some("transaction1".into()), - sample_rate: None, - other: BTreeMap::new(), - }; + assert!(matches!(MatchedRuleIds::from_string("123.456"), Err(_))); - let result = rules.get_matching_trace_rule(&trace_context, None, Utc::now()); - // should match the fourth rule because of the unknown user segment - assert_eq!( - result.unwrap().id, - RuleId(5), - "did not match the expected fourth rule" - ); + assert!(matches!(MatchedRuleIds::from_string("a,b"), Err(_))); } #[test] - fn test_decaying_trace_rule_precedence() { - let rules = decaying_sampling_config(&RuleType::Trace); - - let new_release = DynamicSamplingContext { - trace_id: Uuid::new_v4(), - public_key: ProjectKey::parse("abd0f232775f45feab79864e580d160b").unwrap(), - release: Some("1.1.1".to_string()), - user: TraceUserContext { - user_segment: "vip".to_owned(), - user_id: "user-id".to_owned(), - }, - environment: Some("testing".to_string()), - transaction: Some("transaction1".into()), - sample_rate: None, - other: BTreeMap::new(), - }; - let result = rules.get_matching_trace_rule(&new_release, None, Utc::now()); - assert_eq!(result.unwrap().id, RuleId(4), "Did not match expected rule"); - - let old_release = DynamicSamplingContext { - trace_id: Uuid::new_v4(), - public_key: ProjectKey::parse("abd0f232775f45feab79864e580d160b").unwrap(), - release: Some("1.1.0".to_string()), - user: TraceUserContext { - user_segment: "vip".to_owned(), - user_id: "user-id".to_owned(), - }, - environment: Some("testing".to_string()), - transaction: Some("transaction2".into()), - sample_rate: None, - other: BTreeMap::new(), - }; - let result = rules.get_matching_trace_rule(&old_release, None, Utc::now()); - assert_eq!(result.unwrap().id, RuleId(5), "Did not match expected rule"); - } - - fn decaying_sampling_config(rule_type: &RuleType) -> SamplingConfig { - let rule_prefix = match rule_type { - RuleType::Trace => "trace", - RuleType::Transaction => "event", - // For the Error variant, the prefix is also `event`. However, it's not - // supported in the code, so intentionally not implementing any logic for - // that. This whole helper method should be revisited when the Error variant - // is supported. - RuleType::Error | RuleType::Unsupported => unimplemented!(), - }; - let release_condition = format!("{rule_prefix}.release"); - let env_condition = format!("{rule_prefix}.environment"); - - SamplingConfig { - rules: vec![ - // Expired + /// test that the multi-matching returns none in case there is no match. + fn test_multi_matching_with_transaction_event_non_decaying_rules_and_no_match() { + let result = match_against_rules( + &mocked_sampling_config(vec![ SamplingRule { - condition: eq(&release_condition, &["1.1.1"], true), - sample_rate: 0.2, - ty: *rule_type, + condition: and(vec![eq("event.transaction", &["foo"], true)]), + sampling_value: SamplingValue::Factor { value: 2.0 }, + ty: RuleType::Transaction, id: RuleId(1), - time_range: TimeRange { - start: Some(Utc.with_ymd_and_hms(1970, 10, 10, 0, 0, 0).unwrap()), - end: Some(Utc.with_ymd_and_hms(1970, 10, 30, 0, 0, 0).unwrap()), - }, - decaying_fn: DecayingFunction::Constant, + time_range: Default::default(), + decaying_fn: Default::default(), }, - // Idle SamplingRule { - condition: eq(&release_condition, &["1.1.1"], true), - sample_rate: 0.2, - ty: *rule_type, + condition: and(vec![ + glob("trace.release", &["1.1.1"]), + eq("trace.environment", &["prod"], true), + ]), + sampling_value: SamplingValue::SampleRate { value: 0.5 }, + ty: RuleType::Trace, id: RuleId(2), - time_range: TimeRange { - start: Some(Utc.with_ymd_and_hms(3000, 10, 10, 0, 0, 0).unwrap()), - end: Some(Utc.with_ymd_and_hms(3000, 10, 15, 0, 0, 0).unwrap()), - }, - decaying_fn: DecayingFunction::Constant, - }, - // Start after finishing - SamplingRule { - condition: eq(&release_condition, &["1.1.1"], true), - sample_rate: 0.2, - ty: *rule_type, - id: RuleId(3), - time_range: TimeRange { - start: Some(Utc.with_ymd_and_hms(3000, 10, 10, 0, 0, 0).unwrap()), - end: Some(Utc.with_ymd_and_hms(1970, 10, 30, 0, 0, 0).unwrap()), - }, - decaying_fn: DecayingFunction::Constant, - }, - // Rule is ok - SamplingRule { - condition: eq(&release_condition, &["1.1.1"], true), - sample_rate: 0.2, - ty: *rule_type, - id: RuleId(4), - time_range: TimeRange { - start: Some(Utc.with_ymd_and_hms(1970, 10, 30, 0, 0, 0).unwrap()), - end: Some(Utc.with_ymd_and_hms(3000, 10, 10, 0, 0, 0).unwrap()), - }, - decaying_fn: DecayingFunction::Constant, - }, - // Fallback to non-decaying rule - SamplingRule { - // Environment matching all contexts, so that everyone can fallback - condition: eq(&env_condition, &["testing"], true), - sample_rate: 0.2, - ty: *rule_type, - id: RuleId(5), time_range: Default::default(), decaying_fn: Default::default(), }, - ], - mode: SamplingMode::Received, - next_id: None, - } + ]), + &mocked_event(EventType::Transaction, "healthcheck", "1.1.1", "testing"), + &mocked_dynamic_sampling_context( + "root_transaction", + "1.1.1", + "debug", + "vip", + "user-id", + ), + Utc::now(), + ); + assert_eq!(result, None, "did not return none for no match"); } #[test] - fn test_decaying_transaction_rule_precedence() { - let rules = decaying_sampling_config(&RuleType::Transaction); - - let new_release = Event { - ty: Annotated::new(EventType::Transaction), - release: Annotated::new("1.1.1".to_string().into()), - environment: Annotated::new("testing".to_string()), - ..Default::default() - }; - let result = rules.get_matching_event_rule(&new_release, None, Utc::now()); - assert_eq!(result.unwrap().id, RuleId(4), "Did not match expected rule"); + /// Tests that the multi-matching works for a mixture of trace and transaction rules with interleaved strategies. + fn test_match_against_rules_with_multiple_event_types_non_decaying_rules_and_matches() { + let config = mocked_sampling_config(vec![ + SamplingRule { + condition: and(vec![glob("event.transaction", &["*healthcheck*"])]), + sampling_value: SamplingValue::SampleRate { value: 0.1 }, + ty: RuleType::Transaction, + id: RuleId(1), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + SamplingRule { + condition: and(vec![glob("trace.environment", &["*dev*"])]), + sampling_value: SamplingValue::SampleRate { value: 1.0 }, + ty: RuleType::Trace, + id: RuleId(2), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + SamplingRule { + condition: and(vec![eq("event.transaction", &["foo"], true)]), + sampling_value: SamplingValue::Factor { value: 2.0 }, + ty: RuleType::Transaction, + id: RuleId(3), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + SamplingRule { + condition: and(vec![ + glob("trace.release", &["1.1.1"]), + eq("trace.user.segment", &["vip"], true), + ]), + sampling_value: SamplingValue::SampleRate { value: 0.5 }, + ty: RuleType::Trace, + id: RuleId(4), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + SamplingRule { + condition: and(vec![ + eq("trace.release", &["1.1.1"], true), + eq("trace.environment", &["prod"], true), + ]), + sampling_value: SamplingValue::Factor { value: 1.5 }, + ty: RuleType::Trace, + id: RuleId(5), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + SamplingRule { + condition: and(vec![]), + sampling_value: SamplingValue::SampleRate { value: 0.02 }, + ty: RuleType::Trace, + id: RuleId(6), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + ]); - let old_release = Event { - ty: Annotated::new(EventType::Transaction), - release: Annotated::new("1.1.0".to_string().into()), - environment: Annotated::new("testing".to_string()), - ..Default::default() - }; - let result = rules.get_matching_event_rule(&old_release, None, Utc::now()); - assert_eq!(result.unwrap().id, RuleId(5), "Did not match expected rule"); + // early return of first rule + let event = mocked_event(EventType::Transaction, "healthcheck", "1.1.1", "testing"); + let dsc = + mocked_dynamic_sampling_context("root_transaction", "1.1.1", "debug", "vip", "user-id"); + let result = match_against_rules(&config, &event, &dsc, Utc::now()); + assert_transaction_match!(result, 0.1, event, 1); + + // early return of second rule + let event = mocked_event(EventType::Transaction, "foo", "1.1.1", "testing"); + let dsc = + mocked_dynamic_sampling_context("root_transaction", "1.1.1", "dev", "vip", "user-id"); + let result = match_against_rules(&config, &event, &dsc, Utc::now()); + assert_trace_match!(result, 1.0, dsc, 2); + + // factor match third rule and early return sixth rule + let event = mocked_event(EventType::Transaction, "foo", "1.1.1", "testing"); + let dsc = mocked_dynamic_sampling_context( + "root_transaction", + "1.1.2", + "testing", + "non-vip", + "user-id", + ); + let result = match_against_rules(&config, &event, &dsc, Utc::now()); + assert_trace_match!(result, 0.04, dsc, 3, 6); + + // factor match third rule and early return fourth rule + let event = mocked_event(EventType::Transaction, "foo", "1.1.1", "testing"); + let dsc = + mocked_dynamic_sampling_context("root_transaction", "1.1.1", "prod", "vip", "user-id"); + let result = match_against_rules(&config, &event, &dsc, Utc::now()); + assert_trace_match!(result, 1.0, dsc, 3, 4); + + // factor match third, fifth rule and early return sixth rule + let event = mocked_event(EventType::Transaction, "foo", "1.1.1", "testing"); + let dsc = mocked_dynamic_sampling_context( + "root_transaction", + "1.1.1", + "prod", + "non-vip", + "user-id", + ); + let result = match_against_rules(&config, &event, &dsc, Utc::now()); + assert_trace_match!(result, 0.06, dsc, 3, 5, 6); + + // factor match fifth and early return sixth rule + let event = mocked_event(EventType::Transaction, "transaction", "1.1.1", "testing"); + let dsc = mocked_dynamic_sampling_context( + "root_transaction", + "1.1.1", + "prod", + "non-vip", + "user-id", + ); + let result = match_against_rules(&config, &event, &dsc, Utc::now()); + assert_trace_match!(result, 0.03, dsc, 5, 6); } #[test] - fn test_open_decaying_rules() { - let transaction = Event { - ty: Annotated::new(EventType::Transaction), - release: Annotated::new("1.1.1".to_string().into()), - ..Default::default() - }; - let trace = DynamicSamplingContext { - trace_id: Uuid::new_v4(), - public_key: ProjectKey::parse("abd0f232775f45feab79864e580d160b").unwrap(), - release: Some("1.1.1".to_string()), - user: TraceUserContext { - user_segment: "vip".to_owned(), - user_id: "user-id".to_owned(), - }, - environment: Some("testing".to_string()), - transaction: Some("transaction2".into()), - sample_rate: None, - other: BTreeMap::new(), - }; - - let ranges = vec![ - TimeRange { - start: Some(Utc.with_ymd_and_hms(1970, 10, 10, 0, 0, 0).unwrap()), - end: None, + /// Test that the multi-matching works for a mixture of decaying and non-decaying rules. + fn test_match_against_rules_with_trace_event_type_decaying_rules_and_matches() { + let config = mocked_sampling_config(vec![ + SamplingRule { + condition: and(vec![ + eq("trace.release", &["1.1.1"], true), + eq("trace.environment", &["dev"], true), + ]), + sampling_value: SamplingValue::Factor { value: 2.0 }, + ty: RuleType::Trace, + id: RuleId(1), + time_range: TimeRange { + start: Some(Utc.with_ymd_and_hms(1970, 10, 10, 0, 0, 0).unwrap()), + end: Some(Utc.with_ymd_and_hms(1970, 10, 12, 0, 0, 0).unwrap()), + }, + decaying_fn: DecayingFunction::Linear { decayed_value: 1.0 }, }, - TimeRange { - start: None, - end: Some(Utc.with_ymd_and_hms(3000, 10, 10, 0, 0, 0).unwrap()), + SamplingRule { + condition: and(vec![ + eq("trace.release", &["1.1.1"], true), + eq("trace.environment", &["prod"], true), + ]), + sampling_value: SamplingValue::SampleRate { value: 0.6 }, + ty: RuleType::Trace, + id: RuleId(2), + time_range: TimeRange { + start: Some(Utc.with_ymd_and_hms(1970, 10, 10, 0, 0, 0).unwrap()), + end: Some(Utc.with_ymd_and_hms(1970, 10, 12, 0, 0, 0).unwrap()), + }, + decaying_fn: DecayingFunction::Linear { decayed_value: 0.3 }, }, - TimeRange { - start: None, - end: None, + SamplingRule { + condition: and(vec![]), + sampling_value: SamplingValue::SampleRate { value: 0.02 }, + ty: RuleType::Trace, + id: RuleId(3), + time_range: Default::default(), + decaying_fn: Default::default(), }, - ]; - - for range in ranges { - let config = SamplingConfig { - rules: vec![SamplingRule { - condition: eq("trace.release", &["1.1.1"], true), - sample_rate: 0.2, - ty: RuleType::Trace, - id: RuleId(1), - time_range: range, - decaying_fn: Default::default(), - }], - mode: SamplingMode::Received, - next_id: None, - }; - assert_eq!( - config - .get_matching_trace_rule(&trace, None, Utc::now()) - .unwrap() - .id, - RuleId(1), - "Trace rule did not match", - ); + ]); - let config = SamplingConfig { - rules: vec![SamplingRule { - condition: eq("event.release", &["1.1.1"], true), - sample_rate: 0.2, - ty: RuleType::Transaction, - id: RuleId(1), - time_range: range, - decaying_fn: Default::default(), - }], - mode: SamplingMode::Received, - next_id: None, - }; - assert_eq!( - config - .get_matching_event_rule(&transaction, None, Utc::now()) - .unwrap() - .id, - RuleId(1), - "Transaction rule did not match", - ); + // factor match first rule and early return third rule + let event = mocked_event(EventType::Transaction, "transaction", "1.1.1", "testing"); + let dsc = + mocked_dynamic_sampling_context("root_transaction", "1.1.1", "dev", "vip", "user-id"); + // We will use a time in the middle of 10th and 11th. + let result = match_against_rules( + &config, + &event, + &dsc, + Utc.with_ymd_and_hms(1970, 10, 11, 0, 0, 0).unwrap(), + ); + assert_trace_match!(result, 0.03, dsc, 1, 3); + + // early return second rule + let event = mocked_event(EventType::Transaction, "transaction", "1.1.1", "testing"); + let dsc = + mocked_dynamic_sampling_context("root_transaction", "1.1.1", "prod", "vip", "user-id"); + // We will use a time in the middle of 10th and 11th. + let result = match_against_rules( + &config, + &event, + &dsc, + Utc.with_ymd_and_hms(1970, 10, 11, 0, 0, 0).unwrap(), + ); + assert!(matches!(result, Some(SamplingMatch { .. }))); + if let Some(spec) = result { + assert!( + (spec.sample_rate - 0.45).abs() < f64::EPSILON, // 0.45 + "did not use the sample rate of the second rule" + ) } + + // early return third rule + let event = mocked_event(EventType::Transaction, "transaction", "1.1.1", "testing"); + let dsc = mocked_dynamic_sampling_context( + "root_transaction", + "1.1.1", + "testing", + "vip", + "user-id", + ); + // We will use a time in the middle of 10th and 11th. + let result = match_against_rules( + &config, + &event, + &dsc, + Utc.with_ymd_and_hms(1970, 10, 11, 0, 0, 0).unwrap(), + ); + assert_trace_match!(result, 0.02, dsc, 3); } #[test] @@ -2804,7 +3065,7 @@ mod tests { let rule: SamplingRule = serde_json::from_value(serde_json::json!({ "id": 1, "type": "trace", - "sampleRate": 1, + "samplingValue": {"type": "sampleRate", "value": 1.0}, "condition": {"op": "and", "inner": []} })) .unwrap(); @@ -2816,10 +3077,75 @@ mod tests { let rule: SamplingRule = serde_json::from_value(serde_json::json!({ "id": 1, "type": "new_rule_type_unknown_to_this_relay", - "sampleRate": 1, + "samplingValue": {"type": "sampleRate", "value": 1.0}, "condition": {"op": "and", "inner": []} })) .unwrap(); assert!(!rule.supported()); } + + #[test] + /// Tests the merged config of the two configs with rules. + fn test_get_merged_config_with_rules_in_both_project_config_and_root_project_config() { + assert_rule_ids_eq!( + [1, 2, 7], + merge_root_and_non_root_configs_with( + vec![ + mocked_sampling_rule(1, RuleType::Transaction, 0.1), + mocked_sampling_rule(2, RuleType::Error, 0.2), + mocked_sampling_rule(3, RuleType::Trace, 0.3), + mocked_sampling_rule(4, RuleType::Unsupported, 0.1), + ], + vec![ + mocked_sampling_rule(5, RuleType::Transaction, 0.4), + mocked_sampling_rule(6, RuleType::Error, 0.5), + mocked_sampling_rule(7, RuleType::Trace, 0.6), + mocked_sampling_rule(8, RuleType::Unsupported, 0.1), + ], + ) + ); + } + + #[test] + /// Tests the merged config of the two configs without rules. + fn test_get_merged_config_with_no_rules_in_both_project_config_and_root_project_config() { + assert!(merge_root_and_non_root_configs_with(vec![], vec![]).is_empty()); + } + + #[test] + /// Tests the merged config of the project config with rules and the root project config + /// without rules. + fn test_get_merged_config_with_rules_in_project_config_and_no_rules_in_root_project_config() { + assert_rule_ids_eq!( + [1, 2], + merge_root_and_non_root_configs_with( + vec![ + mocked_sampling_rule(1, RuleType::Transaction, 0.1), + mocked_sampling_rule(2, RuleType::Error, 0.2), + mocked_sampling_rule(3, RuleType::Trace, 0.3), + mocked_sampling_rule(4, RuleType::Unsupported, 0.1), + ], + vec![], + ) + ); + } + + #[test] + /// Tests the merged config of the project config without rules and the root project config + /// with rules. + fn test_get_merged_config_with_no_rules_in_project_config_and_with_rules_in_root_project_config( + ) { + assert_rule_ids_eq!( + [6], + merge_root_and_non_root_configs_with( + vec![], + vec![ + mocked_sampling_rule(4, RuleType::Transaction, 0.4), + mocked_sampling_rule(5, RuleType::Error, 0.5), + mocked_sampling_rule(6, RuleType::Trace, 0.6), + mocked_sampling_rule(7, RuleType::Unsupported, 0.1), + ] + ) + ); + } } diff --git a/relay-server/Cargo.toml b/relay-server/Cargo.toml index ae22f2e9f5..0802a3c4bc 100644 --- a/relay-server/Cargo.toml +++ b/relay-server/Cargo.toml @@ -69,6 +69,7 @@ tokio-timer = "0.2.13" url = { version = "2.1.1", features = ["serde"] } uuid = { version = "0.8.1", features = ["v5"] } zstd = { version = "0.11.2"} +similar-asserts = "1.4.2" [target."cfg(not(windows))".dependencies] libc = "0.2.71" diff --git a/relay-server/src/actors/outcome.rs b/relay-server/src/actors/outcome.rs index bb79cc0468..46db36037a 100644 --- a/relay-server/src/actors/outcome.rs +++ b/relay-server/src/actors/outcome.rs @@ -28,7 +28,7 @@ use relay_kafka::{ClientError, KafkaClient, KafkaTopic}; #[cfg(feature = "processing")] use relay_log::LogError; use relay_quotas::{ReasonCode, Scoping}; -use relay_sampling::RuleId; +use relay_sampling::MatchedRuleIds; use relay_statsd::metric; use relay_system::{Addr, FromMessage, Service}; @@ -168,7 +168,7 @@ pub enum Outcome { Filtered(FilterStatKey), /// The event has been filtered by a Sampling Rule - FilteredSampling(RuleId), + FilteredSampling(MatchedRuleIds), /// The event has been rate limited. RateLimited(Option), @@ -201,7 +201,7 @@ impl Outcome { match self { Outcome::Invalid(discard_reason) => Some(Cow::Borrowed(discard_reason.name())), Outcome::Filtered(filter_key) => Some(Cow::Borrowed(filter_key.name())), - Outcome::FilteredSampling(rule_id) => Some(Cow::Owned(format!("Sampled:{rule_id}"))), + Outcome::FilteredSampling(rule_ids) => Some(Cow::Owned(format!("Sampled:{rule_ids}"))), //TODO can we do better ? (not re copying the string ) Outcome::RateLimited(code_opt) => code_opt .as_ref() @@ -232,7 +232,7 @@ impl fmt::Display for Outcome { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Outcome::Filtered(key) => write!(f, "filtered by {key}"), - Outcome::FilteredSampling(rule) => write!(f, "sampling rule {rule}"), + Outcome::FilteredSampling(rule_ids) => write!(f, "sampling rule {rule_ids}"), Outcome::RateLimited(None) => write!(f, "rate limited"), Outcome::RateLimited(Some(reason)) => write!(f, "rate limited with reason {reason}"), Outcome::Invalid(DiscardReason::Internal) => write!(f, "internal error"), diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 0c25e4496e..4f5d937b6b 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -35,7 +35,7 @@ use relay_log::LogError; use relay_metrics::{Bucket, InsertMetrics, MergeBuckets, Metric}; use relay_quotas::{DataCategory, ReasonCode}; use relay_redis::RedisPool; -use relay_sampling::{DynamicSamplingContext, RuleId}; +use relay_sampling::{DynamicSamplingContext, MatchedRuleIds}; use relay_statsd::metric; use relay_system::{Addr, FromMessage, NoResponse, Service}; @@ -122,7 +122,7 @@ pub enum ProcessingError { QuotasFailed(#[from] RateLimitingError), #[error("event dropped by sampling rule {0}")] - Sampled(RuleId), + Sampled(MatchedRuleIds), #[error("invalid pii config")] PiiConfigError(PiiConfigError), @@ -372,9 +372,8 @@ enum ClientReportField { fn outcome_from_parts(field: ClientReportField, reason: &str) -> Result { match field { ClientReportField::FilteredSampling => match reason.strip_prefix("Sampled:") { - Some(rule_id) => rule_id - .parse() - .map(|id| Outcome::FilteredSampling(RuleId(id))) + Some(rule_ids) => MatchedRuleIds::from_string(rule_ids) + .map(Outcome::FilteredSampling) .map_err(|_| ()), None => Err(()), }, @@ -2116,19 +2115,19 @@ impl EnvelopeProcessorService { let client_ip = state.envelope.meta().client_addr(); match utils::should_keep_event( + self.config.processing_enabled(), + &state.project_state, + state.sampling_project_state.as_deref(), state.envelope.dsc(), state.event.value(), client_ip, - &state.project_state, - state.sampling_project_state.as_deref(), - self.config.processing_enabled(), ) { - SamplingResult::Drop(rule_id) => { + SamplingResult::Drop(rule_ids) => { state .envelope_context - .reject(Outcome::FilteredSampling(rule_id)); + .reject(Outcome::FilteredSampling(rule_ids.clone())); - Err(ProcessingError::Sampled(rule_id)) + Err(ProcessingError::Sampled(rule_ids)) } SamplingResult::Keep => Ok(()), } @@ -2500,12 +2499,15 @@ impl Service for EnvelopeProcessorService { #[cfg(test)] mod tests { + use similar_asserts::assert_eq; + use std::str::FromStr; use chrono::{DateTime, TimeZone, Utc}; + use relay_general::pii::{DataScrubbingConfig, PiiConfig}; use relay_general::protocol::EventId; - use relay_sampling::{RuleCondition, RuleType, SamplingMode}; + use relay_sampling::{RuleCondition, RuleId, RuleType, SamplingMode}; use crate::service::ServiceState; use crate::testutils::{new_envelope, state_with_rule_and_condition}; @@ -3180,18 +3182,44 @@ mod tests { outcome_from_parts(ClientReportField::FilteredSampling, "adsf"), Err(_) )); + assert!(matches!( outcome_from_parts(ClientReportField::FilteredSampling, "Sampled:"), Err(_) )); + assert!(matches!( outcome_from_parts(ClientReportField::FilteredSampling, "Sampled:foo"), Err(_) )); + assert!(matches!( - outcome_from_parts(ClientReportField::FilteredSampling, "Sampled:123"), - Ok(Outcome::FilteredSampling(RuleId(123))) + outcome_from_parts(ClientReportField::FilteredSampling, "Sampled:"), + Err(()) )); + + assert!(matches!( + outcome_from_parts(ClientReportField::FilteredSampling, "Sampled:;"), + Err(()) + )); + + assert!(matches!( + outcome_from_parts(ClientReportField::FilteredSampling, "Sampled:ab;12"), + Err(()) + )); + + assert_eq!( + outcome_from_parts(ClientReportField::FilteredSampling, "Sampled:123,456"), + Ok(Outcome::FilteredSampling(MatchedRuleIds(vec![ + RuleId(123), + RuleId(456) + ]))) + ); + + assert_eq!( + outcome_from_parts(ClientReportField::FilteredSampling, "Sampled:123"), + Ok(Outcome::FilteredSampling(MatchedRuleIds(vec![RuleId(123)]))) + ); } #[test] diff --git a/relay-server/src/testutils.rs b/relay-server/src/testutils.rs index e55e96c0ca..433844613e 100644 --- a/relay-server/src/testutils.rs +++ b/relay-server/src/testutils.rs @@ -2,7 +2,7 @@ use bytes::Bytes; use relay_general::protocol::EventId; use relay_sampling::{ DynamicSamplingContext, RuleCondition, RuleId, RuleType, SamplingConfig, SamplingMode, - SamplingRule, + SamplingRule, SamplingValue, }; use crate::actors::project::ProjectState; @@ -17,7 +17,7 @@ pub fn state_with_rule_and_condition( let rules = match sample_rate { Some(sample_rate) => vec![SamplingRule { condition, - sample_rate, + sampling_value: SamplingValue::SampleRate { value: sample_rate }, ty: rule_type, id: RuleId(1), time_range: Default::default(), @@ -26,42 +26,19 @@ pub fn state_with_rule_and_condition( None => Vec::new(), }; - state_with_config(SamplingConfig { - rules, + project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: rules, mode, - next_id: None, }) } -pub fn state_with_config(sampling_config: SamplingConfig) -> ProjectState { +pub fn project_state_with_config(sampling_config: SamplingConfig) -> ProjectState { let mut state = ProjectState::allowed(); state.config.dynamic_sampling = Some(sampling_config); state } -pub fn state_with_rule( - sample_rate: Option, - rule_type: RuleType, - mode: SamplingMode, -) -> ProjectState { - let rules = match sample_rate { - Some(sample_rate) => vec![SamplingRule { - condition: RuleCondition::all(), - sample_rate, - ty: rule_type, - id: RuleId(1), - time_range: Default::default(), - decaying_fn: relay_sampling::DecayingFunction::Constant, - }], - None => Vec::new(), - }; - - state_with_config(SamplingConfig { - rules, - mode, - next_id: None, - }) -} pub fn create_sampling_context(sample_rate: Option) -> DynamicSamplingContext { DynamicSamplingContext { trace_id: uuid::Uuid::new_v4(), diff --git a/relay-server/src/utils/dynamic_sampling.rs b/relay-server/src/utils/dynamic_sampling.rs index 7a2a4934b3..d36b172d4d 100644 --- a/relay-server/src/utils/dynamic_sampling.rs +++ b/relay-server/src/utils/dynamic_sampling.rs @@ -1,170 +1,141 @@ //! Functionality for calculating if a trace should be processed or dropped. //! -use chrono::{DateTime, Utc}; use std::net::IpAddr; -use relay_common::{ProjectKey, Uuid}; +use chrono::{DateTime, Utc}; + +use relay_common::ProjectKey; use relay_general::protocol::Event; -use relay_sampling::{DynamicSamplingContext, RuleId, SamplingConfig, SamplingMode}; +use relay_sampling::{ + merge_rules_from_configs, DynamicSamplingContext, MatchedRuleIds, SamplingConfig, + SamplingMatch, SamplingMode, +}; use crate::actors::project::ProjectState; use crate::envelope::{Envelope, ItemType}; -macro_rules! or_ok_none { - ($e:expr) => { - match $e { - Some(x) => x, - None => return Ok(None), - } - }; -} - /// The result of a sampling operation. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum SamplingResult { /// Keep the event. Relay either applied a sampling rule or was unable to parse all rules (so /// it bailed out) Keep, /// Drop the event, due to the rule with provided identifier. - Drop(RuleId), + Drop(MatchedRuleIds), } +/// Checks whether unsupported rules result in a direct keep of the event or depending on the +/// type of Relay an ignore of unsupported rules. fn check_unsupported_rules( processing_enabled: bool, sampling_config: &SamplingConfig, -) -> Result<(), SamplingResult> { - // when we have unsupported rules disable sampling for non processing relays - if sampling_config.has_unsupported_rules() { + root_sampling_config: Option<&SamplingConfig>, +) -> Option<()> { + // When we have unsupported rules disable sampling for non processing relays. + if sampling_config.has_unsupported_rules() + || root_sampling_config.map_or(false, |config| config.has_unsupported_rules()) + { if !processing_enabled { - return Err(SamplingResult::Keep); + return None; } else { relay_log::error!("found unsupported rules even as processing relay"); } } - Ok(()) -} - -#[derive(Clone, Debug, PartialEq)] -struct SamplingSpec { - sample_rate: f64, - rule_id: RuleId, - seed: Uuid, + Some(()) } -fn get_trace_sampling_rule( +/// Gets the sampling match result by creating the merged configuration and matching it against +/// the sampling configuration. +fn get_sampling_match_result( processing_enabled: bool, - sampling_project_state: Option<&ProjectState>, + project_state: &ProjectState, + root_project_state: Option<&ProjectState>, dsc: Option<&DynamicSamplingContext>, + event: Option<&Event>, ip_addr: Option, now: DateTime, -) -> Result, SamplingResult> { - let dsc = or_ok_none!(dsc); - - if sampling_project_state.is_none() { - relay_log::trace!("found sampling context, but no corresponding project state"); - } - let sampling_project_state = or_ok_none!(sampling_project_state); - let sampling_config = or_ok_none!(&sampling_project_state.config.dynamic_sampling); - check_unsupported_rules(processing_enabled, sampling_config)?; - - let rule = or_ok_none!(sampling_config.get_matching_trace_rule(dsc, ip_addr, now)); - let sample_rate = match sampling_config.mode { - SamplingMode::Received => rule.get_sample_rate(now), - SamplingMode::Total => dsc.adjusted_sample_rate(rule.get_sample_rate(now)), +) -> Option { + // We get all the required data for transaction-based dynamic sampling. + let event = event?; + let sampling_config = project_state.config.dynamic_sampling.as_ref()?; + let root_sampling_config = + root_project_state.and_then(|state| state.config.dynamic_sampling.as_ref()); + + // We check if there are unsupported rules in any of the two configurations. + check_unsupported_rules(processing_enabled, sampling_config, root_sampling_config)?; + + // We perform the rule matching with the multi-matching logic on the merged rules. + let rules = merge_rules_from_configs(sampling_config, root_sampling_config); + let mut match_result = SamplingMatch::match_against_rules(rules, event, dsc, ip_addr, now)?; + + // If we have a match, we will try to derive the sample rate based on the sampling mode. + // + // Keep in mind that the sample rate received here has already been derived by the matching + // logic, based on multiple matches and decaying functions. + // + // We also decide to use the sampling mode of the project to which the event belongs. + match_result.set_sample_rate(match sampling_config.mode { + SamplingMode::Received => match_result.sample_rate, + SamplingMode::Total => match dsc { + Some(dsc) => dsc.adjusted_sample_rate(match_result.sample_rate), + None => match_result.sample_rate, + }, SamplingMode::Unsupported => { if processing_enabled { - relay_log::error!("Found unsupported sampling mode even as processing Relay, keep"); + relay_log::error!("found unsupported sampling mode even as processing Relay"); } - return Err(SamplingResult::Keep); + + return None; } - }; + }); - Ok(Some(SamplingSpec { - sample_rate, - rule_id: rule.id, - seed: dsc.trace_id, - })) + // Only if we arrive at this stage, it means that we have found a match and we want to prepare + // the data for making the sampling decision. + Some(match_result) } -fn get_event_sampling_rule( +/// Checks whether an incoming event should be kept or dropped based on the result of the sampling +/// configuration match. +pub fn should_keep_event( processing_enabled: bool, project_state: &ProjectState, + root_project_state: Option<&ProjectState>, dsc: Option<&DynamicSamplingContext>, event: Option<&Event>, ip_addr: Option, - now: DateTime, -) -> Result, SamplingResult> { - let event = or_ok_none!(event); - let event_id = or_ok_none!(event.id.value()); - - let sampling_config = or_ok_none!(&project_state.config.dynamic_sampling); - check_unsupported_rules(processing_enabled, sampling_config)?; - - let rule = or_ok_none!(sampling_config.get_matching_event_rule(event, ip_addr, now)); - let sample_rate = match (dsc, sampling_config.mode) { - (Some(dsc), SamplingMode::Total) => dsc.adjusted_sample_rate(rule.get_sample_rate(now)), - _ => rule.get_sample_rate(now), - }; - - Ok(Some(SamplingSpec { - sample_rate, - rule_id: rule.id, - seed: event_id.0, - })) -} - -/// Checks whether an event should be kept or removed by dynamic sampling. -/// -/// This runs both trace- and event/transaction/error-based rules at once. -pub fn should_keep_event( - dsc: Option<&DynamicSamplingContext>, - event: Option<&Event>, - ip_addr: Option, - project_state: &ProjectState, - sampling_project_state: Option<&ProjectState>, - processing_enabled: bool, ) -> SamplingResult { - // For consistency reasons we take a snapshot in time and use that time across all code that - // requires it. - let now = Utc::now(); - - let matching_trace_rule = match get_trace_sampling_rule( - processing_enabled, - sampling_project_state, - dsc, - ip_addr, - now, - ) { - Ok(spec) => spec, - Err(sampling_result) => return sampling_result, - }; - - let matching_event_rule = match get_event_sampling_rule( + match get_sampling_match_result( processing_enabled, project_state, + root_project_state, dsc, event, ip_addr, - now, + // For consistency reasons we take a snapshot in time and use that time across all code that + // requires it. + Utc::now(), ) { - Ok(spec) => spec, - Err(sampling_result) => return sampling_result, - }; - - // NOTE: Event rules take precedence over trace rules. If the event rule has a lower sample rate - // than the trace rule, this means that traces will be incomplete. - // We could guarantee consistent traces if trace rules took precedence over event rules, - // but we need the current behavior to allow health check rules - // to take precedence over the overall base rate, which is set on the trace. - if let Some(spec) = matching_event_rule.or(matching_trace_rule) { - let random_number = relay_sampling::pseudo_random_from_uuid(spec.seed); - if random_number >= spec.sample_rate { - return SamplingResult::Drop(spec.rule_id); + Some(SamplingMatch { + sample_rate, + matched_rule_ids, + seed, + }) => { + let random_number = relay_sampling::pseudo_random_from_uuid(seed); + relay_log::trace!("sampling event with sample rate {}", sample_rate); + if random_number >= sample_rate { + relay_log::trace!("dropping event that matched the configuration"); + SamplingResult::Drop(matched_rule_ids) + } else { + relay_log::trace!("keeping event that matched the configuration"); + SamplingResult::Keep + } + } + None => { + relay_log::trace!("keeping event that didn't match the configuration"); + SamplingResult::Keep } } - - SamplingResult::Keep } /// Returns the project key defined in the `trace` header of the envelope. @@ -183,762 +154,760 @@ pub fn get_sampling_key(envelope: &Envelope) -> Option { #[cfg(test)] mod tests { - - use chrono::DateTime; + use similar_asserts::assert_eq; use chrono::Duration as DateDuration; + use uuid::Uuid; + use relay_common::EventType; - use relay_general::protocol::EventId; + use relay_general::protocol::{EventId, LenientString}; use relay_general::types::Annotated; use relay_sampling::{ - DecayingFunction, EqCondition, RuleCondition, RuleId, RuleType, SamplingConfig, - SamplingRule, TimeRange, + DecayingFunction, EqCondOptions, EqCondition, RuleCondition, RuleId, RuleType, + SamplingConfig, SamplingMatch, SamplingRule, SamplingValue, TimeRange, }; - use crate::testutils::create_sampling_context; - use crate::testutils::new_envelope; - use crate::testutils::state_with_config; - use crate::testutils::state_with_rule; - use crate::testutils::state_with_rule_and_condition; + use crate::testutils::project_state_with_config; use super::*; - fn state_with_decaying_rule( - sample_rate: Option, - rule_type: RuleType, - mode: SamplingMode, - decaying_fn: DecayingFunction, - start: Option>, - end: Option>, - ) -> ProjectState { - let rules = match sample_rate { - Some(sample_rate) => vec![SamplingRule { - condition: RuleCondition::all(), - sample_rate, - ty: rule_type, - id: RuleId(1), - time_range: TimeRange { start, end }, - decaying_fn, - }], - None => Vec::new(), - }; + macro_rules! assert_transaction_match { + ($res:expr, $sr:expr, $sd:expr, $( $id:expr ),*) => { + assert_eq!( + $res, + Some(SamplingMatch { + sample_rate: $sr, + seed: $sd.id.value().unwrap().0, + matched_rule_ids: MatchedRuleIds(vec![$(RuleId($id),)*]) + } ) + + ) + } + } - state_with_config(SamplingConfig { - rules, - mode, - next_id: None, + macro_rules! assert_trace_match { + ($res:expr, $sr:expr, $sd:expr, $( $id:expr ),*) => { + assert_eq!( + $res, + Some(SamplingMatch { + sample_rate: $sr, + seed: $sd.trace_id, + matched_rule_ids: MatchedRuleIds(vec![$(RuleId($id),)*]) }) + + ) + } } - fn prepare_and_get_sampling_rule( - client_sample_rate: f64, - event_type: EventType, - project_state: &ProjectState, - now: DateTime, - ) -> Result, SamplingResult> { - let sampling_context = create_sampling_context(Some(client_sample_rate)); - let event = Event { - id: Annotated::new(EventId::new()), - ty: Annotated::new(event_type), - ..Event::default() + macro_rules! assert_no_match { + ($res:expr) => { + assert_eq!($res, None) }; + } - get_event_sampling_rule( - true, // irrelevant, just skips unsupported rules - project_state, - Some(&sampling_context), - Some(&event), - None, // ip address not needed for uniform rule - now, - ) + fn eq(name: &str, value: &[&str], ignore_case: bool) -> RuleCondition { + RuleCondition::Eq(EqCondition { + name: name.to_owned(), + value: value.iter().map(|s| s.to_string()).collect(), + options: EqCondOptions { ignore_case }, + }) } - fn samplingresult_from_rules_and_processing_flag( - rules: Vec, - processing_enabled: bool, - ) -> SamplingResult { - let event_state = state_with_config(SamplingConfig { - rules, - mode: SamplingMode::Received, - next_id: None, - }); + fn mocked_dynamic_sampling_context( + sample_rate: Option, + release: Option<&str>, + transaction: Option<&str>, + environment: Option<&str>, + ) -> DynamicSamplingContext { + DynamicSamplingContext { + trace_id: Uuid::new_v4(), + public_key: "12345678901234567890123456789012".parse().unwrap(), + release: release.map(|value| value.to_string()), + environment: environment.map(|value| value.to_string()), + transaction: transaction.map(|value| value.to_string()), + sample_rate, + user: Default::default(), + other: Default::default(), + } + } - let some_event = Event { + fn mocked_event(event_type: EventType, transaction: &str, release: &str) -> Event { + Event { id: Annotated::new(EventId::new()), - ty: Annotated::new(EventType::Transaction), - transaction: Annotated::new("testing".to_owned()), + ty: Annotated::new(event_type), + transaction: Annotated::new(transaction.to_string()), + release: Annotated::new(LenientString(release.to_string())), ..Event::default() - }; + } + } - let some_envelope = new_envelope(true, "testing"); + fn mocked_project_state(mode: SamplingMode) -> ProjectState { + project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![ + SamplingRule { + condition: eq("event.transaction", &["healthcheck"], true), + sampling_value: SamplingValue::SampleRate { value: 0.1 }, + ty: RuleType::Transaction, + id: RuleId(1), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + SamplingRule { + condition: eq("event.transaction", &["bar"], true), + sampling_value: SamplingValue::Factor { value: 1.0 }, + ty: RuleType::Transaction, + id: RuleId(2), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + SamplingRule { + condition: eq("event.transaction", &["foo"], true), + sampling_value: SamplingValue::SampleRate { value: 0.5 }, + ty: RuleType::Transaction, + id: RuleId(3), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + // We put this trace rule here just for testing purposes, even though it will never + // be considered if put within a non-root project. + SamplingRule { + condition: RuleCondition::all(), + sampling_value: SamplingValue::SampleRate { value: 0.5 }, + ty: RuleType::Trace, + id: RuleId(4), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + ], + mode, + }) + } - should_keep_event( - some_envelope.dsc(), - Some(&some_event), - None, - &event_state, - None, - processing_enabled, - ) + fn mocked_root_project_state(mode: SamplingMode) -> ProjectState { + project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![ + SamplingRule { + condition: eq("trace.release", &["3.0"], true), + sampling_value: SamplingValue::Factor { value: 1.5 }, + ty: RuleType::Trace, + id: RuleId(5), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + SamplingRule { + condition: eq("trace.environment", &["dev"], true), + sampling_value: SamplingValue::SampleRate { value: 1.0 }, + ty: RuleType::Trace, + id: RuleId(6), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + SamplingRule { + condition: RuleCondition::all(), + sampling_value: SamplingValue::SampleRate { value: 0.5 }, + ty: RuleType::Trace, + id: RuleId(7), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + ], + mode, + }) } - /// Checks that events aren't dropped if they contain an unsupported rule, - /// checks the cases with and without the process_enabled flag - #[test] - fn test_bad_dynamic_rules() { - // adds a rule which should always match (meaning the event will be dropped) - let mut rules = vec![SamplingRule { + fn mocked_sampling_rule(id: u32, ty: RuleType, sample_rate: f64) -> SamplingRule { + SamplingRule { condition: RuleCondition::all(), - sample_rate: 0.0, - ty: RuleType::Transaction, - id: RuleId(1), + sampling_value: SamplingValue::SampleRate { value: sample_rate }, + ty, + id: RuleId(id), time_range: Default::default(), decaying_fn: Default::default(), - }]; - - // ensures the event is indeed dropped with and without processing enabled - let res = samplingresult_from_rules_and_processing_flag(rules.clone(), false); - assert!(matches!(res, SamplingResult::Drop(_))); - - let res = samplingresult_from_rules_and_processing_flag(rules.clone(), true); - assert!(matches!(res, SamplingResult::Drop(_))); + } + } - rules.push(SamplingRule { - condition: RuleCondition::Unsupported, - sample_rate: 0.0, + fn mocked_decaying_sampling_rule( + id: u32, + start: Option>, + end: Option>, + sampling_value: SamplingValue, + decaying_fn: DecayingFunction, + ) -> SamplingRule { + SamplingRule { + condition: RuleCondition::all(), + sampling_value, ty: RuleType::Transaction, - id: RuleId(1), - time_range: Default::default(), - decaying_fn: Default::default(), - }); - - // now that an unsupported rule has been pushed, it should keep the event if processing is disabled - let res = samplingresult_from_rules_and_processing_flag(rules.clone(), false); - assert!(matches!(res, SamplingResult::Keep)); + id: RuleId(id), + time_range: TimeRange { start, end }, + decaying_fn, + } + } - let res = samplingresult_from_rules_and_processing_flag(rules, true); - assert!(matches!(res, SamplingResult::Drop(_))); // should also log an error + fn add_sampling_rule_to_project_state( + project_state: &mut ProjectState, + sampling_rule: SamplingRule, + ) { + project_state + .config + .dynamic_sampling + .as_mut() + .unwrap() + .rules_v2 + .push(sampling_rule); } #[test] - fn test_trace_rules_applied_after_event_rules() { - // a transaction rule that drops everything - let event_state = state_with_rule_and_condition( - Some(0.0), - RuleType::Transaction, - SamplingMode::Received, - RuleCondition::Eq(EqCondition { - name: "event.transaction".to_owned(), - value: "healthcheck".into(), - options: Default::default(), - }), - ); - - // a trace rule that keeps everything - let trace_state = state_with_rule(Some(1.0), RuleType::Trace, SamplingMode::Received); - - let healthcheck_envelope = new_envelope(true, "healthcheck"); - let other_envelope = new_envelope(true, "test1"); - - let healthcheck_event = Event { - id: Annotated::new(EventId::new()), - ty: Annotated::new(EventType::Transaction), - transaction: Annotated::new("healthcheck".to_owned()), - ..Event::default() - }; - - let other_event = Event { - id: Annotated::new(EventId::new()), - ty: Annotated::new(EventType::Transaction), - transaction: Annotated::new("test1".to_owned()), - ..Event::default() - }; + /// Tests that no match is done when there are no matching rules. + fn test_get_sampling_match_result_with_no_match() { + let project_state = mocked_project_state(SamplingMode::Received); + let event = mocked_event(EventType::Transaction, "transaction", "2.0"); - // if it matches the transaction rule, the transaction should be dropped - let should_drop = should_keep_event( - healthcheck_envelope.dsc(), - Some(&healthcheck_event), + let result = get_sampling_match_result( + true, + &project_state, None, - &event_state, - Some(&trace_state), - false, - ); - - // if it doesn't match the transaction rule, the transaction shouldn't be dropped - let should_keep = should_keep_event( - other_envelope.dsc(), - Some(&other_event), None, - &event_state, - Some(&trace_state), - false, - ); - - let now = Utc::now(); - - // matching event should return an event rule - assert!(get_event_sampling_rule( - false, - &event_state, - healthcheck_envelope.dsc(), - Some(&healthcheck_event), - None, - now - ) - .unwrap() - .is_some()); - - // non-matching event should not return an event rule - assert!(get_event_sampling_rule( - false, - &event_state, - other_envelope.dsc(), - Some(&other_event), + Some(&event), None, - now - ) - .unwrap() - .is_none()); - - assert!(matches!(should_keep, SamplingResult::Keep)); - assert!(matches!(should_drop, SamplingResult::Drop(_))); - } - - #[test] - /// Should_keep_event returns the expected results. - fn test_should_keep_event() { - let event = Event { - id: Annotated::new(EventId::new()), - ty: Annotated::new(EventType::Error), - ..Event::default() - }; - - let proj_state = state_with_rule(Some(0.0), RuleType::Error, SamplingMode::default()); - - assert_eq!( - SamplingResult::Drop(RuleId(1)), - should_keep_event(None, Some(&event), None, &proj_state, None, true) - ); - let proj_state = state_with_rule(Some(1.0), RuleType::Error, SamplingMode::default()); - assert_eq!( - SamplingResult::Keep, - should_keep_event(None, Some(&event), None, &proj_state, None, true) - ); - let proj_state = state_with_rule(None, RuleType::Error, SamplingMode::default()); - assert_eq!( - SamplingResult::Keep, - should_keep_event(None, Some(&event), None, &proj_state, None, true) + Utc::now(), ); + assert_no_match!(result); } #[test] - fn test_unsampled_envelope_with_sample_rate() { - //create an envelope with a event and a transaction - let envelope = new_envelope(true, ""); - let state = state_with_rule(Some(1.0), RuleType::Trace, SamplingMode::default()); - let sampling_state = state_with_rule(Some(0.0), RuleType::Trace, SamplingMode::default()); - let result = should_keep_event( - envelope.dsc(), + /// Tests that matching is still done on the transaction rules in case trace params are invalid. + fn test_get_sampling_match_result_with_invalid_trace_params() { + let project_state = mocked_project_state(SamplingMode::Received); + let root_project_state = mocked_root_project_state(SamplingMode::Received); + let dsc = mocked_dynamic_sampling_context(Some(1.0), Some("1.0"), None, None); + + let event = mocked_event(EventType::Transaction, "foo", "2.0"); + let result = get_sampling_match_result( + true, + &project_state, + Some(&root_project_state), None, + Some(&event), None, - &state, - Some(&sampling_state), - true, + Utc::now(), ); - assert_eq!(result, SamplingResult::Drop(RuleId(1))); - } + assert_transaction_match!(result, 0.5, event, 3); - #[test] - /// Should keep transaction when no trace context is present - fn test_should_keep_transaction_no_trace() { - //create an envelope with a event and a transaction - let envelope = new_envelope(false, ""); - let state = state_with_rule(Some(1.0), RuleType::Trace, SamplingMode::default()); - let sampling_state = state_with_rule(Some(0.0), RuleType::Trace, SamplingMode::default()); - - let result = should_keep_event( - envelope.dsc(), + let event = mocked_event(EventType::Transaction, "healthcheck", "2.0"); + let result = get_sampling_match_result( + true, + &project_state, None, + Some(&dsc), + Some(&event), None, - &state, - Some(&sampling_state), - true, + Utc::now(), ); - assert_eq!(result, SamplingResult::Keep); - // both the event and the transaction item should have been left in the envelope - assert_eq!(envelope.len(), 3); + assert_transaction_match!(result, 0.1, event, 1); } #[test] - /// When the envelope becomes empty due to sampling we should get back the rule that dropped the - /// transaction - fn test_should_signal_when_envelope_becomes_empty() { - //create an envelope with a event and a transaction - let envelope = new_envelope(true, ""); - let state = state_with_rule(Some(1.0), RuleType::Trace, SamplingMode::default()); - let sampling_state = state_with_rule(Some(0.0), RuleType::Trace, SamplingMode::default()); - - let result = should_keep_event( - envelope.dsc(), - None, - None, - &state, - Some(&sampling_state), + /// Tests that a match with early return is done in the project sampling config. + fn test_get_sampling_match_result_with_project_config_match() { + let project_state = mocked_project_state(SamplingMode::Received); + let root_project_state = mocked_root_project_state(SamplingMode::Received); + let dsc = mocked_dynamic_sampling_context(Some(1.0), Some("1.0"), None, None); + let event = mocked_event(EventType::Transaction, "healthcheck", "2.0"); + + let result = get_sampling_match_result( true, + &project_state, + Some(&root_project_state), + Some(&dsc), + Some(&event), + None, + Utc::now(), ); - assert_eq!(result, SamplingResult::Drop(RuleId(1))); + assert_transaction_match!(result, 0.1, event, 1); } #[test] - /// When there's a mixture of event rules and trace rules, the event rules - /// take precedence. - fn test_event_rule_precedence() { - let sampling_config = serde_json::json!( - { - "rules": [ - { - "sampleRate": 0, - "type": "trace", - "active": true, - "condition": { - "op": "and", - "inner": [] - }, - "id": 1000 - }, - { - "sampleRate": 1, - "type": "transaction", - "condition": { - "op": "or", - "inner": [ - { - "op": "glob", - "name": "event.transaction", - "value": [ - "my-important-transaction", - ], - "options": { - "ignoreCase": true - } - } - ] - }, - "active": true, - "id": 1002 - } - ] - } - ); - - let sampling_config = serde_json::from_value(sampling_config).unwrap(); - let project_state = state_with_config(sampling_config); - - let envelope = new_envelope(true, ""); - - let event = Event { - id: Annotated::new(EventId::new()), - ty: Annotated::new(EventType::Transaction), - transaction: Annotated::new("my-important-transaction".to_owned()), - ..Event::default() - }; - - let keep_event = should_keep_event( - envelope.dsc(), + /// Tests that a match with early return is done in the root project sampling config. + fn test_get_sampling_match_result_with_root_project_config_match() { + let project_state = mocked_project_state(SamplingMode::Received); + let root_project_state = mocked_root_project_state(SamplingMode::Received); + let dsc = mocked_dynamic_sampling_context(Some(1.0), Some("1.0"), None, Some("dev")); + let event = mocked_event(EventType::Transaction, "my_transaction", "2.0"); + + let result = get_sampling_match_result( + true, + &project_state, + Some(&root_project_state), + Some(&dsc), Some(&event), None, - &project_state, - Some(&project_state), - true, + Utc::now(), ); - - assert_eq!(keep_event, SamplingResult::Keep); + assert_trace_match!(result, 1.0, dsc, 6); } #[test] - fn test_trace_rule_received() { - let project_state = state_with_rule(Some(0.1), RuleType::Trace, SamplingMode::Received); - let sampling_context = create_sampling_context(Some(0.5)); - let spec = get_trace_sampling_rule( - true, // irrelevant, just skips unsupported rules - Some(&project_state), - Some(&sampling_context), + /// Tests that the multiple matches are done across root and non-root project sampling configs. + fn test_get_sampling_match_result_with_both_project_configs_match() { + let project_state = mocked_project_state(SamplingMode::Received); + let root_project_state = mocked_root_project_state(SamplingMode::Received); + let dsc = mocked_dynamic_sampling_context(Some(1.0), Some("3.0"), None, None); + let event = mocked_event(EventType::Transaction, "bar", "2.0"); + + let result = get_sampling_match_result( + true, + &project_state, + Some(&root_project_state), + Some(&dsc), + Some(&event), None, Utc::now(), ); - - assert_eq!(spec.unwrap().unwrap().sample_rate, 0.1); + assert_trace_match!(result, 0.75, dsc, 2, 5, 7); } #[test] - fn test_trace_rule_adjusted() { - let project_state = state_with_rule(Some(0.1), RuleType::Trace, SamplingMode::Total); - let sampling_context = create_sampling_context(Some(0.5)); - let spec = get_trace_sampling_rule( - true, // irrelevant, just skips unsupported rules - Some(&project_state), - Some(&sampling_context), + /// Tests that a match is done when no dynamic sampling context and root project state are + /// available. + fn test_get_sampling_match_result_with_no_dynamic_sampling_context_and_no_root_project_state() { + let project_state = mocked_project_state(SamplingMode::Received); + let event = mocked_event(EventType::Transaction, "foo", "1.0"); + + let result = get_sampling_match_result( + true, + &project_state, + None, + None, + Some(&event), None, Utc::now(), ); - - assert_eq!(spec.unwrap().unwrap().sample_rate, 0.2); + assert_transaction_match!(result, 0.5, event, 3); } #[test] - fn test_trace_rule_unsupported() { - let project_state = state_with_rule(Some(0.1), RuleType::Trace, SamplingMode::Unsupported); - let sampling_context = create_sampling_context(Some(0.5)); - let spec = get_trace_sampling_rule( + /// Tests that a match is done and the sample rate is adjusted when sampling mode is total. + fn test_get_sampling_match_result_with_total_sampling_mode_in_project_state() { + let project_state = mocked_project_state(SamplingMode::Total); + let root_project_state = mocked_root_project_state(SamplingMode::Total); + let dsc = mocked_dynamic_sampling_context(Some(0.8), Some("1.0"), None, None); + let event = mocked_event(EventType::Transaction, "foo", "2.0"); + + let result = get_sampling_match_result( true, - Some(&project_state), - Some(&sampling_context), + &project_state, + Some(&root_project_state), + Some(&dsc), + Some(&event), None, Utc::now(), ); - - assert!(matches!(spec, Err(SamplingResult::Keep))); + assert_transaction_match!(result, 0.625, event, 3); } #[test] - fn test_event_rule_received() { - let project_state = - state_with_rule(Some(0.1), RuleType::Transaction, SamplingMode::Received); - let sampling_context = create_sampling_context(Some(0.5)); - let event = Event { - id: Annotated::new(EventId::new()), - ty: Annotated::new(EventType::Transaction), - ..Event::default() - }; + /// Tests that the correct match is raised in case we have unsupported rules with processing both + /// enabled and disabled. + fn test_get_sampling_match_result_with_unsupported_rules() { + let mut project_state = mocked_project_state(SamplingMode::Received); + add_sampling_rule_to_project_state( + &mut project_state, + SamplingRule { + condition: RuleCondition::Unsupported, + sampling_value: SamplingValue::SampleRate { value: 0.5 }, + ty: RuleType::Transaction, + id: RuleId(1), + time_range: Default::default(), + decaying_fn: Default::default(), + }, + ); + + let root_project_state = mocked_root_project_state(SamplingMode::Received); + let dsc = mocked_dynamic_sampling_context(Some(1.0), Some("1.0"), None, None); + let event = mocked_event(EventType::Transaction, "foo", "2.0"); - let spec = get_event_sampling_rule( - true, // irrelevant, just skips unsupported rules + let result = get_sampling_match_result( + false, &project_state, - Some(&sampling_context), + Some(&root_project_state), + Some(&dsc), Some(&event), - None, // ip address not needed for uniform rule + None, Utc::now(), ); + assert_no_match!(result); - assert_eq!(spec.unwrap().unwrap().sample_rate, 0.1); + let result = get_sampling_match_result( + true, + &project_state, + Some(&root_project_state), + Some(&dsc), + Some(&event), + None, + Utc::now(), + ); + assert_transaction_match!(result, 0.5, event, 3); } #[test] - fn test_event_rule_adjusted() { - let project_state = state_with_rule(Some(0.1), RuleType::Transaction, SamplingMode::Total); - let sampling_context = create_sampling_context(Some(0.5)); - let event = Event { - id: Annotated::new(EventId::new()), - ty: Annotated::new(EventType::Transaction), - ..Event::default() - }; - - let spec = get_event_sampling_rule( - true, // irrelevant, just skips unsupported rules + /// Tests that a no match is raised in case we have an unsupported sampling mode and a match. + fn test_get_sampling_match_result_with_unsupported_sampling_mode_and_match() { + let project_state = mocked_project_state(SamplingMode::Unsupported); + let root_project_state = mocked_root_project_state(SamplingMode::Unsupported); + let dsc = mocked_dynamic_sampling_context(Some(1.0), Some("1.0"), None, None); + let event = mocked_event(EventType::Transaction, "foo", "2.0"); + + let result = get_sampling_match_result( + true, &project_state, - Some(&sampling_context), + Some(&root_project_state), + Some(&dsc), Some(&event), - None, // ip address not needed for uniform rule + None, Utc::now(), ); - - assert_eq!(spec.unwrap().unwrap().sample_rate, 0.2); + assert_no_match!(result); } #[test] - fn test_sample_rate() { - let event_state_drop = state_with_rule_and_condition( - Some(0.0), - RuleType::Transaction, - SamplingMode::Received, - RuleCondition::all(), - ); - - let envelope = new_envelope(true, "foo"); + /// Tests that only transaction rules are matched in case no root project or dsc are supplied. + fn test_get_sampling_match_result_with_invalid_root_project_and_dsc_combination() { + let project_state = mocked_project_state(SamplingMode::Received); + let event = mocked_event(EventType::Transaction, "healthcheck", "2.0"); - let event = Event { - id: Annotated::new(EventId::new()), - ty: Annotated::new(EventType::Transaction), - transaction: Annotated::new("foo".to_owned()), - ..Event::default() - }; - - // if it matches the transaction rule, the transaction should be dropped - let should_drop = should_keep_event( - envelope.dsc(), + let dsc = mocked_dynamic_sampling_context(Some(1.0), Some("1.0"), None, None); + let result = get_sampling_match_result( + true, + &project_state, + None, + Some(&dsc), Some(&event), None, - &event_state_drop, - Some(&event_state_drop), - false, - ); - - assert!(matches!(should_drop, SamplingResult::Drop(_))); - - let event_state_keep = state_with_rule_and_condition( - Some(1.0), - RuleType::Transaction, - SamplingMode::Received, - RuleCondition::all(), + Utc::now(), ); + assert_transaction_match!(result, 0.1, event, 1); - let should_keep = should_keep_event( - envelope.dsc(), + let root_project_state = mocked_root_project_state(SamplingMode::Received); + let result = get_sampling_match_result( + true, + &project_state, + Some(&root_project_state), + None, Some(&event), None, - &event_state_keep, - Some(&event_state_keep), - false, - ); - - assert!(matches!(should_keep, SamplingResult::Keep)); - } - #[test] - fn test_event_decaying_rule_with_linear_function() { - let now = Utc::now(); - let project_state = state_with_decaying_rule( - Some(0.7), - RuleType::Transaction, - SamplingMode::Total, - DecayingFunction::Linear { - decayed_sample_rate: 0.2, - }, - Some(now - DateDuration::days(1)), - Some(now + DateDuration::days(1)), + Utc::now(), ); - - let sample_rate = - prepare_and_get_sampling_rule(1.0, EventType::Transaction, &project_state, now) - .unwrap() - .unwrap() - .sample_rate; - let expected_sample_rate = 0.44999999999999996; - - // Workaround against floating point precision differences. - // https://rust-lang.github.io/rust-clippy/master/#float_cmp - assert!((sample_rate - expected_sample_rate).abs() < f64::EPSILON) + assert_transaction_match!(result, 0.1, event, 1); } #[test] - fn test_event_decaying_rule_with_open_time_range_and_linear_function() { - let now = Utc::now(); - let project_state = state_with_decaying_rule( - Some(0.7), - RuleType::Transaction, - SamplingMode::Total, - DecayingFunction::Linear { - decayed_sample_rate: 0.2, + /// Tests that a match of a rule of type error with a transaction event results in no match. + fn test_get_sampling_match_result_with_transaction_event_and_error_rule() { + let mut project_state = mocked_project_state(SamplingMode::Received); + add_sampling_rule_to_project_state( + &mut project_state, + SamplingRule { + condition: RuleCondition::all(), + sampling_value: SamplingValue::SampleRate { value: 0.5 }, + ty: RuleType::Error, + id: RuleId(1), + time_range: Default::default(), + decaying_fn: Default::default(), }, - Some(now - DateDuration::days(1)), - None, ); + let event = mocked_event(EventType::Transaction, "transaction", "2.0"); - assert!( - prepare_and_get_sampling_rule(1.0, EventType::Transaction, &project_state, now) - .unwrap() - .is_none() - ); - - let project_state = state_with_decaying_rule( - Some(0.7), - RuleType::Transaction, - SamplingMode::Total, - DecayingFunction::Linear { - decayed_sample_rate: 0.2, - }, + let result = get_sampling_match_result( + true, + &project_state, None, - Some(now + DateDuration::days(1)), - ); - - assert!( - prepare_and_get_sampling_rule(1.0, EventType::Transaction, &project_state, now) - .unwrap() - .is_none() + None, + Some(&event), + None, + Utc::now(), ); + assert_no_match!(result); } #[test] - fn test_event_decaying_rule_with_no_time_range_and_linear_function() { - let now = Utc::now(); - let project_state = state_with_decaying_rule( - Some(0.7), - RuleType::Transaction, - SamplingMode::Total, - DecayingFunction::Linear { - decayed_sample_rate: 0.2, + /// Tests that a match of a rule of type error with an error event results in a match. + fn test_get_sampling_match_result_with_error_event_and_error_rule() { + let mut project_state = mocked_project_state(SamplingMode::Received); + add_sampling_rule_to_project_state( + &mut project_state, + SamplingRule { + condition: RuleCondition::all(), + sampling_value: SamplingValue::SampleRate { value: 0.5 }, + ty: RuleType::Error, + id: RuleId(10), + time_range: Default::default(), + decaying_fn: Default::default(), }, - None, - None, ); + let event = mocked_event(EventType::Error, "transaction", "2.0"); - assert!( - prepare_and_get_sampling_rule(1.0, EventType::Transaction, &project_state, now) - .unwrap() - .is_none() + let result = get_sampling_match_result( + true, + &project_state, + None, + None, + Some(&event), + None, + Utc::now(), ); + assert_transaction_match!(result, 0.5, event, 10); } #[test] - fn test_event_decaying_rule_with_now_equal_start_and_linear_function() { - let now = Utc::now(); - let project_state = state_with_decaying_rule( - Some(0.7), - RuleType::Transaction, - SamplingMode::Total, - DecayingFunction::Linear { - decayed_sample_rate: 0.2, + /// Tests that a match of a rule of type default with an error event results in a match. + fn test_get_sampling_match_result_with_default_event_and_error_rule() { + let mut project_state = mocked_project_state(SamplingMode::Received); + add_sampling_rule_to_project_state( + &mut project_state, + SamplingRule { + condition: RuleCondition::all(), + sampling_value: SamplingValue::SampleRate { value: 0.5 }, + ty: RuleType::Error, + id: RuleId(10), + time_range: Default::default(), + decaying_fn: Default::default(), }, - Some(now), - Some(now + DateDuration::days(1)), ); + let event = mocked_event(EventType::Default, "transaction", "2.0"); - assert_eq!( - prepare_and_get_sampling_rule(1.0, EventType::Transaction, &project_state, now) - .unwrap() - .unwrap() - .sample_rate, - 0.7 + let result = get_sampling_match_result( + true, + &project_state, + None, + None, + Some(&event), + None, + Utc::now(), ); + assert_transaction_match!(result, 0.5, event, 10); } #[test] - fn test_event_decaying_rule_with_now_equal_end_and_linear_function() { + /// Tests that match is returned with sample rate value interpolated with linear decaying function. + fn test_get_sampling_match_result_with_linear_decaying_function() { let now = Utc::now(); - let project_state = state_with_decaying_rule( - Some(0.7), - RuleType::Transaction, - SamplingMode::Total, - DecayingFunction::Linear { - decayed_sample_rate: 0.2, - }, - Some(now - DateDuration::days(1)), - Some(now), - ); - - assert!( - prepare_and_get_sampling_rule(1.0, EventType::Transaction, &project_state, now) - .unwrap() - .is_none() - ); + let event = mocked_event(EventType::Transaction, "transaction", "2.0"); + + let project_state = project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![mocked_decaying_sampling_rule( + 1, + Some(now - DateDuration::days(1)), + Some(now + DateDuration::days(1)), + SamplingValue::SampleRate { value: 1.0 }, + DecayingFunction::Linear { decayed_value: 0.5 }, + )], + mode: SamplingMode::Received, + }); + let result = + get_sampling_match_result(true, &project_state, None, None, Some(&event), None, now); + assert_transaction_match!(result, 0.75, event, 1); + + let project_state = project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![mocked_decaying_sampling_rule( + 1, + Some(now), + Some(now + DateDuration::days(1)), + SamplingValue::SampleRate { value: 1.0 }, + DecayingFunction::Linear { decayed_value: 0.5 }, + )], + mode: SamplingMode::Received, + }); + let result = + get_sampling_match_result(true, &project_state, None, None, Some(&event), None, now); + assert_transaction_match!(result, 1.0, event, 1); + + let project_state = project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![mocked_decaying_sampling_rule( + 1, + Some(now - DateDuration::days(1)), + Some(now), + SamplingValue::SampleRate { value: 1.0 }, + DecayingFunction::Linear { decayed_value: 0.5 }, + )], + mode: SamplingMode::Received, + }); + let result = + get_sampling_match_result(true, &project_state, None, None, Some(&event), None, now); + assert_no_match!(result); } #[test] - fn test_event_decaying_rule_with_base_less_then_decayed_and_linear_function() { + /// Tests that no match is returned when the linear decaying function has invalid time range. + fn test_get_sampling_match_result_with_linear_decaying_function_and_invalid_time_range() { let now = Utc::now(); - let project_state = state_with_decaying_rule( - Some(0.3), - RuleType::Transaction, - SamplingMode::Total, - DecayingFunction::Linear { - decayed_sample_rate: 0.7, - }, - Some(now - DateDuration::days(1)), - Some(now + DateDuration::days(1)), - ); - - assert!( - prepare_and_get_sampling_rule(1.0, EventType::Transaction, &project_state, now) - .unwrap() - .is_none() - ); + let event = mocked_event(EventType::Transaction, "transaction", "2.0"); + + let project_state = project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![mocked_decaying_sampling_rule( + 1, + Some(now - DateDuration::days(1)), + None, + SamplingValue::SampleRate { value: 1.0 }, + DecayingFunction::Linear { decayed_value: 0.5 }, + )], + mode: SamplingMode::Received, + }); + let result = + get_sampling_match_result(true, &project_state, None, None, Some(&event), None, now); + assert_no_match!(result); + + let project_state = project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![mocked_decaying_sampling_rule( + 1, + None, + Some(now + DateDuration::days(1)), + SamplingValue::SampleRate { value: 1.0 }, + DecayingFunction::Linear { decayed_value: 0.5 }, + )], + mode: SamplingMode::Received, + }); + let result = + get_sampling_match_result(true, &project_state, None, None, Some(&event), None, now); + assert_no_match!(result); + + let project_state = project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![mocked_decaying_sampling_rule( + 1, + None, + None, + SamplingValue::SampleRate { value: 1.0 }, + DecayingFunction::Linear { decayed_value: 0.5 }, + )], + mode: SamplingMode::Received, + }); + let result = + get_sampling_match_result(true, &project_state, None, None, Some(&event), None, now); + assert_no_match!(result); } #[test] - fn test_event_decaying_rule_with_constant_function() { + /// Tests that match is returned when there are multiple decaying rules with factor and sample rate. + fn test_get_sampling_match_result_with_multiple_decaying_functions_with_factor_and_sample_rate() + { let now = Utc::now(); - let project_state = state_with_decaying_rule( - Some(0.6), - RuleType::Transaction, - SamplingMode::Total, - DecayingFunction::Constant, - Some(now - DateDuration::days(1)), - Some(now + DateDuration::days(1)), - ); + let event = mocked_event(EventType::Transaction, "transaction", "2.0"); + + let project_state = project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![ + mocked_decaying_sampling_rule( + 1, + Some(now - DateDuration::days(1)), + Some(now + DateDuration::days(1)), + SamplingValue::Factor { value: 5.0 }, + DecayingFunction::Linear { decayed_value: 1.0 }, + ), + mocked_decaying_sampling_rule( + 2, + Some(now - DateDuration::days(1)), + Some(now + DateDuration::days(1)), + SamplingValue::SampleRate { value: 0.3 }, + DecayingFunction::Constant, + ), + ], + mode: SamplingMode::Received, + }); - assert_eq!( - prepare_and_get_sampling_rule(1.0, EventType::Transaction, &project_state, now) - .unwrap() - .unwrap() - .sample_rate, - 0.6 - ); + let result = + get_sampling_match_result(true, &project_state, None, None, Some(&event), None, now); + assert!(result.is_some()); + if let Some(SamplingMatch { + sample_rate, + seed, + matched_rule_ids, + }) = result + { + assert!((sample_rate - 0.9).abs() < f64::EPSILON); + assert_eq!(seed, event.id.0.unwrap().0); + assert_eq!(matched_rule_ids, MatchedRuleIds(vec![RuleId(1), RuleId(2)])) + } } #[test] - fn test_event_decaying_rule_with_open_time_range_and_constant_function() { - let now = Utc::now(); - let project_state = state_with_decaying_rule( - Some(0.7), - RuleType::Transaction, - SamplingMode::Total, - DecayingFunction::Constant, - Some(now - DateDuration::days(1)), - None, - ); + /// Tests that an event is kept when there is a match and we have 100% sample rate. + fn test_should_keep_event_return_keep_with_match_and_100_sample_rate() { + let project_state = project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![mocked_sampling_rule(1, RuleType::Transaction, 1.0)], + mode: SamplingMode::Received, + }); + let event = mocked_event(EventType::Transaction, "transaction", "2.0"); - assert_eq!( - prepare_and_get_sampling_rule(1.0, EventType::Transaction, &project_state, now) - .unwrap() - .unwrap() - .sample_rate, - 0.7 - ); + let result = should_keep_event(true, &project_state, None, None, Some(&event), None); + assert_eq!(result, SamplingResult::Keep) + } - let project_state = state_with_decaying_rule( - Some(0.7), - RuleType::Transaction, - SamplingMode::Total, - DecayingFunction::Constant, - None, - Some(now + DateDuration::days(1)), - ); + #[test] + /// Tests that an event is dropped when there is a match and we have 0% sample rate. + fn test_should_keep_event_return_drop_with_match_and_0_sample_rate() { + let project_state = project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![mocked_sampling_rule(1, RuleType::Transaction, 0.0)], + mode: SamplingMode::Received, + }); + let event = mocked_event(EventType::Transaction, "transaction", "2.0"); + let result = should_keep_event(true, &project_state, None, None, Some(&event), None); assert_eq!( - prepare_and_get_sampling_rule(1.0, EventType::Transaction, &project_state, now) - .unwrap() - .unwrap() - .sample_rate, - 0.7 - ); + result, + SamplingResult::Drop(MatchedRuleIds(vec![RuleId(1)])) + ) } #[test] - fn test_event_decaying_rule_with_no_time_range_and_constant_function() { - let now = Utc::now(); - let project_state = state_with_decaying_rule( - Some(0.7), - RuleType::Transaction, - SamplingMode::Total, - DecayingFunction::Constant, - None, - None, - ); + /// Tests that an event is kept when there is no match. + fn test_should_keep_event_return_keep_with_no_match() { + let project_state = project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![SamplingRule { + condition: eq("event.transaction", &["foo"], true), + sampling_value: SamplingValue::SampleRate { value: 0.5 }, + ty: RuleType::Transaction, + id: RuleId(3), + time_range: Default::default(), + decaying_fn: Default::default(), + }], + mode: SamplingMode::Received, + }); + let event = mocked_event(EventType::Transaction, "bar", "2.0"); - assert_eq!( - prepare_and_get_sampling_rule(1.0, EventType::Transaction, &project_state, now) - .unwrap() - .unwrap() - .sample_rate, - 0.7 - ); + let result = should_keep_event(true, &project_state, None, None, Some(&event), None); + assert_eq!(result, SamplingResult::Keep) } #[test] - fn test_event_decaying_rule_with_inverse_time_range_and_constant_function() { - let now = Utc::now(); - let project_state = state_with_decaying_rule( - Some(0.6), - RuleType::Transaction, - SamplingMode::Total, - DecayingFunction::Constant, - Some(now + DateDuration::days(1)), - Some(now - DateDuration::days(1)), - ); + /// Tests that an event is kept when there are unsupported rules with no processing and vice versa. + fn test_should_keep_event_return_keep_with_unsupported_rule() { + let project_state = project_state_with_config(SamplingConfig { + rules: vec![], + rules_v2: vec![ + mocked_sampling_rule(1, RuleType::Unsupported, 0.0), + mocked_sampling_rule(2, RuleType::Transaction, 0.0), + ], + mode: SamplingMode::Received, + }); + let event = mocked_event(EventType::Transaction, "transaction", "2.0"); - assert!( - prepare_and_get_sampling_rule(1.0, EventType::Transaction, &project_state, now) - .unwrap() - .is_none() - ); + let result = should_keep_event(false, &project_state, None, None, Some(&event), None); + assert_eq!(result, SamplingResult::Keep); + + let result = should_keep_event(true, &project_state, None, None, Some(&event), None); + assert_eq!( + result, + SamplingResult::Drop(MatchedRuleIds(vec![RuleId(2)])) + ) } } diff --git a/tests/integration/test_dynamic_sampling.py b/tests/integration/test_dynamic_sampling.py index 9a9f577953..df2b55c1ee 100644 --- a/tests/integration/test_dynamic_sampling.py +++ b/tests/integration/test_dynamic_sampling.py @@ -88,7 +88,12 @@ def _add_sampling_config( """ Adds a sampling configuration rule to a project configuration """ - rules = config["config"].setdefault("dynamicSampling", {}).setdefault("rules", []) + 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", []) + if rule_type is None: rule_type = "trace" conditions = [] @@ -125,7 +130,7 @@ def _add_sampling_config( ) rule = { - "sampleRate": sample_rate, + "samplingValue": {"type": "sampleRate", "value": sample_rate}, "type": rule_type, "condition": {"op": "and", "inner": conditions}, "id": len(rules) + 1, @@ -366,7 +371,6 @@ def test_multi_item_envelope(mini_sentry, relay, rule_type, event_factory): config = mini_sentry.add_basic_project_config(project_id) # add a sampling rule to project config that removes all transactions (sample_rate=0) public_key = config["publicKeys"][0]["publicKey"] - # add a sampling rule to project config that drops all events (sample_rate=0), it should be ignored # because there is an invalid rule in the configuration _add_sampling_config(config, sample_rate=0, rule_type=rule_type) @@ -376,6 +380,7 @@ def test_multi_item_envelope(mini_sentry, relay, rule_type, event_factory): envelope = Envelope() # create an envelope with a trace context that is initiated by this project (for simplicity) envelope, trace_id, event_id = event_factory(public_key) + print(envelope) envelope.add_item( Item(payload=PayloadRef(json={"x": "some attachment"}), type="attachment") ) diff --git a/tests/integration/test_metrics.py b/tests/integration/test_metrics.py index bbfbe96362..8aa36e0ef8 100644 --- a/tests/integration/test_metrics.py +++ b/tests/integration/test_metrics.py @@ -580,9 +580,11 @@ def test_transaction_metrics( if discard_data: # Make sure Relay drops the transaction - config.setdefault("dynamicSampling", {}).setdefault("rules", []).append( + ds = config.setdefault("dynamicSampling", {}) + ds.setdefault("rules", []) + ds.setdefault("rulesV2", []).append( { - "sampleRate": 0, + "samplingValue": {"type": "sampleRate", "value": 0.0}, "type": discard_data, "condition": {"op": "and", "inner": []}, "id": 1, diff --git a/tests/integration/test_outcome.py b/tests/integration/test_outcome.py index fcbd003253..b11cfea269 100644 --- a/tests/integration/test_outcome.py +++ b/tests/integration/test_outcome.py @@ -616,15 +616,15 @@ def test_outcomes_rate_limit( def test_outcome_to_client_report(relay, mini_sentry): - # Create project config project_id = 42 project_config = mini_sentry.add_full_project_config(project_id) project_config["config"]["dynamicSampling"] = { - "rules": [ + "rules": [], + "rulesV2": [ { "id": 1, - "sampleRate": 0.0, + "samplingValue": {"type": "sampleRate", "value": 0.0}, "type": "error", "condition": { "op": "eq", @@ -632,7 +632,7 @@ def test_outcome_to_client_report(relay, mini_sentry): "value": "production", }, } - ] + ], } upstream = relay( @@ -787,10 +787,11 @@ 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": [ + "rules": [], + "rulesV2": [ { "id": 1, - "sampleRate": 0.0, + "samplingValue": {"type": "sampleRate", "value": 0.0}, "type": "error", "condition": { "op": "eq", @@ -798,7 +799,7 @@ def test_outcomes_aggregate_dynamic_sampling(relay, mini_sentry): "value": "production", }, } - ] + ], } upstream = relay( @@ -907,10 +908,11 @@ def test_graceful_shutdown(relay, mini_sentry): project_id = 42 project_config = mini_sentry.add_full_project_config(project_id) project_config["config"]["dynamicSampling"] = { - "rules": [ + "rules": [], + "rulesV2": [ { "id": 1, - "sampleRate": 0.0, + "samplingValue": {"type": "sampleRate", "value": 0.0}, "type": "error", "condition": { "op": "eq", @@ -918,7 +920,7 @@ def test_graceful_shutdown(relay, mini_sentry): "value": "production", }, } - ] + ], } relay = relay( diff --git a/tests/integration/test_projectconfigs.py b/tests/integration/test_projectconfigs.py index b4d686e56d..d590e32b6f 100644 --- a/tests/integration/test_projectconfigs.py +++ b/tests/integration/test_projectconfigs.py @@ -249,7 +249,9 @@ def test_unparsable_project_config(mini_sentry, relay): # Config is broken and will produce the invalid project state. config = mini_sentry.project_configs[project_key]["config"] - config.setdefault("dynamicSampling", {}).setdefault("rules", []).append( + ds = config.setdefault("dynamicSampling", {}) + ds.setdefault("rules", []) + ds.setdefault("rulesV2", []).append( { "condition": { "op": "and", @@ -257,7 +259,7 @@ def test_unparsable_project_config(mini_sentry, relay): {"op": "glob", "name": "releases", "value": ["1.1.1", "1.1.2"]} ], }, - "sampleRate": 0.7, + "samplingValue": {"strategy": "sampleRate", "value": 0.7}, "type": "trace", "id": 1, "timeRange": { @@ -304,7 +306,8 @@ def test_unparsable_project_config(mini_sentry, relay): # Fix the config. config = mini_sentry.project_configs[project_key]["config"] - config["dynamicSampling"]["rules"] = [ + config["dynamicSampling"]["rules"] = [] + config["dynamicSampling"]["rulesV2"] = [ { "condition": { "op": "and", @@ -312,14 +315,14 @@ def test_unparsable_project_config(mini_sentry, relay): {"op": "glob", "name": "releases", "value": ["1.1.1", "1.1.2"]} ], }, - "sampleRate": 0.7, + "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", "decayedSampleRate": 0.9}, + "decayingFn": {"type": "linear", "decayedValue": 0.9}, } ] @@ -380,7 +383,9 @@ def test_cached_project_config(mini_sentry, relay): # Introduce unparsable config. config = mini_sentry.project_configs[project_key]["config"] - config.setdefault("dynamicSampling", {}).setdefault("rules", []).append( + ds = config.setdefault("dynamicSampling", {}) + ds.setdefault("rules", []) + ds.setdefault("rulesV2", []).append( { "condition": { "op": "and", @@ -388,7 +393,7 @@ def test_cached_project_config(mini_sentry, relay): {"op": "glob", "name": "releases", "value": ["1.1.1", "1.1.2"]} ], }, - "sampleRate": 0.7, + "samplingValue": {"type": "sampleRate", "value": 0.7}, "type": "trace", "id": 1, "timeRange": {