client/beefy: add more metrics for production visibility (#12910)

* few beefy metrics

* more beefy metrics

* some beefy metrics

* some beefy metrics

* more metrics

* other metrics

* fix tests

* merge changes

* Apply suggestions from code review

* client/beefy: fix metrics

* client/beefy: separate metrics per component, avoid double registering

* client/beefy: deduplicate metrics registration code

* remove unused metric

* impl review suggestions

---------

Co-authored-by: Adrian Catangiu <[email protected]>

Author: dharjeezy

client/beefy/src/communication/request_response/incoming_requests_handler.rs

@@ -29,8 +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,
+use crate::{
+	communication::request_response::{
+		on_demand_justifications_protocol_config, Error, JustificationRequest,
+		BEEFY_SYNC_LOG_TARGET,
+	},
+	metric_inc,
+	metrics::{register_metrics, OnDemandIncomingRequestsMetrics},
 };
 
 /// A request coming in, including a sender for sending responses.
@@ -119,6 +124,7 @@ pub struct BeefyJustifsRequestHandler<B, Client> {
 	pub(crate) request_receiver: IncomingRequestReceiver,
 	pub(crate) justif_protocol_name: ProtocolName,
 	pub(crate) client: Arc<Client>,
+	pub(crate) metrics: Option<OnDemandIncomingRequestsMetrics>,
 	pub(crate) _block: PhantomData<B>,
 }
 
@@ -132,12 +138,16 @@ where
 		genesis_hash: Hash,
 		fork_id: Option<&str>,
 		client: Arc<Client>,
+		prometheus_registry: Option<prometheus::Registry>,
 	) -> (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)
+		let metrics = register_metrics(prometheus_registry);
+		(
+			Self { request_receiver, justif_protocol_name, client, metrics, _block: PhantomData },
+			config,
+		)
 	}
 
 	/// Network request-response protocol name used by this handler.
@@ -180,12 +190,14 @@ where
 			let peer = request.peer;
 			match self.handle_request(request) {
 				Ok(()) => {
+					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_responses);
 					// TODO (issue #12293): apply reputation changes here based on error type.
 					debug!(
 						target: BEEFY_SYNC_LOG_TARGET,

client/beefy/src/communication/request_response/outgoing_requests_engine.rs

@@ -34,6 +34,8 @@ use std::{collections::VecDeque, result::Result, sync::Arc};
 use crate::{
 	communication::request_response::{Error, JustificationRequest, BEEFY_SYNC_LOG_TARGET},
 	justification::{decode_and_verify_finality_proof, BeefyVersionedFinalityProof},
+	metric_inc,
+	metrics::{register_metrics, OnDemandOutgoingRequestsMetrics},
 	KnownPeers,
 };
 
@@ -61,20 +63,24 @@ pub struct OnDemandJustificationsEngine<B: Block> {
 	peers_cache: VecDeque<PeerId>,
 
 	state: State<B>,
+	metrics: Option<OnDemandOutgoingRequestsMetrics>,
 }
 
 impl<B: Block> OnDemandJustificationsEngine<B> {
 	pub fn new(
 		network: Arc<dyn NetworkRequest + Send + Sync>,
 		protocol_name: ProtocolName,
 		live_peers: Arc<Mutex<KnownPeers<B>>>,
+		prometheus_registry: Option<prometheus::Registry>,
 	) -> Self {
+		let metrics = register_metrics(prometheus_registry);
 		Self {
 			network,
 			protocol_name,
 			live_peers,
 			peers_cache: VecDeque::new(),
 			state: State::Idle,
+			metrics,
 		}
 	}
 
@@ -130,6 +136,7 @@ impl<B: Block> OnDemandJustificationsEngine<B> {
 		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_LOG_TARGET,
 				"🥩 no good peers to request justif #{:?} from", block
@@ -159,6 +166,7 @@ impl<B: Block> OnDemandJustificationsEngine<B> {
 	) -> Result<BeefyVersionedFinalityProof<B>, Error> {
 		response
 			.map_err(|e| {
+				metric_inc!(self, beefy_on_demand_justification_peer_hang_up);
 				debug!(
 					target: BEEFY_SYNC_LOG_TARGET,
 					"🥩 for on demand justification #{:?}, peer {:?} hung up: {:?}",
@@ -169,6 +177,7 @@ impl<B: Block> OnDemandJustificationsEngine<B> {
 				Error::InvalidResponse
 			})?
 			.map_err(|e| {
+				metric_inc!(self, beefy_on_demand_justification_peer_error);
 				debug!(
 					target: BEEFY_SYNC_LOG_TARGET,
 					"🥩 for on demand justification #{:?}, peer {:?} error: {:?}",
@@ -185,6 +194,7 @@ impl<B: Block> OnDemandJustificationsEngine<B> {
 					&req_info.active_set,
 				)
 				.map_err(|e| {
+					metric_inc!(self, beefy_on_demand_justification_invalid_proof);
 					debug!(
 						target: BEEFY_SYNC_LOG_TARGET,
 						"🥩 for on demand justification #{:?}, peer {:?} responded with invalid proof: {:?}",
@@ -224,6 +234,7 @@ impl<B: Block> OnDemandJustificationsEngine<B> {
 				}
 			})
 			.map(|proof| {
+				metric_inc!(self, beefy_on_demand_justification_good_proof);
 				debug!(
 					target: BEEFY_SYNC_LOG_TARGET,
 					"🥩 received valid on-demand justif #{:?} from {:?}", block, peer

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,
+	metrics::BlockImportMetrics,
 	LOG_TARGET,
 };
 
@@ -49,6 +51,7 @@ pub struct BeefyBlockImport<Block: BlockT, Backend, RuntimeApi, I> {
 	runtime: Arc<RuntimeApi>,
 	inner: I,
 	justification_sender: BeefyVersionedFinalityProofSender<Block>,
+	metrics: Option<BlockImportMetrics>,
 }
 
 impl<Block: BlockT, BE, Runtime, I: Clone> Clone for BeefyBlockImport<Block, BE, Runtime, I> {
@@ -58,6 +61,7 @@ impl<Block: BlockT, BE, Runtime, I: Clone> Clone for BeefyBlockImport<Block, BE,
 			runtime: self.runtime.clone(),
 			inner: self.inner.clone(),
 			justification_sender: self.justification_sender.clone(),
+			metrics: self.metrics.clone(),
 		}
 	}
 }
@@ -69,8 +73,9 @@ impl<Block: BlockT, BE, Runtime, I> BeefyBlockImport<Block, BE, Runtime, I> {
 		runtime: Arc<Runtime>,
 		inner: I,
 		justification_sender: BeefyVersionedFinalityProofSender<Block>,
+		metrics: Option<BlockImportMetrics>,
 	) -> BeefyBlockImport<Block, BE, Runtime, I> {
-		BeefyBlockImport { backend, runtime, inner, justification_sender }
+		BeefyBlockImport { backend, runtime, inner, justification_sender, metrics }
 	}
 }
 
@@ -147,13 +152,16 @@ where
 					self.justification_sender
 						.notify(|| Ok::<_, ()>(proof))
 						.expect("forwards closure result; the closure always returns Ok; qed.");
+
+					metric_inc!(self, beefy_good_justification_imports);
 				} else {
 					debug!(
 						target: LOG_TARGET,
 						"🥩 error decoding justification: {:?} for imported block {:?}",
 						encoded,
 						number,
 					);
+					metric_inc!(self, beefy_bad_justification_imports);
 				}
 			},
 			_ => (),

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};
@@ -133,6 +134,7 @@ pub fn beefy_block_import_and_links<B, BE, RuntimeApi, I>(
 	wrapped_block_import: I,
 	backend: Arc<BE>,
 	runtime: Arc<RuntimeApi>,
+	prometheus_registry: Option<Registry>,
 ) -> (BeefyBlockImport<B, BE, RuntimeApi, I>, BeefyVoterLinks<B>, BeefyRPCLinks<B>)
 where
 	B: Block,
@@ -152,10 +154,16 @@ where
 	// BlockImport -> Voter links
 	let (to_voter_justif_sender, from_block_import_justif_stream) =
 		BeefyVersionedFinalityProofStream::<B>::channel();
+	let metrics = register_metrics(prometheus_registry);
 
 	// 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,
@@ -242,28 +250,16 @@ where
 		gossip_validator.clone(),
 		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(
 		network.clone(),
 		justifications_protocol_name,
 		known_peers,
+		prometheus_registry.clone(),
 	);
 
-	let metrics =
-		prometheus_registry.as_ref().map(metrics::Metrics::register).and_then(
-			|result| match result {
-				Ok(metrics) => {
-					debug!(target: LOG_TARGET, "🥩 Registered metrics");
-					Some(metrics)
-				},
-				Err(err) => {
-					debug!(target: LOG_TARGET, "🥩 Failed to register metrics: {:?}", err);
-					None
-				},
-			},
-		);
-
 	// 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();