Repository: paritytech/substrate
File: client/network/src/service/out_events.rs (Rust)

Code before:
}

fn increase_metrics(metrics: &Metrics, event: &Event, num: u64) {
	match event {
		Event::Dht(_) => metrics.out_events_dht_count.inc(),
		Event::NotificationStreamOpened { engine_id, .. } => {
			metrics.out_events_notifications_opened_count
				.with_label_values(&[&engine_id_to_string(engine_id)])
				.add(num);
		},
		Event::NotificationStreamClosed { engine_id, .. } => {
			metrics.out_events_notifications_closed_count
				.with_label_values(&[&engine_id_to_string(engine_id)])
				.add(num);
		},
		Event::NotificationsReceived { messages, .. } => {
			for (engine_id, message) in messages {
				metrics.out_events_notifications_sizes
					.with_label_values(&[&engine_id_to_string(engine_id)])
					.add(num.saturating_mul(u64::try_from(message.len()).unwrap_or(u64::max_value())));
			}
		},
	}
}

fn decrease_metrics(metrics: &Metrics, event: &Event) {
	match event {
		Event::Dht(_) => metrics.out_events_dht_count.inc(),
		Event::NotificationStreamOpened { engine_id, .. } => {
			metrics.out_events_notifications_opened_count
				.with_label_values(&[&engine_id_to_string(engine_id)])
				.dec();
		},
		Event::NotificationStreamClosed { engine_id, .. } => {
			metrics.out_events_notifications_closed_count
				.with_label_values(&[&engine_id_to_string(engine_id)])
				.dec();
		},
		Event::NotificationsReceived { messages, .. } => {
			for (engine_id, message) in messages {
				metrics.out_events_notifications_sizes
					.with_label_values(&[&engine_id_to_string(engine_id)])
					.sub(u64::try_from(message.len()).unwrap_or(u64::max_value()));
			}
		},
	}
}


Review comment (suggestion):
Decreasing metrics instead of monotonically increasing hides bursts.

Instead of using a Gauge to count the amount of elements in the channel, one could have a counter for sending and one for receiving. Subtracting the two still leads to the amount of items in the channel, but looking at each of them individually one can spot bursts.

The sending and receiving does not need to be realized as two counters, but one could just use a single counter introducing an `action` label with `send` and `received`.

Code after:
			},
			Event::NotificationStreamClosed { engine_id, .. } => {
				self.events_total
					.with_label_values(&[&format!("notif-closed-{:?}", engine_id), "sent"])
					.inc_by(num);
			},
			Event::NotificationsReceived { messages, .. } => {
				for (engine_id, message) in messages {
					self.events_total
						.with_label_values(&[&format!("notif-{:?}", engine_id), "sent"])
						.inc_by(num);
					self.notifications_sizes
						.with_label_values(&[&engine_id_to_string(engine_id), "sent"])
						.inc_by(num.saturating_mul(u64::try_from(message.len()).unwrap_or(u64::max_value())));
				}
			},
		}
	}

	fn event_out(&self, event: &Event) {
		match event {
			Event::Dht(_) => {
				self.events_total
					.with_label_values(&["dht", "received"])
					.inc();
			}
			Event::NotificationStreamOpened { engine_id, .. } => {
				self.events_total
					.with_label_values(&[&format!("notif-open-{:?}", engine_id), "received"])
					.inc();
			},
			Event::NotificationStreamClosed { engine_id, .. } => {
				self.events_total
					.with_label_values(&[&format!("notif-closed-{:?}", engine_id), "received"])
					.inc();
			},
			Event::NotificationsReceived { messages, .. } => {
				for (engine_id, message) in messages {
					self.events_total
						.with_label_values(&[&format!("notif-{:?}", engine_id), "received"])
						.inc();
					self.notifications_sizes
						.with_label_values(&[&engine_id_to_string(engine_id), "received"])
						.inc_by(u64::try_from(message.len()).unwrap_or(u64::max_value()));
				}
			},
		}
	}
}