From f9b4a97b4221ccfc85760b8c17ca94ab895ee1c6 Mon Sep 17 00:00:00 2001 From: Lars Strojny Date: Thu, 19 Jan 2023 00:14:13 +0100 Subject: [PATCH] Replace HashMap with BTreeMap to sort metrics deterministically --- examples/actix-web.rs | 4 ++-- examples/tide.rs | 4 ++-- src/encoding/text.rs | 48 +++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 4 ++-- src/metrics/exemplar.rs | 8 +++---- src/metrics/family.rs | 19 ++++++++-------- 6 files changed, 67 insertions(+), 20 deletions(-) diff --git a/examples/actix-web.rs b/examples/actix-web.rs index d01a1027..038e37f4 100644 --- a/examples/actix-web.rs +++ b/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, } diff --git a/examples/tide.rs b/examples/tide.rs index 68cacac6..f90f6a45 100644 --- a/examples/tide.rs +++ b/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, diff --git a/src/encoding/text.rs b/src/encoding/text.rs index dea22e05..cb7d4868 100644 --- a/src/encoding/text.rs +++ b/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, ®istry).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::, 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, ®istry).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); + } } diff --git a/src/lib.rs b/src/lib.rs index 69bac37a..09a5c7b1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -30,7 +30,7 @@ //! // //! // 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, @@ -38,7 +38,7 @@ //! path: String, //! }; //! -//! #[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelValue)] +//! #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, EncodeLabelValue)] //! enum Method { //! GET, //! PUT, diff --git a/src/metrics/exemplar.rs b/src/metrics/exemplar.rs index c8478c6a..0eb82865 100644 --- a/src/metrics/exemplar.rs +++ b/src/metrics/exemplar.rs @@ -42,12 +42,12 @@ pub struct Exemplar { /// # 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, /// } diff --git a/src/metrics/family.rs b/src/metrics/family.rs index 3b70eeb9..6fefbbfc 100644 --- a/src/metrics/family.rs +++ b/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 M> { - metrics: Arc>>, + metrics: Arc>>, /// Function that when called constructs a new metric. /// /// For most metric types this would simply be its [`Default`] @@ -169,7 +168,7 @@ impl M> MetricConstructor for F { } } -impl Default for Family { +impl Default for Family { fn default() -> Self { Self { metrics: Arc::new(RwLock::new(Default::default())), @@ -178,7 +177,7 @@ impl Default for Family { } } -impl Family { +impl Family { /// Create a metric family using a custom constructor to construct new /// metrics. /// @@ -208,7 +207,7 @@ impl Family { } } -impl> Family { +impl> Family { /// Access a metric with the given label set, creating it if one does not /// yet exist. /// @@ -289,7 +288,7 @@ impl> Family RwLockReadGuard> { + pub(crate) fn read(&self) -> RwLockReadGuard> { self.metrics.read() } } @@ -309,7 +308,7 @@ impl TypedMetric for Family { impl EncodeMetric for Family where - S: Clone + std::hash::Hash + Eq + EncodeLabelSet, + S: Clone + Eq + Ord + EncodeLabelSet, M: EncodeMetric + TypedMetric, C: MetricConstructor, {