From 5d8dd76107ed970e9fcf4a7c2867e136789131e5 Mon Sep 17 00:00:00 2001 From: Lars Strojny Date: Thu, 19 Jan 2023 00:14:13 +0100 Subject: [PATCH 1/5] Replace HashMap with BTreeMap to sort metrics deterministically Signed-off-by: Lars Strojny Signed-off-by: Jean-Baptiste Skutnik --- examples/actix-web.rs | 4 +-- examples/axum.rs | 4 +-- examples/tide.rs | 4 +-- src/encoding/text.rs | 54 ++++++++++++++++++++++++++++++++++++++--- src/lib.rs | 4 +-- src/metrics/exemplar.rs | 8 +++--- src/metrics/family.rs | 26 ++++++++------------ 7 files changed, 73 insertions(+), 31 deletions(-) diff --git a/examples/actix-web.rs b/examples/actix-web.rs index a1ce8e79..515082d0 100644 --- a/examples/actix-web.rs +++ b/examples/actix-web.rs @@ -8,13 +8,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/axum.rs b/examples/axum.rs index c3e81705..490996ed 100644 --- a/examples/axum.rs +++ b/examples/axum.rs @@ -13,13 +13,13 @@ use prometheus_client_derive_encode::{EncodeLabelSet, EncodeLabelValue}; use std::sync::Arc; use tokio::sync::Mutex; -#[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelValue)] +#[derive(Clone, Debug, Ord, PartialOrd, PartialEq, Eq, EncodeLabelValue)] pub enum Method { Get, Post, } -#[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet)] +#[derive(Clone, Debug, Ord, PartialOrd, PartialEq, Eq, 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 ebb453a6..8ebd1bbc 100644 --- a/src/encoding/text.rs +++ b/src/encoding/text.rs @@ -933,7 +933,7 @@ mod tests { Family::new_with_constructor(|| Histogram::new(exponential_buckets(1.0, 2.0, 10))); registry.register("my_histogram", "My histogram", family.clone()); - #[derive(Eq, PartialEq, Hash, Debug, Clone)] + #[derive(Eq, PartialEq, Ord, PartialOrd, Debug, Clone)] struct EmptyLabels {} impl EncodeLabelSet for EmptyLabels { @@ -1117,7 +1117,7 @@ mod tests { #[test] fn label_sets_can_be_composed() { - #[derive(Clone, Debug, Eq, Hash, PartialEq)] + #[derive(Clone, Debug, Ord, PartialOrd, Eq, PartialEq)] struct Color(&'static str); impl EncodeLabelSet for Color { fn encode( @@ -1132,7 +1132,7 @@ mod tests { } } - #[derive(Clone, Debug, Eq, Hash, PartialEq)] + #[derive(Clone, Debug, Ord, PartialOrd, Eq, PartialEq)] struct Size(&'static str); impl EncodeLabelSet for Size { fn encode( @@ -1243,4 +1243,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 cfff6238..664e19a9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -31,7 +31,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, @@ -39,7 +39,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 3dfa677e..98b37b2a 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 1d0da87e..350028a4 100644 --- a/src/metrics/family.rs +++ b/src/metrics/family.rs @@ -6,7 +6,7 @@ use crate::encoding::{EncodeLabelSet, EncodeMetric, MetricEncoder}; use super::{MetricType, TypedMetric}; use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard, RwLockWriteGuard}; -use std::collections::HashMap; +use std::collections::BTreeMap; use std::sync::Arc; /// Representation of the OpenMetrics *MetricFamily* data type. @@ -68,12 +68,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, @@ -99,9 +99,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`] @@ -168,7 +167,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())), @@ -177,7 +176,7 @@ impl Default for Family { } } -impl Family { +impl Family { /// Create a metric family using a custom constructor to construct new /// metrics. /// @@ -207,12 +206,7 @@ impl Family { } } -impl> Family -where - S: Clone + std::hash::Hash + Eq, - M: Clone, - C: MetricConstructor, -{ +impl> Family { /// Access a metric with the given label set, creating it if one does not yet exist. /// /// ``` @@ -241,7 +235,7 @@ where } } -impl> Family { +impl> Family { /// Access a metric with the given label set, creating it if one does not /// yet exist. /// @@ -341,7 +335,7 @@ impl> Family RwLockReadGuard<'_, HashMap> { + pub(crate) fn read(&self) -> RwLockReadGuard<'_, BTreeMap> { self.metrics.read() } } @@ -361,7 +355,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, { From bef91be8c9d36bc4fb808be03fa864de4c3b2f63 Mon Sep 17 00:00:00 2001 From: Lars Strojny Date: Thu, 19 Jan 2023 00:17:24 +0100 Subject: [PATCH 2/5] Remove Hash from tide example Signed-off-by: Lars Strojny Signed-off-by: Jean-Baptiste Skutnik --- examples/tide.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/tide.rs b/examples/tide.rs index f90f6a45..ede2363f 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, PartialOrd, Ord, EncodeLabelSet)] +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, EncodeLabelSet)] struct Labels { method: Method, path: String, } -#[derive(Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord, EncodeLabelValue)] +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, EncodeLabelValue)] enum Method { Get, Put, From 96448be1c53e2b2ccb4745725f02cd9451351e54 Mon Sep 17 00:00:00 2001 From: Lars Strojny Date: Thu, 19 Jan 2023 00:27:26 +0100 Subject: [PATCH 3/5] =?UTF-8?q?Let=E2=80=99s=20not=20lie=20in=20the=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Lars Strojny Signed-off-by: Jean-Baptiste Skutnik --- src/encoding/text.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/encoding/text.rs b/src/encoding/text.rs index 8ebd1bbc..863fd8ed 100644 --- a/src/encoding/text.rs +++ b/src/encoding/text.rs @@ -1245,7 +1245,7 @@ def parse(input): } #[test] - fn metrics_are_sorted_by_registration_order() { + fn metrics_are_sorted_in_registration_order() { let mut registry = Registry::default(); let counter: Counter = Counter::default(); let another_counter: Counter = Counter::default(); @@ -1266,20 +1266,20 @@ def parse(input): } #[test] - fn metric_family_is_sorted_by_registration_order() { + fn metric_family_is_sorted_lexicographically() { 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); - } + gauge + .get_or_create(&vec![("label".to_string(), "0".to_string())]) + .set(0); + gauge + .get_or_create(&vec![("label".to_string(), "2".to_string())]) + .set(2); + gauge + .get_or_create(&vec![("label".to_string(), "1".to_string())]) + .set(1); let mut encoded = String::new(); encode(&mut encoded, ®istry).unwrap(); @@ -1288,6 +1288,7 @@ def parse(input): + "# TYPE my_gauge gauge\n" + "my_gauge{label=\"0\"} 0\n" + "my_gauge{label=\"1\"} 1\n" + + "my_gauge{label=\"2\"} 2\n" + "# EOF\n"; assert_eq!(expected, encoded); } From 3ff8388f77f1cbfd085fe3113b6e140b2c91338d Mon Sep 17 00:00:00 2001 From: Lars Strojny Date: Thu, 19 Jan 2023 09:08:46 +0100 Subject: [PATCH 4/5] Require Ord consistently Signed-off-by: Lars Strojny Signed-off-by: Jean-Baptiste Skutnik --- src/metrics/family.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/metrics/family.rs b/src/metrics/family.rs index 350028a4..e65d3df7 100644 --- a/src/metrics/family.rs +++ b/src/metrics/family.rs @@ -167,7 +167,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())), @@ -176,7 +176,7 @@ impl Default for Family { } } -impl Family { +impl Family { /// Create a metric family using a custom constructor to construct new /// metrics. /// From 0f877ae7cdbe7cb3a3da25cb63ceb48b554ffaa7 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Skutnik Date: Mon, 11 Aug 2025 12:29:43 +0200 Subject: [PATCH 5/5] Update benches Signed-off-by: Jean-Baptiste Skutnik --- benches/encoding/text.rs | 6 +++--- benches/family.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/benches/encoding/text.rs b/benches/encoding/text.rs index 52b13f4e..31c5ced0 100644 --- a/benches/encoding/text.rs +++ b/benches/encoding/text.rs @@ -10,21 +10,21 @@ use std::fmt::Write; pub fn text(c: &mut Criterion) { c.bench_function("encode", |b| { - #[derive(Clone, Hash, PartialEq, Eq, EncodeLabelSet, Debug)] + #[derive(Clone, Ord, PartialOrd, PartialEq, Eq, EncodeLabelSet, Debug)] struct Labels { method: Method, status: Status, some_number: u64, } - #[derive(Clone, Hash, PartialEq, Eq, EncodeLabelValue, Debug)] + #[derive(Clone, Ord, PartialOrd, PartialEq, Eq, EncodeLabelValue, Debug)] enum Method { Get, #[allow(dead_code)] Put, } - #[derive(Clone, Hash, PartialEq, Eq, Debug)] + #[derive(Clone, Ord, PartialOrd, PartialEq, Eq, Debug)] enum Status { Two, #[allow(dead_code)] diff --git a/benches/family.rs b/benches/family.rs index bc08df3c..feea9a9f 100644 --- a/benches/family.rs +++ b/benches/family.rs @@ -43,20 +43,20 @@ pub fn family(c: &mut Criterion) { }); c.bench_function("counter family with custom type label set", |b| { - #[derive(Clone, Hash, PartialEq, Eq)] + #[derive(Clone, Ord, PartialOrd, PartialEq, Eq)] struct Labels { method: Method, status: Status, } - #[derive(Clone, Hash, PartialEq, Eq)] + #[derive(Clone, Ord, PartialOrd, PartialEq, Eq)] enum Method { Get, #[allow(dead_code)] Put, } - #[derive(Clone, Hash, PartialEq, Eq)] + #[derive(Clone, Ord, PartialOrd, PartialEq, Eq)] enum Status { Two, #[allow(dead_code)]