Replace HashMap with BTreeMap to sort metrics deterministically

Signed-off-by: Lars Strojny <[email protected]>

Author: lstrojny

examples/actix-web.rs

@@ -7,13 +7,13 @@ use prometheus_client::metrics::counter::Counter;
 use prometheus_client::metrics::family::Family;
 use prometheus_client::registry::Registry;
 
-#[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelValue)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, EncodeLabelValue)]
 pub enum Method {
     Get,
     Post,
 }
 
-#[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, EncodeLabelSet)]
 pub struct MethodLabels {
     pub method: Method,
 }

examples/tide.rs

@@ -44,13 +44,13 @@ async fn main() -> std::result::Result<(), std::io::Error> {
     Ok(())
 }
 
-#[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet)]
+#[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord, EncodeLabelSet)]
 struct Labels {
     method: Method,
     path: String,
 }
 
-#[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelValue)]
+#[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord, EncodeLabelValue)]
 enum Method {
     Get,
     Put,

src/encoding/text.rs

@@ -751,4 +751,52 @@ def parse(input):
                 .unwrap();
         })
     }
+
+    #[test]
+    fn metrics_are_sorted_by_registration_order() {
+        let mut registry = Registry::default();
+        let counter: Counter = Counter::default();
+        let another_counter: Counter = Counter::default();
+        registry.register("my_counter", "My counter", counter);
+        registry.register("another_counter", "Another counter", another_counter);
+
+        let mut encoded = String::new();
+        encode(&mut encoded, &registry).unwrap();
+
+        let expected = "# HELP my_counter My counter.\n".to_owned()
+            + "# TYPE my_counter counter\n"
+            + "my_counter_total 0\n"
+            + "# HELP another_counter Another counter.\n"
+            + "# TYPE another_counter counter\n"
+            + "another_counter_total 0\n"
+            + "# EOF\n";
+        assert_eq!(expected, encoded);
+    }
+
+    #[test]
+    fn metric_family_is_sorted_by_registration_order() {
+        let mut registry = Registry::default();
+        let gauge = Family::<Vec<(String, String)>, Gauge>::default();
+        registry.register("my_gauge", "My gauge", gauge.clone());
+
+        {
+            let gauge0 = gauge.get_or_create(&vec![("label".to_string(), "0".to_string())]);
+            gauge0.set(0);
+        }
+
+        {
+            let gauge1 = gauge.get_or_create(&vec![("label".to_string(), "1".to_string())]);
+            gauge1.set(1);
+        }
+
+        let mut encoded = String::new();
+        encode(&mut encoded, &registry).unwrap();
+
+        let expected = "# HELP my_gauge My gauge.\n".to_owned()
+            + "# TYPE my_gauge gauge\n"
+            + "my_gauge{label=\"0\"} 0\n"
+            + "my_gauge{label=\"1\"} 1\n"
+            + "# EOF\n";
+        assert_eq!(expected, encoded);
+    }
 }

src/lib.rs

@@ -30,15 +30,15 @@
 //! //
 //! // You could as well use `(String, String)` to represent a label set,
 //! // instead of the custom type below.
-//! #[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet)]
+//! #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, EncodeLabelSet)]
 //! struct Labels {
 //!   // Use your own enum types to represent label values.
 //!   method: Method,
 //!   // Or just a plain string.
 //!   path: String,
 //! };
 //!
-//! #[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelValue)]
+//! #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, EncodeLabelValue)]
 //! enum Method {
 //!   GET,
 //!   PUT,

src/metrics/exemplar.rs

@@ -42,12 +42,12 @@ pub struct Exemplar<S, V> {
 /// # use prometheus_client::metrics::histogram::exponential_buckets;
 /// # use prometheus_client::metrics::family::Family;
 /// # use prometheus_client_derive_encode::EncodeLabelSet;
-/// #[derive(Clone, Hash, PartialEq, Eq, EncodeLabelSet, Debug, Default)]
+/// #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, EncodeLabelSet, Debug, Default)]
 /// pub struct ResultLabel {
 ///     pub result: String,
 /// }
 ///
-/// #[derive(Clone, Hash, PartialEq, Eq, EncodeLabelSet, Debug, Default)]
+/// #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, EncodeLabelSet, Debug, Default)]
 /// pub struct TraceLabel {
 ///     pub trace_id: String,
 /// }
@@ -183,12 +183,12 @@ where
 /// # use prometheus_client::metrics::histogram::exponential_buckets;
 /// # use prometheus_client::metrics::family::Family;
 /// # use prometheus_client::encoding::EncodeLabelSet;
-/// #[derive(Clone, Hash, PartialEq, Eq, EncodeLabelSet, Debug, Default)]
+/// #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, EncodeLabelSet, Debug, Default)]
 /// pub struct ResultLabel {
 ///     pub result: String,
 /// }
 ///
-/// #[derive(Clone, Hash, PartialEq, Eq, EncodeLabelSet, Debug, Default)]
+/// #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, EncodeLabelSet, Debug, Default)]
 /// pub struct TraceLabel {
 ///     pub trace_id: String,
 /// }

src/metrics/family.rs

@@ -7,7 +7,7 @@ use crate::encoding::{EncodeLabelSet, EncodeMetric, MetricEncoder};
 use super::{MetricType, TypedMetric};
 use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard, RwLockWriteGuard};
 use std::cell::RefCell;
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 use std::sync::Arc;
 
 /// Representation of the OpenMetrics *MetricFamily* data type.
@@ -69,12 +69,12 @@ use std::sync::Arc;
 /// # use std::io::Write;
 /// #
 /// # let mut registry = Registry::default();
-/// #[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet)]
+/// #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, EncodeLabelSet)]
 /// struct Labels {
 ///   method: Method,
 /// };
 ///
-/// #[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelValue)]
+/// #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, EncodeLabelValue)]
 /// enum Method {
 ///   GET,
 ///   PUT,
@@ -100,9 +100,8 @@ use std::sync::Arc;
 /// #                "# EOF\n";
 /// # assert_eq!(expected, buffer);
 /// ```
-// TODO: Consider exposing hash algorithm.
 pub struct Family<S, M, C = fn() -> M> {
-    metrics: Arc<RwLock<HashMap<S, M>>>,
+    metrics: Arc<RwLock<BTreeMap<S, M>>>,
     /// Function that when called constructs a new metric.
     ///
     /// For most metric types this would simply be its [`Default`]
@@ -169,7 +168,7 @@ impl<M, F: Fn() -> M> MetricConstructor<M> for F {
     }
 }
 
-impl<S: Clone + std::hash::Hash + Eq, M: Default> Default for Family<S, M> {
+impl<S: Clone + Eq, M: Default> Default for Family<S, M> {
     fn default() -> Self {
         Self {
             metrics: Arc::new(RwLock::new(Default::default())),
@@ -178,7 +177,7 @@ impl<S: Clone + std::hash::Hash + Eq, M: Default> Default for Family<S, M> {
     }
 }
 
-impl<S: Clone + std::hash::Hash + Eq, M, C> Family<S, M, C> {
+impl<S: Clone + Eq, M, C> Family<S, M, C> {
     /// Create a metric family using a custom constructor to construct new
     /// metrics.
     ///
@@ -208,7 +207,7 @@ impl<S: Clone + std::hash::Hash + Eq, M, C> Family<S, M, C> {
     }
 }
 
-impl<S: Clone + std::hash::Hash + Eq, M, C: MetricConstructor<M>> Family<S, M, C> {
+impl<S: Clone + Eq + Ord, M, C: MetricConstructor<M>> Family<S, M, C> {
     /// Access a metric with the given label set, creating it if one does not
     /// yet exist.
     ///
@@ -289,7 +288,7 @@ impl<S: Clone + std::hash::Hash + Eq, M, C: MetricConstructor<M>> Family<S, M, C
         self.metrics.write().clear()
     }
 
-    pub(crate) fn read(&self) -> RwLockReadGuard<HashMap<S, M>> {
+    pub(crate) fn read(&self) -> RwLockReadGuard<BTreeMap<S, M>> {
         self.metrics.read()
     }
 }
@@ -309,7 +308,7 @@ impl<S, M: TypedMetric, C> TypedMetric for Family<S, M, C> {
 
 impl<S, M, C> EncodeMetric for Family<S, M, C>
 where
-    S: Clone + std::hash::Hash + Eq + EncodeLabelSet,
+    S: Clone + Eq + Ord + EncodeLabelSet,
     M: EncodeMetric + TypedMetric,
     C: MetricConstructor<M>,
 {