A0-3060: Separate network metrics (#1371)

# Description Move the network metrics out of the global metrics, the last ones that are there and shouldn't be. ## Type of change # Checklist: Co-authored-by: timorl <[email protected]>
Cardinal-Cryptography · Aug 29, 2023 · 7fd599c · 7fd599c
1 parent c073c96
commit 7fd599c
Show file tree

Hide file tree

Showing 6 changed files with 168 additions and 102 deletions.
diff --git a/finality-aleph/src/metrics.rs b/finality-aleph/src/metrics.rs
@@ -10,12 +10,7 @@ use log::{trace, warn};
 use lru::LruCache;
 use parking_lot::Mutex;
 use sc_service::Arc;
-use substrate_prometheus_endpoint::{
-    exponential_buckets, prometheus::HistogramTimer, register, Gauge, Histogram, HistogramOpts,
-    Opts, PrometheusError, Registry, U64,
-};
-
-use crate::Protocol;
+use substrate_prometheus_endpoint::{register, Gauge, PrometheusError, Registry, U64};
 
 // How many entries (block hash + timestamp) we keep in memory per one checkpoint type.
 // Each entry takes 32B (Hash) + 16B (Instant), so a limit of 5000 gives ~234kB (per checkpoint).
@@ -32,7 +27,6 @@ struct Inner<H: Key> {
     prev: HashMap<Checkpoint, Checkpoint>,
     gauges: HashMap<Checkpoint, Gauge<U64>>,
     starts: HashMap<Checkpoint, LruCache<H, Instant>>,
-    network_send_times: HashMap<Protocol, Histogram>,
 }
 
 impl<H: Key> Inner<H> {
@@ -60,29 +54,6 @@ impl<H: Key> Inner<H> {
             );
         }
 
-        use Protocol::*;
-        let mut network_send_times = HashMap::new();
-        for key in [Authentication, BlockSync] {
-            network_send_times.insert(
-                key,
-                register(
-                    Histogram::with_opts(HistogramOpts {
-                        common_opts: Opts {
-                            namespace: "gossip_network".to_string(),
-                            subsystem: protocol_name(key),
-                            name: "send_duration".to_string(),
-                            help: "How long did it take for substrate to send a message."
-                                .to_string(),
-                            const_labels: Default::default(),
-                            variable_labels: Default::default(),
-                        },
-                        buckets: exponential_buckets(0.001, 1.26, 30)?,
-                    })?,
-                    registry,
-                )?,
-            );
-        }
-
         Ok(Self {
             prev,
             gauges,
@@ -95,7 +66,6 @@ impl<H: Key> Inner<H> {
                     )
                 })
                 .collect(),
-            network_send_times,
         })
     }
 
@@ -141,19 +111,6 @@ impl<H: Key> Inner<H> {
             }
         }
     }
-
-    fn start_sending_in(&self, protocol: Protocol) -> HistogramTimer {
-        self.network_send_times[&protocol].start_timer()
-    }
-}
-
-fn protocol_name(protocol: Protocol) -> String {
-    use Protocol::*;
-    match protocol {
-        Authentication => "authentication",
-        BlockSync => "block_sync",
-    }
-    .to_string()
 }
 
 #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
@@ -189,12 +146,6 @@ impl<H: Key> Metrics<H> {
                 .report_block(hash, checkpoint_time, checkpoint_type);
         }
     }
-
-    pub fn start_sending_in(&self, protocol: Protocol) -> Option<HistogramTimer> {
-        self.inner
-            .as_ref()
-            .map(|inner| inner.lock().start_sending_in(protocol))
-    }
 }
 
 #[cfg(test)]

diff --git a/finality-aleph/src/network/gossip/metrics.rs b/finality-aleph/src/network/gossip/metrics.rs
@@ -0,0 +1,71 @@
+use std::collections::HashMap;
+
+use substrate_prometheus_endpoint::{
+    exponential_buckets, prometheus::HistogramTimer, register, Histogram, HistogramOpts, Opts,
+    PrometheusError, Registry,
+};
+
+use crate::Protocol;
+
+fn protocol_name(protocol: Protocol) -> String {
+    use Protocol::*;
+    match protocol {
+        Authentication => "authentication",
+        BlockSync => "block_sync",
+    }
+    .to_string()
+}
+
+#[derive(Clone)]
+pub enum Metrics {
+    Prometheus {
+        send_times: HashMap<Protocol, Histogram>,
+    },
+    Noop,
+}
+
+impl Metrics {
+    pub fn new(registry: Option<Registry>) -> Result<Self, PrometheusError> {
+        use Protocol::*;
+        let registry = match registry {
+            Some(registry) => registry,
+            None => return Ok(Metrics::Noop),
+        };
+
+        let mut send_times = HashMap::new();
+        for protocol in [Authentication, BlockSync] {
+            send_times.insert(
+                protocol,
+                register(
+                    Histogram::with_opts(HistogramOpts {
+                        common_opts: Opts {
+                            namespace: "gossip_network".to_string(),
+                            subsystem: protocol_name(protocol),
+                            name: "send_duration".to_string(),
+                            help: "How long did it take for substrate to send a message."
+                                .to_string(),
+                            const_labels: Default::default(),
+                            variable_labels: Default::default(),
+                        },
+                        buckets: exponential_buckets(0.001, 1.26, 30)?,
+                    })?,
+                    &registry,
+                )?,
+            );
+        }
+        Ok(Metrics::Prometheus { send_times })
+    }
+
+    pub fn noop() -> Self {
+        Metrics::Noop
+    }
+
+    pub fn start_sending_in(&self, protocol: Protocol) -> Option<HistogramTimer> {
+        match self {
+            Metrics::Prometheus { send_times } => send_times
+                .get(&protocol)
+                .map(|histogram| histogram.start_timer()),
+            Metrics::Noop => None,
+        }
+    }
+}
diff --git a/finality-aleph/src/network/gossip/mod.rs b/finality-aleph/src/network/gossip/mod.rs
@@ -9,6 +9,7 @@ use bytes::Bytes;
 
 use crate::network::Data;
 
+mod metrics;
 #[cfg(test)]
 pub mod mock;
 mod service;