Skip to content

Commit

Permalink
Decorate mpsc-notification-to-protocol with the protocol name (#3873)
Browse files Browse the repository at this point in the history
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 <alexandru.gheorghe@parity.io>
  • Loading branch information
alexggh authored Mar 29, 2024
1 parent b310b57 commit 5638d1a
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
18 changes: 16 additions & 2 deletions substrate/client/network/src/protocol/notifications/service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,8 @@ impl NotificationService for NotificationHandle {
// Clone [`NotificationService`]
fn clone(&mut self) -> Result<Box<dyn NotificationService>, ()> {
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 {
Expand Down Expand Up @@ -624,11 +625,24 @@ pub fn notification_service(
protocol: ProtocolName,
) -> (ProtocolHandlePair, Box<dyn NotificationService>) {
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]));

(
ProtocolHandlePair::new(protocol.clone(), subscribers.clone(), cmd_rx),
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::<Vec<_>>();
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))
}
7 changes: 6 additions & 1 deletion substrate/client/utils/src/mpsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ pub fn tracing_unbounded<T>(
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)
}

Expand Down Expand Up @@ -157,6 +157,11 @@ impl<T> TracingUnboundedReceiver<T> {
pub fn len(&self) -> usize {
self.inner.len()
}

/// The name of this receiver
pub fn name(&self) -> &'static str {
self.name
}
}

impl<T> Drop for TracingUnboundedReceiver<T> {
Expand Down

0 comments on commit 5638d1a

Please sign in to comment.