diff --git a/CHANGELOG.md b/CHANGELOG.md index 0532a102..c1a4445d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Always use dynamic dispatch on Registry, i.e. remove generic type parameter `M` from `Registry`. + - Move`Encode` trait from `prometheus_client::encoding::text` to `prometheus_client::encoding`. See [PR 83]. [PR 83]: https://github.com/prometheus/client_rust/pull/83 diff --git a/examples/actix-web.rs b/examples/actix-web.rs index 80144aa2..5b59cd2c 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, Hash, PartialEq, Eq, Encode)] +#[derive(Clone, Debug, Hash, PartialEq, Eq, Encode)] pub enum Method { Get, Post, } -#[derive(Clone, Hash, PartialEq, Eq, Encode)] +#[derive(Clone, Debug, Hash, PartialEq, Eq, Encode)] pub struct MethodLabels { pub method: Method, } @@ -55,11 +55,9 @@ async fn main() -> std::io::Result<()> { let mut state = AppState { registry: Registry::default(), }; - state.registry.register( - "requests", - "Count of requests", - Box::new(metrics.requests.clone()), - ); + state + .registry + .register("requests", "Count of requests", metrics.requests.clone()); let state = web::Data::new(Mutex::new(state)); HttpServer::new(move || { diff --git a/examples/custom-metric.rs b/examples/custom-metric.rs index 7fcda080..fe8f3cef 100644 --- a/examples/custom-metric.rs +++ b/examples/custom-metric.rs @@ -7,6 +7,7 @@ use prometheus_client::registry::Registry; /// Related to the concept of "Custom Collectors" in other implementations. /// /// [`MyCustomMetric`] generates and encodes a random number on each scrape. +#[derive(Debug)] struct MyCustomMetric {} impl EncodeMetric for MyCustomMetric { diff --git a/examples/hyper.rs b/examples/hyper.rs index 404b6c17..93884ca0 100644 --- a/examples/hyper.rs +++ b/examples/hyper.rs @@ -21,7 +21,7 @@ async fn main() { registry.register( "requests", "How many requests the application has received", - Box::new(request_counter.clone()), + request_counter.clone(), ); // Spawn a server to serve the OpenMetrics endpoint. diff --git a/examples/tide.rs b/examples/tide.rs index 244db06d..677105d3 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, Hash, PartialEq, Eq, Encode)] +#[derive(Clone, Debug, Hash, PartialEq, Eq, Encode)] struct Labels { method: Method, path: String, } -#[derive(Clone, Hash, PartialEq, Eq, Encode)] +#[derive(Clone, Debug, Hash, PartialEq, Eq, Encode)] enum Method { Get, Put, @@ -58,7 +58,7 @@ enum Method { #[derive(Clone)] struct State { - registry: Arc>>, + registry: Arc, } #[derive(Default)] diff --git a/src/encoding/proto.rs b/src/encoding/proto.rs index 1230ee4b..f183551c 100644 --- a/src/encoding/proto.rs +++ b/src/encoding/proto.rs @@ -47,7 +47,7 @@ pub use prometheus_client_derive_encode::*; /// Encode the metrics registered with the provided [`Registry`] into MetricSet /// using the OpenMetrics protobuf format. -pub fn encode(registry: &Registry) -> openmetrics_data_model::MetricSet +pub fn encode(registry: &Registry) -> openmetrics_data_model::MetricSet where M: EncodeMetric, { @@ -57,7 +57,8 @@ where let mut family = openmetrics_data_model::MetricFamily { name: desc.name().to_string(), r#type: { - let metric_type: openmetrics_data_model::MetricType = metric.metric_type().into(); + let metric_type: openmetrics_data_model::MetricType = + EncodeMetric::metric_type(metric.as_ref()).into(); metric_type as i32 }, unit: if let Some(unit) = desc.unit() { @@ -71,7 +72,7 @@ where let mut labels = vec![]; desc.labels().encode(&mut labels); - metric.encode(labels, &mut family.metrics); + EncodeMetric::encode(metric.as_ref(), labels, &mut family.metrics); metric_set.metric_families.push(family); } @@ -118,11 +119,6 @@ impl EncodeMetric for Box { } } -/// Trait combining [`EncodeMetric`] and [`Send`]. -pub trait SendEncodeMetric: EncodeMetric + Send {} - -impl SendEncodeMetric for T {} - /// Trait to implement its label encoding in the OpenMetrics protobuf format. pub trait EncodeLabels { /// Encode the given instance into Labels in the OpenMetrics protobuf diff --git a/src/encoding/text.rs b/src/encoding/text.rs index 754b08a7..7504b97b 100644 --- a/src/encoding/text.rs +++ b/src/encoding/text.rs @@ -40,10 +40,9 @@ use std::ops::Deref; /// Encode the metrics registered with the provided [`Registry`] into the /// provided [`Write`]r using the OpenMetrics text format. -pub fn encode(writer: &mut W, registry: &Registry) -> Result<(), std::io::Error> +pub fn encode(writer: &mut W, registry: &Registry) -> Result<(), std::io::Error> where W: Write, - M: EncodeMetric, { for (desc, metric) in registry.iter() { writer.write_all(b"# HELP ")?; @@ -63,7 +62,7 @@ where unit.encode(writer)?; } writer.write_all(b" ")?; - metric.metric_type().encode(writer)?; + EncodeMetric::metric_type(metric.as_ref()).encode(writer)?; writer.write_all(b"\n")?; if let Some(unit) = desc.unit() { @@ -84,7 +83,7 @@ where labels: None, }; - metric.encode(encoder)?; + EncodeMetric::encode(metric.as_ref(), encoder)?; } writer.write_all(b"# EOF\n")?; @@ -402,21 +401,6 @@ impl EncodeMetric for Box { } } -/// Trait combining [`EncodeMetric`], [`Send`] and [`Sync`]. -pub trait SendSyncEncodeMetric: EncodeMetric + Send + Sync {} - -impl SendSyncEncodeMetric for T {} - -impl EncodeMetric for Box { - fn encode(&self, encoder: Encoder) -> Result<(), std::io::Error> { - self.deref().encode(encoder) - } - - fn metric_type(&self) -> MetricType { - self.deref().metric_type() - } -} - ///////////////////////////////////////////////////////////////////////////////// // Counter diff --git a/src/lib.rs b/src/lib.rs index dbc70f31..061b5cf7 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, Hash, PartialEq, Eq, Encode)] +//! #[derive(Clone, Debug, Hash, PartialEq, Eq, Encode)] //! struct Labels { //! // Use your own enum types to represent label values. //! method: Method, @@ -38,7 +38,7 @@ //! path: String, //! }; //! -//! #[derive(Clone, Hash, PartialEq, Eq, Encode)] +//! #[derive(Clone, Debug, Hash, PartialEq, Eq, Encode)] //! enum Method { //! GET, //! PUT, @@ -54,7 +54,7 @@ //! "http_requests", //! // And the metric help text. //! "Number of HTTP requests received", -//! Box::new(http_requests.clone()), +//! http_requests.clone(), //! ); //! //! // Somewhere in your business logic record a single HTTP GET request. diff --git a/src/metrics/family.rs b/src/metrics/family.rs index c3cf6b9b..0dbf51ee 100644 --- a/src/metrics/family.rs +++ b/src/metrics/family.rs @@ -65,12 +65,12 @@ use std::sync::Arc; /// # use std::io::Write; /// # /// # let mut registry = Registry::default(); -/// #[derive(Clone, Hash, PartialEq, Eq, Encode)] +/// #[derive(Clone, Debug, Hash, PartialEq, Eq, Encode)] /// struct Labels { /// method: Method, /// }; /// -/// #[derive(Clone, Hash, PartialEq, Eq, Encode)] +/// #[derive(Clone, Debug, Hash, PartialEq, Eq, Encode)] /// enum Method { /// GET, /// PUT, @@ -97,7 +97,6 @@ use std::sync::Arc; /// # assert_eq!(expected, String::from_utf8(buffer).unwrap()); /// ``` // TODO: Consider exposing hash algorithm. -#[derive(Debug)] pub struct Family M> { metrics: Arc>>, /// Function that when called constructs a new metric. @@ -111,6 +110,14 @@ pub struct Family M> { constructor: C, } +impl std::fmt::Debug for Family { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Family") + .field("metrics", &self.metrics) + .finish() + } +} + /// A constructor for creating new metrics in a [`Family`] when calling /// [`Family::get_or_create`]. Such constructor is provided via /// [`Family::new_with_constructor`]. diff --git a/src/registry.rs b/src/registry.rs index b88dc46b..e69ad3b9 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -25,10 +25,7 @@ use std::borrow::Cow; /// # use prometheus_client::registry::Registry; /// # /// // Create a metric registry. -/// // -/// // Note the angle brackets to make sure to use the default (dynamic -/// // dispatched boxed metric) for the generic type parameter. -/// let mut registry = ::default(); +/// let mut registry = Registry::default(); /// /// let counter: Counter = Counter::default(); /// let gauge: Gauge = Gauge::default(); @@ -36,12 +33,12 @@ use std::borrow::Cow; /// registry.register( /// "my_counter", /// "This is my counter", -/// Box::new(counter.clone()), +/// counter.clone(), /// ); /// registry.register( /// "my_gauge", /// "This is my gauge", -/// Box::new(gauge.clone()), +/// gauge.clone(), /// ); /// /// # // Encode all metrics in the registry in the text format. @@ -58,14 +55,14 @@ use std::borrow::Cow; /// # assert_eq!(expected, String::from_utf8(buffer).unwrap()); /// ``` #[derive(Debug)] -pub struct Registry> { +pub struct Registry { prefix: Option, labels: Vec<(Cow<'static, str>, Cow<'static, str>)>, - metrics: Vec<(Descriptor, M)>, - sub_registries: Vec>, + metrics: Vec<(Descriptor, Box)>, + sub_registries: Vec, } -impl Default for Registry { +impl Default for Registry { fn default() -> Self { Self { prefix: None, @@ -76,7 +73,7 @@ impl Default for Registry { } } -impl Registry { +impl Registry { /// Creates a new default [`Registry`] with the given prefix. pub fn with_prefix(prefix: impl Into) -> Self { Self { @@ -103,12 +100,17 @@ impl Registry { /// # use prometheus_client::metrics::counter::{Atomic as _, Counter}; /// # use prometheus_client::registry::{Registry, Unit}; /// # - /// let mut registry: Registry = Registry::default(); - /// let counter = Counter::default(); + /// let mut registry = Registry::default(); + /// let counter: Counter = Counter::default(); /// /// registry.register("my_counter", "This is my counter", counter.clone()); /// ``` - pub fn register, H: Into>(&mut self, name: N, help: H, metric: M) { + pub fn register, H: Into>( + &mut self, + name: N, + help: H, + metric: impl Metric, + ) { self.priv_register(name, help, metric, None) } @@ -124,8 +126,8 @@ impl Registry { /// # use prometheus_client::metrics::counter::{Atomic as _, Counter}; /// # use prometheus_client::registry::{Registry, Unit}; /// # - /// let mut registry: Registry = Registry::default(); - /// let counter = Counter::default(); + /// let mut registry = Registry::default(); + /// let counter: Counter = Counter::default(); /// /// registry.register_with_unit( /// "my_counter", @@ -139,7 +141,7 @@ impl Registry { name: N, help: H, unit: Unit, - metric: M, + metric: impl Metric, ) { self.priv_register(name, help, metric, Some(unit)) } @@ -148,7 +150,7 @@ impl Registry { &mut self, name: N, help: H, - metric: M, + metric: impl Metric, unit: Option, ) { let name = name.into(); @@ -164,7 +166,7 @@ impl Registry { labels: self.labels.clone(), }; - self.metrics.push((descriptor, metric)); + self.metrics.push((descriptor, Box::new(metric))); } // TODO: Update doc. @@ -183,17 +185,17 @@ impl Registry { /// # use prometheus_client::metrics::counter::{Atomic as _, Counter}; /// # use prometheus_client::registry::{Registry, Unit}; /// # - /// let mut registry: Registry = Registry::default(); + /// let mut registry = Registry::default(); /// - /// let subsystem_a_counter_1 = Counter::default(); - /// let subsystem_a_counter_2 = Counter::default(); + /// let subsystem_a_counter_1: Counter = Counter::default(); + /// let subsystem_a_counter_2: Counter = Counter::default(); /// /// let subsystem_a_registry = registry.sub_registry_with_prefix("subsystem_a"); /// registry.register("counter_1", "", subsystem_a_counter_1.clone()); /// registry.register("counter_2", "", subsystem_a_counter_2.clone()); /// - /// let subsystem_b_counter_1 = Counter::default(); - /// let subsystem_b_counter_2 = Counter::default(); + /// let subsystem_b_counter_1: Counter = Counter::default(); + /// let subsystem_b_counter_2: Counter = Counter::default(); /// /// let subsystem_a_registry = registry.sub_registry_with_prefix("subsystem_b"); /// registry.register("counter_1", "", subsystem_b_counter_1.clone()); @@ -239,7 +241,7 @@ impl Registry { } /// [`Iterator`] over all metrics registered with the [`Registry`]. - pub fn iter(&self) -> RegistryIterator { + pub fn iter(&self) -> RegistryIterator { let metrics = self.metrics.iter(); let sub_registries = self.sub_registries.iter(); RegistryIterator { @@ -253,14 +255,14 @@ impl Registry { /// Iterator iterating both the metrics registered directly with the registry as /// well as all metrics registered with sub-registries. #[derive(Debug)] -pub struct RegistryIterator<'a, M> { - metrics: std::slice::Iter<'a, (Descriptor, M)>, - sub_registries: std::slice::Iter<'a, Registry>, - sub_registry: Option>>, +pub struct RegistryIterator<'a> { + metrics: std::slice::Iter<'a, (Descriptor, Box)>, + sub_registries: std::slice::Iter<'a, Registry>, + sub_registry: Option>>, } -impl<'a, M> Iterator for RegistryIterator<'a, M> { - type Item = &'a (Descriptor, M); +impl<'a> Iterator for RegistryIterator<'a> { + type Item = &'a (Descriptor, Box); fn next(&mut self) -> Option { if let Some(metric) = self.metrics.next() { @@ -365,6 +367,45 @@ impl Unit { } } +// TODO: Instead of depending on dynamic dispatch for all metrics, we could use +// static dispatch for the most common ones and only fall back to dynamic +// dispatch. That said, this needs to be proven performant in a benchmark first. +/// Super trait representing an abstract Prometheus metric. +#[cfg(not(feature = "protobuf"))] +pub trait Metric: + crate::encoding::text::EncodeMetric + Send + Sync + std::fmt::Debug + 'static +{ +} + +/// Super trait representing an abstract Prometheus metric. +#[cfg(feature = "protobuf")] +pub trait Metric: + crate::encoding::text::EncodeMetric + + crate::encoding::proto::EncodeMetric + + Send + + Sync + + std::fmt::Debug + + 'static +{ +} + +#[cfg(not(feature = "protobuf"))] +impl Metric for T where + T: crate::encoding::text::EncodeMetric + Send + Sync + std::fmt::Debug + 'static +{ +} + +#[cfg(feature = "protobuf")] +impl Metric for T where + T: crate::encoding::text::EncodeMetric + + crate::encoding::proto::EncodeMetric + + Send + + Sync + + std::fmt::Debug + + 'static +{ +} + #[cfg(test)] mod tests { use super::*; @@ -372,8 +413,8 @@ mod tests { #[test] fn register_and_iterate() { - let mut registry: Registry = Registry::default(); - let counter = Counter::default(); + let mut registry = Registry::default(); + let counter: Counter = Counter::default(); registry.register("my_counter", "My counter", counter); assert_eq!(1, registry.iter().count()) @@ -382,28 +423,29 @@ mod tests { #[test] fn sub_registry_with_prefix_and_label() { let top_level_metric_name = "my_top_level_metric"; - let mut registry = Registry::::default(); - registry.register(top_level_metric_name, "some help", Default::default()); + let mut registry = Registry::default(); + let counter: Counter = Counter::default(); + registry.register(top_level_metric_name, "some help", counter.clone()); let prefix_1 = "prefix_1"; let prefix_1_metric_name = "my_prefix_1_metric"; let sub_registry = registry.sub_registry_with_prefix(prefix_1); - sub_registry.register(prefix_1_metric_name, "some help", Default::default()); + sub_registry.register(prefix_1_metric_name, "some help", counter.clone()); let prefix_1_1 = "prefix_1_1"; let prefix_1_1_metric_name = "my_prefix_1_1_metric"; let sub_sub_registry = sub_registry.sub_registry_with_prefix(prefix_1_1); - sub_sub_registry.register(prefix_1_1_metric_name, "some help", Default::default()); + sub_sub_registry.register(prefix_1_1_metric_name, "some help", counter.clone()); let label_1_2 = (Cow::Borrowed("registry"), Cow::Borrowed("1_2")); let prefix_1_2_metric_name = "my_prefix_1_2_metric"; let sub_sub_registry = sub_registry.sub_registry_with_label(label_1_2.clone()); - sub_sub_registry.register(prefix_1_2_metric_name, "some help", Default::default()); + sub_sub_registry.register(prefix_1_2_metric_name, "some help", counter.clone()); let prefix_1_2_1 = "prefix_1_2_1"; let prefix_1_2_1_metric_name = "my_prefix_1_2_1_metric"; let sub_sub_sub_registry = sub_sub_registry.sub_registry_with_prefix(prefix_1_2_1); - sub_sub_sub_registry.register(prefix_1_2_1_metric_name, "some help", Default::default()); + sub_sub_sub_registry.register(prefix_1_2_1_metric_name, "some help", counter.clone()); let prefix_2 = "prefix_2"; let _ = registry.sub_registry_with_prefix(prefix_2); @@ -411,7 +453,7 @@ mod tests { let prefix_3 = "prefix_3"; let prefix_3_metric_name = "my_prefix_3_metric"; let sub_registry = registry.sub_registry_with_prefix(prefix_3); - sub_registry.register(prefix_3_metric_name, "some help", Default::default()); + sub_registry.register(prefix_3_metric_name, "some help", counter.clone()); let mut metric_iter = registry .iter()