Skip to content

Commit

Permalink
Replace HashMap with BTreeMap to sort metrics deterministically
Browse files Browse the repository at this point in the history
  • Loading branch information
lstrojny committed Jan 18, 2023
1 parent 9141c11 commit f9b4a97
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 20 deletions.
4 changes: 2 additions & 2 deletions examples/actix-web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
4 changes: 2 additions & 2 deletions examples/tide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
48 changes: 48 additions & 0 deletions src/encoding/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions src/metrics/exemplar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
/// }
Expand Down Expand Up @@ -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,
/// }
Expand Down
19 changes: 9 additions & 10 deletions src/metrics/family.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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`]
Expand Down Expand Up @@ -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())),
Expand All @@ -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.
///
Expand Down Expand Up @@ -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.
///
Expand Down Expand Up @@ -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()
}
}
Expand All @@ -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>,
{
Expand Down

0 comments on commit f9b4a97

Please sign in to comment.