Merge pull request #2177 from EspressoSystems/sishan/metrics_block_he…

…ight [Metrics] Passing in metrics for metrics initialization
EspressoSystems · Dec 6, 2023 · 75e8a9a · 75e8a9a
2 parents f1eec4f + ba96cf9
commit 75e8a9a
Show file tree

Hide file tree

Showing 8 changed files with 24 additions and 35 deletions.
diff --git a/crates/hotshot/examples/infra/mod.rs b/crates/hotshot/examples/infra/mod.rs
@@ -252,7 +252,7 @@ match node_type {
     let node_config = config_builder.build().unwrap();
 
     Libp2pNetwork::new(
-        NetworkingMetricsValue::new(),
+        NetworkingMetricsValue::default(),
         node_config,
         pub_key.clone(),
         Arc::new(RwLock::new(
@@ -360,7 +360,7 @@ pub trait RunDA<
             memberships,
             networks_bundle,
             initializer,
-            ConsensusMetricsValue::new(),
+            ConsensusMetricsValue::default(),
         )
         .await
         .expect("Could not init hotshot")

diff --git a/crates/hotshot/src/traits/networking.rs b/crates/hotshot/src/traits/networking.rs
@@ -15,7 +15,7 @@ use std::{
 };
 
 use custom_debug::Debug;
-use hotshot_types::traits::metrics::{Counter, Gauge, Histogram, Label, Metrics};
+use hotshot_types::traits::metrics::{Counter, Gauge, Histogram, Label, Metrics, NoMetrics};
 pub use hotshot_types::traits::network::{
     ChannelSendSnafu, CouldNotDeliverSnafu, FailedToDeserializeSnafu, FailedToSerializeSnafu,
     NetworkError, NetworkReliability, NoSuchNodeSnafu, ShutDownSnafu,
@@ -25,8 +25,6 @@ pub use hotshot_types::traits::network::{
 #[derive(Clone, Debug)]
 pub struct NetworkingMetricsValue {
     #[allow(dead_code)]
-    /// The values that are being tracked
-    pub values: Arc<Mutex<InnerNetworkingMetrics>>,
     /// A [`Gauge`] which tracks how many peers are connected
     pub connected_peers: Box<dyn Gauge>,
     /// A [`Counter`] which tracks how many messages have been received directly
@@ -163,14 +161,8 @@ impl Label for NetworkingMetrics {
 impl NetworkingMetricsValue {
     /// Create a new instance of this [`NetworkingMetricsValue`] struct, setting all the counters and gauges
     #[must_use]
-    pub fn new() -> Self {
-        let values = Arc::default();
-        let metrics: Box<dyn Metrics> = Box::new(NetworkingMetrics {
-            prefix: String::new(),
-            values: Arc::clone(&values),
-        });
+    pub fn new(metrics: &dyn Metrics) -> Self {
         Self {
-            values,
             connected_peers: metrics.create_gauge(String::from("connected_peers"), None),
             incoming_direct_message_count: metrics
                 .create_counter(String::from("incoming_direct_message_count"), None),
@@ -188,6 +180,6 @@ impl NetworkingMetricsValue {
 
 impl Default for NetworkingMetricsValue {
     fn default() -> Self {
-        Self::new()
+        Self::new(&*NoMetrics::boxed())
     }
 }
diff --git a/crates/hotshot/src/traits/networking/libp2p_network.rs b/crates/hotshot/src/traits/networking/libp2p_network.rs
@@ -228,7 +228,7 @@ where
                 let da = da_keys.clone();
                 async_block_on(async move {
                     match Libp2pNetwork::new(
-                        NetworkingMetricsValue::new(),
+                        NetworkingMetricsValue::default(),
                         config,
                         pubkey.clone(),
                         bootstrap_addrs_ref,

diff --git a/crates/hotshot/src/traits/networking/memory_network.rs b/crates/hotshot/src/traits/networking/memory_network.rs
@@ -255,7 +255,12 @@ impl<TYPES: NodeType> TestableNetworkingImplementation<TYPES>
         Box::new(move |node_id| {
             let privkey = TYPES::SignatureKey::generated_from_seed_indexed([0u8; 32], node_id).1;
             let pubkey = TYPES::SignatureKey::from_private(&privkey);
-            MemoryNetwork::new(pubkey, NetworkingMetricsValue::new(), master.clone(), None)
+            MemoryNetwork::new(
+                pubkey,
+                NetworkingMetricsValue::default(),
+                master.clone(),
+                None,
+            )
         })
     }
 

diff --git a/crates/testing/src/task_helpers.rs b/crates/testing/src/task_helpers.rs
@@ -96,7 +96,7 @@ pub async fn build_system_handle(
         memberships,
         networks_bundle,
         initializer,
-        ConsensusMetricsValue::new(),
+        ConsensusMetricsValue::default(),
     )
     .await
     .expect("Could not init hotshot")

diff --git a/crates/testing/src/test_runner.rs b/crates/testing/src/test_runner.rs
@@ -268,7 +268,7 @@ where
             memberships,
             network_bundle,
             initializer,
-            ConsensusMetricsValue::new(),
+            ConsensusMetricsValue::default(),
         )
         .await
         .expect("Could not init hotshot")

diff --git a/crates/testing/tests/memory_network.rs b/crates/testing/tests/memory_network.rs
@@ -167,15 +167,15 @@ async fn memory_network_direct_queue() {
     let pub_key_1 = get_pubkey();
     let network1 = MemoryNetwork::new(
         pub_key_1,
-        NetworkingMetricsValue::new(),
+        NetworkingMetricsValue::default(),
         group.clone(),
         Option::None,
     );
 
     let pub_key_2 = get_pubkey();
     let network2 = MemoryNetwork::new(
         pub_key_2,
-        NetworkingMetricsValue::new(),
+        NetworkingMetricsValue::default(),
         group,
         Option::None,
     );
@@ -232,14 +232,14 @@ async fn memory_network_broadcast_queue() {
     let pub_key_1 = get_pubkey();
     let network1 = MemoryNetwork::new(
         pub_key_1,
-        NetworkingMetricsValue::new(),
+        NetworkingMetricsValue::default(),
         group.clone(),
         Option::None,
     );
     let pub_key_2 = get_pubkey();
     let network2 = MemoryNetwork::new(
         pub_key_2,
-        NetworkingMetricsValue::new(),
+        NetworkingMetricsValue::default(),
         group,
         Option::None,
     );
@@ -302,14 +302,14 @@ async fn memory_network_test_in_flight_message_count() {
     let pub_key_1 = get_pubkey();
     let network1 = MemoryNetwork::new(
         pub_key_1,
-        NetworkingMetricsValue::new(),
+        NetworkingMetricsValue::default(),
         group.clone(),
         Option::None,
     );
     let pub_key_2 = get_pubkey();
     let network2 = MemoryNetwork::new(
         pub_key_2,
-        NetworkingMetricsValue::new(),
+        NetworkingMetricsValue::default(),
         group,
         Option::None,
     );

diff --git a/crates/types/src/consensus.rs b/crates/types/src/consensus.rs
@@ -11,7 +11,7 @@ use crate::{
     error::HotShotError,
     simple_certificate::QuorumCertificate,
     traits::{
-        metrics::{Counter, Gauge, Histogram, Label, Metrics},
+        metrics::{Counter, Gauge, Histogram, Label, Metrics, NoMetrics},
         node_implementation::NodeType,
     },
     utils::Terminator,
@@ -66,9 +66,7 @@ pub struct Consensus<TYPES: NodeType> {
 /// Contains several `ConsensusMetrics` that we're interested in from the consensus interfaces
 #[derive(Clone, Debug)]
 pub struct ConsensusMetricsValue {
-    /// The values that are being tracked
-    pub values: Arc<Mutex<InnerConsensusMetrics>>,
-    /// The number of last synced synced block height
+    /// The number of last synced block height
     pub last_synced_block_height: Box<dyn Gauge>,
     /// The number of last decided view
     pub last_decided_view: Box<dyn Gauge>,
@@ -205,14 +203,8 @@ impl Label for ConsensusMetrics {
 impl ConsensusMetricsValue {
     /// Create a new instance of this [`ConsensusMetricsValue`] struct, setting all the counters and gauges
     #[must_use]
-    pub fn new() -> Self {
-        let values = Arc::default();
-        let metrics: Box<dyn Metrics> = Box::new(ConsensusMetrics {
-            prefix: String::new(),
-            values: Arc::clone(&values),
-        });
+    pub fn new(metrics: &dyn Metrics) -> Self {
         Self {
-            values,
             last_synced_block_height: metrics
                 .create_gauge(String::from("last_synced_block_height"), None),
             last_decided_view: metrics.create_gauge(String::from("last_decided_view"), None),
@@ -233,7 +225,7 @@ impl ConsensusMetricsValue {
 
 impl Default for ConsensusMetricsValue {
     fn default() -> Self {
-        Self::new()
+        Self::new(&*NoMetrics::boxed())
     }
 }