From 6bc994a3c3a09383b5677b73da54266000074723 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 9 Oct 2024 12:08:46 +0200 Subject: [PATCH 01/23] Example using a dedicated struct --- mithril-signer/src/metrics/service.rs | 104 ++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index 2c281e99d6d..a3078e12264 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -40,6 +40,49 @@ pub struct MetricsService { signature_registration_success_last_epoch_gauge: Box, runtime_cycle_success_since_startup_counter: Box, runtime_cycle_total_since_startup_counter: Box, + + signer_registration_success_since_startup_counter_struct: MetricCounter, + signer_registration_success_last_epoch_gauge_struct: MetricGauge, +} + +pub struct MetricCounter { + counter: Counter, +} + +impl MetricCounter { + fn record(&self) -> () { + self.counter.inc(); + } + + fn get(&self) -> CounterValue { + self.counter.get().round() as CounterValue + } + + fn new(name: &str, help: &str) -> Self { + Self { + counter: MetricsService::create_metric_counter(name, help).unwrap(), + } + } +} + +pub struct MetricGauge { + gauge: Gauge, +} + +impl MetricGauge { + fn record(&self, epoch: Epoch) -> () { + self.gauge.set(epoch.0 as f64); + } + + fn get(&self) -> Epoch { + Epoch(self.gauge.get().round() as u64) + } + + fn new(name: &str, help: &str) -> Self { + Self { + gauge: MetricsService::create_metric_gauge(name, help).unwrap(), + } + } } impl MetricsService { @@ -48,6 +91,11 @@ impl MetricsService { let registry = Registry::new(); // Signer registration metrics + let signer_registration_success_since_startup_counter_struct = MetricCounter::new( + SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME, + SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_HELP, + ); + let signer_registration_success_since_startup_counter = Box::new(Self::create_metric_counter( SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME, @@ -62,6 +110,10 @@ impl MetricsService { )?); registry.register(signer_registration_total_since_startup_counter.clone())?; + let signer_registration_success_last_epoch_gauge_struct = MetricGauge::new( + SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME, + SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP, + ); let signer_registration_success_last_epoch_gauge = Box::new(Self::create_metric_gauge( SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME, SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP, @@ -113,6 +165,8 @@ impl MetricsService { signature_registration_success_last_epoch_gauge, runtime_cycle_success_since_startup_counter, runtime_cycle_total_since_startup_counter, + signer_registration_success_since_startup_counter_struct, + signer_registration_success_last_epoch_gauge_struct, }) } @@ -458,4 +512,54 @@ mod tests { metrics_service.runtime_cycle_total_since_startup_counter_get(), ); } + + //// + + #[test] + fn test_signature_registration_success_last_epoch_gauge_set_struct() { + let metrics_service = MetricsService::new().unwrap(); + assert_eq!( + Epoch(0), + metrics_service + .signer_registration_success_last_epoch_gauge_struct + .get(), + ); + + metrics_service + .signer_registration_success_last_epoch_gauge_struct + .record(Epoch(123)); + assert_eq!( + Epoch(123), + metrics_service + .signer_registration_success_last_epoch_gauge_struct + .get(), + ); + } + + #[test] + fn test_runtime_cycle_success_since_startup_counter_increment_struct() { + let metrics_service = MetricsService::new().unwrap(); + assert_eq!( + 0, + metrics_service + .signer_registration_success_since_startup_counter_struct + .get(), + ); + + metrics_service + .signer_registration_success_since_startup_counter_struct + .record(); + assert_eq!( + 1, + metrics_service + .signer_registration_success_since_startup_counter_struct + .get(), + ); + } + + // Structs + // + Pas besoin de redéfinir des méthodes pour chaque compteur + // +/- ? L'appelant accède à un attribut puis .get() ou .record() + // + Possibilité de passer/retourner les types adaptés au compteur + // - Pas d'énumeration des métriques } From b5acdcdaa480eba1872940b76aa6108849e4d5a0 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Thu, 10 Oct 2024 16:05:59 +0200 Subject: [PATCH 02/23] Create a wrapper to prometheus Metrics --- mithril-signer/src/metrics/mod.rs | 1 + mithril-signer/src/metrics/service.rs | 381 ++++++++++++-------------- 2 files changed, 176 insertions(+), 206 deletions(-) diff --git a/mithril-signer/src/metrics/mod.rs b/mithril-signer/src/metrics/mod.rs index 5ea9f30ce87..0b1851081b7 100644 --- a/mithril-signer/src/metrics/mod.rs +++ b/mithril-signer/src/metrics/mod.rs @@ -6,6 +6,7 @@ mod service; pub use server::MetricsServer; pub use service::MetricsService; +pub use service::MithrilMetric; /// 'signer_registration_success_since_startup' metric name pub const SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME: &str = diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index a3078e12264..2fd6a26d95a 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -1,4 +1,4 @@ -use prometheus::{Counter, Encoder, Gauge, Opts, Registry, TextEncoder}; +use prometheus::{core::Collector, Counter, Encoder, Gauge, Opts, Registry, TextEncoder}; use slog::{debug, Logger}; use mithril_common::logging::LoggerExtensions; @@ -28,29 +28,33 @@ pub type MetricName = str; /// Type alias for a counter value. type CounterValue = u32; -/// Metrics service which is responsible for recording and exposing metrics. -pub struct MetricsService { - registry: Registry, - logger: Logger, - signer_registration_success_since_startup_counter: Box, - signer_registration_total_since_startup_counter: Box, - signer_registration_success_last_epoch_gauge: Box, - signature_registration_success_since_startup_counter: Box, - signature_registration_total_since_startup_counter: Box, - signature_registration_success_last_epoch_gauge: Box, - runtime_cycle_success_since_startup_counter: Box, - runtime_cycle_total_since_startup_counter: Box, - - signer_registration_success_since_startup_counter_struct: MetricCounter, - signer_registration_success_last_epoch_gauge_struct: MetricGauge, +/// Mithril metric +pub trait MithrilMetric { + /// Metric name + fn name(&self) -> String; + + /// Wrapped prometheus collector + fn collector(&self) -> Box; } pub struct MetricCounter { - counter: Counter, + name: String, + logger: Logger, + counter: Box, } impl MetricCounter { - fn record(&self) -> () { + fn new(logger: Logger, name: &str, help: &str) -> StdResult { + let counter = MetricCounter::create_metric_counter(name, help)?; + Ok(Self { + logger, + name: name.to_string(), + counter: Box::new(counter), + }) + } + + fn record(&self) { + debug!(self.logger, "incrementing '{}' counter", self.name); self.counter.inc(); } @@ -58,19 +62,45 @@ impl MetricCounter { self.counter.get().round() as CounterValue } - fn new(name: &str, help: &str) -> Self { - Self { - counter: MetricsService::create_metric_counter(name, help).unwrap(), - } + fn create_metric_counter(name: &MetricName, help: &str) -> StdResult { + let counter_opts = Opts::new(name, help); + let counter = Counter::with_opts(counter_opts)?; + + Ok(counter) + } +} + +impl MithrilMetric for MetricCounter { + fn collector(&self) -> Box { + self.counter.clone() + } + + fn name(&self) -> String { + self.name.clone() } } pub struct MetricGauge { - gauge: Gauge, + name: String, + logger: Logger, + gauge: Box, } impl MetricGauge { - fn record(&self, epoch: Epoch) -> () { + fn new(logger: Logger, name: &str, help: &str) -> StdResult { + let gauge = MetricGauge::create_metric_gauge(name, help)?; + Ok(Self { + logger, + name: name.to_string(), + gauge: Box::new(gauge), + }) + } + + fn record(&self, epoch: Epoch) { + debug!( + self.logger, + "set '{}' gauge value to {}", self.name, epoch.0 + ); self.gauge.set(epoch.0 as f64); } @@ -78,85 +108,122 @@ impl MetricGauge { Epoch(self.gauge.get().round() as u64) } - fn new(name: &str, help: &str) -> Self { - Self { - gauge: MetricsService::create_metric_gauge(name, help).unwrap(), - } + fn create_metric_gauge(name: &MetricName, help: &str) -> StdResult { + let gauge_opts = Opts::new(name, help); + let gauge = Gauge::with_opts(gauge_opts)?; + + Ok(gauge) + } +} +impl MithrilMetric for MetricGauge { + fn collector(&self) -> Box { + self.gauge.clone() } + fn name(&self) -> String { + self.name.clone() + } +} + +/// Metrics service which is responsible for recording and exposing metrics. +pub struct MetricsService { + registry: Registry, + signer_registration_success_since_startup_counter: MetricCounter, + signer_registration_total_since_startup_counter: MetricCounter, + signer_registration_success_last_epoch_gauge: MetricGauge, + signature_registration_success_since_startup_counter: MetricCounter, + signature_registration_total_since_startup_counter: MetricCounter, + signature_registration_success_last_epoch_gauge: MetricGauge, + runtime_cycle_success_since_startup_counter: MetricCounter, + runtime_cycle_total_since_startup_counter: MetricCounter, } impl MetricsService { /// Create a new `MetricsService` instance. pub fn new(logger: Logger) -> StdResult { + let logger = logger.new_with_component_name::(); + let registry = Registry::new(); - // Signer registration metrics - let signer_registration_success_since_startup_counter_struct = MetricCounter::new( - SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME, - SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_HELP, - ); + fn register(registry: &Registry, metric: T) -> StdResult { + registry.register(metric.collector())?; + Ok(metric) + } - let signer_registration_success_since_startup_counter = - Box::new(Self::create_metric_counter( + let signer_registration_success_since_startup_counter = register( + ®istry, + MetricCounter::new( + logger.clone(), SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME, SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_HELP, - )?); - registry.register(signer_registration_success_since_startup_counter.clone())?; + )?, + )?; - let signer_registration_total_since_startup_counter = - Box::new(Self::create_metric_counter( + let signer_registration_total_since_startup_counter = register( + ®istry, + MetricCounter::new( + logger.clone(), SIGNER_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME, SIGNER_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_HELP, - )?); - registry.register(signer_registration_total_since_startup_counter.clone())?; - - let signer_registration_success_last_epoch_gauge_struct = MetricGauge::new( - SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME, - SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP, - ); - let signer_registration_success_last_epoch_gauge = Box::new(Self::create_metric_gauge( - SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME, - SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP, - )?); - registry.register(signer_registration_success_last_epoch_gauge.clone())?; - + )?, + )?; + + let signer_registration_success_last_epoch_gauge = register( + ®istry, + MetricGauge::new( + logger.clone(), + SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME, + SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP, + )?, + )?; // Signature registration metrics - let signature_registration_success_since_startup_counter = - Box::new(Self::create_metric_counter( + + let signature_registration_success_since_startup_counter = register( + ®istry, + MetricCounter::new( + logger.clone(), SIGNATURE_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME, SIGNATURE_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_HELP, - )?); - registry.register(signature_registration_success_since_startup_counter.clone())?; + )?, + )?; - let signature_registration_total_since_startup_counter = - Box::new(Self::create_metric_counter( + let signature_registration_total_since_startup_counter = register( + ®istry, + MetricCounter::new( + logger.clone(), SIGNATURE_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME, SIGNATURE_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_HELP, - )?); - registry.register(signature_registration_total_since_startup_counter.clone())?; - - let signature_registration_success_last_epoch_gauge = Box::new(Self::create_metric_gauge( - SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME, - SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP, - )?); - registry.register(signature_registration_success_last_epoch_gauge.clone())?; + )?, + )?; + + let signature_registration_success_last_epoch_gauge = register( + ®istry, + MetricGauge::new( + logger.clone(), + SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME, + SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP, + )?, + )?; // Runtime cycle metrics - let runtime_cycle_success_since_startup_counter = Box::new(Self::create_metric_counter( - RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_NAME, - RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_HELP, - )?); - registry.register(runtime_cycle_success_since_startup_counter.clone())?; - - let runtime_cycle_total_since_startup_counter = Box::new(Self::create_metric_counter( - RUNTIME_CYCLE_TOTAL_SINCE_STARTUP_METRIC_NAME, - RUNTIME_CYCLE_TOTAL_SINCE_STARTUP_METRIC_HELP, - )?); - registry.register(runtime_cycle_total_since_startup_counter.clone())?; + let runtime_cycle_success_since_startup_counter = register( + ®istry, + MetricCounter::new( + logger.clone(), + RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_NAME, + RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_HELP, + )?, + )?; + let runtime_cycle_total_since_startup_counter = register( + ®istry, + MetricCounter::new( + logger.clone(), + RUNTIME_CYCLE_TOTAL_SINCE_STARTUP_METRIC_NAME, + RUNTIME_CYCLE_TOTAL_SINCE_STARTUP_METRIC_HELP, + )?, + )?; Ok(Self { registry, - logger: logger.new_with_component_name::(), signer_registration_success_since_startup_counter, signer_registration_total_since_startup_counter, signer_registration_success_last_epoch_gauge, @@ -165,25 +232,9 @@ impl MetricsService { signature_registration_success_last_epoch_gauge, runtime_cycle_success_since_startup_counter, runtime_cycle_total_since_startup_counter, - signer_registration_success_since_startup_counter_struct, - signer_registration_success_last_epoch_gauge_struct, }) } - fn create_metric_counter(name: &MetricName, help: &str) -> StdResult { - let counter_opts = Opts::new(name, help); - let counter = Counter::with_opts(counter_opts)?; - - Ok(counter) - } - - fn create_metric_gauge(name: &MetricName, help: &str) -> StdResult { - let gauge_opts = Opts::new(name, help); - let gauge = Gauge::with_opts(gauge_opts)?; - - Ok(gauge) - } - /// Export the metrics as a string with the Open Metrics standard format. /// These metrics can be exposed on an HTTP server. pub fn export_metrics(&self) -> StdResult { @@ -197,136 +248,90 @@ impl MetricsService { /// Increment the `signer_registration_success_since_startup` counter. pub fn signer_registration_success_since_startup_counter_increment(&self) { - debug!( - self.logger, - "incrementing 'signer_registration_success_since_startup' counter" - ); - self.signer_registration_success_since_startup_counter.inc(); + self.signer_registration_success_since_startup_counter + .record(); } /// Get the `signer_registration_success_since_startup` counter. pub fn signer_registration_success_since_startup_counter_get(&self) -> CounterValue { - self.signer_registration_success_since_startup_counter - .get() - .round() as CounterValue + self.signer_registration_success_since_startup_counter.get() } /// Increment the `signer_registration_total_since_startup` counter. pub fn signer_registration_total_since_startup_counter_increment(&self) { - debug!( - self.logger, - "incrementing 'signer_registration_total_since_startup' counter" - ); - self.signer_registration_total_since_startup_counter.inc(); + self.signer_registration_total_since_startup_counter + .record(); } /// Get the `signer_registration_total_since_startup` counter. pub fn signer_registration_total_since_startup_counter_get(&self) -> CounterValue { - self.signer_registration_total_since_startup_counter - .get() - .round() as CounterValue + self.signer_registration_total_since_startup_counter.get() } /// Set the `signer_registration_success_last_epoch` gauge value. pub fn signer_registration_success_last_epoch_gauge_set(&self, value: Epoch) { - debug!( - self.logger, - "set 'signer_registration_success_last_epoch_set' gauge value to {value}" - ); self.signer_registration_success_last_epoch_gauge - .set(value.0 as f64); + .record(value); } /// Get the `signer_registration_success_last_epoch` gauge value. pub fn signer_registration_success_last_epoch_gauge_get(&self) -> Epoch { - Epoch( - self.signer_registration_success_last_epoch_gauge - .get() - .round() as u64, - ) + self.signer_registration_success_last_epoch_gauge.get() } /// Increment the `signature_registration_success_since_startup` counter. pub fn signature_registration_success_since_startup_counter_increment(&self) { - debug!( - self.logger, - "incrementing 'signature_registration_success_since_startup' counter" - ); self.signature_registration_success_since_startup_counter - .inc(); + .record(); } /// Get the `signature_registration_success_since_startup` counter. pub fn signature_registration_success_since_startup_counter_get(&self) -> CounterValue { self.signature_registration_success_since_startup_counter .get() - .round() as CounterValue } /// Increment the `signature_registration_total_since_startup` counter. pub fn signature_registration_total_since_startup_counter_increment(&self) { - debug!( - self.logger, - "incrementing 'signature_registration_total_since_startup' counter" - ); self.signature_registration_total_since_startup_counter - .inc(); + .record(); } /// Get the `signature_registration_total_since_startup` counter. pub fn signature_registration_total_since_startup_counter_get(&self) -> CounterValue { self.signature_registration_total_since_startup_counter .get() - .round() as CounterValue } /// Set the `signature_registration_success_last_epoch` gauge value. pub fn signature_registration_success_last_epoch_gauge_set(&self, value: Epoch) { - debug!( - self.logger, - "set 'signature_registration_success_last_epoch_set' gauge value to {value}" - ); self.signature_registration_success_last_epoch_gauge - .set(value.0 as f64); + .record(value); } /// Get the `signature_registration_success_last_epoch` gauge value. pub fn signature_registration_success_last_epoch_gauge_get(&self) -> Epoch { - Epoch( - self.signature_registration_success_last_epoch_gauge - .get() - .round() as u64, - ) + self.signature_registration_success_last_epoch_gauge.get() } /// Increment the `runtime_cycle_total_since_startup` counter. pub fn runtime_cycle_total_since_startup_counter_increment(&self) { - debug!( - self.logger, - "incrementing 'runtime_cycle_total_since_startup' counter" - ); - self.runtime_cycle_total_since_startup_counter.inc(); + self.runtime_cycle_total_since_startup_counter.record(); } /// Get the `runtime_cycle_total_since_startup` counter. pub fn runtime_cycle_total_since_startup_counter_get(&self) -> CounterValue { - self.runtime_cycle_total_since_startup_counter.get().round() as CounterValue + self.runtime_cycle_total_since_startup_counter.get() } /// Increment the `runtime_cycle_success_since_startup` counter. pub fn runtime_cycle_success_since_startup_counter_increment(&self) { - debug!( - self.logger, - "incrementing 'runtime_cycle_success_since_startup' counter" - ); - self.runtime_cycle_success_since_startup_counter.inc(); + self.runtime_cycle_success_since_startup_counter.record(); } /// Get the `runtime_cycle_success_since_startup` counter. pub fn runtime_cycle_success_since_startup_counter_get(&self) -> CounterValue { - self.runtime_cycle_success_since_startup_counter - .get() - .round() as CounterValue + self.runtime_cycle_success_since_startup_counter.get() } } @@ -393,6 +398,20 @@ mod tests { assert_eq!(parsed_metrics_expected, parsed_metrics); } + #[test] + fn test_retrieve_metric_by_name() { + let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); + let name = metrics_service + .runtime_cycle_success_since_startup_counter + .name(); + assert_eq!(name, RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_NAME); + + let name = metrics_service + .signature_registration_success_last_epoch_gauge + .name(); + assert_eq!(name, SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME); + } + #[test] fn test_signer_registration_success_since_startup_counter_increment() { let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); @@ -512,54 +531,4 @@ mod tests { metrics_service.runtime_cycle_total_since_startup_counter_get(), ); } - - //// - - #[test] - fn test_signature_registration_success_last_epoch_gauge_set_struct() { - let metrics_service = MetricsService::new().unwrap(); - assert_eq!( - Epoch(0), - metrics_service - .signer_registration_success_last_epoch_gauge_struct - .get(), - ); - - metrics_service - .signer_registration_success_last_epoch_gauge_struct - .record(Epoch(123)); - assert_eq!( - Epoch(123), - metrics_service - .signer_registration_success_last_epoch_gauge_struct - .get(), - ); - } - - #[test] - fn test_runtime_cycle_success_since_startup_counter_increment_struct() { - let metrics_service = MetricsService::new().unwrap(); - assert_eq!( - 0, - metrics_service - .signer_registration_success_since_startup_counter_struct - .get(), - ); - - metrics_service - .signer_registration_success_since_startup_counter_struct - .record(); - assert_eq!( - 1, - metrics_service - .signer_registration_success_since_startup_counter_struct - .get(), - ); - } - - // Structs - // + Pas besoin de redéfinir des méthodes pour chaque compteur - // +/- ? L'appelant accède à un attribut puis .get() ou .record() - // + Possibilité de passer/retourner les types adaptés au compteur - // - Pas d'énumeration des métriques } From 43f3113bd12fdf3e7f17ff61dc799e7fc3c57683 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Thu, 10 Oct 2024 17:22:25 +0200 Subject: [PATCH 03/23] Extract common part from the service file --- mithril-signer/src/metrics/commons.rs | 106 +++++++++++++++++++++++++ mithril-signer/src/metrics/mod.rs | 3 +- mithril-signer/src/metrics/service.rs | 108 +------------------------- 3 files changed, 112 insertions(+), 105 deletions(-) create mode 100644 mithril-signer/src/metrics/commons.rs diff --git a/mithril-signer/src/metrics/commons.rs b/mithril-signer/src/metrics/commons.rs new file mode 100644 index 00000000000..0f3012b0478 --- /dev/null +++ b/mithril-signer/src/metrics/commons.rs @@ -0,0 +1,106 @@ +use prometheus::{core::Collector, Counter, Gauge, Opts}; +use slog::{debug, Logger}; + +use mithril_common::{entities::Epoch, StdResult}; + +/// Type alias for a metric name. +pub type MetricName = str; + +/// Type alias for a counter value. +pub type CounterValue = u32; + +/// Mithril metric +pub trait MithrilMetric { + /// Metric name + fn name(&self) -> String; + + /// Wrapped prometheus collector + fn collector(&self) -> Box; +} + +pub struct MetricCounter { + name: String, + logger: Logger, + counter: Box, +} + +impl MetricCounter { + pub fn new(logger: Logger, name: &str, help: &str) -> StdResult { + let counter = MetricCounter::create_metric_counter(name, help)?; + Ok(Self { + logger, + name: name.to_string(), + counter: Box::new(counter), + }) + } + + pub fn record(&self) { + debug!(self.logger, "incrementing '{}' counter", self.name); + self.counter.inc(); + } + + pub fn get(&self) -> CounterValue { + self.counter.get().round() as CounterValue + } + + fn create_metric_counter(name: &MetricName, help: &str) -> StdResult { + let counter_opts = Opts::new(name, help); + let counter = Counter::with_opts(counter_opts)?; + + Ok(counter) + } +} + +impl MithrilMetric for MetricCounter { + fn collector(&self) -> Box { + self.counter.clone() + } + + fn name(&self) -> String { + self.name.clone() + } +} + +pub struct MetricGauge { + name: String, + logger: Logger, + gauge: Box, +} + +impl MetricGauge { + pub fn new(logger: Logger, name: &str, help: &str) -> StdResult { + let gauge = MetricGauge::create_metric_gauge(name, help)?; + Ok(Self { + logger, + name: name.to_string(), + gauge: Box::new(gauge), + }) + } + + pub fn record(&self, epoch: Epoch) { + debug!( + self.logger, + "set '{}' gauge value to {}", self.name, epoch.0 + ); + self.gauge.set(epoch.0 as f64); + } + + pub fn get(&self) -> Epoch { + Epoch(self.gauge.get().round() as u64) + } + + fn create_metric_gauge(name: &MetricName, help: &str) -> StdResult { + let gauge_opts = Opts::new(name, help); + let gauge = Gauge::with_opts(gauge_opts)?; + + Ok(gauge) + } +} +impl MithrilMetric for MetricGauge { + fn collector(&self) -> Box { + self.gauge.clone() + } + fn name(&self) -> String { + self.name.clone() + } +} diff --git a/mithril-signer/src/metrics/mod.rs b/mithril-signer/src/metrics/mod.rs index 0b1851081b7..22caa608820 100644 --- a/mithril-signer/src/metrics/mod.rs +++ b/mithril-signer/src/metrics/mod.rs @@ -1,12 +1,13 @@ //! metrics module. //! This module contains the signer metrics service and metrics server. +mod commons; mod server; mod service; +pub use commons::MithrilMetric; pub use server::MetricsServer; pub use service::MetricsService; -pub use service::MithrilMetric; /// 'signer_registration_success_since_startup' metric name pub const SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME: &str = diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index 2fd6a26d95a..8919da7f548 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -1,9 +1,11 @@ -use prometheus::{core::Collector, Counter, Encoder, Gauge, Opts, Registry, TextEncoder}; -use slog::{debug, Logger}; +use prometheus::{Encoder, Registry, TextEncoder}; +use slog::Logger; use mithril_common::logging::LoggerExtensions; use mithril_common::{entities::Epoch, StdResult}; +use crate::metrics::commons::{CounterValue, MetricCounter, MetricGauge, MithrilMetric}; + use super::{ RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_HELP, RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_NAME, RUNTIME_CYCLE_TOTAL_SINCE_STARTUP_METRIC_HELP, @@ -22,108 +24,6 @@ use super::{ SIGNER_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME, }; -/// Type alias for a metric name. -pub type MetricName = str; - -/// Type alias for a counter value. -type CounterValue = u32; - -/// Mithril metric -pub trait MithrilMetric { - /// Metric name - fn name(&self) -> String; - - /// Wrapped prometheus collector - fn collector(&self) -> Box; -} - -pub struct MetricCounter { - name: String, - logger: Logger, - counter: Box, -} - -impl MetricCounter { - fn new(logger: Logger, name: &str, help: &str) -> StdResult { - let counter = MetricCounter::create_metric_counter(name, help)?; - Ok(Self { - logger, - name: name.to_string(), - counter: Box::new(counter), - }) - } - - fn record(&self) { - debug!(self.logger, "incrementing '{}' counter", self.name); - self.counter.inc(); - } - - fn get(&self) -> CounterValue { - self.counter.get().round() as CounterValue - } - - fn create_metric_counter(name: &MetricName, help: &str) -> StdResult { - let counter_opts = Opts::new(name, help); - let counter = Counter::with_opts(counter_opts)?; - - Ok(counter) - } -} - -impl MithrilMetric for MetricCounter { - fn collector(&self) -> Box { - self.counter.clone() - } - - fn name(&self) -> String { - self.name.clone() - } -} - -pub struct MetricGauge { - name: String, - logger: Logger, - gauge: Box, -} - -impl MetricGauge { - fn new(logger: Logger, name: &str, help: &str) -> StdResult { - let gauge = MetricGauge::create_metric_gauge(name, help)?; - Ok(Self { - logger, - name: name.to_string(), - gauge: Box::new(gauge), - }) - } - - fn record(&self, epoch: Epoch) { - debug!( - self.logger, - "set '{}' gauge value to {}", self.name, epoch.0 - ); - self.gauge.set(epoch.0 as f64); - } - - fn get(&self) -> Epoch { - Epoch(self.gauge.get().round() as u64) - } - - fn create_metric_gauge(name: &MetricName, help: &str) -> StdResult { - let gauge_opts = Opts::new(name, help); - let gauge = Gauge::with_opts(gauge_opts)?; - - Ok(gauge) - } -} -impl MithrilMetric for MetricGauge { - fn collector(&self) -> Box { - self.gauge.clone() - } - fn name(&self) -> String { - self.name.clone() - } -} - /// Metrics service which is responsible for recording and exposing metrics. pub struct MetricsService { registry: Registry, From b1e0768628732f13c108a0e0663ff8415c0c7710 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Thu, 10 Oct 2024 17:59:41 +0200 Subject: [PATCH 04/23] Remove metric constants --- mithril-signer/src/metrics/mod.rs | 56 -------- mithril-signer/src/metrics/service.rs | 136 ++++++++++-------- .../test_extensions/state_machine_tester.rs | 24 +++- 3 files changed, 98 insertions(+), 118 deletions(-) diff --git a/mithril-signer/src/metrics/mod.rs b/mithril-signer/src/metrics/mod.rs index 22caa608820..4b0e4b0da75 100644 --- a/mithril-signer/src/metrics/mod.rs +++ b/mithril-signer/src/metrics/mod.rs @@ -8,59 +8,3 @@ mod service; pub use commons::MithrilMetric; pub use server::MetricsServer; pub use service::MetricsService; - -/// 'signer_registration_success_since_startup' metric name -pub const SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME: &str = - "mithril_signer_signer_registration_success_since_startup"; -/// 'signer_registration_success_since_startup' metric help -pub const SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_HELP: &str = - "Number of successful signer registrations since startup on a Mithril signer node"; - -/// 'signer_registration_total_since_startup' metric name -pub const SIGNER_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME: &str = - "mithril_signer_signer_registration_total_since_startup"; -/// 'signer_registration_total_since_startup' metric help -pub const SIGNER_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_HELP: &str = - "Number of signer registrations since startup on a Mithril signer node"; - -/// 'signer_registration_success_last_epoch' metric name -pub const SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME: &str = - "mithril_signer_signer_registration_success_last_epoch"; -/// 'signer_registration_success_last_epoch' metric help -pub const SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP: &str = - "Latest epoch at which signer successfully registered on a Mithril signer node"; - -/// 'signature_registration_success_since_startup' metric name -pub const SIGNATURE_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME: &str = - "mithril_signer_signature_registration_success_since_startup"; -/// 'signature_registration_success_since_startup' metric help -pub const SIGNATURE_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_HELP: &str = - "Number of successful signature registrations since startup on a Mithril signer node"; - -/// 'signature_registration_total_since_startup' metric name -pub const SIGNATURE_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME: &str = - "mithril_signer_signature_registration_total_since_startup"; -/// 'signature_registration_total_since_startup' metric help -pub const SIGNATURE_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_HELP: &str = - "Number of signature registrations since startup on a Mithril signer node"; - -/// 'signature_registration_success_last_epoch' metric name -pub const SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME: &str = - "mithril_signer_signature_registration_success_last_epoch"; -/// 'signature_registration_success_last_epoch' metric help -pub const SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP: &str = - "Latest epoch at which signature successfully registered on a Mithril signer node"; - -/// 'runtime_cycle_success_since_startup' metric name -pub const RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_NAME: &str = - "mithril_signer_runtime_cycle_success_since_startup"; -/// 'runtime_cycle_success_since_startup' metric help -pub const RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_HELP: &str = - "Number of successful runtime cycles since startup on a Mithril signer node"; - -/// 'runtime_cycle_total_since_startup' metric name -pub const RUNTIME_CYCLE_TOTAL_SINCE_STARTUP_METRIC_NAME: &str = - "mithril_signer_runtime_cycle_total_since_startup"; -/// 'runtime_cycle_total_since_startup' metric help -pub const RUNTIME_CYCLE_TOTAL_SINCE_STARTUP_METRIC_HELP: &str = - "Number of runtime cycles since startup on a Mithril signer node"; diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index 8919da7f548..61e6326cd56 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -6,24 +6,6 @@ use mithril_common::{entities::Epoch, StdResult}; use crate::metrics::commons::{CounterValue, MetricCounter, MetricGauge, MithrilMetric}; -use super::{ - RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_HELP, - RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_NAME, RUNTIME_CYCLE_TOTAL_SINCE_STARTUP_METRIC_HELP, - RUNTIME_CYCLE_TOTAL_SINCE_STARTUP_METRIC_NAME, - SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP, - SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME, - SIGNATURE_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_HELP, - SIGNATURE_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME, - SIGNATURE_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_HELP, - SIGNATURE_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME, - SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP, - SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME, - SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_HELP, - SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME, - SIGNER_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_HELP, - SIGNER_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME, -}; - /// Metrics service which is responsible for recording and exposing metrics. pub struct MetricsService { registry: Registry, @@ -53,8 +35,8 @@ impl MetricsService { ®istry, MetricCounter::new( logger.clone(), - SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME, - SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_HELP, + "mithril_signer_signer_registration_success_since_startup", + "Number of successful signer registrations since startup on a Mithril signer node", )?, )?; @@ -62,8 +44,8 @@ impl MetricsService { ®istry, MetricCounter::new( logger.clone(), - SIGNER_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME, - SIGNER_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_HELP, + "mithril_signer_signer_registration_total_since_startup", + "Number of signer registrations since startup on a Mithril signer node", )?, )?; @@ -71,8 +53,8 @@ impl MetricsService { ®istry, MetricGauge::new( logger.clone(), - SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME, - SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP, + "mithril_signer_signer_registration_success_last_epoch", + "Latest epoch at which signer successfully registered on a Mithril signer node", )?, )?; // Signature registration metrics @@ -81,8 +63,8 @@ impl MetricsService { ®istry, MetricCounter::new( logger.clone(), - SIGNATURE_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME, - SIGNATURE_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_HELP, + "mithril_signer_signature_registration_success_since_startup", + "Number of successful signature registrations since startup on a Mithril signer node", )?, )?; @@ -90,8 +72,8 @@ impl MetricsService { ®istry, MetricCounter::new( logger.clone(), - SIGNATURE_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME, - SIGNATURE_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_HELP, + "mithril_signer_signature_registration_total_since_startup", + "Number of signature registrations since startup on a Mithril signer node", )?, )?; @@ -99,8 +81,8 @@ impl MetricsService { ®istry, MetricGauge::new( logger.clone(), - SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME, - SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_HELP, + "mithril_signer_signature_registration_success_last_epoch", + "Latest epoch at which signature successfully registered on a Mithril signer node", )?, )?; @@ -109,16 +91,16 @@ impl MetricsService { ®istry, MetricCounter::new( logger.clone(), - RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_NAME, - RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_HELP, + "mithril_signer_runtime_cycle_success_since_startup", + "Number of successful runtime cycles since startup on a Mithril signer node", )?, )?; let runtime_cycle_total_since_startup_counter = register( ®istry, MetricCounter::new( logger.clone(), - RUNTIME_CYCLE_TOTAL_SINCE_STARTUP_METRIC_NAME, - RUNTIME_CYCLE_TOTAL_SINCE_STARTUP_METRIC_HELP, + "mithril_signer_runtime_cycle_total_since_startup", + "Number of runtime cycles since startup on a Mithril signer node", )?, )?; @@ -233,6 +215,46 @@ impl MetricsService { pub fn runtime_cycle_success_since_startup_counter_get(&self) -> CounterValue { self.runtime_cycle_success_since_startup_counter.get() } + + /// Get the `signer_registration_success_since_startup_counter` counter. + pub fn get_signer_registration_success_since_startup_counter(&self) -> &MetricCounter { + &self.signer_registration_success_since_startup_counter + } + + /// Get the `signer_registration_total_since_startup_counter` counter. + pub fn get_signer_registration_total_since_startup_counter(&self) -> &MetricCounter { + &self.signer_registration_total_since_startup_counter + } + + /// Get the `signer_registration_success_last_epoch_gauge` counter. + pub fn get_signer_registration_success_last_epoch_gauge(&self) -> &MetricGauge { + &self.signer_registration_success_last_epoch_gauge + } + + /// Get the `signature_registration_success_since_startup_counter` counter. + pub fn get_signature_registration_success_since_startup_counter(&self) -> &MetricCounter { + &self.signature_registration_success_since_startup_counter + } + + /// Get the `signature_registration_total_since_startup_counter` counter. + pub fn get_signature_registration_total_since_startup_counter(&self) -> &MetricCounter { + &self.signature_registration_total_since_startup_counter + } + + /// Get the `signature_registration_success_last_epoch_gauge` counter. + pub fn get_signature_registration_success_last_epoch_gauge(&self) -> &MetricGauge { + &self.signature_registration_success_last_epoch_gauge + } + + /// Get the `runtime_cycle_success_since_startup_counter` counter. + pub fn get_runtime_cycle_success_since_startup_counter(&self) -> &MetricCounter { + &self.runtime_cycle_success_since_startup_counter + } + + /// Get the `runtime_cycle_total_since_startup_counter` counter. + pub fn get_runtime_cycle_total_since_startup_counter(&self) -> &MetricCounter { + &self.runtime_cycle_total_since_startup_counter + } } #[cfg(test)] @@ -263,55 +285,57 @@ mod tests { let parsed_metrics_expected = BTreeMap::from([ ( - RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_NAME.to_string(), + metrics_service + .get_runtime_cycle_success_since_startup_counter() + .name(), Value::Counter(0.0), ), ( - RUNTIME_CYCLE_TOTAL_SINCE_STARTUP_METRIC_NAME.to_string(), + metrics_service + .get_runtime_cycle_total_since_startup_counter() + .name(), Value::Counter(0.0), ), ( - SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME.to_string(), + metrics_service + .get_signature_registration_success_last_epoch_gauge() + .name(), Value::Gauge(0.0), ), ( - SIGNATURE_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME.to_string(), + metrics_service + .get_signature_registration_success_since_startup_counter() + .name(), Value::Counter(0.0), ), ( - SIGNATURE_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME.to_string(), + metrics_service + .get_signature_registration_total_since_startup_counter() + .name(), Value::Counter(0.0), ), ( - SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME.to_string(), + metrics_service + .get_signer_registration_success_last_epoch_gauge() + .name(), Value::Gauge(0.0), ), ( - SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME.to_string(), + metrics_service + .get_signer_registration_success_since_startup_counter() + .name(), Value::Counter(0.0), ), ( - SIGNER_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME.to_string(), + metrics_service + .get_signer_registration_total_since_startup_counter() + .name(), Value::Counter(0.0), ), ]); assert_eq!(parsed_metrics_expected, parsed_metrics); } - #[test] - fn test_retrieve_metric_by_name() { - let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); - let name = metrics_service - .runtime_cycle_success_since_startup_counter - .name(); - assert_eq!(name, RUNTIME_CYCLE_SUCCESS_SINCE_STARTUP_METRIC_NAME); - - let name = metrics_service - .signature_registration_success_last_epoch_gauge - .name(); - assert_eq!(name, SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME); - } - #[test] fn test_signer_registration_success_since_startup_counter_increment() { let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); diff --git a/mithril-signer/tests/test_extensions/state_machine_tester.rs b/mithril-signer/tests/test_extensions/state_machine_tester.rs index 23ac348b13a..215fe76eb19 100644 --- a/mithril-signer/tests/test_extensions/state_machine_tester.rs +++ b/mithril-signer/tests/test_extensions/state_machine_tester.rs @@ -695,27 +695,39 @@ impl StateMachineTester { let metrics = Self::parse_exported_metrics(&self.metrics_service)?; let mut expected_metrics = Self::parse_exported_metrics(&self.expected_metrics_service)?; expected_metrics.insert( - SIGNATURE_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME.to_string(), + self.metrics_service + .get_signature_registration_success_last_epoch_gauge() + .name(), Value::Gauge(self.current_epoch().await?.0 as f64), ); expected_metrics.insert( - SIGNATURE_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME.to_string(), + self.metrics_service + .get_signature_registration_success_since_startup_counter() + .name(), Value::Counter(total_signature_registrations_expected as f64), ); expected_metrics.insert( - SIGNATURE_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME.to_string(), + self.metrics_service + .get_signature_registration_total_since_startup_counter() + .name(), Value::Counter(total_signature_registrations_expected as f64), ); expected_metrics.insert( - SIGNER_REGISTRATION_SUCCESS_LAST_EPOCH_METRIC_NAME.to_string(), + self.metrics_service + .get_signer_registration_success_last_epoch_gauge() + .name(), Value::Gauge(self.current_epoch().await?.0 as f64), ); expected_metrics.insert( - SIGNER_REGISTRATION_SUCCESS_SINCE_STARTUP_METRIC_NAME.to_string(), + self.metrics_service + .get_signer_registration_success_since_startup_counter() + .name(), Value::Counter(total_signer_registrations_expected as f64), ); expected_metrics.insert( - SIGNER_REGISTRATION_TOTAL_SINCE_STARTUP_METRIC_NAME.to_string(), + self.metrics_service + .get_signer_registration_total_since_startup_counter() + .name(), Value::Counter(total_signer_registrations_expected as f64), ); self.assert( From 941db951c42e1eeb31ef5ad0f249b3f1ff40f567 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Fri, 11 Oct 2024 11:27:48 +0200 Subject: [PATCH 05/23] Create a `internal/mithril-metric` crate --- Cargo.lock | 16 ++++++ Cargo.toml | 1 + internal/mithril-metric/Cargo.toml | 28 ++++++++++ internal/mithril-metric/Makefile | 19 +++++++ internal/mithril-metric/README.md | 5 ++ .../mithril-metric/src}/commons.rs | 51 +++++++++++++++++++ internal/mithril-metric/src/lib.rs | 9 ++++ .../mithril-metric/src}/server.rs | 44 +++++++++------- mithril-signer/Cargo.toml | 1 + mithril-signer/src/main.rs | 3 +- mithril-signer/src/metrics/mod.rs | 4 -- mithril-signer/src/metrics/service.rs | 27 +++++----- .../test_extensions/state_machine_tester.rs | 2 +- 13 files changed, 175 insertions(+), 35 deletions(-) create mode 100644 internal/mithril-metric/Cargo.toml create mode 100644 internal/mithril-metric/Makefile create mode 100644 internal/mithril-metric/README.md rename {mithril-signer/src/metrics => internal/mithril-metric/src}/commons.rs (63%) create mode 100644 internal/mithril-metric/src/lib.rs rename {mithril-signer/src/metrics => internal/mithril-metric/src}/server.rs (80%) diff --git a/Cargo.lock b/Cargo.lock index 87f991253bd..c50db8c8834 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3803,6 +3803,21 @@ dependencies = [ "tokio-util", ] +[[package]] +name = "mithril-metric" +version = "0.0.1" +dependencies = [ + "anyhow", + "axum", + "mithril-common", + "prometheus", + "reqwest 0.12.7", + "slog", + "slog-async", + "slog-term", + "tokio", +] + [[package]] name = "mithril-persistence" version = "0.2.30" @@ -3863,6 +3878,7 @@ dependencies = [ "httpmock", "mithril-common", "mithril-doc", + "mithril-metric", "mithril-persistence", "mockall", "openssl", diff --git a/Cargo.toml b/Cargo.toml index dbb0c960f1d..a75d9437ed8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ members = [ "internal/mithril-build-script", "internal/mithril-doc", "internal/mithril-doc-derive", + "internal/mithril-metric", "internal/mithril-persistence", "mithril-aggregator", "mithril-client", diff --git a/internal/mithril-metric/Cargo.toml b/internal/mithril-metric/Cargo.toml new file mode 100644 index 00000000000..f8ff6f616b1 --- /dev/null +++ b/internal/mithril-metric/Cargo.toml @@ -0,0 +1,28 @@ +[package] +name = "mithril-metric" +version = "0.0.1" +description = "Common tools to expose metrics." +authors = { workspace = true } +edition = { workspace = true } +homepage = { workspace = true } +license = { workspace = true } +repository = { workspace = true } + +[lib] +crate-type = ["lib", "cdylib", "staticlib"] + +[dependencies] +anyhow = "1.0.86" +axum = "0.7.5" +mithril-common = { path = "../../mithril-common", features = ["full"] } +prometheus = "0.13.4" +reqwest = { version = "0.12.7", features = ["json", "stream"] } +slog = { version = "2.7.0", features = [ + "max_level_trace", + "release_max_level_debug", +] } +tokio = { version = "1.40.0", features = ["full"] } + +[dev-dependencies] +slog-async = "2.8.0" +slog-term = "2.9.1" \ No newline at end of file diff --git a/internal/mithril-metric/Makefile b/internal/mithril-metric/Makefile new file mode 100644 index 00000000000..adc3e3d5c11 --- /dev/null +++ b/internal/mithril-metric/Makefile @@ -0,0 +1,19 @@ +.PHONY: all build test check doc + +CARGO = cargo + +all: test build + +build: + ${CARGO} build --release + +test: + ${CARGO} test + +check: + ${CARGO} check --release --all-features --all-targets + ${CARGO} clippy --release --all-features --all-targets + ${CARGO} fmt --check + +doc: + ${CARGO} doc --no-deps --open --features full diff --git a/internal/mithril-metric/README.md b/internal/mithril-metric/README.md new file mode 100644 index 00000000000..6fc6d0a05d7 --- /dev/null +++ b/internal/mithril-metric/README.md @@ -0,0 +1,5 @@ +# Mithril-metric + +**This is a work in progress** 🛠 + +This crate contains material to expose metrics. diff --git a/mithril-signer/src/metrics/commons.rs b/internal/mithril-metric/src/commons.rs similarity index 63% rename from mithril-signer/src/metrics/commons.rs rename to internal/mithril-metric/src/commons.rs index 0f3012b0478..febff723900 100644 --- a/mithril-signer/src/metrics/commons.rs +++ b/internal/mithril-metric/src/commons.rs @@ -104,3 +104,54 @@ impl MithrilMetric for MetricGauge { self.name.clone() } } + +#[cfg(test)] +pub mod test_tools { + use std::{io, sync::Arc}; + + use slog::{Drain, Logger}; + use slog_async::Async; + use slog_term::{CompactFormat, PlainDecorator}; + pub struct TestLogger; + + impl TestLogger { + fn from_writer(writer: W) -> Logger { + let decorator = PlainDecorator::new(writer); + let drain = CompactFormat::new(decorator).build().fuse(); + let drain = Async::new(drain).build().fuse(); + Logger::root(Arc::new(drain), slog::o!()) + } + + pub fn stdout() -> Logger { + Self::from_writer(slog_term::TestStdoutWriter) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use test_tools::TestLogger; + + #[test] + fn test_metric_counter_can_be_incremented() { + let metric = + MetricCounter::new(TestLogger::stdout(), "test_counter", "test counter help").unwrap(); + assert_eq!(metric.name(), "test_counter"); + assert_eq!(metric.get(), 0); + + metric.record(); + assert_eq!(metric.get(), 1); + } + + #[test] + fn test_metric_gauge_can_be_set() { + let metric = + MetricGauge::new(TestLogger::stdout(), "test_gauge", "test gauge help").unwrap(); + assert_eq!(metric.name(), "test_gauge"); + assert_eq!(metric.get(), Epoch(0)); + + metric.record(Epoch(12)); + assert_eq!(metric.get(), Epoch(12)); + } +} diff --git a/internal/mithril-metric/src/lib.rs b/internal/mithril-metric/src/lib.rs new file mode 100644 index 00000000000..7a23044b7b7 --- /dev/null +++ b/internal/mithril-metric/src/lib.rs @@ -0,0 +1,9 @@ +//! metrics module. +//! This module contains the tools to create a metrics service and a metrics server. + +pub mod commons; +mod server; + +pub use commons::*; +pub use server::MetricsServer; +pub use server::MetricsServiceTrait; diff --git a/mithril-signer/src/metrics/server.rs b/internal/mithril-metric/src/server.rs similarity index 80% rename from mithril-signer/src/metrics/server.rs rename to internal/mithril-metric/src/server.rs index 9365b49ddd8..9bb2b7b37a7 100644 --- a/mithril-signer/src/metrics/server.rs +++ b/internal/mithril-metric/src/server.rs @@ -13,7 +13,9 @@ use tokio::sync::oneshot::Receiver; use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; -use crate::MetricsService; +pub trait MetricsServiceTrait { + fn export_metrics(&self) -> StdResult; +} /// Metrics server errors #[derive(Debug)] @@ -34,26 +36,21 @@ impl IntoResponse for MetricsServerError { } /// The MetricsServer is responsible for exposing the metrics of the signer. -pub struct MetricsServer { +pub struct MetricsServer { server_port: u16, server_ip: String, - metrics_service: Arc, + metrics_service: Arc, logger: Logger, } -struct RouterState { - metrics_service: Arc, +struct RouterState { + metrics_service: Arc, logger: Logger, } -impl MetricsServer { +impl MetricsServer { /// Create a new MetricsServer instance. - pub fn new( - server_ip: &str, - server_port: u16, - metrics_service: Arc, - logger: Logger, - ) -> Self { + pub fn new(server_ip: &str, server_port: u16, metrics_service: Arc, logger: Logger) -> Self { Self { server_port, server_ip: server_ip.to_string(), @@ -81,7 +78,7 @@ impl MetricsServer { let app = Router::new() .route( "/metrics", - get(|State(state): State>| async move { + get(|State(state): State>>| async move { state.metrics_service.export_metrics().map_err(|e| { error!(state.logger, "error exporting metrics"; "error" => ?e); MetricsServerError::Internal(e) @@ -110,20 +107,33 @@ impl MetricsServer { #[cfg(test)] mod tests { + use crate::test_tools::TestLogger; use anyhow::anyhow; use reqwest::StatusCode; use std::time::Duration; use tokio::{sync::oneshot, task::yield_now, time::sleep}; - use crate::test_tools::TestLogger; - use super::*; + pub struct PseudoMetricsService {} + + impl PseudoMetricsService { + pub fn new() -> Self { + Self {} + } + } + + impl MetricsServiceTrait for PseudoMetricsService { + fn export_metrics(&self) -> StdResult { + Ok("pseudo metrics".to_string()) + } + } + #[tokio::test] async fn test_metrics_server() { let logger = TestLogger::stdout(); let (shutdown_tx, shutdown_rx) = oneshot::channel(); - let metrics_service = Arc::new(MetricsService::new(logger.clone()).unwrap()); + let metrics_service = Arc::new(PseudoMetricsService::new()); let metrics_server = Arc::new(MetricsServer::new( "0.0.0.0", 9090, @@ -141,7 +151,7 @@ mod tests { .unwrap(); assert_eq!(StatusCode::OK, response.status()); - assert_ne!("", response.text().await.unwrap()); + assert_eq!("pseudo metrics", response.text().await.unwrap()); }); tokio::select!( diff --git a/mithril-signer/Cargo.toml b/mithril-signer/Cargo.toml index 641fda20d56..8ebe64bc0b3 100644 --- a/mithril-signer/Cargo.toml +++ b/mithril-signer/Cargo.toml @@ -23,6 +23,7 @@ config = "0.14.0" hex = "0.4.3" mithril-common = { path = "../mithril-common", features = ["full"] } mithril-doc = { path = "../internal/mithril-doc" } +mithril-metric = { path = "../internal/mithril-metric" } mithril-persistence = { path = "../internal/mithril-persistence" } openssl = { version = "0.10.66", features = ["vendored"], optional = true } openssl-probe = { version = "0.1.5", optional = true } diff --git a/mithril-signer/src/main.rs b/mithril-signer/src/main.rs index cf8a83750ca..1051a7fc377 100644 --- a/mithril-signer/src/main.rs +++ b/mithril-signer/src/main.rs @@ -14,9 +14,10 @@ use tokio::{ use mithril_common::StdResult; use mithril_doc::{Documenter, DocumenterDefault, GenerateDocCommands, StructDoc}; +use mithril_metric::MetricsServer; use mithril_signer::dependency_injection::DependenciesBuilder; use mithril_signer::{ - Configuration, DefaultConfiguration, MetricsServer, SignerRunner, SignerState, StateMachine, + Configuration, DefaultConfiguration, SignerRunner, SignerState, StateMachine, }; /// CLI args diff --git a/mithril-signer/src/metrics/mod.rs b/mithril-signer/src/metrics/mod.rs index 4b0e4b0da75..4ffc3b7b88f 100644 --- a/mithril-signer/src/metrics/mod.rs +++ b/mithril-signer/src/metrics/mod.rs @@ -1,10 +1,6 @@ //! metrics module. //! This module contains the signer metrics service and metrics server. -mod commons; -mod server; mod service; -pub use commons::MithrilMetric; -pub use server::MetricsServer; pub use service::MetricsService; diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index 61e6326cd56..ae9902905c5 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -1,10 +1,11 @@ +use mithril_metric::MetricsServiceTrait; use prometheus::{Encoder, Registry, TextEncoder}; use slog::Logger; use mithril_common::logging::LoggerExtensions; use mithril_common::{entities::Epoch, StdResult}; -use crate::metrics::commons::{CounterValue, MetricCounter, MetricGauge, MithrilMetric}; +use mithril_metric::commons::{CounterValue, MetricCounter, MetricGauge, MithrilMetric}; /// Metrics service which is responsible for recording and exposing metrics. pub struct MetricsService { @@ -19,6 +20,19 @@ pub struct MetricsService { runtime_cycle_total_since_startup_counter: MetricCounter, } +impl MetricsServiceTrait for MetricsService { + /// Export the metrics as a string with the Open Metrics standard format. + /// These metrics can be exposed on an HTTP server. + fn export_metrics(&self) -> StdResult { + let mut buffer = vec![]; + let encoder = TextEncoder::new(); + let metric_families = self.registry.gather(); + encoder.encode(&metric_families, &mut buffer)?; + + Ok(String::from_utf8(buffer)?) + } +} + impl MetricsService { /// Create a new `MetricsService` instance. pub fn new(logger: Logger) -> StdResult { @@ -117,17 +131,6 @@ impl MetricsService { }) } - /// Export the metrics as a string with the Open Metrics standard format. - /// These metrics can be exposed on an HTTP server. - pub fn export_metrics(&self) -> StdResult { - let mut buffer = vec![]; - let encoder = TextEncoder::new(); - let metric_families = self.registry.gather(); - encoder.encode(&metric_families, &mut buffer)?; - - Ok(String::from_utf8(buffer)?) - } - /// Increment the `signer_registration_success_since_startup` counter. pub fn signer_registration_success_since_startup_counter_increment(&self) { self.signer_registration_success_since_startup_counter diff --git a/mithril-signer/tests/test_extensions/state_machine_tester.rs b/mithril-signer/tests/test_extensions/state_machine_tester.rs index 215fe76eb19..31ef5846747 100644 --- a/mithril-signer/tests/test_extensions/state_machine_tester.rs +++ b/mithril-signer/tests/test_extensions/state_machine_tester.rs @@ -1,5 +1,6 @@ #![allow(dead_code)] use anyhow::anyhow; +use mithril_metric::{MetricsServiceTrait, MithrilMetric}; use prometheus_parse::Value; use slog::Drain; use slog_scope::debug; @@ -43,7 +44,6 @@ use mithril_persistence::{ use mithril_signer::{ database::repository::SignedBeaconRepository, dependency_injection::{DependenciesBuilder, SignerDependencyContainer}, - metrics::*, services::{ AggregatorClient, CardanoTransactionsImporter, MithrilEpochService, MithrilSingleSigner, SignerCertifierService, SignerSignableSeedBuilder, SignerSignedEntityConfigProvider, From d38d52dafc8b049010c3c5cde1846648eaa6344e Mon Sep 17 00:00:00 2001 From: sfauvel Date: Fri, 11 Oct 2024 12:15:18 +0200 Subject: [PATCH 06/23] Move the export function to the `mothril-metric` --- Cargo.lock | 1 + internal/mithril-metric/Cargo.toml | 1 + internal/mithril-metric/src/commons.rs | 65 ++++++++++++++++++++++++++ mithril-signer/src/metrics/service.rs | 10 ++-- 4 files changed, 70 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c50db8c8834..254981c0e1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3811,6 +3811,7 @@ dependencies = [ "axum", "mithril-common", "prometheus", + "prometheus-parse", "reqwest 0.12.7", "slog", "slog-async", diff --git a/internal/mithril-metric/Cargo.toml b/internal/mithril-metric/Cargo.toml index f8ff6f616b1..010c1eff49f 100644 --- a/internal/mithril-metric/Cargo.toml +++ b/internal/mithril-metric/Cargo.toml @@ -24,5 +24,6 @@ slog = { version = "2.7.0", features = [ tokio = { version = "1.40.0", features = ["full"] } [dev-dependencies] +prometheus-parse = "0.2.5" slog-async = "2.8.0" slog-term = "2.9.1" \ No newline at end of file diff --git a/internal/mithril-metric/src/commons.rs b/internal/mithril-metric/src/commons.rs index febff723900..af04a921596 100644 --- a/internal/mithril-metric/src/commons.rs +++ b/internal/mithril-metric/src/commons.rs @@ -105,6 +105,25 @@ impl MithrilMetric for MetricGauge { } } +pub mod metrics_tools { + + use mithril_common::StdResult; + use prometheus::TextEncoder; + + pub fn export_metrics(registry: &prometheus::Registry) -> StdResult { + // let mut buffer = vec![]; + let encoder = TextEncoder::new(); + let metric_families = registry.gather(); + // encoder.encode(&metric_families, &mut buffer)?; + + // Ok(String::from_utf8(buffer)?) + + let mut buffer = String::new(); + encoder.encode_utf8(&metric_families, &mut buffer)?; + Ok(buffer) + } +} + #[cfg(test)] pub mod test_tools { use std::{io, sync::Arc}; @@ -154,4 +173,50 @@ mod tests { metric.record(Epoch(12)); assert_eq!(metric.get(), Epoch(12)); } + + mod tools { + use super::*; + use prometheus::Registry; + use prometheus_parse::Value; + use std::{collections::BTreeMap, sync::Arc}; + + use mithril_common::entities::Epoch; + + fn parse_metrics(raw_metrics: &str) -> StdResult> { + Ok( + prometheus_parse::Scrape::parse(raw_metrics.lines().map(|s| Ok(s.to_owned())))? + .samples + .into_iter() + .map(|s| (s.metric, s.value)) + .collect::>(), + ) + } + + #[test] + fn test_export_metrics() { + let counter_metric = + MetricCounter::new(TestLogger::stdout(), "test_counter", "test counter help") + .unwrap(); + counter_metric.record(); + + let gauge_metric = + MetricGauge::new(TestLogger::stdout(), "test_gauge", "test gauge help").unwrap(); + gauge_metric.record(Epoch(12)); + + let registry = Registry::new(); + registry.register(counter_metric.collector()); + registry.register(gauge_metric.collector()); + + let exported_metrics = metrics_tools::export_metrics(®istry).unwrap(); + + let parsed_metrics = parse_metrics(&exported_metrics).unwrap(); + + let parsed_metrics_expected = BTreeMap::from([ + (counter_metric.name(), Value::Counter(1.0)), + (gauge_metric.name(), Value::Gauge(12.0)), + ]); + + assert_eq!(parsed_metrics_expected, parsed_metrics); + } + } } diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index ae9902905c5..f0d28108941 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -1,10 +1,11 @@ use mithril_metric::MetricsServiceTrait; -use prometheus::{Encoder, Registry, TextEncoder}; +use prometheus::Registry; use slog::Logger; use mithril_common::logging::LoggerExtensions; use mithril_common::{entities::Epoch, StdResult}; +use mithril_metric::commons::metrics_tools; use mithril_metric::commons::{CounterValue, MetricCounter, MetricGauge, MithrilMetric}; /// Metrics service which is responsible for recording and exposing metrics. @@ -24,12 +25,7 @@ impl MetricsServiceTrait for MetricsService { /// Export the metrics as a string with the Open Metrics standard format. /// These metrics can be exposed on an HTTP server. fn export_metrics(&self) -> StdResult { - let mut buffer = vec![]; - let encoder = TextEncoder::new(); - let metric_families = self.registry.gather(); - encoder.encode(&metric_families, &mut buffer)?; - - Ok(String::from_utf8(buffer)?) + metrics_tools::export_metrics(&self.registry) } } From 1f9da7be293fb12aced1606007ef448d8f257eba Mon Sep 17 00:00:00 2001 From: sfauvel Date: Fri, 11 Oct 2024 18:20:02 +0200 Subject: [PATCH 07/23] Add a macro to generate the metric service --- Cargo.lock | 2 + internal/mithril-metric/Cargo.toml | 1 + internal/mithril-metric/src/commons.rs | 303 +++++++++++++++++++++++-- mithril-signer/Cargo.toml | 1 + mithril-signer/src/metrics/service.rs | 233 +++++-------------- 5 files changed, 347 insertions(+), 193 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 254981c0e1f..664d78f83ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3810,6 +3810,7 @@ dependencies = [ "anyhow", "axum", "mithril-common", + "paste", "prometheus", "prometheus-parse", "reqwest 0.12.7", @@ -3884,6 +3885,7 @@ dependencies = [ "mockall", "openssl", "openssl-probe", + "paste", "prometheus", "prometheus-parse", "rand_chacha", diff --git a/internal/mithril-metric/Cargo.toml b/internal/mithril-metric/Cargo.toml index 010c1eff49f..35e8c5e089f 100644 --- a/internal/mithril-metric/Cargo.toml +++ b/internal/mithril-metric/Cargo.toml @@ -15,6 +15,7 @@ crate-type = ["lib", "cdylib", "staticlib"] anyhow = "1.0.86" axum = "0.7.5" mithril-common = { path = "../../mithril-common", features = ["full"] } +paste = "1.0.15" prometheus = "0.13.4" reqwest = { version = "0.12.7", features = ["json", "stream"] } slog = { version = "2.7.0", features = [ diff --git a/internal/mithril-metric/src/commons.rs b/internal/mithril-metric/src/commons.rs index af04a921596..7d2700ce8a4 100644 --- a/internal/mithril-metric/src/commons.rs +++ b/internal/mithril-metric/src/commons.rs @@ -9,6 +9,116 @@ pub type MetricName = str; /// Type alias for a counter value. pub type CounterValue = u32; +/// Create a MetricService. +/// +/// To build the service you need to provide the structure name and a list of metrics. +/// Each metrics is defined by an attribute name, a type, a metric name and a help message. +/// When the metric name is not provided the attribute name will be used. +/// We can specify a prefix that will be added to all metric names. +/// +/// The attribute name will be used to create a getter method for the metric. +/// +/// Crate that use this macro should have `paste` as dependency. +/// +/// build_metrics_service!( +/// MetricsService, +/// counter_example: MetricCounter( +/// "custom_counter_example_name", +/// "Example of a counter metric" +/// ), +/// gauge_example: MetricGauge( +/// "custom_gauge_example_name", +/// "Example of a gauge metric" +/// ) +/// ); +/// +/// build_metrics_service!( +/// MetricsService, +/// counter_example: MetricCounter("Example of a counter metric"), +/// gauge_example: MetricGauge("Example of a gauge metric") +/// ); +/// +/// build_metrics_service!( +/// MetricsService, +/// "metric_prefix", +/// counter_example: MetricCounter("Example of a counter metric"), +/// gauge_example: MetricGauge("Example of a gauge metric") +/// ); +/// +/// let service = MetricsServiceExampleBuildWithMacro::new(TestLogger::stdout()).unwrap(); +/// service.get_counter_example().record(); +/// service.get_gauge_example().record(Epoch(12)); +#[macro_export] +macro_rules! build_metrics_service { + ($service:ident, $($metric_attribute:ident:$metric_type:ident($help:literal)),*) => { + build_metrics_service!($service, "", $($metric_attribute:$metric_type("", $help)),*); + }; + ($service:ident, $prefix:literal, $($metric_attribute:ident:$metric_type:ident($help:literal)),*) => { + build_metrics_service!($service, $prefix, $($metric_attribute:$metric_type("", $help)),*); + }; + ($service:ident, $($metric_attribute:ident:$metric_type:ident($name:literal, $help:literal)),*) => { + build_metrics_service!($service, "", $($metric_attribute:$metric_type($name, $help)),*); + }; + ($service:ident, $prefix:literal, $($metric_attribute:ident:$metric_type:ident($name:literal, $help:literal)),*) => { + paste::item! { + /// Metrics service which is responsible for recording and exposing metrics. + pub struct $service { + registry: Registry, + $( + $metric_attribute: $metric_type, + )* + } + + impl $service { + pub fn new(logger: Logger) -> StdResult { + + let registry = Registry::new(); + + let prefix = if $prefix.is_empty() { + "".to_string() + } else { + format!("{}_", $prefix) + }; + + $( + let metric_name = if $name.is_empty() { + stringify!($metric_attribute) + } else { + $name + }; + let $metric_attribute = $metric_type::new( + logger.clone(), + &format!("{}{}", prefix, metric_name), + $help, + )?; + registry.register($metric_attribute.collector())?; + )* + + Ok(Self { + registry, + $( + $metric_attribute, + )* + }) + } + $( + /// Get the `$metric_attribute` counter. + pub fn [](&self) -> &$metric_type { + &self.$metric_attribute + } + )* + } + + impl MetricsServiceTrait for $service { + fn export_metrics(&self) -> StdResult { + metrics_tools::export_metrics(&self.registry) + } + } + + } + }; +} + /// Mithril metric pub trait MithrilMetric { /// Metric name @@ -111,11 +221,11 @@ pub mod metrics_tools { use prometheus::TextEncoder; pub fn export_metrics(registry: &prometheus::Registry) -> StdResult { - // let mut buffer = vec![]; let encoder = TextEncoder::new(); let metric_families = registry.gather(); + // TODO check encode_utf8 do the same as encode and remove the commented code + // let mut buffer = vec![]; // encoder.encode(&metric_families, &mut buffer)?; - // Ok(String::from_utf8(buffer)?) let mut buffer = String::new(); @@ -149,9 +259,25 @@ pub mod test_tools { #[cfg(test)] mod tests { + use std::collections::BTreeMap; + + use crate::MetricsServiceTrait; + use super::*; + use prometheus::Registry; + use prometheus_parse::Value; use test_tools::TestLogger; + fn parse_metrics(raw_metrics: &str) -> StdResult> { + Ok( + prometheus_parse::Scrape::parse(raw_metrics.lines().map(|s| Ok(s.to_owned())))? + .samples + .into_iter() + .map(|s| (s.metric, s.value)) + .collect::>(), + ) + } + #[test] fn test_metric_counter_can_be_incremented() { let metric = @@ -177,21 +303,9 @@ mod tests { mod tools { use super::*; use prometheus::Registry; - use prometheus_parse::Value; - use std::{collections::BTreeMap, sync::Arc}; use mithril_common::entities::Epoch; - fn parse_metrics(raw_metrics: &str) -> StdResult> { - Ok( - prometheus_parse::Scrape::parse(raw_metrics.lines().map(|s| Ok(s.to_owned())))? - .samples - .into_iter() - .map(|s| (s.metric, s.value)) - .collect::>(), - ) - } - #[test] fn test_export_metrics() { let counter_metric = @@ -204,8 +318,8 @@ mod tests { gauge_metric.record(Epoch(12)); let registry = Registry::new(); - registry.register(counter_metric.collector()); - registry.register(gauge_metric.collector()); + registry.register(counter_metric.collector()).unwrap(); + registry.register(gauge_metric.collector()).unwrap(); let exported_metrics = metrics_tools::export_metrics(®istry).unwrap(); @@ -219,4 +333,161 @@ mod tests { assert_eq!(parsed_metrics_expected, parsed_metrics); } } + + pub struct MetricsServiceExample { + registry: Registry, + counter_example: MetricCounter, + gauge_example: MetricGauge, + } + + impl MetricsServiceExample { + pub fn new(logger: Logger) -> StdResult { + let registry = Registry::new(); + + let counter_example = MetricCounter::new( + logger.clone(), + "counter_example", + "Example of a counter metric", + )?; + registry.register(counter_example.collector())?; + + let gauge_example = + MetricGauge::new(logger.clone(), "gauge_example", "Example of a gauge metric")?; + registry.register(gauge_example.collector())?; + + Ok(Self { + registry, + counter_example, + gauge_example, + }) + } + + /// Get the `counter_example` counter. + pub fn get_counter_example(&self) -> &MetricCounter { + &self.counter_example + } + + /// Get the `gauge_example` counter. + pub fn get_gauge_example(&self) -> &MetricGauge { + &self.gauge_example + } + } + + impl MetricsServiceTrait for MetricsServiceExample { + fn export_metrics(&self) -> StdResult { + metrics_tools::export_metrics(&self.registry) + } + } + + #[test] + fn test_service_creation() { + let service = MetricsServiceExample::new(TestLogger::stdout()).unwrap(); + service.get_counter_example().record(); + service.get_counter_example().record(); + service.get_gauge_example().record(Epoch(12)); + + assert_eq!(2, service.get_counter_example().get()); + assert_eq!(Epoch(12), service.get_gauge_example().get()); + } + + build_metrics_service!( + MetricsServiceExampleBuildWithMacro, + counter_example: MetricCounter( + "custom_counter_example_name", + "Example of a counter metric" + ), + gauge_example: MetricGauge( + "custom_gauge_example_name", + "Example of a gauge metric" + ) + ); + + #[test] + fn test_service_creation_using_build_metrics_service_macro() { + let service = MetricsServiceExampleBuildWithMacro::new(TestLogger::stdout()).unwrap(); + service.get_counter_example().record(); + service.get_counter_example().record(); + service.get_gauge_example().record(Epoch(12)); + + assert_eq!(2, service.get_counter_example().get()); + assert_eq!(Epoch(12), service.get_gauge_example().get()); + } + + #[test] + fn test_build_metrics_service_named_metrics_with_attribute_name() { + let service = MetricsServiceExampleBuildWithMacro::new(TestLogger::stdout()).unwrap(); + assert_eq!( + "custom_counter_example_name", + service.get_counter_example().name() + ); + assert_eq!( + "custom_gauge_example_name", + service.get_gauge_example().name() + ); + } + + build_metrics_service!( + MetricsServiceExampleBuildWithMacroWithoutMetricName, + counter_example: MetricCounter("Example of a counter metric"), + gauge_example: MetricGauge("Example of a gauge metric") + ); + + #[test] + fn test_build_metrics_service_macro_without_metric_name() { + let service = MetricsServiceExampleBuildWithMacro::new(TestLogger::stdout()).unwrap(); + service.get_counter_example().record(); + service.get_counter_example().record(); + service.get_gauge_example().record(Epoch(12)); + + assert_eq!(2, service.get_counter_example().get()); + assert_eq!(Epoch(12), service.get_gauge_example().get()); + } + + #[test] + fn test_build_metrics_service_named_metrics_without_attribute_name() { + let service = + MetricsServiceExampleBuildWithMacroWithoutMetricName::new(TestLogger::stdout()) + .unwrap(); + assert_eq!("counter_example", service.get_counter_example().name()); + assert_eq!("gauge_example", service.get_gauge_example().name()); + } + + build_metrics_service!( + MetricsServiceExampleBuildWithMacroUsingPrefix, + "metric_prefix", + counter_example: MetricCounter("Example of a counter metric"), + gauge_example: MetricGauge("Example of a gauge metric") + ); + + #[test] + fn test_build_metrics_service_named_metrics_with_attribute_name_and_prefix() { + let service = + MetricsServiceExampleBuildWithMacroUsingPrefix::new(TestLogger::stdout()).unwrap(); + assert_eq!( + "metric_prefix_counter_example", + service.get_counter_example().name() + ); + assert_eq!( + "metric_prefix_gauge_example", + service.get_gauge_example().name() + ); + } + + #[test] + fn test_build_metrics_service_implement_export() { + let service = + MetricsServiceExampleBuildWithMacroUsingPrefix::new(TestLogger::stdout()).unwrap(); + service.get_gauge_example().record(Epoch(12)); + + let export = service.export_metrics().unwrap(); + let metrics = parse_metrics(&export).unwrap(); + + assert_eq!( + Value::Gauge(12.0), + metrics + .get(&service.get_gauge_example().name()) + .unwrap() + .clone() + ); + } } diff --git a/mithril-signer/Cargo.toml b/mithril-signer/Cargo.toml index 8ebe64bc0b3..9fdf76cf49d 100644 --- a/mithril-signer/Cargo.toml +++ b/mithril-signer/Cargo.toml @@ -27,6 +27,7 @@ mithril-metric = { path = "../internal/mithril-metric" } mithril-persistence = { path = "../internal/mithril-persistence" } openssl = { version = "0.10.66", features = ["vendored"], optional = true } openssl-probe = { version = "0.1.5", optional = true } +paste = "1.0.15" prometheus = "0.13.4" rand_chacha = "0.3.1" rand_core = "0.6.4" diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index f0d28108941..d0e3a7cf133 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -1,258 +1,137 @@ -use mithril_metric::MetricsServiceTrait; +use mithril_metric::{build_metrics_service, metrics_tools, MetricsServiceTrait}; use prometheus::Registry; use slog::Logger; -use mithril_common::logging::LoggerExtensions; use mithril_common::{entities::Epoch, StdResult}; -use mithril_metric::commons::metrics_tools; use mithril_metric::commons::{CounterValue, MetricCounter, MetricGauge, MithrilMetric}; -/// Metrics service which is responsible for recording and exposing metrics. -pub struct MetricsService { - registry: Registry, - signer_registration_success_since_startup_counter: MetricCounter, - signer_registration_total_since_startup_counter: MetricCounter, - signer_registration_success_last_epoch_gauge: MetricGauge, - signature_registration_success_since_startup_counter: MetricCounter, - signature_registration_total_since_startup_counter: MetricCounter, - signature_registration_success_last_epoch_gauge: MetricGauge, - runtime_cycle_success_since_startup_counter: MetricCounter, - runtime_cycle_total_since_startup_counter: MetricCounter, -} - -impl MetricsServiceTrait for MetricsService { - /// Export the metrics as a string with the Open Metrics standard format. - /// These metrics can be exposed on an HTTP server. - fn export_metrics(&self) -> StdResult { - metrics_tools::export_metrics(&self.registry) - } -} - +/// Build the metrics service with the required metrics. +build_metrics_service!( + MetricsService, + "mithril_signer", + signer_registration_success_since_startup_counter: MetricCounter( + "Number of successful signer registrations since startup on a Mithril signer node" + ), + signer_registration_total_since_startup_counter: MetricCounter( + "Number of signer registrations since startup on a Mithril signer node" + ), + signer_registration_success_last_epoch_gauge: MetricGauge( + "Latest epoch at which signer successfully registered on a Mithril signer node" + ), + signature_registration_success_since_startup_counter: MetricCounter( + "Number of successful signature registrations since startup on a Mithril signer node" + ), + signature_registration_total_since_startup_counter: MetricCounter( + "Number of signature registrations since startup on a Mithril signer node" + ), + signature_registration_success_last_epoch_gauge: MetricGauge( + "Latest epoch at which signature successfully registered on a Mithril signer node" + ), + // Runtime cycle metrics + runtime_cycle_success_since_startup_counter: MetricCounter( + "Number of successful runtime cycles since startup on a Mithril signer node" + ), + runtime_cycle_total_since_startup_counter: MetricCounter( + "Number of runtime cycles since startup on a Mithril signer node" + ) + +); + +// TODO these functions are kept while changes are made for all uses impl MetricsService { - /// Create a new `MetricsService` instance. - pub fn new(logger: Logger) -> StdResult { - let logger = logger.new_with_component_name::(); - - let registry = Registry::new(); - - fn register(registry: &Registry, metric: T) -> StdResult { - registry.register(metric.collector())?; - Ok(metric) - } - - let signer_registration_success_since_startup_counter = register( - ®istry, - MetricCounter::new( - logger.clone(), - "mithril_signer_signer_registration_success_since_startup", - "Number of successful signer registrations since startup on a Mithril signer node", - )?, - )?; - - let signer_registration_total_since_startup_counter = register( - ®istry, - MetricCounter::new( - logger.clone(), - "mithril_signer_signer_registration_total_since_startup", - "Number of signer registrations since startup on a Mithril signer node", - )?, - )?; - - let signer_registration_success_last_epoch_gauge = register( - ®istry, - MetricGauge::new( - logger.clone(), - "mithril_signer_signer_registration_success_last_epoch", - "Latest epoch at which signer successfully registered on a Mithril signer node", - )?, - )?; - // Signature registration metrics - - let signature_registration_success_since_startup_counter = register( - ®istry, - MetricCounter::new( - logger.clone(), - "mithril_signer_signature_registration_success_since_startup", - "Number of successful signature registrations since startup on a Mithril signer node", - )?, - )?; - - let signature_registration_total_since_startup_counter = register( - ®istry, - MetricCounter::new( - logger.clone(), - "mithril_signer_signature_registration_total_since_startup", - "Number of signature registrations since startup on a Mithril signer node", - )?, - )?; - - let signature_registration_success_last_epoch_gauge = register( - ®istry, - MetricGauge::new( - logger.clone(), - "mithril_signer_signature_registration_success_last_epoch", - "Latest epoch at which signature successfully registered on a Mithril signer node", - )?, - )?; - - // Runtime cycle metrics - let runtime_cycle_success_since_startup_counter = register( - ®istry, - MetricCounter::new( - logger.clone(), - "mithril_signer_runtime_cycle_success_since_startup", - "Number of successful runtime cycles since startup on a Mithril signer node", - )?, - )?; - let runtime_cycle_total_since_startup_counter = register( - ®istry, - MetricCounter::new( - logger.clone(), - "mithril_signer_runtime_cycle_total_since_startup", - "Number of runtime cycles since startup on a Mithril signer node", - )?, - )?; - - Ok(Self { - registry, - signer_registration_success_since_startup_counter, - signer_registration_total_since_startup_counter, - signer_registration_success_last_epoch_gauge, - signature_registration_success_since_startup_counter, - signature_registration_total_since_startup_counter, - signature_registration_success_last_epoch_gauge, - runtime_cycle_success_since_startup_counter, - runtime_cycle_total_since_startup_counter, - }) - } - /// Increment the `signer_registration_success_since_startup` counter. pub fn signer_registration_success_since_startup_counter_increment(&self) { - self.signer_registration_success_since_startup_counter + self.get_signer_registration_success_since_startup_counter() .record(); } /// Get the `signer_registration_success_since_startup` counter. pub fn signer_registration_success_since_startup_counter_get(&self) -> CounterValue { - self.signer_registration_success_since_startup_counter.get() + self.get_signer_registration_success_since_startup_counter() + .get() } /// Increment the `signer_registration_total_since_startup` counter. pub fn signer_registration_total_since_startup_counter_increment(&self) { - self.signer_registration_total_since_startup_counter + self.get_signer_registration_total_since_startup_counter() .record(); } /// Get the `signer_registration_total_since_startup` counter. pub fn signer_registration_total_since_startup_counter_get(&self) -> CounterValue { - self.signer_registration_total_since_startup_counter.get() + self.get_signer_registration_total_since_startup_counter() + .get() } /// Set the `signer_registration_success_last_epoch` gauge value. pub fn signer_registration_success_last_epoch_gauge_set(&self, value: Epoch) { - self.signer_registration_success_last_epoch_gauge + self.get_signer_registration_success_last_epoch_gauge() .record(value); } /// Get the `signer_registration_success_last_epoch` gauge value. pub fn signer_registration_success_last_epoch_gauge_get(&self) -> Epoch { - self.signer_registration_success_last_epoch_gauge.get() + self.get_signer_registration_success_last_epoch_gauge() + .get() } /// Increment the `signature_registration_success_since_startup` counter. pub fn signature_registration_success_since_startup_counter_increment(&self) { - self.signature_registration_success_since_startup_counter + self.get_signature_registration_success_since_startup_counter() .record(); } /// Get the `signature_registration_success_since_startup` counter. pub fn signature_registration_success_since_startup_counter_get(&self) -> CounterValue { - self.signature_registration_success_since_startup_counter + self.get_signature_registration_success_since_startup_counter() .get() } /// Increment the `signature_registration_total_since_startup` counter. pub fn signature_registration_total_since_startup_counter_increment(&self) { - self.signature_registration_total_since_startup_counter + self.get_signature_registration_total_since_startup_counter() .record(); } /// Get the `signature_registration_total_since_startup` counter. pub fn signature_registration_total_since_startup_counter_get(&self) -> CounterValue { - self.signature_registration_total_since_startup_counter + self.get_signature_registration_total_since_startup_counter() .get() } /// Set the `signature_registration_success_last_epoch` gauge value. pub fn signature_registration_success_last_epoch_gauge_set(&self, value: Epoch) { - self.signature_registration_success_last_epoch_gauge + self.get_signature_registration_success_last_epoch_gauge() .record(value); } /// Get the `signature_registration_success_last_epoch` gauge value. pub fn signature_registration_success_last_epoch_gauge_get(&self) -> Epoch { - self.signature_registration_success_last_epoch_gauge.get() + self.get_signature_registration_success_last_epoch_gauge() + .get() } /// Increment the `runtime_cycle_total_since_startup` counter. pub fn runtime_cycle_total_since_startup_counter_increment(&self) { - self.runtime_cycle_total_since_startup_counter.record(); + self.get_runtime_cycle_total_since_startup_counter() + .record(); } /// Get the `runtime_cycle_total_since_startup` counter. pub fn runtime_cycle_total_since_startup_counter_get(&self) -> CounterValue { - self.runtime_cycle_total_since_startup_counter.get() + self.get_runtime_cycle_total_since_startup_counter().get() } /// Increment the `runtime_cycle_success_since_startup` counter. pub fn runtime_cycle_success_since_startup_counter_increment(&self) { - self.runtime_cycle_success_since_startup_counter.record(); + self.get_runtime_cycle_success_since_startup_counter() + .record(); } /// Get the `runtime_cycle_success_since_startup` counter. pub fn runtime_cycle_success_since_startup_counter_get(&self) -> CounterValue { - self.runtime_cycle_success_since_startup_counter.get() - } - - /// Get the `signer_registration_success_since_startup_counter` counter. - pub fn get_signer_registration_success_since_startup_counter(&self) -> &MetricCounter { - &self.signer_registration_success_since_startup_counter - } - - /// Get the `signer_registration_total_since_startup_counter` counter. - pub fn get_signer_registration_total_since_startup_counter(&self) -> &MetricCounter { - &self.signer_registration_total_since_startup_counter - } - - /// Get the `signer_registration_success_last_epoch_gauge` counter. - pub fn get_signer_registration_success_last_epoch_gauge(&self) -> &MetricGauge { - &self.signer_registration_success_last_epoch_gauge - } - - /// Get the `signature_registration_success_since_startup_counter` counter. - pub fn get_signature_registration_success_since_startup_counter(&self) -> &MetricCounter { - &self.signature_registration_success_since_startup_counter - } - - /// Get the `signature_registration_total_since_startup_counter` counter. - pub fn get_signature_registration_total_since_startup_counter(&self) -> &MetricCounter { - &self.signature_registration_total_since_startup_counter - } - - /// Get the `signature_registration_success_last_epoch_gauge` counter. - pub fn get_signature_registration_success_last_epoch_gauge(&self) -> &MetricGauge { - &self.signature_registration_success_last_epoch_gauge - } - - /// Get the `runtime_cycle_success_since_startup_counter` counter. - pub fn get_runtime_cycle_success_since_startup_counter(&self) -> &MetricCounter { - &self.runtime_cycle_success_since_startup_counter - } - - /// Get the `runtime_cycle_total_since_startup_counter` counter. - pub fn get_runtime_cycle_total_since_startup_counter(&self) -> &MetricCounter { - &self.runtime_cycle_total_since_startup_counter + self.get_runtime_cycle_success_since_startup_counter().get() } } From 1c0127eae15aa42183120f3b2cbd91d1f98be646 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Fri, 11 Oct 2024 18:34:03 +0200 Subject: [PATCH 08/23] Rename the `MetricsServiceTrait` to `MetricsServiceExporter` --- internal/mithril-metric/src/commons.rs | 6 +++--- internal/mithril-metric/src/lib.rs | 2 +- internal/mithril-metric/src/server.rs | 10 +++++----- mithril-signer/src/metrics/service.rs | 2 +- .../tests/test_extensions/state_machine_tester.rs | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/internal/mithril-metric/src/commons.rs b/internal/mithril-metric/src/commons.rs index 7d2700ce8a4..28fd0d92111 100644 --- a/internal/mithril-metric/src/commons.rs +++ b/internal/mithril-metric/src/commons.rs @@ -109,7 +109,7 @@ macro_rules! build_metrics_service { )* } - impl MetricsServiceTrait for $service { + impl MetricsServiceExporter for $service { fn export_metrics(&self) -> StdResult { metrics_tools::export_metrics(&self.registry) } @@ -261,7 +261,7 @@ pub mod test_tools { mod tests { use std::collections::BTreeMap; - use crate::MetricsServiceTrait; + use crate::MetricsServiceExporter; use super::*; use prometheus::Registry; @@ -373,7 +373,7 @@ mod tests { } } - impl MetricsServiceTrait for MetricsServiceExample { + impl MetricsServiceExporter for MetricsServiceExample { fn export_metrics(&self) -> StdResult { metrics_tools::export_metrics(&self.registry) } diff --git a/internal/mithril-metric/src/lib.rs b/internal/mithril-metric/src/lib.rs index 7a23044b7b7..1c2475db5dd 100644 --- a/internal/mithril-metric/src/lib.rs +++ b/internal/mithril-metric/src/lib.rs @@ -6,4 +6,4 @@ mod server; pub use commons::*; pub use server::MetricsServer; -pub use server::MetricsServiceTrait; +pub use server::MetricsServiceExporter; diff --git a/internal/mithril-metric/src/server.rs b/internal/mithril-metric/src/server.rs index 9bb2b7b37a7..11af8dcd88f 100644 --- a/internal/mithril-metric/src/server.rs +++ b/internal/mithril-metric/src/server.rs @@ -13,7 +13,7 @@ use tokio::sync::oneshot::Receiver; use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; -pub trait MetricsServiceTrait { +pub trait MetricsServiceExporter { fn export_metrics(&self) -> StdResult; } @@ -36,19 +36,19 @@ impl IntoResponse for MetricsServerError { } /// The MetricsServer is responsible for exposing the metrics of the signer. -pub struct MetricsServer { +pub struct MetricsServer { server_port: u16, server_ip: String, metrics_service: Arc, logger: Logger, } -struct RouterState { +struct RouterState { metrics_service: Arc, logger: Logger, } -impl MetricsServer { +impl MetricsServer { /// Create a new MetricsServer instance. pub fn new(server_ip: &str, server_port: u16, metrics_service: Arc, logger: Logger) -> Self { Self { @@ -123,7 +123,7 @@ mod tests { } } - impl MetricsServiceTrait for PseudoMetricsService { + impl MetricsServiceExporter for PseudoMetricsService { fn export_metrics(&self) -> StdResult { Ok("pseudo metrics".to_string()) } diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index d0e3a7cf133..5f69713b1ad 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -1,4 +1,4 @@ -use mithril_metric::{build_metrics_service, metrics_tools, MetricsServiceTrait}; +use mithril_metric::{build_metrics_service, metrics_tools, MetricsServiceExporter}; use prometheus::Registry; use slog::Logger; diff --git a/mithril-signer/tests/test_extensions/state_machine_tester.rs b/mithril-signer/tests/test_extensions/state_machine_tester.rs index 31ef5846747..8f7dc4e1724 100644 --- a/mithril-signer/tests/test_extensions/state_machine_tester.rs +++ b/mithril-signer/tests/test_extensions/state_machine_tester.rs @@ -1,6 +1,6 @@ #![allow(dead_code)] use anyhow::anyhow; -use mithril_metric::{MetricsServiceTrait, MithrilMetric}; +use mithril_metric::{MetricsServiceExporter, MithrilMetric}; use prometheus_parse::Value; use slog::Drain; use slog_scope::debug; From fdd5adccbbd97493b98a8ecee4ee0c42bfd586eb Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 14 Oct 2024 09:39:36 +0200 Subject: [PATCH 09/23] Fix clippy warning on documentation and cargo.toml sort --- internal/mithril-metric/Cargo.toml | 2 +- internal/mithril-metric/src/commons.rs | 1 + mithril-signer/src/metrics/service.rs | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/mithril-metric/Cargo.toml b/internal/mithril-metric/Cargo.toml index 35e8c5e089f..a2f5b8abfff 100644 --- a/internal/mithril-metric/Cargo.toml +++ b/internal/mithril-metric/Cargo.toml @@ -27,4 +27,4 @@ tokio = { version = "1.40.0", features = ["full"] } [dev-dependencies] prometheus-parse = "0.2.5" slog-async = "2.8.0" -slog-term = "2.9.1" \ No newline at end of file +slog-term = "2.9.1" diff --git a/internal/mithril-metric/src/commons.rs b/internal/mithril-metric/src/commons.rs index 28fd0d92111..f77ac4ed2b4 100644 --- a/internal/mithril-metric/src/commons.rs +++ b/internal/mithril-metric/src/commons.rs @@ -70,6 +70,7 @@ macro_rules! build_metrics_service { } impl $service { + /// Create a new MetricsService instance. pub fn new(logger: Logger) -> StdResult { let registry = Registry::new(); diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index 5f69713b1ad..e991f00ea31 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -6,7 +6,6 @@ use mithril_common::{entities::Epoch, StdResult}; use mithril_metric::commons::{CounterValue, MetricCounter, MetricGauge, MithrilMetric}; -/// Build the metrics service with the required metrics. build_metrics_service!( MetricsService, "mithril_signer", From 404bf8617f7cc9fc9f43d8d7553a7acab48fed83 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 14 Oct 2024 14:17:20 +0200 Subject: [PATCH 10/23] Rename `MithrilMetric` to `MetricCollector` and `record`to `increment`in `MetricCounter` --- internal/mithril-metric/src/commons.rs | 28 ++++++++++--------- mithril-signer/src/metrics/service.rs | 14 +++++----- .../test_extensions/state_machine_tester.rs | 2 +- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/internal/mithril-metric/src/commons.rs b/internal/mithril-metric/src/commons.rs index f77ac4ed2b4..0d96f61fbbe 100644 --- a/internal/mithril-metric/src/commons.rs +++ b/internal/mithril-metric/src/commons.rs @@ -120,8 +120,8 @@ macro_rules! build_metrics_service { }; } -/// Mithril metric -pub trait MithrilMetric { +/// Metric collector +pub trait MetricCollector { /// Metric name fn name(&self) -> String; @@ -129,6 +129,7 @@ pub trait MithrilMetric { fn collector(&self) -> Box; } +/// Metric counter pub struct MetricCounter { name: String, logger: Logger, @@ -145,7 +146,7 @@ impl MetricCounter { }) } - pub fn record(&self) { + pub fn increment(&self) { debug!(self.logger, "incrementing '{}' counter", self.name); self.counter.inc(); } @@ -162,7 +163,7 @@ impl MetricCounter { } } -impl MithrilMetric for MetricCounter { +impl MetricCollector for MetricCounter { fn collector(&self) -> Box { self.counter.clone() } @@ -172,6 +173,7 @@ impl MithrilMetric for MetricCounter { } } +/// Metric gauge pub struct MetricGauge { name: String, logger: Logger, @@ -207,7 +209,7 @@ impl MetricGauge { Ok(gauge) } } -impl MithrilMetric for MetricGauge { +impl MetricCollector for MetricGauge { fn collector(&self) -> Box { self.gauge.clone() } @@ -286,7 +288,7 @@ mod tests { assert_eq!(metric.name(), "test_counter"); assert_eq!(metric.get(), 0); - metric.record(); + metric.increment(); assert_eq!(metric.get(), 1); } @@ -312,7 +314,7 @@ mod tests { let counter_metric = MetricCounter::new(TestLogger::stdout(), "test_counter", "test counter help") .unwrap(); - counter_metric.record(); + counter_metric.increment(); let gauge_metric = MetricGauge::new(TestLogger::stdout(), "test_gauge", "test gauge help").unwrap(); @@ -383,8 +385,8 @@ mod tests { #[test] fn test_service_creation() { let service = MetricsServiceExample::new(TestLogger::stdout()).unwrap(); - service.get_counter_example().record(); - service.get_counter_example().record(); + service.get_counter_example().increment(); + service.get_counter_example().increment(); service.get_gauge_example().record(Epoch(12)); assert_eq!(2, service.get_counter_example().get()); @@ -406,8 +408,8 @@ mod tests { #[test] fn test_service_creation_using_build_metrics_service_macro() { let service = MetricsServiceExampleBuildWithMacro::new(TestLogger::stdout()).unwrap(); - service.get_counter_example().record(); - service.get_counter_example().record(); + service.get_counter_example().increment(); + service.get_counter_example().increment(); service.get_gauge_example().record(Epoch(12)); assert_eq!(2, service.get_counter_example().get()); @@ -436,8 +438,8 @@ mod tests { #[test] fn test_build_metrics_service_macro_without_metric_name() { let service = MetricsServiceExampleBuildWithMacro::new(TestLogger::stdout()).unwrap(); - service.get_counter_example().record(); - service.get_counter_example().record(); + service.get_counter_example().increment(); + service.get_counter_example().increment(); service.get_gauge_example().record(Epoch(12)); assert_eq!(2, service.get_counter_example().get()); diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index e991f00ea31..3f6625bbc29 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -4,7 +4,7 @@ use slog::Logger; use mithril_common::{entities::Epoch, StdResult}; -use mithril_metric::commons::{CounterValue, MetricCounter, MetricGauge, MithrilMetric}; +use mithril_metric::commons::{CounterValue, MetricCollector, MetricCounter, MetricGauge}; build_metrics_service!( MetricsService, @@ -42,7 +42,7 @@ impl MetricsService { /// Increment the `signer_registration_success_since_startup` counter. pub fn signer_registration_success_since_startup_counter_increment(&self) { self.get_signer_registration_success_since_startup_counter() - .record(); + .increment(); } /// Get the `signer_registration_success_since_startup` counter. @@ -54,7 +54,7 @@ impl MetricsService { /// Increment the `signer_registration_total_since_startup` counter. pub fn signer_registration_total_since_startup_counter_increment(&self) { self.get_signer_registration_total_since_startup_counter() - .record(); + .increment(); } /// Get the `signer_registration_total_since_startup` counter. @@ -78,7 +78,7 @@ impl MetricsService { /// Increment the `signature_registration_success_since_startup` counter. pub fn signature_registration_success_since_startup_counter_increment(&self) { self.get_signature_registration_success_since_startup_counter() - .record(); + .increment(); } /// Get the `signature_registration_success_since_startup` counter. @@ -90,7 +90,7 @@ impl MetricsService { /// Increment the `signature_registration_total_since_startup` counter. pub fn signature_registration_total_since_startup_counter_increment(&self) { self.get_signature_registration_total_since_startup_counter() - .record(); + .increment(); } /// Get the `signature_registration_total_since_startup` counter. @@ -114,7 +114,7 @@ impl MetricsService { /// Increment the `runtime_cycle_total_since_startup` counter. pub fn runtime_cycle_total_since_startup_counter_increment(&self) { self.get_runtime_cycle_total_since_startup_counter() - .record(); + .increment(); } /// Get the `runtime_cycle_total_since_startup` counter. @@ -125,7 +125,7 @@ impl MetricsService { /// Increment the `runtime_cycle_success_since_startup` counter. pub fn runtime_cycle_success_since_startup_counter_increment(&self) { self.get_runtime_cycle_success_since_startup_counter() - .record(); + .increment(); } /// Get the `runtime_cycle_success_since_startup` counter. diff --git a/mithril-signer/tests/test_extensions/state_machine_tester.rs b/mithril-signer/tests/test_extensions/state_machine_tester.rs index 8f7dc4e1724..39280c615b5 100644 --- a/mithril-signer/tests/test_extensions/state_machine_tester.rs +++ b/mithril-signer/tests/test_extensions/state_machine_tester.rs @@ -1,6 +1,6 @@ #![allow(dead_code)] use anyhow::anyhow; -use mithril_metric::{MetricsServiceExporter, MithrilMetric}; +use mithril_metric::{MetricsServiceExporter, MetricCollector}; use prometheus_parse::Value; use slog::Drain; use slog_scope::debug; From 7280f432e843458d2ff659dd03e1430d04691ac1 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 14 Oct 2024 15:05:52 +0200 Subject: [PATCH 11/23] Split `commons`file to `helper`and `metric` files --- .../src/{commons.rs => helper.rs} | 138 +----------------- internal/mithril-metric/src/lib.rs | 6 +- internal/mithril-metric/src/metric.rs | 137 +++++++++++++++++ internal/mithril-metric/src/server.rs | 3 +- mithril-signer/src/metrics/service.rs | 2 +- 5 files changed, 150 insertions(+), 136 deletions(-) rename internal/mithril-metric/src/{commons.rs => helper.rs} (79%) create mode 100644 internal/mithril-metric/src/metric.rs diff --git a/internal/mithril-metric/src/commons.rs b/internal/mithril-metric/src/helper.rs similarity index 79% rename from internal/mithril-metric/src/commons.rs rename to internal/mithril-metric/src/helper.rs index 0d96f61fbbe..5b2bb3b3336 100644 --- a/internal/mithril-metric/src/commons.rs +++ b/internal/mithril-metric/src/helper.rs @@ -1,14 +1,3 @@ -use prometheus::{core::Collector, Counter, Gauge, Opts}; -use slog::{debug, Logger}; - -use mithril_common::{entities::Epoch, StdResult}; - -/// Type alias for a metric name. -pub type MetricName = str; - -/// Type alias for a counter value. -pub type CounterValue = u32; - /// Create a MetricService. /// /// To build the service you need to provide the structure name and a list of metrics. @@ -120,104 +109,6 @@ macro_rules! build_metrics_service { }; } -/// Metric collector -pub trait MetricCollector { - /// Metric name - fn name(&self) -> String; - - /// Wrapped prometheus collector - fn collector(&self) -> Box; -} - -/// Metric counter -pub struct MetricCounter { - name: String, - logger: Logger, - counter: Box, -} - -impl MetricCounter { - pub fn new(logger: Logger, name: &str, help: &str) -> StdResult { - let counter = MetricCounter::create_metric_counter(name, help)?; - Ok(Self { - logger, - name: name.to_string(), - counter: Box::new(counter), - }) - } - - pub fn increment(&self) { - debug!(self.logger, "incrementing '{}' counter", self.name); - self.counter.inc(); - } - - pub fn get(&self) -> CounterValue { - self.counter.get().round() as CounterValue - } - - fn create_metric_counter(name: &MetricName, help: &str) -> StdResult { - let counter_opts = Opts::new(name, help); - let counter = Counter::with_opts(counter_opts)?; - - Ok(counter) - } -} - -impl MetricCollector for MetricCounter { - fn collector(&self) -> Box { - self.counter.clone() - } - - fn name(&self) -> String { - self.name.clone() - } -} - -/// Metric gauge -pub struct MetricGauge { - name: String, - logger: Logger, - gauge: Box, -} - -impl MetricGauge { - pub fn new(logger: Logger, name: &str, help: &str) -> StdResult { - let gauge = MetricGauge::create_metric_gauge(name, help)?; - Ok(Self { - logger, - name: name.to_string(), - gauge: Box::new(gauge), - }) - } - - pub fn record(&self, epoch: Epoch) { - debug!( - self.logger, - "set '{}' gauge value to {}", self.name, epoch.0 - ); - self.gauge.set(epoch.0 as f64); - } - - pub fn get(&self) -> Epoch { - Epoch(self.gauge.get().round() as u64) - } - - fn create_metric_gauge(name: &MetricName, help: &str) -> StdResult { - let gauge_opts = Opts::new(name, help); - let gauge = Gauge::with_opts(gauge_opts)?; - - Ok(gauge) - } -} -impl MetricCollector for MetricGauge { - fn collector(&self) -> Box { - self.gauge.clone() - } - fn name(&self) -> String { - self.name.clone() - } -} - pub mod metrics_tools { use mithril_common::StdResult; @@ -237,6 +128,7 @@ pub mod metrics_tools { } } +// TODO do we create a module for that or put it in lib.rs ? #[cfg(test)] pub mod test_tools { use std::{io, sync::Arc}; @@ -264,11 +156,13 @@ pub mod test_tools { mod tests { use std::collections::BTreeMap; - use crate::MetricsServiceExporter; + use crate::{MetricCollector, MetricCounter, MetricGauge, MetricsServiceExporter}; use super::*; + use mithril_common::{entities::Epoch, StdResult}; use prometheus::Registry; use prometheus_parse::Value; + use slog::Logger; use test_tools::TestLogger; fn parse_metrics(raw_metrics: &str) -> StdResult> { @@ -281,29 +175,9 @@ mod tests { ) } - #[test] - fn test_metric_counter_can_be_incremented() { - let metric = - MetricCounter::new(TestLogger::stdout(), "test_counter", "test counter help").unwrap(); - assert_eq!(metric.name(), "test_counter"); - assert_eq!(metric.get(), 0); - - metric.increment(); - assert_eq!(metric.get(), 1); - } - - #[test] - fn test_metric_gauge_can_be_set() { - let metric = - MetricGauge::new(TestLogger::stdout(), "test_gauge", "test gauge help").unwrap(); - assert_eq!(metric.name(), "test_gauge"); - assert_eq!(metric.get(), Epoch(0)); - - metric.record(Epoch(12)); - assert_eq!(metric.get(), Epoch(12)); - } - mod tools { + use crate::MetricCounter; + use super::*; use prometheus::Registry; diff --git a/internal/mithril-metric/src/lib.rs b/internal/mithril-metric/src/lib.rs index 1c2475db5dd..2365d49eb34 100644 --- a/internal/mithril-metric/src/lib.rs +++ b/internal/mithril-metric/src/lib.rs @@ -1,9 +1,11 @@ //! metrics module. //! This module contains the tools to create a metrics service and a metrics server. -pub mod commons; +pub mod helper; +pub mod metric; mod server; -pub use commons::*; +pub use helper::*; +pub use metric::*; pub use server::MetricsServer; pub use server::MetricsServiceExporter; diff --git a/internal/mithril-metric/src/metric.rs b/internal/mithril-metric/src/metric.rs new file mode 100644 index 00000000000..af781ce1724 --- /dev/null +++ b/internal/mithril-metric/src/metric.rs @@ -0,0 +1,137 @@ +use prometheus::{core::Collector, Counter, Gauge, Opts}; +use slog::{debug, Logger}; + +use mithril_common::{entities::Epoch, StdResult}; + +/// Type alias for a metric name. +pub type MetricName = str; + +/// Type alias for a counter value. +pub type CounterValue = u32; + +/// Metric collector +pub trait MetricCollector { + /// Metric name + fn name(&self) -> String; + + /// Wrapped prometheus collector + fn collector(&self) -> Box; +} + +/// Metric counter +pub struct MetricCounter { + name: String, + logger: Logger, + counter: Box, +} + +impl MetricCounter { + pub fn new(logger: Logger, name: &str, help: &str) -> StdResult { + let counter = MetricCounter::create_metric_counter(name, help)?; + Ok(Self { + logger, + name: name.to_string(), + counter: Box::new(counter), + }) + } + + pub fn increment(&self) { + debug!(self.logger, "incrementing '{}' counter", self.name); + self.counter.inc(); + } + + pub fn get(&self) -> CounterValue { + self.counter.get().round() as CounterValue + } + + fn create_metric_counter(name: &MetricName, help: &str) -> StdResult { + let counter_opts = Opts::new(name, help); + let counter = Counter::with_opts(counter_opts)?; + + Ok(counter) + } +} + +impl MetricCollector for MetricCounter { + fn collector(&self) -> Box { + self.counter.clone() + } + + fn name(&self) -> String { + self.name.clone() + } +} + +/// Metric gauge +pub struct MetricGauge { + name: String, + logger: Logger, + gauge: Box, +} + +impl MetricGauge { + pub fn new(logger: Logger, name: &str, help: &str) -> StdResult { + let gauge = MetricGauge::create_metric_gauge(name, help)?; + Ok(Self { + logger, + name: name.to_string(), + gauge: Box::new(gauge), + }) + } + + pub fn record(&self, epoch: Epoch) { + debug!( + self.logger, + "set '{}' gauge value to {}", self.name, epoch.0 + ); + self.gauge.set(epoch.0 as f64); + } + + pub fn get(&self) -> Epoch { + Epoch(self.gauge.get().round() as u64) + } + + fn create_metric_gauge(name: &MetricName, help: &str) -> StdResult { + let gauge_opts = Opts::new(name, help); + let gauge = Gauge::with_opts(gauge_opts)?; + + Ok(gauge) + } +} +impl MetricCollector for MetricGauge { + fn collector(&self) -> Box { + self.gauge.clone() + } + fn name(&self) -> String { + self.name.clone() + } +} + +#[cfg(test)] +mod tests { + use crate::helper::test_tools::TestLogger; + + use super::*; + + #[test] + fn test_metric_counter_can_be_incremented() { + let metric = + MetricCounter::new(TestLogger::stdout(), "test_counter", "test counter help").unwrap(); + assert_eq!(metric.name(), "test_counter"); + assert_eq!(metric.get(), 0); + + metric.increment(); + assert_eq!(metric.get(), 1); + } + + #[test] + fn test_metric_gauge_can_be_set() { + let metric = + MetricGauge::new(TestLogger::stdout(), "test_gauge", "test gauge help").unwrap(); + assert_eq!(metric.name(), "test_gauge"); + assert_eq!(metric.get(), Epoch(0)); + + metric.record(Epoch(12)); + assert_eq!(metric.get(), Epoch(12)); + } +} diff --git a/internal/mithril-metric/src/server.rs b/internal/mithril-metric/src/server.rs index 11af8dcd88f..432648066f3 100644 --- a/internal/mithril-metric/src/server.rs +++ b/internal/mithril-metric/src/server.rs @@ -107,12 +107,13 @@ impl MetricsServer { #[cfg(test)] mod tests { - use crate::test_tools::TestLogger; use anyhow::anyhow; use reqwest::StatusCode; use std::time::Duration; use tokio::{sync::oneshot, task::yield_now, time::sleep}; + use crate::test_tools::TestLogger; + use super::*; pub struct PseudoMetricsService {} diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index 3f6625bbc29..c5860e4d2ab 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -4,7 +4,7 @@ use slog::Logger; use mithril_common::{entities::Epoch, StdResult}; -use mithril_metric::commons::{CounterValue, MetricCollector, MetricCounter, MetricGauge}; +use mithril_metric::metric::{CounterValue, MetricCollector, MetricCounter, MetricGauge}; build_metrics_service!( MetricsService, From 1e515d9e7bb5514bb75e3e26c180c07b855f0129 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 14 Oct 2024 15:11:39 +0200 Subject: [PATCH 12/23] Remove feature='full' --- internal/mithril-metric/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/mithril-metric/Cargo.toml b/internal/mithril-metric/Cargo.toml index a2f5b8abfff..a36f1af903f 100644 --- a/internal/mithril-metric/Cargo.toml +++ b/internal/mithril-metric/Cargo.toml @@ -14,7 +14,7 @@ crate-type = ["lib", "cdylib", "staticlib"] [dependencies] anyhow = "1.0.86" axum = "0.7.5" -mithril-common = { path = "../../mithril-common", features = ["full"] } +mithril-common = { path = "../../mithril-common" } paste = "1.0.15" prometheus = "0.13.4" reqwest = { version = "0.12.7", features = ["json", "stream"] } @@ -22,7 +22,7 @@ slog = { version = "2.7.0", features = [ "max_level_trace", "release_max_level_debug", ] } -tokio = { version = "1.40.0", features = ["full"] } +tokio = { version = "1.40.0" } [dev-dependencies] prometheus-parse = "0.2.5" From 141fb1efecb407d53fdc067c77a0b0078f0c4b7e Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 14 Oct 2024 15:29:38 +0200 Subject: [PATCH 13/23] Use a generic instead of `Epoch` for `MetricGauge` --- Cargo.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 664d78f83ec..934b0cbf40c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3813,7 +3813,7 @@ dependencies = [ "paste", "prometheus", "prometheus-parse", - "reqwest 0.12.7", + "reqwest 0.12.8", "slog", "slog-async", "slog-term", From 6ce5a1e75a9f2c2847187c170bbef81f69e9539b Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 14 Oct 2024 15:29:38 +0200 Subject: [PATCH 14/23] Use a generic instead of `Epoch` for `MetricGauge` --- internal/mithril-metric/src/helper.rs | 6 +++--- internal/mithril-metric/src/metric.rs | 22 ++++++++++++---------- mithril-common/src/entities/epoch.rs | 6 ++++++ mithril-signer/src/metrics/service.rs | 12 ++++++++---- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/internal/mithril-metric/src/helper.rs b/internal/mithril-metric/src/helper.rs index 5b2bb3b3336..1c3162903c2 100644 --- a/internal/mithril-metric/src/helper.rs +++ b/internal/mithril-metric/src/helper.rs @@ -264,7 +264,7 @@ mod tests { service.get_gauge_example().record(Epoch(12)); assert_eq!(2, service.get_counter_example().get()); - assert_eq!(Epoch(12), service.get_gauge_example().get()); + assert_eq!(Epoch(12), Epoch(service.get_gauge_example().get() as u64)); } build_metrics_service!( @@ -287,7 +287,7 @@ mod tests { service.get_gauge_example().record(Epoch(12)); assert_eq!(2, service.get_counter_example().get()); - assert_eq!(Epoch(12), service.get_gauge_example().get()); + assert_eq!(Epoch(12), Epoch(service.get_gauge_example().get() as u64)); } #[test] @@ -317,7 +317,7 @@ mod tests { service.get_gauge_example().record(Epoch(12)); assert_eq!(2, service.get_counter_example().get()); - assert_eq!(Epoch(12), service.get_gauge_example().get()); + assert_eq!(Epoch(12), Epoch(service.get_gauge_example().get() as u64)); } #[test] diff --git a/internal/mithril-metric/src/metric.rs b/internal/mithril-metric/src/metric.rs index af781ce1724..380fb2a9a1f 100644 --- a/internal/mithril-metric/src/metric.rs +++ b/internal/mithril-metric/src/metric.rs @@ -1,7 +1,7 @@ use prometheus::{core::Collector, Counter, Gauge, Opts}; use slog::{debug, Logger}; -use mithril_common::{entities::Epoch, StdResult}; +use mithril_common::StdResult; /// Type alias for a metric name. pub type MetricName = str; @@ -79,16 +79,18 @@ impl MetricGauge { }) } - pub fn record(&self, epoch: Epoch) { + pub fn record + Copy>(&self, value: T) { debug!( self.logger, - "set '{}' gauge value to {}", self.name, epoch.0 + "set '{}' gauge value to {}", + self.name, + value.into() ); - self.gauge.set(epoch.0 as f64); + self.gauge.set(value.into()); } - pub fn get(&self) -> Epoch { - Epoch(self.gauge.get().round() as u64) + pub fn get(&self) -> f64 { + self.gauge.get().round() } fn create_metric_gauge(name: &MetricName, help: &str) -> StdResult { @@ -125,13 +127,13 @@ mod tests { } #[test] - fn test_metric_gauge_can_be_set() { + fn test_metric_gauge_can_be_set_and_the_getter_rounded_the_value() { let metric = MetricGauge::new(TestLogger::stdout(), "test_gauge", "test gauge help").unwrap(); assert_eq!(metric.name(), "test_gauge"); - assert_eq!(metric.get(), Epoch(0)); + assert_eq!(metric.get(), 0.0); - metric.record(Epoch(12)); - assert_eq!(metric.get(), Epoch(12)); + metric.record(12.3); + assert_eq!(metric.get(), 12.0); } } diff --git a/mithril-common/src/entities/epoch.rs b/mithril-common/src/entities/epoch.rs index 5e99e99ff42..a2f15c7290c 100644 --- a/mithril-common/src/entities/epoch.rs +++ b/mithril-common/src/entities/epoch.rs @@ -137,6 +137,12 @@ impl TryInto for &Epoch { } } +impl From for f64 { + fn from(value: Epoch) -> f64 { + value.0 as f64 + } +} + /// EpochError is an error triggered by an [Epoch] #[derive(Error, Debug)] pub enum EpochError { diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index c5860e4d2ab..96aa4a58aac 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -71,8 +71,10 @@ impl MetricsService { /// Get the `signer_registration_success_last_epoch` gauge value. pub fn signer_registration_success_last_epoch_gauge_get(&self) -> Epoch { - self.get_signer_registration_success_last_epoch_gauge() - .get() + Epoch( + self.get_signer_registration_success_last_epoch_gauge() + .get() as u64, + ) } /// Increment the `signature_registration_success_since_startup` counter. @@ -107,8 +109,10 @@ impl MetricsService { /// Get the `signature_registration_success_last_epoch` gauge value. pub fn signature_registration_success_last_epoch_gauge_get(&self) -> Epoch { - self.get_signature_registration_success_last_epoch_gauge() - .get() + Epoch( + self.get_signature_registration_success_last_epoch_gauge() + .get() as u64, + ) } /// Increment the `runtime_cycle_total_since_startup` counter. From 53bcd5400697003dc01e4d90989234b4e64d3855 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 14 Oct 2024 18:07:40 +0200 Subject: [PATCH 15/23] Move code of the function `metrics_tools::export_metrics` into the `export_metrics` function create by the macro and remove `metrics_tools` --- internal/mithril-metric/src/helper.rs | 82 ++++++++------------------- internal/mithril-metric/src/lib.rs | 4 +- internal/mithril-metric/src/server.rs | 2 +- mithril-signer/src/metrics/service.rs | 2 +- 4 files changed, 29 insertions(+), 61 deletions(-) diff --git a/internal/mithril-metric/src/helper.rs b/internal/mithril-metric/src/helper.rs index 1c3162903c2..858952eb09d 100644 --- a/internal/mithril-metric/src/helper.rs +++ b/internal/mithril-metric/src/helper.rs @@ -101,7 +101,7 @@ macro_rules! build_metrics_service { impl MetricsServiceExporter for $service { fn export_metrics(&self) -> StdResult { - metrics_tools::export_metrics(&self.registry) + Ok(prometheus::TextEncoder::new().encode_to_string(&self.registry.gather())?) } } @@ -109,25 +109,6 @@ macro_rules! build_metrics_service { }; } -pub mod metrics_tools { - - use mithril_common::StdResult; - use prometheus::TextEncoder; - - pub fn export_metrics(registry: &prometheus::Registry) -> StdResult { - let encoder = TextEncoder::new(); - let metric_families = registry.gather(); - // TODO check encode_utf8 do the same as encode and remove the commented code - // let mut buffer = vec![]; - // encoder.encode(&metric_families, &mut buffer)?; - // Ok(String::from_utf8(buffer)?) - - let mut buffer = String::new(); - encoder.encode_utf8(&metric_families, &mut buffer)?; - Ok(buffer) - } -} - // TODO do we create a module for that or put it in lib.rs ? #[cfg(test)] pub mod test_tools { @@ -160,7 +141,7 @@ mod tests { use super::*; use mithril_common::{entities::Epoch, StdResult}; - use prometheus::Registry; + use prometheus::{Registry, TextEncoder}; use prometheus_parse::Value; use slog::Logger; use test_tools::TestLogger; @@ -175,42 +156,6 @@ mod tests { ) } - mod tools { - use crate::MetricCounter; - - use super::*; - use prometheus::Registry; - - use mithril_common::entities::Epoch; - - #[test] - fn test_export_metrics() { - let counter_metric = - MetricCounter::new(TestLogger::stdout(), "test_counter", "test counter help") - .unwrap(); - counter_metric.increment(); - - let gauge_metric = - MetricGauge::new(TestLogger::stdout(), "test_gauge", "test gauge help").unwrap(); - gauge_metric.record(Epoch(12)); - - let registry = Registry::new(); - registry.register(counter_metric.collector()).unwrap(); - registry.register(gauge_metric.collector()).unwrap(); - - let exported_metrics = metrics_tools::export_metrics(®istry).unwrap(); - - let parsed_metrics = parse_metrics(&exported_metrics).unwrap(); - - let parsed_metrics_expected = BTreeMap::from([ - (counter_metric.name(), Value::Counter(1.0)), - (gauge_metric.name(), Value::Gauge(12.0)), - ]); - - assert_eq!(parsed_metrics_expected, parsed_metrics); - } - } - pub struct MetricsServiceExample { registry: Registry, counter_example: MetricCounter, @@ -252,7 +197,7 @@ mod tests { impl MetricsServiceExporter for MetricsServiceExample { fn export_metrics(&self) -> StdResult { - metrics_tools::export_metrics(&self.registry) + Ok(TextEncoder::new().encode_to_string(&self.registry.gather())?) } } @@ -329,6 +274,27 @@ mod tests { assert_eq!("gauge_example", service.get_gauge_example().name()); } + #[test] + fn test_build_metrics_service_provide_a_functional_export_metrics_function() { + let service = + MetricsServiceExampleBuildWithMacroWithoutMetricName::new(TestLogger::stdout()) + .unwrap(); + + service.counter_example.increment(); + service.gauge_example.record(Epoch(12)); + + let exported_metrics = service.export_metrics().unwrap(); + + let parsed_metrics = parse_metrics(&exported_metrics).unwrap(); + + let parsed_metrics_expected = BTreeMap::from([ + (service.counter_example.name(), Value::Counter(1.0)), + (service.gauge_example.name(), Value::Gauge(12.0)), + ]); + + assert_eq!(parsed_metrics_expected, parsed_metrics); + } + build_metrics_service!( MetricsServiceExampleBuildWithMacroUsingPrefix, "metric_prefix", diff --git a/internal/mithril-metric/src/lib.rs b/internal/mithril-metric/src/lib.rs index 2365d49eb34..78fe0f2cdfa 100644 --- a/internal/mithril-metric/src/lib.rs +++ b/internal/mithril-metric/src/lib.rs @@ -5,7 +5,9 @@ pub mod helper; pub mod metric; mod server; -pub use helper::*; pub use metric::*; pub use server::MetricsServer; pub use server::MetricsServiceExporter; + +#[cfg(test)] +pub use helper::test_tools::TestLogger; diff --git a/internal/mithril-metric/src/server.rs b/internal/mithril-metric/src/server.rs index 432648066f3..fb146ad90c9 100644 --- a/internal/mithril-metric/src/server.rs +++ b/internal/mithril-metric/src/server.rs @@ -112,7 +112,7 @@ mod tests { use std::time::Duration; use tokio::{sync::oneshot, task::yield_now, time::sleep}; - use crate::test_tools::TestLogger; + use crate::helper::test_tools::TestLogger; use super::*; diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index 96aa4a58aac..3d1f8cca358 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -1,4 +1,4 @@ -use mithril_metric::{build_metrics_service, metrics_tools, MetricsServiceExporter}; +use mithril_metric::{build_metrics_service, MetricsServiceExporter}; use prometheus::Registry; use slog::Logger; From 51cad724f710a1eddb2735763832f0fe7216f886 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Mon, 14 Oct 2024 18:22:49 +0200 Subject: [PATCH 16/23] The name of the metric must be provided to the macro --- internal/mithril-metric/src/helper.rs | 109 +------------------------- mithril-signer/src/metrics/service.rs | 25 +++--- 2 files changed, 19 insertions(+), 115 deletions(-) diff --git a/internal/mithril-metric/src/helper.rs b/internal/mithril-metric/src/helper.rs index 858952eb09d..2d8211404ef 100644 --- a/internal/mithril-metric/src/helper.rs +++ b/internal/mithril-metric/src/helper.rs @@ -2,8 +2,6 @@ /// /// To build the service you need to provide the structure name and a list of metrics. /// Each metrics is defined by an attribute name, a type, a metric name and a help message. -/// When the metric name is not provided the attribute name will be used. -/// We can specify a prefix that will be added to all metric names. /// /// The attribute name will be used to create a getter method for the metric. /// @@ -21,34 +19,12 @@ /// ) /// ); /// -/// build_metrics_service!( -/// MetricsService, -/// counter_example: MetricCounter("Example of a counter metric"), -/// gauge_example: MetricGauge("Example of a gauge metric") -/// ); -/// -/// build_metrics_service!( -/// MetricsService, -/// "metric_prefix", -/// counter_example: MetricCounter("Example of a counter metric"), -/// gauge_example: MetricGauge("Example of a gauge metric") -/// ); -/// -/// let service = MetricsServiceExampleBuildWithMacro::new(TestLogger::stdout()).unwrap(); +/// let service = MetricsService::new(TestLogger::stdout()).unwrap(); /// service.get_counter_example().record(); /// service.get_gauge_example().record(Epoch(12)); #[macro_export] macro_rules! build_metrics_service { - ($service:ident, $($metric_attribute:ident:$metric_type:ident($help:literal)),*) => { - build_metrics_service!($service, "", $($metric_attribute:$metric_type("", $help)),*); - }; - ($service:ident, $prefix:literal, $($metric_attribute:ident:$metric_type:ident($help:literal)),*) => { - build_metrics_service!($service, $prefix, $($metric_attribute:$metric_type("", $help)),*); - }; ($service:ident, $($metric_attribute:ident:$metric_type:ident($name:literal, $help:literal)),*) => { - build_metrics_service!($service, "", $($metric_attribute:$metric_type($name, $help)),*); - }; - ($service:ident, $prefix:literal, $($metric_attribute:ident:$metric_type:ident($name:literal, $help:literal)),*) => { paste::item! { /// Metrics service which is responsible for recording and exposing metrics. pub struct $service { @@ -64,21 +40,10 @@ macro_rules! build_metrics_service { let registry = Registry::new(); - let prefix = if $prefix.is_empty() { - "".to_string() - } else { - format!("{}_", $prefix) - }; - $( - let metric_name = if $name.is_empty() { - stringify!($metric_attribute) - } else { - $name - }; let $metric_attribute = $metric_type::new( logger.clone(), - &format!("{}{}", prefix, metric_name), + $name, $help, )?; registry.register($metric_attribute.collector())?; @@ -109,7 +74,6 @@ macro_rules! build_metrics_service { }; } -// TODO do we create a module for that or put it in lib.rs ? #[cfg(test)] pub mod test_tools { use std::{io, sync::Arc}; @@ -248,37 +212,9 @@ mod tests { ); } - build_metrics_service!( - MetricsServiceExampleBuildWithMacroWithoutMetricName, - counter_example: MetricCounter("Example of a counter metric"), - gauge_example: MetricGauge("Example of a gauge metric") - ); - - #[test] - fn test_build_metrics_service_macro_without_metric_name() { - let service = MetricsServiceExampleBuildWithMacro::new(TestLogger::stdout()).unwrap(); - service.get_counter_example().increment(); - service.get_counter_example().increment(); - service.get_gauge_example().record(Epoch(12)); - - assert_eq!(2, service.get_counter_example().get()); - assert_eq!(Epoch(12), Epoch(service.get_gauge_example().get() as u64)); - } - - #[test] - fn test_build_metrics_service_named_metrics_without_attribute_name() { - let service = - MetricsServiceExampleBuildWithMacroWithoutMetricName::new(TestLogger::stdout()) - .unwrap(); - assert_eq!("counter_example", service.get_counter_example().name()); - assert_eq!("gauge_example", service.get_gauge_example().name()); - } - #[test] fn test_build_metrics_service_provide_a_functional_export_metrics_function() { - let service = - MetricsServiceExampleBuildWithMacroWithoutMetricName::new(TestLogger::stdout()) - .unwrap(); + let service = MetricsServiceExampleBuildWithMacro::new(TestLogger::stdout()).unwrap(); service.counter_example.increment(); service.gauge_example.record(Epoch(12)); @@ -294,43 +230,4 @@ mod tests { assert_eq!(parsed_metrics_expected, parsed_metrics); } - - build_metrics_service!( - MetricsServiceExampleBuildWithMacroUsingPrefix, - "metric_prefix", - counter_example: MetricCounter("Example of a counter metric"), - gauge_example: MetricGauge("Example of a gauge metric") - ); - - #[test] - fn test_build_metrics_service_named_metrics_with_attribute_name_and_prefix() { - let service = - MetricsServiceExampleBuildWithMacroUsingPrefix::new(TestLogger::stdout()).unwrap(); - assert_eq!( - "metric_prefix_counter_example", - service.get_counter_example().name() - ); - assert_eq!( - "metric_prefix_gauge_example", - service.get_gauge_example().name() - ); - } - - #[test] - fn test_build_metrics_service_implement_export() { - let service = - MetricsServiceExampleBuildWithMacroUsingPrefix::new(TestLogger::stdout()).unwrap(); - service.get_gauge_example().record(Epoch(12)); - - let export = service.export_metrics().unwrap(); - let metrics = parse_metrics(&export).unwrap(); - - assert_eq!( - Value::Gauge(12.0), - metrics - .get(&service.get_gauge_example().name()) - .unwrap() - .clone() - ); - } } diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index 3d1f8cca358..52fba37237e 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -8,30 +8,37 @@ use mithril_metric::metric::{CounterValue, MetricCollector, MetricCounter, Metri build_metrics_service!( MetricsService, - "mithril_signer", - signer_registration_success_since_startup_counter: MetricCounter( + signer_registration_success_since_startup_counter:MetricCounter( + "mithril_signer_signer_registration_success_since_startup", "Number of successful signer registrations since startup on a Mithril signer node" ), - signer_registration_total_since_startup_counter: MetricCounter( + signer_registration_total_since_startup_counter:MetricCounter( + "mithril_signer_signer_registration_total_since_startup", "Number of signer registrations since startup on a Mithril signer node" ), - signer_registration_success_last_epoch_gauge: MetricGauge( + signer_registration_success_last_epoch_gauge:MetricGauge( + "mithril_signer_signer_registration_success_last_epoch", "Latest epoch at which signer successfully registered on a Mithril signer node" ), - signature_registration_success_since_startup_counter: MetricCounter( + signature_registration_success_since_startup_counter:MetricCounter( + "mithril_signer_signature_registration_success_since_startup", "Number of successful signature registrations since startup on a Mithril signer node" ), - signature_registration_total_since_startup_counter: MetricCounter( + signature_registration_total_since_startup_counter:MetricCounter( + "mithril_signer_signature_registration_total_since_startup", "Number of signature registrations since startup on a Mithril signer node" ), - signature_registration_success_last_epoch_gauge: MetricGauge( + signature_registration_success_last_epoch_gauge:MetricGauge( + "mithril_signer_signature_registration_success_last_epoch", "Latest epoch at which signature successfully registered on a Mithril signer node" ), // Runtime cycle metrics - runtime_cycle_success_since_startup_counter: MetricCounter( + runtime_cycle_success_since_startup_counter:MetricCounter( + "mithril_signer_runtime_cycle_success_since_startup", "Number of successful runtime cycles since startup on a Mithril signer node" ), - runtime_cycle_total_since_startup_counter: MetricCounter( + runtime_cycle_total_since_startup_counter:MetricCounter( + "mithril_signer_runtime_cycle_total_since_startup", "Number of runtime cycles since startup on a Mithril signer node" ) From fa1e5fa774a739314183c8e2b51829c9f97ce744 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 15 Oct 2024 11:33:26 +0200 Subject: [PATCH 17/23] Upgrade versions and fix fmt check warning --- internal/mithril-metric/Cargo.toml | 6 +++--- .../tests/test_extensions/state_machine_tester.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/mithril-metric/Cargo.toml b/internal/mithril-metric/Cargo.toml index a36f1af903f..36044ad507d 100644 --- a/internal/mithril-metric/Cargo.toml +++ b/internal/mithril-metric/Cargo.toml @@ -12,12 +12,12 @@ repository = { workspace = true } crate-type = ["lib", "cdylib", "staticlib"] [dependencies] -anyhow = "1.0.86" -axum = "0.7.5" +anyhow = "1.0.89" +axum = "0.7.7" mithril-common = { path = "../../mithril-common" } paste = "1.0.15" prometheus = "0.13.4" -reqwest = { version = "0.12.7", features = ["json", "stream"] } +reqwest = { version = "0.12.8", features = ["json", "stream"] } slog = { version = "2.7.0", features = [ "max_level_trace", "release_max_level_debug", diff --git a/mithril-signer/tests/test_extensions/state_machine_tester.rs b/mithril-signer/tests/test_extensions/state_machine_tester.rs index 39280c615b5..667f50b4d89 100644 --- a/mithril-signer/tests/test_extensions/state_machine_tester.rs +++ b/mithril-signer/tests/test_extensions/state_machine_tester.rs @@ -1,6 +1,6 @@ #![allow(dead_code)] use anyhow::anyhow; -use mithril_metric::{MetricsServiceExporter, MetricCollector}; +use mithril_metric::{MetricCollector, MetricsServiceExporter}; use prometheus_parse::Value; use slog::Drain; use slog_scope::debug; From 97c31a592e8af172658f37dcd54441bd77083eb5 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Tue, 15 Oct 2024 14:35:42 +0200 Subject: [PATCH 18/23] Harmonize log messages --- internal/mithril-metric/src/metric.rs | 4 ++-- internal/mithril-metric/src/server.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/mithril-metric/src/metric.rs b/internal/mithril-metric/src/metric.rs index 380fb2a9a1f..d4ccb309254 100644 --- a/internal/mithril-metric/src/metric.rs +++ b/internal/mithril-metric/src/metric.rs @@ -36,7 +36,7 @@ impl MetricCounter { } pub fn increment(&self) { - debug!(self.logger, "incrementing '{}' counter", self.name); + debug!(self.logger, "Incrementing '{}' counter", self.name); self.counter.inc(); } @@ -82,7 +82,7 @@ impl MetricGauge { pub fn record + Copy>(&self, value: T) { debug!( self.logger, - "set '{}' gauge value to {}", + "Set '{}' gauge value to {}", self.name, value.into() ); diff --git a/internal/mithril-metric/src/server.rs b/internal/mithril-metric/src/server.rs index fb146ad90c9..67b9279c533 100644 --- a/internal/mithril-metric/src/server.rs +++ b/internal/mithril-metric/src/server.rs @@ -68,7 +68,7 @@ impl MetricsServer { pub async fn start(&self, shutdown_rx: Receiver<()>) -> StdResult<()> { info!( self.logger, - "starting HTTP server for metrics on port {}", self.server_port + "Starting HTTP server for metrics on port {}", self.server_port ); let router_state = Arc::new(RouterState { @@ -80,7 +80,7 @@ impl MetricsServer { "/metrics", get(|State(state): State>>| async move { state.metrics_service.export_metrics().map_err(|e| { - error!(state.logger, "error exporting metrics"; "error" => ?e); + error!(state.logger, "Error exporting metrics"; "error" => ?e); MetricsServerError::Internal(e) }) }), From 441dd74f1eccd2558f844b0d52deeec72561d7b8 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 16 Oct 2024 08:58:34 +0200 Subject: [PATCH 19/23] Add missing documentation --- internal/mithril-metric/Makefile | 2 +- internal/mithril-metric/src/helper.rs | 10 ++++++---- internal/mithril-metric/src/lib.rs | 2 ++ internal/mithril-metric/src/metric.rs | 8 ++++++++ internal/mithril-metric/src/server.rs | 2 ++ mithril-signer/src/metrics/service.rs | 5 ++--- 6 files changed, 21 insertions(+), 8 deletions(-) diff --git a/internal/mithril-metric/Makefile b/internal/mithril-metric/Makefile index adc3e3d5c11..d66d6d9fefc 100644 --- a/internal/mithril-metric/Makefile +++ b/internal/mithril-metric/Makefile @@ -16,4 +16,4 @@ check: ${CARGO} fmt --check doc: - ${CARGO} doc --no-deps --open --features full + ${CARGO} doc --no-deps --open diff --git a/internal/mithril-metric/src/helper.rs b/internal/mithril-metric/src/helper.rs index 2d8211404ef..cd4493690b5 100644 --- a/internal/mithril-metric/src/helper.rs +++ b/internal/mithril-metric/src/helper.rs @@ -1,3 +1,5 @@ +//! Helper to create a metric service. + /// Create a MetricService. /// /// To build the service you need to provide the structure name and a list of metrics. @@ -28,7 +30,7 @@ macro_rules! build_metrics_service { paste::item! { /// Metrics service which is responsible for recording and exposing metrics. pub struct $service { - registry: Registry, + registry: prometheus::Registry, $( $metric_attribute: $metric_type, )* @@ -36,9 +38,9 @@ macro_rules! build_metrics_service { impl $service { /// Create a new MetricsService instance. - pub fn new(logger: Logger) -> StdResult { + pub fn new(logger: slog::Logger) -> mithril_common::StdResult { - let registry = Registry::new(); + let registry = prometheus::Registry::new(); $( let $metric_attribute = $metric_type::new( @@ -65,7 +67,7 @@ macro_rules! build_metrics_service { } impl MetricsServiceExporter for $service { - fn export_metrics(&self) -> StdResult { + fn export_metrics(&self) -> mithril_common::StdResult { Ok(prometheus::TextEncoder::new().encode_to_string(&self.registry.gather())?) } } diff --git a/internal/mithril-metric/src/lib.rs b/internal/mithril-metric/src/lib.rs index 78fe0f2cdfa..26fcf63e61f 100644 --- a/internal/mithril-metric/src/lib.rs +++ b/internal/mithril-metric/src/lib.rs @@ -1,3 +1,5 @@ +#![warn(missing_docs)] + //! metrics module. //! This module contains the tools to create a metrics service and a metrics server. diff --git a/internal/mithril-metric/src/metric.rs b/internal/mithril-metric/src/metric.rs index d4ccb309254..462a5c0f79a 100644 --- a/internal/mithril-metric/src/metric.rs +++ b/internal/mithril-metric/src/metric.rs @@ -1,3 +1,5 @@ +//! This module contains wrapper to prometheus metrics for use in a metrics service. + use prometheus::{core::Collector, Counter, Gauge, Opts}; use slog::{debug, Logger}; @@ -26,6 +28,7 @@ pub struct MetricCounter { } impl MetricCounter { + /// Create a new metric counter. pub fn new(logger: Logger, name: &str, help: &str) -> StdResult { let counter = MetricCounter::create_metric_counter(name, help)?; Ok(Self { @@ -35,11 +38,13 @@ impl MetricCounter { }) } + /// Increment the counter. pub fn increment(&self) { debug!(self.logger, "Incrementing '{}' counter", self.name); self.counter.inc(); } + /// Get the counter value. pub fn get(&self) -> CounterValue { self.counter.get().round() as CounterValue } @@ -70,6 +75,7 @@ pub struct MetricGauge { } impl MetricGauge { + /// Create a new metric gauge. pub fn new(logger: Logger, name: &str, help: &str) -> StdResult { let gauge = MetricGauge::create_metric_gauge(name, help)?; Ok(Self { @@ -79,6 +85,7 @@ impl MetricGauge { }) } + /// Record a value in the gauge. pub fn record + Copy>(&self, value: T) { debug!( self.logger, @@ -89,6 +96,7 @@ impl MetricGauge { self.gauge.set(value.into()); } + /// Get the gauge value. pub fn get(&self) -> f64 { self.gauge.get().round() } diff --git a/internal/mithril-metric/src/server.rs b/internal/mithril-metric/src/server.rs index 67b9279c533..1c63031c63d 100644 --- a/internal/mithril-metric/src/server.rs +++ b/internal/mithril-metric/src/server.rs @@ -13,7 +13,9 @@ use tokio::sync::oneshot::Receiver; use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; +/// Metrics service exporter gives the possibility of exporting metrics. pub trait MetricsServiceExporter { + /// Export metrics. fn export_metrics(&self) -> StdResult; } diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index 52fba37237e..e44326455e4 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -1,8 +1,6 @@ use mithril_metric::{build_metrics_service, MetricsServiceExporter}; -use prometheus::Registry; -use slog::Logger; -use mithril_common::{entities::Epoch, StdResult}; +use mithril_common::entities::Epoch; use mithril_metric::metric::{CounterValue, MetricCollector, MetricCounter, MetricGauge}; @@ -147,6 +145,7 @@ impl MetricsService { #[cfg(test)] mod tests { + use mithril_common::StdResult; use prometheus_parse::Value; use std::collections::BTreeMap; From cedeb2642334e6453b647f6b8c6d4adbffbebefd Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 16 Oct 2024 09:20:13 +0200 Subject: [PATCH 20/23] Add `Send + Sync` to MetricServiceExporter trait and remove `Copy` from generic in `MetricGauge::record` function and the `round` in getter --- internal/mithril-metric/src/metric.rs | 16 ++++++---------- internal/mithril-metric/src/server.rs | 8 ++++---- mithril-signer/src/metrics/service.rs | 1 - 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/internal/mithril-metric/src/metric.rs b/internal/mithril-metric/src/metric.rs index 462a5c0f79a..24ea8680af0 100644 --- a/internal/mithril-metric/src/metric.rs +++ b/internal/mithril-metric/src/metric.rs @@ -86,19 +86,15 @@ impl MetricGauge { } /// Record a value in the gauge. - pub fn record + Copy>(&self, value: T) { - debug!( - self.logger, - "Set '{}' gauge value to {}", - self.name, - value.into() - ); - self.gauge.set(value.into()); + pub fn record>(&self, value: T) { + let value = value.into(); + debug!(self.logger, "Set '{}' gauge value to {}", self.name, value); + self.gauge.set(value); } /// Get the gauge value. pub fn get(&self) -> f64 { - self.gauge.get().round() + self.gauge.get() } fn create_metric_gauge(name: &MetricName, help: &str) -> StdResult { @@ -142,6 +138,6 @@ mod tests { assert_eq!(metric.get(), 0.0); metric.record(12.3); - assert_eq!(metric.get(), 12.0); + assert_eq!(metric.get(), 12.3); } } diff --git a/internal/mithril-metric/src/server.rs b/internal/mithril-metric/src/server.rs index 1c63031c63d..c9ab4baabe5 100644 --- a/internal/mithril-metric/src/server.rs +++ b/internal/mithril-metric/src/server.rs @@ -14,7 +14,7 @@ use mithril_common::logging::LoggerExtensions; use mithril_common::StdResult; /// Metrics service exporter gives the possibility of exporting metrics. -pub trait MetricsServiceExporter { +pub trait MetricsServiceExporter: Send + Sync { /// Export metrics. fn export_metrics(&self) -> StdResult; } @@ -38,19 +38,19 @@ impl IntoResponse for MetricsServerError { } /// The MetricsServer is responsible for exposing the metrics of the signer. -pub struct MetricsServer { +pub struct MetricsServer { server_port: u16, server_ip: String, metrics_service: Arc, logger: Logger, } -struct RouterState { +struct RouterState { metrics_service: Arc, logger: Logger, } -impl MetricsServer { +impl MetricsServer { /// Create a new MetricsServer instance. pub fn new(server_ip: &str, server_port: u16, metrics_service: Arc, logger: Logger) -> Self { Self { diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index e44326455e4..651534d92fa 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -30,7 +30,6 @@ build_metrics_service!( "mithril_signer_signature_registration_success_last_epoch", "Latest epoch at which signature successfully registered on a Mithril signer node" ), - // Runtime cycle metrics runtime_cycle_success_since_startup_counter:MetricCounter( "mithril_signer_runtime_cycle_success_since_startup", "Number of successful runtime cycles since startup on a Mithril signer node" From 006e9ef4e9e0cb0a99fb4485c77020c947424052 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 16 Oct 2024 10:33:52 +0200 Subject: [PATCH 21/23] Fix code in documentation --- internal/mithril-metric/src/helper.rs | 37 +++++++++++++++++---------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/internal/mithril-metric/src/helper.rs b/internal/mithril-metric/src/helper.rs index cd4493690b5..05759c4a61d 100644 --- a/internal/mithril-metric/src/helper.rs +++ b/internal/mithril-metric/src/helper.rs @@ -9,21 +9,30 @@ /// /// Crate that use this macro should have `paste` as dependency. /// -/// build_metrics_service!( -/// MetricsService, -/// counter_example: MetricCounter( -/// "custom_counter_example_name", -/// "Example of a counter metric" -/// ), -/// gauge_example: MetricGauge( -/// "custom_gauge_example_name", -/// "Example of a gauge metric" -/// ) -/// ); +/// # Example of 'build_metrics_service' and metrics usage /// -/// let service = MetricsService::new(TestLogger::stdout()).unwrap(); -/// service.get_counter_example().record(); -/// service.get_gauge_example().record(Epoch(12)); +/// ``` +/// use slog::Logger; +/// use mithril_common::{entities::Epoch, StdResult}; +/// use mithril_metric::build_metrics_service; +/// use mithril_metric::{MetricCollector, MetricCounter, MetricGauge, MetricsServiceExporter}; +/// +/// build_metrics_service!( +/// MetricsService, +/// counter_example: MetricCounter( +/// "custom_counter_example_name", +/// "Example of a counter metric" +/// ), +/// gauge_example: MetricGauge( +/// "custom_gauge_example_name", +/// "Example of a gauge metric" +/// ) +/// ); +/// +/// let service = MetricsService::new(Logger::root(slog::Discard, slog::o!())).unwrap(); +/// service.get_counter_example().increment(); +/// service.get_gauge_example().record(Epoch(12)); +/// ``` #[macro_export] macro_rules! build_metrics_service { ($service:ident, $($metric_attribute:ident:$metric_type:ident($name:literal, $help:literal)),*) => { From 78b11313b0a5fd48a6f1f10611a51f5b4e6ee46a Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 16 Oct 2024 14:53:44 +0200 Subject: [PATCH 22/23] Remove obsolete functions from signer MetricsService --- mithril-signer/src/metrics/service.rs | 306 +----------------- mithril-signer/src/runtime/state_machine.rs | 25 +- .../test_extensions/state_machine_tester.rs | 15 +- 3 files changed, 28 insertions(+), 318 deletions(-) diff --git a/mithril-signer/src/metrics/service.rs b/mithril-signer/src/metrics/service.rs index 651534d92fa..171f3ec8e7b 100644 --- a/mithril-signer/src/metrics/service.rs +++ b/mithril-signer/src/metrics/service.rs @@ -1,8 +1,6 @@ use mithril_metric::{build_metrics_service, MetricsServiceExporter}; -use mithril_common::entities::Epoch; - -use mithril_metric::metric::{CounterValue, MetricCollector, MetricCounter, MetricGauge}; +use mithril_metric::metric::{MetricCollector, MetricCounter, MetricGauge}; build_metrics_service!( MetricsService, @@ -40,305 +38,3 @@ build_metrics_service!( ) ); - -// TODO these functions are kept while changes are made for all uses -impl MetricsService { - /// Increment the `signer_registration_success_since_startup` counter. - pub fn signer_registration_success_since_startup_counter_increment(&self) { - self.get_signer_registration_success_since_startup_counter() - .increment(); - } - - /// Get the `signer_registration_success_since_startup` counter. - pub fn signer_registration_success_since_startup_counter_get(&self) -> CounterValue { - self.get_signer_registration_success_since_startup_counter() - .get() - } - - /// Increment the `signer_registration_total_since_startup` counter. - pub fn signer_registration_total_since_startup_counter_increment(&self) { - self.get_signer_registration_total_since_startup_counter() - .increment(); - } - - /// Get the `signer_registration_total_since_startup` counter. - pub fn signer_registration_total_since_startup_counter_get(&self) -> CounterValue { - self.get_signer_registration_total_since_startup_counter() - .get() - } - - /// Set the `signer_registration_success_last_epoch` gauge value. - pub fn signer_registration_success_last_epoch_gauge_set(&self, value: Epoch) { - self.get_signer_registration_success_last_epoch_gauge() - .record(value); - } - - /// Get the `signer_registration_success_last_epoch` gauge value. - pub fn signer_registration_success_last_epoch_gauge_get(&self) -> Epoch { - Epoch( - self.get_signer_registration_success_last_epoch_gauge() - .get() as u64, - ) - } - - /// Increment the `signature_registration_success_since_startup` counter. - pub fn signature_registration_success_since_startup_counter_increment(&self) { - self.get_signature_registration_success_since_startup_counter() - .increment(); - } - - /// Get the `signature_registration_success_since_startup` counter. - pub fn signature_registration_success_since_startup_counter_get(&self) -> CounterValue { - self.get_signature_registration_success_since_startup_counter() - .get() - } - - /// Increment the `signature_registration_total_since_startup` counter. - pub fn signature_registration_total_since_startup_counter_increment(&self) { - self.get_signature_registration_total_since_startup_counter() - .increment(); - } - - /// Get the `signature_registration_total_since_startup` counter. - pub fn signature_registration_total_since_startup_counter_get(&self) -> CounterValue { - self.get_signature_registration_total_since_startup_counter() - .get() - } - - /// Set the `signature_registration_success_last_epoch` gauge value. - pub fn signature_registration_success_last_epoch_gauge_set(&self, value: Epoch) { - self.get_signature_registration_success_last_epoch_gauge() - .record(value); - } - - /// Get the `signature_registration_success_last_epoch` gauge value. - pub fn signature_registration_success_last_epoch_gauge_get(&self) -> Epoch { - Epoch( - self.get_signature_registration_success_last_epoch_gauge() - .get() as u64, - ) - } - - /// Increment the `runtime_cycle_total_since_startup` counter. - pub fn runtime_cycle_total_since_startup_counter_increment(&self) { - self.get_runtime_cycle_total_since_startup_counter() - .increment(); - } - - /// Get the `runtime_cycle_total_since_startup` counter. - pub fn runtime_cycle_total_since_startup_counter_get(&self) -> CounterValue { - self.get_runtime_cycle_total_since_startup_counter().get() - } - - /// Increment the `runtime_cycle_success_since_startup` counter. - pub fn runtime_cycle_success_since_startup_counter_increment(&self) { - self.get_runtime_cycle_success_since_startup_counter() - .increment(); - } - - /// Get the `runtime_cycle_success_since_startup` counter. - pub fn runtime_cycle_success_since_startup_counter_get(&self) -> CounterValue { - self.get_runtime_cycle_success_since_startup_counter().get() - } -} - -#[cfg(test)] -mod tests { - use mithril_common::StdResult; - use prometheus_parse::Value; - use std::collections::BTreeMap; - - use crate::test_tools::TestLogger; - - use super::*; - - fn parse_metrics(raw_metrics: &str) -> StdResult> { - Ok( - prometheus_parse::Scrape::parse(raw_metrics.lines().map(|s| Ok(s.to_owned())))? - .samples - .into_iter() - .map(|s| (s.metric, s.value)) - .collect::>(), - ) - } - - #[test] - fn test_export_metrics() { - let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); - let exported_metrics = metrics_service.export_metrics().unwrap(); - - let parsed_metrics = parse_metrics(&exported_metrics).unwrap(); - - let parsed_metrics_expected = BTreeMap::from([ - ( - metrics_service - .get_runtime_cycle_success_since_startup_counter() - .name(), - Value::Counter(0.0), - ), - ( - metrics_service - .get_runtime_cycle_total_since_startup_counter() - .name(), - Value::Counter(0.0), - ), - ( - metrics_service - .get_signature_registration_success_last_epoch_gauge() - .name(), - Value::Gauge(0.0), - ), - ( - metrics_service - .get_signature_registration_success_since_startup_counter() - .name(), - Value::Counter(0.0), - ), - ( - metrics_service - .get_signature_registration_total_since_startup_counter() - .name(), - Value::Counter(0.0), - ), - ( - metrics_service - .get_signer_registration_success_last_epoch_gauge() - .name(), - Value::Gauge(0.0), - ), - ( - metrics_service - .get_signer_registration_success_since_startup_counter() - .name(), - Value::Counter(0.0), - ), - ( - metrics_service - .get_signer_registration_total_since_startup_counter() - .name(), - Value::Counter(0.0), - ), - ]); - assert_eq!(parsed_metrics_expected, parsed_metrics); - } - - #[test] - fn test_signer_registration_success_since_startup_counter_increment() { - let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); - assert_eq!( - 0, - metrics_service.signer_registration_success_since_startup_counter_get(), - ); - - metrics_service.signer_registration_success_since_startup_counter_increment(); - assert_eq!( - 1, - metrics_service.signer_registration_success_since_startup_counter_get(), - ); - } - - #[test] - fn test_signer_registration_total_since_startup_counter_increment() { - let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); - assert_eq!( - 0, - metrics_service.signer_registration_total_since_startup_counter_get(), - ); - - metrics_service.signer_registration_total_since_startup_counter_increment(); - assert_eq!( - 1, - metrics_service.signer_registration_total_since_startup_counter_get(), - ); - } - - #[test] - fn test_signer_registration_success_last_epoch_gauge_set() { - let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); - assert_eq!( - Epoch(0), - metrics_service.signer_registration_success_last_epoch_gauge_get(), - ); - - metrics_service.signer_registration_success_last_epoch_gauge_set(Epoch(123)); - assert_eq!( - Epoch(123), - metrics_service.signer_registration_success_last_epoch_gauge_get(), - ); - } - - #[test] - fn test_signature_registration_success_since_startup_counter_increment() { - let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); - assert_eq!( - 0, - metrics_service.signature_registration_success_since_startup_counter_get(), - ); - - metrics_service.signature_registration_success_since_startup_counter_increment(); - assert_eq!( - 1, - metrics_service.signature_registration_success_since_startup_counter_get(), - ); - } - - #[test] - fn test_signature_registration_total_since_startup_counter_increment() { - let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); - assert_eq!( - 0, - metrics_service.signature_registration_total_since_startup_counter_get(), - ); - - metrics_service.signature_registration_total_since_startup_counter_increment(); - assert_eq!( - 1, - metrics_service.signature_registration_total_since_startup_counter_get(), - ); - } - - #[test] - fn test_signature_registration_success_last_epoch_gauge_set() { - let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); - assert_eq!( - Epoch(0), - metrics_service.signature_registration_success_last_epoch_gauge_get(), - ); - - metrics_service.signature_registration_success_last_epoch_gauge_set(Epoch(123)); - assert_eq!( - Epoch(123), - metrics_service.signature_registration_success_last_epoch_gauge_get(), - ); - } - - #[test] - fn test_runtime_cycle_success_since_startup_counter_increment() { - let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); - assert_eq!( - 0, - metrics_service.runtime_cycle_success_since_startup_counter_get(), - ); - - metrics_service.runtime_cycle_success_since_startup_counter_increment(); - assert_eq!( - 1, - metrics_service.runtime_cycle_success_since_startup_counter_get(), - ); - } - - #[test] - fn test_runtime_cycle_total_since_startup_counter_increment() { - let metrics_service = MetricsService::new(TestLogger::stdout()).unwrap(); - assert_eq!( - 0, - metrics_service.runtime_cycle_total_since_startup_counter_get(), - ); - - metrics_service.runtime_cycle_total_since_startup_counter_increment(); - assert_eq!( - 1, - metrics_service.runtime_cycle_total_since_startup_counter_get(), - ); - } -} diff --git a/mithril-signer/src/runtime/state_machine.rs b/mithril-signer/src/runtime/state_machine.rs index 1cce7410e11..8e66889fe84 100644 --- a/mithril-signer/src/runtime/state_machine.rs +++ b/mithril-signer/src/runtime/state_machine.rs @@ -138,7 +138,8 @@ impl StateMachine { info!(self.logger, "New cycle: {}", *state); self.metrics_service - .runtime_cycle_total_since_startup_counter_increment(); + .get_runtime_cycle_total_since_startup_counter() + .increment(); match state.deref() { SignerState::Init => { @@ -233,7 +234,8 @@ impl StateMachine { }; self.metrics_service - .runtime_cycle_success_since_startup_counter_increment(); + .get_runtime_cycle_success_since_startup_counter() + .increment(); Ok(()) } @@ -280,7 +282,8 @@ impl StateMachine { epoch_settings: SignerEpochSettings, ) -> Result { self.metrics_service - .signer_registration_total_since_startup_counter_increment(); + .get_signer_registration_total_since_startup_counter() + .increment(); let time_point = self .get_current_time_point("unregistered → registered") @@ -314,9 +317,12 @@ impl StateMachine { })?; self.metrics_service - .signer_registration_success_since_startup_counter_increment(); + .get_signer_registration_success_since_startup_counter() + .increment(); + self.metrics_service - .signer_registration_success_last_epoch_gauge_set(epoch); + .get_signer_registration_success_last_epoch_gauge() + .record(epoch); self.runner .upkeep(epoch) @@ -378,7 +384,8 @@ impl StateMachine { ); self.metrics_service - .signature_registration_total_since_startup_counter_increment(); + .get_signature_registration_total_since_startup_counter() + .increment(); let message = self .runner @@ -397,9 +404,11 @@ impl StateMachine { })?; self.metrics_service - .signature_registration_success_since_startup_counter_increment(); + .get_signature_registration_success_since_startup_counter() + .increment(); self.metrics_service - .signature_registration_success_last_epoch_gauge_set(current_epoch); + .get_signature_registration_success_last_epoch_gauge() + .record(current_epoch); Ok(SignerState::ReadyToSign { epoch: current_epoch, diff --git a/mithril-signer/tests/test_extensions/state_machine_tester.rs b/mithril-signer/tests/test_extensions/state_machine_tester.rs index 667f50b4d89..23d9a9dbf32 100644 --- a/mithril-signer/tests/test_extensions/state_machine_tester.rs +++ b/mithril-signer/tests/test_extensions/state_machine_tester.rs @@ -336,12 +336,14 @@ impl StateMachineTester { /// trigger a cycle in the state machine async fn cycle(&mut self) -> Result<&mut Self> { self.expected_metrics_service - .runtime_cycle_total_since_startup_counter_increment(); + .get_runtime_cycle_total_since_startup_counter() + .increment(); self.state_machine.cycle().await?; self.expected_metrics_service - .runtime_cycle_success_since_startup_counter_increment(); + .get_runtime_cycle_success_since_startup_counter() + .increment(); Ok(self) } @@ -372,7 +374,8 @@ impl StateMachineTester { ) -> Result<&mut Self> { let metric_before = self .metrics_service - .signature_registration_success_since_startup_counter_get(); + .get_signature_registration_success_since_startup_counter() + .get(); self.cycle_ready_to_sign().await?; @@ -386,7 +389,8 @@ impl StateMachineTester { ) -> Result<&mut Self> { let metric_before = self .metrics_service - .signature_registration_success_since_startup_counter_get(); + .get_signature_registration_success_since_startup_counter() + .get(); self.cycle_ready_to_sign().await?; @@ -678,7 +682,8 @@ impl StateMachineTester { ) -> Result<&mut Self> { let metric = self .metrics_service - .signature_registration_success_since_startup_counter_get(); + .get_signature_registration_success_since_startup_counter() + .get(); self.assert( expected_metric == metric, From 98038e1826993c48290523bccdce21bce3f3d90a Mon Sep 17 00:00:00 2001 From: sfauvel Date: Wed, 16 Oct 2024 14:59:08 +0200 Subject: [PATCH 23/23] chore: upgrade crate versions * mithril-metric from `0.0.1` to `0.1.0` * mithril-common from `0.4.70` to `0.4.71` * mithril-signer from `0.2.201` to `0.2.202` --- Cargo.lock | 6 +++--- internal/mithril-metric/Cargo.toml | 2 +- mithril-common/Cargo.toml | 2 +- mithril-signer/Cargo.toml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 934b0cbf40c..1a39f2d741f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3708,7 +3708,7 @@ dependencies = [ [[package]] name = "mithril-common" -version = "0.4.70" +version = "0.4.71" dependencies = [ "anyhow", "async-trait", @@ -3805,7 +3805,7 @@ dependencies = [ [[package]] name = "mithril-metric" -version = "0.0.1" +version = "0.1.0" dependencies = [ "anyhow", "axum", @@ -3866,7 +3866,7 @@ dependencies = [ [[package]] name = "mithril-signer" -version = "0.2.201" +version = "0.2.202" dependencies = [ "anyhow", "async-trait", diff --git a/internal/mithril-metric/Cargo.toml b/internal/mithril-metric/Cargo.toml index 36044ad507d..7e610ab1145 100644 --- a/internal/mithril-metric/Cargo.toml +++ b/internal/mithril-metric/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-metric" -version = "0.0.1" +version = "0.1.0" description = "Common tools to expose metrics." authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index da5dbf73c40..a0ac19208fa 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-common" -version = "0.4.70" +version = "0.4.71" description = "Common types, interfaces, and utilities for Mithril nodes." authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-signer/Cargo.toml b/mithril-signer/Cargo.toml index 9fdf76cf49d..0ffd65e490c 100644 --- a/mithril-signer/Cargo.toml +++ b/mithril-signer/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-signer" -version = "0.2.201" +version = "0.2.202" description = "A Mithril Signer" authors = { workspace = true } edition = { workspace = true }