From c860048db19c25256cf37aa7c69e104af6c5a5aa Mon Sep 17 00:00:00 2001 From: Damilare Date: Mon, 12 Dec 2022 18:17:23 +0100 Subject: [PATCH 01/14] few beefy metrics --- client/beefy/src/metrics.rs | 34 +++++++++++++++++++++++++++++++++- client/beefy/src/worker.rs | 3 +++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index 71e34e24c4fa0..5e8347e22a63f 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -19,7 +19,6 @@ //! BEEFY Prometheus metrics definition use prometheus::{register, Counter, Gauge, PrometheusError, Registry, U64}; - /// BEEFY metrics exposed through Prometheus pub(crate) struct Metrics { /// Current active validator set id @@ -34,6 +33,14 @@ pub(crate) struct Metrics { pub beefy_should_vote_on: Gauge, /// Number of sessions with lagging signed commitment on mandatory block pub beefy_lagging_sessions: Counter, + /// Number of validator's trying to vote with no session initialized + pub beefy_voting_with_no_session_initialized: Counter, + /// Number of times no Authority public key found in store + pub beefy_no_authority_found_in_store: Counter, + /// Number of Buffered votes + pub beefy_buffered_votes: Counter, + /// Number of times Buffered votes is full + pub beefy_buffered_votes_full: Counter, } impl Metrics { @@ -72,6 +79,31 @@ impl Metrics { )?, registry, )?, + beefy_voting_with_no_session_initialized: register( + Counter::new( + "substrate_voting_with_no_session_initialized", + "Number of validator's trying to vote with no session initialized", + )?, + registry, + )?, + beefy_no_authority_found_in_store: register( + Counter::new( + "substrate_beefy_no_authority_found_in_store", + "Number of times no Authority public key found in store", + )?, + registry, + )?, + beefy_buffered_votes: register( + Counter::new("substrate_beefy_buffered_votes", "Number of Buffered votes")?, + registry, + )?, + beefy_buffered_votes_full: register( + Counter::new( + "substrate_beefy_buffered_votes_full", + "Number of times Buffered votes is full", + )?, + registry, + )?, }) } } diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index bba3563a8f70e..496d72e1f1b1b 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -406,6 +406,7 @@ where if store.intersection(&active).count() == 0 { let msg = "no authority public key found in store".to_string(); debug!(target: "beefy", "🥩 for block {:?} {}", block, msg); + metric_inc!(self, beefy_no_authority_found_in_store); Err(Error::Keystore(msg)) } else { Ok(()) @@ -491,6 +492,7 @@ where false, )?, RoundAction::Enqueue => { + metric_inc!(self, beefy_buffered_votes); debug!(target: "beefy", "🥩 Buffer vote for round: {:?}.", block_num); if self.pending_votes.len() < MAX_BUFFERED_VOTE_ROUNDS { let votes_vec = self.pending_votes.entry(block_num).or_default(); @@ -498,6 +500,7 @@ where warn!(target: "beefy", "🥩 Buffer vote dropped for round: {:?}", block_num) } } else { + metric_inc!(self, beefy_buffered_votes_full); error!(target: "beefy", "🥩 Buffer justification dropped for round: {:?}.", block_num); } }, From 4cff8adff8ad730efb146a96f39dfdc1b2f8d178 Mon Sep 17 00:00:00 2001 From: Damilare Date: Mon, 12 Dec 2022 22:51:37 +0100 Subject: [PATCH 02/14] more beefy metrics --- client/beefy/src/metrics.rs | 27 +++++++++++++++++++++++++++ client/beefy/src/worker.rs | 3 +++ 2 files changed, 30 insertions(+) diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index 5e8347e22a63f..e616c68f16de0 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -41,6 +41,12 @@ pub(crate) struct Metrics { pub beefy_buffered_votes: Counter, /// Number of times Buffered votes is full pub beefy_buffered_votes_full: Counter, + /// Number of Buffered justifications + pub beefy_buffered_justifications: Counter, + /// Number of times Buffered justifications is full + pub beefy_buffered_justifications_full: Counter, + /// Trying to set Best Beefy block to old block + pub beefy_best_block_to_old_block: Gauge, } impl Metrics { @@ -104,6 +110,27 @@ impl Metrics { )?, registry, )?, + beefy_buffered_justifications: register( + Counter::new( + "substrate_beefy_buffered_justifications", + "Number of Buffered justifications", + )?, + registry, + )?, + beefy_buffered_justifications_full: register( + Counter::new( + "substrate_beefy_buffered_justifications_full", + "Number of times Buffered justifications is full", + )?, + registry, + )?, + beefy_best_block_to_old_block: register( + Gauge::new( + "substrate_beefy_best_block_to_old_block", + "Trying to set Best Beefy block to old block", + )?, + registry, + )?, }) } } diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 496d72e1f1b1b..659db30970da9 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -527,10 +527,12 @@ where self.finalize(justification)? }, RoundAction::Enqueue => { + metric_inc!(self, beefy_buffered_justifications); debug!(target: "beefy", "🥩 Buffer justification for round: {:?}.", block_num); if self.pending_justifications.len() < MAX_BUFFERED_JUSTIFICATIONS { self.pending_justifications.entry(block_num).or_insert(justification); } else { + metric_inc!(self, beefy_buffered_justifications_full); error!(target: "beefy", "🥩 Buffer justification dropped for round: {:?}.", block_num); } }, @@ -639,6 +641,7 @@ where .expect("forwards closure result; the closure always returns Ok; qed."); } else { debug!(target: "beefy", "🥩 Can't set best beefy to older: {}", block_num); + metric_set!(self, beefy_best_block_to_old_block, block_num); } Ok(()) } From f7e48e99dd4ee646129386c2faef3e05fa76591f Mon Sep 17 00:00:00 2001 From: Damilare Date: Wed, 14 Dec 2022 00:22:59 +0100 Subject: [PATCH 03/14] some beefy metrics --- client/beefy/src/import.rs | 10 +++++++++- client/beefy/src/lib.rs | 24 ++++++++++++++++++++++-- client/beefy/src/metrics.rs | 36 +++++++++++++++++++++++++++++++++++- client/beefy/src/worker.rs | 3 +++ 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/client/beefy/src/import.rs b/client/beefy/src/import.rs index 0ed50d0ec8c98..9543223bdea97 100644 --- a/client/beefy/src/import.rs +++ b/client/beefy/src/import.rs @@ -35,6 +35,8 @@ use sc_consensus::{BlockCheckParams, BlockImport, BlockImportParams, ImportResul use crate::{ communication::notification::BeefyVersionedFinalityProofSender, justification::{decode_and_verify_finality_proof, BeefyVersionedFinalityProof}, + metric_inc, metric_set, + metrics::Metrics, }; /// A block-import handler for BEEFY. @@ -48,6 +50,7 @@ pub struct BeefyBlockImport { runtime: Arc, inner: I, justification_sender: BeefyVersionedFinalityProofSender, + metrics: Option, } impl Clone for BeefyBlockImport { @@ -57,6 +60,7 @@ impl Clone for BeefyBlockImport BeefyBlockImport { runtime: Arc, inner: I, justification_sender: BeefyVersionedFinalityProofSender, + metrics: Option, ) -> BeefyBlockImport { - BeefyBlockImport { backend, runtime, inner, justification_sender } + BeefyBlockImport { backend, runtime, inner, justification_sender, metrics } } } @@ -143,12 +148,15 @@ where self.justification_sender .notify(|| Ok::<_, ()>(proof)) .expect("forwards closure result; the closure always returns Ok; qed."); + + metric_set!(self, beefy_good_justification_imports, number); } else { debug!( target: "beefy", "🥩 error decoding justification: {:?} for imported block {:?}", encoded, number, ); + metric_set!(self, beefy_bad_justification_imports, number); } }, _ => (), diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index a057a9fdc597d..a3dd0772e5284 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -131,6 +131,7 @@ pub fn beefy_block_import_and_links( wrapped_block_import: I, backend: Arc, runtime: Arc, + prometheus_registry: Option, ) -> (BeefyBlockImport, BeefyVoterLinks, BeefyRPCLinks) where B: Block, @@ -151,9 +152,28 @@ where let (to_voter_justif_sender, from_block_import_justif_stream) = BeefyVersionedFinalityProofStream::::channel(); + let metrics = + prometheus_registry.as_ref().map(metrics::Metrics::register).and_then( + |result| match result { + Ok(metrics) => { + debug!(target: "beefy", "🥩 Registered metrics"); + Some(metrics) + }, + Err(err) => { + debug!(target: "beefy", "🥩 Failed to register metrics: {:?}", err); + None + }, + }, + ); + // BlockImport - let import = - BeefyBlockImport::new(backend, runtime, wrapped_block_import, to_voter_justif_sender); + let import = BeefyBlockImport::new( + backend, + runtime, + wrapped_block_import, + to_voter_justif_sender, + metrics, + ); let voter_links = BeefyVoterLinks { from_block_import_justif_stream, to_rpc_justif_sender, diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index e616c68f16de0..8732860335f90 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -20,7 +20,8 @@ use prometheus::{register, Counter, Gauge, PrometheusError, Registry, U64}; /// BEEFY metrics exposed through Prometheus -pub(crate) struct Metrics { +#[derive(Clone)] +pub struct Metrics { /// Current active validator set id pub beefy_validator_set_id: Gauge, /// Total number of votes sent by this node @@ -47,6 +48,14 @@ pub(crate) struct Metrics { pub beefy_buffered_justifications_full: Counter, /// Trying to set Best Beefy block to old block pub beefy_best_block_to_old_block: Gauge, + /// Number of Successful handled votes + pub beefy_successful_handled_votes: Counter, + /// Number of Successful votes + pub beefy_successful_votes: Counter, + /// Number of Bad Justification imports + pub beefy_bad_justification_imports: Gauge, + /// Number of Good Justification imports + pub beefy_good_justification_imports: Gauge, } impl Metrics { @@ -131,6 +140,31 @@ impl Metrics { )?, registry, )?, + beefy_successful_handled_votes: register( + Counter::new( + "substrate_beefy_successful_handled_votes", + "Number of Successful handled votes", + )?, + registry, + )?, + beefy_successful_votes: register( + Counter::new("substrate_beefy_successful_votes", "Number of Successful votes")?, + registry, + )?, + beefy_bad_justification_imports: register( + Gauge::new( + "substrate_beefy_bad_justification_imports", + "Number of Bad Justification imports", + )?, + registry, + )?, + beefy_good_justification_imports: register( + Gauge::new( + "substrate_beefy_good_justification_imports", + "Number of Good Justification imports", + )?, + registry, + )?, }) } } diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 659db30970da9..9c964a1bdab5a 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -591,6 +591,7 @@ where } } } + metric_inc!(self, beefy_successful_handled_votes); Ok(()) } @@ -799,6 +800,8 @@ where self.gossip_engine.gossip_message(topic::(), encoded_message, false); + metric_inc!(self, beefy_successful_votes); + Ok(()) } From f0891f5b3059cf37c7da004d4bb24d303b09b5bc Mon Sep 17 00:00:00 2001 From: Damilare Date: Wed, 14 Dec 2022 18:22:37 +0100 Subject: [PATCH 04/14] some beefy metrics --- .../request_response/incoming_requests_handler.rs | 11 ++++++++--- client/beefy/src/metrics.rs | 14 +++++++++++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/client/beefy/src/communication/request_response/incoming_requests_handler.rs b/client/beefy/src/communication/request_response/incoming_requests_handler.rs index 9f02b7162b54c..c6e84d59f96e8 100644 --- a/client/beefy/src/communication/request_response/incoming_requests_handler.rs +++ b/client/beefy/src/communication/request_response/incoming_requests_handler.rs @@ -32,6 +32,7 @@ use std::{marker::PhantomData, sync::Arc}; use crate::communication::request_response::{ on_demand_justifications_protocol_config, Error, JustificationRequest, }; +use crate::{metrics::Metrics, metric_inc, metric_set}; /// A request coming in, including a sender for sending responses. #[derive(Debug)] @@ -89,7 +90,7 @@ impl IncomingRequest { /// /// Takes care of decoding and handling of invalid encoded requests. pub(crate) struct IncomingRequestReceiver { - raw: mpsc::Receiver, + raw: mpsc::Receiver } impl IncomingRequestReceiver { @@ -101,7 +102,7 @@ impl IncomingRequestReceiver { /// /// Any received request will be decoded, on decoding errors the provided reputation changes /// will be applied and an error will be reported. - pub async fn recv(&mut self, reputation_changes: F) -> Result, Error> + pub async fn recv(&mut self, reputation_changes: F,) -> Result, Error> where B: Block, F: FnOnce() -> Vec, @@ -120,6 +121,7 @@ pub struct BeefyJustifsRequestHandler { pub(crate) justif_protocol_name: ProtocolName, pub(crate) client: Arc, pub(crate) _block: PhantomData, + metrics: Option, } impl BeefyJustifsRequestHandler @@ -132,12 +134,13 @@ where genesis_hash: Hash, fork_id: Option<&str>, client: Arc, + metrics: Option ) -> (Self, RequestResponseConfig) { let (request_receiver, config) = on_demand_justifications_protocol_config(genesis_hash, fork_id); let justif_protocol_name = config.name.clone(); - (Self { request_receiver, justif_protocol_name, client, _block: PhantomData }, config) + (Self { request_receiver, justif_protocol_name, client, _block: PhantomData, metrics }, config) } /// Network request-response protocol name used by this handler. @@ -180,12 +183,14 @@ where let peer = request.peer; match self.handle_request(request) { Ok(()) => { + metric_inc!(self, beefy_successful_justification_respond_request); debug!( target: "beefy::sync", "🥩 Handled BEEFY justification request from {:?}.", peer ) }, Err(e) => { + metric_inc!(self, beefy_failed_justification_respond_request); // TODO (issue #12293): apply reputation changes here based on error type. debug!( target: "beefy::sync", diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index 8732860335f90..5bf629d08ca2f 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -20,7 +20,7 @@ use prometheus::{register, Counter, Gauge, PrometheusError, Registry, U64}; /// BEEFY metrics exposed through Prometheus -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct Metrics { /// Current active validator set id pub beefy_validator_set_id: Gauge, @@ -56,6 +56,10 @@ pub struct Metrics { pub beefy_bad_justification_imports: Gauge, /// Number of Good Justification imports pub beefy_good_justification_imports: Gauge, + /// Number of Successful Justification respond request + pub beefy_successful_justification_respond_request: Counter, + /// Number of Failed Justification respond request + pub beefy_failed_justification_respond_request: Counter, } impl Metrics { @@ -165,6 +169,14 @@ impl Metrics { )?, registry, )?, + beefy_successful_justification_respond_request: register( + Counter::new("substrate_beefy_successful_justification_respond_request", "Number of Successful Justification respond request")?, + registry, + )?, + beefy_failed_justification_respond_request: register( + Counter::new("substrate_beefy_failed_justification_respond_request", "Number of Failed Justification respond request")?, + registry, + )?, }) } } From f3357f910495ba1b2a5caa2c42352dd192bf3b84 Mon Sep 17 00:00:00 2001 From: Damilare Date: Wed, 4 Jan 2023 23:43:27 +0100 Subject: [PATCH 05/14] more metrics --- .../request_response/outgoing_requests_engine.rs | 5 +++++ client/beefy/src/lib.rs | 15 ++++++++------- client/beefy/src/metrics.rs | 6 ++++++ client/beefy/src/worker.rs | 1 + 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/client/beefy/src/communication/request_response/outgoing_requests_engine.rs b/client/beefy/src/communication/request_response/outgoing_requests_engine.rs index 00ee7610dd4f0..64142751b8833 100644 --- a/client/beefy/src/communication/request_response/outgoing_requests_engine.rs +++ b/client/beefy/src/communication/request_response/outgoing_requests_engine.rs @@ -36,6 +36,7 @@ use crate::{ justification::{decode_and_verify_finality_proof, BeefyVersionedFinalityProof}, KnownPeers, }; +use crate::{metrics::Metrics, metric_inc, metric_set}; /// Response type received from network. type Response = Result, RequestFailure>; @@ -61,6 +62,7 @@ pub struct OnDemandJustificationsEngine { peers_cache: VecDeque, state: State, + metrics: Option, } impl OnDemandJustificationsEngine { @@ -68,6 +70,7 @@ impl OnDemandJustificationsEngine { network: Arc, protocol_name: ProtocolName, live_peers: Arc>>, + metrics: Option ) -> Self { Self { network, @@ -75,6 +78,7 @@ impl OnDemandJustificationsEngine { live_peers, peers_cache: VecDeque::new(), state: State::Idle, + metrics } } @@ -132,6 +136,7 @@ impl OnDemandJustificationsEngine { if let Some(peer) = self.try_next_peer() { self.request_from_peer(peer, RequestInfo { block, active_set }); } else { + metric_inc!(self, beefy_on_demand_justification_no_peer_to_request_from); debug!(target: "beefy::sync", "🥩 no good peers to request justif #{:?} from", block); } } diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index a3dd0772e5284..95ca56a6e2248 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -261,13 +261,6 @@ where None, ); - // The `GossipValidator` adds and removes known peers based on valid votes and network events. - let on_demand_justifications = OnDemandJustificationsEngine::new( - network.clone(), - justifications_protocol_name, - known_peers, - ); - let metrics = prometheus_registry.as_ref().map(metrics::Metrics::register).and_then( |result| match result { @@ -282,6 +275,14 @@ where }, ); + // The `GossipValidator` adds and removes known peers based on valid votes and network events. + let on_demand_justifications = OnDemandJustificationsEngine::new( + network.clone(), + justifications_protocol_name, + known_peers, + metrics.clone() + ); + // Subscribe to finality notifications and justifications before waiting for runtime pallet and // reuse the streams, so we don't miss notifications while waiting for pallet to be available. let mut finality_notifications = client.finality_notification_stream().fuse(); diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index 5bf629d08ca2f..134f43157daa7 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -60,6 +60,8 @@ pub struct Metrics { pub beefy_successful_justification_respond_request: Counter, /// Number of Failed Justification respond request pub beefy_failed_justification_respond_request: Counter, + /// Number of On demand justification when there is no peer to request from + pub beefy_on_demand_justification_no_peer_to_request_from: Counter, } impl Metrics { @@ -177,6 +179,10 @@ impl Metrics { Counter::new("substrate_beefy_failed_justification_respond_request", "Number of Failed Justification respond request")?, registry, )?, + beefy_on_demand_justification_no_peer_to_request_from: register( + Counter::new("substrate_beefy_on_demand_justification_no_peer_to_request_from", "Number of on demand justification when there is no peer to request from")?, + registry, + )?, }) } } diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 9c964a1bdab5a..a698fd14b9385 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -1059,6 +1059,7 @@ pub(crate) mod tests { network.clone(), "/beefy/justifs/1".into(), known_peers, + metrics: None, ); let at = BlockId::number(Zero::zero()); let genesis_header = backend.blockchain().expect_header(at).unwrap(); From 0dbd02493dab38ebbb94a569da9c426ce551ac36 Mon Sep 17 00:00:00 2001 From: Damilare Date: Wed, 25 Jan 2023 00:03:42 +0100 Subject: [PATCH 06/14] other metrics --- .../incoming_requests_handler.rs | 20 +++++--- .../outgoing_requests_engine.rs | 11 ++-- client/beefy/src/import.rs | 2 +- client/beefy/src/lib.rs | 2 +- client/beefy/src/metrics.rs | 51 +++++++++++++++++-- client/beefy/src/worker.rs | 5 +- 6 files changed, 74 insertions(+), 17 deletions(-) diff --git a/client/beefy/src/communication/request_response/incoming_requests_handler.rs b/client/beefy/src/communication/request_response/incoming_requests_handler.rs index c6e84d59f96e8..847585ee07229 100644 --- a/client/beefy/src/communication/request_response/incoming_requests_handler.rs +++ b/client/beefy/src/communication/request_response/incoming_requests_handler.rs @@ -29,10 +29,13 @@ use sc_network_common::protocol::ProtocolName; use sp_runtime::traits::Block; use std::{marker::PhantomData, sync::Arc}; -use crate::communication::request_response::{ - on_demand_justifications_protocol_config, Error, JustificationRequest, +use crate::{ + communication::request_response::{ + on_demand_justifications_protocol_config, Error, JustificationRequest, + }, + metric_inc, + metrics::Metrics, }; -use crate::{metrics::Metrics, metric_inc, metric_set}; /// A request coming in, including a sender for sending responses. #[derive(Debug)] @@ -90,7 +93,7 @@ impl IncomingRequest { /// /// Takes care of decoding and handling of invalid encoded requests. pub(crate) struct IncomingRequestReceiver { - raw: mpsc::Receiver + raw: mpsc::Receiver, } impl IncomingRequestReceiver { @@ -102,7 +105,7 @@ impl IncomingRequestReceiver { /// /// Any received request will be decoded, on decoding errors the provided reputation changes /// will be applied and an error will be reported. - pub async fn recv(&mut self, reputation_changes: F,) -> Result, Error> + pub async fn recv(&mut self, reputation_changes: F) -> Result, Error> where B: Block, F: FnOnce() -> Vec, @@ -134,13 +137,16 @@ where genesis_hash: Hash, fork_id: Option<&str>, client: Arc, - metrics: Option + metrics: Option, ) -> (Self, RequestResponseConfig) { let (request_receiver, config) = on_demand_justifications_protocol_config(genesis_hash, fork_id); let justif_protocol_name = config.name.clone(); - (Self { request_receiver, justif_protocol_name, client, _block: PhantomData, metrics }, config) + ( + Self { request_receiver, justif_protocol_name, client, _block: PhantomData, metrics }, + config, + ) } /// Network request-response protocol name used by this handler. diff --git a/client/beefy/src/communication/request_response/outgoing_requests_engine.rs b/client/beefy/src/communication/request_response/outgoing_requests_engine.rs index 64142751b8833..2c672c97a6d8c 100644 --- a/client/beefy/src/communication/request_response/outgoing_requests_engine.rs +++ b/client/beefy/src/communication/request_response/outgoing_requests_engine.rs @@ -34,9 +34,10 @@ use std::{collections::VecDeque, result::Result, sync::Arc}; use crate::{ communication::request_response::{Error, JustificationRequest}, justification::{decode_and_verify_finality_proof, BeefyVersionedFinalityProof}, + metric_inc, + metrics::Metrics, KnownPeers, }; -use crate::{metrics::Metrics, metric_inc, metric_set}; /// Response type received from network. type Response = Result, RequestFailure>; @@ -70,7 +71,7 @@ impl OnDemandJustificationsEngine { network: Arc, protocol_name: ProtocolName, live_peers: Arc>>, - metrics: Option + metrics: Option, ) -> Self { Self { network, @@ -78,7 +79,7 @@ impl OnDemandJustificationsEngine { live_peers, peers_cache: VecDeque::new(), state: State::Idle, - metrics + metrics, } } @@ -163,6 +164,7 @@ impl OnDemandJustificationsEngine { ) -> Result, Error> { response .map_err(|e| { + metric_inc!(self, beefy_on_demand_justification_peer_hang_up); debug!( target: "beefy::sync", "🥩 for on demand justification #{:?}, peer {:?} hung up: {:?}", @@ -171,6 +173,7 @@ impl OnDemandJustificationsEngine { Error::InvalidResponse })? .map_err(|e| { + metric_inc!(self, beefy_on_demand_justification_peer_error); debug!( target: "beefy::sync", "🥩 for on demand justification #{:?}, peer {:?} error: {:?}", @@ -185,6 +188,7 @@ impl OnDemandJustificationsEngine { &req_info.active_set, ) .map_err(|e| { + metric_inc!(self, beefy_on_demand_justification_invalid_proof); debug!( target: "beefy::sync", "🥩 for on demand justification #{:?}, peer {:?} responded with invalid proof: {:?}", @@ -222,6 +226,7 @@ impl OnDemandJustificationsEngine { } }) .map(|proof| { + metric_inc!(self, beefy_on_demand_justification_good_proof); debug!( target: "beefy::sync", "🥩 received valid on-demand justif #{:?} from {:?}", diff --git a/client/beefy/src/import.rs b/client/beefy/src/import.rs index 9543223bdea97..83a6db5e0cad5 100644 --- a/client/beefy/src/import.rs +++ b/client/beefy/src/import.rs @@ -35,7 +35,7 @@ use sc_consensus::{BlockCheckParams, BlockImport, BlockImportParams, ImportResul use crate::{ communication::notification::BeefyVersionedFinalityProofSender, justification::{decode_and_verify_finality_proof, BeefyVersionedFinalityProof}, - metric_inc, metric_set, + metric_set, metrics::Metrics, }; diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index 95ca56a6e2248..97e3a9952a929 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -280,7 +280,7 @@ where network.clone(), justifications_protocol_name, known_peers, - metrics.clone() + metrics.clone(), ); // Subscribe to finality notifications and justifications before waiting for runtime pallet and diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index 134f43157daa7..e3673b197e9d6 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -62,6 +62,14 @@ pub struct Metrics { pub beefy_failed_justification_respond_request: Counter, /// Number of On demand justification when there is no peer to request from pub beefy_on_demand_justification_no_peer_to_request_from: Counter, + /// Number of On demand justification peer hang up + pub beefy_on_demand_justification_peer_hang_up: Counter, + /// Number of On demand justification peer error + pub beefy_on_demand_justification_peer_error: Counter, + /// Number of On demand justification invalid proof + pub beefy_on_demand_justification_invalid_proof: Counter, + /// Number of On demand justification good proof + pub beefy_on_demand_justification_good_proof: Counter, } impl Metrics { @@ -172,15 +180,52 @@ impl Metrics { registry, )?, beefy_successful_justification_respond_request: register( - Counter::new("substrate_beefy_successful_justification_respond_request", "Number of Successful Justification respond request")?, + Counter::new( + "substrate_beefy_successful_justification_respond_request", + "Number of Successful Justification respond request", + )?, registry, )?, beefy_failed_justification_respond_request: register( - Counter::new("substrate_beefy_failed_justification_respond_request", "Number of Failed Justification respond request")?, + Counter::new( + "substrate_beefy_failed_justification_respond_request", + "Number of Failed Justification respond request", + )?, registry, )?, beefy_on_demand_justification_no_peer_to_request_from: register( - Counter::new("substrate_beefy_on_demand_justification_no_peer_to_request_from", "Number of on demand justification when there is no peer to request from")?, + Counter::new( + "substrate_beefy_on_demand_justification_no_peer_to_request_from", + "Number of on demand justification when there is no peer to request from", + )?, + registry, + )?, + beefy_on_demand_justification_peer_hang_up: register( + Counter::new( + "substrate_beefy_on_demand_justification_peer_hang_up", + "Number of On demand justification peer hang up", + )?, + registry, + )?, + beefy_on_demand_justification_peer_error: register( + Counter::new( + "substrate_beefy_on_demand_justification_peer_error", + "Number of On demand justification peer error", + )?, + registry, + )?, + beefy_on_demand_justification_invalid_proof: register( + Counter::new( + "substrate_beefy_on_demand_justification_invalid_proof", + "Number of On demand justification invalid proof", + )?, + registry, + )?, + beefy_on_demand_justification_good_proof: register( + Counter::new( + "substrate_beefy_on_demand_justification_good_proof", + "Number of On demand justification good proof", + )?, registry, )?, }) diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index a698fd14b9385..7a32d2dd122b5 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -1055,11 +1055,12 @@ pub(crate) mod tests { let gossip_validator = Arc::new(GossipValidator::new(known_peers.clone())); let gossip_engine = GossipEngine::new(network.clone(), "/beefy/1", gossip_validator.clone(), None); + let metrics = None; let on_demand_justifications = OnDemandJustificationsEngine::new( network.clone(), "/beefy/justifs/1".into(), known_peers, - metrics: None, + metrics.clone(), ); let at = BlockId::number(Zero::zero()); let genesis_header = backend.blockchain().expect_header(at).unwrap(); @@ -1078,7 +1079,7 @@ pub(crate) mod tests { links, gossip_engine, gossip_validator, - metrics: None, + metrics, network, on_demand_justifications, persisted_state, From c4b0bc7c13e8cce84e1109fed7fedf1b8bb61bdd Mon Sep 17 00:00:00 2001 From: Damilare Date: Thu, 26 Jan 2023 11:11:35 +0100 Subject: [PATCH 07/14] fix tests --- .../communication/request_response/incoming_requests_handler.rs | 2 +- client/beefy/src/tests.rs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/client/beefy/src/communication/request_response/incoming_requests_handler.rs b/client/beefy/src/communication/request_response/incoming_requests_handler.rs index 847585ee07229..18f3b6c0c4877 100644 --- a/client/beefy/src/communication/request_response/incoming_requests_handler.rs +++ b/client/beefy/src/communication/request_response/incoming_requests_handler.rs @@ -124,7 +124,7 @@ pub struct BeefyJustifsRequestHandler { pub(crate) justif_protocol_name: ProtocolName, pub(crate) client: Arc, pub(crate) _block: PhantomData, - metrics: Option, + pub(crate) metrics: Option, } impl BeefyJustifsRequestHandler diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index f6ab0dd1020f1..1a8398f7036c4 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -136,6 +136,7 @@ impl BeefyTestNet { justif_protocol_name, client, _block: PhantomData, + metrics: None, }; *net.peers[i].data.beefy_justif_req_handler.lock() = Some(justif_handler); } @@ -201,6 +202,7 @@ impl TestNetFactory for BeefyTestNet { inner, client.as_backend(), Arc::new(two_validators::TestApi {}), + None, ); let peer_data = PeerData { beefy_rpc_links: Mutex::new(Some(rpc_links)), From 04400a28437c3be5a20e49f76fd562f74465ce9e Mon Sep 17 00:00:00 2001 From: Damilare Date: Thu, 26 Jan 2023 12:10:45 +0100 Subject: [PATCH 08/14] merge changes --- .../request_response/incoming_requests_handler.rs | 12 +++++++----- client/beefy/src/import.rs | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/client/beefy/src/communication/request_response/incoming_requests_handler.rs b/client/beefy/src/communication/request_response/incoming_requests_handler.rs index 35697ed3cbd1e..3ea7f565f84a6 100644 --- a/client/beefy/src/communication/request_response/incoming_requests_handler.rs +++ b/client/beefy/src/communication/request_response/incoming_requests_handler.rs @@ -29,11 +29,13 @@ use sc_network_common::protocol::ProtocolName; use sp_runtime::traits::Block; use std::{marker::PhantomData, sync::Arc}; -use crate::{communication::request_response::{ - on_demand_justifications_protocol_config, Error, JustificationRequest, BEEFY_SYNC_LOG_TARGET, -}, -metric_inc, -metrics::Metrics, +use crate::{ + communication::request_response::{ + on_demand_justifications_protocol_config, Error, JustificationRequest, + BEEFY_SYNC_LOG_TARGET, + }, + metric_inc, + metrics::Metrics, }; /// A request coming in, including a sender for sending responses. diff --git a/client/beefy/src/import.rs b/client/beefy/src/import.rs index f5a3266cff3b8..27793b8a7a11c 100644 --- a/client/beefy/src/import.rs +++ b/client/beefy/src/import.rs @@ -35,9 +35,9 @@ use sc_consensus::{BlockCheckParams, BlockImport, BlockImportParams, ImportResul use crate::{ communication::notification::BeefyVersionedFinalityProofSender, justification::{decode_and_verify_finality_proof, BeefyVersionedFinalityProof}, - LOG_TARGET, metric_set, metrics::Metrics, + LOG_TARGET, }; /// A block-import handler for BEEFY. From 863b2b2f03c1ae4a579490cd55e9ffc38b4a066f Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Mon, 13 Feb 2023 17:01:07 +0200 Subject: [PATCH 09/14] Apply suggestions from code review --- .../request_response/incoming_requests_handler.rs | 4 ++-- client/beefy/src/import.rs | 4 ++-- client/beefy/src/metrics.rs | 4 ++-- client/beefy/src/worker.rs | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/client/beefy/src/communication/request_response/incoming_requests_handler.rs b/client/beefy/src/communication/request_response/incoming_requests_handler.rs index 3ea7f565f84a6..084c3d727f29e 100644 --- a/client/beefy/src/communication/request_response/incoming_requests_handler.rs +++ b/client/beefy/src/communication/request_response/incoming_requests_handler.rs @@ -124,8 +124,8 @@ pub struct BeefyJustifsRequestHandler { pub(crate) request_receiver: IncomingRequestReceiver, pub(crate) justif_protocol_name: ProtocolName, pub(crate) client: Arc, - pub(crate) _block: PhantomData, pub(crate) metrics: Option, + pub(crate) _block: PhantomData, } impl BeefyJustifsRequestHandler @@ -145,7 +145,7 @@ where let justif_protocol_name = config.name.clone(); ( - Self { request_receiver, justif_protocol_name, client, _block: PhantomData, metrics }, + Self { request_receiver, justif_protocol_name, client, metrics, _block: PhantomData }, config, ) } diff --git a/client/beefy/src/import.rs b/client/beefy/src/import.rs index 27793b8a7a11c..54fbc6e10676d 100644 --- a/client/beefy/src/import.rs +++ b/client/beefy/src/import.rs @@ -153,7 +153,7 @@ where .notify(|| Ok::<_, ()>(proof)) .expect("forwards closure result; the closure always returns Ok; qed."); - metric_set!(self, beefy_good_justification_imports, number); + metric_inc!(self, beefy_block_import_good_justification); } else { debug!( target: LOG_TARGET, @@ -161,7 +161,7 @@ where encoded, number, ); - metric_set!(self, beefy_bad_justification_imports, number); + metric_inc!(self, beefy_block_import_bad_justification); } }, _ => (), diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index e3673b197e9d6..afc64e27b9477 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -34,8 +34,8 @@ pub struct Metrics { pub beefy_should_vote_on: Gauge, /// Number of sessions with lagging signed commitment on mandatory block pub beefy_lagging_sessions: Counter, - /// Number of validator's trying to vote with no session initialized - pub beefy_voting_with_no_session_initialized: Counter, + /// Number of times trying to vote with no session initialized + pub beefy_no_session_initialized: Counter, /// Number of times no Authority public key found in store pub beefy_no_authority_found_in_store: Counter, /// Number of Buffered votes diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 6ccefec1cfca5..bce7811593cbb 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -505,7 +505,7 @@ where ) } } else { - metric_inc!(self, beefy_buffered_votes_full); + metric_inc!(self, beefy_buffered_votes_dropped); warn!(target: LOG_TARGET, "🥩 Buffer vote dropped for round: {:?}.", block_num); } }, @@ -529,6 +529,7 @@ where match self.voting_oracle().triage_round(block_num, best_grandpa)? { RoundAction::Process => { debug!(target: LOG_TARGET, "🥩 Process justification for round: {:?}.", block_num); + metric_inc!(self, beefy_imported_justifications); self.finalize(justification)? }, RoundAction::Enqueue => { @@ -537,7 +538,7 @@ where if self.pending_justifications.len() < MAX_BUFFERED_JUSTIFICATIONS { self.pending_justifications.entry(block_num).or_insert(justification); } else { - metric_inc!(self, beefy_buffered_justifications_full); + metric_inc!(self, beefy_buffered_justifications_dropped); warn!( target: LOG_TARGET, "🥩 Buffer justification dropped for round: {:?}.", block_num @@ -655,7 +656,6 @@ where .notify(|| Ok::<_, ()>(finality_proof)) .expect("forwards closure result; the closure always returns Ok; qed."); } else { - debug!(target: "beefy", "🥩 Can't set best beefy to older: {}", block_num); debug!(target: LOG_TARGET, "🥩 Can't set best beefy to older: {}", block_num); } Ok(()) @@ -829,7 +829,7 @@ where self.gossip_engine.gossip_message(topic::(), encoded_message, false); - metric_inc!(self, beefy_successful_votes); + metric_inc!(self, beefy_self_votes); Ok(()) } From 1c1553be46cbf1c18fbec9a8255a6d866a51ce4c Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 13 Feb 2023 18:00:03 +0200 Subject: [PATCH 10/14] client/beefy: fix metrics --- client/beefy/src/import.rs | 6 +-- client/beefy/src/metrics.rs | 101 ++++++++++++++++++++---------------- client/beefy/src/worker.rs | 31 ++++++----- 3 files changed, 78 insertions(+), 60 deletions(-) diff --git a/client/beefy/src/import.rs b/client/beefy/src/import.rs index 54fbc6e10676d..e4dd2db5f0336 100644 --- a/client/beefy/src/import.rs +++ b/client/beefy/src/import.rs @@ -35,7 +35,7 @@ use sc_consensus::{BlockCheckParams, BlockImport, BlockImportParams, ImportResul use crate::{ communication::notification::BeefyVersionedFinalityProofSender, justification::{decode_and_verify_finality_proof, BeefyVersionedFinalityProof}, - metric_set, + metric_inc, metrics::Metrics, LOG_TARGET, }; @@ -153,7 +153,7 @@ where .notify(|| Ok::<_, ()>(proof)) .expect("forwards closure result; the closure always returns Ok; qed."); - metric_inc!(self, beefy_block_import_good_justification); + metric_inc!(self, beefy_good_justification_imports); } else { debug!( target: LOG_TARGET, @@ -161,7 +161,7 @@ where encoded, number, ); - metric_inc!(self, beefy_block_import_bad_justification); + metric_inc!(self, beefy_bad_justification_imports); } }, _ => (), diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index afc64e27b9477..694c1b184c5ba 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -26,8 +26,6 @@ pub struct Metrics { pub beefy_validator_set_id: Gauge, /// Total number of votes sent by this node pub beefy_votes_sent: Counter, - /// Most recent concluded voting round - pub beefy_round_concluded: Gauge, /// Best block finalized by BEEFY pub beefy_best_block: Gauge, /// Next block BEEFY should vote on @@ -38,24 +36,28 @@ pub struct Metrics { pub beefy_no_session_initialized: Counter, /// Number of times no Authority public key found in store pub beefy_no_authority_found_in_store: Counter, - /// Number of Buffered votes - pub beefy_buffered_votes: Counter, - /// Number of times Buffered votes is full - pub beefy_buffered_votes_full: Counter, - /// Number of Buffered justifications - pub beefy_buffered_justifications: Counter, - /// Number of times Buffered justifications is full - pub beefy_buffered_justifications_full: Counter, + /// Number of currently buffered votes + pub beefy_buffered_votes: Gauge, + /// Number of valid but stale votes received + pub beefy_stale_votes: Counter, + /// Number of votes dropped due to full buffers + pub beefy_buffered_votes_dropped: Counter, + /// Number of currently buffered justifications + pub beefy_buffered_justifications: Gauge, + /// Number of valid but stale justifications received + pub beefy_stale_justifications: Counter, + /// Number of valid justifications successfully imported + pub beefy_imported_justifications: Counter, + /// Number of justifications dropped due to full buffers + pub beefy_buffered_justifications_dropped: Counter, /// Trying to set Best Beefy block to old block pub beefy_best_block_to_old_block: Gauge, /// Number of Successful handled votes pub beefy_successful_handled_votes: Counter, - /// Number of Successful votes - pub beefy_successful_votes: Counter, - /// Number of Bad Justification imports - pub beefy_bad_justification_imports: Gauge, /// Number of Good Justification imports - pub beefy_good_justification_imports: Gauge, + pub beefy_good_justification_imports: Counter, + /// Number of Bad Justification imports + pub beefy_bad_justification_imports: Counter, /// Number of Successful Justification respond request pub beefy_successful_justification_respond_request: Counter, /// Number of Failed Justification respond request @@ -86,13 +88,6 @@ impl Metrics { Counter::new("substrate_beefy_votes_sent", "Number of votes sent by this node")?, registry, )?, - beefy_round_concluded: register( - Gauge::new( - "substrate_beefy_round_concluded", - "Voting round, that has been concluded", - )?, - registry, - )?, beefy_best_block: register( Gauge::new("substrate_beefy_best_block", "Best block finalized by BEEFY")?, registry, @@ -108,10 +103,10 @@ impl Metrics { )?, registry, )?, - beefy_voting_with_no_session_initialized: register( + beefy_no_session_initialized: register( Counter::new( - "substrate_voting_with_no_session_initialized", - "Number of validator's trying to vote with no session initialized", + "substrate_beefy_no_session_initialized", + "Number of times trying to vote with no session initialized", )?, registry, )?, @@ -123,27 +118,48 @@ impl Metrics { registry, )?, beefy_buffered_votes: register( - Counter::new("substrate_beefy_buffered_votes", "Number of Buffered votes")?, + Gauge::new("substrate_beefy_buffered_votes", "Number of currently buffered votes")?, registry, )?, - beefy_buffered_votes_full: register( + beefy_stale_votes: register( Counter::new( - "substrate_beefy_buffered_votes_full", - "Number of times Buffered votes is full", + "substrate_beefy_stale_votes", + "Number of valid but stale votes received", )?, registry, )?, - beefy_buffered_justifications: register( + beefy_buffered_votes_dropped: register( Counter::new( + "substrate_beefy_buffered_votes_dropped", + "Number of votes dropped due to full buffers", + )?, + registry, + )?, + beefy_buffered_justifications: register( + Gauge::new( "substrate_beefy_buffered_justifications", - "Number of Buffered justifications", + "Number of currently buffered justifications", + )?, + registry, + )?, + beefy_stale_justifications: register( + Counter::new( + "substrate_beefy_stale_justifications", + "Number of valid but stale justifications received", + )?, + registry, + )?, + beefy_imported_justifications: register( + Counter::new( + "substrate_beefy_imported_justifications", + "Number of valid justifications successfully imported", )?, registry, )?, - beefy_buffered_justifications_full: register( + beefy_buffered_justifications_dropped: register( Counter::new( - "substrate_beefy_buffered_justifications_full", - "Number of times Buffered justifications is full", + "substrate_beefy_buffered_justifications_dropped", + "Number of justifications dropped due to full buffers", )?, registry, )?, @@ -161,24 +177,20 @@ impl Metrics { )?, registry, )?, - beefy_successful_votes: register( - Counter::new("substrate_beefy_successful_votes", "Number of Successful votes")?, + beefy_good_justification_imports: register( + Counter::new( + "substrate_beefy_good_justification_imports", + "Number of Good Justification imports", + )?, registry, )?, beefy_bad_justification_imports: register( - Gauge::new( + Counter::new( "substrate_beefy_bad_justification_imports", "Number of Bad Justification imports", )?, registry, )?, - beefy_good_justification_imports: register( - Gauge::new( - "substrate_beefy_good_justification_imports", - "Number of Good Justification imports", - )?, - registry, - )?, beefy_successful_justification_respond_request: register( Counter::new( "substrate_beefy_successful_justification_respond_request", @@ -254,7 +266,6 @@ macro_rules! metric_inc { }}; } -#[cfg(test)] #[macro_export] macro_rules! metric_get { ($self:ident, $m:ident) => {{ diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index bce7811593cbb..e3b473b82748c 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -24,7 +24,7 @@ use crate::{ error::Error, justification::BeefyVersionedFinalityProof, keystore::BeefyKeystore, - metric_inc, metric_set, + metric_get, metric_inc, metric_set, metrics::Metrics, round::Rounds, BeefyVoterLinks, LOG_TARGET, @@ -494,22 +494,24 @@ where false, )?, RoundAction::Enqueue => { - metric_inc!(self, beefy_buffered_votes); debug!(target: LOG_TARGET, "🥩 Buffer vote for round: {:?}.", block_num); if self.pending_votes.len() < MAX_BUFFERED_VOTE_ROUNDS { let votes_vec = self.pending_votes.entry(block_num).or_default(); - if votes_vec.try_push(vote).is_err() { + if votes_vec.try_push(vote).is_ok() { + metric_inc!(self, beefy_buffered_votes); + } else { warn!( target: LOG_TARGET, "🥩 Buffer vote dropped for round: {:?}", block_num - ) + ); + metric_inc!(self, beefy_buffered_votes_dropped); } } else { - metric_inc!(self, beefy_buffered_votes_dropped); warn!(target: LOG_TARGET, "🥩 Buffer vote dropped for round: {:?}.", block_num); + metric_inc!(self, beefy_buffered_votes_dropped); } }, - RoundAction::Drop => (), + RoundAction::Drop => metric_inc!(self, beefy_stale_votes), }; Ok(()) } @@ -533,10 +535,10 @@ where self.finalize(justification)? }, RoundAction::Enqueue => { - metric_inc!(self, beefy_buffered_justifications); debug!(target: LOG_TARGET, "🥩 Buffer justification for round: {:?}.", block_num); if self.pending_justifications.len() < MAX_BUFFERED_JUSTIFICATIONS { self.pending_justifications.entry(block_num).or_insert(justification); + metric_inc!(self, beefy_buffered_justifications); } else { metric_inc!(self, beefy_buffered_justifications_dropped); warn!( @@ -545,7 +547,7 @@ where ); } }, - RoundAction::Drop => (), + RoundAction::Drop => metric_inc!(self, beefy_stale_justifications), }; Ok(()) } @@ -578,8 +580,6 @@ where let finality_proof = VersionedFinalityProof::V1(SignedCommitment { commitment, signatures }); - metric_set!(self, beefy_round_concluded, block_num); - info!( target: LOG_TARGET, "🥩 Round #{} concluded, finality_proof: {:?}.", round.1, finality_proof @@ -657,6 +657,7 @@ where .expect("forwards closure result; the closure always returns Ok; qed."); } else { debug!(target: LOG_TARGET, "🥩 Can't set best beefy to older: {}", block_num); + metric_set!(self, beefy_best_block_to_old_block, block_num); } Ok(()) } @@ -688,19 +689,23 @@ where let justifs_to_handle = to_process_for(&mut self.pending_justifications, interval, _ph); for (num, justification) in justifs_to_handle.into_iter() { debug!(target: LOG_TARGET, "🥩 Handle buffered justification for: {:?}.", num); + metric_inc!(self, beefy_imported_justifications); if let Err(err) = self.finalize(justification) { error!(target: LOG_TARGET, "🥩 Error finalizing block: {}", err); } } + metric_set!(self, beefy_buffered_justifications, self.pending_justifications.len()); // Possibly new interval after processing justifications. interval = self.voting_oracle().accepted_interval(best_grandpa)?; } // Process pending votes. if !self.pending_votes.is_empty() { + let mut processed = 0u64; let votes_to_handle = to_process_for(&mut self.pending_votes, interval, _ph); for (num, votes) in votes_to_handle.into_iter() { debug!(target: LOG_TARGET, "🥩 Handle buffered votes for: {:?}.", num); + processed += votes.len() as u64; for v in votes.into_iter() { if let Err(err) = self.handle_vote( (v.commitment.payload, v.commitment.block_number), @@ -711,6 +716,10 @@ where }; } } + if let Some(previous) = metric_get!(self, beefy_buffered_votes) { + previous.sub(processed); + metric_set!(self, beefy_buffered_votes, previous.get()); + } } Ok(()) } @@ -829,8 +838,6 @@ where self.gossip_engine.gossip_message(topic::(), encoded_message, false); - metric_inc!(self, beefy_self_votes); - Ok(()) } From 365fcf74020fa25301bc69fd4b7e4c696af21351 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 13 Feb 2023 18:42:34 +0200 Subject: [PATCH 11/14] client/beefy: separate metrics per component, avoid double registering --- .../incoming_requests_handler.rs | 30 +++++- .../outgoing_requests_engine.rs | 26 ++++- client/beefy/src/import.rs | 6 +- client/beefy/src/lib.rs | 39 +++---- client/beefy/src/metrics.rs | 101 ++++++++++++------ client/beefy/src/worker.rs | 8 +- 6 files changed, 144 insertions(+), 66 deletions(-) diff --git a/client/beefy/src/communication/request_response/incoming_requests_handler.rs b/client/beefy/src/communication/request_response/incoming_requests_handler.rs index 084c3d727f29e..689a7398ffdcf 100644 --- a/client/beefy/src/communication/request_response/incoming_requests_handler.rs +++ b/client/beefy/src/communication/request_response/incoming_requests_handler.rs @@ -35,7 +35,7 @@ use crate::{ BEEFY_SYNC_LOG_TARGET, }, metric_inc, - metrics::Metrics, + metrics::OnDemandIncomingRequestsMetrics, }; /// A request coming in, including a sender for sending responses. @@ -124,7 +124,7 @@ pub struct BeefyJustifsRequestHandler { pub(crate) request_receiver: IncomingRequestReceiver, pub(crate) justif_protocol_name: ProtocolName, pub(crate) client: Arc, - pub(crate) metrics: Option, + pub(crate) metrics: Option, pub(crate) _block: PhantomData, } @@ -138,11 +138,31 @@ where genesis_hash: Hash, fork_id: Option<&str>, client: Arc, - metrics: Option, + prometheus_registry: Option, ) -> (Self, RequestResponseConfig) { let (request_receiver, config) = on_demand_justifications_protocol_config(genesis_hash, fork_id); let justif_protocol_name = config.name.clone(); + let metrics = prometheus_registry + .as_ref() + .map(OnDemandIncomingRequestsMetrics::register) + .and_then(|result| match result { + Ok(metrics) => { + debug!( + target: "beefy", + "🥩 Registered on-demand incoming justification requests metrics" + ); + Some(metrics) + }, + Err(err) => { + debug!( + target: "beefy", + "🥩 Failed to register incoming justification requests metrics: {:?}", + err + ); + None + }, + }); ( Self { request_receiver, justif_protocol_name, client, metrics, _block: PhantomData }, @@ -190,14 +210,14 @@ where let peer = request.peer; match self.handle_request(request) { Ok(()) => { - metric_inc!(self, beefy_successful_justification_respond_request); + metric_inc!(self, beefy_successful_justification_responses); debug!( target: BEEFY_SYNC_LOG_TARGET, "🥩 Handled BEEFY justification request from {:?}.", peer ) }, Err(e) => { - metric_inc!(self, beefy_failed_justification_respond_request); + metric_inc!(self, beefy_failed_justification_responses); // TODO (issue #12293): apply reputation changes here based on error type. debug!( target: BEEFY_SYNC_LOG_TARGET, diff --git a/client/beefy/src/communication/request_response/outgoing_requests_engine.rs b/client/beefy/src/communication/request_response/outgoing_requests_engine.rs index 930d715d4b1d4..37700cdc6c3d7 100644 --- a/client/beefy/src/communication/request_response/outgoing_requests_engine.rs +++ b/client/beefy/src/communication/request_response/outgoing_requests_engine.rs @@ -35,7 +35,7 @@ use crate::{ communication::request_response::{Error, JustificationRequest, BEEFY_SYNC_LOG_TARGET}, justification::{decode_and_verify_finality_proof, BeefyVersionedFinalityProof}, metric_inc, - metrics::Metrics, + metrics::OnDemandOutgoingRequestsMetrics, KnownPeers, }; @@ -63,7 +63,7 @@ pub struct OnDemandJustificationsEngine { peers_cache: VecDeque, state: State, - metrics: Option, + metrics: Option, } impl OnDemandJustificationsEngine { @@ -71,8 +71,28 @@ impl OnDemandJustificationsEngine { network: Arc, protocol_name: ProtocolName, live_peers: Arc>>, - metrics: Option, + prometheus_registry: Option, ) -> Self { + let metrics = prometheus_registry + .as_ref() + .map(OnDemandOutgoingRequestsMetrics::register) + .and_then(|result| match result { + Ok(metrics) => { + debug!( + target: "beefy", + "🥩 Registered on-demand outgoing justification requests metrics" + ); + Some(metrics) + }, + Err(err) => { + debug!( + target: "beefy", + "🥩 Failed to register outgoing justification requests metrics: {:?}", + err + ); + None + }, + }); Self { network, protocol_name, diff --git a/client/beefy/src/import.rs b/client/beefy/src/import.rs index e4dd2db5f0336..1b5dda3795aae 100644 --- a/client/beefy/src/import.rs +++ b/client/beefy/src/import.rs @@ -36,7 +36,7 @@ use crate::{ communication::notification::BeefyVersionedFinalityProofSender, justification::{decode_and_verify_finality_proof, BeefyVersionedFinalityProof}, metric_inc, - metrics::Metrics, + metrics::BlockImportMetrics, LOG_TARGET, }; @@ -51,7 +51,7 @@ pub struct BeefyBlockImport { runtime: Arc, inner: I, justification_sender: BeefyVersionedFinalityProofSender, - metrics: Option, + metrics: Option, } impl Clone for BeefyBlockImport { @@ -73,7 +73,7 @@ impl BeefyBlockImport { runtime: Arc, inner: I, justification_sender: BeefyVersionedFinalityProofSender, - metrics: Option, + metrics: Option, ) -> BeefyBlockImport { BeefyBlockImport { backend, runtime, inner, justification_sender, metrics } } diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index d03298d1bd2ec..2dfc34186cb14 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -154,19 +154,19 @@ where let (to_voter_justif_sender, from_block_import_justif_stream) = BeefyVersionedFinalityProofStream::::channel(); - let metrics = - prometheus_registry.as_ref().map(metrics::Metrics::register).and_then( - |result| match result { - Ok(metrics) => { - debug!(target: "beefy", "🥩 Registered metrics"); - Some(metrics) - }, - Err(err) => { - debug!(target: "beefy", "🥩 Failed to register metrics: {:?}", err); - None - }, + let metrics = prometheus_registry + .as_ref() + .map(metrics::BlockImportMetrics::register) + .and_then(|result| match result { + Ok(metrics) => { + debug!(target: "beefy", "🥩 Registered block-import metrics"); + Some(metrics) }, - ); + Err(err) => { + debug!(target: "beefy", "🥩 Failed to register block-import metrics: {:?}", err); + None + }, + }); // BlockImport let import = BeefyBlockImport::new( @@ -264,25 +264,26 @@ where ); let metrics = - prometheus_registry.as_ref().map(metrics::Metrics::register).and_then( - |result| match result { + prometheus_registry + .as_ref() + .map(metrics::VoterMetrics::register) + .and_then(|result| match result { Ok(metrics) => { - debug!(target: LOG_TARGET, "🥩 Registered metrics"); + debug!(target: LOG_TARGET, "🥩 Registered voter metrics"); Some(metrics) }, Err(err) => { - debug!(target: LOG_TARGET, "🥩 Failed to register metrics: {:?}", err); + debug!(target: LOG_TARGET, "🥩 Failed to register voter metrics: {:?}", err); None }, - }, - ); + }); // The `GossipValidator` adds and removes known peers based on valid votes and network events. let on_demand_justifications = OnDemandJustificationsEngine::new( network.clone(), justifications_protocol_name, known_peers, - metrics.clone(), + prometheus_registry.clone(), ); // Subscribe to finality notifications and justifications before waiting for runtime pallet and diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index 694c1b184c5ba..c2526876f0eb5 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -19,9 +19,10 @@ //! BEEFY Prometheus metrics definition use prometheus::{register, Counter, Gauge, PrometheusError, Registry, U64}; -/// BEEFY metrics exposed through Prometheus + +/// BEEFY voting-related metrics exposed through Prometheus #[derive(Clone, Debug)] -pub struct Metrics { +pub struct VoterMetrics { /// Current active validator set id pub beefy_validator_set_id: Gauge, /// Total number of votes sent by this node @@ -54,27 +55,9 @@ pub struct Metrics { pub beefy_best_block_to_old_block: Gauge, /// Number of Successful handled votes pub beefy_successful_handled_votes: Counter, - /// Number of Good Justification imports - pub beefy_good_justification_imports: Counter, - /// Number of Bad Justification imports - pub beefy_bad_justification_imports: Counter, - /// Number of Successful Justification respond request - pub beefy_successful_justification_respond_request: Counter, - /// Number of Failed Justification respond request - pub beefy_failed_justification_respond_request: Counter, - /// Number of On demand justification when there is no peer to request from - pub beefy_on_demand_justification_no_peer_to_request_from: Counter, - /// Number of On demand justification peer hang up - pub beefy_on_demand_justification_peer_hang_up: Counter, - /// Number of On demand justification peer error - pub beefy_on_demand_justification_peer_error: Counter, - /// Number of On demand justification invalid proof - pub beefy_on_demand_justification_invalid_proof: Counter, - /// Number of On demand justification good proof - pub beefy_on_demand_justification_good_proof: Counter, } -impl Metrics { +impl VoterMetrics { pub(crate) fn register(registry: &Registry) -> Result { Ok(Self { beefy_validator_set_id: register( @@ -177,6 +160,22 @@ impl Metrics { )?, registry, )?, + }) + } +} + +/// BEEFY block-import-related metrics exposed through Prometheus +#[derive(Clone, Debug)] +pub struct BlockImportMetrics { + /// Number of Good Justification imports + pub beefy_good_justification_imports: Counter, + /// Number of Bad Justification imports + pub beefy_bad_justification_imports: Counter, +} + +impl BlockImportMetrics { + pub(crate) fn register(registry: &Registry) -> Result { + Ok(Self { beefy_good_justification_imports: register( Counter::new( "substrate_beefy_good_justification_imports", @@ -191,52 +190,90 @@ impl Metrics { )?, registry, )?, - beefy_successful_justification_respond_request: register( + }) + } +} + +/// BEEFY on-demand-justifications-related metrics exposed through Prometheus +#[derive(Clone, Debug)] +pub struct OnDemandIncomingRequestsMetrics { + /// Number of Successful Justification responses + pub beefy_successful_justification_responses: Counter, + /// Number of Failed Justification responses + pub beefy_failed_justification_responses: Counter, +} + +impl OnDemandIncomingRequestsMetrics { + pub(crate) fn register(registry: &Registry) -> Result { + Ok(Self { + beefy_successful_justification_responses: register( Counter::new( - "substrate_beefy_successful_justification_respond_request", - "Number of Successful Justification respond request", + "substrate_beefy_successful_justification_responses", + "Number of Successful Justification responses", )?, registry, )?, - beefy_failed_justification_respond_request: register( + beefy_failed_justification_responses: register( Counter::new( - "substrate_beefy_failed_justification_respond_request", - "Number of Failed Justification respond request", + "substrate_beefy_failed_justification_responses", + "Number of Failed Justification responses", )?, registry, )?, + }) + } +} + +/// BEEFY on-demand-justifications-related metrics exposed through Prometheus +#[derive(Clone, Debug)] +pub struct OnDemandOutgoingRequestsMetrics { + /// Number of times there was no good peer to request justification from + pub beefy_on_demand_justification_no_peer_to_request_from: Counter, + /// Number of on-demand justification peer hang up + pub beefy_on_demand_justification_peer_hang_up: Counter, + /// Number of on-demand justification peer error + pub beefy_on_demand_justification_peer_error: Counter, + /// Number of on-demand justification invalid proof + pub beefy_on_demand_justification_invalid_proof: Counter, + /// Number of on-demand justification good proof + pub beefy_on_demand_justification_good_proof: Counter, +} + +impl OnDemandOutgoingRequestsMetrics { + pub(crate) fn register(registry: &Registry) -> Result { + Ok(Self { beefy_on_demand_justification_no_peer_to_request_from: register( Counter::new( "substrate_beefy_on_demand_justification_no_peer_to_request_from", - "Number of on demand justification when there is no peer to request from", + "Number of times there was no good peer to request justification from", )?, registry, )?, beefy_on_demand_justification_peer_hang_up: register( Counter::new( "substrate_beefy_on_demand_justification_peer_hang_up", - "Number of On demand justification peer hang up", + "Number of on-demand justification peer hang up", )?, registry, )?, beefy_on_demand_justification_peer_error: register( Counter::new( "substrate_beefy_on_demand_justification_peer_error", - "Number of On demand justification peer error", + "Number of on-demand justification peer error", )?, registry, )?, beefy_on_demand_justification_invalid_proof: register( Counter::new( "substrate_beefy_on_demand_justification_invalid_proof", - "Number of On demand justification invalid proof", + "Number of on-demand justification invalid proof", )?, registry, )?, beefy_on_demand_justification_good_proof: register( Counter::new( "substrate_beefy_on_demand_justification_good_proof", - "Number of On demand justification good proof", + "Number of on-demand justification good proof", )?, registry, )?, diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index e3b473b82748c..233bb16654c13 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -25,7 +25,7 @@ use crate::{ justification::BeefyVersionedFinalityProof, keystore::BeefyKeystore, metric_get, metric_inc, metric_set, - metrics::Metrics, + metrics::VoterMetrics, round::Rounds, BeefyVoterLinks, LOG_TARGET, }; @@ -254,7 +254,7 @@ pub(crate) struct WorkerParams { pub gossip_validator: Arc>, pub on_demand_justifications: OnDemandJustificationsEngine, pub links: BeefyVoterLinks, - pub metrics: Option, + pub metrics: Option, pub persisted_state: PersistedState, } @@ -311,7 +311,7 @@ pub(crate) struct BeefyWorker { // voter state /// BEEFY client metrics. - metrics: Option, + metrics: Option, /// Buffer holding votes for future processing. pending_votes: BTreeMap< NumberFor, @@ -1095,7 +1095,7 @@ pub(crate) mod tests { network.clone(), "/beefy/justifs/1".into(), known_peers, - metrics.clone(), + None, ); let genesis_header = backend .blockchain() From fe3ef4cd4a56be2b4739594be65a0459ed4e60b6 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 14 Feb 2023 17:29:34 +0200 Subject: [PATCH 12/14] client/beefy: deduplicate metrics registration code --- .../incoming_requests_handler.rs | 24 +---------- .../outgoing_requests_engine.rs | 23 +--------- client/beefy/src/lib.rs | 34 ++------------- client/beefy/src/metrics.rs | 42 +++++++++++++++---- 4 files changed, 42 insertions(+), 81 deletions(-) diff --git a/client/beefy/src/communication/request_response/incoming_requests_handler.rs b/client/beefy/src/communication/request_response/incoming_requests_handler.rs index 689a7398ffdcf..854ece11cf93c 100644 --- a/client/beefy/src/communication/request_response/incoming_requests_handler.rs +++ b/client/beefy/src/communication/request_response/incoming_requests_handler.rs @@ -35,7 +35,7 @@ use crate::{ BEEFY_SYNC_LOG_TARGET, }, metric_inc, - metrics::OnDemandIncomingRequestsMetrics, + metrics::{register_metrics, OnDemandIncomingRequestsMetrics}, }; /// A request coming in, including a sender for sending responses. @@ -143,27 +143,7 @@ where let (request_receiver, config) = on_demand_justifications_protocol_config(genesis_hash, fork_id); let justif_protocol_name = config.name.clone(); - let metrics = prometheus_registry - .as_ref() - .map(OnDemandIncomingRequestsMetrics::register) - .and_then(|result| match result { - Ok(metrics) => { - debug!( - target: "beefy", - "🥩 Registered on-demand incoming justification requests metrics" - ); - Some(metrics) - }, - Err(err) => { - debug!( - target: "beefy", - "🥩 Failed to register incoming justification requests metrics: {:?}", - err - ); - None - }, - }); - + let metrics = register_metrics(prometheus_registry); ( Self { request_receiver, justif_protocol_name, client, metrics, _block: PhantomData }, config, diff --git a/client/beefy/src/communication/request_response/outgoing_requests_engine.rs b/client/beefy/src/communication/request_response/outgoing_requests_engine.rs index 5e60915e78750..3e0d6e47d9432 100644 --- a/client/beefy/src/communication/request_response/outgoing_requests_engine.rs +++ b/client/beefy/src/communication/request_response/outgoing_requests_engine.rs @@ -35,7 +35,7 @@ use crate::{ communication::request_response::{Error, JustificationRequest, BEEFY_SYNC_LOG_TARGET}, justification::{decode_and_verify_finality_proof, BeefyVersionedFinalityProof}, metric_inc, - metrics::OnDemandOutgoingRequestsMetrics, + metrics::{register_metrics, OnDemandOutgoingRequestsMetrics}, KnownPeers, }; @@ -73,26 +73,7 @@ impl OnDemandJustificationsEngine { live_peers: Arc>>, prometheus_registry: Option, ) -> Self { - let metrics = prometheus_registry - .as_ref() - .map(OnDemandOutgoingRequestsMetrics::register) - .and_then(|result| match result { - Ok(metrics) => { - debug!( - target: "beefy", - "🥩 Registered on-demand outgoing justification requests metrics" - ); - Some(metrics) - }, - Err(err) => { - debug!( - target: "beefy", - "🥩 Failed to register outgoing justification requests metrics: {:?}", - err - ); - None - }, - }); + let metrics = register_metrics(prometheus_registry); Self { network, protocol_name, diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index 673e92cab1547..5f74b052e99fe 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -28,6 +28,7 @@ use crate::{ }, }, import::BeefyBlockImport, + metrics::register_metrics, round::Rounds, worker::PersistedState, }; @@ -36,7 +37,7 @@ use beefy_primitives::{ GENESIS_AUTHORITY_SET_ID, }; use futures::{stream::Fuse, StreamExt}; -use log::{debug, error, info}; +use log::{error, info}; use parking_lot::Mutex; use prometheus::Registry; use sc_client_api::{Backend, BlockBackend, BlockchainEvents, FinalityNotifications, Finalizer}; @@ -153,20 +154,7 @@ where // BlockImport -> Voter links let (to_voter_justif_sender, from_block_import_justif_stream) = BeefyVersionedFinalityProofStream::::channel(); - - let metrics = prometheus_registry - .as_ref() - .map(metrics::BlockImportMetrics::register) - .and_then(|result| match result { - Ok(metrics) => { - debug!(target: "beefy", "🥩 Registered block-import metrics"); - Some(metrics) - }, - Err(err) => { - debug!(target: "beefy", "🥩 Failed to register block-import metrics: {:?}", err); - None - }, - }); + let metrics = register_metrics(prometheus_registry); // BlockImport let import = BeefyBlockImport::new( @@ -262,21 +250,7 @@ where gossip_validator.clone(), None, ); - - let metrics = - prometheus_registry - .as_ref() - .map(metrics::VoterMetrics::register) - .and_then(|result| match result { - Ok(metrics) => { - debug!(target: LOG_TARGET, "🥩 Registered voter metrics"); - Some(metrics) - }, - Err(err) => { - debug!(target: LOG_TARGET, "🥩 Failed to register voter metrics: {:?}", err); - None - }, - }); + let metrics = register_metrics(prometheus_registry.clone()); // The `GossipValidator` adds and removes known peers based on valid votes and network events. let on_demand_justifications = OnDemandJustificationsEngine::new( diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index 24ddf2c7c25ab..ce261687a70ce 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -18,8 +18,15 @@ //! BEEFY Prometheus metrics definition +use log::debug; use prometheus::{register, Counter, Gauge, PrometheusError, Registry, U64}; +/// Helper trait for registering BEEFY metrics to Prometheus registry. +pub(crate) trait PrometheusRegister: Sized { + const DESCRIPTION: &'static str; + fn register(registry: &Registry) -> Result; +} + /// BEEFY voting-related metrics exposed through Prometheus #[derive(Clone, Debug)] pub struct VoterMetrics { @@ -59,8 +66,9 @@ pub struct VoterMetrics { pub beefy_successful_handled_votes: Counter, } -impl VoterMetrics { - pub(crate) fn register(registry: &Registry) -> Result { +impl PrometheusRegister for VoterMetrics { + const DESCRIPTION: &'static str = "voter"; + fn register(registry: &Registry) -> Result { Ok(Self { beefy_validator_set_id: register( Gauge::new( @@ -179,8 +187,9 @@ pub struct BlockImportMetrics { pub beefy_bad_justification_imports: Counter, } -impl BlockImportMetrics { - pub(crate) fn register(registry: &Registry) -> Result { +impl PrometheusRegister for BlockImportMetrics { + const DESCRIPTION: &'static str = "block-import"; + fn register(registry: &Registry) -> Result { Ok(Self { beefy_good_justification_imports: register( Counter::new( @@ -209,8 +218,9 @@ pub struct OnDemandIncomingRequestsMetrics { pub beefy_failed_justification_responses: Counter, } -impl OnDemandIncomingRequestsMetrics { - pub(crate) fn register(registry: &Registry) -> Result { +impl PrometheusRegister for OnDemandIncomingRequestsMetrics { + const DESCRIPTION: &'static str = "on-demand incoming justification requests"; + fn register(registry: &Registry) -> Result { Ok(Self { beefy_successful_justification_responses: register( Counter::new( @@ -245,8 +255,9 @@ pub struct OnDemandOutgoingRequestsMetrics { pub beefy_on_demand_justification_good_proof: Counter, } -impl OnDemandOutgoingRequestsMetrics { - pub(crate) fn register(registry: &Registry) -> Result { +impl PrometheusRegister for OnDemandOutgoingRequestsMetrics { + const DESCRIPTION: &'static str = "on-demand outgoing justification requests"; + fn register(registry: &Registry) -> Result { Ok(Self { beefy_on_demand_justification_no_peer_to_request_from: register( Counter::new( @@ -287,6 +298,21 @@ impl OnDemandOutgoingRequestsMetrics { } } +pub(crate) fn register_metrics( + prometheus_registry: Option, +) -> Option { + prometheus_registry.as_ref().map(T::register).and_then(|result| match result { + Ok(metrics) => { + debug!(target: "beefy", "🥩 Registered {} metrics", T::DESCRIPTION); + Some(metrics) + }, + Err(err) => { + debug!(target: "beefy", "🥩 Failed to register {} metrics: {:?}", T::DESCRIPTION, err); + None + }, + }) +} + // Note: we use the `format` macro to convert an expr into a `u64`. This will fail, // if expr does not derive `Display`. #[macro_export] From 3becf323704354285248e740586c213d87b304ae Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 14 Feb 2023 17:32:05 +0200 Subject: [PATCH 13/14] remove unused metric --- client/beefy/src/metrics.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index ce261687a70ce..7a6a53d008f84 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -42,8 +42,6 @@ pub struct VoterMetrics { pub beefy_should_vote_on: Gauge, /// Number of sessions with lagging signed commitment on mandatory block pub beefy_lagging_sessions: Counter, - /// Number of times trying to vote with no session initialized - pub beefy_no_session_initialized: Counter, /// Number of times no Authority public key found in store pub beefy_no_authority_found_in_store: Counter, /// Number of currently buffered votes @@ -100,13 +98,6 @@ impl PrometheusRegister for VoterMetrics { )?, registry, )?, - beefy_no_session_initialized: register( - Counter::new( - "substrate_beefy_no_session_initialized", - "Number of times trying to vote with no session initialized", - )?, - registry, - )?, beefy_no_authority_found_in_store: register( Counter::new( "substrate_beefy_no_authority_found_in_store", From 74bbb52709c341935f4fd5fefd742f1b325b354a Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 16 Feb 2023 11:34:52 +0200 Subject: [PATCH 14/14] impl review suggestions --- client/beefy/src/metrics.rs | 4 ++-- client/beefy/src/worker.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index 7a6a53d008f84..e3b48e6ec8d54 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -59,7 +59,7 @@ pub struct VoterMetrics { /// Number of justifications dropped due to full buffers pub beefy_buffered_justifications_dropped: Counter, /// Trying to set Best Beefy block to old block - pub beefy_best_block_to_old_block: Gauge, + pub beefy_best_block_set_last_failure: Gauge, /// Number of Successful handled votes pub beefy_successful_handled_votes: Counter, } @@ -151,7 +151,7 @@ impl PrometheusRegister for VoterMetrics { )?, registry, )?, - beefy_best_block_to_old_block: register( + beefy_best_block_set_last_failure: register( Gauge::new( "substrate_beefy_best_block_to_old_block", "Trying to set Best Beefy block to old block", diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 98030fe6f453e..6528aef12508a 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -644,8 +644,8 @@ where .notify(|| Ok::<_, ()>(finality_proof)) .expect("forwards closure result; the closure always returns Ok; qed."); } else { - debug!(target: LOG_TARGET, "🥩 Can't set best beefy to older: {}", block_num); - metric_set!(self, beefy_best_block_to_old_block, block_num); + debug!(target: LOG_TARGET, "🥩 Can't set best beefy to old: {}", block_num); + metric_set!(self, beefy_best_block_set_last_failure, block_num); } Ok(()) }