Add req/resp metrics from additional PeerDAS list #6430
Conversation
| )?, | ||
| }, | ||
| ))) | ||
| } | ||
| SupportedProtocol::PingV1 => Ok(Some(InboundRequest::Ping(Ping { |
There was a problem hiding this comment.
Do I need to count Ping in TOTAL_RPC_REQUESTS_BYTES_RECEIVED metric?
| Ok(Some(InboundRequest::Status(StatusMessage::from_ssz_bytes( | ||
| decoded_buffer, | ||
| )?))) | ||
| } | ||
| SupportedProtocol::GoodbyeV1 => Ok(Some(InboundRequest::Goodbye( |
There was a problem hiding this comment.
Do I need to count Goodbye in TOTAL_RPC_REQUESTS_BYTES_RECEIVED metric?
chong-he
added
ready-for-review
| &metrics::TOTAL_RPC_REQUESTS_BYTES_SENT, | ||
| &[item.into()], | ||
| dst.len() as u64, | ||
| ); |
There was a problem hiding this comment.
Feels wrong to place a metric inside an encode function in a code struct, may be best to place this on the consumer of this function?
| )?, | ||
| }, | ||
| ))), | ||
| SupportedProtocol::DataColumnsByRangeV1 => { |
There was a problem hiding this comment.
There's a lot of repetition in this match, you can turn SupportedProtocol into a str and have a single register metric call
| @@ -122,9 +122,64 @@ pub static TOTAL_RPC_ERRORS_PER_CLIENT: LazyLock<Result<IntCounterVec>> = LazyLo | |||
| &["client", "rpc_error", "direction"], | |||
| ) | |||
| }); | |||
| pub static TOTAL_RPC_REQUESTS: LazyLock<Result<IntCounterVec>> = LazyLock::new(|| { | |||
| try_create_int_counter_vec("libp2p_rpc_requests_total", "RPC requests total", &["type"]) | |||
There was a problem hiding this comment.
We generally avoid renaming metrics as it is a breaking change for users, but I do agree the current name is not just unclear but also a bit misleading, so I think this is a good change. I'll marked this PR as breaking change so we remember to mention it in our release notes.
This dashboard needs to be updated too:
https://github.com/sigp/lighthouse-metrics/blob/9c654c48d108180cfec48398c9410ae80839047e/dashboards/Network.json#L2620
jimmygchen
added
waiting-on-author
Issue Addressed
This PR addresses issue #6018
Proposed Changes
The following list of metrics is implemented: