From 5638d1a830dc70f56e5fdd7eded21a4f592d382c Mon Sep 17 00:00:00 2001 From: Alexandru Gheorghe <49718502+alexggh@users.noreply.github.com> Date: Fri, 29 Mar 2024 13:24:26 +0200 Subject: [PATCH] Decorate mpsc-notification-to-protocol with the protocol name (#3873) Currently, all protocols use the same metric name for `mpsc-notification-to-protocol` this is bad because we can't actually tell which protocol might cause problems. This patch proposes we derive the name of the metric from the protocol name, so that we have separate metrics for each protocol and properly detect which one is having problem processing its messages. --------- Signed-off-by: Alexandru Gheorghe --- .../src/protocol/notifications/service/mod.rs | 18 ++++++++++++++++-- substrate/client/utils/src/mpsc.rs | 7 ++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/substrate/client/network/src/protocol/notifications/service/mod.rs b/substrate/client/network/src/protocol/notifications/service/mod.rs index 6d9873f45d..dfb19daa28 100644 --- a/substrate/client/network/src/protocol/notifications/service/mod.rs +++ b/substrate/client/network/src/protocol/notifications/service/mod.rs @@ -338,7 +338,8 @@ impl NotificationService for NotificationHandle { // Clone [`NotificationService`] fn clone(&mut self) -> Result, ()> { let mut subscribers = self.subscribers.lock(); - let (event_tx, event_rx) = tracing_unbounded("mpsc-notification-to-protocol", 100_000); + + let (event_tx, event_rx) = tracing_unbounded(self.rx.name(), 100_000); subscribers.push(event_tx); Ok(Box::new(NotificationHandle { @@ -624,7 +625,9 @@ pub fn notification_service( protocol: ProtocolName, ) -> (ProtocolHandlePair, Box) { let (cmd_tx, cmd_rx) = mpsc::channel(COMMAND_QUEUE_SIZE); - let (event_tx, event_rx) = tracing_unbounded("mpsc-notification-to-protocol", 100_000); + + let (event_tx, event_rx) = + tracing_unbounded(metric_label_for_protocol(&protocol).leak(), 100_000); let subscribers = Arc::new(Mutex::new(vec![event_tx])); ( @@ -632,3 +635,14 @@ pub fn notification_service( Box::new(NotificationHandle::new(protocol.clone(), cmd_tx, event_rx, subscribers)), ) } + +// Decorates the mpsc-notification-to-protocol metric with the name of the protocol, +// to be able to distiguish between different protocols in dashboards. +fn metric_label_for_protocol(protocol: &ProtocolName) -> String { + let protocol_name = protocol.to_string(); + let keys = protocol_name.split("/").collect::>(); + keys.iter() + .rev() + .take(2) // Last two tokens give the protocol name and version + .fold("mpsc-notification-to-protocol".into(), |acc, val| format!("{}-{}", acc, val)) +} diff --git a/substrate/client/utils/src/mpsc.rs b/substrate/client/utils/src/mpsc.rs index c24a5bd890..91db7e1e7b 100644 --- a/substrate/client/utils/src/mpsc.rs +++ b/substrate/client/utils/src/mpsc.rs @@ -86,7 +86,7 @@ pub fn tracing_unbounded( warning_fired: Arc::new(AtomicBool::new(false)), creation_backtrace: Arc::new(Backtrace::force_capture()), }; - let receiver = TracingUnboundedReceiver { inner: r, name }; + let receiver = TracingUnboundedReceiver { inner: r, name: name.into() }; (sender, receiver) } @@ -157,6 +157,11 @@ impl TracingUnboundedReceiver { pub fn len(&self) -> usize { self.inner.len() } + + /// The name of this receiver + pub fn name(&self) -> &'static str { + self.name + } } impl Drop for TracingUnboundedReceiver {