Merge #1467: Overhaul stats: Refactor metrics (part 1)

f9ad729 feat: [#1445] add logs for metrics initialization (Jose Celano)
ad782eb refactor: [#1445] rename metric label (Jose Celano)
482a1be feat: [#1445] add request kind label to total errors metric (Jose Celano)
13ea091 feat: [#1445] add logs to the event listener's initialization (Jose Celano)
2ccb247 refactor: [#1445] return errors instead of panicking in the SampleCollection struct (Jose Celano)
42e1524 refactor: [#1445] return errors instead of panicking in the MetricCollection struct (Jose Celano)
785a978 refactor: [#1445] remove panic from MetricColelction::new method (Jose Celano)
d263be7 refactor: [#1445] return optionals for metric values in metric collection (Jose Celano)
4d68267 refactor: [#1445] replace metric label name constructor by macro (Jose Celano)
7a24f85 refactor: [#1445] new macro to create metric label names (Jose Celano)
5497970 refactor: [#1445] replace metric name constructor by macro (Jose Celano)
fc14a81 refactor: [#1445] new macro to create metric names (Jose Celano)

Pull request description:

  Refactor metrics (part 1)

  ### Subtasks

  - [x] Extract macro for metric name constructor (to fail at compilation time with empty names)
  - [x] Replace metric name constructor by new macro
  - [x] Extract macro for label name constructor (to fail at compilation time with empty names)
  - [x] Replace label name constructor by new macro
  - [x] Return an optional in `get_counter_value` and `get_gauge_value`.
  - [x] Return errors instead of panicking in the `MetricCollection` struct.
  - [x] Add logs to the listener's initialization (when it starts and finishes).
  - [x] Add req kind label to the number of errors metric.
  - [x] Add `tracing` crate.

ACKs for top commit:
  josecelano:
    ACK f9ad729

Tree-SHA512: 38abbc070a7f3aa0134295667430fa8d7ce90294ae872699786f973d8c790c908a00ed4c5506253dbccd7ae5990c76fd92d187ff935a0806e84909d2e7b64b50

Author: josecelano

Cargo.lock

@@ -1,6 +1,7 @@
 use std::net::{IpAddr, SocketAddr};
 
-use torrust_tracker_metrics::label::{LabelName, LabelSet, LabelValue};
+use torrust_tracker_metrics::label::{LabelSet, LabelValue};
+use torrust_tracker_metrics::label_name;
 use torrust_tracker_primitives::service_binding::ServiceBinding;
 
 pub mod sender;
@@ -65,15 +66,15 @@ impl From<ConnectionContext> for LabelSet {
     fn from(connection_context: ConnectionContext) -> Self {
         LabelSet::from([
             (
-                LabelName::new("server_binding_protocol"),
+                label_name!("server_binding_protocol"),
                 LabelValue::new(&connection_context.server.service_binding.protocol().to_string()),
             ),
             (
-                LabelName::new("server_binding_ip"),
+                label_name!("server_binding_ip"),
                 LabelValue::new(&connection_context.server.service_binding.bind_address().ip().to_string()),
             ),
             (
-                LabelName::new("server_binding_port"),
+                label_name!("server_binding_port"),
                 LabelValue::new(&connection_context.server.service_binding.bind_address().port().to_string()),
             ),
         ])

packages/http-tracker-core/src/event/mod.rs

@@ -1,7 +1,7 @@
 use std::net::IpAddr;
 
-use torrust_tracker_metrics::label::{LabelName, LabelSet, LabelValue};
-use torrust_tracker_metrics::metric::MetricName;
+use torrust_tracker_metrics::label::{LabelSet, LabelValue};
+use torrust_tracker_metrics::{label_name, metric_name};
 use torrust_tracker_primitives::DurationSinceUnixEpoch;
 
 use crate::event::Event;
@@ -29,11 +29,15 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura
             // Extendable metrics
 
             let mut label_set = LabelSet::from(connection);
-            label_set.upsert(LabelName::new("request_kind"), LabelValue::new("announce"));
-
-            stats_repository
-                .increase_counter(&MetricName::new(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now)
-                .await;
+            label_set.upsert(label_name!("request_kind"), LabelValue::new("announce"));
+
+            match stats_repository
+                .increase_counter(&metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now)
+                .await
+            {
+                Ok(()) => {}
+                Err(err) => tracing::error!("Failed to increase the counter: {}", err),
+            };
         }
         Event::TcpScrape { connection } => {
             // Global fixed metrics
@@ -50,11 +54,15 @@ pub async fn handle_event(event: Event, stats_repository: &Repository, now: Dura
             // Extendable metrics
 
             let mut label_set = LabelSet::from(connection);
-            label_set.upsert(LabelName::new("request_kind"), LabelValue::new("scrape"));
-
-            stats_repository
-                .increase_counter(&MetricName::new(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now)
-                .await;
+            label_set.upsert(label_name!("request_kind"), LabelValue::new("scrape"));
+
+            match stats_repository
+                .increase_counter(&metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL), &label_set, now)
+                .await
+            {
+                Ok(()) => {}
+                Err(err) => tracing::error!("Failed to increase the counter: {}", err),
+            };
         }
     }
 

packages/http-tracker-core/src/statistics/event/handler.rs

@@ -3,6 +3,7 @@ use tokio::sync::broadcast::Receiver;
 use super::event::listener::dispatch_events;
 use super::repository::Repository;
 use crate::event::Event;
+use crate::HTTP_TRACKER_LOG_TARGET;
 
 /// The service responsible for keeping tracker metrics (listening to statistics events and handle them).
 ///
@@ -29,7 +30,13 @@ impl Keeper {
     pub fn run_event_listener(&mut self, receiver: Receiver<Event>) {
         let stats_repository = self.repository.clone();
 
-        tokio::spawn(async move { dispatch_events(receiver, stats_repository).await });
+        tracing::info!(target: HTTP_TRACKER_LOG_TARGET, "Starting HTTP tracker core event listener");
+
+        tokio::spawn(async move {
+            dispatch_events(receiver, stats_repository).await;
+
+            tracing::info!(target: HTTP_TRACKER_LOG_TARGET, "HTTP tracker core event listener finished");
+        });
     }
 }
 

packages/http-tracker-core/src/statistics/keeper.rs

@@ -1,7 +1,7 @@
 use serde::Serialize;
 use torrust_tracker_metrics::label::LabelSet;
 use torrust_tracker_metrics::metric::MetricName;
-use torrust_tracker_metrics::metric_collection::MetricCollection;
+use torrust_tracker_metrics::metric_collection::{Error, MetricCollection};
 use torrust_tracker_primitives::DurationSinceUnixEpoch;
 
 /// Metrics collected by the tracker.
@@ -24,11 +24,28 @@ pub struct Metrics {
 }
 
 impl Metrics {
-    pub fn increase_counter(&mut self, metric_name: &MetricName, labels: &LabelSet, now: DurationSinceUnixEpoch) {
-        self.metric_collection.increase_counter(metric_name, labels, now);
+    /// # Errors
+    ///
+    /// Returns an error if the metric does not exist and it cannot be created.
+    pub fn increase_counter(
+        &mut self,
+        metric_name: &MetricName,
+        labels: &LabelSet,
+        now: DurationSinceUnixEpoch,
+    ) -> Result<(), Error> {
+        self.metric_collection.increase_counter(metric_name, labels, now)
     }
 
-    pub fn set_gauge(&mut self, metric_name: &MetricName, labels: &LabelSet, value: f64, now: DurationSinceUnixEpoch) {
-        self.metric_collection.set_gauge(metric_name, labels, value, now);
+    /// # Errors
+    ///
+    /// Returns an error if the metric does not exist and it cannot be created.
+    pub fn set_gauge(
+        &mut self,
+        metric_name: &MetricName,
+        labels: &LabelSet,
+        value: f64,
+        now: DurationSinceUnixEpoch,
+    ) -> Result<(), Error> {
+        self.metric_collection.set_gauge(metric_name, labels, value, now)
     }
 }

packages/http-tracker-core/src/statistics/metrics.rs

@@ -7,7 +7,7 @@ pub mod setup;
 
 use metrics::Metrics;
 use torrust_tracker_metrics::metric::description::MetricDescription;
-use torrust_tracker_metrics::metric::MetricName;
+use torrust_tracker_metrics::metric_name;
 use torrust_tracker_metrics::unit::Unit;
 
 const HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL: &str = "http_tracker_core_requests_received_total";
@@ -17,7 +17,7 @@ pub fn describe_metrics() -> Metrics {
     let mut metrics = Metrics::default();
 
     metrics.metric_collection.describe_counter(
-        &MetricName::new(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
+        &metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
         Some(Unit::Count),
         Some(MetricDescription::new("Total number of HTTP requests received")),
     );

packages/http-tracker-core/src/statistics/mod.rs

@@ -3,6 +3,7 @@ use std::sync::Arc;
 use tokio::sync::{RwLock, RwLockReadGuard};
 use torrust_tracker_metrics::label::LabelSet;
 use torrust_tracker_metrics::metric::MetricName;
+use torrust_tracker_metrics::metric_collection::Error;
 use torrust_tracker_primitives::DurationSinceUnixEpoch;
 
 use super::describe_metrics;
@@ -56,9 +57,22 @@ impl Repository {
         drop(stats_lock);
     }
 
-    pub async fn increase_counter(&self, metric_name: &MetricName, labels: &LabelSet, now: DurationSinceUnixEpoch) {
+    /// # Errors
+    ///
+    /// This function will return an error if the metric collection fails to
+    /// increase the counter.
+    pub async fn increase_counter(
+        &self,
+        metric_name: &MetricName,
+        labels: &LabelSet,
+        now: DurationSinceUnixEpoch,
+    ) -> Result<(), Error> {
         let mut stats_lock = self.stats.write().await;
-        stats_lock.increase_counter(metric_name, labels, now);
+
+        let result = stats_lock.increase_counter(metric_name, labels, now);
+
         drop(stats_lock);
+
+        result
     }
 }

packages/http-tracker-core/src/statistics/repository.rs

@@ -21,6 +21,7 @@ serde = { version = "1", features = ["derive"] }
 serde_json = "1.0.140"
 thiserror = "2"
 torrust-tracker-primitives = { version = "3.0.0-develop", path = "../primitives" }
+tracing = "0.1.41"
 
 [dev-dependencies]
 approx = "0.5.1"

packages/metrics/Cargo.toml

@@ -1,7 +1,7 @@
-mod name;
+pub mod name;
 mod pair;
 mod set;
-mod value;
+pub mod value;
 
 pub type LabelName = name::LabelName;
 pub type LabelValue = value::LabelValue;

packages/metrics/src/label/mod.rs

@@ -14,11 +14,7 @@ impl LabelName {
     /// Panics if the provided name is empty.
     #[must_use]
     pub fn new(name: &str) -> Self {
-        assert!(
-            !name.is_empty(),
-            "Label name cannot be empty. It must have at least one character."
-        );
-
+        assert!(!name.is_empty(), "Label name cannot be empty.");
         Self(name.to_owned())
     }
 }
@@ -69,6 +65,19 @@ impl PrometheusSerializable for LabelName {
         }
     }
 }
+
+#[macro_export]
+macro_rules! label_name {
+    ("") => {
+        compile_error!("Label name cannot be empty");
+    };
+    ($name:literal) => {
+        $crate::label::name::LabelName::new($name)
+    };
+    ($name:ident) => {
+        $crate::label::name::LabelName::new($name)
+    };
+}
 #[cfg(test)]
 mod tests {
     mod serialization_of_label_name_to_prometheus {
@@ -83,7 +92,7 @@ mod tests {
         #[case("3 leading lowercase", "v123", "v123")]
         #[case("4 leading uppercase", "V123", "V123")]
         fn valid_names_in_prometheus(#[case] case: &str, #[case] input: &str, #[case] output: &str) {
-            assert_eq!(LabelName::new(input).to_prometheus(), output, "{case} failed: {input:?}");
+            assert_eq!(label_name!(input).to_prometheus(), output, "{case} failed: {input:?}");
         }
 
         #[rstest]
@@ -96,7 +105,7 @@ mod tests {
         #[case("7 all invalid characters", "!@#$%^&*()", "__________")]
         #[case("8 non_ascii_characters", "ñaca©", "_aca_")]
         fn names_that_need_changes_in_prometheus(#[case] case: &str, #[case] input: &str, #[case] output: &str) {
-            assert_eq!(LabelName::new(input).to_prometheus(), output, "{case} failed: {input:?}");
+            assert_eq!(label_name!(input).to_prometheus(), output, "{case} failed: {input:?}");
         }
 
         #[rstest]
@@ -105,11 +114,11 @@ mod tests {
         #[case("3 processed to double underscore", "^^name", "___name")]
         #[case("4 processed to double underscore after first char", "0__name", "___name")]
         fn names_starting_with_double_underscore(#[case] case: &str, #[case] input: &str, #[case] output: &str) {
-            assert_eq!(LabelName::new(input).to_prometheus(), output, "{case} failed: {input:?}");
+            assert_eq!(label_name!(input).to_prometheus(), output, "{case} failed: {input:?}");
         }
 
         #[test]
-        #[should_panic(expected = "Label name cannot be empty. It must have at least one character.")]
+        #[should_panic(expected = "Label name cannot be empty.")]
         fn empty_name() {
             let _name = LabelName::new("");
         }