feat(pocket-ic): support custom blockmaker metrics at each round #3685
Conversation
Co-authored-by: mraszyk <[email protected]>
pietrodimarco-dfinity
changed the title
What does this even mean? PocketIC does not include IC's consensus, so I'm likely missing some context here. |
Currently PocketIC at each tick() executes a round with The metrics_collector_canister I am building store and serve node metrics from all the subnet. To test this canister I need then to modify PocketIC so that at each "round" it stores blockmaker metrics that I can pass in from the test |
pietrodimarco-dfinity
changed the title
PocketIC doesn't include production consensus, but its blocks must still include stats stored in the replicated state and served by the management canister via the node metrics history endpoint. These stats are currently mocked and this PR allows to override the mocked value for testing the node metrics history endpoint. |
|
Thanks for the answers, so I think a small edit of the title to "support custom blockmaker stats at each round" would immediately make it more clear. |
pietrodimarco-dfinity
changed the title
| @@ -1335,7 +1335,7 @@ impl StateMachine { | |||
| /// and execute a round with this payload. | |||
| /// Note that only ingress messages submitted via `Self::submit_ingress` | |||
| /// will be considered during payload building. | |||
| pub fn execute_round(&self) { | |||
| pub fn execute_round(&self, blockmaker_metrics: Option<BlockmakerMetrics>) { | |||
There was a problem hiding this comment.
Could you please rename this function to fn execute_round_impl and introduce a pair of public functions:
pub fn execute_round(&self) // calls execute_round_impl(None)
pub fn execute_round_with_blockmaker_metrics(&self, blockmaker_metrics: BlockmakerMetrics)
this way, there's no breaking change to the existing function
| // management canister calls | ||
|
|
||
| #[update] | ||
| async fn node_metrics_history_proxy( |
There was a problem hiding this comment.
Could you please move this to the existing test canister under //packages/pocket-ic and use that test canister instead of creating a new one?
| @@ -1101,9 +1101,10 @@ pub async fn handler_tick( | |||
| State(AppState { api_state, .. }): State<AppState>, | |||
| Path(instance_id): Path<InstanceId>, | |||
| headers: HeaderMap, | |||
| axum::extract::Json(raw): axum::extract::Json<TickConfigs>, | |||
There was a problem hiding this comment.
| axum::extract::Json(raw): axum::extract::Json<TickConfigs>, | |
| axum::extract::Json(ticks_configs): axum::extract::Json<TickConfigs>, |
| @@ -1,4 +1,5 @@ | |||
| #![allow(clippy::test_attr_in_doctest)] | |||
| use crate::common::rest::TickConfigs; | |||
There was a problem hiding this comment.
let's please move this closed to the remaining rest types
| let endpoint = "update/tick"; | ||
| self.post::<(), _>(endpoint, "").await; | ||
| self.post::<(), _>(endpoint, TickConfigs::default()).await; |
There was a problem hiding this comment.
self.tick_with_configs(TickConfigs::default()).await;
| } | ||
|
|
||
| #[test] | ||
| fn test_custom_blockmaker() { |
There was a problem hiding this comment.
let's move this test to //packages/pocket-ic and extend the test canister there; please use candid::Principal instead of ic_types::PrincipalId and mgmt canister types from pocket_ic::management_canister_types
Currently PocketIC at each tick() executes a round with state_machine_tests. However a dummy blockmaker is used.
The metrics_collector_canister I am building store and serve node metrics from all the subnet. To test this canister I need then to modify PocketIC so that at each "round" it stores blockmaker metrics that I can pass in from tests