From 4ed7d5fcec0caa3ca5db9f616527dcd612bfd5e4 Mon Sep 17 00:00:00 2001 From: Riccardo Busetti Date: Mon, 16 Oct 2023 13:22:20 +0200 Subject: [PATCH] ref(rules): Move rules DSL to relay-protocol (#2608) --- CHANGELOG.md | 1 + Cargo.lock | 8 +- relay-cabi/src/processing.rs | 3 +- relay-dynamic-config/Cargo.toml | 1 + relay-dynamic-config/src/defaults.rs | 2 +- relay-dynamic-config/src/metrics.rs | 2 +- relay-protocol/Cargo.toml | 3 + .../src/condition.rs | 100 +++++++++--------- relay-protocol/src/lib.rs | 4 + .../src/utils.rs | 0 relay-sampling/src/config.rs | 7 +- relay-sampling/src/evaluation.rs | 2 +- relay-sampling/src/lib.rs | 2 - relay-server/src/actors/processor.rs | 2 +- relay-server/src/testutils.rs | 2 +- relay-server/src/utils/dynamic_sampling.rs | 2 +- 16 files changed, 77 insertions(+), 64 deletions(-) rename {relay-sampling => relay-protocol}/src/condition.rs (93%) rename {relay-sampling => relay-protocol}/src/utils.rs (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 434a1c0abf..cb9448296d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ - Introduce a dedicated usage metric for transactions that replaces the duration metric. ([#2571](https://github.com/getsentry/relay/pull/2571), [#2589](https://github.com/getsentry/relay/pull/2589)) - Restore the profiling killswitch. ([#2573](https://github.com/getsentry/relay/pull/2573)) - Add `scraping_attempts` field to the event schema. ([#2575](https://github.com/getsentry/relay/pull/2575)) +- Move `condition.rs` from `relay-sampling` to `relay-protocol`. ([#2608](https://github.com/getsentry/relay/pull/2608)) ## 23.9.1 diff --git a/Cargo.lock b/Cargo.lock index 6f00fe7158..e40cbbb76c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3517,6 +3517,7 @@ dependencies = [ "relay-event-normalization", "relay-filter", "relay-pii", + "relay-protocol", "relay-quotas", "relay-sampling", "serde", @@ -3753,13 +3754,16 @@ dependencies = [ name = "relay-protocol" version = "23.9.1" dependencies = [ + "insta", "num-traits", + "relay-common", "relay-protocol-derive", "schemars", "serde", "serde_json", "similar-asserts", "smallvec", + "unicase", "uuid", ] @@ -5515,9 +5519,9 @@ dependencies = [ [[package]] name = "unicase" -version = "2.6.0" +version = "2.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50f37be617794602aabbeee0be4f259dc1778fabe05e2d67ee8f79326d5cb4f6" +checksum = "f7d2d4dafb69621809a81864c9c1b864479e1235c0dd4e199924b9742439ed89" dependencies = [ "version_check", ] diff --git a/relay-cabi/src/processing.rs b/relay-cabi/src/processing.rs index f9f1085c42..30f2fba2f5 100644 --- a/relay-cabi/src/processing.rs +++ b/relay-cabi/src/processing.rs @@ -20,8 +20,7 @@ use relay_event_schema::protocol::{Event, VALID_PLATFORMS}; use relay_pii::{ selector_suggestions_from_value, DataScrubbingConfig, PiiConfig, PiiConfigError, PiiProcessor, }; -use relay_protocol::{Annotated, Remark}; -use relay_sampling::condition::RuleCondition; +use relay_protocol::{Annotated, Remark, RuleCondition}; use relay_sampling::SamplingConfig; use crate::core::{RelayBuf, RelayStr}; diff --git a/relay-dynamic-config/Cargo.toml b/relay-dynamic-config/Cargo.toml index 2305b57648..72ab787322 100644 --- a/relay-dynamic-config/Cargo.toml +++ b/relay-dynamic-config/Cargo.toml @@ -21,6 +21,7 @@ relay-common = { path = "../relay-common" } relay-filter = { path = "../relay-filter" } relay-event-normalization = { path = "../relay-event-normalization" } relay-pii = { path = "../relay-pii" } +relay-protocol = { path = "../relay-protocol" } relay-quotas = { path = "../relay-quotas" } relay-sampling = { path = "../relay-sampling" } serde = { workspace = true } diff --git a/relay-dynamic-config/src/defaults.rs b/relay-dynamic-config/src/defaults.rs index bc943bc527..949890db87 100644 --- a/relay-dynamic-config/src/defaults.rs +++ b/relay-dynamic-config/src/defaults.rs @@ -1,6 +1,6 @@ use relay_base_schema::data_category::DataCategory; use relay_common::glob2::LazyGlob; -use relay_sampling::condition::RuleCondition; +use relay_protocol::RuleCondition; use crate::feature::Feature; use crate::metrics::{MetricExtractionConfig, MetricSpec, TagMapping, TagSpec}; diff --git a/relay-dynamic-config/src/metrics.rs b/relay-dynamic-config/src/metrics.rs index 59ceebe318..3976e24cc8 100644 --- a/relay-dynamic-config/src/metrics.rs +++ b/relay-dynamic-config/src/metrics.rs @@ -4,7 +4,7 @@ use std::collections::BTreeSet; use relay_base_schema::data_category::DataCategory; use relay_common::glob2::LazyGlob; -use relay_sampling::condition::RuleCondition; +use relay_protocol::RuleCondition; use serde::{Deserialize, Serialize}; use crate::project::ProjectConfig; diff --git a/relay-protocol/Cargo.toml b/relay-protocol/Cargo.toml index e28b646dd3..d1121f4141 100644 --- a/relay-protocol/Cargo.toml +++ b/relay-protocol/Cargo.toml @@ -11,14 +11,17 @@ publish = false [dependencies] num-traits = "0.2.12" +relay-common = { path = "../relay-common" } relay-protocol-derive = { path = "../relay-protocol-derive", optional = true } schemars = { workspace = true, optional = true } serde = { workspace = true } serde_json = { workspace = true } smallvec = { workspace = true } uuid = { workspace = true } +unicase = "2.6.0" [dev-dependencies] +insta = { workspace = true } similar-asserts = { workspace = true } [features] diff --git a/relay-sampling/src/condition.rs b/relay-protocol/src/condition.rs similarity index 93% rename from relay-sampling/src/condition.rs rename to relay-protocol/src/condition.rs index b2332a9481..75c1667925 100644 --- a/relay-sampling/src/condition.rs +++ b/relay-protocol/src/condition.rs @@ -3,11 +3,11 @@ //! The root type is [`RuleCondition`]. use relay_common::glob3::GlobPatterns; -use relay_protocol::{Getter, Val}; use serde::{Deserialize, Serialize}; use serde_json::{Number, Value}; use crate::utils; +use crate::{Getter, Val}; /// Options for [`EqCondition`]. #[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq)] @@ -326,7 +326,7 @@ impl NotCondition { /// # Example /// /// ``` -/// use relay_sampling::condition::RuleCondition; +/// use relay_protocol::RuleCondition; /// /// let condition = !RuleCondition::eq("obj.status", "invalid"); /// ``` @@ -343,7 +343,7 @@ pub enum RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::eq("obj.status", "invalid"); /// ``` @@ -354,7 +354,7 @@ pub enum RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::gte("obj.length", 10); /// ``` @@ -365,7 +365,7 @@ pub enum RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::lte("obj.length", 10); /// ``` @@ -376,7 +376,7 @@ pub enum RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::gt("obj.length", 10); /// ``` @@ -387,7 +387,7 @@ pub enum RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::lt("obj.length", 10); /// ``` @@ -398,7 +398,7 @@ pub enum RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::glob("obj.name", "error: *"); /// ``` @@ -409,7 +409,7 @@ pub enum RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::eq("obj.status", "invalid") /// | RuleCondition::eq("obj.status", "unknown"); @@ -421,7 +421,7 @@ pub enum RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::eq("obj.status", "invalid") /// & RuleCondition::gte("obj.length", 10); @@ -433,7 +433,7 @@ pub enum RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = !RuleCondition::eq("obj.status", "invalid"); /// ``` @@ -465,7 +465,7 @@ impl RuleCondition { /// # Examples /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// // Matches if the value is identical to the given string: /// let condition = RuleCondition::eq("obj.status", "invalid"); @@ -485,7 +485,7 @@ impl RuleCondition { /// # Examples /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// // Matches if the value is identical to the given string: /// let condition = RuleCondition::eq_ignore_case("obj.status", "invalid"); @@ -502,7 +502,7 @@ impl RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// // Match a single pattern: /// let condition = RuleCondition::glob("obj.name", "error: *"); @@ -519,7 +519,7 @@ impl RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::gt("obj.length", 10); /// ``` @@ -532,7 +532,7 @@ impl RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::gte("obj.length", 10); /// ``` @@ -545,7 +545,7 @@ impl RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::lt("obj.length", 10); /// ``` @@ -558,7 +558,7 @@ impl RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::lte("obj.length", 10); /// ``` @@ -573,7 +573,7 @@ impl RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::eq("obj.status", "invalid") /// & RuleCondition::gte("obj.length", 10); @@ -596,7 +596,7 @@ impl RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = RuleCondition::eq("obj.status", "invalid") /// | RuleCondition::eq("obj.status", "unknown"); @@ -619,7 +619,7 @@ impl RuleCondition { /// # Example /// /// ``` - /// use relay_sampling::condition::RuleCondition; + /// use relay_protocol::RuleCondition; /// /// let condition = !RuleCondition::eq("obj.status", "invalid"); /// ``` @@ -698,31 +698,35 @@ impl std::ops::Not for RuleCondition { #[cfg(test)] mod tests { - use std::collections::BTreeMap; - - use relay_base_schema::project::ProjectKey; - use uuid::Uuid; + use super::*; - use crate::dsc::TraceUserContext; - use crate::DynamicSamplingContext; + struct MockDSC { + transaction: String, + release: String, + environment: String, + user_segment: String, + } - use super::*; + impl Getter for MockDSC { + fn get_value(&self, path: &str) -> Option> { + Some(match path.strip_prefix("trace.")? { + "transaction" => self.transaction.as_str().into(), + "release" => self.release.as_str().into(), + "environment" => self.environment.as_str().into(), + "user.segment" => self.user_segment.as_str().into(), + _ => { + return None; + } + }) + } + } - fn dsc_dummy() -> DynamicSamplingContext { - 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(), - }, - replay_id: Some(Uuid::new_v4()), - environment: Some("debug".to_string()), - transaction: Some("transaction1".into()), - sample_rate: None, - sampled: None, - other: BTreeMap::new(), + fn mock_dsc() -> MockDSC { + MockDSC { + transaction: "transaction1".to_string(), + release: "1.1.1".to_string(), + environment: "debug".to_string(), + user_segment: "vip".to_string(), } } @@ -928,7 +932,7 @@ mod tests { ("match no conditions", RuleCondition::all()), ]; - let dsc = dsc_dummy(); + let dsc = mock_dsc(); for (rule_test_name, condition) in conditions.iter() { let failure_name = format!("Failed on test: '{rule_test_name}'!!!"); @@ -971,7 +975,7 @@ mod tests { ("never", false, RuleCondition::never()), ]; - let dsc = dsc_dummy(); + let dsc = mock_dsc(); for (rule_test_name, expected, condition) in conditions.iter() { let failure_name = format!("Failed on test: '{rule_test_name}'!!!"); @@ -1014,7 +1018,7 @@ mod tests { ("all", true, RuleCondition::all()), ]; - let dsc = dsc_dummy(); + let dsc = mock_dsc(); for (rule_test_name, expected, condition) in conditions.iter() { let failure_name = format!("Failed on test: '{rule_test_name}'!!!"); @@ -1037,7 +1041,7 @@ mod tests { ), ]; - let dsc = dsc_dummy(); + let dsc = mock_dsc(); for (rule_test_name, expected, condition) in conditions.iter() { let failure_name = format!("Failed on test: '{rule_test_name}'!!!"); @@ -1075,7 +1079,7 @@ mod tests { ), ]; - let dsc = dsc_dummy(); + let dsc = mock_dsc(); for (rule_test_name, condition) in conditions.iter() { let failure_name = format!("Failed on test: '{rule_test_name}'!!!"); diff --git a/relay-protocol/src/lib.rs b/relay-protocol/src/lib.rs index 363f08135e..efed386a09 100644 --- a/relay-protocol/src/lib.rs +++ b/relay-protocol/src/lib.rs @@ -17,19 +17,23 @@ )] mod annotated; +mod condition; mod impls; mod macros; mod meta; mod size; mod traits; +mod utils; mod value; pub use self::annotated::*; +pub use self::condition::*; pub use self::impls::*; pub use self::macros::*; pub use self::meta::*; pub use self::size::*; pub use self::traits::*; +pub use self::utils::*; pub use self::value::*; #[cfg(feature = "derive")] diff --git a/relay-sampling/src/utils.rs b/relay-protocol/src/utils.rs similarity index 100% rename from relay-sampling/src/utils.rs rename to relay-protocol/src/utils.rs diff --git a/relay-sampling/src/config.rs b/relay-sampling/src/config.rs index 653392d4b3..fd750060c3 100644 --- a/relay-sampling/src/config.rs +++ b/relay-sampling/src/config.rs @@ -5,8 +5,7 @@ use std::fmt; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; -use crate::condition::RuleCondition; -use crate::utils; +use relay_protocol::{is_default, RuleCondition}; /// Represents the dynamic sampling configuration available to a project. /// @@ -27,7 +26,7 @@ pub struct SamplingConfig { pub rules_v2: Vec, /// Defines which population of items a dynamic sample rate applies to. - #[serde(default, skip_serializing_if = "utils::is_default")] + #[serde(default, skip_serializing_if = "is_default")] pub mode: SamplingMode, } @@ -72,7 +71,7 @@ pub struct SamplingRule { pub time_range: TimeRange, /// Declares how to interpolate the sample rate for rules with bounded time range. - #[serde(default, skip_serializing_if = "utils::is_default")] + #[serde(default, skip_serializing_if = "is_default")] pub decaying_fn: DecayingFunction, } diff --git a/relay-sampling/src/evaluation.rs b/relay-sampling/src/evaluation.rs index cf9962d602..a2f9b26706 100644 --- a/relay-sampling/src/evaluation.rs +++ b/relay-sampling/src/evaluation.rs @@ -399,13 +399,13 @@ mod tests { use similar_asserts::assert_eq; use uuid::Uuid; - use crate::condition::RuleCondition; use crate::config::{ DecayingFunction, RuleId, RuleType, SamplingRule, SamplingValue, TimeRange, }; use crate::dsc::TraceUserContext; use crate::evaluation::MatchedRuleIds; use crate::DynamicSamplingContext; + use relay_protocol::RuleCondition; use super::*; diff --git a/relay-sampling/src/lib.rs b/relay-sampling/src/lib.rs index b79edefbd8..ed6bf42917 100644 --- a/relay-sampling/src/lib.rs +++ b/relay-sampling/src/lib.rs @@ -73,13 +73,11 @@ )] #![warn(missing_docs)] -pub mod condition; pub mod config; pub mod dsc; pub mod evaluation; #[cfg(feature = "redis")] mod redis_sampling; -mod utils; pub use config::SamplingConfig; pub use dsc::DynamicSamplingContext; diff --git a/relay-server/src/actors/processor.rs b/relay-server/src/actors/processor.rs index 9cbf3aadd3..9e7969fee9 100644 --- a/relay-server/src/actors/processor.rs +++ b/relay-server/src/actors/processor.rs @@ -3057,7 +3057,7 @@ mod tests { use relay_event_normalization::{MeasurementsConfig, RedactionRule, TransactionNameRule}; use relay_event_schema::protocol::{EventId, TransactionSource}; use relay_pii::DataScrubbingConfig; - use relay_sampling::condition::RuleCondition; + use relay_protocol::RuleCondition; use relay_sampling::config::{ DecayingFunction, RuleId, RuleType, SamplingConfig, SamplingMode, SamplingRule, SamplingValue, TimeRange, diff --git a/relay-server/src/testutils.rs b/relay-server/src/testutils.rs index 1080225cba..8c4de7ba08 100644 --- a/relay-server/src/testutils.rs +++ b/relay-server/src/testutils.rs @@ -1,6 +1,6 @@ use bytes::Bytes; use relay_event_schema::protocol::EventId; -use relay_sampling::condition::RuleCondition; +use relay_protocol::RuleCondition; use relay_sampling::config::{ DecayingFunction, RuleId, RuleType, SamplingMode, SamplingRule, SamplingValue, }; diff --git a/relay-server/src/utils/dynamic_sampling.rs b/relay-server/src/utils/dynamic_sampling.rs index 4c1450ccc3..ead0b83b25 100644 --- a/relay-server/src/utils/dynamic_sampling.rs +++ b/relay-server/src/utils/dynamic_sampling.rs @@ -159,7 +159,7 @@ mod tests { use relay_base_schema::events::EventType; use relay_event_schema::protocol::{Event, EventId, LenientString}; use relay_protocol::Annotated; - use relay_sampling::condition::RuleCondition; + use relay_protocol::RuleCondition; use relay_sampling::config::{ RuleId, RuleType, SamplingConfig, SamplingMode, SamplingRule, SamplingValue, };