From dfaa81df64d2d9e72c4f4bbc1cdbb60c27735d7e Mon Sep 17 00:00:00 2001 From: Ben Sully Date: Tue, 20 Jul 2021 21:53:55 +0100 Subject: [PATCH] Use an internal Registry for Rocket metrics This registry is created by `PrometheusMetrics::with_registry` and is private to each `PrometheusMetrics` instance, allowing multiple `PrometheusMetrics` instances to share the same `extra_registry`. Previously the fairing tried to register the internal metrics on the `extra_registry`, which caused conflicts if the same registry was passed twice. This is now avoided by using an internal registry for those metrics. Both registries are gathered and serialized to the same buffer when the /metrics endpoint is called. Closes #14. --- CHANGELOG.md | 3 +++ src/lib.rs | 54 +++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92bc5a8..7ccd6fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Changed + +- The two Rocket related metrics (`http_requests_total` and `http_requests_duration_seconds`) are now stored inside a separate registry to additional metrics. This allows multiple `PrometheusMetrics` fairings to exist even when using the global `prometheus::Registry`, such as the one used for metrics created by the macros in the `prometheus` crate. Previously this would cause a panic because the two fairing instances would attempt to register identical metrics to the same registry, which is an error. The implication of this is that the registry returned by `PrometheusMetrics::registry` no longer contains the Rocket related metrics. In practice this is unlikely to be a problem, since metrics from both registries are returned by the fairing's handler as before. ## [0.8.0] - 2021-07-10 ### Changed diff --git a/src/lib.rs b/src/lib.rs index 8fa915c..0b4dd79 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -190,17 +190,33 @@ pub struct PrometheusMetrics { http_requests_total: IntCounterVec, http_requests_duration_seconds: HistogramVec, - // The registry used by the fairing. - registry: Registry, + // The registry used by the fairing for Rocket metrics. + // + // This registry is created by `PrometheusMetrics::with_registry` and is + // private to each `PrometheusMetrics` instance, allowing multiple + // `PrometheusMetrics` instances to share the same `extra_registry`. + // + // Previously the fairing tried to register the internal metrics on the `extra_registry`, + // which caused conflicts if the same registry was passed twice. This is now avoided + // by using an internal registry for those metrics. + rocket_registry: Registry, + + // The registry used by the fairing for custom metrics. + // + // See `rocket_registry` for details on why these metrics are stored on a separate registry. + custom_registry: Registry, } impl PrometheusMetrics { - /// Get the registry used by this fairing. + /// Get the registry used by this fairing to track additional metrics. /// /// You can use this to register further metrics, /// causing them to be exposed along with the default metrics /// on the `PrometheusMetrics` handler. /// + /// Note that the `http_requests_total` and `http_requests_duration_seconds` metrics + /// are _not_ included in this registry. + /// /// ```rust /// use once_cell::sync::Lazy; /// use prometheus::{opts, IntCounter}; @@ -216,7 +232,7 @@ impl PrometheusMetrics { /// ``` #[must_use] pub const fn registry(&self) -> &Registry { - &self.registry + &self.custom_registry } } @@ -236,6 +252,7 @@ impl PrometheusMetrics { /// Create a new `PrometheusMetrics` with a custom `Registry`. pub fn with_registry(registry: Registry) -> Self { + let rocket_registry = Registry::new(); let namespace = env::var(NAMESPACE_ENV_VAR).unwrap_or_else(|_| "rocket".into()); let http_requests_total_opts = @@ -244,10 +261,6 @@ impl PrometheusMetrics { let http_requests_total = IntCounterVec::new(http_requests_total_opts, &["endpoint", "method", "status"]) .unwrap(); - registry - .register(Box::new(http_requests_total.clone())) - .unwrap(); - let http_requests_duration_seconds_opts = opts!( "http_requests_duration_seconds", "HTTP request duration in seconds for all requests" @@ -258,14 +271,19 @@ impl PrometheusMetrics { &["endpoint", "method", "status"], ) .unwrap(); - registry + + rocket_registry + .register(Box::new(http_requests_total.clone())) + .unwrap(); + rocket_registry .register(Box::new(http_requests_duration_seconds.clone())) .unwrap(); PrometheusMetrics { http_requests_total, http_requests_duration_seconds, - registry, + rocket_registry, + custom_registry: registry, } } } @@ -321,7 +339,10 @@ impl Handler for PrometheusMetrics { let mut buffer = vec![]; let encoder = TextEncoder::new(); encoder - .encode(&self.registry.gather(), &mut buffer) + .encode(&self.custom_registry.gather(), &mut buffer) + .unwrap(); + encoder + .encode(&self.rocket_registry.gather(), &mut buffer) .unwrap(); let body = String::from_utf8(buffer).unwrap(); Outcome::from( @@ -343,3 +364,14 @@ impl From for Vec { vec![Route::new(Method::Get, "/", other)] } } + +#[cfg(test)] +mod test { + use super::PrometheusMetrics; + + #[test] + fn test_multiple_instantiations() { + let _pm1 = PrometheusMetrics::with_default_registry(); + let _pm2 = PrometheusMetrics::with_default_registry(); + } +}