From 9795c7c8acfbb963f45df4bba6a589f32544ee45 Mon Sep 17 00:00:00 2001 From: Simon Hornby Date: Fri, 19 Jul 2024 11:28:46 +0200 Subject: [PATCH] feat: upgrade types library, unfortunately this does two things - adds new optional metrics data and adds backwards compatability for impression_data (#489) --- Cargo.lock | 8 +-- server/Cargo.toml | 4 +- server/src/client_api.rs | 27 +++++++- server/src/frontend_api.rs | 13 +++- server/src/http/feature_refresher.rs | 9 ++- server/src/metrics/client_metrics.rs | 99 ++++++++++++++++++++++++++-- 6 files changed, 142 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ad9f84ed..43cb40cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3686,9 +3686,9 @@ dependencies = [ [[package]] name = "unleash-types" -version = "0.12.0" +version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cad053850a28a8997168919c4c1ab4f0d86e490f62aefca27d0f2ff897a12f62" +checksum = "2ec98d6933713aaffd77d7dc2663601b97dd893d9a1e12c44c1d971fe507c9f2" dependencies = [ "base64 0.21.7", "chrono", @@ -3701,9 +3701,9 @@ dependencies = [ [[package]] name = "unleash-yggdrasil" -version = "0.12.0" +version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ad42bf8522f78f7c6101a8bb6f200adead20aa54cb3742ae872e019dd5c402b5" +checksum = "2d692dd729a613ff15d879bc19b9f304a29a5ee148d1e25f9b850a8c65c691c7" dependencies = [ "chrono", "convert_case 0.6.0", diff --git a/server/Cargo.toml b/server/Cargo.toml index c82c6c62..9b71f6f5 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -83,8 +83,8 @@ tokio = { version = "1.38.0", features = [ tracing = { version = "0.1.40", features = ["log"] } tracing-subscriber = { version = "0.3.18", features = ["json", "env-filter"] } ulid = "1.1.2" -unleash-types = { version = "0.12", features = ["openapi", "hashes"] } -unleash-yggdrasil = { version = "0.12.0" } +unleash-types = { version = "0.13", features = ["openapi", "hashes"] } +unleash-yggdrasil = { version = "0.13.0" } utoipa = { version = "4.2.3", features = ["actix_extras", "chrono"] } utoipa-swagger-ui = { version = "7.1.0", features = ["actix-web"] } [dev-dependencies] diff --git a/server/src/client_api.rs b/server/src/client_api.rs index bfea433c..72896ded 100644 --- a/server/src/client_api.rs +++ b/server/src/client_api.rs @@ -283,7 +283,7 @@ mod tests { ClientFeature, Constraint, Operator, Strategy, StrategyVariant, }; use unleash_types::client_metrics::{ - ClientMetricsEnv, ConnectViaBuilder, MetricBucket, ToggleStats, + ClientMetricsEnv, ConnectViaBuilder, MetricBucket, MetricsMetadata, ToggleStats, }; use unleash_yggdrasil::EngineState; @@ -310,6 +310,12 @@ mod tests { }, }, environment: Some("development".into()), + metadata: MetricsMetadata { + platform_name: Some("test".into()), + platform_version: Some("1.0".into()), + sdk_version: Some("1.0".into()), + yggdrasil_version: None, + }, })) .to_request() } @@ -329,9 +335,14 @@ mod tests { environment: None, instance_id: None, interval: 10, - sdk_version: None, started: Default::default(), strategies: vec![], + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }], metrics: vec![ClientMetricsEnv { feature_name: "".to_string(), @@ -341,6 +352,12 @@ mod tests { yes: 0, no: 0, variants: Default::default(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }], })) .to_request() @@ -407,6 +424,12 @@ mod tests { yes: 1, no: 0, variants: HashMap::new(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None + } }; assert_eq!(found_metric.yes, expected.yes); diff --git a/server/src/frontend_api.rs b/server/src/frontend_api.rs index be8798cc..63fe6ec2 100644 --- a/server/src/frontend_api.rs +++ b/server/src/frontend_api.rs @@ -487,6 +487,7 @@ pub fn evaluate_feature( payload: r.variant.payload, }, impression_data: r.impression_data, + impressionData: r.impression_data }) .ok_or_else(|| EdgeError::FeatureNotFound(feature_name.clone())) } @@ -735,6 +736,7 @@ pub fn frontend_from_yggdrasil( payload: resolved.variant.payload.clone(), }, impression_data: resolved.impression_data, + impressionData: resolved.impression_data }) .collect::>(); FrontendResult { toggles } @@ -792,7 +794,7 @@ mod tests { use std::str::FromStr; use std::sync::Arc; use tracing_test::traced_test; - use unleash_types::client_metrics::ClientMetricsEnv; + use unleash_types::client_metrics::{ClientMetricsEnv, MetricsMetadata}; use unleash_types::{ client_features::{ClientFeature, ClientFeatures, Constraint, Operator, Strategy}, frontend::{EvaluatedToggle, EvaluatedVariant, FrontendResult}, @@ -1005,6 +1007,7 @@ mod tests { payload: None, }, impression_data: false, + impressionData: false }], }; @@ -1062,6 +1065,7 @@ mod tests { payload: None, }, impression_data: false, + impressionData: false }], }; @@ -1117,6 +1121,7 @@ mod tests { payload: None, }, impression_data: false, + impressionData: false }], }; @@ -1192,6 +1197,12 @@ mod tests { yes: 1, no: 0, variants: HashMap::new(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }; assert_eq!(found_metric.yes, expected.yes); diff --git a/server/src/http/feature_refresher.rs b/server/src/http/feature_refresher.rs index fa339161..b2a1bffe 100644 --- a/server/src/http/feature_refresher.rs +++ b/server/src/http/feature_refresher.rs @@ -7,7 +7,7 @@ use dashmap::DashMap; use reqwest::StatusCode; use tracing::{debug, info, warn}; use unleash_types::client_features::Segment; -use unleash_types::client_metrics::ClientApplication; +use unleash_types::client_metrics::{ClientApplication, MetricsMetadata}; use unleash_types::{ client_features::{ClientFeature, ClientFeatures}, Deduplicate, @@ -124,9 +124,14 @@ fn client_application_from_token(token: EdgeToken, refresh_interval: i64) -> Cli environment: token.environment, instance_id: None, interval: refresh_interval as u32, - sdk_version: Some(format!("unleash-edge:{}", build::PKG_VERSION)), started: Utc::now(), strategies: vec![], + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: Some(format!("unleash-edge:{}", build::PKG_VERSION)), + yggdrasil_version: None + } } } diff --git a/server/src/metrics/client_metrics.rs b/server/src/metrics/client_metrics.rs index 878acbb9..e6fe7369 100644 --- a/server/src/metrics/client_metrics.rs +++ b/server/src/metrics/client_metrics.rs @@ -141,6 +141,7 @@ pub(crate) fn register_client_metrics( edge_token .environment .unwrap_or_else(|| "development".into()), + metrics.metadata.clone(), ); metrics_cache.sink_metrics(&metrics); @@ -361,7 +362,9 @@ mod test { use std::collections::HashMap; use std::str::FromStr; use test_case::test_case; - use unleash_types::client_metrics::{ClientMetricsEnv, ConnectVia, ConnectViaBuilder}; + use unleash_types::client_metrics::{ + ClientMetricsEnv, ConnectVia, ConnectViaBuilder, MetricsMetadata, + }; #[test] fn cache_aggregates_data_correctly() { @@ -377,6 +380,12 @@ mod test { yes: 1, no: 0, variants: HashMap::new(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }; let metrics = vec![ @@ -410,6 +419,12 @@ mod test { yes: 2, no: 0, variants: HashMap::new(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }; assert_eq!(found_metric.yes, expected.yes); @@ -436,6 +451,12 @@ mod test { yes: 1, no: 0, variants: HashMap::new(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }; let metrics = vec![ @@ -469,6 +490,12 @@ mod test { yes: 2, no: 0, variants: HashMap::new(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }; let new_metric = cache @@ -489,6 +516,12 @@ mod test { yes: 1, no: 0, variants: HashMap::new(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }; assert_eq!(cache.metrics.len(), 2); @@ -519,6 +552,12 @@ mod test { yes: 1, no: 0, variants: HashMap::new(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }; let metrics = vec![ @@ -542,9 +581,14 @@ mod test { environment: Some("development".into()), instance_id: Some("test".into()), interval: 60, - sdk_version: None, started: Default::default(), strategies: vec![], + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }; let connected_via_test_instance = client_application.connect_via("test", "instance"); let connected_via_edge_as_well = connected_via_test_instance.connect_via("edge", "edgeid"); @@ -571,10 +615,10 @@ mod test { } #[test_case(10, 100, 1; "10 apps 100 toggles. Will not be split")] - #[test_case(1, 10000, 16; "1 app 10k toggles, will be split into 16 batches")] - #[test_case(1000, 1000, 4; "1000 apps 1000 toggles, will be split into 4 batches")] - #[test_case(500, 5000, 10; "500 apps 5000 toggles, will be split into 10 batches")] - #[test_case(5000, 1, 14; "5000 apps 1 metric will be split")] + #[test_case(1, 10000, 25; "1 app 10k toggles, will be split into 25 batches")] + #[test_case(1000, 1000, 7; "1000 apps 1000 toggles, will be split into 7 batches")] + #[test_case(500, 5000, 15; "500 apps 5000 toggles, will be split into 15 batches")] + #[test_case(5000, 1, 17; "5000 apps 1 metric will be split")] fn splits_successfully_into_sendable_chunks(apps: u64, toggles: u64, batch_count: usize) { let apps: Vec = (1..=apps) .map(|app_id| ClientApplication { @@ -586,11 +630,16 @@ mod test { app_name: "edge".into(), instance_id: "some-instance-id".into(), }]), - sdk_version: Some("some-test-sdk".into()), started: DateTime::parse_from_rfc3339("1867-11-07T12:00:00Z") .unwrap() .with_timezone(&Utc), strategies: vec![], + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: Some("some-test-sdk".into()), + yggdrasil_version: None, + }, }) .collect(); @@ -605,6 +654,12 @@ mod test { yes: 1, no: 1, variants: HashMap::new(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }) .collect(); @@ -646,6 +701,12 @@ mod test { yes: 0, no: 0, variants: HashMap::new(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }; let metrics = vec![ @@ -684,6 +745,12 @@ mod test { yes: 50, no: 10, variants: Default::default(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }, ClientMetricsEnv { feature_name: "feature_two".to_string(), @@ -693,6 +760,12 @@ mod test { yes: 50, no: 10, variants: Default::default(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }, ], }; @@ -716,6 +789,12 @@ mod test { yes: 50, no: 10, variants: Default::default(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }, ClientMetricsEnv { feature_name: "feature_two".to_string(), @@ -725,6 +804,12 @@ mod test { yes: 50, no: 10, variants: Default::default(), + metadata: MetricsMetadata { + platform_name: None, + platform_version: None, + sdk_version: None, + yggdrasil_version: None, + }, }, ]; let cache = MetricsCache::default();