diff --git a/consensus/core/src/authority_service.rs b/consensus/core/src/authority_service.rs index 444312190f9a1..5b7a0828fc2ad 100644 --- a/consensus/core/src/authority_service.rs +++ b/consensus/core/src/authority_service.rs @@ -94,7 +94,7 @@ impl NetworkService for AuthorityService { .metrics .node_metrics .invalid_blocks - .with_label_values(&[peer_hostname, "handle_send_block"]) + .with_label_values(&[peer_hostname, "handle_send_block", "UnexpectedAuthority"]) .inc(); let e = ConsensusError::UnexpectedAuthority(signed_block.author(), peer); info!("Block with wrong authority from {}: {}", peer, e); @@ -108,7 +108,7 @@ impl NetworkService for AuthorityService { .metrics .node_metrics .invalid_blocks - .with_label_values(&[peer_hostname, "handle_send_block"]) + .with_label_values(&[peer_hostname, "handle_send_block", e.clone().name()]) .inc(); info!("Invalid block from {}: {}", peer, e); return Err(e); diff --git a/consensus/core/src/block_manager.rs b/consensus/core/src/block_manager.rs index ba4a6e740e063..86fc1ac96752a 100644 --- a/consensus/core/src/block_manager.rs +++ b/consensus/core/src/block_manager.rs @@ -159,11 +159,18 @@ impl BlockManager { } } for (block_ref, block) in blocks_to_reject { + let hostname = self + .context + .committee + .authority(block_ref.author) + .hostname + .clone(); + self.context .metrics .node_metrics .invalid_blocks - .with_label_values(&[&block_ref.author.to_string(), "accept_block"]) + .with_label_values(&[&hostname, "accept_block", "InvalidAncestors"]) .inc(); warn!("Invalid block {:?} is rejected", block); } diff --git a/consensus/core/src/error.rs b/consensus/core/src/error.rs index 9dadc6c84da06..32336d363e2b2 100644 --- a/consensus/core/src/error.rs +++ b/consensus/core/src/error.rs @@ -192,6 +192,13 @@ pub(crate) enum ConsensusError { Shutdown, } +impl ConsensusError { + /// Returns the error name - only the enun name without any parameters - as a static string. + pub fn name(&self) -> &'static str { + self.into() + } +} + pub type ConsensusResult = Result; #[macro_export] @@ -209,3 +216,39 @@ macro_rules! ensure { } }; } + +#[cfg(test)] +mod test { + use super::*; + + /// This test ensures that consensus errors when converted to a static string are the same as the enum name without + /// any parameterers included to the result string. + #[test] + fn test_error_name() { + { + let error = ConsensusError::InvalidAncestorRound { + ancestor: 10, + block: 11, + }; + let error: &'static str = error.into(); + + assert_eq!(error, "InvalidAncestorRound"); + } + + { + let error = ConsensusError::InvalidAuthorityIndex { + index: AuthorityIndex::new_for_test(3), + max: 10, + }; + assert_eq!(error.name(), "InvalidAuthorityIndex"); + } + + { + let error = ConsensusError::InsufficientParentStakes { + parent_stakes: 5, + quorum: 20, + }; + assert_eq!(error.name(), "InsufficientParentStakes"); + } + } +} diff --git a/consensus/core/src/metrics.rs b/consensus/core/src/metrics.rs index 4915b6601c0e7..9588658fbc8ea 100644 --- a/consensus/core/src/metrics.rs +++ b/consensus/core/src/metrics.rs @@ -365,7 +365,7 @@ impl NodeMetrics { invalid_blocks: register_int_counter_vec_with_registry!( "invalid_blocks", "Number of invalid blocks per peer authority", - &["authority", "source"], + &["authority", "source", "error"], registry, ).unwrap(), rejected_blocks: register_int_counter_vec_with_registry!( diff --git a/consensus/core/src/synchronizer.rs b/consensus/core/src/synchronizer.rs index cf97dc8b40559..e3b6f2abf505d 100644 --- a/consensus/core/src/synchronizer.rs +++ b/consensus/core/src/synchronizer.rs @@ -611,11 +611,13 @@ impl Synchronizer Synchronizer