Prometheus Metrics: Turn notifications_total counter into notificatio…

…ns_sizes histogram (#5535) * Turn notifications_total into notifications_sizes * Address review
paritytech · Apr 6, 2020 · fc6ddaf · fc6ddaf
1 parent 3f134a1
commit fc6ddaf
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 10 deletions.
diff --git a/client/network/src/service.rs b/client/network/src/service.rs
@@ -43,7 +43,7 @@ use libp2p::{kad::record, Multiaddr, PeerId};
 use log::{error, info, trace, warn};
 use parking_lot::Mutex;
 use prometheus_endpoint::{
-	register, Counter, CounterVec, Gauge, GaugeVec, HistogramOpts, HistogramVec, Opts, PrometheusError, Registry, U64,
+	register, Counter, Gauge, GaugeVec, HistogramOpts, HistogramVec, Opts, PrometheusError, Registry, U64,
 };
 use sc_peerset::PeersetHandle;
 use sp_consensus::import_queue::{BlockImportError, BlockImportResult, ImportQueue, Link};
@@ -824,7 +824,7 @@ struct Metrics {
 	kbuckets_num_nodes: Gauge<U64>,
 	network_per_sec_bytes: GaugeVec<U64>,
 	notifications_queues_size: HistogramVec,
-	notifications_total: CounterVec<U64>,
+	notifications_sizes: HistogramVec,
 	num_event_stream_channels: Gauge<U64>,
 	opened_notification_streams: GaugeVec<U64>,
 	peers_count: Gauge<U64>,
@@ -879,11 +879,15 @@ impl Metrics {
 				},
 				&["protocol"]
 			)?, registry)?,
-			notifications_total: register(CounterVec::new(
-				Opts::new(
-					"sub_libp2p_notifications_total",
-					"Number of notification received from all nodes"
-				),
+			notifications_sizes: register(HistogramVec::new(
+				HistogramOpts {
+					common_opts: Opts::new(
+						"sub_libp2p_notifications_sizes",
+						"Sizes of the notifications send to and received from all nodes"
+					),
+					buckets: prometheus_endpoint::exponential_buckets(64.0, 4.0, 8)
+						.expect("parameters are always valid values; qed"),
+				},
 				&["direction", "protocol"]
 			)?, registry)?,
 			num_event_stream_channels: register(Gauge::new(
@@ -921,8 +925,10 @@ impl Metrics {
 				self.opened_notification_streams.with_label_values(&[&engine_id_to_string(&engine_id)]).dec();
 			},
 			Event::NotificationsReceived { messages, .. } => {
-				for (engine_id, _) in messages {
-					self.notifications_total.with_label_values(&["in", &engine_id_to_string(&engine_id)]).inc();
+				for (engine_id, message) in messages {
+					self.notifications_sizes
+						.with_label_values(&["in", &engine_id_to_string(&engine_id)])
+						.observe(message.len() as f64);
 				}
 			},
 			_ => {}
@@ -983,7 +989,9 @@ impl<B: BlockT + 'static, H: ExHashT> Future for NetworkWorker<B, H> {
 					this.event_streams.push(sender),
 				ServiceToWorkerMsg::WriteNotification { message, engine_id, target } => {
 					if let Some(metrics) = this.metrics.as_ref() {
-						metrics.notifications_total.with_label_values(&["out", &engine_id_to_string(&engine_id)]).inc();
+						metrics.notifications_sizes
+							.with_label_values(&["out", &engine_id_to_string(&engine_id)])
+							.observe(message.len() as f64);
 					}
 					this.network_service.user_protocol_mut().write_notification(target, engine_id, message)
 				},

diff --git a/utils/prometheus/src/lib.rs b/utils/prometheus/src/lib.rs
@@ -19,6 +19,7 @@ pub use prometheus::{
 	self,
 	Registry, Error as PrometheusError, Opts,
 	Histogram, HistogramOpts, HistogramVec,
+	exponential_buckets,
 	core::{
 		GenericGauge as Gauge, GenericCounter as Counter,
 		GenericGaugeVec as GaugeVec, GenericCounterVec as CounterVec,