From e09844feef03ab2231a2261e3cdaa3fd975795a6 Mon Sep 17 00:00:00 2001 From: Dalitso Banda Date: Fri, 21 Apr 2023 13:13:14 -0700 Subject: [PATCH 1/7] make metrics a trait --- rust_snuba/rust_arroyo/src/utils/metrics.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/rust_snuba/rust_arroyo/src/utils/metrics.rs b/rust_snuba/rust_arroyo/src/utils/metrics.rs index b92a9acb8c..d769432194 100644 --- a/rust_snuba/rust_arroyo/src/utils/metrics.rs +++ b/rust_snuba/rust_arroyo/src/utils/metrics.rs @@ -8,7 +8,7 @@ use std::collections::HashMap; use std::net::{ToSocketAddrs, UdpSocket}; use std::sync::Arc; -pub trait Metrics { +pub trait MetricsClientTrait: Send + Sync { fn counter( &self, key: &str, @@ -34,12 +34,13 @@ pub trait Metrics { ); } +// #[derive( Clone)] pub struct MetricsClient { statsd_client: StatsdClient, prefix: String, } -impl Metrics for MetricsClient { +impl MetricsClientTrait for MetricsClient { fn counter( &self, key: &str, @@ -142,7 +143,7 @@ impl MetricsClient { } lazy_static! { - static ref METRICS_CLIENT: RwLock>> = RwLock::new(None); + static ref METRICS_CLIENT: RwLock>> = RwLock::new(None); } const METRICS_MAX_QUEUE_SIZE: usize = 1024; @@ -164,6 +165,9 @@ pub fn init(prefix: &str, host: A) { println!("Emitting metrics with prefix {}", metrics_client.prefix); *METRICS_CLIENT.write() = Some(Arc::new(metrics_client)); } +pub fn configure_metrics(metrics_client: impl MetricsClientTrait + 'static){ + *METRICS_CLIENT.write() = Some(Arc::new(metrics_client)); +} // TODO: Remove cloning METRICS_CLIENT each time this is called using thread local storage. pub fn increment( From 4d6db8556d8fc1f706536e8625a7d6c6be04ba7a Mon Sep 17 00:00:00 2001 From: Dalitso Banda Date: Fri, 21 Apr 2023 13:19:07 -0700 Subject: [PATCH 2/7] add some implementations of metrics backends --- rust_snuba/Cargo.toml | 6 + rust_snuba/src/lib.rs | 1 + .../metrics/backends/abstract_backend.rs | 17 +++ .../src/utils/metrics/backends/datadog.rs | 96 +++++++++++++++ rust_snuba/src/utils/metrics/backends/mod.rs | 3 + .../src/utils/metrics/backends/testing.rs | 109 ++++++++++++++++++ rust_snuba/src/utils/metrics/mod.rs | 3 + rust_snuba/src/utils/mod.rs | 1 + 8 files changed, 236 insertions(+) create mode 100644 rust_snuba/src/utils/metrics/backends/abstract_backend.rs create mode 100644 rust_snuba/src/utils/metrics/backends/datadog.rs create mode 100644 rust_snuba/src/utils/metrics/backends/mod.rs create mode 100644 rust_snuba/src/utils/metrics/backends/testing.rs create mode 100644 rust_snuba/src/utils/metrics/mod.rs create mode 100644 rust_snuba/src/utils/mod.rs diff --git a/rust_snuba/Cargo.toml b/rust_snuba/Cargo.toml index e94409e963..54d588339d 100644 --- a/rust_snuba/Cargo.toml +++ b/rust_snuba/Cargo.toml @@ -27,3 +27,9 @@ glob = "0.3.1" pyo3 = { version = "0.18.1", features = ["chrono", "extension-module"] } ctrlc = "3.2.5" sentry = "0.31.0" + +# these are unofficial libs. haven't validated them yet +dogstatsd = "0.8.0" +cadence = "0.29.0" + +lazy_static = "1.4.0" diff --git a/rust_snuba/src/lib.rs b/rust_snuba/src/lib.rs index 06306a5a0c..0a558ff294 100644 --- a/rust_snuba/src/lib.rs +++ b/rust_snuba/src/lib.rs @@ -2,6 +2,7 @@ mod config; mod consumer; mod strategies; mod types; +mod utils; use pyo3::prelude::*; diff --git a/rust_snuba/src/utils/metrics/backends/abstract_backend.rs b/rust_snuba/src/utils/metrics/backends/abstract_backend.rs new file mode 100644 index 0000000000..a731eeeda7 --- /dev/null +++ b/rust_snuba/src/utils/metrics/backends/abstract_backend.rs @@ -0,0 +1,17 @@ +use rust_arroyo::utils::metrics::MetricsClientTrait; + +pub trait MetricsBackend: MetricsClientTrait { + // fn increment(&self, name: &str, value: i64, tags: &[&str]); + // fn gauge(&self, name: &str, value: f64, tags: &[&str]); + // fn timing(&self, name: &str, value: i64, tags: &[&str]); + fn events( + &self, + title: &str, + text: &str, + alert_type: &str, + priority: &str, + tags: &[&str], + ); + // fn counter(&self, name: &str, value: f64, tags: &[&str]) -> Result<(), Error>; + // fn histogram(&self, name: &str, value: f64, tags: &[&str]) -> Result<(), Error>; +} diff --git a/rust_snuba/src/utils/metrics/backends/datadog.rs b/rust_snuba/src/utils/metrics/backends/datadog.rs new file mode 100644 index 0000000000..1b2c364d48 --- /dev/null +++ b/rust_snuba/src/utils/metrics/backends/datadog.rs @@ -0,0 +1,96 @@ +// use crate::utils::metrics::backends::MetricsBackend; + +use rust_arroyo::utils::metrics::{gauge, increment, init, time, MetricsClientTrait}; +use dogstatsd; +use cadence::StatsdClient; + +use super::abstract_backend::MetricsBackend; +pub struct DatadogMetricsBackend { + client_sd: dogstatsd::Client, + host: String, + port: u16, + tags: Vec, +} + +impl MetricsBackend for DatadogMetricsBackend { + fn events( + &self, + title: &str, + text: &str, + alert_type: &str, + priority: &str, + tags: &[&str], + ) { + // TODO figure out how to send priority and alert_type + self.client_sd.event(title, text, tags ); + } +} + + +impl DatadogMetricsBackend { + pub fn new(host: String, port: u16 , tags:Vec) -> Self { + + let host_port: String = format!("{}:{}", host, port); + let built_options = dogstatsd::OptionsBuilder::new() + .to_addr(host_port).build(); + + let client = dogstatsd::Client::new(dogstatsd::Options::default()).unwrap(); + DatadogMetricsBackend { + client_sd: client, + host, + port, + tags + } + } +} + +impl MetricsClientTrait for DatadogMetricsBackend { + fn counter( + &self, + key: &str, + value: Option, + tags: Option>, + sample_rate: Option, + ) { + let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{}:{}", k, v)).collect(); + + match value { + Some(v) => { + for i in 0..v.abs() { + if v < 0 { + self.client_sd.decr(key, &tags_str).unwrap(); + } else { + self.client_sd.incr(key, &tags_str).unwrap(); + } + } + } + None => { + self.client_sd.incr(key, tags_str); + } + } + + + } + + fn gauge( + &self, + key: &str, + value: u64, + tags: Option>, + sample_rate: Option, + ) { + let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{}:{}", k, v)).collect(); + self.client_sd.gauge(key, value.to_string(), tags_str).unwrap(); + } + + fn time( + &self, + key: &str, + value: u64, + tags: Option>, + sample_rate: Option, + ) { + let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{}:{}", k, v)).collect(); + self.client_sd.timing(key, value.try_into().unwrap(), tags_str); + } +} diff --git a/rust_snuba/src/utils/metrics/backends/mod.rs b/rust_snuba/src/utils/metrics/backends/mod.rs new file mode 100644 index 0000000000..4936a59d05 --- /dev/null +++ b/rust_snuba/src/utils/metrics/backends/mod.rs @@ -0,0 +1,3 @@ +pub mod datadog; +pub mod abstract_backend; +pub mod testing; diff --git a/rust_snuba/src/utils/metrics/backends/testing.rs b/rust_snuba/src/utils/metrics/backends/testing.rs new file mode 100644 index 0000000000..5e6abdea0c --- /dev/null +++ b/rust_snuba/src/utils/metrics/backends/testing.rs @@ -0,0 +1,109 @@ +use std::{collections::HashMap, sync::{Arc, Mutex}}; + +use rust_arroyo::utils::metrics::MetricsClientTrait; + +use super::abstract_backend::MetricsBackend; +use lazy_static::lazy_static; + +lazy_static! { + // static ref METRICS_CLIENT: RwLock>> = RwLock::new(None); + static ref METRICS: Mutex>> = { + let mut m = HashMap::new(); + Mutex::new(m) + }; +} +// static METRICS: Arc>> = Arc::new(HashMap::new()); + + +pub struct MetricCall { + value: String, + tags: Vec, +} +pub struct TestingMetricsBackend { +} + +impl MetricsBackend for TestingMetricsBackend { + fn events( + &self, + title: &str, + text: &str, + alert_type: &str, + priority: &str, + tags: &[&str], + ) { + todo!() + } +} + +impl MetricsClientTrait for TestingMetricsBackend{ + fn counter(&self, name: &str, value: Option, tags: Option>, sample_rate: Option) { + let mut tags_vec = Vec::new(); + if let Some(tags) = tags { + for (k, v) in tags { + tags_vec.push(format!("{}:{}", k, v)); + } + } + let mut metrics_map = METRICS.lock().unwrap(); + let metric = metrics_map.entry(name.to_string()).or_insert(Vec::new()); + metric.push(MetricCall { + value: value.unwrap().to_string(), + tags: tags_vec, + }); + } + + fn gauge(&self, name: &str, value: u64, tags: Option>, sample_rate: Option) { + let mut tags_vec = Vec::new(); + if let Some(tags) = tags { + for (k, v) in tags { + tags_vec.push(format!("{}:{}", k, v)); + } + } + let mut metrics_map = METRICS.lock().unwrap(); + let metric = metrics_map.entry(name.to_string()).or_insert(Vec::new()); + + metric.push(MetricCall { + value: value.to_string(), + tags: tags_vec, + }); + } + + fn time(&self, name: &str, value: u64, tags: Option>, sample_rate: Option) { + let mut tags_vec = Vec::new(); + if let Some(tags) = tags { + for (k, v) in tags { + tags_vec.push(format!("{}:{}", k, v)); + } + } + let mut metrics_map = METRICS.lock().unwrap(); + let metric = metrics_map.entry(name.to_string()).or_insert(Vec::new()); + + metric.push(MetricCall { + value: value.to_string(), + tags: tags_vec, + }); + } + +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use rust_arroyo::utils::metrics::{configure_metrics, MetricsClientTrait}; + + + #[test] + fn test_testing_backend() { + let testing_backend = super::TestingMetricsBackend {}; + + let mut tags: HashMap<&str, &str> = HashMap::new(); + tags.insert("tag1", "value1"); + tags.insert("tag2", "value2"); + + testing_backend.counter("test_counter", Some(1), Some(tags.clone()), None); + testing_backend.gauge("test_gauge", 1, Some(tags.clone()), None); + testing_backend.time("test_time", 1, Some(tags.clone()), None); + + configure_metrics(testing_backend); + } +} diff --git a/rust_snuba/src/utils/metrics/mod.rs b/rust_snuba/src/utils/metrics/mod.rs new file mode 100644 index 0000000000..de9ee6ff27 --- /dev/null +++ b/rust_snuba/src/utils/metrics/mod.rs @@ -0,0 +1,3 @@ +pub mod metrics; +pub mod backends; +pub mod metrics_wrapper; diff --git a/rust_snuba/src/utils/mod.rs b/rust_snuba/src/utils/mod.rs new file mode 100644 index 0000000000..e144883287 --- /dev/null +++ b/rust_snuba/src/utils/mod.rs @@ -0,0 +1 @@ +pub mod metrics; From a185dcc86cd00017f6f33515fb7a28b0cde0f399 Mon Sep 17 00:00:00 2001 From: Dalitso Banda Date: Fri, 28 Apr 2023 13:21:17 -0700 Subject: [PATCH 3/7] add unit tests --- .../src/utils/metrics/backends/datadog.rs | 52 +++++++++++++++---- .../src/utils/metrics/backends/testing.rs | 10 +++- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/rust_snuba/src/utils/metrics/backends/datadog.rs b/rust_snuba/src/utils/metrics/backends/datadog.rs index 1b2c364d48..8a9b701321 100644 --- a/rust_snuba/src/utils/metrics/backends/datadog.rs +++ b/rust_snuba/src/utils/metrics/backends/datadog.rs @@ -1,14 +1,9 @@ -// use crate::utils::metrics::backends::MetricsBackend; - use rust_arroyo::utils::metrics::{gauge, increment, init, time, MetricsClientTrait}; use dogstatsd; -use cadence::StatsdClient; use super::abstract_backend::MetricsBackend; pub struct DatadogMetricsBackend { client_sd: dogstatsd::Client, - host: String, - port: u16, tags: Vec, } @@ -37,8 +32,6 @@ impl DatadogMetricsBackend { let client = dogstatsd::Client::new(dogstatsd::Options::default()).unwrap(); DatadogMetricsBackend { client_sd: client, - host, - port, tags } } @@ -56,7 +49,7 @@ impl MetricsClientTrait for DatadogMetricsBackend { match value { Some(v) => { - for i in 0..v.abs() { + for _ in 0..v.abs() { if v < 0 { self.client_sd.decr(key, &tags_str).unwrap(); } else { @@ -65,7 +58,7 @@ impl MetricsClientTrait for DatadogMetricsBackend { } } None => { - self.client_sd.incr(key, tags_str); + self.client_sd.incr(key, tags_str).unwrap(); } } @@ -80,7 +73,8 @@ impl MetricsClientTrait for DatadogMetricsBackend { sample_rate: Option, ) { let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{}:{}", k, v)).collect(); - self.client_sd.gauge(key, value.to_string(), tags_str).unwrap(); + let res = self.client_sd.gauge(key, value.to_string(), tags_str).unwrap(); + println!("ok = {:?}", res) } fn time( @@ -91,6 +85,42 @@ impl MetricsClientTrait for DatadogMetricsBackend { sample_rate: Option, ) { let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{}:{}", k, v)).collect(); - self.client_sd.timing(key, value.try_into().unwrap(), tags_str); + self.client_sd.timing(key, value.try_into().unwrap(), tags_str).unwrap(); + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use dogstatsd::Options; + use rust_arroyo::utils::metrics::{configure_metrics, MetricsClientTrait, self}; + + use crate::utils::metrics::backends::{datadog::DatadogMetricsBackend}; + + + #[test] + fn test_testing_backend() { + + let custom_options = Options::new("0.0.0.0:9000", "0.0.0.0:8125", "analytics", vec!(String::new())); + // let client = dogstatsd::Client::new(dogstatsd::Options::default()).unwrap(); + let client = dogstatsd::Client::new(custom_options).unwrap(); + let client_tags: Vec = Vec::new(); + let testing_backend = DatadogMetricsBackend { + client_sd: client, + tags: client_tag + }; + + let mut tags: HashMap<&str, &str> = HashMap::new(); + tags.insert("tag1", "value1"); + tags.insert("tag2", "value2"); + + testing_backend.counter("test_counter", Some(1), Some(tags.clone()), None); + testing_backend.gauge("test_gauge", 1, Some(tags.clone()), None); + testing_backend.time("test_time", 1, Some(tags.clone()), None); + + // check configure_metrics writes to METRICS + configure_metrics(testing_backend); + metrics::time("c", 30, Some(HashMap::from([("tag3", "value3")])), None); } } diff --git a/rust_snuba/src/utils/metrics/backends/testing.rs b/rust_snuba/src/utils/metrics/backends/testing.rs index 5e6abdea0c..c24ef60d8d 100644 --- a/rust_snuba/src/utils/metrics/backends/testing.rs +++ b/rust_snuba/src/utils/metrics/backends/testing.rs @@ -89,7 +89,9 @@ impl MetricsClientTrait for TestingMetricsBackend{ mod tests { use std::collections::HashMap; - use rust_arroyo::utils::metrics::{configure_metrics, MetricsClientTrait}; + use rust_arroyo::utils::metrics::{configure_metrics, MetricsClientTrait, self}; + + use crate::utils::metrics::backends::testing::METRICS; #[test] @@ -103,7 +105,13 @@ mod tests { testing_backend.counter("test_counter", Some(1), Some(tags.clone()), None); testing_backend.gauge("test_gauge", 1, Some(tags.clone()), None); testing_backend.time("test_time", 1, Some(tags.clone()), None); + assert!(METRICS.lock().unwrap().contains_key("test_counter")); + assert!(METRICS.lock().unwrap().contains_key("test_gauge")); + assert!(METRICS.lock().unwrap().contains_key("test_time")); + // check configure_metrics writes to METRICS configure_metrics(testing_backend); + metrics::time("c", 30, Some(HashMap::from([("tag3", "value3")])), None); + assert!(METRICS.lock().unwrap().contains_key("c")); } } From 4c95e5cbfd9c151cecdaddb264362d249e9bce60 Mon Sep 17 00:00:00 2001 From: Dalitso Banda Date: Mon, 22 May 2023 21:22:35 -0700 Subject: [PATCH 4/7] linting --- rust_snuba/rust_arroyo/src/utils/metrics.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rust_snuba/rust_arroyo/src/utils/metrics.rs b/rust_snuba/rust_arroyo/src/utils/metrics.rs index d769432194..69076e9395 100644 --- a/rust_snuba/rust_arroyo/src/utils/metrics.rs +++ b/rust_snuba/rust_arroyo/src/utils/metrics.rs @@ -62,7 +62,7 @@ impl MetricsClientTrait for MetricsClient { match result { Ok(_) => {} Err(_err) => { - println!("Failed to send metric {}: {}", key, _err) + println!("Failed to send metric {key}: {_err}") } } } @@ -87,7 +87,7 @@ impl MetricsClientTrait for MetricsClient { match result { Ok(_) => {} Err(_err) => { - println!("Failed to send metric {}: {}", key, _err) + println!("Failed to send metric {key}: {_err}") } } } @@ -112,7 +112,7 @@ impl MetricsClientTrait for MetricsClient { match result { Ok(_) => {} Err(_err) => { - println!("Failed to send metric {}: {}", key, _err) + println!("Failed to send metric {key}: {_err}") } } } @@ -136,7 +136,7 @@ impl MetricsClient { match result { Ok(_) => {} Err(_err) => { - println!("Failed to send metric with tags: {}", _err) + println!("Failed to send metric with tags: {_err}") } } } From 7b73d0d8d986783adbe9a7c8dc87fb26890f35c1 Mon Sep 17 00:00:00 2001 From: Dalitso Banda Date: Mon, 22 May 2023 22:14:06 -0700 Subject: [PATCH 5/7] linting, remove some comments and old libs --- rust_snuba/Cargo.lock | 11 +++++ rust_snuba/Cargo.toml | 3 +- rust_snuba/rust_arroyo/src/utils/metrics.rs | 9 +--- rust_snuba/src/lib.rs | 2 +- .../metrics/backends/abstract_backend.rs | 5 -- .../src/utils/metrics/backends/datadog.rs | 46 +++++++++---------- .../src/utils/metrics/backends/testing.rs | 45 +++++++++--------- rust_snuba/src/utils/metrics/mod.rs | 2 - 8 files changed, 60 insertions(+), 63 deletions(-) diff --git a/rust_snuba/Cargo.lock b/rust_snuba/Cargo.lock index 3752097c58..baaaae885f 100644 --- a/rust_snuba/Cargo.lock +++ b/rust_snuba/Cargo.lock @@ -302,6 +302,15 @@ dependencies = [ "uuid 1.3.1", ] +[[package]] +name = "dogstatsd" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4609d1443932a4fcee5d2df820ef0597d039fa4154a5c60f798650a5b4f48e14" +dependencies = [ + "chrono", +] + [[package]] name = "encoding_rs" version = "0.8.32" @@ -1355,8 +1364,10 @@ version = "0.1.0" dependencies = [ "anyhow", "ctrlc", + "dogstatsd", "env_logger 0.10.0", "glob", + "lazy_static", "log", "pyo3", "rust_arroyo", diff --git a/rust_snuba/Cargo.toml b/rust_snuba/Cargo.toml index 54d588339d..781d8c54eb 100644 --- a/rust_snuba/Cargo.toml +++ b/rust_snuba/Cargo.toml @@ -28,8 +28,7 @@ pyo3 = { version = "0.18.1", features = ["chrono", "extension-module"] } ctrlc = "3.2.5" sentry = "0.31.0" -# these are unofficial libs. haven't validated them yet +# unofficial lib. haven't validated it yet dogstatsd = "0.8.0" -cadence = "0.29.0" lazy_static = "1.4.0" diff --git a/rust_snuba/rust_arroyo/src/utils/metrics.rs b/rust_snuba/rust_arroyo/src/utils/metrics.rs index 69076e9395..c85628fff4 100644 --- a/rust_snuba/rust_arroyo/src/utils/metrics.rs +++ b/rust_snuba/rust_arroyo/src/utils/metrics.rs @@ -210,16 +210,11 @@ mod tests { fn test_metrics() { init("my_host", "0.0.0.0:8125"); - assert!(!METRICS_CLIENT - .read() - .clone() - .unwrap() - .should_sample(Some(0.0)),); assert!(METRICS_CLIENT .read() .clone() - .unwrap() - .should_sample(Some(1.0)),); + .is_some() + ); increment( "a", diff --git a/rust_snuba/src/lib.rs b/rust_snuba/src/lib.rs index 0a558ff294..390078057e 100644 --- a/rust_snuba/src/lib.rs +++ b/rust_snuba/src/lib.rs @@ -2,7 +2,7 @@ mod config; mod consumer; mod strategies; mod types; -mod utils; +pub mod utils; use pyo3::prelude::*; diff --git a/rust_snuba/src/utils/metrics/backends/abstract_backend.rs b/rust_snuba/src/utils/metrics/backends/abstract_backend.rs index a731eeeda7..83820f31ad 100644 --- a/rust_snuba/src/utils/metrics/backends/abstract_backend.rs +++ b/rust_snuba/src/utils/metrics/backends/abstract_backend.rs @@ -1,9 +1,6 @@ use rust_arroyo::utils::metrics::MetricsClientTrait; pub trait MetricsBackend: MetricsClientTrait { - // fn increment(&self, name: &str, value: i64, tags: &[&str]); - // fn gauge(&self, name: &str, value: f64, tags: &[&str]); - // fn timing(&self, name: &str, value: i64, tags: &[&str]); fn events( &self, title: &str, @@ -12,6 +9,4 @@ pub trait MetricsBackend: MetricsClientTrait { priority: &str, tags: &[&str], ); - // fn counter(&self, name: &str, value: f64, tags: &[&str]) -> Result<(), Error>; - // fn histogram(&self, name: &str, value: f64, tags: &[&str]) -> Result<(), Error>; } diff --git a/rust_snuba/src/utils/metrics/backends/datadog.rs b/rust_snuba/src/utils/metrics/backends/datadog.rs index 8a9b701321..3a5c2fbe6c 100644 --- a/rust_snuba/src/utils/metrics/backends/datadog.rs +++ b/rust_snuba/src/utils/metrics/backends/datadog.rs @@ -1,10 +1,10 @@ -use rust_arroyo::utils::metrics::{gauge, increment, init, time, MetricsClientTrait}; +use rust_arroyo::utils::metrics::{MetricsClientTrait}; use dogstatsd; use super::abstract_backend::MetricsBackend; pub struct DatadogMetricsBackend { client_sd: dogstatsd::Client, - tags: Vec, + _tags: Vec, } impl MetricsBackend for DatadogMetricsBackend { @@ -12,12 +12,12 @@ impl MetricsBackend for DatadogMetricsBackend { &self, title: &str, text: &str, - alert_type: &str, - priority: &str, + _alert_type: &str, + _priority: &str, tags: &[&str], ) { // TODO figure out how to send priority and alert_type - self.client_sd.event(title, text, tags ); + self.client_sd.event(title, text, tags ).unwrap() } } @@ -25,14 +25,14 @@ impl MetricsBackend for DatadogMetricsBackend { impl DatadogMetricsBackend { pub fn new(host: String, port: u16 , tags:Vec) -> Self { - let host_port: String = format!("{}:{}", host, port); - let built_options = dogstatsd::OptionsBuilder::new() + let host_port: String = format!("{host}:{port}"); + let options = dogstatsd::OptionsBuilder::new() .to_addr(host_port).build(); - let client = dogstatsd::Client::new(dogstatsd::Options::default()).unwrap(); + let client = dogstatsd::Client::new(options).unwrap(); DatadogMetricsBackend { client_sd: client, - tags + _tags: tags } } } @@ -43,9 +43,9 @@ impl MetricsClientTrait for DatadogMetricsBackend { key: &str, value: Option, tags: Option>, - sample_rate: Option, + _sample_rate: Option, ) { - let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{}:{}", k, v)).collect(); + let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{k}:{v}")).collect(); match value { Some(v) => { @@ -70,11 +70,10 @@ impl MetricsClientTrait for DatadogMetricsBackend { key: &str, value: u64, tags: Option>, - sample_rate: Option, + _sample_rate: Option, ) { - let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{}:{}", k, v)).collect(); - let res = self.client_sd.gauge(key, value.to_string(), tags_str).unwrap(); - println!("ok = {:?}", res) + let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{k}:{v}")).collect(); + self.client_sd.gauge(key, value.to_string(), tags_str).unwrap(); } fn time( @@ -82,9 +81,9 @@ impl MetricsClientTrait for DatadogMetricsBackend { key: &str, value: u64, tags: Option>, - sample_rate: Option, + _sample_rate: Option, ) { - let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{}:{}", k, v)).collect(); + let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{k}:{v}")).collect(); self.client_sd.timing(key, value.try_into().unwrap(), tags_str).unwrap(); } } @@ -93,7 +92,6 @@ impl MetricsClientTrait for DatadogMetricsBackend { mod tests { use std::collections::HashMap; - use dogstatsd::Options; use rust_arroyo::utils::metrics::{configure_metrics, MetricsClientTrait, self}; use crate::utils::metrics::backends::{datadog::DatadogMetricsBackend}; @@ -101,14 +99,12 @@ mod tests { #[test] fn test_testing_backend() { - - let custom_options = Options::new("0.0.0.0:9000", "0.0.0.0:8125", "analytics", vec!(String::new())); - // let client = dogstatsd::Client::new(dogstatsd::Options::default()).unwrap(); - let client = dogstatsd::Client::new(custom_options).unwrap(); + // default client sends metrics to statsd daemon on localhost:8125 + let client = dogstatsd::Client::new(dogstatsd::Options::default()).unwrap(); let client_tags: Vec = Vec::new(); let testing_backend = DatadogMetricsBackend { client_sd: client, - tags: client_tag + _tags: client_tags }; let mut tags: HashMap<&str, &str> = HashMap::new(); @@ -122,5 +118,9 @@ mod tests { // check configure_metrics writes to METRICS configure_metrics(testing_backend); metrics::time("c", 30, Some(HashMap::from([("tag3", "value3")])), None); + + // test constructor + DatadogMetricsBackend::new("0.0.0.0".to_owned(), 8125, Vec::new()) + .counter("test_counter2", Some(2), Some(tags.clone()), None); } } diff --git a/rust_snuba/src/utils/metrics/backends/testing.rs b/rust_snuba/src/utils/metrics/backends/testing.rs index c24ef60d8d..a28b609784 100644 --- a/rust_snuba/src/utils/metrics/backends/testing.rs +++ b/rust_snuba/src/utils/metrics/backends/testing.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, sync::{Arc, Mutex}}; +use std::{collections::HashMap, sync::{Mutex}}; use rust_arroyo::utils::metrics::MetricsClientTrait; @@ -6,80 +6,79 @@ use super::abstract_backend::MetricsBackend; use lazy_static::lazy_static; lazy_static! { - // static ref METRICS_CLIENT: RwLock>> = RwLock::new(None); static ref METRICS: Mutex>> = { - let mut m = HashMap::new(); + let m = HashMap::new(); Mutex::new(m) }; } -// static METRICS: Arc>> = Arc::new(HashMap::new()); pub struct MetricCall { - value: String, - tags: Vec, + _value: String, + _tags: Vec, } + pub struct TestingMetricsBackend { } impl MetricsBackend for TestingMetricsBackend { fn events( &self, - title: &str, - text: &str, - alert_type: &str, - priority: &str, - tags: &[&str], + _title: &str, + _text: &str, + _alert_type: &str, + _priority: &str, + _tags: &[&str], ) { todo!() } } impl MetricsClientTrait for TestingMetricsBackend{ - fn counter(&self, name: &str, value: Option, tags: Option>, sample_rate: Option) { + fn counter(&self, name: &str, value: Option, tags: Option>, _sample_rate: Option) { let mut tags_vec = Vec::new(); if let Some(tags) = tags { for (k, v) in tags { - tags_vec.push(format!("{}:{}", k, v)); + tags_vec.push(format!("{k}:{v}")); } } let mut metrics_map = METRICS.lock().unwrap(); let metric = metrics_map.entry(name.to_string()).or_insert(Vec::new()); metric.push(MetricCall { - value: value.unwrap().to_string(), - tags: tags_vec, + _value: value.unwrap().to_string(), + _tags: tags_vec, }); } - fn gauge(&self, name: &str, value: u64, tags: Option>, sample_rate: Option) { + fn gauge(&self, name: &str, value: u64, tags: Option>, _sample_rate: Option) { let mut tags_vec = Vec::new(); if let Some(tags) = tags { for (k, v) in tags { - tags_vec.push(format!("{}:{}", k, v)); + tags_vec.push(format!("{k}:{v}")); } } let mut metrics_map = METRICS.lock().unwrap(); let metric = metrics_map.entry(name.to_string()).or_insert(Vec::new()); metric.push(MetricCall { - value: value.to_string(), - tags: tags_vec, + _value: value.to_string(), + _tags: tags_vec, }); } - fn time(&self, name: &str, value: u64, tags: Option>, sample_rate: Option) { + fn time(&self, name: &str, value: u64, tags: Option>, _sample_rate: Option) { let mut tags_vec = Vec::new(); if let Some(tags) = tags { for (k, v) in tags { - tags_vec.push(format!("{}:{}", k, v)); + tags_vec.push(format!("{k}:{v}")); } } let mut metrics_map = METRICS.lock().unwrap(); let metric = metrics_map.entry(name.to_string()).or_insert(Vec::new()); metric.push(MetricCall { - value: value.to_string(), - tags: tags_vec, + _value: value.to_string(), + _tags: tags_vec, }); } diff --git a/rust_snuba/src/utils/metrics/mod.rs b/rust_snuba/src/utils/metrics/mod.rs index de9ee6ff27..257edb5a63 100644 --- a/rust_snuba/src/utils/metrics/mod.rs +++ b/rust_snuba/src/utils/metrics/mod.rs @@ -1,3 +1 @@ -pub mod metrics; pub mod backends; -pub mod metrics_wrapper; From e04575f9ea2365b4c0795160853de6383c52c7f1 Mon Sep 17 00:00:00 2001 From: Dalitso Banda Date: Mon, 22 May 2023 23:22:30 -0700 Subject: [PATCH 6/7] add sampling --- rust_snuba/rust_arroyo/src/utils/metrics.rs | 19 +++++++++++++------ .../src/utils/metrics/backends/datadog.rs | 16 +++++++++++++--- .../src/utils/metrics/backends/testing.rs | 17 ++++++++++++++--- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/rust_snuba/rust_arroyo/src/utils/metrics.rs b/rust_snuba/rust_arroyo/src/utils/metrics.rs index c85628fff4..9aee1197c3 100644 --- a/rust_snuba/rust_arroyo/src/utils/metrics.rs +++ b/rust_snuba/rust_arroyo/src/utils/metrics.rs @@ -9,6 +9,11 @@ use std::net::{ToSocketAddrs, UdpSocket}; use std::sync::Arc; pub trait MetricsClientTrait: Send + Sync { + + fn should_sample(&self, sample_rate: Option) -> bool { + rand::thread_rng().gen_range(0.0..=1.0) < sample_rate.unwrap_or(1.0) + } + fn counter( &self, key: &str, @@ -34,7 +39,6 @@ pub trait MetricsClientTrait: Send + Sync { ); } -// #[derive( Clone)] pub struct MetricsClient { statsd_client: StatsdClient, prefix: String, @@ -120,9 +124,6 @@ impl MetricsClientTrait for MetricsClient { } impl MetricsClient { - fn should_sample(&self, sample_rate: Option) -> bool { - rand::thread_rng().gen_range(0.0..=1.0) < sample_rate.unwrap_or(1.0) - } fn send_with_tags<'t, T: cadence::Metric + From>( &self, @@ -210,11 +211,17 @@ mod tests { fn test_metrics() { init("my_host", "0.0.0.0:8125"); + assert!(!METRICS_CLIENT + .read() + .clone() + .unwrap() + .should_sample(Some(0.0)),); + assert!(METRICS_CLIENT .read() .clone() - .is_some() - ); + .unwrap() + .should_sample(Some(1.0)),); increment( "a", diff --git a/rust_snuba/src/utils/metrics/backends/datadog.rs b/rust_snuba/src/utils/metrics/backends/datadog.rs index 3a5c2fbe6c..da12a8aa27 100644 --- a/rust_snuba/src/utils/metrics/backends/datadog.rs +++ b/rust_snuba/src/utils/metrics/backends/datadog.rs @@ -43,8 +43,12 @@ impl MetricsClientTrait for DatadogMetricsBackend { key: &str, value: Option, tags: Option>, - _sample_rate: Option, + sample_rate: Option, ) { + if !self.should_sample(sample_rate) { + return; + } + let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{k}:{v}")).collect(); match value { @@ -70,8 +74,11 @@ impl MetricsClientTrait for DatadogMetricsBackend { key: &str, value: u64, tags: Option>, - _sample_rate: Option, + sample_rate: Option, ) { + if !self.should_sample(sample_rate) { + return; + } let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{k}:{v}")).collect(); self.client_sd.gauge(key, value.to_string(), tags_str).unwrap(); } @@ -81,8 +88,11 @@ impl MetricsClientTrait for DatadogMetricsBackend { key: &str, value: u64, tags: Option>, - _sample_rate: Option, + sample_rate: Option, ) { + if !self.should_sample(sample_rate) { + return; + } let tags_str: Vec = tags.unwrap().iter().map(|(k, v)| format!("{k}:{v}")).collect(); self.client_sd.timing(key, value.try_into().unwrap(), tags_str).unwrap(); } diff --git a/rust_snuba/src/utils/metrics/backends/testing.rs b/rust_snuba/src/utils/metrics/backends/testing.rs index a28b609784..3a61d38559 100644 --- a/rust_snuba/src/utils/metrics/backends/testing.rs +++ b/rust_snuba/src/utils/metrics/backends/testing.rs @@ -35,7 +35,11 @@ impl MetricsBackend for TestingMetricsBackend { } impl MetricsClientTrait for TestingMetricsBackend{ - fn counter(&self, name: &str, value: Option, tags: Option>, _sample_rate: Option) { + fn counter(&self, name: &str, value: Option, tags: Option>, sample_rate: Option) { + if !self.should_sample(sample_rate) { + return; + } + let mut tags_vec = Vec::new(); if let Some(tags) = tags { for (k, v) in tags { @@ -50,7 +54,10 @@ impl MetricsClientTrait for TestingMetricsBackend{ }); } - fn gauge(&self, name: &str, value: u64, tags: Option>, _sample_rate: Option) { + fn gauge(&self, name: &str, value: u64, tags: Option>, sample_rate: Option) { + if !self.should_sample(sample_rate) { + return; + } let mut tags_vec = Vec::new(); if let Some(tags) = tags { for (k, v) in tags { @@ -66,7 +73,11 @@ impl MetricsClientTrait for TestingMetricsBackend{ }); } - fn time(&self, name: &str, value: u64, tags: Option>, _sample_rate: Option) { + fn time(&self, name: &str, value: u64, tags: Option>, sample_rate: Option) { + if !self.should_sample(sample_rate) { + return; + } + let mut tags_vec = Vec::new(); if let Some(tags) = tags { for (k, v) in tags { From 14d6aa8c3339397f4b6de7a1a38f87348e60aea5 Mon Sep 17 00:00:00 2001 From: Dalitso Banda Date: Tue, 27 Jun 2023 14:20:44 -0700 Subject: [PATCH 7/7] use count instead of a series on incr/decr --- rust_snuba/src/utils/metrics/backends/datadog.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/rust_snuba/src/utils/metrics/backends/datadog.rs b/rust_snuba/src/utils/metrics/backends/datadog.rs index da12a8aa27..70cdb6e389 100644 --- a/rust_snuba/src/utils/metrics/backends/datadog.rs +++ b/rust_snuba/src/utils/metrics/backends/datadog.rs @@ -53,13 +53,7 @@ impl MetricsClientTrait for DatadogMetricsBackend { match value { Some(v) => { - for _ in 0..v.abs() { - if v < 0 { - self.client_sd.decr(key, &tags_str).unwrap(); - } else { - self.client_sd.incr(key, &tags_str).unwrap(); - } - } + self.client_sd.count(key, v, &tags_str).unwrap(); } None => { self.client_sd.incr(key, tags_str).unwrap();