Add a histogram for tcp events based on Event type #3064
Conversation
| [dependencies.metrics_external] | ||
| version = "0.22" | ||
| package = "metrics" | ||
|
|
| Ok(event) => Ok(Some(event)), | ||
| Ok(event) => { | ||
| #[cfg(feature = "metrics")] | ||
| histogram!(metrics::tcp::TCP_GATEWAY, "event" => event.name()).record(bytes_len); |
There was a problem hiding this comment.
Use the histogram from snarkvm_metrics
There was a problem hiding this comment.
In snarkvm_metrics, you can only update a histogram with a static name and value, which would require creating a separate metric name for every event type-- using just the regular histogram macro allows you to add labels which keeps the number of separate metrics lower and allows for easier analysis in Prometheus
I tried to call the original metrics crate histogram macro from here but it looked like I would need to add it to expose it in the snarkvm_metrics crate 😅
There was a problem hiding this comment.
Got it, thanks for the clarification! I'll take a look at this when things are more stable.
| @@ -21,6 +21,7 @@ use rayon::{ | |||
| iter::{IndexedParallelIterator, ParallelIterator}, | |||
| prelude::ParallelSlice, | |||
| }; | |||
| use metrics_external::histogram; | |||
|
closing this in favor of https://github.com/AleoHQ/snarkOS/pull/3097 |
Motivation
To better understand what is going on in SnarkOS TCP land, it may be beneficial to create a histogram (1 metric in Prometheus) that shows count and sum in bytes of each Event Message type, so this PR adds one called
snarkos_tcp_gateway_messages_received, with dynamic labeling based on the event name, in theDecoderimpl of theEventCodec. I am using the basicmetricsrust crate to call the histogram macro, however it appears that in SnarkVM this functionality is overridden by the implementation of the snarkVM metrics crate, so I figured the easiest thing to do was to just add an alias for the default crate.Note, the
histogram!macro used currently only needs to be called once, so we don't need to register the metrics in/node/metrics/src/names.rs, which might be worth revisiting at a later date.Test Plan
Run on local devnets