Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add req/resp metrics from additional PeerDAS list #6430

Open
wants to merge 8 commits into
base: unstable
Choose a base branch
from
+269 −27
Open

Add req/resp metrics from additional PeerDAS list #6430

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
11ad2ae
add req/resp metrics
KatyaRyazantseva Sep 24, 2024
eb5a60c
group req/resp metrics together
KatyaRyazantseva Sep 24, 2024
556df28
merge libp2p-metrics branch into 'origin/unstable'
KatyaRyazantseva Sep 26, 2024
3769eef
add derive IntoStaticStr for request, response
KatyaRyazantseva Sep 27, 2024
807ffc6
Merge remote-tracking branch 'origin/unstable' into libp2p-metrics
KatyaRyazantseva Sep 27, 2024
5a92a92
Merge branch 'unstable' into libp2p-metrics
KatyaRyazantseva Oct 23, 2024
0682cd1
Merge remote-tracking branch 'origin/unstable' into libp2p-metrics
KatyaRyazantseva Nov 4, 2024
92a7453
Merge 'origin/unstable' into libp2p-metrics
KatyaRyazantseva Nov 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 57 additions & 2 deletions beacon_node/lighthouse_network/src/metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

pub static TOTAL_RPC_REQUESTS_SENT: LazyLock<Result<IntCounterVec>> = LazyLock::new(|| {
try_create_int_counter_vec(
"libp2p_rpc_requests_sent_total",
"RPC requests sent total",
&["type"],
)
});
pub static TOTAL_RPC_REQUESTS_BYTES_SENT: LazyLock<Result<IntCounterVec>> = LazyLock::new(|| {
try_create_int_counter_vec(
"libp2p_rpc_requests_bytes_sent_total",
"RPC requests bytes sent total",
&["type"],
)
});
pub static TOTAL_RPC_REQUESTS_RECEIVED: LazyLock<Result<IntCounterVec>> = LazyLock::new(|| {
try_create_int_counter_vec(
"libp2p_rpc_requests_received_total",
"RPC requests received total",
&["type"],
)
});
pub static TOTAL_RPC_REQUESTS_BYTES_RECEIVED: LazyLock<Result<IntCounterVec>> =
LazyLock::new(|| {
try_create_int_counter_vec(
"libp2p_rpc_requests_bytes_received_total",
"RPC requests bytes received total",
&["type"],
)
});
pub static TOTAL_RPC_RESPONSES_SENT: LazyLock<Result<IntCounterVec>> = LazyLock::new(|| {
try_create_int_counter_vec(
"libp2p_rpc_responses_sent_total",
"RPC responses sent total",
&["type"],
)
});
pub static TOTAL_RPC_RESPONSES_BYTES_SENT: LazyLock<Result<IntCounterVec>> = LazyLock::new(|| {
try_create_int_counter_vec(
"libp2p_rpc_responses_bytes_sent_total",
"RPC responses bytes sent total",
&["type"],
)
});
pub static TOTAL_RPC_RESPONSES_RECEIVED: LazyLock<Result<IntCounterVec>> = LazyLock::new(|| {
try_create_int_counter_vec(
"libp2p_rpc_responses_received_total",
"RPC responses received total",
&["type"],
)
});
pub static TOTAL_RPC_RESPONSES_BYTES_RECEIVED: LazyLock<Result<IntCounterVec>> =
LazyLock::new(|| {
try_create_int_counter_vec(
"libp2p_rpc_responses_bytes_received_total",
"RPC responses bytes received total",
&["type"],
)
});
pub static PEER_ACTION_EVENTS_PER_CLIENT: LazyLock<Result<IntCounterVec>> = LazyLock::new(|| {
try_create_int_counter_vec(
"libp2p_peer_actions_per_client",
Expand Down
174 changes: 131 additions & 43 deletions beacon_node/lighthouse_network/src/rpc/codec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::metrics;
use crate::rpc::methods::*;
use crate::rpc::protocol::{
Encoding, ProtocolId, RPCError, SupportedProtocol, ERROR_TYPE_MAX, ERROR_TYPE_MIN,
Expand Down Expand Up @@ -359,6 +360,11 @@ impl<E: EthSpec> Encoder<OutboundRequest<E>> for SSZSnappyOutboundCodec<E> {

// Write compressed bytes to `dst`
dst.extend_from_slice(writer.get_ref());
metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_SENT,
&[item.into()],
dst.len() as u64,
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Ok(())
}
}
Expand Down Expand Up @@ -538,68 +544,150 @@ fn handle_rpc_request<E: EthSpec>(
spec: &ChainSpec,
) -> Result<Option<InboundRequest<E>>, RPCError> {
match versioned_protocol {
SupportedProtocol::StatusV1 => Ok(Some(InboundRequest::Status(
StatusMessage::from_ssz_bytes(decoded_buffer)?,
))),
SupportedProtocol::StatusV1 => {
metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_RECEIVED,
&["status"],
decoded_buffer.len() as u64,
);
Ok(Some(InboundRequest::Status(StatusMessage::from_ssz_bytes(
decoded_buffer,
)?)))
}
SupportedProtocol::GoodbyeV1 => Ok(Some(InboundRequest::Goodbye(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I need to count Goodbye in TOTAL_RPC_REQUESTS_BYTES_RECEIVED metric?

GoodbyeReason::from_ssz_bytes(decoded_buffer)?,
))),
SupportedProtocol::BlocksByRangeV2 => Ok(Some(InboundRequest::BlocksByRange(
OldBlocksByRangeRequest::V2(OldBlocksByRangeRequestV2::from_ssz_bytes(decoded_buffer)?),
))),
SupportedProtocol::BlocksByRangeV1 => Ok(Some(InboundRequest::BlocksByRange(
OldBlocksByRangeRequest::V1(OldBlocksByRangeRequestV1::from_ssz_bytes(decoded_buffer)?),
))),
SupportedProtocol::BlocksByRootV2 => Ok(Some(InboundRequest::BlocksByRoot(
BlocksByRootRequest::V2(BlocksByRootRequestV2 {
block_roots: RuntimeVariableList::from_ssz_bytes(
SupportedProtocol::BlocksByRangeV2 => {
metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_RECEIVED,
&["blocks_by_range"],
decoded_buffer.len() as u64,
);
Ok(Some(InboundRequest::BlocksByRange(
OldBlocksByRangeRequest::V2(OldBlocksByRangeRequestV2::from_ssz_bytes(
decoded_buffer,
spec.max_request_blocks as usize,
)?,
}),
))),
SupportedProtocol::BlocksByRootV1 => Ok(Some(InboundRequest::BlocksByRoot(
BlocksByRootRequest::V1(BlocksByRootRequestV1 {
block_roots: RuntimeVariableList::from_ssz_bytes(
)?),
)))
}
SupportedProtocol::BlocksByRangeV1 => {
metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_RECEIVED,
&["blocks_by_range"],
decoded_buffer.len() as u64,
);
Ok(Some(InboundRequest::BlocksByRange(
OldBlocksByRangeRequest::V1(OldBlocksByRangeRequestV1::from_ssz_bytes(
decoded_buffer,
spec.max_request_blocks as usize,
)?,
}),
))),
SupportedProtocol::BlobsByRangeV1 => Ok(Some(InboundRequest::BlobsByRange(
BlobsByRangeRequest::from_ssz_bytes(decoded_buffer)?,
))),
)?),
)))
}
SupportedProtocol::BlocksByRootV2 => {
metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_RECEIVED,
&["blocks_by_root"],
decoded_buffer.len() as u64,
);
Ok(Some(InboundRequest::BlocksByRoot(BlocksByRootRequest::V2(
BlocksByRootRequestV2 {
block_roots: RuntimeVariableList::from_ssz_bytes(
decoded_buffer,
spec.max_request_blocks as usize,
)?,
},
))))
}
SupportedProtocol::BlocksByRootV1 => {
metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_RECEIVED,
&["blocks_by_root"],
decoded_buffer.len() as u64,
);
Ok(Some(InboundRequest::BlocksByRoot(BlocksByRootRequest::V1(
BlocksByRootRequestV1 {
block_roots: RuntimeVariableList::from_ssz_bytes(
decoded_buffer,
spec.max_request_blocks as usize,
)?,
},
))))
}
SupportedProtocol::BlobsByRangeV1 => {
metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_RECEIVED,
&["blobs_by_range"],
decoded_buffer.len() as u64,
);
Ok(Some(InboundRequest::BlobsByRange(
BlobsByRangeRequest::from_ssz_bytes(decoded_buffer)?,
)))
}
SupportedProtocol::BlobsByRootV1 => {
metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_RECEIVED,
&["blobs_by_root"],
decoded_buffer.len() as u64,
);
Ok(Some(InboundRequest::BlobsByRoot(BlobsByRootRequest {
blob_ids: RuntimeVariableList::from_ssz_bytes(
decoded_buffer,
spec.max_request_blob_sidecars as usize,
)?,
})))
}
SupportedProtocol::DataColumnsByRangeV1 => Ok(Some(InboundRequest::DataColumnsByRange(
DataColumnsByRangeRequest::from_ssz_bytes(decoded_buffer)?,
))),
SupportedProtocol::DataColumnsByRootV1 => Ok(Some(InboundRequest::DataColumnsByRoot(
DataColumnsByRootRequest {
data_column_ids: RuntimeVariableList::from_ssz_bytes(
decoded_buffer,
spec.max_request_data_column_sidecars as usize,
)?,
},
))),
SupportedProtocol::DataColumnsByRangeV1 => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of repetition in this match, you can turn SupportedProtocol into a str and have a single register metric call

metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_RECEIVED,
&["data_columns_by_range"],
decoded_buffer.len() as u64,
);
Ok(Some(InboundRequest::DataColumnsByRange(
DataColumnsByRangeRequest::from_ssz_bytes(decoded_buffer)?,
)))
}
SupportedProtocol::DataColumnsByRootV1 => {
metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_RECEIVED,
&["data_columns_by_root"],
decoded_buffer.len() as u64,
);
Ok(Some(InboundRequest::DataColumnsByRoot(
DataColumnsByRootRequest {
data_column_ids: RuntimeVariableList::from_ssz_bytes(
decoded_buffer,
spec.max_request_data_column_sidecars as usize,
)?,
},
)))
}
SupportedProtocol::PingV1 => Ok(Some(InboundRequest::Ping(Ping {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I need to count Ping in TOTAL_RPC_REQUESTS_BYTES_RECEIVED metric?

data: u64::from_ssz_bytes(decoded_buffer)?,
}))),
SupportedProtocol::LightClientBootstrapV1 => Ok(Some(
InboundRequest::LightClientBootstrap(LightClientBootstrapRequest {
root: Hash256::from_ssz_bytes(decoded_buffer)?,
}),
)),
SupportedProtocol::LightClientBootstrapV1 => {
metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_RECEIVED,
&["light_client_bootstrap"],
decoded_buffer.len() as u64,
);
Ok(Some(InboundRequest::LightClientBootstrap(
LightClientBootstrapRequest {
root: Hash256::from_ssz_bytes(decoded_buffer)?,
},
)))
}
SupportedProtocol::LightClientOptimisticUpdateV1 => {
metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_RECEIVED,
&["light_client_optimistic_update"],
decoded_buffer.len() as u64,
);
Ok(Some(InboundRequest::LightClientOptimisticUpdate))
}
SupportedProtocol::LightClientFinalityUpdateV1 => {
metrics::inc_counter_vec_by(
&metrics::TOTAL_RPC_REQUESTS_BYTES_RECEIVED,
&["light_client_finality_update"],
decoded_buffer.len() as u64,
);
Ok(Some(InboundRequest::LightClientFinalityUpdate))
}
// MetaData requests return early from InboundUpgrade and do not reach the decoder.
Expand Down Expand Up @@ -636,7 +724,7 @@ fn handle_rpc_request<E: EthSpec>(
/// `decoded_buffer` should be an ssz-encoded bytestream with
/// length = length-prefix received in the beginning of the stream.
///
/// For BlocksByRange/BlocksByRoot reponses, decodes the appropriate response
/// For BlocksByRange/BlocksByRoot responses, decodes the appropriate response
/// according to the received `ForkName`.
fn handle_rpc_response<E: EthSpec>(
versioned_protocol: SupportedProtocol,
Expand Down
41 changes: 6 additions & 35 deletions beacon_node/lighthouse_network/src/service/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -944,8 +944,9 @@ impl<E: EthSpec> Network<E> {
self.eth2_rpc_mut().send_request(
peer_id,
RequestId::Application(request_id),
request.into(),
request.clone().into(),
);
metrics::inc_counter_vec(&metrics::TOTAL_RPC_REQUESTS_SENT, &[request.into()]);
Ok(())
}

Expand Down Expand Up @@ -1187,40 +1188,10 @@ impl<E: EthSpec> Network<E> {
request: Request,
) -> NetworkEvent<E> {
// Increment metrics
match &request {
Request::Status(_) => {
metrics::inc_counter_vec(&metrics::TOTAL_RPC_REQUESTS, &["status"])
}
Request::LightClientBootstrap(_) => {
metrics::inc_counter_vec(&metrics::TOTAL_RPC_REQUESTS, &["light_client_bootstrap"])
}
Request::LightClientOptimisticUpdate => metrics::inc_counter_vec(
&metrics::TOTAL_RPC_REQUESTS,
&["light_client_optimistic_update"],
),
Request::LightClientFinalityUpdate => metrics::inc_counter_vec(
&metrics::TOTAL_RPC_REQUESTS,
&["light_client_finality_update"],
),
Request::BlocksByRange { .. } => {
metrics::inc_counter_vec(&metrics::TOTAL_RPC_REQUESTS, &["blocks_by_range"])
}
Request::BlocksByRoot { .. } => {
metrics::inc_counter_vec(&metrics::TOTAL_RPC_REQUESTS, &["blocks_by_root"])
}
Request::BlobsByRange { .. } => {
metrics::inc_counter_vec(&metrics::TOTAL_RPC_REQUESTS, &["blobs_by_range"])
}
Request::BlobsByRoot { .. } => {
metrics::inc_counter_vec(&metrics::TOTAL_RPC_REQUESTS, &["blobs_by_root"])
}
Request::DataColumnsByRoot { .. } => {
metrics::inc_counter_vec(&metrics::TOTAL_RPC_REQUESTS, &["data_columns_by_root"])
}
Request::DataColumnsByRange { .. } => {
metrics::inc_counter_vec(&metrics::TOTAL_RPC_REQUESTS, &["data_columns_by_range"])
}
}
metrics::inc_counter_vec(
&metrics::TOTAL_RPC_REQUESTS_RECEIVED,
&[request.clone().into()],
);
NetworkEvent::RequestReceived {
peer_id,
id,
Expand Down