Merge #1559: Overhaul stats: add TYPE and HELP to Prometheus export

bb2392d test: [#1514] add tests to metrics package (Jose Celano)
507b480 fix: [#1514] bug. Don't allow merge metric collections with the same metrin name (Jose Celano)
376f242 test: [#1514] add tests to metrics package (Jose Celano)
642d774 fix: [#1514] HELP line in Prometheus export must contain metric name (Jose Celano)
748e6a5 test: [#1514] add tests to metrics package (Jose Celano)
a89406d refactor: [#1514] remove duplicate code in Metric type (Jose Celano)
ed1322b refactor: [#1514] reorganize SampleCollection tests (Jose Celano)
842739f feat: [#1514] add HELP and TYPE to prometehous metric export (Jose Celano)
458497b feat: [#1514] add unit and description to metrics (Jose Celano)
031bf65 refactor: [#1514] remove unused code (Jose Celano)
2ee3111 refactor: [#1514] ensure_metric_exists method to pass the whole metric (Jose Celano)
552697b feat!: [#1514] rename ffield kind to type in JSON metrics (Jose Celano)

Pull request description:

  Add metadata describing the metric to the Prometheus export format. For example:

  ```
  # HELP udp_tracker_server_requests_received_total Total number of UDP requests received
  # TYPE udp_tracker_server_requests_received_total counter
  udp_tracker_server_requests_received_total{server_binding_ip="0.0.0.0",server_binding_port="6868",server_binding_protocol="udp"} 36661
  ```

  ### Subtasks

  - [x] Rename `kind` JSON field to `type` to use the same name in JSON and Prometheus export formats.
  - [x] Change `ensure_metric_exists` to pass the whole metric, not only the name.
  - [x] Add metadata fields to generic metric: `opt_unit: Option<Unit>` and `opt_description: Option<&MetricDescription>` to `Metric<T>`
  - [x] Change JSON serialization to include the metric description (`description`).
  - [x] Change Prometheus serialization to include the metric type (`TYPE`) and description (`HELP`).
  - [x] Review test coverage.

ACKs for top commit:
  josecelano:
    ACK bb2392d

Tree-SHA512: 8778d5b65b027a88441f4746926ade66a7959ca4818de1a52c54b018f302c125633491ca2e7f304ed429c501bee300cd5be2e69f19dd6b6866852050e0e4511f

Author: josecelano

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

@@ -17,7 +17,7 @@ pub fn describe_metrics() -> Metrics {
     metrics.metric_collection.describe_counter(
         &metric_name!(HTTP_TRACKER_CORE_REQUESTS_RECEIVED_TOTAL),
         Some(Unit::Count),
-        Some(&MetricDescription::new("Total number of HTTP requests received")),
+        Some(MetricDescription::new("Total number of HTTP requests received")),
     );
 
     metrics

packages/metrics/src/label/set.rs

@@ -175,6 +175,7 @@ impl PrometheusSerializable for LabelSet {
 mod tests {
 
     use std::collections::BTreeMap;
+    use std::hash::{DefaultHasher, Hash};
 
     use pretty_assertions::assert_eq;
 
@@ -195,54 +196,6 @@ mod tests {
         ]
     }
 
-    #[test]
-    fn it_should_allow_instantiation_from_an_array_of_label_pairs() {
-        let label_set: LabelSet = sample_array_of_label_pairs().into();
-
-        assert_eq!(
-            label_set,
-            LabelSet {
-                items: BTreeMap::from(sample_array_of_label_pairs())
-            }
-        );
-    }
-
-    #[test]
-    fn it_should_allow_instantiation_from_a_vec_of_label_pairs() {
-        let label_set: LabelSet = sample_vec_of_label_pairs().into();
-
-        assert_eq!(
-            label_set,
-            LabelSet {
-                items: BTreeMap::from(sample_array_of_label_pairs())
-            }
-        );
-    }
-
-    #[test]
-    fn it_should_allow_instantiation_from_a_b_tree_map() {
-        let label_set: LabelSet = BTreeMap::from(sample_array_of_label_pairs()).into();
-
-        assert_eq!(
-            label_set,
-            LabelSet {
-                items: BTreeMap::from(sample_array_of_label_pairs())
-            }
-        );
-    }
-
-    #[test]
-    fn it_should_allow_instantiation_from_a_label_pair() {
-        let label_set: LabelSet = (label_name!("label_name"), LabelValue::new("value")).into();
-
-        assert_eq!(
-            label_set,
-            LabelSet {
-                items: BTreeMap::from([(label_name!("label_name"), LabelValue::new("value"))])
-            }
-        );
-    }
-
     #[test]
     fn it_should_allow_inserting_a_new_label_pair() {
         let mut label_set = LabelSet::default();
@@ -338,4 +291,155 @@ mod tests {
 
         assert_eq!(label_set.to_string(), r#"{label_name="label value"}"#);
     }
+
+    #[test]
+    fn it_should_allow_instantiation_from_an_array_of_label_pairs() {
+        let label_set: LabelSet = sample_array_of_label_pairs().into();
+
+        assert_eq!(
+            label_set,
+            LabelSet {
+                items: BTreeMap::from(sample_array_of_label_pairs())
+            }
+        );
+    }
+
+    #[test]
+    fn it_should_allow_instantiation_from_a_vec_of_label_pairs() {
+        let label_set: LabelSet = sample_vec_of_label_pairs().into();
+
+        assert_eq!(
+            label_set,
+            LabelSet {
+                items: BTreeMap::from(sample_array_of_label_pairs())
+            }
+        );
+    }
+
+    #[test]
+    fn it_should_allow_instantiation_from_a_b_tree_map() {
+        let label_set: LabelSet = BTreeMap::from(sample_array_of_label_pairs()).into();
+
+        assert_eq!(
+            label_set,
+            LabelSet {
+                items: BTreeMap::from(sample_array_of_label_pairs())
+            }
+        );
+    }
+
+    #[test]
+    fn it_should_allow_instantiation_from_a_label_pair() {
+        let label_set: LabelSet = (label_name!("label_name"), LabelValue::new("value")).into();
+
+        assert_eq!(
+            label_set,
+            LabelSet {
+                items: BTreeMap::from([(label_name!("label_name"), LabelValue::new("value"))])
+            }
+        );
+    }
+
+    #[test]
+    fn it_should_allow_instantiation_from_vec_of_str_tuples() {
+        let label_set: LabelSet = vec![("foo", "bar"), ("baz", "qux")].into();
+
+        let mut expected = BTreeMap::new();
+        expected.insert(LabelName::new("foo"), LabelValue::new("bar"));
+        expected.insert(LabelName::new("baz"), LabelValue::new("qux"));
+
+        assert_eq!(label_set, LabelSet { items: expected });
+    }
+
+    #[test]
+    fn it_should_allow_instantiation_from_vec_of_string_tuples() {
+        let label_set: LabelSet = vec![("foo".to_string(), "bar".to_string()), ("baz".to_string(), "qux".to_string())].into();
+
+        let mut expected = BTreeMap::new();
+        expected.insert(LabelName::new("foo"), LabelValue::new("bar"));
+        expected.insert(LabelName::new("baz"), LabelValue::new("qux"));
+
+        assert_eq!(label_set, LabelSet { items: expected });
+    }
+
+    #[test]
+    fn it_should_allow_instantiation_from_vec_of_serialized_label() {
+        use super::SerializedLabel;
+        let label_set: LabelSet = vec![
+            SerializedLabel {
+                name: LabelName::new("foo"),
+                value: LabelValue::new("bar"),
+            },
+            SerializedLabel {
+                name: LabelName::new("baz"),
+                value: LabelValue::new("qux"),
+            },
+        ]
+        .into();
+
+        let mut expected = BTreeMap::new();
+        expected.insert(LabelName::new("foo"), LabelValue::new("bar"));
+        expected.insert(LabelName::new("baz"), LabelValue::new("qux"));
+
+        assert_eq!(label_set, LabelSet { items: expected });
+    }
+
+    #[test]
+    fn it_should_allow_instantiation_from_array_of_string_tuples() {
+        let arr: [(String, String); 2] = [("foo".to_string(), "bar".to_string()), ("baz".to_string(), "qux".to_string())];
+        let label_set: LabelSet = arr.into();
+
+        let mut expected = BTreeMap::new();
+
+        expected.insert(LabelName::new("foo"), LabelValue::new("bar"));
+        expected.insert(LabelName::new("baz"), LabelValue::new("qux"));
+
+        assert_eq!(label_set, LabelSet { items: expected });
+    }
+
+    #[test]
+    fn it_should_allow_instantiation_from_array_of_str_tuples() {
+        let arr: [(&str, &str); 2] = [("foo", "bar"), ("baz", "qux")];
+        let label_set: LabelSet = arr.into();
+
+        let mut expected = BTreeMap::new();
+
+        expected.insert(LabelName::new("foo"), LabelValue::new("bar"));
+        expected.insert(LabelName::new("baz"), LabelValue::new("qux"));
+
+        assert_eq!(label_set, LabelSet { items: expected });
+    }
+
+    #[test]
+    fn it_should_be_comparable() {
+        let a: LabelSet = (label_name!("x"), LabelValue::new("1")).into();
+        let b: LabelSet = (label_name!("x"), LabelValue::new("1")).into();
+        let c: LabelSet = (label_name!("y"), LabelValue::new("2")).into();
+
+        assert_eq!(a, b);
+        assert_ne!(a, c);
+    }
+
+    #[test]
+    fn it_should_be_allow_ordering() {
+        let a: LabelSet = (label_name!("x"), LabelValue::new("1")).into();
+        let b: LabelSet = (label_name!("y"), LabelValue::new("2")).into();
+
+        assert!(a < b);
+    }
+
+    #[test]
+    fn it_should_be_hashable() {
+        let a: LabelSet = (label_name!("x"), LabelValue::new("1")).into();
+
+        let mut hasher = DefaultHasher::new();
+
+        a.hash(&mut hasher);
+    }
+
+    #[test]
+    fn it_should_implement_clone() {
+        let a: LabelSet = (label_name!("x"), LabelValue::new("1")).into();
+        let _unused = a.clone();
+    }
 }

packages/metrics/src/label/value.rs

@@ -33,6 +33,9 @@ impl From<String> for LabelValue {
 
 #[cfg(test)]
 mod tests {
+    use std::collections::hash_map::DefaultHasher;
+    use std::hash::Hash;
+
     use crate::label::value::LabelValue;
     use crate::prometheus::PrometheusSerializable;
 
@@ -41,4 +44,60 @@ mod tests {
         let label_value = LabelValue::new("value");
         assert_eq!(label_value.to_prometheus(), "value");
     }
+
+    #[test]
+    fn it_could_be_initialized_from_str() {
+        let lv = LabelValue::new("abc");
+        assert_eq!(lv.0, "abc");
+    }
+
+    #[test]
+    fn it_should_allow_to_create_an_ignored_label_value() {
+        let lv = LabelValue::ignore();
+        assert_eq!(lv.0, "");
+    }
+
+    #[test]
+    fn it_should_be_converted_from_string() {
+        let s = String::from("foo");
+        let lv: LabelValue = s.clone().into();
+        assert_eq!(lv.0, s);
+    }
+
+    #[test]
+    fn it_should_be_comparable() {
+        let a = LabelValue::new("x");
+        let b = LabelValue::new("x");
+        let c = LabelValue::new("y");
+
+        assert_eq!(a, b);
+        assert_ne!(a, c);
+    }
+
+    #[test]
+    fn it_should_be_allow_ordering() {
+        let a = LabelValue::new("x");
+        let b = LabelValue::new("y");
+
+        assert!(a < b);
+    }
+
+    #[test]
+    fn it_should_be_hashable() {
+        let a = LabelValue::new("x");
+        let mut hasher = DefaultHasher::new();
+        a.hash(&mut hasher);
+    }
+
+    #[test]
+    fn it_should_implement_clone() {
+        let a = LabelValue::new("x");
+        let _unused = a.clone();
+    }
+
+    #[test]
+    fn it_should_implement_display() {
+        let a = LabelValue::new("x");
+        assert_eq!(format!("{a}"), "x");
+    }
 }

packages/metrics/src/metric/description.rs

@@ -1,6 +1,8 @@
 use derive_more::Display;
 use serde::{Deserialize, Serialize};
 
+use crate::prometheus::PrometheusSerializable;
+
 #[derive(Debug, Display, Clone, Eq, PartialEq, Default, Deserialize, Serialize, Hash, Ord, PartialOrd)]
 pub struct MetricDescription(String);
 
@@ -11,6 +13,11 @@ impl MetricDescription {
     }
 }
 
+impl PrometheusSerializable for MetricDescription {
+    fn to_prometheus(&self) -> String {
+        self.0.clone()
+    }
+}
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -21,6 +28,12 @@ mod tests {
         assert_eq!(metric.0, "Metric description");
     }
 
+    #[test]
+    fn it_serializes_to_prometheus() {
+        let label_value = MetricDescription::new("name");
+        assert_eq!(label_value.to_prometheus(), "name");
+    }
+
     #[test]
     fn it_should_be_displayed() {
         let metric = MetricDescription::new("Metric description");