refactor(metrics): [#1580] remove AggregateValue wrapper, return primitive types from aggregates

josecelano · josecelano · commit 00ac210a9025 · 2025-06-17T08:40:03.000+01:00
- Remove AggregateValue struct and its entire module from metrics package
- Simplify Sum trait in metric collections to return Option&lt;f64&gt; directly
- Update MetricKindCollection implementations to cast counter values to f64
- Remove AggregateValue dependencies from http-tracker-core, udp-tracker-core, and udp-tracker-server
- Eliminate unnecessary wrapper overhead in aggregate operations
- Maintain backward compatibility by converting all aggregate results to f64

This change completes the metrics package refactoring by removing the generic
AggregateValue wrapper that added no value when aggregate functions can return
mathematically appropriate primitive types directly.
diff --git a/packages/http-tracker-core/src/statistics/metrics.rs b/packages/http-tracker-core/src/statistics/metrics.rs
@@ -53,8 +53,7 @@ impl Metrics {
                 &metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
                 &[("server_binding_address_ip_family", "inet"), ("request_kind", "announce")].into(),
             )
-            .unwrap_or_default()
-            .value() as u64
+            .unwrap_or_default() as u64
     }
 
     /// Total number of TCP (HTTP tracker) `scrape` requests from IPv4 peers.
@@ -67,8 +66,7 @@ impl Metrics {
                 &metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
                 &[("server_binding_address_ip_family", "inet"), ("request_kind", "scrape")].into(),
             )
-            .unwrap_or_default()
-            .value() as u64
+            .unwrap_or_default() as u64
     }
 
     /// Total number of TCP (HTTP tracker) `announce` requests from IPv6 peers.
@@ -81,8 +79,7 @@ impl Metrics {
                 &metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
                 &[("server_binding_address_ip_family", "inet6"), ("request_kind", "announce")].into(),
             )
-            .unwrap_or_default()
-            .value() as u64
+            .unwrap_or_default() as u64
     }
 
     /// Total number of TCP (HTTP tracker) `scrape` requests from IPv6 peers.
@@ -95,7 +92,6 @@ impl Metrics {
                 &metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
                 &[("server_binding_address_ip_family", "inet6"), ("request_kind", "scrape")].into(),
             )
-            .unwrap_or_default()
-            .value() as u64
+            .unwrap_or_default() as u64
     }
 }
diff --git a/packages/metrics/src/aggregate.rs b/packages/metrics/src/aggregate.rs
diff --git a/packages/metrics/src/lib.rs b/packages/metrics/src/lib.rs
@@ -1,4 +1,3 @@
-pub mod aggregate;
 pub mod counter;
 pub mod gauge;
 pub mod label;
diff --git a/packages/metrics/src/metric_collection/aggregate.rs b/packages/metrics/src/metric_collection/aggregate.rs
@@ -1,4 +1,3 @@
-use crate::aggregate::AggregateValue;
 use crate::counter::Counter;
 use crate::gauge::Gauge;
 use crate::label::LabelSet;
@@ -7,39 +6,34 @@ use crate::metric::MetricName;
 use crate::metric_collection::{MetricCollection, MetricKindCollection};
 
 pub trait Sum {
-    type Output;
-    fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Self::Output;
+    fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Option<f64>;
 }
 
 impl Sum for MetricCollection {
-    type Output = Option<AggregateValue>;
-
-    fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Self::Output {
+    fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Option<f64> {
         if let Some(value) = self.counters.sum(metric_name, label_set_criteria) {
-            #[allow(clippy::cast_precision_loss)]
-            return Some(AggregateValue::new(value as f64));
+            return Some(value);
         }
 
         if let Some(value) = self.gauges.sum(metric_name, label_set_criteria) {
-            return Some(AggregateValue::new(value));
+            return Some(value);
         }
 
         None
     }
 }
 
 impl Sum for MetricKindCollection<Counter> {
-    type Output = Option<u64>;
-
-    fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Self::Output {
-        self.metrics.get(metric_name).map(|metric| metric.sum(label_set_criteria))
+    fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Option<f64> {
+        #[allow(clippy::cast_precision_loss)]
+        self.metrics
+            .get(metric_name)
+            .map(|metric| metric.sum(label_set_criteria) as f64)
     }
 }
 
 impl Sum for MetricKindCollection<Gauge> {
-    type Output = Option<f64>;
-
-    fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Self::Output {
+    fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Option<f64> {
         self.metrics.get(metric_name).map(|metric| metric.sum(label_set_criteria))
     }
 }
@@ -81,10 +75,10 @@ mod tests {
                 )
                 .unwrap();
 
-            assert_eq!(collection.sum(&metric_name, &LabelSet::empty()), Some(2.0.into()));
+            assert_eq!(collection.sum(&metric_name, &LabelSet::empty()), Some(2.0));
             assert_eq!(
                 collection.sum(&metric_name, &(label_name!("label_1"), LabelValue::new("value_1")).into()),
-                Some(1.0.into())
+                Some(1.0)
             );
         }
 
@@ -114,10 +108,10 @@ mod tests {
                 )
                 .unwrap();
 
-            assert_eq!(collection.sum(&metric_name, &LabelSet::empty()), Some(2.0.into()));
+            assert_eq!(collection.sum(&metric_name, &LabelSet::empty()), Some(2.0));
             assert_eq!(
                 collection.sum(&metric_name, &(label_name!("label_1"), LabelValue::new("value_1")).into()),
-                Some(1.0.into())
+                Some(1.0)
             );
         }
     }
diff --git a/packages/udp-tracker-core/src/statistics/metrics.rs b/packages/udp-tracker-core/src/statistics/metrics.rs
@@ -54,8 +54,7 @@ impl Metrics {
                 &metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
                 &[("server_binding_address_ip_family", "inet"), ("request_kind", "connect")].into(),
             )
-            .unwrap_or_default()
-            .value() as u64
+            .unwrap_or_default() as u64
     }
 
     /// Total number of UDP (UDP tracker) `announce` requests from IPv4 peers.
@@ -68,8 +67,7 @@ impl Metrics {
                 &metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
                 &[("server_binding_address_ip_family", "inet"), ("request_kind", "announce")].into(),
             )
-            .unwrap_or_default()
-            .value() as u64
+            .unwrap_or_default() as u64
     }
 
     /// Total number of UDP (UDP tracker) `scrape` requests from IPv4 peers.
@@ -82,8 +80,7 @@ impl Metrics {
                 &metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
                 &[("server_binding_address_ip_family", "inet"), ("request_kind", "scrape")].into(),
             )
-            .unwrap_or_default()
-            .value() as u64
+            .unwrap_or_default() as u64
     }
 
     /// Total number of UDP (UDP tracker) `connection` requests from IPv6 peers.
@@ -96,8 +93,7 @@ impl Metrics {
                 &metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
                 &[("server_binding_address_ip_family", "inet6"), ("request_kind", "connect")].into(),
             )
-            .unwrap_or_default()
-            .value() as u64
+            .unwrap_or_default() as u64
     }
 
     /// Total number of UDP (UDP tracker) `announce` requests from IPv6 peers.
@@ -110,8 +106,7 @@ impl Metrics {
                 &metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
                 &[("server_binding_address_ip_family", "inet6"), ("request_kind", "announce")].into(),
             )
-            .unwrap_or_default()
-            .value() as u64
+            .unwrap_or_default() as u64
     }
 
     /// Total number of UDP (UDP tracker) `scrape` requests from IPv6 peers.
@@ -124,7 +119,6 @@ impl Metrics {
                 &metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
                 &[("server_binding_address_ip_family", "inet6"), ("request_kind", "scrape")].into(),
             )
-            .unwrap_or_default()
-            .value() as u64
+            .unwrap_or_default() as u64
     }
 }
diff --git a/packages/udp-tracker-server/src/statistics/metrics.rs b/packages/udp-tracker-server/src/statistics/metrics.rs

Original file line number	Diff line number	Diff line change
`@@ -53,8 +53,7 @@ impl Metrics {`
`53`	`53`	`&metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),`
`54`	`54`	`&[("server_binding_address_ip_family", "inet"), ("request_kind", "announce")].into(),`
`55`	`55`	`)`
`56`		`- .unwrap_or_default()`
`57`		`- .value() as u64`
	`56`	`+ .unwrap_or_default() as u64`
`58`	`57`	`}`
`59`	`58`
`60`	`59`	/// Total number of TCP (HTTP tracker) `scrape` requests from IPv4 peers.
`@@ -67,8 +66,7 @@ impl Metrics {`
`67`	`66`	`&metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),`
`68`	`67`	`&[("server_binding_address_ip_family", "inet"), ("request_kind", "scrape")].into(),`
`69`	`68`	`)`
`70`		`- .unwrap_or_default()`
`71`		`- .value() as u64`
	`69`	`+ .unwrap_or_default() as u64`
`72`	`70`	`}`
`73`	`71`
`74`	`72`	/// Total number of TCP (HTTP tracker) `announce` requests from IPv6 peers.
`@@ -81,8 +79,7 @@ impl Metrics {`
`81`	`79`	`&metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),`
`82`	`80`	`&[("server_binding_address_ip_family", "inet6"), ("request_kind", "announce")].into(),`
`83`	`81`	`)`
`84`		`- .unwrap_or_default()`
`85`		`- .value() as u64`
	`82`	`+ .unwrap_or_default() as u64`
`86`	`83`	`}`
`87`	`84`
`88`	`85`	/// Total number of TCP (HTTP tracker) `scrape` requests from IPv6 peers.
`@@ -95,7 +92,6 @@ impl Metrics {`
`95`	`92`	`&metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),`
`96`	`93`	`&[("server_binding_address_ip_family", "inet6"), ("request_kind", "scrape")].into(),`
`97`	`94`	`)`
`98`		`- .unwrap_or_default()`
`99`		`- .value() as u64`
	`95`	`+ .unwrap_or_default() as u64`
`100`	`96`	`}`
`101`	`97`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-pub mod aggregate;`
`2`	`1`	`pub mod counter;`
`3`	`2`	`pub mod gauge;`
`4`	`3`	`pub mod label;`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-use crate::aggregate::AggregateValue;`
`2`	`1`	`use crate::counter::Counter;`
`3`	`2`	`use crate::gauge::Gauge;`
`4`	`3`	`use crate::label::LabelSet;`
`@@ -7,39 +6,34 @@ use crate::metric::MetricName;`
`7`	`6`	`use crate::metric_collection::{MetricCollection, MetricKindCollection};`
`8`	`7`
`9`	`8`	`pub trait Sum {`
`10`		`- type Output;`
`11`		`- fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Self::Output;`
	`9`	`+ fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Option<f64>;`
`12`	`10`	`}`
`13`	`11`
`14`	`12`	`impl Sum for MetricCollection {`
`15`		`- type Output = Option<AggregateValue>;`
`16`		`-`
`17`		`- fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Self::Output {`
	`13`	`+ fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Option<f64> {`
`18`	`14`	`if let Some(value) = self.counters.sum(metric_name, label_set_criteria) {`
`19`		`- #[allow(clippy::cast_precision_loss)]`
`20`		`- return Some(AggregateValue::new(value as f64));`
	`15`	`+ return Some(value);`
`21`	`16`	`}`
`22`	`17`
`23`	`18`	`if let Some(value) = self.gauges.sum(metric_name, label_set_criteria) {`
`24`		`- return Some(AggregateValue::new(value));`
	`19`	`+ return Some(value);`
`25`	`20`	`}`
`26`	`21`
`27`	`22`	`None`
`28`	`23`	`}`
`29`	`24`	`}`
`30`	`25`
`31`	`26`	`impl Sum for MetricKindCollection<Counter> {`
`32`		`- type Output = Option<u64>;`
`33`		`-`
`34`		`- fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Self::Output {`
`35`		`- self.metrics.get(metric_name).map(\|metric\| metric.sum(label_set_criteria))`
	`27`	`+ fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Option<f64> {`
	`28`	`+ #[allow(clippy::cast_precision_loss)]`
	`29`	`+ self.metrics`
	`30`	`+ .get(metric_name)`
	`31`	`+ .map(\|metric\| metric.sum(label_set_criteria) as f64)`
`36`	`32`	`}`
`37`	`33`	`}`
`38`	`34`
`39`	`35`	`impl Sum for MetricKindCollection<Gauge> {`
`40`		`- type Output = Option<f64>;`
`41`		`-`
`42`		`- fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Self::Output {`
	`36`	`+ fn sum(&self, metric_name: &MetricName, label_set_criteria: &LabelSet) -> Option<f64> {`
`43`	`37`	`self.metrics.get(metric_name).map(\|metric\| metric.sum(label_set_criteria))`
`44`	`38`	`}`
`45`	`39`	`}`
`@@ -81,10 +75,10 @@ mod tests {`
`81`	`75`	`)`
`82`	`76`	`.unwrap();`
`83`	`77`
`84`		`- assert_eq!(collection.sum(&metric_name, &LabelSet::empty()), Some(2.0.into()));`
	`78`	`+ assert_eq!(collection.sum(&metric_name, &LabelSet::empty()), Some(2.0));`
`85`	`79`	`assert_eq!(`
`86`	`80`	`collection.sum(&metric_name, &(label_name!("label_1"), LabelValue::new("value_1")).into()),`
`87`		`- Some(1.0.into())`
	`81`	`+ Some(1.0)`
`88`	`82`	`);`
`89`	`83`	`}`
`90`	`84`
`@@ -114,10 +108,10 @@ mod tests {`
`114`	`108`	`)`
`115`	`109`	`.unwrap();`
`116`	`110`
`117`		`- assert_eq!(collection.sum(&metric_name, &LabelSet::empty()), Some(2.0.into()));`
	`111`	`+ assert_eq!(collection.sum(&metric_name, &LabelSet::empty()), Some(2.0));`
`118`	`112`	`assert_eq!(`
`119`	`113`	`collection.sum(&metric_name, &(label_name!("label_1"), LabelValue::new("value_1")).into()),`
`120`		`- Some(1.0.into())`
	`114`	`+ Some(1.0)`
`121`	`115`	`);`
`122`	`116`	`}`
`123`	`117`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,8 +54,7 @@ impl Metrics {`
`54`	`54`	`&metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),`
`55`	`55`	`&[("server_binding_address_ip_family", "inet"), ("request_kind", "connect")].into(),`
`56`	`56`	`)`
`57`		`- .unwrap_or_default()`
`58`		`- .value() as u64`
	`57`	`+ .unwrap_or_default() as u64`
`59`	`58`	`}`
`60`	`59`
`61`	`60`	/// Total number of UDP (UDP tracker) `announce` requests from IPv4 peers.
`@@ -68,8 +67,7 @@ impl Metrics {`
`68`	`67`	`&metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),`
`69`	`68`	`&[("server_binding_address_ip_family", "inet"), ("request_kind", "announce")].into(),`
`70`	`69`	`)`
`71`		`- .unwrap_or_default()`
`72`		`- .value() as u64`
	`70`	`+ .unwrap_or_default() as u64`
`73`	`71`	`}`
`74`	`72`
`75`	`73`	/// Total number of UDP (UDP tracker) `scrape` requests from IPv4 peers.
`@@ -82,8 +80,7 @@ impl Metrics {`
`82`	`80`	`&metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),`
`83`	`81`	`&[("server_binding_address_ip_family", "inet"), ("request_kind", "scrape")].into(),`
`84`	`82`	`)`
`85`		`- .unwrap_or_default()`
`86`		`- .value() as u64`
	`83`	`+ .unwrap_or_default() as u64`
`87`	`84`	`}`
`88`	`85`
`89`	`86`	/// Total number of UDP (UDP tracker) `connection` requests from IPv6 peers.
`@@ -96,8 +93,7 @@ impl Metrics {`
`96`	`93`	`&metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),`
`97`	`94`	`&[("server_binding_address_ip_family", "inet6"), ("request_kind", "connect")].into(),`
`98`	`95`	`)`
`99`		`- .unwrap_or_default()`
`100`		`- .value() as u64`
	`96`	`+ .unwrap_or_default() as u64`
`101`	`97`	`}`
`102`	`98`
`103`	`99`	/// Total number of UDP (UDP tracker) `announce` requests from IPv6 peers.
`@@ -110,8 +106,7 @@ impl Metrics {`
`110`	`106`	`&metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),`
`111`	`107`	`&[("server_binding_address_ip_family", "inet6"), ("request_kind", "announce")].into(),`
`112`	`108`	`)`
`113`		`- .unwrap_or_default()`
`114`		`- .value() as u64`
	`109`	`+ .unwrap_or_default() as u64`
`115`	`110`	`}`
`116`	`111`
`117`	`112`	/// Total number of UDP (UDP tracker) `scrape` requests from IPv6 peers.
`@@ -124,7 +119,6 @@ impl Metrics {`
`124`	`119`	`&metric_name!(UDP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),`
`125`	`120`	`&[("server_binding_address_ip_family", "inet6"), ("request_kind", "scrape")].into(),`
`126`	`121`	`)`
`127`		`- .unwrap_or_default()`
`128`		`- .value() as u64`
	`122`	`+ .unwrap_or_default() as u64`
`129`	`123`	`}`
`130`	`124`	`}`