From f7eb6e39e5ad76edf3ddd3e194dd1071656dadb9 Mon Sep 17 00:00:00 2001 From: Oliver Browne Date: Wed, 18 Sep 2024 14:38:58 +0100 Subject: [PATCH] feat(featureflags): get some metrics wired up (#25021) Co-authored-by: Dylan Martin --- rust/Cargo.lock | 3 + rust/capture/src/exceptions.rs | 11 + rust/feature-flags/Cargo.toml | 3 + rust/feature-flags/src/api.rs | 1 - rust/feature-flags/src/config.rs | 26 +- rust/feature-flags/src/database.rs | 46 +- rust/feature-flags/src/flag_definitions.rs | 15 +- rust/feature-flags/src/flag_matching.rs | 12 +- rust/feature-flags/src/flag_request.rs | 2 +- rust/feature-flags/src/metrics_consts.rs | 0 rust/feature-flags/src/property_matching.rs | 1433 +++++++---------- rust/feature-flags/src/router.rs | 44 +- rust/feature-flags/src/server.rs | 43 +- rust/feature-flags/src/test_utils.rs | 16 +- .../tests/test_flag_matching_consistency.rs | 8 +- 15 files changed, 703 insertions(+), 960 deletions(-) create mode 100644 rust/capture/src/exceptions.rs create mode 100644 rust/feature-flags/src/metrics_consts.rs diff --git a/rust/Cargo.lock b/rust/Cargo.lock index fa17aa0e7e504..3158a8f3a9148 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -1168,9 +1168,11 @@ dependencies = [ "base64 0.22.0", "bytes", "common-alloc", + "common-metrics", "envconfig", "flate2", "futures", + "health", "maxminddb", "once_cell", "rand", @@ -1184,6 +1186,7 @@ dependencies = [ "sqlx", "thiserror", "tokio", + "tower", "tracing", "tracing-subscriber", "uuid", diff --git a/rust/capture/src/exceptions.rs b/rust/capture/src/exceptions.rs new file mode 100644 index 0000000000000..b20c4eff7fd58 --- /dev/null +++ b/rust/capture/src/exceptions.rs @@ -0,0 +1,11 @@ +pub trait Exception { + fn fingerprint(&self) -> String; +} + +pub trait Stacked { + type Frame; + + fn raw_ident(&self) -> String; + fn stack(&self) -> Vec; + fn lang_hint(&self) -> String; +} diff --git a/rust/feature-flags/Cargo.toml b/rust/feature-flags/Cargo.toml index 9bb1cd2bf7a23..c6a1982d77d35 100644 --- a/rust/feature-flags/Cargo.toml +++ b/rust/feature-flags/Cargo.toml @@ -34,6 +34,9 @@ uuid = { workspace = true } base64.workspace = true flate2.workspace = true common-alloc = { path = "../common/alloc" } +health = { path = "../common/health" } +common-metrics = { path = "../common/metrics" } +tower = { workspace = true } [lints] workspace = true diff --git a/rust/feature-flags/src/api.rs b/rust/feature-flags/src/api.rs index 2bf8f265e30ae..30ddf27809dec 100644 --- a/rust/feature-flags/src/api.rs +++ b/rust/feature-flags/src/api.rs @@ -201,7 +201,6 @@ impl From for FlagError { impl From for FlagError { fn from(e: CustomDatabaseError) -> Self { match e { - CustomDatabaseError::NotFound => FlagError::TokenValidationError, CustomDatabaseError::Other(_) => { tracing::error!("failed to get connection: {}", e); FlagError::DatabaseUnavailable diff --git a/rust/feature-flags/src/config.rs b/rust/feature-flags/src/config.rs index 1f1c47a99249b..02fd621224345 100644 --- a/rust/feature-flags/src/config.rs +++ b/rust/feature-flags/src/config.rs @@ -16,10 +16,10 @@ pub struct Config { #[envconfig(default = "postgres://posthog:posthog@localhost:5432/posthog")] pub read_database_url: String, - #[envconfig(default = "1024")] - pub max_concurrent_jobs: usize, + #[envconfig(default = "1000")] + pub max_concurrency: usize, - #[envconfig(default = "100")] + #[envconfig(default = "10")] pub max_pg_connections: u32, #[envconfig(default = "redis://localhost:6379/")] @@ -30,6 +30,9 @@ pub struct Config { #[envconfig(from = "MAXMIND_DB_PATH", default = "")] pub maxmind_db_path: String, + + #[envconfig(default = "false")] + pub enable_metrics: bool, } impl Config { @@ -40,10 +43,11 @@ impl Config { write_database_url: "postgres://posthog:posthog@localhost:5432/test_posthog" .to_string(), read_database_url: "postgres://posthog:posthog@localhost:5432/test_posthog".to_string(), - max_concurrent_jobs: 1024, - max_pg_connections: 100, + max_concurrency: 1000, + max_pg_connections: 10, acquire_timeout_secs: 1, maxmind_db_path: "".to_string(), + enable_metrics: false, } } @@ -83,8 +87,8 @@ mod tests { config.read_database_url, "postgres://posthog:posthog@localhost:5432/posthog" ); - assert_eq!(config.max_concurrent_jobs, 1024); - assert_eq!(config.max_pg_connections, 100); + assert_eq!(config.max_concurrency, 1000); + assert_eq!(config.max_pg_connections, 10); assert_eq!(config.redis_url, "redis://localhost:6379/"); } @@ -100,8 +104,8 @@ mod tests { config.read_database_url, "postgres://posthog:posthog@localhost:5432/test_posthog" ); - assert_eq!(config.max_concurrent_jobs, 1024); - assert_eq!(config.max_pg_connections, 100); + assert_eq!(config.max_concurrency, 1000); + assert_eq!(config.max_pg_connections, 10); assert_eq!(config.redis_url, "redis://localhost:6379/"); } @@ -117,8 +121,8 @@ mod tests { config.read_database_url, "postgres://posthog:posthog@localhost:5432/test_posthog" ); - assert_eq!(config.max_concurrent_jobs, 1024); - assert_eq!(config.max_pg_connections, 100); + assert_eq!(config.max_concurrency, 1000); + assert_eq!(config.max_pg_connections, 10); assert_eq!(config.redis_url, "redis://localhost:6379/"); } } diff --git a/rust/feature-flags/src/database.rs b/rust/feature-flags/src/database.rs index 29360d22b9444..c7a45aeffc2d1 100644 --- a/rust/feature-flags/src/database.rs +++ b/rust/feature-flags/src/database.rs @@ -5,20 +5,15 @@ use async_trait::async_trait; use sqlx::{ pool::PoolConnection, postgres::{PgPoolOptions, PgRow}, - Postgres, + PgPool, Postgres, }; use thiserror::Error; use tokio::time::timeout; -use crate::config::Config; - const DATABASE_TIMEOUT_MILLISECS: u64 = 1000; #[derive(Error, Debug)] pub enum CustomDatabaseError { - #[error("Not found in database")] - NotFound, - #[error("Pg error: {0}")] Other(#[from] sqlx::Error), @@ -40,36 +35,17 @@ pub trait Client { ) -> Result, CustomDatabaseError>; } -pub struct PgClient { - pool: sqlx::PgPool, -} - -impl PgClient { - pub async fn new_read_client(config: &Config) -> Result { - let pool = PgPoolOptions::new() - .max_connections(config.max_pg_connections) - .acquire_timeout(Duration::from_secs(1)) - .test_before_acquire(true) - .connect(&config.read_database_url) - .await?; - - Ok(PgClient { pool }) - } - - pub async fn new_write_client(config: &Config) -> Result { - let pool = PgPoolOptions::new() - .max_connections(config.max_pg_connections) - .acquire_timeout(Duration::from_secs(1)) - .test_before_acquire(true) - .connect(&config.write_database_url) - .await?; - - Ok(PgClient { pool }) - } +pub async fn get_pool(url: &str, max_connections: u32) -> Result { + PgPoolOptions::new() + .max_connections(max_connections) + .acquire_timeout(Duration::from_secs(1)) + .test_before_acquire(true) + .connect(url) + .await } #[async_trait] -impl Client for PgClient { +impl Client for PgPool { async fn run_query( &self, query: String, @@ -80,7 +56,7 @@ impl Client for PgClient { let built_query = parameters .iter() .fold(built_query, |acc, param| acc.bind(param)); - let query_results = built_query.fetch_all(&self.pool); + let query_results = built_query.fetch_all(self); let timeout_ms = match timeout_ms { Some(ms) => ms, @@ -93,6 +69,6 @@ impl Client for PgClient { } async fn get_connection(&self) -> Result, CustomDatabaseError> { - Ok(self.pool.acquire().await?) + Ok(self.acquire().await?) } } diff --git a/rust/feature-flags/src/flag_definitions.rs b/rust/feature-flags/src/flag_definitions.rs index 7cf66c88f7b99..e61dabe2babbe 100644 --- a/rust/feature-flags/src/flag_definitions.rs +++ b/rust/feature-flags/src/flag_definitions.rs @@ -237,7 +237,10 @@ mod tests { .await .expect("Failed to fetch flags from redis"); assert_eq!(flags_from_redis.flags.len(), 1); - let flag = flags_from_redis.flags.get(0).expect("Empty flags in redis"); + let flag = flags_from_redis + .flags + .first() + .expect("Empty flags in redis"); assert_eq!(flag.key, "flag1"); assert_eq!(flag.team_id, team.id); assert_eq!(flag.filters.groups.len(), 1); @@ -288,7 +291,7 @@ mod tests { .expect("Failed to fetch flags from pg"); assert_eq!(flags_from_pg.flags.len(), 1); - let flag = flags_from_pg.flags.get(0).expect("Flags should be in pg"); + let flag = flags_from_pg.flags.first().expect("Flags should be in pg"); assert_eq!(flag.key, "flag1"); assert_eq!(flag.team_id, team.id); @@ -414,13 +417,11 @@ mod tests { async fn test_fetch_empty_team_from_pg() { let client = setup_pg_client(None).await; - match FeatureFlagList::from_pg(client.clone(), 1234) + let FeatureFlagList { flags } = FeatureFlagList::from_pg(client.clone(), 1234) .await - .expect("Failed to fetch flags from pg") + .expect("Failed to fetch flags from pg"); { - FeatureFlagList { flags } => { - assert_eq!(flags.len(), 0); - } + assert_eq!(flags.len(), 0); } } diff --git a/rust/feature-flags/src/flag_matching.rs b/rust/feature-flags/src/flag_matching.rs index d4487874a2081..4af88ed6a8770 100644 --- a/rust/feature-flags/src/flag_matching.rs +++ b/rust/feature-flags/src/flag_matching.rs @@ -91,7 +91,7 @@ impl GroupTypeMappingCache { return Err(e); } }; - self.group_types_to_indexes = mapping.clone(); + self.group_types_to_indexes.clone_from(&mapping); Ok(mapping) } @@ -108,7 +108,7 @@ impl GroupTypeMappingCache { types_to_indexes.into_iter().map(|(k, v)| (v, k)).collect(); if !result.is_empty() { - self.group_indexes_to_types = result.clone(); + self.group_indexes_to_types.clone_from(&result); Ok(result) } else { Err(FlagError::NoGroupTypeMappings) @@ -836,7 +836,7 @@ mod tests { None, ); let match_result = matcher.get_match(&flag, None).await.unwrap(); - assert_eq!(match_result.matches, true); + assert!(match_result.matches); assert_eq!(match_result.variant, None); let mut matcher = FeatureFlagMatcher::new( @@ -848,7 +848,7 @@ mod tests { None, ); let match_result = matcher.get_match(&flag, None).await.unwrap(); - assert_eq!(match_result.matches, false); + assert!(!match_result.matches); assert_eq!(match_result.variant, None); let mut matcher = FeatureFlagMatcher::new( @@ -860,7 +860,7 @@ mod tests { None, ); let match_result = matcher.get_match(&flag, None).await.unwrap(); - assert_eq!(match_result.matches, false); + assert!(!match_result.matches); assert_eq!(match_result.variant, None); } @@ -1041,7 +1041,7 @@ mod tests { .is_condition_match(&flag, &condition, None) .await .unwrap(); - assert_eq!(is_match, true); + assert!(is_match); assert_eq!(reason, "CONDITION_MATCH"); } diff --git a/rust/feature-flags/src/flag_request.rs b/rust/feature-flags/src/flag_request.rs index d41ac5d1e07b9..78f8637177745 100644 --- a/rust/feature-flags/src/flag_request.rs +++ b/rust/feature-flags/src/flag_request.rs @@ -210,7 +210,7 @@ mod tests { #[test] fn too_large_distinct_id_is_truncated() { let json = json!({ - "distinct_id": std::iter::repeat("a").take(210).collect::(), + "distinct_id": "a".repeat(210), "token": "my_token1", }); let bytes = Bytes::from(json.to_string()); diff --git a/rust/feature-flags/src/metrics_consts.rs b/rust/feature-flags/src/metrics_consts.rs new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/rust/feature-flags/src/property_matching.rs b/rust/feature-flags/src/property_matching.rs index f29f93d90690c..3487c99dce113 100644 --- a/rust/feature-flags/src/property_matching.rs +++ b/rust/feature-flags/src/property_matching.rs @@ -259,69 +259,50 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("value"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("value2"))]), - true - ) - .expect("expected match to exist"), - false - ); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(""))]), - true - ) - .expect("expected match to exist"), - false - ); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(null))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value2"))]), + true + ) + .expect("expected match to exist")); + assert!(!match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(""))]), + true + ) + .expect("expected match to exist")); + assert!(!match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(null))]), + true + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key2".to_string(), json!("value"))]), - true - ) - .is_err(), + assert!(match_property( + &property_a, + &HashMap::from([("key2".to_string(), json!("value"))]), true - ); + ) + .is_err()); assert_eq!( match_property( &property_a, &HashMap::from([("key2".to_string(), json!("value"))]), true ) - .err() - .expect("expected match to exist"), + .expect_err("expected match to exist"), FlagMatchingError::MissingProperty( "can't match properties without a value. Missing property: key".to_string() ) ); - assert_eq!( - match_property(&property_a, &HashMap::from([]), true).is_err(), - true - ); + assert!(match_property(&property_a, &HashMap::from([]), true).is_err()); let property_b = PropertyFilter { key: "key".to_string(), @@ -331,25 +312,19 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!("value"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("value"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!("value2"))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("value2"))]), + true + ) + .expect("expected match to exist")); let property_c = PropertyFilter { key: "key".to_string(), @@ -359,53 +334,38 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("value1"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value1"))]), true - ); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("value2"))]), - true - ) - .expect("expected match to exist"), + ) + .expect("expected match to exist")); + assert!(match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value2"))]), true - ); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("value3"))]), - true - ) - .expect("expected match to exist"), + ) + .expect("expected match to exist")); + assert!(match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value3"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("value4"))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value4"))]), + true + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key2".to_string(), json!("value"))]), - true - ) - .is_err(), + assert!(match_property( + &property_c, + &HashMap::from([("key2".to_string(), json!("value"))]), true - ); + ) + .is_err()); } #[test] @@ -418,46 +378,34 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("value2"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value2"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(""))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(""))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(null))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(null))]), true - ); + ) + .expect("expected match to exist")); // partial mode returns error when key doesn't exist - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key2".to_string(), json!("value1"))]), - true - ) - .is_err(), + assert!(match_property( + &property_a, + &HashMap::from([("key2".to_string(), json!("value1"))]), true - ); + ) + .is_err()); let property_c = PropertyFilter { key: "key".to_string(), @@ -467,95 +415,68 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("value4"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value4"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("value5"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value5"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("value6"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value6"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!(""))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(""))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!(null))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(null))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("value2"))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value2"))]), + true + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("value3"))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value3"))]), + true + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("value1"))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value1"))]), + true + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key2".to_string(), json!("value1"))]), - true - ) - .is_err(), + assert!(match_property( + &property_c, + &HashMap::from([("key2".to_string(), json!("value1"))]), true - ); + ) + .is_err()); } #[test] @@ -568,60 +489,42 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("value"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("value2"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value2"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(""))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(""))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(null))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(null))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key2".to_string(), json!("value1"))]), - true - ) - .is_err(), + assert!(match_property( + &property_a, + &HashMap::from([("key2".to_string(), json!("value1"))]), true - ); + ) + .is_err()); - assert_eq!( - match_property(&property_a, &HashMap::from([]), true).is_err(), - true - ); + assert!(match_property(&property_a, &HashMap::from([]), true).is_err()); } #[test] @@ -634,75 +537,54 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("value"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("value2"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value2"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("value3"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value3"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("vaLue4"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("vaLue4"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("343tfvalue5"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("343tfvalue5"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("Alakazam"))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("Alakazam"))]), + true + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(123))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(123))]), + true + ) + .expect("expected match to exist")); let property_b = PropertyFilter { key: "key".to_string(), @@ -712,45 +594,33 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!("3"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("3"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!(323))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(323))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!("val3"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("val3"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!("three"))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("three"))]), + true + ) + .expect("expected match to exist")); } #[test] @@ -763,52 +633,37 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("value.com"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value.com"))]), true - ); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("value2.com"))]), - true - ) - .expect("expected match to exist"), + ) + .expect("expected match to exist")); + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("value2.com"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(".com343tfvalue5"))]), - true - ) - .expect("expected match to exist"), - false - ); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("Alakazam"))]), - true - ) - .expect("expected match to exist"), - false - ); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(123))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(".com343tfvalue5"))]), + true + ) + .expect("expected match to exist")); + assert!(!match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("Alakazam"))]), + true + ) + .expect("expected match to exist")); + assert!(!match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(123))]), + true + ) + .expect("expected match to exist")); let property_b = PropertyFilter { key: "key".to_string(), @@ -817,43 +672,31 @@ mod test_match_properties { prop_type: "person".to_string(), group_type_index: None, }; - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!("3"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("3"))]), true - ); - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!(323))]), - true - ) - .expect("expected match to exist"), + ) + .expect("expected match to exist")); + assert!(match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(323))]), true - ); - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!("val3"))]), - true - ) - .expect("expected match to exist"), + ) + .expect("expected match to exist")); + assert!(match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("val3"))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!("three"))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("three"))]), + true + ) + .expect("expected match to exist")); // invalid regex let property_c = PropertyFilter { @@ -864,24 +707,18 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("value"))]), - true - ) - .expect("expected match to exist"), - false - ); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("value2"))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value"))]), + true + ) + .expect("expected match to exist")); + assert!(!match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("value2"))]), + true + ) + .expect("expected match to exist")); // non string value let property_d = PropertyFilter { @@ -891,94 +728,70 @@ mod test_match_properties { prop_type: "person".to_string(), group_type_index: None, }; - assert_eq!( - match_property( - &property_d, - &HashMap::from([("key".to_string(), json!("4"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_d, + &HashMap::from([("key".to_string(), json!("4"))]), true - ); - assert_eq!( - match_property( - &property_d, - &HashMap::from([("key".to_string(), json!(4))]), - true - ) - .expect("expected match to exist"), + ) + .expect("expected match to exist")); + assert!(match_property( + &property_d, + &HashMap::from([("key".to_string(), json!(4))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_d, - &HashMap::from([("key".to_string(), json!("value"))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_d, + &HashMap::from([("key".to_string(), json!("value"))]), + true + ) + .expect("expected match to exist")); } #[test] - fn test_match_properties_math_operators() { - let property_a = PropertyFilter { - key: "key".to_string(), - value: json!(1), - operator: Some(OperatorType::Gt), - prop_type: "person".to_string(), - group_type_index: None, - }; - - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(2))]), - true - ) - .expect("expected match to exist"), - true - ); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(3))]), - true - ) - .expect("expected match to exist"), - true - ); - - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(0))]), - true - ) - .expect("expected match to exist"), - false - ); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(-1))]), - true - ) - .expect("expected match to exist"), - false - ); + fn test_match_properties_math_operators() { + let property_a = PropertyFilter { + key: "key".to_string(), + value: json!(1), + operator: Some(OperatorType::Gt), + prop_type: "person".to_string(), + group_type_index: None, + }; + + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(2))]), + true + ) + .expect("expected match to exist")); + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(3))]), + true + ) + .expect("expected match to exist")); + + assert!(!match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(0))]), + true + ) + .expect("expected match to exist")); + assert!(!match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(-1))]), + true + ) + .expect("expected match to exist")); // # we handle type mismatches so this should be true - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("23"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("23"))]), true - ); + ) + .expect("expected match to exist")); let property_b = PropertyFilter { key: "key".to_string(), @@ -988,61 +801,43 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!(0))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(0))]), true - ); - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!(-1))]), - true - ) - .expect("expected match to exist"), + ) + .expect("expected match to exist")); + assert!(match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(-1))]), true - ); - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!(-3))]), - true - ) - .expect("expected match to exist"), + ) + .expect("expected match to exist")); + assert!(match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(-3))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!(1))]), - true - ) - .expect("expected match to exist"), - false - ); - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!("1"))]), - true - ) - .expect("expected match to exist"), - false - ); - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!("3"))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(1))]), + true + ) + .expect("expected match to exist")); + assert!(!match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("1"))]), + true + ) + .expect("expected match to exist")); + assert!(!match_property( + &property_b, + &HashMap::from([("key".to_string(), json!("3"))]), + true + ) + .expect("expected match to exist")); let property_c = PropertyFilter { key: "key".to_string(), @@ -1052,53 +847,38 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!(1))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(1))]), true - ); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!(2))]), - true - ) - .expect("expected match to exist"), + ) + .expect("expected match to exist")); + assert!(match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(2))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!(0))]), - true - ) - .expect("expected match to exist"), - false - ); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!(-1))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(0))]), + true + ) + .expect("expected match to exist")); + assert!(!match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(-1))]), + true + ) + .expect("expected match to exist")); // # now we handle type mismatches so this should be true - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("3"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("3"))]), true - ); + ) + .expect("expected match to exist")); let property_d = PropertyFilter { key: "key".to_string(), @@ -1108,61 +888,43 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_d, - &HashMap::from([("key".to_string(), json!("41"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_d, + &HashMap::from([("key".to_string(), json!("41"))]), true - ); - assert_eq!( - match_property( - &property_d, - &HashMap::from([("key".to_string(), json!("42"))]), - true - ) - .expect("expected match to exist"), + ) + .expect("expected match to exist")); + assert!(match_property( + &property_d, + &HashMap::from([("key".to_string(), json!("42"))]), true - ); - assert_eq!( - match_property( - &property_d, - &HashMap::from([("key".to_string(), json!(42))]), - true - ) - .expect("expected match to exist"), + ) + .expect("expected match to exist")); + assert!(match_property( + &property_d, + &HashMap::from([("key".to_string(), json!(42))]), true - ); + ) + .expect("expected match to exist")); - assert_eq!( - match_property( - &property_d, - &HashMap::from([("key".to_string(), json!("43"))]), - true - ) - .expect("expected match to exist"), - false - ); - assert_eq!( - match_property( - &property_d, - &HashMap::from([("key".to_string(), json!("44"))]), - true - ) - .expect("expected match to exist"), - false - ); - assert_eq!( - match_property( - &property_d, - &HashMap::from([("key".to_string(), json!(44))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_d, + &HashMap::from([("key".to_string(), json!("43"))]), + true + ) + .expect("expected match to exist")); + assert!(!match_property( + &property_d, + &HashMap::from([("key".to_string(), json!("44"))]), + true + ) + .expect("expected match to exist")); + assert!(!match_property( + &property_d, + &HashMap::from([("key".to_string(), json!(44))]), + true + ) + .expect("expected match to exist")); let property_e = PropertyFilter { key: "key".to_string(), @@ -1172,15 +934,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_e, - &HashMap::from([("key".to_string(), json!("29"))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_e, + &HashMap::from([("key".to_string(), json!("29"))]), true - ); + ) + .expect("expected match to exist")); // # depending on the type of override, we adjust type comparison // This is wonky, do we want to continue this behavior? :/ @@ -1194,15 +953,12 @@ mod test_match_properties { // .expect("expected match to exist"), // true // ); - assert_eq!( - match_property( - &property_e, - &HashMap::from([("key".to_string(), json!(100))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_e, + &HashMap::from([("key".to_string(), json!(100))]), + true + ) + .expect("expected match to exist")); // let property_f = PropertyFilter { // key: "key".to_string(), @@ -1256,24 +1012,18 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!(null))]), - true - ) - .expect("expected match to exist"), - false - ); - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key".to_string(), json!("non"))]), - true - ) - .expect("expected match to exist"), + assert!(!match_property( + &property_a, + &HashMap::from([("key".to_string(), json!(null))]), true - ); + ) + .expect("expected match to exist")); + assert!(match_property( + &property_a, + &HashMap::from([("key".to_string(), json!("non"))]), + true + ) + .expect("expected match to exist")); let property_b = PropertyFilter { key: "key".to_string(), @@ -1283,15 +1033,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_b, - &HashMap::from([("key".to_string(), json!(null))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_b, + &HashMap::from([("key".to_string(), json!(null))]), true - ); + ) + .expect("expected match to exist")); let property_c = PropertyFilter { key: "key".to_string(), @@ -1301,24 +1048,18 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!(null))]), - true - ) - .expect("expected match to exist"), + assert!(match_property( + &property_c, + &HashMap::from([("key".to_string(), json!(null))]), true - ); - assert_eq!( - match_property( - &property_c, - &HashMap::from([("key".to_string(), json!("smh"))]), - true - ) - .expect("expected match to exist"), - false - ); + ) + .expect("expected match to exist")); + assert!(!match_property( + &property_c, + &HashMap::from([("key".to_string(), json!("smh"))]), + true + ) + .expect("expected match to exist")); let property_d = PropertyFilter { key: "key".to_string(), @@ -1328,15 +1069,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_d, - &HashMap::from([("key".to_string(), json!(null))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_d, + &HashMap::from([("key".to_string(), json!(null))]), + true + ) + .expect("expected match to exist")); let property_d_upper_case = PropertyFilter { key: "key".to_string(), @@ -1346,15 +1084,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_d_upper_case, - &HashMap::from([("key".to_string(), json!(null))]), - true - ) - .expect("expected match to exist"), - false - ); + assert!(!match_property( + &property_d_upper_case, + &HashMap::from([("key".to_string(), json!(null))]), + true + ) + .expect("expected match to exist")); // TODO: Fails because not a number // let property_e = PropertyFilter { @@ -1382,15 +1117,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_a, - &HashMap::from([("key2".to_string(), json!("value"))]), - false - ) - .expect("Expected no errors with full props mode for non-existent keys"), + assert!(!match_property( + &property_a, + &HashMap::from([("key2".to_string(), json!("value"))]), false - ); + ) + .expect("Expected no errors with full props mode for non-existent keys")); assert_eq!( match_property(&property_a, &HashMap::from([]), false), Ok(false) @@ -1404,15 +1136,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_exact, - &HashMap::from([("key2".to_string(), json!("value"))]), - false - ) - .expect("Expected no errors with full props mode"), + assert!(!match_property( + &property_exact, + &HashMap::from([("key2".to_string(), json!("value"))]), false - ); + ) + .expect("Expected no errors with full props mode")); let property_is_set = PropertyFilter { key: "key".to_string(), @@ -1422,15 +1151,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_is_set, - &HashMap::from([("key2".to_string(), json!("value"))]), - false - ) - .expect("Expected no errors with full props mode"), + assert!(!match_property( + &property_is_set, + &HashMap::from([("key2".to_string(), json!("value"))]), false - ); + ) + .expect("Expected no errors with full props mode")); let property_is_not_set = PropertyFilter { key: "key".to_string(), @@ -1440,45 +1166,33 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_is_not_set, - &HashMap::from([("key2".to_string(), json!("value"))]), - false - ) - .expect("Expected no errors with full props mode"), - true - ); - assert_eq!( - match_property( - &property_is_not_set, - &HashMap::from([("key".to_string(), json!("value"))]), - false - ) - .expect("Expected no errors with full props mode"), + assert!(match_property( + &property_is_not_set, + &HashMap::from([("key2".to_string(), json!("value"))]), false - ); + ) + .expect("Expected no errors with full props mode")); + assert!(!match_property( + &property_is_not_set, + &HashMap::from([("key".to_string(), json!("value"))]), + false + ) + .expect("Expected no errors with full props mode")); // is not set with partial props returns false when key exists - assert_eq!( - match_property( - &property_is_not_set, - &HashMap::from([("key".to_string(), json!("value"))]), - true - ) - .expect("Expected no errors with full props mode"), - false - ); + assert!(!match_property( + &property_is_not_set, + &HashMap::from([("key".to_string(), json!("value"))]), + true + ) + .expect("Expected no errors with full props mode")); // is not set returns error when key doesn't exist - assert_eq!( - match_property( - &property_is_not_set, - &HashMap::from([("key2".to_string(), json!("value"))]), - true - ) - .is_err(), + assert!(match_property( + &property_is_not_set, + &HashMap::from([("key2".to_string(), json!("value"))]), true - ); + ) + .is_err()); let property_icontains = PropertyFilter { key: "key".to_string(), @@ -1488,15 +1202,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_icontains, - &HashMap::from([("key2".to_string(), json!("value"))]), - false - ) - .expect("Expected no errors with full props mode"), + assert!(!match_property( + &property_icontains, + &HashMap::from([("key2".to_string(), json!("value"))]), false - ); + ) + .expect("Expected no errors with full props mode")); let property_not_icontains = PropertyFilter { key: "key".to_string(), @@ -1506,15 +1217,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_not_icontains, - &HashMap::from([("key2".to_string(), json!("value"))]), - false - ) - .expect("Expected no errors with full props mode"), + assert!(!match_property( + &property_not_icontains, + &HashMap::from([("key2".to_string(), json!("value"))]), false - ); + ) + .expect("Expected no errors with full props mode")); let property_regex = PropertyFilter { key: "key".to_string(), @@ -1524,15 +1232,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_regex, - &HashMap::from([("key2".to_string(), json!("value.com"))]), - false - ) - .expect("Expected no errors with full props mode"), + assert!(!match_property( + &property_regex, + &HashMap::from([("key2".to_string(), json!("value.com"))]), false - ); + ) + .expect("Expected no errors with full props mode")); let property_not_regex = PropertyFilter { key: "key".to_string(), @@ -1542,15 +1247,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_not_regex, - &HashMap::from([("key2".to_string(), json!("value.com"))]), - false - ) - .expect("Expected no errors with full props mode"), + assert!(!match_property( + &property_not_regex, + &HashMap::from([("key2".to_string(), json!("value.com"))]), false - ); + ) + .expect("Expected no errors with full props mode")); let property_gt = PropertyFilter { key: "key".to_string(), @@ -1560,15 +1262,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_gt, - &HashMap::from([("key2".to_string(), json!(2))]), - false - ) - .expect("Expected no errors with full props mode"), + assert!(!match_property( + &property_gt, + &HashMap::from([("key2".to_string(), json!(2))]), false - ); + ) + .expect("Expected no errors with full props mode")); let property_gte = PropertyFilter { key: "key".to_string(), @@ -1578,15 +1277,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_gte, - &HashMap::from([("key2".to_string(), json!(2))]), - false - ) - .expect("Expected no errors with full props mode"), + assert!(!match_property( + &property_gte, + &HashMap::from([("key2".to_string(), json!(2))]), false - ); + ) + .expect("Expected no errors with full props mode")); let property_lt = PropertyFilter { key: "key".to_string(), @@ -1596,15 +1292,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_lt, - &HashMap::from([("key2".to_string(), json!(0))]), - false - ) - .expect("Expected no errors with full props mode"), + assert!(!match_property( + &property_lt, + &HashMap::from([("key2".to_string(), json!(0))]), false - ); + ) + .expect("Expected no errors with full props mode")); let property_lte = PropertyFilter { key: "key".to_string(), @@ -1614,15 +1307,12 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_lte, - &HashMap::from([("key2".to_string(), json!(0))]), - false - ) - .expect("Expected no errors with full props mode"), + assert!(!match_property( + &property_lte, + &HashMap::from([("key2".to_string(), json!(0))]), false - ); + ) + .expect("Expected no errors with full props mode")); // TODO: Handle date operators let property_is_date_before = PropertyFilter { @@ -1633,14 +1323,11 @@ mod test_match_properties { group_type_index: None, }; - assert_eq!( - match_property( - &property_is_date_before, - &HashMap::from([("key2".to_string(), json!("2021-01-02"))]), - false - ) - .expect("Expected no errors with full props mode"), + assert!(!match_property( + &property_is_date_before, + &HashMap::from([("key2".to_string(), json!("2021-01-02"))]), false - ); + ) + .expect("Expected no errors with full props mode")); } } diff --git a/rust/feature-flags/src/router.rs b/rust/feature-flags/src/router.rs index 14ebac3c16b06..51a4b95e3f9c2 100644 --- a/rust/feature-flags/src/router.rs +++ b/rust/feature-flags/src/router.rs @@ -1,6 +1,12 @@ -use std::sync::Arc; +use std::{future::ready, sync::Arc}; -use axum::{routing::post, Router}; +use axum::{ + routing::{get, post}, + Router, +}; +use common_metrics::setup_metrics_recorder; +use health::HealthRegistry; +use tower::limit::ConcurrencyLimitLayer; use crate::{ database::Client as DatabaseClient, geoip::GeoIpClient, redis::Client as RedisClient, @@ -15,7 +21,14 @@ pub struct State { pub geoip: Arc, } -pub fn router(redis: Arc, postgres: Arc, geoip: Arc) -> Router +pub fn router( + redis: Arc, + postgres: Arc, + geoip: Arc, + liveness: HealthRegistry, + metrics: bool, + concurrency: usize, +) -> Router where R: RedisClient + Send + Sync + 'static, D: DatabaseClient + Send + Sync + 'static, @@ -26,9 +39,30 @@ where geoip, }; - Router::new() + let status_router = Router::new() + .route("/", get(index)) + .route("/_readiness", get(index)) + .route("/_liveness", get(move || ready(liveness.get_status()))); + + let flags_router = Router::new() .route("/flags", post(v0_endpoint::flags).get(v0_endpoint::flags)) - .with_state(state) + .layer(ConcurrencyLimitLayer::new(concurrency)) + .with_state(state); + + let router = Router::new().merge(status_router).merge(flags_router); + + // Don't install metrics unless asked to + // Global metrics recorders can play poorly with e.g. tests + if metrics { + let recorder_handle = setup_metrics_recorder(); + router.route("/metrics", get(move || ready(recorder_handle.render()))) + } else { + router + } +} + +pub async fn index() -> &'static str { + "feature flags service" } // TODO, eventually we can differentiate read and write postgres clients, if needed diff --git a/rust/feature-flags/src/server.rs b/rust/feature-flags/src/server.rs index c718657e3af66..4c476aaf46169 100644 --- a/rust/feature-flags/src/server.rs +++ b/rust/feature-flags/src/server.rs @@ -1,11 +1,13 @@ use std::future::Future; use std::net::SocketAddr; use std::sync::Arc; +use std::time::Duration; +use health::{HealthHandle, HealthRegistry}; use tokio::net::TcpListener; use crate::config::Config; -use crate::database::PgClient; +use crate::database::get_pool; use crate::geoip::GeoIpClient; use crate::redis::RedisClient; use crate::router; @@ -22,13 +24,14 @@ where } }; - let read_postgres_client = match PgClient::new_read_client(&config).await { - Ok(client) => Arc::new(client), - Err(e) => { - tracing::error!("Failed to create read Postgres client: {}", e); - return; - } - }; + let read_postgres_client = + match get_pool(&config.read_database_url, config.max_pg_connections).await { + Ok(client) => Arc::new(client), + Err(e) => { + tracing::error!("Failed to create read Postgres client: {}", e); + return; + } + }; let geoip_service = match GeoIpClient::new(&config) { Ok(service) => Arc::new(service), @@ -38,8 +41,23 @@ where } }; + let health = HealthRegistry::new("liveness"); + + // TODO - we don't have a more complex health check yet, but we should add e.g. some around DB operations + let simple_loop = health + .register("simple_loop".to_string(), Duration::from_secs(30)) + .await; + tokio::spawn(liveness_loop(simple_loop)); + // You can decide which client to pass to the router, or pass both if needed - let app = router::router(redis_client, read_postgres_client, geoip_service); + let app = router::router( + redis_client, + read_postgres_client, + geoip_service, + health, + config.enable_metrics, + config.max_concurrency, + ); tracing::info!("listening on {:?}", listener.local_addr().unwrap()); axum::serve( @@ -50,3 +68,10 @@ where .await .unwrap() } + +async fn liveness_loop(handle: HealthHandle) { + loop { + handle.report_healthy().await; + tokio::time::sleep(std::time::Duration::from_secs(10)).await; + } +} diff --git a/rust/feature-flags/src/test_utils.rs b/rust/feature-flags/src/test_utils.rs index e3a0a916e641d..e4777485eb66c 100644 --- a/rust/feature-flags/src/test_utils.rs +++ b/rust/feature-flags/src/test_utils.rs @@ -1,13 +1,13 @@ use anyhow::Error; use axum::async_trait; use serde_json::{json, Value}; -use sqlx::{pool::PoolConnection, postgres::PgRow, Error as SqlxError, Postgres}; +use sqlx::{pool::PoolConnection, postgres::PgRow, Error as SqlxError, PgPool, Postgres}; use std::sync::Arc; use uuid::Uuid; use crate::{ config::{Config, DEFAULT_TEST_CONFIG}, - database::{Client, CustomDatabaseError, PgClient}, + database::{get_pool, Client, CustomDatabaseError}, flag_definitions::{self, FeatureFlag, FeatureFlagRow}, redis::{Client as RedisClientTrait, RedisClient}, team::{self, Team}, @@ -130,12 +130,12 @@ pub fn create_flag_from_json(json_value: Option) -> Vec { flags } -pub async fn setup_pg_client(config: Option<&Config>) -> Arc { +pub async fn setup_pg_client(config: Option<&Config>) -> Arc { let config = config.unwrap_or(&DEFAULT_TEST_CONFIG); Arc::new( - PgClient::new_read_client(config) + get_pool(&config.read_database_url, config.max_pg_connections) .await - .expect("Failed to create pg read client"), + .expect("Failed to create Postgres client"), ) } @@ -163,7 +163,7 @@ pub async fn setup_invalid_pg_client() -> Arc { Arc::new(MockPgClient) } -pub async fn insert_new_team_in_pg(client: Arc) -> Result { +pub async fn insert_new_team_in_pg(client: Arc) -> Result { const ORG_ID: &str = "019026a4be8000005bf3171d00629163"; client.run_query( @@ -235,7 +235,7 @@ pub async fn insert_new_team_in_pg(client: Arc) -> Result } pub async fn insert_flag_for_team_in_pg( - client: Arc, + client: Arc, team_id: i32, flag: Option, ) -> Result { @@ -284,7 +284,7 @@ pub async fn insert_flag_for_team_in_pg( } pub async fn insert_person_for_team_in_pg( - client: Arc, + client: Arc, team_id: i32, distinct_id: String, properties: Option, diff --git a/rust/feature-flags/tests/test_flag_matching_consistency.rs b/rust/feature-flags/tests/test_flag_matching_consistency.rs index 9a44de9debed9..e8bff239cf901 100644 --- a/rust/feature-flags/tests/test_flag_matching_consistency.rs +++ b/rust/feature-flags/tests/test_flag_matching_consistency.rs @@ -104,7 +104,7 @@ async fn it_is_consistent_with_rollout_calculation_for_simple_flags() { false, true, true, ]; - for i in 0..1000 { + for (i, result) in results.iter().enumerate().take(1000) { let database_client = setup_pg_client(None).await; let distinct_id = format!("distinct_id_{}", i); @@ -115,7 +115,7 @@ async fn it_is_consistent_with_rollout_calculation_for_simple_flags() { .await .unwrap(); - if results[i] { + if *result { assert_eq!( feature_flag_match, FeatureFlagMatch { @@ -1189,7 +1189,7 @@ async fn it_is_consistent_with_rollout_calculation_for_multivariate_flags() { Some("first-variant".to_string()), ]; - for i in 0..1000 { + for (i, result) in results.iter().enumerate().take(1000) { let database_client = setup_pg_client(None).await; let distinct_id = format!("distinct_id_{}", i); @@ -1199,7 +1199,7 @@ async fn it_is_consistent_with_rollout_calculation_for_multivariate_flags() { .await .unwrap(); - if results[i].is_some() { + if result.is_some() { assert_eq!( feature_flag_match, FeatureFlagMatch {