diff --git a/Cargo.lock b/Cargo.lock index e05894e3c..b72047f37 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4791,6 +4791,7 @@ dependencies = [ "serde_json", "thiserror 2.0.12", "torrust-tracker-primitives", + "tracing", ] [[package]] diff --git a/packages/http-tracker-core/src/event/mod.rs b/packages/http-tracker-core/src/event/mod.rs index d235c179f..ae997156a 100644 --- a/packages/http-tracker-core/src/event/mod.rs +++ b/packages/http-tracker-core/src/event/mod.rs @@ -1,6 +1,7 @@ use std::net::{IpAddr, SocketAddr}; -use torrust_tracker_metrics::label::{LabelName, LabelSet, LabelValue}; +use torrust_tracker_metrics::label::{LabelSet, LabelValue}; +use torrust_tracker_metrics::label_name; use torrust_tracker_primitives::service_binding::ServiceBinding; pub mod sender; @@ -65,15 +66,15 @@ impl From for LabelSet { fn from(connection_context: ConnectionContext) -> Self { LabelSet::from([ ( - LabelName::new("server_binding_protocol"), + label_name!("server_binding_protocol"), LabelValue::new(&connection_context.server.service_binding.protocol().to_string()), ), ( - LabelName::new("server_binding_ip"), + label_name!("server_binding_ip"), LabelValue::new(&connection_context.server.service_binding.bind_address().ip().to_string()), ), ( - LabelName::new("server_binding_port"), + label_name!("server_binding_port"), LabelValue::new(&connection_context.server.service_binding.bind_address().port().to_string()), ), ]) diff --git a/packages/http-tracker-core/src/statistics/event/handler.rs b/packages/http-tracker-core/src/statistics/event/handler.rs index 0baec1cd9..182c86b01 100644 --- a/packages/http-tracker-core/src/statistics/event/handler.rs +++ b/packages/http-tracker-core/src/statistics/event/handler.rs @@ -1,7 +1,7 @@ use std::net::IpAddr; -use torrust_tracker_metrics::label::{LabelName, LabelSet, LabelValue}; -use torrust_tracker_metrics::metric::MetricName; +use torrust_tracker_metrics::label::{LabelSet, LabelValue}; +use torrust_tracker_metrics::{label_name, metric_name}; use torrust_tracker_primitives::DurationSinceUnixEpoch; use crate::event::Event; @@ -29,11 +29,15 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura // Extendable metrics let mut label_set = LabelSet::from(connection); - label_set.upsert(LabelName::new("request_kind"), LabelValue::new("announce")); - - stats_repository - .increase_counter(&MetricName::new(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now) - .await; + label_set.upsert(label_name!("request_kind"), LabelValue::new("announce")); + + match stats_repository + .increase_counter(&metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now) + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to increase the counter: {}", err), + }; } Event::TcpScrape { connection } => { // Global fixed metrics @@ -50,11 +54,15 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura // Extendable metrics let mut label_set = LabelSet::from(connection); - label_set.upsert(LabelName::new("request_kind"), LabelValue::new("scrape")); - - stats_repository - .increase_counter(&MetricName::new(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now) - .await; + label_set.upsert(label_name!("request_kind"), LabelValue::new("scrape")); + + match stats_repository + .increase_counter(&metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now) + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to increase the counter: {}", err), + }; } } diff --git a/packages/http-tracker-core/src/statistics/keeper.rs b/packages/http-tracker-core/src/statistics/keeper.rs index 01a7a1569..1b69f032d 100644 --- a/packages/http-tracker-core/src/statistics/keeper.rs +++ b/packages/http-tracker-core/src/statistics/keeper.rs @@ -3,6 +3,7 @@ use tokio::sync::broadcast::Receiver; use super::event::listener::dispatch_events; use super::repository::Repository; use crate::event::Event; +use crate::HTTP_TRACKER_LOG_TARGET; /// The service responsible for keeping tracker metrics (listening to statistics events and handle them). /// @@ -29,7 +30,13 @@ impl Keeper { pub fn run_event_listener(&mut self, receiver: Receiver) { let stats_repository = self.repository.clone(); - tokio::spawn(async move { dispatch_events(receiver, stats_repository).await }); + tracing::info!(target: HTTP_TRACKER_LOG_TARGET, "Starting HTTP tracker core event listener"); + + tokio::spawn(async move { + dispatch_events(receiver, stats_repository).await; + + tracing::info!(target: HTTP_TRACKER_LOG_TARGET, "HTTP tracker core event listener finished"); + }); } } diff --git a/packages/http-tracker-core/src/statistics/metrics.rs b/packages/http-tracker-core/src/statistics/metrics.rs index 0b442c1cb..bf053b04e 100644 --- a/packages/http-tracker-core/src/statistics/metrics.rs +++ b/packages/http-tracker-core/src/statistics/metrics.rs @@ -1,7 +1,7 @@ use serde::Serialize; use torrust_tracker_metrics::label::LabelSet; use torrust_tracker_metrics::metric::MetricName; -use torrust_tracker_metrics::metric_collection::MetricCollection; +use torrust_tracker_metrics::metric_collection::{Error, MetricCollection}; use torrust_tracker_primitives::DurationSinceUnixEpoch; /// Metrics collected by the tracker. @@ -24,11 +24,28 @@ pub struct Metrics { } impl Metrics { - pub fn increase_counter(&mut self, metric_name: &MetricName, labels: &LabelSet, now: DurationSinceUnixEpoch) { - self.metric_collection.increase_counter(metric_name, labels, now); + /// # Errors + /// + /// Returns an error if the metric does not exist and it cannot be created. + pub fn increase_counter( + &mut self, + metric_name: &MetricName, + labels: &LabelSet, + now: DurationSinceUnixEpoch, + ) -> Result<(), Error> { + self.metric_collection.increase_counter(metric_name, labels, now) } - pub fn set_gauge(&mut self, metric_name: &MetricName, labels: &LabelSet, value: f64, now: DurationSinceUnixEpoch) { - self.metric_collection.set_gauge(metric_name, labels, value, now); + /// # Errors + /// + /// Returns an error if the metric does not exist and it cannot be created. + pub fn set_gauge( + &mut self, + metric_name: &MetricName, + labels: &LabelSet, + value: f64, + now: DurationSinceUnixEpoch, + ) -> Result<(), Error> { + self.metric_collection.set_gauge(metric_name, labels, value, now) } } diff --git a/packages/http-tracker-core/src/statistics/mod.rs b/packages/http-tracker-core/src/statistics/mod.rs index 026c435af..a5d6d37a5 100644 --- a/packages/http-tracker-core/src/statistics/mod.rs +++ b/packages/http-tracker-core/src/statistics/mod.rs @@ -7,7 +7,7 @@ pub mod setup; use metrics::Metrics; use torrust_tracker_metrics::metric::description::MetricDescription; -use torrust_tracker_metrics::metric::MetricName; +use torrust_tracker_metrics::metric_name; use torrust_tracker_metrics::unit::Unit; const HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL: &str = "http_tracker_core_requests_received_total"; @@ -17,7 +17,7 @@ pub fn describe_metrics() -> Metrics { let mut metrics = Metrics::default(); metrics.metric_collection.describe_counter( - &MetricName::new(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), + &metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), Some(Unit::Count), Some(MetricDescription::new("Total number of HTTP requests received")), ); diff --git a/packages/http-tracker-core/src/statistics/repository.rs b/packages/http-tracker-core/src/statistics/repository.rs index 88345722b..d5e718821 100644 --- a/packages/http-tracker-core/src/statistics/repository.rs +++ b/packages/http-tracker-core/src/statistics/repository.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use tokio::sync::{RwLock, RwLockReadGuard}; use torrust_tracker_metrics::label::LabelSet; use torrust_tracker_metrics::metric::MetricName; +use torrust_tracker_metrics::metric_collection::Error; use torrust_tracker_primitives::DurationSinceUnixEpoch; use super::describe_metrics; @@ -56,9 +57,22 @@ impl Repository { drop(stats_lock); } - pub async fn increase_counter(&self, metric_name: &MetricName, labels: &LabelSet, now: DurationSinceUnixEpoch) { + /// # Errors + /// + /// This function will return an error if the metric collection fails to + /// increase the counter. + pub async fn increase_counter( + &self, + metric_name: &MetricName, + labels: &LabelSet, + now: DurationSinceUnixEpoch, + ) -> Result<(), Error> { let mut stats_lock = self.stats.write().await; - stats_lock.increase_counter(metric_name, labels, now); + + let result = stats_lock.increase_counter(metric_name, labels, now); + drop(stats_lock); + + result } } diff --git a/packages/metrics/Cargo.toml b/packages/metrics/Cargo.toml index 6520cf244..0597785f4 100644 --- a/packages/metrics/Cargo.toml +++ b/packages/metrics/Cargo.toml @@ -21,6 +21,7 @@ serde = { version = "1", features = ["derive"] } serde_json = "1.0.140" thiserror = "2" torrust-tracker-primitives = { version = "3.0.0-develop", path = "../primitives" } +tracing = "0.1.41" [dev-dependencies] approx = "0.5.1" diff --git a/packages/metrics/src/label/mod.rs b/packages/metrics/src/label/mod.rs index b5fd3b745..880fdbbb1 100644 --- a/packages/metrics/src/label/mod.rs +++ b/packages/metrics/src/label/mod.rs @@ -1,7 +1,7 @@ -mod name; +pub mod name; mod pair; mod set; -mod value; +pub mod value; pub type LabelName = name::LabelName; pub type LabelValue = value::LabelValue; diff --git a/packages/metrics/src/label/name.rs b/packages/metrics/src/label/name.rs index 22e75572f..194aeb2b3 100644 --- a/packages/metrics/src/label/name.rs +++ b/packages/metrics/src/label/name.rs @@ -14,11 +14,7 @@ impl LabelName { /// Panics if the provided name is empty. #[must_use] pub fn new(name: &str) -> Self { - assert!( - !name.is_empty(), - "Label name cannot be empty. It must have at least one character." - ); - + assert!(!name.is_empty(), "Label name cannot be empty."); Self(name.to_owned()) } } @@ -69,6 +65,19 @@ impl PrometheusSerializable for LabelName { } } } + +#[macro_export] +macro_rules! label_name { + ("") => { + compile_error!("Label name cannot be empty"); + }; + ($name:literal) => { + $crate::label::name::LabelName::new($name) + }; + ($name:ident) => { + $crate::label::name::LabelName::new($name) + }; +} #[cfg(test)] mod tests { mod serialization_of_label_name_to_prometheus { @@ -83,7 +92,7 @@ mod tests { #[case("3 leading lowercase", "v123", "v123")] #[case("4 leading uppercase", "V123", "V123")] fn valid_names_in_prometheus(#[case] case: &str, #[case] input: &str, #[case] output: &str) { - assert_eq!(LabelName::new(input).to_prometheus(), output, "{case} failed: {input:?}"); + assert_eq!(label_name!(input).to_prometheus(), output, "{case} failed: {input:?}"); } #[rstest] @@ -96,7 +105,7 @@ mod tests { #[case("7 all invalid characters", "!@#$%^&*()", "__________")] #[case("8 non_ascii_characters", "ñaca©", "_aca_")] fn names_that_need_changes_in_prometheus(#[case] case: &str, #[case] input: &str, #[case] output: &str) { - assert_eq!(LabelName::new(input).to_prometheus(), output, "{case} failed: {input:?}"); + assert_eq!(label_name!(input).to_prometheus(), output, "{case} failed: {input:?}"); } #[rstest] @@ -105,11 +114,11 @@ mod tests { #[case("3 processed to double underscore", "^^name", "___name")] #[case("4 processed to double underscore after first char", "0__name", "___name")] fn names_starting_with_double_underscore(#[case] case: &str, #[case] input: &str, #[case] output: &str) { - assert_eq!(LabelName::new(input).to_prometheus(), output, "{case} failed: {input:?}"); + assert_eq!(label_name!(input).to_prometheus(), output, "{case} failed: {input:?}"); } #[test] - #[should_panic(expected = "Label name cannot be empty. It must have at least one character.")] + #[should_panic(expected = "Label name cannot be empty.")] fn empty_name() { let _name = LabelName::new(""); } diff --git a/packages/metrics/src/label/pair.rs b/packages/metrics/src/label/pair.rs index c89c726bd..858902451 100644 --- a/packages/metrics/src/label/pair.rs +++ b/packages/metrics/src/label/pair.rs @@ -13,16 +13,16 @@ impl PrometheusSerializabl #[cfg(test)] mod tests { mod serialization_of_label_pair_to_prometheus { - use super::super::LabelName; use crate::label::LabelValue; + use crate::label_name; use crate::prometheus::PrometheusSerializable; #[test] fn test_label_pair_serialization_to_prometheus() { - let label_pair = (LabelName::new("label_name"), LabelValue::new("value")); + let label_pair = (label_name!("label_name"), LabelValue::new("value")); assert_eq!(label_pair.to_prometheus(), r#"label_name="value""#); - let label_pair = (&LabelName::new("label_name"), &LabelValue::new("value")); + let label_pair = (&label_name!("label_name"), &LabelValue::new("value")); assert_eq!(label_pair.to_prometheus(), r#"label_name="value""#); } } diff --git a/packages/metrics/src/label/set.rs b/packages/metrics/src/label/set.rs index f46b01095..2b6334fc7 100644 --- a/packages/metrics/src/label/set.rs +++ b/packages/metrics/src/label/set.rs @@ -180,6 +180,7 @@ mod tests { use super::{LabelName, LabelValue}; use crate::label::LabelSet; + use crate::label_name; use crate::prometheus::PrometheusSerializable; fn sample_vec_of_label_pairs() -> Vec<(LabelName, LabelValue)> { @@ -188,9 +189,9 @@ mod tests { fn sample_array_of_label_pairs() -> [(LabelName, LabelValue); 3] { [ - (LabelName::new("server_service_binding_protocol"), LabelValue::new("http")), - (LabelName::new("server_service_binding_ip"), LabelValue::new("0.0.0.0")), - (LabelName::new("server_service_binding_port"), LabelValue::new("7070")), + (label_name!("server_service_binding_protocol"), LabelValue::new("http")), + (label_name!("server_service_binding_ip"), LabelValue::new("0.0.0.0")), + (label_name!("server_service_binding_port"), LabelValue::new("7070")), ] } @@ -232,12 +233,12 @@ mod tests { #[test] fn it_should_allow_instantiation_from_a_label_pair() { - let label_set: LabelSet = (LabelName::new("label_name"), LabelValue::new("value")).into(); + let label_set: LabelSet = (label_name!("label_name"), LabelValue::new("value")).into(); assert_eq!( label_set, LabelSet { - items: BTreeMap::from([(LabelName::new("label_name"), LabelValue::new("value"))]) + items: BTreeMap::from([(label_name!("label_name"), LabelValue::new("value"))]) } ); } @@ -246,10 +247,10 @@ mod tests { fn it_should_allow_inserting_a_new_label_pair() { let mut label_set = LabelSet::default(); - label_set.upsert(LabelName::new("label_name"), LabelValue::new("value")); + label_set.upsert(label_name!("label_name"), LabelValue::new("value")); assert_eq!( - label_set.items.get(&LabelName::new("label_name")).unwrap(), + label_set.items.get(&label_name!("label_name")).unwrap(), &LabelValue::new("value") ); } @@ -258,18 +259,18 @@ mod tests { fn it_should_allow_updating_a_label_value() { let mut label_set = LabelSet::default(); - label_set.upsert(LabelName::new("label_name"), LabelValue::new("old value")); - label_set.upsert(LabelName::new("label_name"), LabelValue::new("new value")); + label_set.upsert(label_name!("label_name"), LabelValue::new("old value")); + label_set.upsert(label_name!("label_name"), LabelValue::new("new value")); assert_eq!( - label_set.items.get(&LabelName::new("label_name")).unwrap(), + label_set.items.get(&label_name!("label_name")).unwrap(), &LabelValue::new("new value") ); } #[test] fn it_should_allow_serializing_to_json_as_an_array_of_label_objects() { - let label_set = LabelSet::from((LabelName::new("label_name"), LabelValue::new("label value"))); + let label_set = LabelSet::from((label_name!("label_name"), LabelValue::new("label value"))); let json = serde_json::to_string(&label_set).unwrap(); @@ -307,13 +308,13 @@ mod tests { assert_eq!( label_set, - LabelSet::from((LabelName::new("label_name"), LabelValue::new("label value"))) + LabelSet::from((label_name!("label_name"), LabelValue::new("label value"))) ); } #[test] fn it_should_allow_serializing_to_prometheus_format() { - let label_set = LabelSet::from((LabelName::new("label_name"), LabelValue::new("label value"))); + let label_set = LabelSet::from((label_name!("label_name"), LabelValue::new("label value"))); assert_eq!(label_set.to_prometheus(), r#"{label_name="label value"}"#); } @@ -321,8 +322,8 @@ mod tests { #[test] fn it_should_alphabetically_order_labels_in_prometheus_format() { let label_set = LabelSet::from([ - (LabelName::new("b_label_name"), LabelValue::new("b label value")), - (LabelName::new("a_label_name"), LabelValue::new("a label value")), + (label_name!("b_label_name"), LabelValue::new("b label value")), + (label_name!("a_label_name"), LabelValue::new("a label value")), ]); assert_eq!( @@ -333,7 +334,7 @@ mod tests { #[test] fn it_should_allow_displaying() { - let label_set = LabelSet::from((LabelName::new("label_name"), LabelValue::new("label value"))); + let label_set = LabelSet::from((label_name!("label_name"), LabelValue::new("label value"))); assert_eq!(label_set.to_string(), r#"{label_name="label value"}"#); } diff --git a/packages/metrics/src/label/value.rs b/packages/metrics/src/label/value.rs index 528a0e2ab..ffdbce333 100644 --- a/packages/metrics/src/label/value.rs +++ b/packages/metrics/src/label/value.rs @@ -25,6 +25,12 @@ impl PrometheusSerializable for LabelValue { } } +impl From for LabelValue { + fn from(value: String) -> Self { + Self(value) + } +} + #[cfg(test)] mod tests { use crate::label::value::LabelValue; diff --git a/packages/metrics/src/lib.rs b/packages/metrics/src/lib.rs index fd677b891..95d70bf6c 100644 --- a/packages/metrics/src/lib.rs +++ b/packages/metrics/src/lib.rs @@ -8,6 +8,8 @@ pub mod sample; pub mod sample_collection; pub mod unit; +pub const METRICS_TARGET: &str = "METRICS"; + #[cfg(test)] mod tests { /// It removes leading and trailing whitespace from each line, and empty lines. diff --git a/packages/metrics/src/metric/mod.rs b/packages/metrics/src/metric/mod.rs index edea035bb..ecce90f18 100644 --- a/packages/metrics/src/metric/mod.rs +++ b/packages/metrics/src/metric/mod.rs @@ -86,12 +86,13 @@ mod tests { mod for_generic_metrics { use super::super::*; use crate::gauge::Gauge; - use crate::label::{LabelName, LabelValue}; + use crate::label::LabelValue; use crate::sample::Sample; + use crate::{label_name, metric_name}; #[test] fn it_should_be_empty_when_it_does_not_have_any_sample() { - let name = MetricName::new("test_metric"); + let name = metric_name!("test_metric"); let samples = SampleCollection::::default(); @@ -103,11 +104,11 @@ mod tests { fn counter_metric_with_one_sample() -> Metric { let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); - let name = MetricName::new("test_metric"); + let name = metric_name!("test_metric"); - let label_set: LabelSet = [(LabelName::new("server_binding_protocol"), LabelValue::new("http"))].into(); + let label_set: LabelSet = [(label_name!("server_binding_protocol"), LabelValue::new("http"))].into(); - let samples = SampleCollection::new(vec![Sample::new(Counter::new(1), time, label_set.clone())]); + let samples = SampleCollection::new(vec![Sample::new(Counter::new(1), time, label_set.clone())]).unwrap(); Metric::::new(name.clone(), samples) } @@ -119,7 +120,7 @@ mod tests { #[test] fn it_should_return_zero_number_of_samples_for_an_empty_metric() { - let name = MetricName::new("test_metric"); + let name = metric_name!("test_metric"); let samples = SampleCollection::::default(); @@ -132,12 +133,13 @@ mod tests { mod for_counter_metrics { use super::super::*; use crate::counter::Counter; - use crate::label::{LabelName, LabelValue}; + use crate::label::LabelValue; use crate::sample::Sample; + use crate::{label_name, metric_name}; #[test] fn it_should_be_created_from_its_name_and_a_collection_of_samples() { - let name = MetricName::new("test_metric"); + let name = metric_name!("test_metric"); let samples = SampleCollection::::default(); @@ -148,11 +150,11 @@ mod tests { fn it_should_allow_incrementing_a_sample() { let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); - let name = MetricName::new("test_metric"); + let name = metric_name!("test_metric"); - let label_set: LabelSet = [(LabelName::new("server_binding_protocol"), LabelValue::new("http"))].into(); + let label_set: LabelSet = [(label_name!("server_binding_protocol"), LabelValue::new("http"))].into(); - let samples = SampleCollection::new(vec![Sample::new(Counter::new(1), time, label_set.clone())]); + let samples = SampleCollection::new(vec![Sample::new(Counter::new(1), time, label_set.clone())]).unwrap(); let metric = Metric::::new(name.clone(), samples); @@ -165,12 +167,13 @@ mod tests { use super::super::*; use crate::gauge::Gauge; - use crate::label::{LabelName, LabelValue}; + use crate::label::LabelValue; use crate::sample::Sample; + use crate::{label_name, metric_name}; #[test] fn it_should_be_created_from_its_name_and_a_collection_of_samples() { - let name = MetricName::new("test_metric"); + let name = metric_name!("test_metric"); let samples = SampleCollection::::default(); @@ -181,11 +184,11 @@ mod tests { fn it_should_allow_setting_a_sample() { let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); - let name = MetricName::new("test_metric"); + let name = metric_name!("test_metric"); - let label_set: LabelSet = [(LabelName::new("server_binding_protocol"), LabelValue::new("http"))].into(); + let label_set: LabelSet = [(label_name!("server_binding_protocol"), LabelValue::new("http"))].into(); - let samples = SampleCollection::new(vec![Sample::new(Gauge::new(1.0), time, label_set.clone())]); + let samples = SampleCollection::new(vec![Sample::new(Gauge::new(1.0), time, label_set.clone())]).unwrap(); let metric = Metric::::new(name.clone(), samples); diff --git a/packages/metrics/src/metric/name.rs b/packages/metrics/src/metric/name.rs index c904f34d3..09c8c9e6d 100644 --- a/packages/metrics/src/metric/name.rs +++ b/packages/metrics/src/metric/name.rs @@ -14,11 +14,7 @@ impl MetricName { /// Panics if the provided name is empty. #[must_use] pub fn new(name: &str) -> Self { - assert!( - !name.is_empty(), - "Metric name cannot be empty. It must have at least one character." - ); - + assert!(!name.is_empty(), "Metric name cannot be empty."); Self(name.to_owned()) } } @@ -50,41 +46,50 @@ impl PrometheusSerializable for MetricName { } } +#[macro_export] +macro_rules! metric_name { + ("") => { + compile_error!("Metric name cannot be empty"); + }; + ($name:literal) => { + $crate::metric::name::MetricName::new($name) + }; + ($name:ident) => { + $crate::metric::name::MetricName::new($name) + }; +} + #[cfg(test)] mod tests { mod serialization_of_metric_name_to_prometheus { - use rstest::rstest; - - use crate::metric::MetricName; + use crate::metric::name::MetricName; use crate::prometheus::PrometheusSerializable; - #[rstest] - #[case("valid name", "valid_name", "valid_name")] - #[case("leading underscore", "_leading_underscore", "_leading_underscore")] - #[case("leading colon", ":leading_colon", ":leading_colon")] - #[case("leading lowercase", "v123", "v123")] - #[case("leading uppercase", "V123", "V123")] - fn valid_names_in_prometheus(#[case] case: &str, #[case] input: &str, #[case] output: &str) { - assert_eq!(MetricName::new(input).to_prometheus(), output, "{case} failed: {input:?}"); + #[test] + fn valid_names_in_prometheus() { + assert_eq!(metric_name!("valid_name").to_prometheus(), "valid_name"); + assert_eq!(metric_name!("_leading_underscore").to_prometheus(), "_leading_underscore"); + assert_eq!(metric_name!(":leading_colon").to_prometheus(), ":leading_colon"); + assert_eq!(metric_name!("v123").to_prometheus(), "v123"); // leading lowercase + assert_eq!(metric_name!("V123").to_prometheus(), "V123"); // leading lowercase } - #[rstest] - #[case("invalid start 1", "9invalid_start", "_invalid_start")] - #[case("invalid start 2", "@test", "_test")] - #[case("invalid dash", "invalid-char", "invalid_char")] - #[case("invalid spaces", "spaces are bad", "spaces_are_bad")] - #[case("invalid special chars", "a!b@c#d$e%f^g&h*i(j)", "a_b_c_d_e_f_g_h_i_j_")] - #[case("invalid slash", "my:metric/version", "my:metric_version")] - #[case("all invalid characters", "!@#$%^&*()", "__________")] - #[case("non_ascii_characters", "ñaca©", "_aca_")] - fn names_that_need_changes_in_prometheus(#[case] case: &str, #[case] input: &str, #[case] output: &str) { - assert_eq!(MetricName::new(input).to_prometheus(), output, "{case} failed: {input:?}"); + #[test] + fn names_that_need_changes_in_prometheus() { + assert_eq!(metric_name!("9invalid_start").to_prometheus(), "_invalid_start"); + assert_eq!(metric_name!("@test").to_prometheus(), "_test"); + assert_eq!(metric_name!("invalid-char").to_prometheus(), "invalid_char"); + assert_eq!(metric_name!("spaces are bad").to_prometheus(), "spaces_are_bad"); + assert_eq!(metric_name!("a!b@c#d$e%f^g&h*i(j)").to_prometheus(), "a_b_c_d_e_f_g_h_i_j_"); + assert_eq!(metric_name!("my:metric/version").to_prometheus(), "my:metric_version"); + assert_eq!(metric_name!("!@#$%^&*()").to_prometheus(), "__________"); + assert_eq!(metric_name!("ñaca©").to_prometheus(), "_aca_"); } #[test] - #[should_panic(expected = "Metric name cannot be empty. It must have at least one character.")] + #[should_panic(expected = "Metric name cannot be empty.")] fn empty_name() { let _name = MetricName::new(""); } diff --git a/packages/metrics/src/metric_collection.rs b/packages/metrics/src/metric_collection.rs index d0ed96554..6a2a7735d 100644 --- a/packages/metrics/src/metric_collection.rs +++ b/packages/metrics/src/metric_collection.rs @@ -12,14 +12,12 @@ use super::prometheus::PrometheusSerializable; use crate::metric::description::MetricDescription; use crate::sample_collection::SampleCollection; use crate::unit::Unit; +use crate::METRICS_TARGET; -// todo: serialize in a deterministic order. For example: +// code-review: serialize in a deterministic order? For example: // - First the counter metrics ordered by name. // - Then the gauge metrics ordered by name. -/// Use this type only when behind a lock that guarantees thread-safety. -/// Otherwise, there could be race conditions that lead to duplicate metric -/// names in different metric types. #[derive(Debug, Clone, Default, PartialEq)] pub struct MetricCollection { counters: MetricKindCollection, @@ -27,21 +25,23 @@ pub struct MetricCollection { } impl MetricCollection { - /// # Panics + /// # Errors /// - /// Panics if there are duplicate metric names across counters and gauges. - #[must_use] - pub fn new(counters: MetricKindCollection, gauges: MetricKindCollection) -> Self { + /// Returns an error if there are duplicate metric names across counters and + /// gauges. + pub fn new(counters: MetricKindCollection, gauges: MetricKindCollection) -> Result { // Check for name collisions across metric types let counter_names: HashSet<_> = counters.names().collect(); let gauge_names: HashSet<_> = gauges.names().collect(); - assert!( - counter_names.is_disjoint(&gauge_names), - "Metric names must be unique across counters and gauges" - ); + if !counter_names.is_disjoint(&gauge_names) { + return Err(Error::MetricNameCollisionInConstructor { + counter_names: counter_names.iter().map(std::string::ToString::to_string).collect(), + gauge_names: gauge_names.iter().map(std::string::ToString::to_string).collect(), + }); + } - Self { counters, gauges } + Ok(Self { counters, gauges }) } /// Merges another `MetricCollection` into this one. @@ -49,7 +49,7 @@ impl MetricCollection { /// # Errors /// /// Returns an error if a metric name already exists in the current collection. - pub fn merge(&mut self, other: &Self) -> Result<(), MergeError> { + pub fn merge(&mut self, other: &Self) -> Result<(), Error> { self.counters.merge(&other.counters)?; self.gauges.merge(&other.gauges)?; Ok(()) @@ -57,25 +57,40 @@ impl MetricCollection { // Counter-specific methods - pub fn describe_counter(&mut self, name: &MetricName, _opt_unit: Option, _opt_description: Option) { + pub fn describe_counter(&mut self, name: &MetricName, opt_unit: Option, opt_description: Option) { + tracing::info!(target: METRICS_TARGET, type = "counter", name = name.to_string(), unit = ?opt_unit, description = ?opt_description); self.counters.ensure_metric_exists(name); } #[must_use] - pub fn get_counter_value(&self, name: &MetricName, label_set: &LabelSet) -> Counter { + pub fn contains_counter(&self, name: &MetricName) -> bool { + self.counters.metrics.contains_key(name) + } + + #[must_use] + pub fn get_counter_value(&self, name: &MetricName, label_set: &LabelSet) -> Option { self.counters.get_value(name, label_set) } - /// # Panics + /// # Errors /// - /// Panics if a gauge with the same name already exists. - pub fn increase_counter(&mut self, name: &MetricName, label_set: &LabelSet, time: DurationSinceUnixEpoch) { - assert!( - !self.gauges.metrics.contains_key(name), - "Cannot create counter with name '{name}': a gauge with this name already exists", - ); + /// Return an error if a metrics of a different type with the same name + /// already exists. + pub fn increase_counter( + &mut self, + name: &MetricName, + label_set: &LabelSet, + time: DurationSinceUnixEpoch, + ) -> Result<(), Error> { + if self.gauges.metrics.contains_key(name) { + return Err(Error::MetricNameCollisionAdding { + metric_name: name.clone(), + }); + } self.counters.increment(name, label_set, time); + + Ok(()) } pub fn ensure_counter_exists(&mut self, name: &MetricName) { @@ -84,25 +99,41 @@ impl MetricCollection { // Gauge-specific methods - pub fn describe_gauge(&mut self, name: &MetricName, _opt_unit: Option, _opt_description: Option) { + pub fn describe_gauge(&mut self, name: &MetricName, opt_unit: Option, opt_description: Option) { + tracing::info!(target: METRICS_TARGET, type = "gauge", name = name.to_string(), unit = ?opt_unit, description = ?opt_description); self.gauges.ensure_metric_exists(name); } #[must_use] - pub fn get_gauge_value(&self, name: &MetricName, label_set: &LabelSet) -> Gauge { + pub fn contains_gauge(&self, name: &MetricName) -> bool { + self.gauges.metrics.contains_key(name) + } + + #[must_use] + pub fn get_gauge_value(&self, name: &MetricName, label_set: &LabelSet) -> Option { self.gauges.get_value(name, label_set) } - /// # Panics + /// # Errors /// - /// Panics if a counter with the same name already exists. - pub fn set_gauge(&mut self, name: &MetricName, label_set: &LabelSet, value: f64, time: DurationSinceUnixEpoch) { - assert!( - !self.counters.metrics.contains_key(name), - "Cannot create gauge with name '{name}': a counter with this name already exists" - ); + /// Return an error if a metrics of a different type with the same name + /// already exists. + pub fn set_gauge( + &mut self, + name: &MetricName, + label_set: &LabelSet, + value: f64, + time: DurationSinceUnixEpoch, + ) -> Result<(), Error> { + if self.counters.metrics.contains_key(name) { + return Err(Error::MetricNameCollisionAdding { + metric_name: name.clone(), + }); + } self.gauges.set(name, label_set, value, time); + + Ok(()) } pub fn ensure_gauge_exists(&mut self, name: &MetricName) { @@ -111,9 +142,21 @@ impl MetricCollection { } #[derive(thiserror::Error, Debug, Clone)] -pub enum MergeError { +pub enum Error { + #[error("Metric names must be unique across all metrics types.")] + MetricNameCollisionInConstructor { + counter_names: Vec, + gauge_names: Vec, + }, + + #[error("Found duplicate metric name in list. Metric names must be unique across all metrics types.")] + DuplicateMetricNameInList { metric_name: MetricName }, + #[error("Cannot merge metric '{metric_name}': it already exists in the current collection")] - MetricNameAlreadyExists { metric_name: MetricName }, + MetricNameCollisionInMerge { metric_name: MetricName }, + + #[error("Cannot create metric with name '{metric_name}': another metric with this name already exists")] + MetricNameCollisionAdding { metric_name: MetricName }, } /// Implements serialization for `MetricCollection`. @@ -167,10 +210,12 @@ impl<'de> Deserialize<'de> for MetricCollection { } } - Ok(MetricCollection::new( - MetricKindCollection::new(counters), - MetricKindCollection::new(gauges), - )) + let counters = MetricKindCollection::new(counters).map_err(serde::de::Error::custom)?; + let gauges = MetricKindCollection::new(gauges).map_err(serde::de::Error::custom)?; + + let metric_collection = MetricCollection::new(counters, gauges).map_err(serde::de::Error::custom)?; + + Ok(metric_collection) } } @@ -201,20 +246,21 @@ pub struct MetricKindCollection { impl MetricKindCollection { /// Creates a new `MetricKindCollection` from a vector of metrics /// - /// # Panics + /// # Errors /// - /// Panics if duplicate metric names are found - #[must_use] - pub fn new(metrics: Vec>) -> Self { + /// Returns an error if duplicate metric names are passed. + pub fn new(metrics: Vec>) -> Result { let mut map = HashMap::with_capacity(metrics.len()); for metric in metrics { - assert!( - map.insert(metric.name().clone(), metric).is_none(), - "Duplicate MetricName found in MetricKindCollection" - ); + let metric_name = metric.name().clone(); + + if let Some(_old_metric) = map.insert(metric.name().clone(), metric) { + return Err(Error::DuplicateMetricNameInList { metric_name }); + } } - Self { metrics: map } + + Ok(Self { metrics: map }) } /// Returns an iterator over all metric names in this collection. @@ -222,10 +268,18 @@ impl MetricKindCollection { self.metrics.keys() } + /// # Panics + /// + /// It should not panic as long as empty sample collections are allowed. pub fn ensure_metric_exists(&mut self, name: &MetricName) { if !self.metrics.contains_key(name) { - self.metrics - .insert(name.clone(), Metric::new(name.clone(), SampleCollection::new(vec![]))); + self.metrics.insert( + name.clone(), + Metric::new( + name.clone(), + SampleCollection::new(vec![]).expect("Empty sample collection creation should not fail"), + ), + ); } } } @@ -236,11 +290,11 @@ impl MetricKindCollection { /// # Errors /// /// Returns an error if a metric name already exists in the current collection. - pub fn merge(&mut self, other: &Self) -> Result<(), MergeError> { + pub fn merge(&mut self, other: &Self) -> Result<(), Error> { // Check for name collisions for metric_name in other.metrics.keys() { if self.metrics.contains_key(metric_name) { - return Err(MergeError::MetricNameAlreadyExists { + return Err(Error::MetricNameCollisionInMerge { metric_name: metric_name.clone(), }); } @@ -248,7 +302,7 @@ impl MetricKindCollection { for (metric_name, metric) in &other.metrics { if self.metrics.insert(metric_name.clone(), metric.clone()).is_some() { - return Err(MergeError::MetricNameAlreadyExists { + return Err(Error::MetricNameCollisionInMerge { metric_name: metric_name.clone(), }); } @@ -275,11 +329,11 @@ impl MetricKindCollection { } #[must_use] - pub fn get_value(&self, name: &MetricName, label_set: &LabelSet) -> Counter { + pub fn get_value(&self, name: &MetricName, label_set: &LabelSet) -> Option { self.metrics .get(name) .and_then(|metric| metric.get_sample_data(label_set)) - .map_or(Counter::default(), |sample| sample.value().clone()) + .map(|sample| sample.value().clone()) } } @@ -300,11 +354,11 @@ impl MetricKindCollection { } #[must_use] - pub fn get_value(&self, name: &MetricName, label_set: &LabelSet) -> Gauge { + pub fn get_value(&self, name: &MetricName, label_set: &LabelSet) -> Option { self.metrics .get(name) .and_then(|metric| metric.get_sample_data(label_set)) - .map_or(Gauge::default(), |sample| sample.value().clone()) + .map(|sample| sample.value().clone()) } } @@ -314,9 +368,10 @@ mod tests { use pretty_assertions::assert_eq; use super::*; - use crate::label::{LabelName, LabelValue}; + use crate::label::LabelValue; use crate::sample::Sample; use crate::tests::{format_prometheus_output, sort_lines}; + use crate::{label_name, metric_name}; /// Fixture for testing serialization and deserialization of `MetricCollection`. /// @@ -347,22 +402,25 @@ mod tests { let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); let label_set_1: LabelSet = [ - (LabelName::new("server_binding_protocol"), LabelValue::new("http")), - (LabelName::new("server_binding_ip"), LabelValue::new("0.0.0.0")), - (LabelName::new("server_binding_port"), LabelValue::new("7070")), + (label_name!("server_binding_protocol"), LabelValue::new("http")), + (label_name!("server_binding_ip"), LabelValue::new("0.0.0.0")), + (label_name!("server_binding_port"), LabelValue::new("7070")), ] .into(); MetricCollection::new( MetricKindCollection::new(vec![Metric::new( - MetricName::new("http_tracker_core_announce_requests_received_total"), - SampleCollection::new(vec![Sample::new(Counter::new(1), time, label_set_1.clone())]), - )]), + metric_name!("http_tracker_core_announce_requests_received_total"), + SampleCollection::new(vec![Sample::new(Counter::new(1), time, label_set_1.clone())]).unwrap(), + )]) + .unwrap(), MetricKindCollection::new(vec![Metric::new( - MetricName::new("udp_tracker_server_performance_avg_announce_processing_time_ns"), - SampleCollection::new(vec![Sample::new(Gauge::new(1.0), time, label_set_1.clone())]), - )]), + metric_name!("udp_tracker_server_performance_avg_announce_processing_time_ns"), + SampleCollection::new(vec![Sample::new(Gauge::new(1.0), time, label_set_1.clone())]).unwrap(), + )]) + .unwrap(), ) + .unwrap() } fn json() -> String { @@ -432,52 +490,52 @@ mod tests { } #[test] - #[should_panic(expected = "Metric names must be unique across counters and gauges")] fn it_should_not_allow_duplicate_names_across_types() { - let counter = MetricKindCollection::new(vec![Metric::new( - MetricName::new("test_metric"), - SampleCollection::new(vec![]), - )]); - - let gauge = MetricKindCollection::new(vec![Metric::new( - MetricName::new("test_metric"), - SampleCollection::new(vec![]), - )]); + let counters = + MetricKindCollection::new(vec![Metric::new(metric_name!("test_metric"), SampleCollection::default())]).unwrap(); + let gauges = + MetricKindCollection::new(vec![Metric::new(metric_name!("test_metric"), SampleCollection::default())]).unwrap(); - let _unused = MetricCollection::new(counter, gauge); + assert!(MetricCollection::new(counters, gauges).is_err()); } #[test] - #[should_panic(expected = "Cannot create gauge with name 'test_metric': a counter with this name already exists")] fn it_should_not_allow_creating_a_gauge_with_the_same_name_as_a_counter() { let mut collection = MetricCollection::default(); let label_set = LabelSet::default(); let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); // First create a counter - collection.increase_counter(&MetricName::new("test_metric"), &label_set, time); + collection + .increase_counter(&metric_name!("test_metric"), &label_set, time) + .unwrap(); - // Then try to create a gauge with the same name - this should panic - collection.set_gauge(&MetricName::new("test_metric"), &label_set, 1.0, time); + // Then try to create a gauge with the same name + let result = collection.set_gauge(&metric_name!("test_metric"), &label_set, 1.0, time); + + assert!(result.is_err()); } #[test] - #[should_panic(expected = "Cannot create counter with name 'test_metric': a gauge with this name already exists")] fn it_should_not_allow_creating_a_counter_with_the_same_name_as_a_gauge() { let mut collection = MetricCollection::default(); let label_set = LabelSet::default(); let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); // First set the gauge - collection.set_gauge(&MetricName::new("test_metric"), &label_set, 1.0, time); + collection + .set_gauge(&metric_name!("test_metric"), &label_set, 1.0, time) + .unwrap(); + + // Then try to create a counter with the same name + let result = collection.increase_counter(&metric_name!("test_metric"), &label_set, time); - // Then try to create a counter with the same name - this should panic - collection.increase_counter(&MetricName::new("test_metric"), &label_set, time); + assert!(result.is_err()); } #[test] fn it_should_allow_serializing_to_json() { - // todo: this test does work with metric with multiple samples becuase + // todo: this test does work with metric with multiple samples because // samples are not serialized in the same order as they are created. let (metric_collection, expected_json, _expected_prometheus) = MetricCollectionFixture::default().deconstruct(); @@ -513,29 +571,32 @@ mod tests { let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); let label_set_1: LabelSet = [ - (LabelName::new("server_binding_protocol"), LabelValue::new("http")), - (LabelName::new("server_binding_ip"), LabelValue::new("0.0.0.0")), - (LabelName::new("server_binding_port"), LabelValue::new("7070")), + (label_name!("server_binding_protocol"), LabelValue::new("http")), + (label_name!("server_binding_ip"), LabelValue::new("0.0.0.0")), + (label_name!("server_binding_port"), LabelValue::new("7070")), ] .into(); let label_set_2: LabelSet = [ - (LabelName::new("server_binding_protocol"), LabelValue::new("http")), - (LabelName::new("server_binding_ip"), LabelValue::new("0.0.0.0")), - (LabelName::new("server_binding_port"), LabelValue::new("7171")), + (label_name!("server_binding_protocol"), LabelValue::new("http")), + (label_name!("server_binding_ip"), LabelValue::new("0.0.0.0")), + (label_name!("server_binding_port"), LabelValue::new("7171")), ] .into(); let metric_collection = MetricCollection::new( MetricKindCollection::new(vec![Metric::new( - MetricName::new("http_tracker_core_announce_requests_received_total"), + metric_name!("http_tracker_core_announce_requests_received_total"), SampleCollection::new(vec![ Sample::new(Counter::new(1), time, label_set_1.clone()), Sample::new(Counter::new(2), time, label_set_2.clone()), - ]), - )]), - MetricKindCollection::new(vec![]), - ); + ]) + .unwrap(), + )]) + .unwrap(), + MetricKindCollection::default(), + ) + .unwrap(); let prometheus_output = metric_collection.to_prometheus(); @@ -554,13 +615,13 @@ mod tests { #[test] fn it_should_exclude_metrics_without_samples_from_prometheus_format() { - let mut counters = MetricKindCollection::new(vec![]); - let mut gauges = MetricKindCollection::new(vec![]); + let mut counters = MetricKindCollection::default(); + let mut gauges = MetricKindCollection::default(); - counters.ensure_metric_exists(&MetricName::new("test_counter")); - gauges.ensure_metric_exists(&MetricName::new("test_gauge")); + counters.ensure_metric_exists(&metric_name!("test_counter")); + gauges.ensure_metric_exists(&metric_name!("test_gauge")); - let metric_collection = MetricCollection::new(counters, gauges); + let metric_collection = MetricCollection::new(counters, gauges).unwrap(); let prometheus_output = metric_collection.to_prometheus(); @@ -572,94 +633,95 @@ mod tests { use pretty_assertions::assert_eq; use super::*; - use crate::label::{LabelName, LabelValue}; + use crate::label::LabelValue; use crate::sample::Sample; #[test] fn it_should_increase_a_preexistent_counter() { let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); - let label_set: LabelSet = (LabelName::new("label_name"), LabelValue::new("value")).into(); + let label_set: LabelSet = (label_name!("label_name"), LabelValue::new("value")).into(); let mut metric_collection = MetricCollection::new( MetricKindCollection::new(vec![Metric::new( - MetricName::new("test_counter"), - SampleCollection::new(vec![Sample::new(Counter::new(0), time, label_set.clone())]), - )]), - MetricKindCollection::new(vec![]), - ); + metric_name!("test_counter"), + SampleCollection::new(vec![Sample::new(Counter::new(0), time, label_set.clone())]).unwrap(), + )]) + .unwrap(), + MetricKindCollection::default(), + ) + .unwrap(); - metric_collection.increase_counter(&MetricName::new("test_counter"), &label_set, time); - metric_collection.increase_counter(&MetricName::new("test_counter"), &label_set, time); + metric_collection + .increase_counter(&metric_name!("test_counter"), &label_set, time) + .unwrap(); + metric_collection + .increase_counter(&metric_name!("test_counter"), &label_set, time) + .unwrap(); assert_eq!( - metric_collection.get_counter_value(&MetricName::new("test_counter"), &label_set), - Counter::new(2) + metric_collection.get_counter_value(&metric_name!("test_counter"), &label_set), + Some(Counter::new(2)) ); } #[test] fn it_should_automatically_create_a_counter_when_increasing_if_it_does_not_exist() { let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); - let label_set: LabelSet = (LabelName::new("label_name"), LabelValue::new("value")).into(); + let label_set: LabelSet = (label_name!("label_name"), LabelValue::new("value")).into(); let mut metric_collection = - MetricCollection::new(MetricKindCollection::new(vec![]), MetricKindCollection::new(vec![])); + MetricCollection::new(MetricKindCollection::default(), MetricKindCollection::default()).unwrap(); - metric_collection.increase_counter(&MetricName::new("test_counter"), &label_set, time); - metric_collection.increase_counter(&MetricName::new("test_counter"), &label_set, time); + metric_collection + .increase_counter(&metric_name!("test_counter"), &label_set, time) + .unwrap(); + metric_collection + .increase_counter(&metric_name!("test_counter"), &label_set, time) + .unwrap(); assert_eq!( - metric_collection.get_counter_value(&MetricName::new("test_counter"), &label_set), - Counter::new(2) + metric_collection.get_counter_value(&metric_name!("test_counter"), &label_set), + Some(Counter::new(2)) ); } #[test] fn it_should_allow_making_sure_a_counter_exists_without_increasing_it() { - let label_set: LabelSet = (LabelName::new("label_name"), LabelValue::new("value")).into(); - let mut metric_collection = - MetricCollection::new(MetricKindCollection::new(vec![]), MetricKindCollection::new(vec![])); + MetricCollection::new(MetricKindCollection::default(), MetricKindCollection::default()).unwrap(); - metric_collection.ensure_counter_exists(&MetricName::new("test_counter")); + metric_collection.ensure_counter_exists(&metric_name!("test_counter")); - assert_eq!( - metric_collection.get_counter_value(&MetricName::new("test_counter"), &label_set), - Counter::default() - ); + assert!(metric_collection.contains_counter(&metric_name!("test_counter"))); } #[test] fn it_should_allow_describing_a_counter_before_using_it() { - let label_set: LabelSet = (LabelName::new("label_name"), LabelValue::new("value")).into(); - let mut metric_collection = - MetricCollection::new(MetricKindCollection::new(vec![]), MetricKindCollection::new(vec![])); + MetricCollection::new(MetricKindCollection::default(), MetricKindCollection::default()).unwrap(); - metric_collection.describe_counter(&MetricName::new("test_counter"), None, None); + metric_collection.describe_counter(&metric_name!("test_counter"), None, None); - assert_eq!( - metric_collection.get_counter_value(&MetricName::new("test_counter"), &label_set), - Counter::default() - ); + assert!(metric_collection.contains_counter(&metric_name!("test_counter"))); } #[test] - #[should_panic(expected = "Duplicate MetricName found in MetricKindCollection")] fn it_should_not_allow_duplicate_metric_names_when_instantiating() { let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); - let label_set: LabelSet = (LabelName::new("label_name"), LabelValue::new("value")).into(); + let label_set: LabelSet = (label_name!("label_name"), LabelValue::new("value")).into(); - let _unused = MetricKindCollection::new(vec![ + let result = MetricKindCollection::new(vec![ Metric::new( - MetricName::new("test_counter"), - SampleCollection::new(vec![Sample::new(Counter::new(0), time, label_set.clone())]), + metric_name!("test_counter"), + SampleCollection::new(vec![Sample::new(Counter::new(0), time, label_set.clone())]).unwrap(), ), Metric::new( - MetricName::new("test_counter"), - SampleCollection::new(vec![Sample::new(Counter::new(0), time, label_set.clone())]), + metric_name!("test_counter"), + SampleCollection::new(vec![Sample::new(Counter::new(0), time, label_set.clone())]).unwrap(), ), ]); + + assert!(result.is_err()); } } @@ -668,92 +730,89 @@ mod tests { use pretty_assertions::assert_eq; use super::*; - use crate::label::{LabelName, LabelValue}; + use crate::label::LabelValue; use crate::sample::Sample; #[test] fn it_should_set_a_preexistent_gauge() { let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); - let label_set: LabelSet = (LabelName::new("label_name"), LabelValue::new("value")).into(); + let label_set: LabelSet = (label_name!("label_name"), LabelValue::new("value")).into(); let mut metric_collection = MetricCollection::new( - MetricKindCollection::new(vec![]), + MetricKindCollection::default(), MetricKindCollection::new(vec![Metric::new( - MetricName::new("test_gauge"), - SampleCollection::new(vec![Sample::new(Gauge::new(0.0), time, label_set.clone())]), - )]), - ); + metric_name!("test_gauge"), + SampleCollection::new(vec![Sample::new(Gauge::new(0.0), time, label_set.clone())]).unwrap(), + )]) + .unwrap(), + ) + .unwrap(); - metric_collection.set_gauge(&MetricName::new("test_gauge"), &label_set, 1.0, time); + metric_collection + .set_gauge(&metric_name!("test_gauge"), &label_set, 1.0, time) + .unwrap(); assert_eq!( - metric_collection.get_gauge_value(&MetricName::new("test_gauge"), &label_set), - Gauge::new(1.0) + metric_collection.get_gauge_value(&metric_name!("test_gauge"), &label_set), + Some(Gauge::new(1.0)) ); } #[test] fn it_should_automatically_create_a_gauge_when_setting_if_it_does_not_exist() { let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); - let label_set: LabelSet = (LabelName::new("label_name"), LabelValue::new("value")).into(); + let label_set: LabelSet = (label_name!("label_name"), LabelValue::new("value")).into(); let mut metric_collection = - MetricCollection::new(MetricKindCollection::new(vec![]), MetricKindCollection::new(vec![])); + MetricCollection::new(MetricKindCollection::default(), MetricKindCollection::default()).unwrap(); - metric_collection.set_gauge(&MetricName::new("test_gauge"), &label_set, 1.0, time); + metric_collection + .set_gauge(&metric_name!("test_gauge"), &label_set, 1.0, time) + .unwrap(); assert_eq!( - metric_collection.get_gauge_value(&MetricName::new("test_gauge"), &label_set), - Gauge::new(1.0) + metric_collection.get_gauge_value(&metric_name!("test_gauge"), &label_set), + Some(Gauge::new(1.0)) ); } #[test] - fn it_should_allow_making_sure_a_gauge_exists_without_increasing_it() { - let label_set: LabelSet = (LabelName::new("label_name"), LabelValue::new("value")).into(); - + fn it_should_allow_making_sure_a_gauge_exists_without_setting_it() { let mut metric_collection = - MetricCollection::new(MetricKindCollection::new(vec![]), MetricKindCollection::new(vec![])); + MetricCollection::new(MetricKindCollection::default(), MetricKindCollection::default()).unwrap(); - metric_collection.ensure_gauge_exists(&MetricName::new("test_gauge")); + metric_collection.ensure_gauge_exists(&metric_name!("test_gauge")); - assert_eq!( - metric_collection.get_gauge_value(&MetricName::new("test_gauge"), &label_set), - Gauge::default() - ); + assert!(metric_collection.contains_gauge(&metric_name!("test_gauge"))); } #[test] fn it_should_allow_describing_a_gauge_before_using_it() { - let label_set: LabelSet = (LabelName::new("label_name"), LabelValue::new("value")).into(); - let mut metric_collection = - MetricCollection::new(MetricKindCollection::new(vec![]), MetricKindCollection::new(vec![])); + MetricCollection::new(MetricKindCollection::default(), MetricKindCollection::default()).unwrap(); - metric_collection.describe_gauge(&MetricName::new("test_gauge"), None, None); + metric_collection.describe_gauge(&metric_name!("test_gauge"), None, None); - assert_eq!( - metric_collection.get_gauge_value(&MetricName::new("test_gauge"), &label_set), - Gauge::default() - ); + assert!(metric_collection.contains_gauge(&metric_name!("test_gauge"))); } #[test] - #[should_panic(expected = "Duplicate MetricName found in MetricKindCollection")] fn it_should_not_allow_duplicate_metric_names_when_instantiating() { let time = DurationSinceUnixEpoch::from_secs(1_743_552_000); - let label_set: LabelSet = (LabelName::new("label_name"), LabelValue::new("value")).into(); + let label_set: LabelSet = (label_name!("label_name"), LabelValue::new("value")).into(); - let _unused = MetricKindCollection::new(vec![ + let result = MetricKindCollection::new(vec![ Metric::new( - MetricName::new("test_gauge"), - SampleCollection::new(vec![Sample::new(Gauge::new(0.0), time, label_set.clone())]), + metric_name!("test_gauge"), + SampleCollection::new(vec![Sample::new(Gauge::new(0.0), time, label_set.clone())]).unwrap(), ), Metric::new( - MetricName::new("test_gauge"), - SampleCollection::new(vec![Sample::new(Gauge::new(0.0), time, label_set.clone())]), + metric_name!("test_gauge"), + SampleCollection::new(vec![Sample::new(Gauge::new(0.0), time, label_set.clone())]).unwrap(), ), ]); + + assert!(result.is_err()); } } } diff --git a/packages/metrics/src/sample_collection.rs b/packages/metrics/src/sample_collection.rs index 436a4bc7d..49c839673 100644 --- a/packages/metrics/src/sample_collection.rs +++ b/packages/metrics/src/sample_collection.rs @@ -1,8 +1,8 @@ use std::collections::hash_map::Iter; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::fmt::Write as _; -use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; use torrust_tracker_primitives::DurationSinceUnixEpoch; use super::counter::Counter; @@ -18,23 +18,28 @@ pub struct SampleCollection { } impl SampleCollection { - /// # Panics + /// Creates a new `MetricKindCollection` from a vector of metrics /// - /// Panics if there are duplicate `LabelSets` in the provided samples. - #[must_use] - pub fn new(samples: Vec>) -> Self { + /// # Errors + /// + /// Returns an error if there are duplicate `LabelSets` in the provided + /// samples. + pub fn new(samples: Vec>) -> Result { let mut map: HashMap> = HashMap::with_capacity(samples.len()); for sample in samples { let (label_set, sample_data): (LabelSet, Measurement) = sample.into(); - assert!( - map.insert(label_set, sample_data).is_none(), - "Duplicate LabelSet found in SampleCollection" - ); + let label_set_clone = label_set.clone(); + + if let Some(_old_measurement) = map.insert(label_set, sample_data) { + return Err(Error::DuplicateLabelSetInList { + label_set: label_set_clone, + }); + } } - Self { samples: map } + Ok(Self { samples: map }) } #[must_use] @@ -59,6 +64,12 @@ impl SampleCollection { } } +#[derive(thiserror::Error, Debug, Clone)] +pub enum Error { + #[error("Found duplicate label set in list. Label set must be unique in a SampleCollection.")] + DuplicateLabelSetInList { label_set: LabelSet }, +} + impl SampleCollection { pub fn increment(&mut self, label_set: &LabelSet, time: DurationSinceUnixEpoch) { let sample = self @@ -104,20 +115,11 @@ where where D: Deserializer<'de>, { - // First deserialize into a temporary Vec let samples = Vec::>::deserialize(deserializer)?; - // Check for duplicate label sets - let mut seen_labels = HashSet::new(); + let sample_collection = SampleCollection::new(samples).map_err(serde::de::Error::custom)?; - for sample in &samples { - if !seen_labels.insert(sample.labels()) { - return Err(de::Error::custom(format!("Duplicate label set found: {}", sample.labels()))); - } - } - - // Convert to HashMap-based storage - Ok(SampleCollection::new(samples)) + Ok(sample_collection) } } @@ -149,14 +151,15 @@ mod tests { } #[test] - #[should_panic(expected = "Duplicate LabelSet found in SampleCollection")] fn it_should_fail_trying_to_create_a_sample_collection_with_duplicate_label_sets() { let samples = vec![ Sample::new(Counter::default(), sample_update_time(), LabelSet::default()), Sample::new(Counter::default(), sample_update_time(), LabelSet::default()), ]; - let _unused = SampleCollection::new(samples); + let result = SampleCollection::new(samples); + + assert!(result.is_err()); } #[test] @@ -165,7 +168,7 @@ mod tests { let sample = Sample::new(Counter::default(), sample_update_time(), label_set.clone()); - let collection = SampleCollection::new(vec![sample.clone()]); + let collection = SampleCollection::new(vec![sample.clone()]).unwrap(); let retrieved = collection.get(&label_set); @@ -180,7 +183,7 @@ mod tests { let sample_1 = Sample::new(Counter::new(1), sample_update_time(), label_set_1.clone()); let sample_2 = Sample::new(Counter::new(2), sample_update_time(), label_set_2.clone()); - let collection = SampleCollection::new(vec![sample_1.clone(), sample_2.clone()]); + let collection = SampleCollection::new(vec![sample_1.clone(), sample_2.clone()]).unwrap(); let retrieved = collection.get(&label_set_1); assert_eq!(retrieved.unwrap(), sample_1.measurement()); @@ -192,7 +195,7 @@ mod tests { #[test] fn it_should_return_the_number_of_samples_in_the_collection() { let samples = vec![Sample::new(Counter::default(), sample_update_time(), LabelSet::default())]; - let collection = SampleCollection::new(samples); + let collection = SampleCollection::new(samples).unwrap(); assert_eq!(collection.len(), 1); } @@ -208,14 +211,14 @@ mod tests { assert!(empty.is_empty()); let samples = vec![Sample::new(Counter::default(), sample_update_time(), LabelSet::default())]; - let collection = SampleCollection::new(samples); + let collection = SampleCollection::new(samples).unwrap(); assert!(!collection.is_empty()); } #[test] fn it_should_be_serializable_and_deserializable_for_json_format() { let sample = Sample::new(Counter::default(), sample_update_time(), LabelSet::default()); - let collection = SampleCollection::new(vec![sample]); + let collection = SampleCollection::new(vec![sample]).unwrap(); let serialized = serde_json::to_string(&collection).unwrap(); let deserialized: SampleCollection = serde_json::from_str(&serialized).unwrap(); @@ -240,7 +243,7 @@ mod tests { #[test] fn it_should_be_exportable_to_prometheus_format_when_empty() { let sample = Sample::new(Counter::default(), sample_update_time(), LabelSet::default()); - let collection = SampleCollection::new(vec![sample]); + let collection = SampleCollection::new(vec![sample]).unwrap(); let prometheus_output = collection.to_prometheus(); @@ -255,7 +258,7 @@ mod tests { LabelSet::from(vec![("labe_name_1", "label value value 1")]), ); - let collection = SampleCollection::new(vec![sample]); + let collection = SampleCollection::new(vec![sample]).unwrap(); let prometheus_output = collection.to_prometheus(); diff --git a/packages/metrics/src/unit.rs b/packages/metrics/src/unit.rs index b98e6836d..f7a528bed 100644 --- a/packages/metrics/src/unit.rs +++ b/packages/metrics/src/unit.rs @@ -4,6 +4,7 @@ //! The `Unit` enum is used to specify the unit of measurement for metrics. //! //! They were copied from the `metrics` crate, to allow future compatibility. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum Unit { Count, Percent, diff --git a/packages/udp-tracker-core/src/event/mod.rs b/packages/udp-tracker-core/src/event/mod.rs index 6cb43e5a1..ddcba7792 100644 --- a/packages/udp-tracker-core/src/event/mod.rs +++ b/packages/udp-tracker-core/src/event/mod.rs @@ -1,6 +1,7 @@ use std::net::SocketAddr; -use torrust_tracker_metrics::label::{LabelName, LabelSet, LabelValue}; +use torrust_tracker_metrics::label::{LabelSet, LabelValue}; +use torrust_tracker_metrics::label_name; use torrust_tracker_primitives::service_binding::ServiceBinding; pub mod sender; @@ -43,15 +44,15 @@ impl From for LabelSet { fn from(connection_context: ConnectionContext) -> Self { LabelSet::from([ ( - LabelName::new("server_binding_protocol"), + label_name!("server_binding_protocol"), LabelValue::new(&connection_context.server_service_binding.protocol().to_string()), ), ( - LabelName::new("server_binding_ip"), + label_name!("server_binding_ip"), LabelValue::new(&connection_context.server_service_binding.bind_address().ip().to_string()), ), ( - LabelName::new("server_binding_port"), + label_name!("server_binding_port"), LabelValue::new(&connection_context.server_service_binding.bind_address().port().to_string()), ), ]) diff --git a/packages/udp-tracker-core/src/statistics/event/handler.rs b/packages/udp-tracker-core/src/statistics/event/handler.rs index 59c382755..2680c442f 100644 --- a/packages/udp-tracker-core/src/statistics/event/handler.rs +++ b/packages/udp-tracker-core/src/statistics/event/handler.rs @@ -1,5 +1,5 @@ -use torrust_tracker_metrics::label::{LabelName, LabelSet, LabelValue}; -use torrust_tracker_metrics::metric::MetricName; +use torrust_tracker_metrics::label::{LabelSet, LabelValue}; +use torrust_tracker_metrics::{label_name, metric_name}; use torrust_tracker_primitives::DurationSinceUnixEpoch; use crate::event::Event; @@ -26,11 +26,15 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura // Extendable metrics let mut label_set = LabelSet::from(context); - label_set.upsert(LabelName::new("request_kind"), LabelValue::new("connect")); - - stats_repository - .increase_counter(&MetricName::new(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now) - .await; + label_set.upsert(label_name!("request_kind"), LabelValue::new("connect")); + + match stats_repository + .increase_counter(&metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now) + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to increase the counter: {}", err), + }; } Event::UdpAnnounce { context } => { // Global fixed metrics @@ -47,11 +51,15 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura // Extendable metrics let mut label_set = LabelSet::from(context); - label_set.upsert(LabelName::new("request_kind"), LabelValue::new("announce")); - - stats_repository - .increase_counter(&MetricName::new(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now) - .await; + label_set.upsert(label_name!("request_kind"), LabelValue::new("announce")); + + match stats_repository + .increase_counter(&metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now) + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to increase the counter: {}", err), + }; } Event::UdpScrape { context } => { // Global fixed metrics @@ -68,11 +76,15 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura // Extendable metrics let mut label_set = LabelSet::from(context); - label_set.upsert(LabelName::new("request_kind"), LabelValue::new("scrape")); - - stats_repository - .increase_counter(&MetricName::new(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now) - .await; + label_set.upsert(label_name!("request_kind"), LabelValue::new("scrape")); + + match stats_repository + .increase_counter(&metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now) + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to increase the counter: {}", err), + }; } } diff --git a/packages/udp-tracker-core/src/statistics/keeper.rs b/packages/udp-tracker-core/src/statistics/keeper.rs index 16ea51aac..d72dcb260 100644 --- a/packages/udp-tracker-core/src/statistics/keeper.rs +++ b/packages/udp-tracker-core/src/statistics/keeper.rs @@ -3,6 +3,7 @@ use tokio::sync::broadcast::Receiver; use super::event::listener::dispatch_events; use super::repository::Repository; use crate::event::Event; +use crate::UDP_TRACKER_LOG_TARGET; /// The service responsible for keeping tracker metrics (listening to statistics events and handle them). /// @@ -29,7 +30,13 @@ impl Keeper { pub fn run_event_listener(&mut self, receiver: Receiver) { let stats_repository = self.repository.clone(); - tokio::spawn(async move { dispatch_events(receiver, stats_repository).await }); + tracing::info!(target: UDP_TRACKER_LOG_TARGET, "Starting UDP tracker core event listener"); + + tokio::spawn(async move { + dispatch_events(receiver, stats_repository).await; + + tracing::info!(target: UDP_TRACKER_LOG_TARGET, "UDP tracker core event listener finished"); + }); } } diff --git a/packages/udp-tracker-core/src/statistics/metrics.rs b/packages/udp-tracker-core/src/statistics/metrics.rs index 23cec8036..94aa7d08f 100644 --- a/packages/udp-tracker-core/src/statistics/metrics.rs +++ b/packages/udp-tracker-core/src/statistics/metrics.rs @@ -1,7 +1,7 @@ use serde::Serialize; use torrust_tracker_metrics::label::LabelSet; use torrust_tracker_metrics::metric::MetricName; -use torrust_tracker_metrics::metric_collection::MetricCollection; +use torrust_tracker_metrics::metric_collection::{Error, MetricCollection}; use torrust_tracker_primitives::DurationSinceUnixEpoch; /// Metrics collected by the tracker. @@ -37,11 +37,30 @@ pub struct Metrics { } impl Metrics { - pub fn increase_counter(&mut self, metric_name: &MetricName, labels: &LabelSet, now: DurationSinceUnixEpoch) { - self.metric_collection.increase_counter(metric_name, labels, now); + /// # Errors + /// + /// This function returns an error if the metric does not exist and it + /// cannot be created. + pub fn increase_counter( + &mut self, + metric_name: &MetricName, + labels: &LabelSet, + now: DurationSinceUnixEpoch, + ) -> Result<(), Error> { + self.metric_collection.increase_counter(metric_name, labels, now) } - pub fn set_gauge(&mut self, metric_name: &MetricName, labels: &LabelSet, value: f64, now: DurationSinceUnixEpoch) { - self.metric_collection.set_gauge(metric_name, labels, value, now); + /// # Errors + /// + /// This function returns an error if the metric does not exist and it + /// cannot be created. + pub fn set_gauge( + &mut self, + metric_name: &MetricName, + labels: &LabelSet, + value: f64, + now: DurationSinceUnixEpoch, + ) -> Result<(), Error> { + self.metric_collection.set_gauge(metric_name, labels, value, now) } } diff --git a/packages/udp-tracker-core/src/statistics/mod.rs b/packages/udp-tracker-core/src/statistics/mod.rs index bc4d8d836..40a30f51b 100644 --- a/packages/udp-tracker-core/src/statistics/mod.rs +++ b/packages/udp-tracker-core/src/statistics/mod.rs @@ -7,7 +7,7 @@ pub mod setup; use metrics::Metrics; use torrust_tracker_metrics::metric::description::MetricDescription; -use torrust_tracker_metrics::metric::MetricName; +use torrust_tracker_metrics::metric_name; use torrust_tracker_metrics::unit::Unit; const UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL: &str = "udp_tracker_core_requests_received_total"; @@ -17,7 +17,7 @@ pub fn describe_metrics() -> Metrics { let mut metrics = Metrics::default(); metrics.metric_collection.describe_counter( - &MetricName::new(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), + &metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), Some(Unit::Count), Some(MetricDescription::new("Total number of UDP requests received")), ); diff --git a/packages/udp-tracker-core/src/statistics/repository.rs b/packages/udp-tracker-core/src/statistics/repository.rs index 49c91c751..c68fa14f7 100644 --- a/packages/udp-tracker-core/src/statistics/repository.rs +++ b/packages/udp-tracker-core/src/statistics/repository.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use tokio::sync::{RwLock, RwLockReadGuard}; use torrust_tracker_metrics::label::LabelSet; use torrust_tracker_metrics::metric::MetricName; +use torrust_tracker_metrics::metric_collection::Error; use torrust_tracker_primitives::DurationSinceUnixEpoch; use super::describe_metrics; @@ -68,9 +69,22 @@ impl Repository { drop(stats_lock); } - pub async fn increase_counter(&self, metric_name: &MetricName, labels: &LabelSet, now: DurationSinceUnixEpoch) { + /// # Errors + /// + /// This function will return an error if the metric collection fails to + /// increase the counter. + pub async fn increase_counter( + &self, + metric_name: &MetricName, + labels: &LabelSet, + now: DurationSinceUnixEpoch, + ) -> Result<(), Error> { let mut stats_lock = self.stats.write().await; - stats_lock.increase_counter(metric_name, labels, now); + + let result = stats_lock.increase_counter(metric_name, labels, now); + drop(stats_lock); + + result } } diff --git a/packages/udp-tracker-server/src/event/mod.rs b/packages/udp-tracker-server/src/event/mod.rs index 316e1a414..a1770acc0 100644 --- a/packages/udp-tracker-server/src/event/mod.rs +++ b/packages/udp-tracker-server/src/event/mod.rs @@ -2,7 +2,8 @@ use std::fmt; use std::net::SocketAddr; use std::time::Duration; -use torrust_tracker_metrics::label::{LabelName, LabelSet, LabelValue}; +use torrust_tracker_metrics::label::{LabelSet, LabelValue}; +use torrust_tracker_metrics::label_name; use torrust_tracker_primitives::service_binding::ServiceBinding; pub mod sender; @@ -30,6 +31,7 @@ pub enum Event { }, UdpError { context: ConnectionContext, + kind: Option, }, } @@ -94,15 +96,15 @@ impl From for LabelSet { fn from(connection_context: ConnectionContext) -> Self { LabelSet::from([ ( - LabelName::new("server_binding_protocol"), + label_name!("server_binding_protocol"), LabelValue::new(&connection_context.server_service_binding.protocol().to_string()), ), ( - LabelName::new("server_binding_ip"), + label_name!("server_binding_ip"), LabelValue::new(&connection_context.server_service_binding.bind_address().ip().to_string()), ), ( - LabelName::new("server_binding_port"), + label_name!("server_binding_port"), LabelValue::new(&connection_context.server_service_binding.bind_address().port().to_string()), ), ]) diff --git a/packages/udp-tracker-server/src/handlers/error.rs b/packages/udp-tracker-server/src/handlers/error.rs index 6a1bce51c..9d9ee8b1d 100644 --- a/packages/udp-tracker-server/src/handlers/error.rs +++ b/packages/udp-tracker-server/src/handlers/error.rs @@ -64,6 +64,7 @@ pub async fn handle_error( udp_server_stats_event_sender .send_event(Event::UdpError { context: ConnectionContext::new(client_socket_addr, server_service_binding), + kind: req_kind, }) .await; } diff --git a/packages/udp-tracker-server/src/statistics/event/handler.rs b/packages/udp-tracker-server/src/statistics/event/handler.rs index 4c10576c0..1e1502339 100644 --- a/packages/udp-tracker-server/src/statistics/event/handler.rs +++ b/packages/udp-tracker-server/src/statistics/event/handler.rs @@ -1,5 +1,5 @@ -use torrust_tracker_metrics::label::{LabelName, LabelSet, LabelValue}; -use torrust_tracker_metrics::metric::MetricName; +use torrust_tracker_metrics::label::{LabelSet, LabelValue}; +use torrust_tracker_metrics::{label_name, metric_name}; use torrust_tracker_primitives::DurationSinceUnixEpoch; use crate::event::{Event, UdpRequestKind, UdpResponseKind}; @@ -23,26 +23,34 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura stats_repository.increase_udp_requests_aborted().await; // Extendable metrics - stats_repository + match stats_repository .increase_counter( - &MetricName::new(UDP_TRACKER_SERVER_REQUESTS_ABORTED_TOTAL), + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_ABORTED_TOTAL), &LabelSet::from(context), now, ) - .await; + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to increase the counter: {}", err), + }; } Event::UdpRequestBanned { context } => { // Global fixed metrics stats_repository.increase_udp_requests_banned().await; // Extendable metrics - stats_repository + match stats_repository .increase_counter( - &MetricName::new(UDP_TRACKER_SERVER_REQUESTS_BANNED_TOTAL), + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_BANNED_TOTAL), &LabelSet::from(context), now, ) - .await; + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to increase the counter: {}", err), + }; } Event::UdpRequestReceived { context } => { // Global fixed metrics @@ -56,13 +64,17 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura } // Extendable metrics - stats_repository + match stats_repository .increase_counter( - &MetricName::new(UDP_TRACKER_SERVER_REQUESTS_RECEIVED_TOTAL), + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_RECEIVED_TOTAL), &LabelSet::from(context), now, ) - .await; + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to increase the counter: {}", err), + }; } Event::UdpRequestAccepted { context, kind } => { // Global fixed metrics @@ -97,11 +109,15 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura let mut label_set = LabelSet::from(context); - label_set.upsert(LabelName::new("kind"), LabelValue::new(&kind.to_string())); + label_set.upsert(label_name!("request_kind"), LabelValue::new(&kind.to_string())); - stats_repository - .increase_counter(&MetricName::new(UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL), &label_set, now) - .await; + match stats_repository + .increase_counter(&metric_name!(UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL), &label_set, now) + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to increase the counter: {}", err), + }; } Event::UdpResponseSent { context, @@ -128,16 +144,20 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura // Extendable metrics let mut label_set = LabelSet::from(context.clone()); - label_set.upsert(LabelName::new("request_kind"), LabelValue::new(&req_kind.to_string())); + label_set.upsert(label_name!("request_kind"), LabelValue::new(&req_kind.to_string())); - stats_repository + match stats_repository .set_gauge( - &MetricName::new(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS), + &metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS), &label_set, new_avg, now, ) - .await; + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to set gauge: {}", err), + } (LabelValue::new("ok"), LabelValue::new(&UdpRequestKind::Connect.to_string())) } @@ -149,16 +169,20 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura // Extendable metrics let mut label_set = LabelSet::from(context.clone()); - label_set.upsert(LabelName::new("request_kind"), LabelValue::new(&req_kind.to_string())); + label_set.upsert(label_name!("request_kind"), LabelValue::new(&req_kind.to_string())); - stats_repository + match stats_repository .set_gauge( - &MetricName::new(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS), + &metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS), &label_set, new_avg, now, ) - .await; + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to set gauge: {}", err), + } (LabelValue::new("ok"), LabelValue::new(&UdpRequestKind::Announce.to_string())) } @@ -170,16 +194,20 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura // Extendable metrics let mut label_set = LabelSet::from(context.clone()); - label_set.upsert(LabelName::new("request_kind"), LabelValue::new(&req_kind.to_string())); + label_set.upsert(label_name!("request_kind"), LabelValue::new(&req_kind.to_string())); - stats_repository + match stats_repository .set_gauge( - &MetricName::new(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS), + &metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS), &label_set, new_avg, now, ) - .await; + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to set gauge: {}", err), + } (LabelValue::new("ok"), LabelValue::new(&UdpRequestKind::Scrape.to_string())) } @@ -192,15 +220,19 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura let mut label_set = LabelSet::from(context); if result_label_value == LabelValue::new("ok") { - label_set.upsert(LabelName::new("request_kind"), kind_label_value); + label_set.upsert(label_name!("request_kind"), kind_label_value); } - label_set.upsert(LabelName::new("result"), result_label_value); - - stats_repository - .increase_counter(&MetricName::new(UDP_TRACKER_SERVER_RESPONSES_SENT_TOTAL), &label_set, now) - .await; + label_set.upsert(label_name!("result"), result_label_value); + + match stats_repository + .increase_counter(&metric_name!(UDP_TRACKER_SERVER_RESPONSES_SENT_TOTAL), &label_set, now) + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to increase the counter: {}", err), + }; } - Event::UdpError { context } => { + Event::UdpError { context, kind } => { // Global fixed metrics match context.client_socket_addr().ip() { std::net::IpAddr::V4(_) => { @@ -212,13 +244,20 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura } // Extendable metrics - stats_repository - .increase_counter( - &MetricName::new(UDP_TRACKER_SERVER_ERRORS_TOTAL), - &LabelSet::from(context), - now, - ) - .await; + + let mut label_set = LabelSet::from(context); + + if let Some(kind) = kind { + label_set.upsert(label_name!("request_kind"), kind.to_string().into()); + } + + match stats_repository + .increase_counter(&metric_name!(UDP_TRACKER_SERVER_ERRORS_TOTAL), &label_set, now) + .await + { + Ok(()) => {} + Err(err) => tracing::error!("Failed to increase the counter: {}", err), + }; } } @@ -478,6 +517,7 @@ mod tests { ) .unwrap(), ), + kind: None, }, &stats_repository, CurrentClock::now(), @@ -609,6 +649,7 @@ mod tests { ) .unwrap(), ), + kind: None, }, &stats_repository, CurrentClock::now(), diff --git a/packages/udp-tracker-server/src/statistics/keeper.rs b/packages/udp-tracker-server/src/statistics/keeper.rs index 62216ce88..c200b4cdf 100644 --- a/packages/udp-tracker-server/src/statistics/keeper.rs +++ b/packages/udp-tracker-server/src/statistics/keeper.rs @@ -1,3 +1,4 @@ +use bittorrent_udp_tracker_core::UDP_TRACKER_LOG_TARGET; use tokio::sync::broadcast::Receiver; use super::event::listener::dispatch_events; @@ -29,7 +30,13 @@ impl Keeper { pub fn run_event_listener(&mut self, receiver: Receiver) { let stats_repository = self.repository.clone(); - tokio::spawn(async move { dispatch_events(receiver, stats_repository).await }); + tracing::info!(target: UDP_TRACKER_LOG_TARGET, "Starting UDP tracker server event listener"); + + tokio::spawn(async move { + dispatch_events(receiver, stats_repository).await; + + tracing::info!(target: UDP_TRACKER_LOG_TARGET, "UDP tracker core server listener finished"); + }); } } diff --git a/packages/udp-tracker-server/src/statistics/metrics.rs b/packages/udp-tracker-server/src/statistics/metrics.rs index 4fe07e7da..7b18f6418 100644 --- a/packages/udp-tracker-server/src/statistics/metrics.rs +++ b/packages/udp-tracker-server/src/statistics/metrics.rs @@ -1,7 +1,7 @@ use serde::Serialize; use torrust_tracker_metrics::label::LabelSet; use torrust_tracker_metrics::metric::MetricName; -use torrust_tracker_metrics::metric_collection::MetricCollection; +use torrust_tracker_metrics::metric_collection::{Error, MetricCollection}; use torrust_tracker_primitives::DurationSinceUnixEpoch; /// Metrics collected by the UDP tracker server. @@ -69,11 +69,28 @@ pub struct Metrics { } impl Metrics { - pub fn increase_counter(&mut self, metric_name: &MetricName, labels: &LabelSet, now: DurationSinceUnixEpoch) { - self.metric_collection.increase_counter(metric_name, labels, now); + /// # Errors + /// + /// Returns an error if the metric does not exist and it cannot be created. + pub fn increase_counter( + &mut self, + metric_name: &MetricName, + labels: &LabelSet, + now: DurationSinceUnixEpoch, + ) -> Result<(), Error> { + self.metric_collection.increase_counter(metric_name, labels, now) } - pub fn set_gauge(&mut self, metric_name: &MetricName, labels: &LabelSet, value: f64, now: DurationSinceUnixEpoch) { - self.metric_collection.set_gauge(metric_name, labels, value, now); + /// # Errors + /// + /// Returns an error if the metric does not exist and it cannot be created. + pub fn set_gauge( + &mut self, + metric_name: &MetricName, + labels: &LabelSet, + value: f64, + now: DurationSinceUnixEpoch, + ) -> Result<(), Error> { + self.metric_collection.set_gauge(metric_name, labels, value, now) } } diff --git a/packages/udp-tracker-server/src/statistics/mod.rs b/packages/udp-tracker-server/src/statistics/mod.rs index 523cd4bac..4eea13224 100644 --- a/packages/udp-tracker-server/src/statistics/mod.rs +++ b/packages/udp-tracker-server/src/statistics/mod.rs @@ -7,7 +7,7 @@ pub mod setup; use metrics::Metrics; use torrust_tracker_metrics::metric::description::MetricDescription; -use torrust_tracker_metrics::metric::MetricName; +use torrust_tracker_metrics::metric_name; use torrust_tracker_metrics::unit::Unit; const UDP_TRACKER_SERVER_REQUESTS_ABORTED_TOTAL: &str = "udp_tracker_server_requests_aborted_total"; @@ -23,43 +23,43 @@ pub fn describe_metrics() -> Metrics { let mut metrics = Metrics::default(); metrics.metric_collection.describe_counter( - &MetricName::new(UDP_TRACKER_SERVER_REQUESTS_ABORTED_TOTAL), + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_ABORTED_TOTAL), Some(Unit::Count), Some(MetricDescription::new("Total number of UDP requests aborted")), ); metrics.metric_collection.describe_counter( - &MetricName::new(UDP_TRACKER_SERVER_REQUESTS_BANNED_TOTAL), + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_BANNED_TOTAL), Some(Unit::Count), Some(MetricDescription::new("Total number of UDP requests banned")), ); metrics.metric_collection.describe_counter( - &MetricName::new(UDP_TRACKER_SERVER_REQUESTS_RECEIVED_TOTAL), + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_RECEIVED_TOTAL), Some(Unit::Count), Some(MetricDescription::new("Total number of UDP requests received")), ); metrics.metric_collection.describe_counter( - &MetricName::new(UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL), + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL), Some(Unit::Count), Some(MetricDescription::new("Total number of UDP requests accepted")), ); metrics.metric_collection.describe_counter( - &MetricName::new(UDP_TRACKER_SERVER_RESPONSES_SENT_TOTAL), + &metric_name!(UDP_TRACKER_SERVER_RESPONSES_SENT_TOTAL), Some(Unit::Count), Some(MetricDescription::new("Total number of UDP responses sent")), ); metrics.metric_collection.describe_counter( - &MetricName::new(UDP_TRACKER_SERVER_ERRORS_TOTAL), + &metric_name!(UDP_TRACKER_SERVER_ERRORS_TOTAL), Some(Unit::Count), Some(MetricDescription::new("Total number of errors processing UDP requests")), ); metrics.metric_collection.describe_gauge( - &MetricName::new(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS), + &metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS), Some(Unit::Nanoseconds), Some(MetricDescription::new( "Average time to process a UDP connect request in nanoseconds", diff --git a/packages/udp-tracker-server/src/statistics/repository.rs b/packages/udp-tracker-server/src/statistics/repository.rs index c33c1231c..1a1db89c7 100644 --- a/packages/udp-tracker-server/src/statistics/repository.rs +++ b/packages/udp-tracker-server/src/statistics/repository.rs @@ -4,6 +4,7 @@ use std::time::Duration; use tokio::sync::{RwLock, RwLockReadGuard}; use torrust_tracker_metrics::label::LabelSet; use torrust_tracker_metrics::metric::MetricName; +use torrust_tracker_metrics::metric_collection::Error; use torrust_tracker_primitives::DurationSinceUnixEpoch; use super::describe_metrics; @@ -181,15 +182,42 @@ impl Repository { drop(stats_lock); } - pub async fn increase_counter(&self, metric_name: &MetricName, labels: &LabelSet, now: DurationSinceUnixEpoch) { + /// # Errors + /// + /// This function will return an error if the metric collection fails to + /// increase the counter. + pub async fn increase_counter( + &self, + metric_name: &MetricName, + labels: &LabelSet, + now: DurationSinceUnixEpoch, + ) -> Result<(), Error> { let mut stats_lock = self.stats.write().await; - stats_lock.increase_counter(metric_name, labels, now); + + let result = stats_lock.increase_counter(metric_name, labels, now); + drop(stats_lock); + + result } - pub async fn set_gauge(&self, metric_name: &MetricName, labels: &LabelSet, value: f64, now: DurationSinceUnixEpoch) { + /// # Errors + /// + /// This function will return an error if the metric collection fails to + /// increase the counter. + pub async fn set_gauge( + &self, + metric_name: &MetricName, + labels: &LabelSet, + value: f64, + now: DurationSinceUnixEpoch, + ) -> Result<(), Error> { let mut stats_lock = self.stats.write().await; - stats_lock.set_gauge(metric_name, labels, value, now); + + let result = stats_lock.set_gauge(metric_name, labels, value, now); + drop(stats_lock); + + result } }