From 3763562b4b860337e0fa162cdb79ab618c98f92a Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Wed, 28 Aug 2024 15:35:30 +0530 Subject: [PATCH 1/3] chore(p2p_service): add metrics for number of blocks requested over p2p req/res protocol --- CHANGELOG.md | 1 + crates/metrics/src/p2p_metrics.rs | 26 +++++++++++++++++++-- crates/services/p2p/src/p2p_service.rs | 15 +++++++++---- crates/services/p2p/src/service.rs | 31 +++++++++++++++++++++++++- 4 files changed, 66 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ee9c5d2a1c..4c10ecb97d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - [2122](https://github.com/FuelLabs/fuel-core/pull/2122): Changed the relayer URI address to be a vector and use a quorum provider. The `relayer` argument now supports multiple URLs to fetch information from different sources. +- [2135](https://github.com/FuelLabs/fuel-core/pull/2135): Added metrics logging for number of blocks served over the p2p req/res protocol. ### Changed - [2113](https://github.com/FuelLabs/fuel-core/pull/2113): Modify the way the gas price service and shared algo is initialized to have some default value based on best guess instead of `None`, and initialize service before graphql. diff --git a/crates/metrics/src/p2p_metrics.rs b/crates/metrics/src/p2p_metrics.rs index b9123bed64b..4aaa813c38f 100644 --- a/crates/metrics/src/p2p_metrics.rs +++ b/crates/metrics/src/p2p_metrics.rs @@ -1,16 +1,24 @@ use crate::global_registry; -use prometheus_client::metrics::counter::Counter; +use prometheus_client::metrics::{ + counter::Counter, + gauge::Gauge, +}; use std::sync::OnceLock; pub struct P2PMetrics { pub unique_peers: Counter, + pub blocks_requested: Gauge, } impl P2PMetrics { fn new() -> Self { let unique_peers = Counter::default(); + let blocks_requested = Gauge::default(); - let metrics = P2PMetrics { unique_peers }; + let metrics = P2PMetrics { + unique_peers, + blocks_requested, + }; let mut registry = global_registry().registry.lock(); registry.register( @@ -19,6 +27,12 @@ impl P2PMetrics { metrics.unique_peers.clone(), ); + registry.register( + "Blocks_Requested", + "A Gauge which keeps track of how many blocks were requested and served over the p2p req/res protocol", + metrics.blocks_requested.clone() + ); + metrics } } @@ -28,3 +42,11 @@ static P2P_METRICS: OnceLock = OnceLock::new(); pub fn p2p_metrics() -> &'static P2PMetrics { P2P_METRICS.get_or_init(P2PMetrics::new) } + +pub fn inc_unique_peers() { + p2p_metrics().unique_peers.inc(); +} + +pub fn set_blocks_requested(count: usize) { + p2p_metrics().blocks_requested.set(count as i64); +} diff --git a/crates/services/p2p/src/p2p_service.rs b/crates/services/p2p/src/p2p_service.rs index 41ef9fb8109..dba7a435759 100644 --- a/crates/services/p2p/src/p2p_service.rs +++ b/crates/services/p2p/src/p2p_service.rs @@ -35,7 +35,7 @@ use crate::{ }, TryPeerId, }; -use fuel_core_metrics::p2p_metrics::p2p_metrics; +use fuel_core_metrics::p2p_metrics::inc_unique_peers; use fuel_core_types::{ fuel_types::BlockHeight, services::p2p::peer_reputation::AppScore, @@ -271,6 +271,15 @@ impl FuelP2PService { } } + pub fn log_metrics(&self, cb: T) + where + T: FnOnce(), + { + if self.metrics { + cb(); + } + } + #[cfg(feature = "test-helpers")] pub fn multiaddrs(&self) -> Vec { let local_peer = self.local_peer_id; @@ -644,9 +653,7 @@ impl FuelP2PService { fn handle_identify_event(&mut self, event: identify::Event) -> Option { match event { identify::Event::Received { peer_id, info } => { - if self.metrics { - p2p_metrics().unique_peers.inc(); - } + self.log_metrics(inc_unique_peers); let mut addresses = info.listen_addrs; let agent_version = info.agent_version; diff --git a/crates/services/p2p/src/service.rs b/crates/services/p2p/src/service.rs index f0b2be9f437..515183f75f5 100644 --- a/crates/services/p2p/src/service.rs +++ b/crates/services/p2p/src/service.rs @@ -26,6 +26,7 @@ use crate::{ }, }; use anyhow::anyhow; +use fuel_core_metrics::p2p_metrics::set_blocks_requested; use fuel_core_services::{ stream::BoxStream, RunnableService, @@ -196,9 +197,20 @@ pub trait TaskP2PService: Send { ) -> anyhow::Result<()>; fn update_block_height(&mut self, height: BlockHeight) -> anyhow::Result<()>; + + fn log_metrics(&self, cb: T) + where + T: FnOnce(); } impl TaskP2PService for FuelP2PService { + fn log_metrics(&self, cb: T) + where + T: FnOnce(), + { + FuelP2PService::log_metrics(self, cb) + } + fn get_all_peer_info(&self) -> Vec<(&PeerId, &PeerInfo)> { self.peer_manager().get_all_peers().collect() } @@ -427,6 +439,13 @@ where V: AtomicView + 'static, V::LatestView: P2pDb, { + fn log_metrics(&self, cb: T) + where + T: FnOnce(), + { + self.p2p_service.log_metrics(cb) + } + fn process_request( &mut self, request_message: RequestMessage, @@ -464,8 +483,11 @@ where // If there are other types of data we send over p2p req/res protocol, then this needs // to be generalized let max_len = self.max_headers_per_request; + let range_len = range.len(); + + self.log_metrics(|| set_blocks_requested(range_len)); - if range.len() > max_len { + if range_len > max_len { tracing::error!( requested_length = range.len(), max_len, @@ -1031,6 +1053,13 @@ pub mod tests { } impl TaskP2PService for FakeP2PService { + fn log_metrics(&self, _: T) + where + T: FnOnce(), + { + unimplemented!() + } + fn get_all_peer_info(&self) -> Vec<(&PeerId, &PeerInfo)> { self.peer_info.iter().map(|tup| (&tup.0, &tup.1)).collect() } From 3fd23e31906023583d346b2c2b3e745bfadda00a Mon Sep 17 00:00:00 2001 From: Aaryamann Challani <43716372+rymnc@users.noreply.github.com> Date: Wed, 28 Aug 2024 21:46:12 +0530 Subject: [PATCH 2/3] fix: changelog ordering --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e051d2642b6..00b02fa32d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - [2122](https://github.com/FuelLabs/fuel-core/pull/2122): Changed the relayer URI address to be a vector and use a quorum provider. The `relayer` argument now supports multiple URLs to fetch information from different sources. - [2119](https://github.com/FuelLabs/fuel-core/pull/2119): GraphQL query fields for retrieving information about upgrades. -- [2116](https://github.com/FuelLabs/fuel-core/pull/2116): Replace `H160` in config and cli options of relayer by `Bytes20` of `fuel-types` ### Changed - [2113](https://github.com/FuelLabs/fuel-core/pull/2113): Modify the way the gas price service and shared algo is initialized to have some default value based on best guess instead of `None`, and initialize service before graphql. From 22f7693a55ce133f6868acf73b64783236be8a75 Mon Sep 17 00:00:00 2001 From: rymnc <43716372+rymnc@users.noreply.github.com> Date: Thu, 29 Aug 2024 16:29:48 +0530 Subject: [PATCH 3/3] chore: address naming issues --- crates/metrics/src/p2p_metrics.rs | 2 +- crates/services/p2p/src/p2p_service.rs | 8 ++++---- crates/services/p2p/src/service.rs | 14 +++++++------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/metrics/src/p2p_metrics.rs b/crates/metrics/src/p2p_metrics.rs index 4aaa813c38f..a139cfb6ee3 100644 --- a/crates/metrics/src/p2p_metrics.rs +++ b/crates/metrics/src/p2p_metrics.rs @@ -43,7 +43,7 @@ pub fn p2p_metrics() -> &'static P2PMetrics { P2P_METRICS.get_or_init(P2PMetrics::new) } -pub fn inc_unique_peers() { +pub fn increment_unique_peers() { p2p_metrics().unique_peers.inc(); } diff --git a/crates/services/p2p/src/p2p_service.rs b/crates/services/p2p/src/p2p_service.rs index dba7a435759..e1afcad3a60 100644 --- a/crates/services/p2p/src/p2p_service.rs +++ b/crates/services/p2p/src/p2p_service.rs @@ -35,7 +35,7 @@ use crate::{ }, TryPeerId, }; -use fuel_core_metrics::p2p_metrics::inc_unique_peers; +use fuel_core_metrics::p2p_metrics::increment_unique_peers; use fuel_core_types::{ fuel_types::BlockHeight, services::p2p::peer_reputation::AppScore, @@ -271,12 +271,12 @@ impl FuelP2PService { } } - pub fn log_metrics(&self, cb: T) + pub fn update_metrics(&self, update_fn: T) where T: FnOnce(), { if self.metrics { - cb(); + update_fn(); } } @@ -653,7 +653,7 @@ impl FuelP2PService { fn handle_identify_event(&mut self, event: identify::Event) -> Option { match event { identify::Event::Received { peer_id, info } => { - self.log_metrics(inc_unique_peers); + self.update_metrics(increment_unique_peers); let mut addresses = info.listen_addrs; let agent_version = info.agent_version; diff --git a/crates/services/p2p/src/service.rs b/crates/services/p2p/src/service.rs index 515183f75f5..bf875289743 100644 --- a/crates/services/p2p/src/service.rs +++ b/crates/services/p2p/src/service.rs @@ -198,17 +198,17 @@ pub trait TaskP2PService: Send { fn update_block_height(&mut self, height: BlockHeight) -> anyhow::Result<()>; - fn log_metrics(&self, cb: T) + fn update_metrics(&self, update_fn: T) where T: FnOnce(); } impl TaskP2PService for FuelP2PService { - fn log_metrics(&self, cb: T) + fn update_metrics(&self, update_fn: T) where T: FnOnce(), { - FuelP2PService::log_metrics(self, cb) + FuelP2PService::update_metrics(self, update_fn) } fn get_all_peer_info(&self) -> Vec<(&PeerId, &PeerInfo)> { @@ -439,11 +439,11 @@ where V: AtomicView + 'static, V::LatestView: P2pDb, { - fn log_metrics(&self, cb: T) + fn update_metrics(&self, update_fn: T) where T: FnOnce(), { - self.p2p_service.log_metrics(cb) + self.p2p_service.update_metrics(update_fn) } fn process_request( @@ -485,7 +485,7 @@ where let max_len = self.max_headers_per_request; let range_len = range.len(); - self.log_metrics(|| set_blocks_requested(range_len)); + self.update_metrics(|| set_blocks_requested(range_len)); if range_len > max_len { tracing::error!( @@ -1053,7 +1053,7 @@ pub mod tests { } impl TaskP2PService for FakeP2PService { - fn log_metrics(&self, _: T) + fn update_metrics(&self, _: T) where T: FnOnce(), {