Add discovery metric for peers with externally configured addresses (M…

…ystenLabs#13432)

crates/sui-network/src/discovery/builder.rs

@@ -1,7 +1,9 @@
 // Copyright (c) Mysten Labs, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use super::{server::Server, Discovery, DiscoveryEventLoop, DiscoveryServer, State};
+use super::{
+    metrics::Metrics, server::Server, Discovery, DiscoveryEventLoop, DiscoveryServer, State,
+};
 use crate::discovery::TrustedPeerChangeEvent;
 use anemo::codegen::InboundRequestLayer;
 use anemo_tower::rate_limit;
@@ -19,6 +21,7 @@ use tokio::{
 /// Discovery Service Builder.
 pub struct Builder {
     config: Option<P2pConfig>,
+    metrics: Option<Metrics>,
     trusted_peer_change_rx: watch::Receiver<TrustedPeerChangeEvent>,
 }
 
@@ -27,6 +30,7 @@ impl Builder {
     pub fn new(trusted_peer_change_rx: watch::Receiver<TrustedPeerChangeEvent>) -> Self {
         Self {
             config: None,
+            metrics: None,
             trusted_peer_change_rx,
         }
     }
@@ -36,6 +40,11 @@ impl Builder {
         self
     }
 
+    pub fn with_metrics(mut self, registry: &prometheus::Registry) -> Self {
+        self.metrics = Some(Metrics::enabled(registry));
+        self
+    }
+
     pub fn build(self) -> (UnstartedDiscovery, DiscoveryServer<impl Discovery>) {
         let discovery_config = self
             .config
@@ -60,9 +69,11 @@ impl Builder {
     pub(super) fn build_internal(self) -> (UnstartedDiscovery, Server) {
         let Builder {
             config,
+            metrics,
             trusted_peer_change_rx,
         } = self;
         let config = config.unwrap();
+        let metrics = metrics.unwrap_or_else(Metrics::disabled);
         let (sender, receiver) = oneshot::channel();
 
         let handle = Handle {
@@ -88,6 +99,7 @@ impl Builder {
                 shutdown_handle: receiver,
                 state,
                 trusted_peer_change_rx,
+                metrics,
             },
             server,
         )
@@ -101,6 +113,7 @@ pub struct UnstartedDiscovery {
     pub(super) shutdown_handle: oneshot::Receiver<()>,
     pub(super) state: Arc<RwLock<State>>,
     pub(super) trusted_peer_change_rx: watch::Receiver<TrustedPeerChangeEvent>,
+    pub(super) metrics: Metrics,
 }
 
 impl UnstartedDiscovery {
@@ -111,6 +124,7 @@ impl UnstartedDiscovery {
             shutdown_handle,
             state,
             trusted_peer_change_rx,
+            metrics,
         } = self;
 
         let discovery_config = config.discovery.clone().unwrap_or_default();
@@ -138,6 +152,7 @@ impl UnstartedDiscovery {
                 shutdown_handle,
                 state,
                 trusted_peer_change_rx,
+                metrics,
             },
             handle,
         )

crates/sui-network/src/discovery/metrics.rs

@@ -0,0 +1,55 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+use prometheus::{register_int_gauge_with_registry, IntGauge, Registry};
+use std::sync::Arc;
+use tap::Pipe;
+
+#[derive(Clone)]
+pub(super) struct Metrics(Option<Arc<Inner>>);
+
+impl std::fmt::Debug for Metrics {
+    fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
+        fmt.debug_struct("Metrics").finish()
+    }
+}
+
+impl Metrics {
+    pub fn enabled(registry: &Registry) -> Self {
+        Metrics(Some(Inner::new(registry)))
+    }
+
+    pub fn disabled() -> Self {
+        Metrics(None)
+    }
+
+    pub fn inc_num_peers_with_external_address(&self) {
+        if let Some(inner) = &self.0 {
+            inner.num_peers_with_external_address.inc();
+        }
+    }
+
+    pub fn dec_num_peers_with_external_address(&self) {
+        if let Some(inner) = &self.0 {
+            inner.num_peers_with_external_address.dec();
+        }
+    }
+}
+
+struct Inner {
+    num_peers_with_external_address: IntGauge,
+}
+
+impl Inner {
+    pub fn new(registry: &Registry) -> Arc<Self> {
+        Self {
+            num_peers_with_external_address: register_int_gauge_with_registry!(
+                "num_peers_with_external_address",
+                "Number of peers with an external address configured for discovery",
+                registry
+            )
+            .unwrap(),
+        }
+        .pipe(Arc::new)
+    }
+}

crates/sui-network/src/discovery/mod.rs

@@ -28,6 +28,7 @@ mod generated {
     include!(concat!(env!("OUT_DIR"), "/sui.Discovery.rs"));
 }
 mod builder;
+mod metrics;
 mod server;
 #[cfg(test)]
 mod tests;
@@ -39,6 +40,8 @@ pub use generated::{
 };
 pub use server::GetKnownPeersResponse;
 
+use self::metrics::Metrics;
+
 /// The internal discovery state shared between the main event loop and the request handler
 struct State {
     our_info: Option<NodeInfo>,
@@ -80,6 +83,7 @@ struct DiscoveryEventLoop {
     shutdown_handle: oneshot::Receiver<()>,
     state: Arc<RwLock<State>>,
     trusted_peer_change_rx: watch::Receiver<TrustedPeerChangeEvent>,
+    metrics: Metrics,
 }
 
 impl DiscoveryEventLoop {
@@ -219,6 +223,7 @@ impl DiscoveryEventLoop {
                     self.tasks.spawn(query_peer_for_their_known_peers(
                         peer,
                         self.state.clone(),
+                        self.metrics.clone(),
                         self.allowlisted_peers.clone(),
                     ));
                 }
@@ -245,6 +250,7 @@ impl DiscoveryEventLoop {
                 self.network.clone(),
                 self.discovery_config.clone(),
                 self.state.clone(),
+                self.metrics.clone(),
                 self.allowlisted_peers.clone(),
             ));
 
@@ -370,6 +376,7 @@ async fn try_to_connect_to_seed_peers(
 async fn query_peer_for_their_known_peers(
     peer: Peer,
     state: Arc<RwLock<State>>,
+    metrics: Metrics,
     allowlisted_peers: Arc<HashMap<PeerId, Option<Multiaddr>>>,
 ) {
     let mut client = DiscoveryClient::new(peer);
@@ -392,14 +399,15 @@ async fn query_peer_for_their_known_peers(
             },
         )
     {
-        update_known_peers(state, found_peers, allowlisted_peers);
+        update_known_peers(state, metrics, found_peers, allowlisted_peers);
     }
 }
 
 async fn query_connected_peers_for_their_known_peers(
     network: Network,
     config: Arc<DiscoveryConfig>,
     state: Arc<RwLock<State>>,
+    metrics: Metrics,
     allowlisted_peers: Arc<HashMap<PeerId, Option<Multiaddr>>>,
 ) {
     use rand::seq::IteratorRandom;
@@ -437,11 +445,12 @@ async fn query_connected_peers_for_their_known_peers(
         .collect::<Vec<_>>()
         .await;
 
-    update_known_peers(state, found_peers, allowlisted_peers);
+    update_known_peers(state, metrics, found_peers, allowlisted_peers);
 }
 
 fn update_known_peers(
     state: Arc<RwLock<State>>,
+    metrics: Metrics,
     found_peers: Vec<NodeInfo>,
     allowlisted_peers: Arc<HashMap<PeerId, Option<Multiaddr>>>,
 ) {
@@ -472,10 +481,19 @@ fn update_known_peers(
         match known_peers.entry(peer.peer_id) {
             Entry::Occupied(mut o) => {
                 if peer.timestamp_ms > o.get().timestamp_ms {
+                    if o.get().addresses.is_empty() && !peer.addresses.is_empty() {
+                        metrics.inc_num_peers_with_external_address();
+                    }
+                    if !o.get().addresses.is_empty() && peer.addresses.is_empty() {
+                        metrics.dec_num_peers_with_external_address();
+                    }
                     o.insert(peer);
                 }
             }
             Entry::Vacant(v) => {
+                if !peer.addresses.is_empty() {
+                    metrics.inc_num_peers_with_external_address();
+                }
                 v.insert(peer);
             }
         }