[bridge] add metrics to SuiClient and Monitor (#19352)

## Description 

as title, boring PR

## Test plan 

existing tests

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:

crates/sui-bridge-cli/src/main.rs

@@ -15,6 +15,7 @@ use std::time::Duration;
 use sui_bridge::client::bridge_authority_aggregator::BridgeAuthorityAggregator;
 use sui_bridge::crypto::{BridgeAuthorityPublicKey, BridgeAuthorityPublicKeyBytes};
 use sui_bridge::eth_transaction_builder::build_eth_transaction;
+use sui_bridge::metrics::BridgeMetrics;
 use sui_bridge::sui_client::SuiClient;
 use sui_bridge::sui_transaction_builder::build_sui_transaction;
 use sui_bridge::types::BridgeActionType;
@@ -80,7 +81,9 @@ async fn main() -> anyhow::Result<()> {
             println!("Chain ID: {:?}", chain_id);
             let config = BridgeCliConfig::load(config_path).expect("Couldn't load BridgeCliConfig");
             let config = LoadedBridgeCliConfig::load(config).await?;
-            let sui_bridge_client = SuiClient::<SuiSdkClient>::new(&config.sui_rpc_url).await?;
+            let metrics = Arc::new(BridgeMetrics::new_for_testing());
+            let sui_bridge_client =
+                SuiClient::<SuiSdkClient>::new(&config.sui_rpc_url, metrics).await?;
 
             let (sui_key, sui_address, gas_object_ref) = config
                 .get_sui_account_info()
@@ -273,7 +276,8 @@ async fn main() -> anyhow::Result<()> {
         }
 
         BridgeCommand::ViewBridgeRegistration { sui_rpc_url } => {
-            let sui_bridge_client = SuiClient::<SuiSdkClient>::new(&sui_rpc_url).await?;
+            let metrics = Arc::new(BridgeMetrics::new_for_testing());
+            let sui_bridge_client = SuiClient::<SuiSdkClient>::new(&sui_rpc_url, metrics).await?;
             let bridge_summary = sui_bridge_client
                 .get_bridge_summary()
                 .await
@@ -358,7 +362,8 @@ async fn main() -> anyhow::Result<()> {
             hex,
             ping,
         } => {
-            let sui_bridge_client = SuiClient::<SuiSdkClient>::new(&sui_rpc_url).await?;
+            let metrics = Arc::new(BridgeMetrics::new_for_testing());
+            let sui_bridge_client = SuiClient::<SuiSdkClient>::new(&sui_rpc_url, metrics).await?;
             let bridge_summary = sui_bridge_client
                 .get_bridge_summary()
                 .await
@@ -504,7 +509,9 @@ async fn main() -> anyhow::Result<()> {
         BridgeCommand::Client { config_path, cmd } => {
             let config = BridgeCliConfig::load(config_path).expect("Couldn't load BridgeCliConfig");
             let config = LoadedBridgeCliConfig::load(config).await?;
-            let sui_bridge_client = SuiClient::<SuiSdkClient>::new(&config.sui_rpc_url).await?;
+            let metrics = Arc::new(BridgeMetrics::new_for_testing());
+            let sui_bridge_client =
+                SuiClient::<SuiSdkClient>::new(&config.sui_rpc_url, metrics).await?;
             cmd.handle(&config, sui_bridge_client).await?;
             return Ok(());
         }

crates/sui-bridge/src/config.rs

@@ -158,7 +158,8 @@ impl BridgeNodeConfig {
 
         // we do this check here instead of `prepare_for_sui` below because
         // that is only called when `run_client` is true.
-        let sui_client = Arc::new(SuiClient::<SuiSdkClient>::new(&self.sui.sui_rpc_url).await?);
+        let sui_client =
+            Arc::new(SuiClient::<SuiSdkClient>::new(&self.sui.sui_rpc_url, metrics.clone()).await?);
         let bridge_committee = sui_client
             .get_bridge_committee()
             .await

crates/sui-bridge/src/e2e_tests/test_utils.rs

@@ -7,6 +7,7 @@ use crate::config::default_ed25519_key_pair;
 use crate::crypto::BridgeAuthorityKeyPair;
 use crate::crypto::BridgeAuthorityPublicKeyBytes;
 use crate::events::*;
+use crate::metrics::BridgeMetrics;
 use crate::server::BridgeNodePublicMetadata;
 use crate::types::BridgeAction;
 use crate::utils::get_eth_signer_client;
@@ -152,6 +153,7 @@ impl BridgeTestClusterBuilder {
     pub async fn build(self) -> BridgeTestCluster {
         init_all_struct_tags();
         std::env::set_var("__TEST_ONLY_CONSENSUS_USE_LONG_MIN_ROUND_DELAY", "1");
+        let metrics = Arc::new(BridgeMetrics::new_for_testing());
         let mut bridge_keys = vec![];
         let mut bridge_keys_copy = vec![];
         for _ in 0..self.num_validators {
@@ -177,7 +179,7 @@ impl BridgeTestClusterBuilder {
                     .await,
             );
         }
-        let bridge_client = SuiBridgeClient::new(&test_cluster.fullnode_handle.rpc_url)
+        let bridge_client = SuiBridgeClient::new(&test_cluster.fullnode_handle.rpc_url, metrics)
             .await
             .unwrap();
         info!(
@@ -836,6 +838,9 @@ impl TempDir {
 
 impl Drop for TempDir {
     fn drop(&mut self) {
-        fs::remove_dir_all(&self.path).unwrap();
+        // Use eprintln! here in case logging is not initialized
+        if let Err(e) = fs::remove_dir_all(&self.path) {
+            eprintln!("Failed to remove temp dir: {:?}", e);
+        }
     }
 }

crates/sui-bridge/src/metrics.rs

@@ -101,6 +101,9 @@ pub struct BridgeMetrics {
     pub(crate) eth_rpc_queries_latency: HistogramVec,
 
     pub(crate) gas_coin_balance: IntGauge,
+
+    pub(crate) sui_rpc_errors: IntCounterVec,
+    pub(crate) observed_governance_actions: IntCounterVec,
 }
 
 impl BridgeMetrics {
@@ -283,6 +286,20 @@ impl BridgeMetrics {
                 registry,
             )
             .unwrap(),
+            sui_rpc_errors: register_int_counter_vec_with_registry!(
+                "bridge_sui_rpc_errors",
+                "Total number of errors from sui RPC, by RPC method",
+                &["method"],
+                registry,
+            )
+            .unwrap(),
+            observed_governance_actions: register_int_counter_vec_with_registry!(
+                "bridge_observed_governance_actions",
+                "Total number of observed governance actions",
+                &["action_type", "chain_id"],
+                registry,
+            )
+            .unwrap(),
         }
     }
 

crates/sui-bridge/src/monitor.rs

@@ -7,6 +7,7 @@ use crate::client::bridge_authority_aggregator::BridgeAuthorityAggregator;
 use crate::crypto::BridgeAuthorityPublicKeyBytes;
 use crate::events::{BlocklistValidatorEvent, CommitteeMemberUrlUpdateEvent};
 use crate::events::{EmergencyOpEvent, SuiBridgeEvent};
+use crate::metrics::BridgeMetrics;
 use crate::retry_with_max_elapsed_time;
 use crate::sui_client::{SuiClient, SuiClientInner};
 use crate::types::{BridgeCommittee, IsBridgePaused};
@@ -25,6 +26,7 @@ pub struct BridgeMonitor<C> {
     bridge_auth_agg: Arc<ArcSwap<BridgeAuthorityAggregator>>,
     bridge_paused_watch_tx: tokio::sync::watch::Sender<IsBridgePaused>,
     sui_token_type_tags: Arc<ArcSwap<HashMap<u8, TypeTag>>>,
+    bridge_metrics: Arc<BridgeMetrics>,
 }
 
 impl<C> BridgeMonitor<C>
@@ -37,13 +39,15 @@ where
         bridge_auth_agg: Arc<ArcSwap<BridgeAuthorityAggregator>>,
         bridge_paused_watch_tx: tokio::sync::watch::Sender<IsBridgePaused>,
         sui_token_type_tags: Arc<ArcSwap<HashMap<u8, TypeTag>>>,
+        bridge_metrics: Arc<BridgeMetrics>,
     ) -> Self {
         Self {
             sui_client,
             monitor_rx,
             bridge_auth_agg,
             bridge_paused_watch_tx,
             sui_token_type_tags,
+            bridge_metrics,
         }
     }
 
@@ -55,6 +59,7 @@ where
             bridge_auth_agg,
             bridge_paused_watch_tx,
             sui_token_type_tags,
+            bridge_metrics,
         } = self;
         let mut latest_token_config = (*sui_token_type_tags.load().clone()).clone();
 
@@ -66,11 +71,15 @@ where
                 SuiBridgeEvent::TokenTransferAlreadyApproved(_) => (),
                 SuiBridgeEvent::TokenTransferAlreadyClaimed(_) => (),
                 SuiBridgeEvent::TokenTransferLimitExceed(_) => {
-                    // TODO
+                    // TODO do we want to do anything here?
                 }
 
                 SuiBridgeEvent::EmergencyOpEvent(event) => {
                     info!("Received EmergencyOpEvent: {:?}", event);
+                    bridge_metrics
+                        .observed_governance_actions
+                        .with_label_values(&["emergency_op", "sui"])
+                        .inc();
                     let is_paused = get_latest_bridge_pause_status_with_emergency_event(
                         sui_client.clone(),
                         event,
@@ -101,6 +110,10 @@ where
 
                 SuiBridgeEvent::BlocklistValidatorEvent(event) => {
                     info!("Received BlocklistValidatorEvent: {:?}", event);
+                    bridge_metrics
+                        .observed_governance_actions
+                        .with_label_values(&["blocklist_validator", "sui"])
+                        .inc();
                     let new_committee = get_latest_bridge_committee_with_blocklist_event(
                         sui_client.clone(),
                         event,
@@ -116,6 +129,11 @@ where
                 SuiBridgeEvent::TokenRegistrationEvent(_) => (),
 
                 SuiBridgeEvent::NewTokenEvent(event) => {
+                    info!("Received NewTokenEvent: {:?}", event);
+                    bridge_metrics
+                        .observed_governance_actions
+                        .with_label_values(&["new_token", "sui"])
+                        .inc();
                     if let std::collections::hash_map::Entry::Vacant(entry) =
                         // We only add new tokens but not remove so it's ok to just insert
                         latest_token_config.entry(event.token_id)
@@ -128,7 +146,13 @@ where
                     }
                 }
 
-                SuiBridgeEvent::UpdateTokenPriceEvent(_) => (),
+                SuiBridgeEvent::UpdateTokenPriceEvent(event) => {
+                    info!("Received UpdateTokenPriceEvent: {:?}", event);
+                    bridge_metrics
+                        .observed_governance_actions
+                        .with_label_values(&["update_token_price", "sui"])
+                        .inc();
+                }
             }
         }
 
@@ -725,6 +749,7 @@ mod tests {
             bridge_pause_tx,
             _bridge_pause_rx,
             mut authorities,
+            bridge_metrics,
         ) = setup();
         let old_committee = BridgeCommittee::new(authorities.clone()).unwrap();
         let agg = Arc::new(ArcSwap::new(Arc::new(BridgeAuthorityAggregator::new(
@@ -738,6 +763,7 @@ mod tests {
                 agg.clone(),
                 bridge_pause_tx,
                 sui_token_type_tags,
+                bridge_metrics,
             )
             .run(),
         );
@@ -779,6 +805,7 @@ mod tests {
             bridge_pause_tx,
             _bridge_pause_rx,
             mut authorities,
+            bridge_metrics,
         ) = setup();
         let old_committee = BridgeCommittee::new(authorities.clone()).unwrap();
         let agg = Arc::new(ArcSwap::new(Arc::new(BridgeAuthorityAggregator::new(
@@ -792,6 +819,7 @@ mod tests {
                 agg.clone(),
                 bridge_pause_tx,
                 sui_token_type_tags,
+                bridge_metrics,
             )
             .run(),
         );
@@ -831,6 +859,7 @@ mod tests {
             bridge_pause_tx,
             bridge_pause_rx,
             authorities,
+            bridge_metrics,
         ) = setup();
         let event = EmergencyOpEvent {
             frozen: !*bridge_pause_tx.borrow(), // toggle the bridge pause status
@@ -847,6 +876,7 @@ mod tests {
                 agg.clone(),
                 bridge_pause_tx,
                 sui_token_type_tags,
+                bridge_metrics,
             )
             .run(),
         );
@@ -872,6 +902,7 @@ mod tests {
             bridge_pause_tx,
             _bridge_pause_rx,
             authorities,
+            bridge_metrics,
         ) = setup();
         let event = NewTokenEvent {
             token_id: 255,
@@ -893,6 +924,7 @@ mod tests {
                 agg.clone(),
                 bridge_pause_tx,
                 sui_token_type_tags_clone,
+                bridge_metrics,
             )
             .run(),
         );
@@ -920,11 +952,13 @@ mod tests {
         tokio::sync::watch::Sender<IsBridgePaused>,
         tokio::sync::watch::Receiver<IsBridgePaused>,
         Vec<BridgeAuthority>,
+        Arc<BridgeMetrics>,
     ) {
         telemetry_subscribers::init_for_testing();
         let registry = Registry::new();
         mysten_metrics::init_metrics(&registry);
         init_all_struct_tags();
+        let bridge_metrics = Arc::new(BridgeMetrics::new_for_testing());
 
         let sui_client_mock = SuiMockClient::default();
         let sui_client = Arc::new(SuiClient::new_for_testing(sui_client_mock.clone()));
@@ -950,6 +984,7 @@ mod tests {
             bridge_pause_tx,
             bridge_pause_rx,
             authorities,
+            bridge_metrics,
         )
     }
 }

crates/sui-bridge/src/node.rs

@@ -146,6 +146,7 @@ async fn start_client_components(
         bridge_auth_agg.clone(),
         bridge_pause_tx,
         sui_token_type_tags,
+        metrics.clone(),
     );
     all_handles.push(spawn_logged_monitored_task!(monitor.run()));