From 64be8472feffb8217cdd7ce505510b4d234d5981 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Wed, 11 Jun 2025 14:16:02 +0100 Subject: [PATCH 1/8] feat: [#1446] add aggregate function sum to metric collection It allows sum metric samples matching a given criteria. The criteria is a label set. Sample values are added if they contain all the label name/value pairs specified in the criteria. For example, given these metric's samples in Prometheus export text format: ``` udp_tracker_server_requests_accepted_total{request_kind="scrape",server_binding_address_type="plain",server_binding_ip="0.0.0.0",server_binding_port="6969",server_binding_protocol="udp"} 213118 udp_tracker_server_requests_accepted_total{request_kind="announce",server_binding_address_type="plain",server_binding_ip="0.0.0.0",server_binding_port="6969",server_binding_protocol="udp"} 16460553 udp_tracker_server_requests_accepted_total{request_kind="connect",server_binding_address_type="plain",server_binding_ip="0.0.0.0",server_binding_port="6868",server_binding_protocol="udp"} 617 udp_tracker_server_requests_accepted_total{request_kind="connect",server_binding_address_type="plain",server_binding_ip="0.0.0.0",server_binding_port="6969",server_binding_protocol="udp"} 17148137 ``` And the criteria: it should contain the label `request_kind` with the value `connect`. It should return: 617 + 17148137 = 17148754 --- .../src/statistics/metrics.rs | 2 +- packages/metrics/src/aggregate.rs | 28 ++ packages/metrics/src/counter.rs | 18 ++ packages/metrics/src/gauge.rs | 11 + packages/metrics/src/label/set.rs | 21 ++ packages/metrics/src/lib.rs | 1 + packages/metrics/src/metric/aggregate/mod.rs | 1 + packages/metrics/src/metric/aggregate/sum.rs | 283 ++++++++++++++++++ packages/metrics/src/metric/mod.rs | 1 + .../src/metric_collection/aggregate.rs | 112 +++++++ .../mod.rs} | 22 +- .../src/statistics/metrics.rs | 2 +- .../tracker-core/src/statistics/metrics.rs | 2 +- .../src/statistics/metrics.rs | 2 +- .../src/statistics/metrics.rs | 2 +- 15 files changed, 493 insertions(+), 15 deletions(-) create mode 100644 packages/metrics/src/aggregate.rs create mode 100644 packages/metrics/src/metric/aggregate/mod.rs create mode 100644 packages/metrics/src/metric/aggregate/sum.rs create mode 100644 packages/metrics/src/metric_collection/aggregate.rs rename packages/metrics/src/{metric_collection.rs => metric_collection/mod.rs} (98%) diff --git a/packages/http-tracker-core/src/statistics/metrics.rs b/packages/http-tracker-core/src/statistics/metrics.rs index bf053b04e..650194d43 100644 --- a/packages/http-tracker-core/src/statistics/metrics.rs +++ b/packages/http-tracker-core/src/statistics/metrics.rs @@ -33,7 +33,7 @@ impl Metrics { labels: &LabelSet, now: DurationSinceUnixEpoch, ) -> Result<(), Error> { - self.metric_collection.increase_counter(metric_name, labels, now) + self.metric_collection.increment_counter(metric_name, labels, now) } /// # Errors diff --git a/packages/metrics/src/aggregate.rs b/packages/metrics/src/aggregate.rs new file mode 100644 index 000000000..875360cd9 --- /dev/null +++ b/packages/metrics/src/aggregate.rs @@ -0,0 +1,28 @@ +use derive_more::Display; + +#[derive(Debug, Display, Clone, Copy, PartialEq)] +pub struct AggregateValue(f64); + +impl AggregateValue { + #[must_use] + pub fn new(value: f64) -> Self { + Self(value) + } + + #[must_use] + pub fn value(&self) -> f64 { + self.0 + } +} + +impl From for AggregateValue { + fn from(value: f64) -> Self { + Self(value) + } +} + +impl From for f64 { + fn from(value: AggregateValue) -> Self { + value.0 + } +} diff --git a/packages/metrics/src/counter.rs b/packages/metrics/src/counter.rs index ac6d21836..3148ab4c3 100644 --- a/packages/metrics/src/counter.rs +++ b/packages/metrics/src/counter.rs @@ -17,6 +17,11 @@ impl Counter { self.0 } + #[must_use] + pub fn primitive(&self) -> u64 { + self.value() + } + pub fn increment(&mut self, value: u64) { self.0 += value; } @@ -26,12 +31,25 @@ impl Counter { } } +impl From for Counter { + fn from(value: u32) -> Self { + Self(u64::from(value)) + } +} + impl From for Counter { fn from(value: u64) -> Self { Self(value) } } +impl From for Counter { + fn from(value: i32) -> Self { + #[allow(clippy::cast_sign_loss)] + Self(value as u64) + } +} + impl From for u64 { fn from(counter: Counter) -> Self { counter.value() diff --git a/packages/metrics/src/gauge.rs b/packages/metrics/src/gauge.rs index 3f6089955..a2ef8135f 100644 --- a/packages/metrics/src/gauge.rs +++ b/packages/metrics/src/gauge.rs @@ -17,6 +17,11 @@ impl Gauge { self.0 } + #[must_use] + pub fn primitive(&self) -> f64 { + self.value() + } + pub fn set(&mut self, value: f64) { self.0 = value; } @@ -30,6 +35,12 @@ impl Gauge { } } +impl From for Gauge { + fn from(value: f32) -> Self { + Self(f64::from(value)) + } +} + impl From for Gauge { fn from(value: f64) -> Self { Self(value) diff --git a/packages/metrics/src/label/set.rs b/packages/metrics/src/label/set.rs index cab457f42..673f330c1 100644 --- a/packages/metrics/src/label/set.rs +++ b/packages/metrics/src/label/set.rs @@ -1,3 +1,4 @@ +use std::collections::btree_map::Iter; use std::collections::BTreeMap; use std::fmt::Display; @@ -12,6 +13,11 @@ pub struct LabelSet { } impl LabelSet { + #[must_use] + pub fn empty() -> Self { + Self { items: BTreeMap::new() } + } + /// Insert a new label pair or update the value of an existing label. pub fn upsert(&mut self, key: LabelName, value: LabelValue) { self.items.insert(key, value); @@ -20,6 +26,21 @@ impl LabelSet { pub fn is_empty(&self) -> bool { self.items.is_empty() } + + pub fn contains_pair(&self, name: &LabelName, value: &LabelValue) -> bool { + match self.items.get(name) { + Some(existing_value) => existing_value == value, + None => false, + } + } + + pub fn matches(&self, criteria: &LabelSet) -> bool { + criteria.iter().all(|(key, value)| self.contains_pair(key, value)) + } + + pub fn iter(&self) -> Iter<'_, LabelName, LabelValue> { + self.items.iter() + } } impl Display for LabelSet { diff --git a/packages/metrics/src/lib.rs b/packages/metrics/src/lib.rs index 997cd3c8c..c53e9dd02 100644 --- a/packages/metrics/src/lib.rs +++ b/packages/metrics/src/lib.rs @@ -1,3 +1,4 @@ +pub mod aggregate; pub mod counter; pub mod gauge; pub mod label; diff --git a/packages/metrics/src/metric/aggregate/mod.rs b/packages/metrics/src/metric/aggregate/mod.rs new file mode 100644 index 000000000..dce785d95 --- /dev/null +++ b/packages/metrics/src/metric/aggregate/mod.rs @@ -0,0 +1 @@ +pub mod sum; diff --git a/packages/metrics/src/metric/aggregate/sum.rs b/packages/metrics/src/metric/aggregate/sum.rs new file mode 100644 index 000000000..f08ea7d55 --- /dev/null +++ b/packages/metrics/src/metric/aggregate/sum.rs @@ -0,0 +1,283 @@ +use crate::aggregate::AggregateValue; +use crate::counter::Counter; +use crate::gauge::Gauge; +use crate::label::LabelSet; +use crate::metric::Metric; + +pub trait Sum { + fn sum(&self, label_set_criteria: &LabelSet) -> AggregateValue; +} + +impl Sum for Metric { + #[allow(clippy::cast_precision_loss)] + fn sum(&self, label_set_criteria: &LabelSet) -> AggregateValue { + let sum: f64 = self + .sample_collection + .iter() + .filter(|(label_set, _measurement)| label_set.matches(label_set_criteria)) + .map(|(_label_set, measurement)| measurement.value().primitive() as f64) + .sum(); + + sum.into() + } +} + +impl Sum for Metric { + fn sum(&self, label_set_criteria: &LabelSet) -> AggregateValue { + let sum: f64 = self + .sample_collection + .iter() + .filter(|(label_set, _measurement)| label_set.matches(label_set_criteria)) + .map(|(_label_set, measurement)| measurement.value().primitive()) + .sum(); + + sum.into() + } +} + +#[cfg(test)] +mod tests { + + use torrust_tracker_primitives::DurationSinceUnixEpoch; + + use crate::aggregate::AggregateValue; + use crate::counter::Counter; + use crate::gauge::Gauge; + use crate::label::LabelSet; + use crate::metric::aggregate::sum::Sum; + use crate::metric::{Metric, MetricName}; + use crate::metric_name; + use crate::sample::Sample; + use crate::sample_collection::SampleCollection; + + struct MetricBuilder { + sample_time: DurationSinceUnixEpoch, + name: MetricName, + samples: Vec>, + } + + impl Default for MetricBuilder { + fn default() -> Self { + Self { + sample_time: DurationSinceUnixEpoch::from_secs(1_743_552_000), + name: metric_name!("test_metric"), + samples: vec![], + } + } + } + + impl MetricBuilder { + fn with_sample(mut self, value: T, label_set: &LabelSet) -> Self { + let sample = Sample::new(value, self.sample_time, label_set.clone()); + self.samples.push(sample); + self + } + + fn build(self) -> Metric { + Metric::new( + self.name, + None, + None, + SampleCollection::new(self.samples).expect("invalid samples"), + ) + } + } + + fn counter_cases() -> Vec<(Metric, LabelSet, AggregateValue)> { + // (metric, label set criteria, expected_aggregate_value) + vec![ + // Metric with one sample without label set + ( + MetricBuilder::default().with_sample(1.into(), &LabelSet::empty()).build(), + LabelSet::empty(), + 1.0.into(), + ), + // Metric with one sample with a label set + ( + MetricBuilder::default() + .with_sample(1.into(), &[("l1", "l1_value")].into()) + .build(), + [("l1", "l1_value")].into(), + 1.0.into(), + ), + // Metric with two samples, different label sets, sum all + ( + MetricBuilder::default() + .with_sample(1.into(), &[("l1", "l1_value")].into()) + .with_sample(2.into(), &[("l2", "l2_value")].into()) + .build(), + LabelSet::empty(), + 3.0.into(), + ), + // Metric with two samples, different label sets, sum one + ( + MetricBuilder::default() + .with_sample(1.into(), &[("l1", "l1_value")].into()) + .with_sample(2.into(), &[("l2", "l2_value")].into()) + .build(), + [("l1", "l1_value")].into(), + 1.0.into(), + ), + // Metric with two samples, same label key, different label values, sum by key + ( + MetricBuilder::default() + .with_sample(1.into(), &[("l1", "l1_value"), ("la", "la_value")].into()) + .with_sample(2.into(), &[("l1", "l1_value"), ("lb", "lb_value")].into()) + .build(), + [("l1", "l1_value")].into(), + 3.0.into(), + ), + // Metric with two samples, different label values, sum by subkey + ( + MetricBuilder::default() + .with_sample(1.into(), &[("l1", "l1_value"), ("la", "la_value")].into()) + .with_sample(2.into(), &[("l1", "l1_value"), ("lb", "lb_value")].into()) + .build(), + [("la", "la_value")].into(), + 1.0.into(), + ), + // Edge: Metric with no samples at all + (MetricBuilder::default().build(), LabelSet::empty(), 0.0.into()), + // Edge: Metric with samples but no matching labels + ( + MetricBuilder::default() + .with_sample(5.into(), &[("foo", "bar")].into()) + .build(), + [("not", "present")].into(), + 0.0.into(), + ), + // Edge: Metric with zero value + ( + MetricBuilder::default() + .with_sample(0.into(), &[("l3", "l3_value")].into()) + .build(), + [("l3", "l3_value")].into(), + 0.0.into(), + ), + // Edge: Metric with a very large value + ( + MetricBuilder::default() + .with_sample(u64::MAX.into(), &LabelSet::empty()) + .build(), + LabelSet::empty(), + #[allow(clippy::cast_precision_loss)] + (u64::MAX as f64).into(), + ), + ] + } + + fn gauge_cases() -> Vec<(Metric, LabelSet, AggregateValue)> { + // (metric, label set criteria, expected_aggregate_value) + vec![ + // Metric with one sample without label set + ( + MetricBuilder::default().with_sample(1.0.into(), &LabelSet::empty()).build(), + LabelSet::empty(), + 1.0.into(), + ), + // Metric with one sample with a label set + ( + MetricBuilder::default() + .with_sample(1.0.into(), &[("l1", "l1_value")].into()) + .build(), + [("l1", "l1_value")].into(), + 1.0.into(), + ), + // Metric with two samples, different label sets, sum all + ( + MetricBuilder::default() + .with_sample(1.0.into(), &[("l1", "l1_value")].into()) + .with_sample(2.0.into(), &[("l2", "l2_value")].into()) + .build(), + LabelSet::empty(), + 3.0.into(), + ), + // Metric with two samples, different label sets, sum one + ( + MetricBuilder::default() + .with_sample(1.0.into(), &[("l1", "l1_value")].into()) + .with_sample(2.0.into(), &[("l2", "l2_value")].into()) + .build(), + [("l1", "l1_value")].into(), + 1.0.into(), + ), + // Metric with two samples, same label key, different label values, sum by key + ( + MetricBuilder::default() + .with_sample(1.0.into(), &[("l1", "l1_value"), ("la", "la_value")].into()) + .with_sample(2.0.into(), &[("l1", "l1_value"), ("lb", "lb_value")].into()) + .build(), + [("l1", "l1_value")].into(), + 3.0.into(), + ), + // Metric with two samples, different label values, sum by subkey + ( + MetricBuilder::default() + .with_sample(1.0.into(), &[("l1", "l1_value"), ("la", "la_value")].into()) + .with_sample(2.0.into(), &[("l1", "l1_value"), ("lb", "lb_value")].into()) + .build(), + [("la", "la_value")].into(), + 1.0.into(), + ), + // Edge: Metric with no samples at all + (MetricBuilder::default().build(), LabelSet::empty(), 0.0.into()), + // Edge: Metric with samples but no matching labels + ( + MetricBuilder::default() + .with_sample(5.0.into(), &[("foo", "bar")].into()) + .build(), + [("not", "present")].into(), + 0.0.into(), + ), + // Edge: Metric with zero value + ( + MetricBuilder::default() + .with_sample(0.0.into(), &[("l3", "l3_value")].into()) + .build(), + [("l3", "l3_value")].into(), + 0.0.into(), + ), + // Edge: Metric with negative values + ( + MetricBuilder::default() + .with_sample((-2.0).into(), &[("l4", "l4_value")].into()) + .with_sample(3.0.into(), &[("l5", "l5_value")].into()) + .build(), + LabelSet::empty(), + 1.0.into(), + ), + // Edge: Metric with a very large value + ( + MetricBuilder::default() + .with_sample(f64::MAX.into(), &LabelSet::empty()) + .build(), + LabelSet::empty(), + f64::MAX.into(), + ), + ] + } + + #[test] + fn test_counter_cases() { + for (idx, (metric, criteria, expected_value)) in counter_cases().iter().enumerate() { + let sum = metric.sum(criteria); + + assert_eq!( + sum, *expected_value, + "at case {idx}, expected sum to be {expected_value}, got {sum}" + ); + } + } + + #[test] + fn test_gauge_cases() { + for (idx, (metric, criteria, expected_value)) in gauge_cases().iter().enumerate() { + let sum = metric.sum(criteria); + + assert_eq!( + sum, *expected_value, + "at case {idx}, expected sum to be {expected_value}, got {sum}" + ); + } + } +} diff --git a/packages/metrics/src/metric/mod.rs b/packages/metrics/src/metric/mod.rs index df743c519..8ee24493a 100644 --- a/packages/metrics/src/metric/mod.rs +++ b/packages/metrics/src/metric/mod.rs @@ -1,3 +1,4 @@ +pub mod aggregate; pub mod description; pub mod name; diff --git a/packages/metrics/src/metric_collection/aggregate.rs b/packages/metrics/src/metric_collection/aggregate.rs new file mode 100644 index 000000000..7fd744d92 --- /dev/null +++ b/packages/metrics/src/metric_collection/aggregate.rs @@ -0,0 +1,112 @@ +use crate::aggregate::AggregateValue; +use crate::counter::Counter; +use crate::gauge::Gauge; +use crate::label::LabelSet; +use crate::metric::aggregate::sum::Sum as MetricSumTrait; +use crate::metric::MetricName; +use crate::metric_collection::{MetricCollection, MetricKindCollection}; + +pub trait Sum { + fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Option; +} + +impl Sum for MetricCollection { + fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Option { + if let Some(value) = self.counters.sum(metric_name, label_set_criteria) { + return Some(value); + } + + self.gauges.sum(metric_name, label_set_criteria) + } +} + +impl Sum for MetricKindCollection { + fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Option { + self.metrics.get(metric_name).map(|metric| metric.sum(label_set_criteria)) + } +} + +impl Sum for MetricKindCollection { + fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Option { + self.metrics.get(metric_name).map(|metric| metric.sum(label_set_criteria)) + } +} + +#[cfg(test)] +mod tests { + + mod it_should_allow_summing_all_metric_samples_containing_some_given_labels { + + use torrust_tracker_primitives::DurationSinceUnixEpoch; + + use crate::label::LabelValue; + use crate::label_name; + use crate::metric_collection::aggregate::Sum; + + #[test] + fn type_counter_with_two_samples() { + use crate::label::LabelSet; + use crate::metric_collection::MetricCollection; + use crate::metric_name; + + let metric_name = metric_name!("test_counter"); + + let mut collection = MetricCollection::default(); + + collection + .increment_counter( + &metric_name!("test_counter"), + &(label_name!("label_1"), LabelValue::new("value_1")).into(), + DurationSinceUnixEpoch::from_secs(1), + ) + .unwrap(); + + collection + .increment_counter( + &metric_name!("test_counter"), + &(label_name!("label_2"), LabelValue::new("value_2")).into(), + DurationSinceUnixEpoch::from_secs(1), + ) + .unwrap(); + + assert_eq!(collection.sum(&metric_name, &LabelSet::empty()), Some(2.0.into())); + assert_eq!( + collection.sum(&metric_name, &(label_name!("label_1"), LabelValue::new("value_1")).into()), + Some(1.0.into()) + ); + } + + #[test] + fn type_gauge_with_two_samples() { + use crate::label::LabelSet; + use crate::metric_collection::MetricCollection; + use crate::metric_name; + + let metric_name = metric_name!("test_gauge"); + + let mut collection = MetricCollection::default(); + + collection + .increment_gauge( + &metric_name!("test_gauge"), + &(label_name!("label_1"), LabelValue::new("value_1")).into(), + DurationSinceUnixEpoch::from_secs(1), + ) + .unwrap(); + + collection + .increment_gauge( + &metric_name!("test_gauge"), + &(label_name!("label_2"), LabelValue::new("value_2")).into(), + DurationSinceUnixEpoch::from_secs(1), + ) + .unwrap(); + + assert_eq!(collection.sum(&metric_name, &LabelSet::empty()), Some(2.0.into())); + assert_eq!( + collection.sum(&metric_name, &(label_name!("label_1"), LabelValue::new("value_1")).into()), + Some(1.0.into()) + ); + } + } +} diff --git a/packages/metrics/src/metric_collection.rs b/packages/metrics/src/metric_collection/mod.rs similarity index 98% rename from packages/metrics/src/metric_collection.rs rename to packages/metrics/src/metric_collection/mod.rs index ff932caae..e183236aa 100644 --- a/packages/metrics/src/metric_collection.rs +++ b/packages/metrics/src/metric_collection/mod.rs @@ -1,3 +1,5 @@ +pub mod aggregate; + use std::collections::{HashMap, HashSet}; use serde::ser::{SerializeSeq, Serializer}; @@ -103,7 +105,7 @@ impl MetricCollection { /// /// Return an error if a metrics of a different type with the same name /// already exists. - pub fn increase_counter( + pub fn increment_counter( &mut self, name: &MetricName, label_set: &LabelSet, @@ -669,7 +671,7 @@ udp_tracker_server_performance_avg_announce_processing_time_ns{server_binding_ip // First create a counter collection - .increase_counter(&metric_name!("test_metric"), &label_set, time) + .increment_counter(&metric_name!("test_metric"), &label_set, time) .unwrap(); // Then try to create a gauge with the same name @@ -690,7 +692,7 @@ udp_tracker_server_performance_avg_announce_processing_time_ns{server_binding_ip .unwrap(); // Then try to create a counter with the same name - let result = collection.increase_counter(&metric_name!("test_metric"), &label_set, time); + let result = collection.increment_counter(&metric_name!("test_metric"), &label_set, time); assert!(result.is_err()); } @@ -803,7 +805,7 @@ http_tracker_core_announce_requests_received_total{server_binding_ip="0.0.0.0",s let mut collection1 = MetricCollection::default(); collection1 - .increase_counter(&metric_name!("test_counter"), &label_set, time) + .increment_counter(&metric_name!("test_counter"), &label_set, time) .unwrap(); let mut collection2 = MetricCollection::default(); @@ -824,12 +826,12 @@ http_tracker_core_announce_requests_received_total{server_binding_ip="0.0.0.0",s let mut collection1 = MetricCollection::default(); collection1 - .increase_counter(&metric_name!("test_metric"), &label_set, time) + .increment_counter(&metric_name!("test_metric"), &label_set, time) .unwrap(); let mut collection2 = MetricCollection::default(); collection2 - .increase_counter(&metric_name!("test_metric"), &label_set, time) + .increment_counter(&metric_name!("test_metric"), &label_set, time) .unwrap(); let result = collection1.merge(&collection2); @@ -843,7 +845,7 @@ http_tracker_core_announce_requests_received_total{server_binding_ip="0.0.0.0",s let mut collection1 = MetricCollection::default(); collection1 - .increase_counter(&metric_name!("test_metric"), &label_set, time) + .increment_counter(&metric_name!("test_metric"), &label_set, time) .unwrap(); let mut collection2 = MetricCollection::default(); @@ -940,7 +942,7 @@ http_tracker_core_announce_requests_received_total{server_binding_ip="0.0.0.0",s let mut collection = collection_with_one_counter(&metric_name, &label_set, Counter::new(0)); collection - .increase_counter(&metric_name!("test_counter"), &label_set, time) + .increment_counter(&metric_name!("test_counter"), &label_set, time) .unwrap(); assert_eq!( @@ -958,10 +960,10 @@ http_tracker_core_announce_requests_received_total{server_binding_ip="0.0.0.0",s MetricCollection::new(MetricKindCollection::default(), MetricKindCollection::default()).unwrap(); metric_collection - .increase_counter(&metric_name!("test_counter"), &label_set, time) + .increment_counter(&metric_name!("test_counter"), &label_set, time) .unwrap(); metric_collection - .increase_counter(&metric_name!("test_counter"), &label_set, time) + .increment_counter(&metric_name!("test_counter"), &label_set, time) .unwrap(); assert_eq!( diff --git a/packages/swarm-coordination-registry/src/statistics/metrics.rs b/packages/swarm-coordination-registry/src/statistics/metrics.rs index f8ab3f9d9..d62a1ba6e 100644 --- a/packages/swarm-coordination-registry/src/statistics/metrics.rs +++ b/packages/swarm-coordination-registry/src/statistics/metrics.rs @@ -21,7 +21,7 @@ impl Metrics { labels: &LabelSet, now: DurationSinceUnixEpoch, ) -> Result<(), Error> { - self.metric_collection.increase_counter(metric_name, labels, now) + self.metric_collection.increment_counter(metric_name, labels, now) } /// # Errors diff --git a/packages/tracker-core/src/statistics/metrics.rs b/packages/tracker-core/src/statistics/metrics.rs index 02cc51499..a5caaf1cf 100644 --- a/packages/tracker-core/src/statistics/metrics.rs +++ b/packages/tracker-core/src/statistics/metrics.rs @@ -21,7 +21,7 @@ impl Metrics { labels: &LabelSet, now: DurationSinceUnixEpoch, ) -> Result<(), Error> { - self.metric_collection.increase_counter(metric_name, labels, now) + self.metric_collection.increment_counter(metric_name, labels, now) } /// # Errors diff --git a/packages/udp-tracker-core/src/statistics/metrics.rs b/packages/udp-tracker-core/src/statistics/metrics.rs index 94aa7d08f..e6ff8d5f6 100644 --- a/packages/udp-tracker-core/src/statistics/metrics.rs +++ b/packages/udp-tracker-core/src/statistics/metrics.rs @@ -47,7 +47,7 @@ impl Metrics { labels: &LabelSet, now: DurationSinceUnixEpoch, ) -> Result<(), Error> { - self.metric_collection.increase_counter(metric_name, labels, now) + self.metric_collection.increment_counter(metric_name, labels, now) } /// # Errors diff --git a/packages/udp-tracker-server/src/statistics/metrics.rs b/packages/udp-tracker-server/src/statistics/metrics.rs index 7b18f6418..ac6250872 100644 --- a/packages/udp-tracker-server/src/statistics/metrics.rs +++ b/packages/udp-tracker-server/src/statistics/metrics.rs @@ -78,7 +78,7 @@ impl Metrics { labels: &LabelSet, now: DurationSinceUnixEpoch, ) -> Result<(), Error> { - self.metric_collection.increase_counter(metric_name, labels, now) + self.metric_collection.increment_counter(metric_name, labels, now) } /// # Errors From 4da4f8351c1e616421bf0e8b5b83b1926fe34cd4 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 12 Jun 2025 08:49:47 +0100 Subject: [PATCH 2/8] refactor: [#1446] rename vars --- packages/metrics/src/label/set.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/metrics/src/label/set.rs b/packages/metrics/src/label/set.rs index 673f330c1..542f5d2e6 100644 --- a/packages/metrics/src/label/set.rs +++ b/packages/metrics/src/label/set.rs @@ -19,8 +19,8 @@ impl LabelSet { } /// Insert a new label pair or update the value of an existing label. - pub fn upsert(&mut self, key: LabelName, value: LabelValue) { - self.items.insert(key, value); + pub fn upsert(&mut self, name: LabelName, value: LabelValue) { + self.items.insert(name, value); } pub fn is_empty(&self) -> bool { @@ -35,7 +35,7 @@ impl LabelSet { } pub fn matches(&self, criteria: &LabelSet) -> bool { - criteria.iter().all(|(key, value)| self.contains_pair(key, value)) + criteria.iter().all(|(name, value)| self.contains_pair(name, value)) } pub fn iter(&self) -> Iter<'_, LabelName, LabelValue> { @@ -48,7 +48,7 @@ impl Display for LabelSet { let items = self .items .iter() - .map(|(key, value)| format!("{key}=\"{value}\"")) + .map(|(name, value)| format!("{name}=\"{value}\"")) .collect::>() .join(","); @@ -90,8 +90,8 @@ impl From> for LabelSet { fn from(vec: Vec) -> Self { let mut items = BTreeMap::new(); - for (key, value) in vec { - items.insert(key, value); + for (name, value) in vec { + items.insert(name, value); } Self { items } @@ -160,8 +160,8 @@ impl Serialize for LabelSet { { self.items .iter() - .map(|(key, value)| SerializedLabel { - name: key.clone(), + .map(|(name, value)| SerializedLabel { + name: name.clone(), value: value.clone(), }) .collect::>() From 0d134396e53c9fe75becaad8f03072a0e53d3a22 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 12 Jun 2025 09:50:24 +0100 Subject: [PATCH 3/8] test: [#1446] add more tests to metrics package --- packages/metrics/.gitignore | 2 +- packages/metrics/README.md | 22 +++ packages/metrics/cSpell.json | 19 +++ packages/metrics/src/aggregate.rs | 115 ++++++++++++++++ packages/metrics/src/counter.rs | 156 ++++++++++++++++++++++ packages/metrics/src/gauge.rs | 124 +++++++++++++++++ packages/metrics/src/label/set.rs | 112 +++++++++++++++- packages/metrics/src/metric/mod.rs | 29 ++++ packages/metrics/src/sample.rs | 18 +++ packages/metrics/src/sample_collection.rs | 50 +++++++ 10 files changed, 645 insertions(+), 2 deletions(-) create mode 100644 packages/metrics/cSpell.json diff --git a/packages/metrics/.gitignore b/packages/metrics/.gitignore index 0b1372e5c..6350e9868 100644 --- a/packages/metrics/.gitignore +++ b/packages/metrics/.gitignore @@ -1 +1 @@ -./.coverage +.coverage diff --git a/packages/metrics/README.md b/packages/metrics/README.md index 627640eec..885d6fa45 100644 --- a/packages/metrics/README.md +++ b/packages/metrics/README.md @@ -6,6 +6,28 @@ A library with the metrics types used by the [Torrust Tracker](https://github.co [Crate documentation](https://docs.rs/torrust-tracker-metrics). +## Testing + +Run coverage report: + +```console +cargo llvm-cov --package torrust-tracker-metrics +``` + +Generate LCOV report with `llvm-cov` (for Visual Studio Code extension): + +```console +mkdir -p ./.coverage +cargo llvm-cov --package torrust-tracker-metrics --lcov --output-path=./.coverage/lcov.info +``` + +Generate HTML report with `llvm-cov`: + +```console +mkdir -p ./.coverage +cargo llvm-cov --package torrust-tracker-metrics --html --output-dir ./.coverage +``` + ## Acknowledgements We copied some parts like units or function names and signatures from the crate [metrics](https://crates.io/crates/metrics) because we wanted to make it compatible as much as possible with it. In the future, we may consider using the `metrics` crate directly instead of maintaining our own version. diff --git a/packages/metrics/cSpell.json b/packages/metrics/cSpell.json new file mode 100644 index 000000000..1a2c13d2e --- /dev/null +++ b/packages/metrics/cSpell.json @@ -0,0 +1,19 @@ +{ + "words": [ + "cloneable", + "formatjson", + "Gibibytes", + "Kibibytes", + "Mebibytes", + "ñaca", + "rstest", + "subsec", + "Tebibytes", + "thiserror" + ], + "enableFiletypes": [ + "dockerfile", + "shellscript", + "toml" + ] +} diff --git a/packages/metrics/src/aggregate.rs b/packages/metrics/src/aggregate.rs index 875360cd9..e480be396 100644 --- a/packages/metrics/src/aggregate.rs +++ b/packages/metrics/src/aggregate.rs @@ -26,3 +26,118 @@ impl From for f64 { value.0 } } + +#[cfg(test)] +mod tests { + use approx::assert_relative_eq; + + use super::*; + + #[test] + fn it_should_be_created_with_new() { + let value = AggregateValue::new(42.5); + assert_relative_eq!(value.value(), 42.5); + } + + #[test] + fn it_should_return_the_inner_value() { + let value = AggregateValue::new(123.456); + assert_relative_eq!(value.value(), 123.456); + } + + #[test] + fn it_should_handle_zero_value() { + let value = AggregateValue::new(0.0); + assert_relative_eq!(value.value(), 0.0); + } + + #[test] + fn it_should_handle_negative_values() { + let value = AggregateValue::new(-42.5); + assert_relative_eq!(value.value(), -42.5); + } + + #[test] + fn it_should_handle_infinity() { + let value = AggregateValue::new(f64::INFINITY); + assert_relative_eq!(value.value(), f64::INFINITY); + } + + #[test] + fn it_should_handle_nan() { + let value = AggregateValue::new(f64::NAN); + assert!(value.value().is_nan()); + } + + #[test] + fn it_should_be_created_from_f64() { + let value: AggregateValue = 42.5.into(); + assert_relative_eq!(value.value(), 42.5); + } + + #[test] + fn it_should_convert_to_f64() { + let value = AggregateValue::new(42.5); + let f64_value: f64 = value.into(); + assert_relative_eq!(f64_value, 42.5); + } + + #[test] + fn it_should_be_displayable() { + let value = AggregateValue::new(42.5); + assert_eq!(value.to_string(), "42.5"); + } + + #[test] + fn it_should_be_debuggable() { + let value = AggregateValue::new(42.5); + let debug_string = format!("{value:?}"); + assert_eq!(debug_string, "AggregateValue(42.5)"); + } + + #[test] + fn it_should_be_cloneable() { + let value = AggregateValue::new(42.5); + let cloned_value = value; + assert_eq!(value, cloned_value); + } + + #[test] + fn it_should_be_copyable() { + let value = AggregateValue::new(42.5); + let copied_value = value; + assert_eq!(value, copied_value); + } + + #[test] + fn it_should_support_equality_comparison() { + let value1 = AggregateValue::new(42.5); + let value2 = AggregateValue::new(42.5); + let value3 = AggregateValue::new(43.0); + + assert_eq!(value1, value2); + assert_ne!(value1, value3); + } + + #[test] + fn it_should_handle_special_float_values_in_equality() { + let nan1 = AggregateValue::new(f64::NAN); + let nan2 = AggregateValue::new(f64::NAN); + let infinity = AggregateValue::new(f64::INFINITY); + let neg_infinity = AggregateValue::new(f64::NEG_INFINITY); + + // NaN is not equal to itself in IEEE 754 + assert_ne!(nan1, nan2); + assert_eq!(infinity, AggregateValue::new(f64::INFINITY)); + assert_eq!(neg_infinity, AggregateValue::new(f64::NEG_INFINITY)); + assert_ne!(infinity, neg_infinity); + } + + #[test] + fn it_should_handle_conversion_roundtrip() { + let original_value = 42.5; + let aggregate_value = AggregateValue::from(original_value); + let converted_back: f64 = aggregate_value.into(); + assert_relative_eq!(original_value, converted_back); + } +} diff --git a/packages/metrics/src/counter.rs b/packages/metrics/src/counter.rs index 3148ab4c3..0e2002181 100644 --- a/packages/metrics/src/counter.rs +++ b/packages/metrics/src/counter.rs @@ -107,4 +107,160 @@ mod tests { let counter = Counter::new(42); assert_eq!(counter.to_prometheus(), "42"); } + + #[test] + fn it_could_be_converted_from_u32() { + let counter: Counter = 42u32.into(); + assert_eq!(counter.value(), 42); + } + + #[test] + fn it_could_be_converted_from_i32() { + let counter: Counter = 42i32.into(); + assert_eq!(counter.value(), 42); + } + + #[test] + fn it_should_return_primitive_value() { + let counter = Counter::new(123); + assert_eq!(counter.primitive(), 123); + } + + #[test] + fn it_should_handle_zero_value() { + let counter = Counter::new(0); + assert_eq!(counter.value(), 0); + assert_eq!(counter.primitive(), 0); + } + + #[test] + fn it_should_handle_large_values() { + let counter = Counter::new(u64::MAX); + assert_eq!(counter.value(), u64::MAX); + } + + #[test] + fn it_should_handle_u32_max_conversion() { + let counter: Counter = u32::MAX.into(); + assert_eq!(counter.value(), u64::from(u32::MAX)); + } + + #[test] + fn it_should_handle_i32_max_conversion() { + let counter: Counter = i32::MAX.into(); + assert_eq!(counter.value(), i32::MAX as u64); + } + + #[test] + fn it_should_handle_negative_i32_conversion() { + let counter: Counter = (-42i32).into(); + #[allow(clippy::cast_sign_loss)] + let expected = (-42i32) as u64; + assert_eq!(counter.value(), expected); + } + + #[test] + fn it_should_handle_i32_min_conversion() { + let counter: Counter = i32::MIN.into(); + #[allow(clippy::cast_sign_loss)] + let expected = i32::MIN as u64; + assert_eq!(counter.value(), expected); + } + + #[test] + fn it_should_handle_large_increments() { + let mut counter = Counter::new(100); + counter.increment(1000); + assert_eq!(counter.value(), 1100); + + counter.increment(u64::MAX - 1100); + assert_eq!(counter.value(), u64::MAX); + } + + #[test] + fn it_should_support_multiple_absolute_operations() { + let mut counter = Counter::new(0); + + counter.absolute(100); + assert_eq!(counter.value(), 100); + + counter.absolute(50); + assert_eq!(counter.value(), 50); + + counter.absolute(0); + assert_eq!(counter.value(), 0); + } + + #[test] + fn it_should_be_displayable() { + let counter = Counter::new(42); + assert_eq!(counter.to_string(), "42"); + + let counter = Counter::new(0); + assert_eq!(counter.to_string(), "0"); + } + + #[test] + fn it_should_be_debuggable() { + let counter = Counter::new(42); + let debug_string = format!("{counter:?}"); + assert_eq!(debug_string, "Counter(42)"); + } + + #[test] + fn it_should_be_cloneable() { + let counter = Counter::new(42); + let cloned_counter = counter.clone(); + assert_eq!(counter, cloned_counter); + assert_eq!(counter.value(), cloned_counter.value()); + } + + #[test] + fn it_should_support_equality_comparison() { + let counter1 = Counter::new(42); + let counter2 = Counter::new(42); + let counter3 = Counter::new(43); + + assert_eq!(counter1, counter2); + assert_ne!(counter1, counter3); + } + + #[test] + fn it_should_have_default_value() { + let counter = Counter::default(); + assert_eq!(counter.value(), 0); + } + + #[test] + fn it_should_handle_conversion_roundtrip() { + let original_value = 12345u64; + let counter = Counter::from(original_value); + let converted_back: u64 = counter.into(); + assert_eq!(original_value, converted_back); + } + + #[test] + fn it_should_handle_u32_conversion_roundtrip() { + let original_value = 12345u32; + let counter = Counter::from(original_value); + assert_eq!(counter.value(), u64::from(original_value)); + } + + #[test] + fn it_should_handle_i32_conversion_roundtrip() { + let original_value = 12345i32; + let counter = Counter::from(original_value); + #[allow(clippy::cast_sign_loss)] + let expected = original_value as u64; + assert_eq!(counter.value(), expected); + } + + #[test] + fn it_should_serialize_large_values_to_prometheus() { + let counter = Counter::new(u64::MAX); + assert_eq!(counter.to_prometheus(), u64::MAX.to_string()); + + let counter = Counter::new(0); + assert_eq!(counter.to_prometheus(), "0"); + } } diff --git a/packages/metrics/src/gauge.rs b/packages/metrics/src/gauge.rs index a2ef8135f..d0883715b 100644 --- a/packages/metrics/src/gauge.rs +++ b/packages/metrics/src/gauge.rs @@ -113,4 +113,128 @@ mod tests { let counter = Gauge::new(42.1); assert_eq!(counter.to_prometheus(), "42.1"); } + + #[test] + fn it_could_be_converted_from_f32() { + let gauge: Gauge = 42.5f32.into(); + assert_relative_eq!(gauge.value(), 42.5); + } + + #[test] + fn it_should_return_primitive_value() { + let gauge = Gauge::new(123.456); + assert_relative_eq!(gauge.primitive(), 123.456); + } + + #[test] + fn it_should_handle_zero_value() { + let gauge = Gauge::new(0.0); + assert_relative_eq!(gauge.value(), 0.0); + assert_relative_eq!(gauge.primitive(), 0.0); + } + + #[test] + fn it_should_handle_negative_values() { + let gauge = Gauge::new(-42.5); + assert_relative_eq!(gauge.value(), -42.5); + } + + #[test] + fn it_should_handle_large_values() { + let gauge = Gauge::new(f64::MAX); + assert_relative_eq!(gauge.value(), f64::MAX); + } + + #[test] + fn it_should_handle_infinity() { + let gauge = Gauge::new(f64::INFINITY); + assert_relative_eq!(gauge.value(), f64::INFINITY); + } + + #[test] + fn it_should_handle_nan() { + let gauge = Gauge::new(f64::NAN); + assert!(gauge.value().is_nan()); + } + + #[test] + fn it_should_be_displayable() { + let gauge = Gauge::new(42.5); + assert_eq!(gauge.to_string(), "42.5"); + + let gauge = Gauge::new(0.0); + assert_eq!(gauge.to_string(), "0"); + } + + #[test] + fn it_should_be_debuggable() { + let gauge = Gauge::new(42.5); + let debug_string = format!("{gauge:?}"); + assert_eq!(debug_string, "Gauge(42.5)"); + } + + #[test] + fn it_should_be_cloneable() { + let gauge = Gauge::new(42.5); + let cloned_gauge = gauge.clone(); + assert_eq!(gauge, cloned_gauge); + assert_relative_eq!(gauge.value(), cloned_gauge.value()); + } + + #[test] + fn it_should_support_equality_comparison() { + let gauge1 = Gauge::new(42.5); + let gauge2 = Gauge::new(42.5); + let gauge3 = Gauge::new(43.0); + + assert_eq!(gauge1, gauge2); + assert_ne!(gauge1, gauge3); + } + + #[test] + fn it_should_have_default_value() { + let gauge = Gauge::default(); + assert_relative_eq!(gauge.value(), 0.0); + } + + #[test] + fn it_should_handle_conversion_roundtrip() { + let original_value = 12345.678; + let gauge = Gauge::from(original_value); + let converted_back: f64 = gauge.into(); + assert_relative_eq!(original_value, converted_back); + } + + #[test] + fn it_should_handle_f32_conversion_roundtrip() { + let original_value = 12345.5f32; + let gauge = Gauge::from(original_value); + assert_relative_eq!(gauge.value(), f64::from(original_value)); + } + + #[test] + fn it_should_handle_multiple_operations() { + let mut gauge = Gauge::new(100.0); + + gauge.increment(50.0); + assert_relative_eq!(gauge.value(), 150.0); + + gauge.decrement(25.0); + assert_relative_eq!(gauge.value(), 125.0); + + gauge.set(200.0); + assert_relative_eq!(gauge.value(), 200.0); + } + + #[test] + fn it_should_serialize_special_values_to_prometheus() { + let gauge = Gauge::new(f64::INFINITY); + assert_eq!(gauge.to_prometheus(), "inf"); + + let gauge = Gauge::new(f64::NEG_INFINITY); + assert_eq!(gauge.to_prometheus(), "-inf"); + + let gauge = Gauge::new(f64::NAN); + assert_eq!(gauge.to_prometheus(), "NaN"); + } } diff --git a/packages/metrics/src/label/set.rs b/packages/metrics/src/label/set.rs index 542f5d2e6..46256e4d5 100644 --- a/packages/metrics/src/label/set.rs +++ b/packages/metrics/src/label/set.rs @@ -297,10 +297,18 @@ mod tests { #[test] fn it_should_allow_serializing_to_prometheus_format() { let label_set = LabelSet::from((label_name!("label_name"), LabelValue::new("label value"))); - assert_eq!(label_set.to_prometheus(), r#"{label_name="label value"}"#); } + #[test] + fn it_should_handle_prometheus_format_with_special_characters() { + let label_set: LabelSet = vec![("label_with_underscores", "value_with_underscores")].into(); + assert_eq!( + label_set.to_prometheus(), + r#"{label_with_underscores="value_with_underscores"}"# + ); + } + #[test] fn it_should_alphabetically_order_labels_in_prometheus_format() { let label_set = LabelSet::from([ @@ -471,4 +479,106 @@ mod tests { let a: LabelSet = (label_name!("x"), LabelValue::new("1")).into(); let _unused = a.clone(); } + + #[test] + fn it_should_check_if_empty() { + let empty_set = LabelSet::empty(); + assert!(empty_set.is_empty()); + } + + #[test] + fn it_should_check_if_non_empty() { + let non_empty_set: LabelSet = (label_name!("label"), LabelValue::new("value")).into(); + assert!(!non_empty_set.is_empty()); + } + + #[test] + fn it_should_create_an_empty_label_set() { + let empty_set = LabelSet::empty(); + assert!(empty_set.is_empty()); + } + + #[test] + fn it_should_check_if_contains_specific_label_pair() { + let label_set: LabelSet = vec![("service", "tracker"), ("protocol", "http")].into(); + + // Test existing pair + assert!(label_set.contains_pair(&LabelName::new("service"), &LabelValue::new("tracker"))); + assert!(label_set.contains_pair(&LabelName::new("protocol"), &LabelValue::new("http"))); + + // Test non-existing name + assert!(!label_set.contains_pair(&LabelName::new("missing"), &LabelValue::new("value"))); + + // Test existing name with wrong value + assert!(!label_set.contains_pair(&LabelName::new("service"), &LabelValue::new("wrong"))); + } + + #[test] + fn it_should_match_against_criteria() { + let label_set: LabelSet = vec![("service", "tracker"), ("protocol", "http"), ("version", "v1")].into(); + + // Empty criteria should match any label set + assert!(label_set.matches(&LabelSet::empty())); + + // Single matching criterion + let single_criteria: LabelSet = vec![("service", "tracker")].into(); + assert!(label_set.matches(&single_criteria)); + + // Multiple matching criteria + let multiple_criteria: LabelSet = vec![("service", "tracker"), ("protocol", "http")].into(); + assert!(label_set.matches(&multiple_criteria)); + + // Non-matching criterion + let non_matching: LabelSet = vec![("service", "wrong")].into(); + assert!(!label_set.matches(&non_matching)); + + // Partially matching criteria (one matches, one doesn't) + let partial_matching: LabelSet = vec![("service", "tracker"), ("missing", "value")].into(); + assert!(!label_set.matches(&partial_matching)); + + // Criteria with label not in original set + let missing_label: LabelSet = vec![("missing_label", "value")].into(); + assert!(!label_set.matches(&missing_label)); + } + + #[test] + fn it_should_allow_iteration_over_label_pairs() { + let label_set: LabelSet = vec![("service", "tracker"), ("protocol", "http")].into(); + + let mut count = 0; + + for (name, value) in label_set.iter() { + count += 1; + // Verify we can access name and value + assert!(!name.to_string().is_empty()); + assert!(!value.to_string().is_empty()); + } + + assert_eq!(count, 2); + } + + #[test] + fn it_should_display_empty_label_set() { + let empty_set = LabelSet::empty(); + assert_eq!(empty_set.to_string(), "{}"); + } + + #[test] + fn it_should_serialize_empty_label_set_to_prometheus_format() { + let empty_set = LabelSet::empty(); + assert_eq!(empty_set.to_prometheus(), ""); + } + + #[test] + fn it_should_maintain_order_in_iteration() { + let label_set: LabelSet = vec![("z_label", "z_value"), ("a_label", "a_value"), ("m_label", "m_value")].into(); + + let mut labels: Vec = vec![]; + for (name, _) in label_set.iter() { + labels.push(name.to_string()); + } + + // Should be in alphabetical order + assert_eq!(labels, vec!["a_label", "m_label", "z_label"]); + } } diff --git a/packages/metrics/src/metric/mod.rs b/packages/metrics/src/metric/mod.rs index 8ee24493a..d1aa01b94 100644 --- a/packages/metrics/src/metric/mod.rs +++ b/packages/metrics/src/metric/mod.rs @@ -322,4 +322,33 @@ mod tests { assert_relative_eq!(metric.get_sample_data(&label_set).unwrap().value().value(), 1.0); } } + + mod for_prometheus_serialization { + use super::super::*; + use crate::counter::Counter; + use crate::metric_name; + + #[test] + fn it_should_return_empty_string_for_prometheus_help_line_when_description_is_none() { + let name = metric_name!("test_metric"); + let samples = SampleCollection::::default(); + let metric = Metric::::new(name, None, None, samples); + + let help_line = metric.prometheus_help_line(); + + assert_eq!(help_line, String::new()); + } + + #[test] + fn it_should_return_formatted_help_line_for_prometheus_when_description_is_some() { + let name = metric_name!("test_metric"); + let description = MetricDescription::new("This is a test metric description"); + let samples = SampleCollection::::default(); + let metric = Metric::::new(name, None, Some(description), samples); + + let help_line = metric.prometheus_help_line(); + + assert_eq!(help_line, "# HELP test_metric This is a test metric description"); + } + } } diff --git a/packages/metrics/src/sample.rs b/packages/metrics/src/sample.rs index b9cd6c312..63f46b9b8 100644 --- a/packages/metrics/src/sample.rs +++ b/packages/metrics/src/sample.rs @@ -279,6 +279,15 @@ mod tests { assert_eq!(sample.to_prometheus(), r#"{label_name="label_value",method="GET"} 42"#); } + + #[test] + fn it_should_allow_exporting_to_prometheus_format_with_empty_label_set() { + let counter = Counter::new(42); + + let sample = Sample::new(counter, DurationSinceUnixEpoch::default(), LabelSet::default()); + + assert_eq!(sample.to_prometheus(), " 42"); + } } mod for_gauge_type_sample { use torrust_tracker_primitives::DurationSinceUnixEpoch; @@ -347,6 +356,15 @@ mod tests { assert_eq!(sample.to_prometheus(), r#"{label_name="label_value",method="GET"} 42"#); } + + #[test] + fn it_should_allow_exporting_to_prometheus_format_with_empty_label_set() { + let gauge = Gauge::new(42.0); + + let sample = Sample::new(gauge, DurationSinceUnixEpoch::default(), LabelSet::default()); + + assert_eq!(sample.to_prometheus(), " 42"); + } } mod serialization_to_json { diff --git a/packages/metrics/src/sample_collection.rs b/packages/metrics/src/sample_collection.rs index ef88b27dd..e520d7310 100644 --- a/packages/metrics/src/sample_collection.rs +++ b/packages/metrics/src/sample_collection.rs @@ -386,6 +386,56 @@ mod tests { assert_eq!(collection.get(&label2).unwrap().value(), &Counter::new(1)); assert_eq!(collection.len(), 2); } + + #[test] + fn it_should_allow_setting_absolute_value_for_a_counter() { + let label_set = LabelSet::default(); + let mut collection = SampleCollection::::default(); + + // Set absolute value for a non-existent label + collection.absolute(&label_set, 42, sample_update_time()); + + // Verify the label exists and has the absolute value + assert!(collection.get(&label_set).is_some()); + let sample = collection.get(&label_set).unwrap(); + assert_eq!(*sample.value(), Counter::new(42)); + } + + #[test] + fn it_should_allow_setting_absolute_value_for_existing_counter() { + let label_set = LabelSet::default(); + let mut collection = SampleCollection::::default(); + + // Initialize the sample with increment + collection.increment(&label_set, sample_update_time()); + + // Verify initial state + let sample = collection.get(&label_set).unwrap(); + assert_eq!(sample.value(), &Counter::new(1)); + + // Set absolute value + collection.absolute(&label_set, 100, sample_update_time()); + let sample = collection.get(&label_set).unwrap(); + assert_eq!(*sample.value(), Counter::new(100)); + } + + #[test] + fn it_should_update_time_when_setting_absolute_value() { + let label_set = LabelSet::default(); + let initial_time = sample_update_time(); + let mut collection = SampleCollection::::default(); + + // Set absolute value with initial time + collection.absolute(&label_set, 50, initial_time); + + // Set absolute value with a new time + let new_time = initial_time.add(DurationSinceUnixEpoch::from_secs(1)); + collection.absolute(&label_set, 75, new_time); + + let sample = collection.get(&label_set).unwrap(); + assert_eq!(sample.recorded_at(), new_time); + assert_eq!(*sample.value(), Counter::new(75)); + } } #[cfg(test)] From 476ece46e7b3b67d01e8fc2031aa0e9faf3578af Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 13 Jun 2025 10:52:53 +0100 Subject: [PATCH 4/8] refactor: [#1446] WIP. Calculate global metrics from labeled metrics We need to add a new label to make it easier to fileter by the server IP family: IPV4 or IPv6. --- Cargo.lock | 1 + .../src/statistics/event/handler.rs | 14 ++- .../http-tracker-core/src/statistics/mod.rs | 2 +- packages/metrics/src/aggregate.rs | 2 +- packages/rest-tracker-api-core/Cargo.toml | 1 + .../src/statistics/services.rs | 117 +++++++++++++++++- .../event/handler/request_accepted.rs | 4 +- .../udp-tracker-server/src/statistics/mod.rs | 2 +- 8 files changed, 135 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 269f7a3a2..6f8215bbf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4668,6 +4668,7 @@ dependencies = [ "torrust-tracker-swarm-coordination-registry", "torrust-tracker-test-helpers", "torrust-udp-tracker-server", + "tracing", ] [[package]] diff --git a/packages/http-tracker-core/src/statistics/event/handler.rs b/packages/http-tracker-core/src/statistics/event/handler.rs index f5506f6e3..dcb814eef 100644 --- a/packages/http-tracker-core/src/statistics/event/handler.rs +++ b/packages/http-tracker-core/src/statistics/event/handler.rs @@ -32,7 +32,12 @@ pub async fn handle_event(event: Event, stats_repository: &Arc, now: .increase_counter(&metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now) .await { - Ok(()) => {} + Ok(()) => { + tracing::debug!( + "Successfully increased the counter for HTTP announce requests received: {}", + label_set + ); + } Err(err) => tracing::error!("Failed to increase the counter: {}", err), }; } @@ -57,7 +62,12 @@ pub async fn handle_event(event: Event, stats_repository: &Arc, now: .increase_counter(&metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now) .await { - Ok(()) => {} + Ok(()) => { + tracing::debug!( + "Successfully increased the counter for HTTP scrape requests received: {}", + label_set + ); + } Err(err) => tracing::error!("Failed to increase the counter: {}", err), }; } diff --git a/packages/http-tracker-core/src/statistics/mod.rs b/packages/http-tracker-core/src/statistics/mod.rs index 7181632aa..b8ca865fa 100644 --- a/packages/http-tracker-core/src/statistics/mod.rs +++ b/packages/http-tracker-core/src/statistics/mod.rs @@ -8,7 +8,7 @@ use torrust_tracker_metrics::metric::description::MetricDescription; 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"; +pub const HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL: &str = "http_tracker_core_requests_received_total"; #[must_use] pub fn describe_metrics() -> Metrics { diff --git a/packages/metrics/src/aggregate.rs b/packages/metrics/src/aggregate.rs index e480be396..39b760fca 100644 --- a/packages/metrics/src/aggregate.rs +++ b/packages/metrics/src/aggregate.rs @@ -1,6 +1,6 @@ use derive_more::Display; -#[derive(Debug, Display, Clone, Copy, PartialEq)] +#[derive(Debug, Display, Clone, Copy, PartialEq, Default)] pub struct AggregateValue(f64); impl AggregateValue { diff --git a/packages/rest-tracker-api-core/Cargo.toml b/packages/rest-tracker-api-core/Cargo.toml index cc8eda903..d9e396960 100644 --- a/packages/rest-tracker-api-core/Cargo.toml +++ b/packages/rest-tracker-api-core/Cargo.toml @@ -23,6 +23,7 @@ torrust-tracker-metrics = { version = "3.0.0-develop", path = "../metrics" } torrust-tracker-primitives = { version = "3.0.0-develop", path = "../primitives" } torrust-tracker-swarm-coordination-registry = { version = "3.0.0-develop", path = "../swarm-coordination-registry" } torrust-udp-tracker-server = { version = "3.0.0-develop", path = "../udp-tracker-server" } +tracing = "0" [dev-dependencies] torrust-tracker-events = { version = "3.0.0-develop", path = "../events" } diff --git a/packages/rest-tracker-api-core/src/statistics/services.rs b/packages/rest-tracker-api-core/src/statistics/services.rs index 6474df0d7..3cfd6653e 100644 --- a/packages/rest-tracker-api-core/src/statistics/services.rs +++ b/packages/rest-tracker-api-core/src/statistics/services.rs @@ -1,11 +1,14 @@ use std::sync::Arc; +use bittorrent_http_tracker_core::statistics::HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL; use bittorrent_tracker_core::torrent::repository::in_memory::InMemoryTorrentRepository; use bittorrent_udp_tracker_core::services::banning::BanService; use bittorrent_udp_tracker_core::{self}; use tokio::sync::RwLock; +use torrust_tracker_metrics::metric_collection::aggregate::Sum; use torrust_tracker_metrics::metric_collection::MetricCollection; -use torrust_udp_tracker_server::statistics as udp_server_statistics; +use torrust_tracker_metrics::metric_name; +use torrust_udp_tracker_server::statistics::{self as udp_server_statistics, UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL}; use super::metrics::TorrentsMetrics; use crate::statistics::metrics::ProtocolMetrics; @@ -32,9 +35,38 @@ pub async fn get_metrics( http_stats_repository: Arc, udp_server_stats_repository: Arc, ) -> TrackerMetrics { + let protocol_metrics_from_global_metrics = get_protocol_metrics( + ban_service.clone(), + http_stats_repository.clone(), + udp_server_stats_repository.clone(), + ) + .await; + + let protocol_metrics_from_labeled_metrics = get_protocol_metrics_from_labeled_metrics( + ban_service.clone(), + http_stats_repository.clone(), + udp_server_stats_repository.clone(), + ) + .await; + + // todo: + // We keep both metrics until we deploy to production and we can + // ensure that the protocol metrics from labeled metrics are correct. + // After that we can remove the `get_protocol_metrics` function and + // use only the `get_protocol_metrics_from_labeled_metrics` function. + // And also remove the code in repositories to generate the global metrics. + let protocol_metrics = if protocol_metrics_from_global_metrics == protocol_metrics_from_labeled_metrics { + protocol_metrics_from_labeled_metrics + } else { + // tracing::warn!("The protocol metrics from global metrics and labeled metrics are different"); + // tracing::warn!("Global metrics: {:?}", protocol_metrics_from_global_metrics); + // tracing::warn!("Labeled metrics: {:?}", protocol_metrics_from_labeled_metrics); + protocol_metrics_from_global_metrics + }; + TrackerMetrics { torrents_metrics: get_torrents_metrics(in_memory_torrent_repository, tracker_core_stats_repository).await, - protocol_metrics: get_protocol_metrics(ban_service, http_stats_repository, udp_server_stats_repository).await, + protocol_metrics, } } @@ -99,6 +131,87 @@ async fn get_protocol_metrics( } } +#[allow(deprecated)] +async fn get_protocol_metrics_from_labeled_metrics( + ban_service: Arc>, + http_stats_repository: Arc, + udp_server_stats_repository: Arc, +) -> ProtocolMetrics { + let udp_banned_ips_total = ban_service.read().await.get_banned_ips_total(); + let http_stats = http_stats_repository.get_stats().await; + let udp_server_stats = udp_server_stats_repository.get_stats().await; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let tcp4_announces_handled = http_stats + .metric_collection + .sum( + &metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), + &[("request_kind", "announce")].into(), // todo: add label for `server_binding_ip_family` with value `inet` (inet/inet6) + ) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp4_announces_handled = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL), + &[("request_kind", "announce")].into(), // todo: add label for `server_binding_ip_family` with value `inet` (inet/inet6) + ) + .unwrap_or_default() + .value() as u64; + + /* + + todo: + + - Add a label for `server_binding_ip_family` with value `inet` (inet/inet6) + to all metrics containing an IP address. This will allow us to distinguish + between IPv4 and IPv6 metrics. + - Continue replacing the other metrics with the labeled metrics. + + */ + + // For backward compatibility we keep the `tcp4_connections_handled` and + // `tcp6_connections_handled` metrics. They don't make sense for the HTTP + // tracker, but we keep them for now. In new major versions we should remove + // them. + + ProtocolMetrics { + // TCPv4 + tcp4_connections_handled: tcp4_announces_handled + http_stats.tcp4_scrapes_handled, + tcp4_announces_handled, + tcp4_scrapes_handled: http_stats.tcp4_scrapes_handled, + // TCPv6 + tcp6_connections_handled: http_stats.tcp6_announces_handled + http_stats.tcp6_scrapes_handled, + tcp6_announces_handled: http_stats.tcp6_announces_handled, + tcp6_scrapes_handled: http_stats.tcp6_scrapes_handled, + // UDP + udp_requests_aborted: udp_server_stats.udp_requests_aborted, + udp_requests_banned: udp_server_stats.udp_requests_banned, + udp_banned_ips_total: udp_banned_ips_total as u64, + udp_avg_connect_processing_time_ns: udp_server_stats.udp_avg_connect_processing_time_ns, + udp_avg_announce_processing_time_ns: udp_server_stats.udp_avg_announce_processing_time_ns, + udp_avg_scrape_processing_time_ns: udp_server_stats.udp_avg_scrape_processing_time_ns, + // UDPv4 + udp4_requests: udp_server_stats.udp4_requests, + udp4_connections_handled: udp_server_stats.udp4_connections_handled, + udp4_announces_handled, + udp4_scrapes_handled: udp_server_stats.udp4_scrapes_handled, + udp4_responses: udp_server_stats.udp4_responses, + udp4_errors_handled: udp_server_stats.udp4_errors_handled, + // UDPv6 + udp6_requests: udp_server_stats.udp6_requests, + udp6_connections_handled: udp_server_stats.udp6_connections_handled, + udp6_announces_handled: udp_server_stats.udp6_announces_handled, + udp6_scrapes_handled: udp_server_stats.udp6_scrapes_handled, + udp6_responses: udp_server_stats.udp6_responses, + udp6_errors_handled: udp_server_stats.udp6_errors_handled, + } +} + #[derive(Debug, PartialEq)] pub struct TrackerLabeledMetrics { pub metrics: MetricCollection, diff --git a/packages/udp-tracker-server/src/statistics/event/handler/request_accepted.rs b/packages/udp-tracker-server/src/statistics/event/handler/request_accepted.rs index b296f8ec9..37b668227 100644 --- a/packages/udp-tracker-server/src/statistics/event/handler/request_accepted.rs +++ b/packages/udp-tracker-server/src/statistics/event/handler/request_accepted.rs @@ -47,7 +47,9 @@ pub async fn handle_event( .increase_counter(&metric_name!(UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL), &label_set, now) .await { - Ok(()) => {} + Ok(()) => { + tracing::debug!("Successfully increased the counter for UDP requests accepted: {}", label_set); + } Err(err) => tracing::error!("Failed to increase the counter: {}", err), }; } diff --git a/packages/udp-tracker-server/src/statistics/mod.rs b/packages/udp-tracker-server/src/statistics/mod.rs index ebb3df0bf..3a25fd51d 100644 --- a/packages/udp-tracker-server/src/statistics/mod.rs +++ b/packages/udp-tracker-server/src/statistics/mod.rs @@ -13,7 +13,7 @@ const UDP_TRACKER_SERVER_REQUESTS_BANNED_TOTAL: &str = "udp_tracker_server_reque pub(crate) const UDP_TRACKER_SERVER_IPS_BANNED_TOTAL: &str = "udp_tracker_server_ips_banned_total"; const UDP_TRACKER_SERVER_CONNECTION_ID_ERRORS_TOTAL: &str = "udp_tracker_server_connection_id_errors_total"; const UDP_TRACKER_SERVER_REQUESTS_RECEIVED_TOTAL: &str = "udp_tracker_server_requests_received_total"; -const UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL: &str = "udp_tracker_server_requests_accepted_total"; +pub const UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL: &str = "udp_tracker_server_requests_accepted_total"; const UDP_TRACKER_SERVER_RESPONSES_SENT_TOTAL: &str = "udp_tracker_server_responses_sent_total"; const UDP_TRACKER_SERVER_ERRORS_TOTAL: &str = "udp_tracker_server_errors_total"; const UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS: &str = "udp_tracker_server_performance_avg_processing_time_ns"; From 1376a7cb20166140c081c2bbf26443043bd1eb77 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 13 Jun 2025 11:02:35 +0100 Subject: [PATCH 5/8] refactor: [#1446] rename AddressType to IpType Address might be a socket address. --- packages/http-tracker-core/src/event.rs | 4 ++-- packages/primitives/src/service_binding.rs | 18 +++++++++--------- packages/udp-tracker-core/src/event.rs | 4 ++-- packages/udp-tracker-server/src/event.rs | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/http-tracker-core/src/event.rs b/packages/http-tracker-core/src/event.rs index cf969b4ff..e3d37569d 100644 --- a/packages/http-tracker-core/src/event.rs +++ b/packages/http-tracker-core/src/event.rs @@ -87,8 +87,8 @@ impl From for LabelSet { LabelValue::new(&connection_context.server.service_binding.bind_address().ip().to_string()), ), ( - label_name!("server_binding_address_type"), - LabelValue::new(&connection_context.server.service_binding.bind_address_type().to_string()), + label_name!("server_binding_address_ip_type"), + LabelValue::new(&connection_context.server.service_binding.bind_address_ip_type().to_string()), ), ( label_name!("server_binding_port"), diff --git a/packages/primitives/src/service_binding.rs b/packages/primitives/src/service_binding.rs index d5055130e..72d5e7f2e 100644 --- a/packages/primitives/src/service_binding.rs +++ b/packages/primitives/src/service_binding.rs @@ -26,7 +26,7 @@ impl fmt::Display for Protocol { } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Hash)] -pub enum AddressType { +pub enum IpType { /// Represents a plain IPv4 or IPv6 address. Plain, @@ -38,7 +38,7 @@ pub enum AddressType { V4MappedV6, } -impl fmt::Display for AddressType { +impl fmt::Display for IpType { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let addr_type_str = match self { Self::Plain => "plain", @@ -120,12 +120,12 @@ impl ServiceBinding { } #[must_use] - pub fn bind_address_type(&self) -> AddressType { + pub fn bind_address_ip_type(&self) -> IpType { if self.is_v4_mapped_v6() { - return AddressType::V4MappedV6; + return IpType::V4MappedV6; } - AddressType::Plain + IpType::Plain } /// # Panics @@ -169,7 +169,7 @@ mod tests { use rstest::rstest; use url::Url; - use crate::service_binding::{AddressType, Error, Protocol, ServiceBinding}; + use crate::service_binding::{Error, IpType, Protocol, ServiceBinding}; #[rstest] #[case("wildcard_ip", Protocol::UDP, SocketAddr::from_str("0.0.0.0:6969").unwrap())] @@ -203,7 +203,7 @@ mod tests { fn should_return_the_bind_address_plain_type_for_ipv4_ips() { let service_binding = ServiceBinding::new(Protocol::UDP, SocketAddr::from_str("127.0.0.1:6969").unwrap()).unwrap(); - assert_eq!(service_binding.bind_address_type(), AddressType::Plain); + assert_eq!(service_binding.bind_address_ip_type(), IpType::Plain); } #[test] @@ -211,7 +211,7 @@ mod tests { let service_binding = ServiceBinding::new(Protocol::UDP, SocketAddr::from_str("[0:0:0:0:0:0:0:1]:6969").unwrap()).unwrap(); - assert_eq!(service_binding.bind_address_type(), AddressType::Plain); + assert_eq!(service_binding.bind_address_ip_type(), IpType::Plain); } #[test] @@ -219,7 +219,7 @@ mod tests { let service_binding = ServiceBinding::new(Protocol::UDP, SocketAddr::from_str("[::ffff:192.0.2.33]:6969").unwrap()).unwrap(); - assert_eq!(service_binding.bind_address_type(), AddressType::V4MappedV6); + assert_eq!(service_binding.bind_address_ip_type(), IpType::V4MappedV6); } #[test] diff --git a/packages/udp-tracker-core/src/event.rs b/packages/udp-tracker-core/src/event.rs index e9264653e..d354d3e7e 100644 --- a/packages/udp-tracker-core/src/event.rs +++ b/packages/udp-tracker-core/src/event.rs @@ -60,8 +60,8 @@ impl From for LabelSet { LabelValue::new(&connection_context.server_service_binding.bind_address().ip().to_string()), ), ( - label_name!("server_binding_address_type"), - LabelValue::new(&connection_context.server_service_binding.bind_address_type().to_string()), + label_name!("server_binding_address_ip_type"), + LabelValue::new(&connection_context.server_service_binding.bind_address_ip_type().to_string()), ), ( label_name!("server_binding_port"), diff --git a/packages/udp-tracker-server/src/event.rs b/packages/udp-tracker-server/src/event.rs index 09fc139cb..c3e736a53 100644 --- a/packages/udp-tracker-server/src/event.rs +++ b/packages/udp-tracker-server/src/event.rs @@ -119,8 +119,8 @@ impl From for LabelSet { LabelValue::new(&connection_context.server_service_binding.bind_address().ip().to_string()), ), ( - label_name!("server_binding_address_type"), - LabelValue::new(&connection_context.server_service_binding.bind_address_type().to_string()), + label_name!("server_binding_address_ip_type"), + LabelValue::new(&connection_context.server_service_binding.bind_address_ip_type().to_string()), ), ( label_name!("server_binding_port"), From 96bae36c5b9bae301f9567bc339a43b7ee80219c Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 13 Jun 2025 11:19:02 +0100 Subject: [PATCH 6/8] feat: [#1446] add new metric label server_binding_address_ip_type Example: ``` udp_tracker_core_requests_received_total{request_kind="connect",server_binding_address_ip_family="inet",server_binding_address_ip_type="plain",server_binding_ip="0.0.0.0",server_binding_port="6969",server_binding_protocol="udp"} 1 ``` It's needed to easily filter metric samples to calculate aggregate values for a given IP family (IPv4 or IPv6). --- packages/http-tracker-core/src/event.rs | 4 ++ packages/primitives/src/service_binding.rs | 43 ++++++++++++++++++++-- packages/udp-tracker-core/src/event.rs | 4 ++ packages/udp-tracker-server/src/event.rs | 4 ++ 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/packages/http-tracker-core/src/event.rs b/packages/http-tracker-core/src/event.rs index e3d37569d..5af88c927 100644 --- a/packages/http-tracker-core/src/event.rs +++ b/packages/http-tracker-core/src/event.rs @@ -90,6 +90,10 @@ impl From for LabelSet { label_name!("server_binding_address_ip_type"), LabelValue::new(&connection_context.server.service_binding.bind_address_ip_type().to_string()), ), + ( + label_name!("server_binding_address_ip_family"), + LabelValue::new(&connection_context.server.service_binding.bind_address_ip_family().to_string()), + ), ( label_name!("server_binding_port"), LabelValue::new(&connection_context.server.service_binding.bind_address().port().to_string()), diff --git a/packages/primitives/src/service_binding.rs b/packages/primitives/src/service_binding.rs index 72d5e7f2e..74ff58e66 100644 --- a/packages/primitives/src/service_binding.rs +++ b/packages/primitives/src/service_binding.rs @@ -1,5 +1,5 @@ use std::fmt; -use std::net::SocketAddr; +use std::net::{IpAddr, SocketAddr}; use serde::{Deserialize, Serialize}; use url::Url; @@ -40,11 +40,43 @@ pub enum IpType { impl fmt::Display for IpType { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let addr_type_str = match self { + let ip_type_str = match self { Self::Plain => "plain", Self::V4MappedV6 => "v4_mapped_v6", }; - write!(f, "{addr_type_str}") + write!(f, "{ip_type_str}") + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Hash)] +pub enum IpFamily { + // IPv4 + Inet, + // IPv6 + Inet6, +} + +impl fmt::Display for IpFamily { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let ip_family_str = match self { + Self::Inet => "inet", + Self::Inet6 => "inet6", + }; + write!(f, "{ip_family_str}") + } +} + +impl From for IpFamily { + fn from(ip: IpAddr) -> Self { + if ip.is_ipv4() { + return IpFamily::Inet; + } + + if ip.is_ipv6() { + return IpFamily::Inet6; + } + + panic!("Unsupported IP address type: {ip}"); } } @@ -128,6 +160,11 @@ impl ServiceBinding { IpType::Plain } + #[must_use] + pub fn bind_address_ip_family(&self) -> IpFamily { + self.bind_address.ip().into() + } + /// # Panics /// /// It never panics because the URL is always valid. diff --git a/packages/udp-tracker-core/src/event.rs b/packages/udp-tracker-core/src/event.rs index d354d3e7e..761b809d8 100644 --- a/packages/udp-tracker-core/src/event.rs +++ b/packages/udp-tracker-core/src/event.rs @@ -63,6 +63,10 @@ impl From for LabelSet { label_name!("server_binding_address_ip_type"), LabelValue::new(&connection_context.server_service_binding.bind_address_ip_type().to_string()), ), + ( + label_name!("server_binding_address_ip_family"), + LabelValue::new(&connection_context.server_service_binding.bind_address_ip_family().to_string()), + ), ( 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/event.rs b/packages/udp-tracker-server/src/event.rs index c3e736a53..5588a2b33 100644 --- a/packages/udp-tracker-server/src/event.rs +++ b/packages/udp-tracker-server/src/event.rs @@ -122,6 +122,10 @@ impl From for LabelSet { label_name!("server_binding_address_ip_type"), LabelValue::new(&connection_context.server_service_binding.bind_address_ip_type().to_string()), ), + ( + label_name!("server_binding_address_ip_family"), + LabelValue::new(&connection_context.server_service_binding.bind_address_ip_family().to_string()), + ), ( label_name!("server_binding_port"), LabelValue::new(&connection_context.server_service_binding.bind_address().port().to_string()), From 3f5216e382e40f2e65e8ca5d2ce40eb7ba4753aa Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 13 Jun 2025 11:21:05 +0100 Subject: [PATCH 7/8] fix: [#1446] clippy error --- packages/rest-tracker-api-core/src/statistics/services.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/rest-tracker-api-core/src/statistics/services.rs b/packages/rest-tracker-api-core/src/statistics/services.rs index 3cfd6653e..4ffecb690 100644 --- a/packages/rest-tracker-api-core/src/statistics/services.rs +++ b/packages/rest-tracker-api-core/src/statistics/services.rs @@ -164,14 +164,14 @@ async fn get_protocol_metrics_from_labeled_metrics( .value() as u64; /* - + todo: - Add a label for `server_binding_ip_family` with value `inet` (inet/inet6) - to all metrics containing an IP address. This will allow us to distinguish + to all metrics containing an IP address. This will allow us to distinguish between IPv4 and IPv6 metrics. - Continue replacing the other metrics with the labeled metrics. - + */ // For backward compatibility we keep the `tcp4_connections_handled` and From dcfb5d5d207b9fad0aceba9aa85c4497923cb33c Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 13 Jun 2025 11:51:53 +0100 Subject: [PATCH 8/8] refactor: [#1446] Calculate global metrics from labeled metrics --- .../src/statistics/services.rs | 309 +++++++++++++++--- .../udp-tracker-server/src/statistics/mod.rs | 16 +- 2 files changed, 274 insertions(+), 51 deletions(-) diff --git a/packages/rest-tracker-api-core/src/statistics/services.rs b/packages/rest-tracker-api-core/src/statistics/services.rs index 4ffecb690..66bacbb06 100644 --- a/packages/rest-tracker-api-core/src/statistics/services.rs +++ b/packages/rest-tracker-api-core/src/statistics/services.rs @@ -5,10 +5,16 @@ use bittorrent_tracker_core::torrent::repository::in_memory::InMemoryTorrentRepo use bittorrent_udp_tracker_core::services::banning::BanService; use bittorrent_udp_tracker_core::{self}; use tokio::sync::RwLock; +use torrust_tracker_metrics::label::LabelSet; use torrust_tracker_metrics::metric_collection::aggregate::Sum; use torrust_tracker_metrics::metric_collection::MetricCollection; use torrust_tracker_metrics::metric_name; -use torrust_udp_tracker_server::statistics::{self as udp_server_statistics, UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL}; +use torrust_udp_tracker_server::statistics::{ + self as udp_server_statistics, UDP_TRACKER_SERVER_ERRORS_TOTAL, UDP_TRACKER_SERVER_IPS_BANNED_TOTAL, + UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS, UDP_TRACKER_SERVER_REQUESTS_ABORTED_TOTAL, + UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL, UDP_TRACKER_SERVER_REQUESTS_BANNED_TOTAL, + UDP_TRACKER_SERVER_REQUESTS_RECEIVED_TOTAL, UDP_TRACKER_SERVER_RESPONSES_SENT_TOTAL, +}; use super::metrics::TorrentsMetrics; use crate::statistics::metrics::ProtocolMetrics; @@ -42,12 +48,8 @@ pub async fn get_metrics( ) .await; - let protocol_metrics_from_labeled_metrics = get_protocol_metrics_from_labeled_metrics( - ban_service.clone(), - http_stats_repository.clone(), - udp_server_stats_repository.clone(), - ) - .await; + let protocol_metrics_from_labeled_metrics = + get_protocol_metrics_from_labeled_metrics(http_stats_repository.clone(), udp_server_stats_repository.clone()).await; // todo: // We keep both metrics until we deploy to production and we can @@ -58,9 +60,9 @@ pub async fn get_metrics( let protocol_metrics = if protocol_metrics_from_global_metrics == protocol_metrics_from_labeled_metrics { protocol_metrics_from_labeled_metrics } else { - // tracing::warn!("The protocol metrics from global metrics and labeled metrics are different"); - // tracing::warn!("Global metrics: {:?}", protocol_metrics_from_global_metrics); - // tracing::warn!("Labeled metrics: {:?}", protocol_metrics_from_labeled_metrics); + tracing::warn!("The protocol metrics from global metrics and labeled metrics are different"); + tracing::warn!("Global metrics: {:?}", protocol_metrics_from_global_metrics); + tracing::warn!("Labeled metrics: {:?}", protocol_metrics_from_labeled_metrics); protocol_metrics_from_global_metrics }; @@ -132,22 +134,153 @@ async fn get_protocol_metrics( } #[allow(deprecated)] +#[allow(clippy::too_many_lines)] async fn get_protocol_metrics_from_labeled_metrics( - ban_service: Arc>, http_stats_repository: Arc, udp_server_stats_repository: Arc, ) -> ProtocolMetrics { - let udp_banned_ips_total = ban_service.read().await.get_banned_ips_total(); let http_stats = http_stats_repository.get_stats().await; let udp_server_stats = udp_server_stats_repository.get_stats().await; + /* + + todo: We have to delete the global metrics from Metric types: + + - bittorrent_http_tracker_core::statistics::metrics::Metrics + - bittorrent_udp_tracker_core::statistics::metrics::Metrics + - torrust_udp_tracker_server::statistics::metrics::Metrics + + Internally only the labeled metrics should be used. + + */ + + // TCPv4 + #[allow(clippy::cast_sign_loss)] #[allow(clippy::cast_possible_truncation)] let tcp4_announces_handled = http_stats .metric_collection .sum( &metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), - &[("request_kind", "announce")].into(), // todo: add label for `server_binding_ip_family` with value `inet` (inet/inet6) + &[("server_binding_address_ip_family", "inet"), ("request_kind", "announce")].into(), + ) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let tcp4_scrapes_handled = http_stats + .metric_collection + .sum( + &metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), + &[("server_binding_address_ip_family", "inet"), ("request_kind", "scrape")].into(), + ) + .unwrap_or_default() + .value() as u64; + + // TCPv6 + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let tcp6_announces_handled = http_stats + .metric_collection + .sum( + &metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), + &[("server_binding_address_ip_family", "inet6"), ("request_kind", "announce")].into(), + ) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let tcp6_scrapes_handled = http_stats + .metric_collection + .sum( + &metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), + &[("server_binding_address_ip_family", "inet6"), ("request_kind", "scrape")].into(), + ) + .unwrap_or_default() + .value() as u64; + + // UDP + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp_requests_aborted = udp_server_stats + .metric_collection + .sum(&metric_name!(UDP_TRACKER_SERVER_REQUESTS_ABORTED_TOTAL), &LabelSet::empty()) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp_requests_banned = udp_server_stats + .metric_collection + .sum(&metric_name!(UDP_TRACKER_SERVER_REQUESTS_BANNED_TOTAL), &LabelSet::empty()) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp_banned_ips_total = udp_server_stats + .metric_collection + .sum(&metric_name!(UDP_TRACKER_SERVER_IPS_BANNED_TOTAL), &LabelSet::empty()) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp_avg_connect_processing_time_ns = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS), + &[("request_kind", "connect")].into(), + ) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp_avg_announce_processing_time_ns = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS), + &[("request_kind", "announce")].into(), + ) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp_avg_scrape_processing_time_ns = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS), + &[("request_kind", "scrape")].into(), + ) + .unwrap_or_default() + .value() as u64; + + // UDPv4 + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp4_requests = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_RECEIVED_TOTAL), + &[("server_binding_address_ip_family", "inet")].into(), + ) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp4_connections_handled = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL), + &[("server_binding_address_ip_family", "inet"), ("request_kind", "connect")].into(), ) .unwrap_or_default() .value() as u64; @@ -158,21 +291,111 @@ async fn get_protocol_metrics_from_labeled_metrics( .metric_collection .sum( &metric_name!(UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL), - &[("request_kind", "announce")].into(), // todo: add label for `server_binding_ip_family` with value `inet` (inet/inet6) + &[("server_binding_address_ip_family", "inet"), ("request_kind", "announce")].into(), ) .unwrap_or_default() .value() as u64; - /* + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp4_scrapes_handled = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL), + &[("server_binding_address_ip_family", "inet"), ("request_kind", "scrape")].into(), + ) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp4_responses = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_RESPONSES_SENT_TOTAL), + &[("server_binding_address_ip_family", "inet")].into(), + ) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp4_errors_handled = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_ERRORS_TOTAL), + &[("server_binding_address_ip_family", "inet")].into(), + ) + .unwrap_or_default() + .value() as u64; - todo: + // UDPv6 - - Add a label for `server_binding_ip_family` with value `inet` (inet/inet6) - to all metrics containing an IP address. This will allow us to distinguish - between IPv4 and IPv6 metrics. - - Continue replacing the other metrics with the labeled metrics. + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp6_requests = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_RECEIVED_TOTAL), + &[("server_binding_address_ip_family", "inet6")].into(), + ) + .unwrap_or_default() + .value() as u64; - */ + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp6_connections_handled = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL), + &[("server_binding_address_ip_family", "inet6"), ("request_kind", "connect")].into(), + ) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp6_announces_handled = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL), + &[("server_binding_address_ip_family", "inet6"), ("request_kind", "announce")].into(), + ) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp6_scrapes_handled = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL), + &[("server_binding_address_ip_family", "inet6"), ("request_kind", "scrape")].into(), + ) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp6_responses = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_RESPONSES_SENT_TOTAL), + &[("server_binding_address_ip_family", "inet6")].into(), + ) + .unwrap_or_default() + .value() as u64; + + #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_possible_truncation)] + let udp6_errors_handled = udp_server_stats + .metric_collection + .sum( + &metric_name!(UDP_TRACKER_SERVER_ERRORS_TOTAL), + &[("server_binding_address_ip_family", "inet6")].into(), + ) + .unwrap_or_default() + .value() as u64; // For backward compatibility we keep the `tcp4_connections_handled` and // `tcp6_connections_handled` metrics. They don't make sense for the HTTP @@ -181,34 +404,34 @@ async fn get_protocol_metrics_from_labeled_metrics( ProtocolMetrics { // TCPv4 - tcp4_connections_handled: tcp4_announces_handled + http_stats.tcp4_scrapes_handled, + tcp4_connections_handled: tcp4_announces_handled + tcp4_scrapes_handled, tcp4_announces_handled, - tcp4_scrapes_handled: http_stats.tcp4_scrapes_handled, + tcp4_scrapes_handled, // TCPv6 - tcp6_connections_handled: http_stats.tcp6_announces_handled + http_stats.tcp6_scrapes_handled, - tcp6_announces_handled: http_stats.tcp6_announces_handled, - tcp6_scrapes_handled: http_stats.tcp6_scrapes_handled, + tcp6_connections_handled: tcp6_announces_handled + tcp6_scrapes_handled, + tcp6_announces_handled, + tcp6_scrapes_handled, // UDP - udp_requests_aborted: udp_server_stats.udp_requests_aborted, - udp_requests_banned: udp_server_stats.udp_requests_banned, - udp_banned_ips_total: udp_banned_ips_total as u64, - udp_avg_connect_processing_time_ns: udp_server_stats.udp_avg_connect_processing_time_ns, - udp_avg_announce_processing_time_ns: udp_server_stats.udp_avg_announce_processing_time_ns, - udp_avg_scrape_processing_time_ns: udp_server_stats.udp_avg_scrape_processing_time_ns, + udp_requests_aborted, + udp_requests_banned, + udp_banned_ips_total, + udp_avg_connect_processing_time_ns, + udp_avg_announce_processing_time_ns, + udp_avg_scrape_processing_time_ns, // UDPv4 - udp4_requests: udp_server_stats.udp4_requests, - udp4_connections_handled: udp_server_stats.udp4_connections_handled, + udp4_requests, + udp4_connections_handled, udp4_announces_handled, - udp4_scrapes_handled: udp_server_stats.udp4_scrapes_handled, - udp4_responses: udp_server_stats.udp4_responses, - udp4_errors_handled: udp_server_stats.udp4_errors_handled, + udp4_scrapes_handled, + udp4_responses, + udp4_errors_handled, // UDPv6 - udp6_requests: udp_server_stats.udp6_requests, - udp6_connections_handled: udp_server_stats.udp6_connections_handled, - udp6_announces_handled: udp_server_stats.udp6_announces_handled, - udp6_scrapes_handled: udp_server_stats.udp6_scrapes_handled, - udp6_responses: udp_server_stats.udp6_responses, - udp6_errors_handled: udp_server_stats.udp6_errors_handled, + udp6_requests, + udp6_connections_handled, + udp6_announces_handled, + udp6_scrapes_handled, + udp6_responses, + udp6_errors_handled, } } diff --git a/packages/udp-tracker-server/src/statistics/mod.rs b/packages/udp-tracker-server/src/statistics/mod.rs index 3a25fd51d..b42a73f27 100644 --- a/packages/udp-tracker-server/src/statistics/mod.rs +++ b/packages/udp-tracker-server/src/statistics/mod.rs @@ -8,15 +8,15 @@ use torrust_tracker_metrics::metric::description::MetricDescription; 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"; -const UDP_TRACKER_SERVER_REQUESTS_BANNED_TOTAL: &str = "udp_tracker_server_requests_banned_total"; -pub(crate) const UDP_TRACKER_SERVER_IPS_BANNED_TOTAL: &str = "udp_tracker_server_ips_banned_total"; -const UDP_TRACKER_SERVER_CONNECTION_ID_ERRORS_TOTAL: &str = "udp_tracker_server_connection_id_errors_total"; -const UDP_TRACKER_SERVER_REQUESTS_RECEIVED_TOTAL: &str = "udp_tracker_server_requests_received_total"; +pub const UDP_TRACKER_SERVER_REQUESTS_ABORTED_TOTAL: &str = "udp_tracker_server_requests_aborted_total"; +pub const UDP_TRACKER_SERVER_REQUESTS_BANNED_TOTAL: &str = "udp_tracker_server_requests_banned_total"; +pub const UDP_TRACKER_SERVER_IPS_BANNED_TOTAL: &str = "udp_tracker_server_ips_banned_total"; +pub const UDP_TRACKER_SERVER_CONNECTION_ID_ERRORS_TOTAL: &str = "udp_tracker_server_connection_id_errors_total"; +pub const UDP_TRACKER_SERVER_REQUESTS_RECEIVED_TOTAL: &str = "udp_tracker_server_requests_received_total"; pub const UDP_TRACKER_SERVER_REQUESTS_ACCEPTED_TOTAL: &str = "udp_tracker_server_requests_accepted_total"; -const UDP_TRACKER_SERVER_RESPONSES_SENT_TOTAL: &str = "udp_tracker_server_responses_sent_total"; -const UDP_TRACKER_SERVER_ERRORS_TOTAL: &str = "udp_tracker_server_errors_total"; -const UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS: &str = "udp_tracker_server_performance_avg_processing_time_ns"; +pub const UDP_TRACKER_SERVER_RESPONSES_SENT_TOTAL: &str = "udp_tracker_server_responses_sent_total"; +pub const UDP_TRACKER_SERVER_ERRORS_TOTAL: &str = "udp_tracker_server_errors_total"; +pub const UDP_TRACKER_SERVER_PERFORMANCE_AVG_PROCESSING_TIME_NS: &str = "udp_tracker_server_performance_avg_processing_time_ns"; #[must_use] pub fn describe_metrics() -> Metrics {