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.

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

feat(nns/sns): Add query stats to canister status #3710

Merged
merged 3 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
109 changes: 96 additions & 13 deletions rs/nervous_system/clients/src/canister_status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ pub enum LogVisibility {
AllowedViewers(Vec<PrincipalId>),
}

/// Partial copy-paste of ic-types::ic_00::DefiniteCanisterSettings.
/// Partial copy-paste of `ic_management_canister_types::DefiniteCanisterSettings`, and it's used
/// for the response type in the NNS/SNS Root `canister_status` method.
///
/// Only the fields that we need are copied.
/// Candid deserialization is supposed to be tolerant to having data for unknown
/// fields (which is simply discarded).
/// Only the fields that we need are copied. Candid deserialization is supposed to be tolerant to
/// having data for unknown fields (which is simply discarded).
#[derive(Clone, Eq, PartialEq, Debug, CandidType, Deserialize)]
pub struct DefiniteCanisterSettings {
pub controllers: Vec<PrincipalId>,
Expand All @@ -71,11 +71,11 @@ pub struct DefiniteCanisterSettings {
pub wasm_memory_threshold: Option<candid::Nat>,
}

/// Partial copy-paste of ic-types::ic_00::CanisterStatusResult.
/// Partial copy-paste of `ic_management_canister_types::CanisterStatusResultV2`, and it's used for
/// the response type in the NNS/SNS Root `canister_status` method.
///
/// Only the fields that we need are copied.
/// Candid deserialization is supposed to be tolerant to having data for unknown
/// fields (which are simply discarded).
/// Only the fields that we need are copied. Candid deserialization is supposed to be tolerant to
/// having data for unknown fields (which is simply discarded).
#[derive(Clone, Eq, PartialEq, Debug, CandidType, Deserialize)]
pub struct CanisterStatusResult {
pub status: CanisterStatusType,
Expand All @@ -86,9 +86,24 @@ pub struct CanisterStatusResult {
pub cycles: candid::Nat,
pub idle_cycles_burned_per_day: Option<candid::Nat>,
pub reserved_cycles: Option<candid::Nat>,
pub query_stats: Option<QueryStats>,
}

/// Copy-paste of `ic_management_canister_types::CanisterStatusResultV2`.
/// Partial copy-paste of `ic_management_canister_types::QueryStats`, and it's used for the response
/// type in the NNS/SNS Root `canister_status` method.
///
/// Only the fields that we need are copied. Candid deserialization is supposed to be tolerant to
/// having data for unknown fields (which is simply discarded).
#[derive(Clone, Eq, PartialEq, Debug, CandidType, Deserialize)]
pub struct QueryStats {
pub num_calls_total: Option<candid::Nat>,
pub num_instructions_total: Option<candid::Nat>,
pub request_payload_bytes_total: Option<candid::Nat>,
pub response_payload_bytes_total: Option<candid::Nat>,
}

/// Copy-paste of `ic_management_canister_types::CanisterStatusResultV2`, and it's used for the
/// `canister_status`` method on the management canister.
#[derive(Clone, Eq, PartialEq, Debug, Default, CandidType, Deserialize)]
pub struct CanisterStatusResultFromManagementCanister {
pub status: CanisterStatusType,
Expand All @@ -98,13 +113,14 @@ pub struct CanisterStatusResultFromManagementCanister {
pub cycles: candid::Nat,
pub idle_cycles_burned_per_day: candid::Nat,
pub reserved_cycles: candid::Nat,
pub query_stats: QueryStatsFromManagementCanister,
}

/// Partial copy-paste of `ic_management_canister_types::DefiniteCanisterSettingsArgs`.
/// Partial copy-paste of `ic_management_canister_types::DefiniteCanisterSettingsArgs`, and it's
/// used for the response type in the management canister `canister_status` method.
///
/// Only the fields that we need are copied.
/// Candid deserialization is supposed to be tolerant to having data for unknown
/// fields (which is simply discarded).
/// Only the fields that we need are copied. Candid deserialization is supposed to be tolerant to
/// having data for unknown fields (which is simply discarded).
#[derive(Clone, Eq, PartialEq, Debug, Default, CandidType, Deserialize)]
pub struct DefiniteCanisterSettingsFromManagementCanister {
pub controllers: Vec<PrincipalId>,
Expand All @@ -117,6 +133,16 @@ pub struct DefiniteCanisterSettingsFromManagementCanister {
pub wasm_memory_threshold: candid::Nat,
}

/// Partial copy-paste of `ic_management_canister_types::QueryStats`, and it's used for the response
/// type in the management canister `canister_status` method.
#[derive(Clone, Eq, PartialEq, Debug, Default, CandidType, Deserialize)]
pub struct QueryStatsFromManagementCanister {
pub num_calls_total: candid::Nat,
pub num_instructions_total: candid::Nat,
pub request_payload_bytes_total: candid::Nat,
pub response_payload_bytes_total: candid::Nat,
}

impl From<CanisterStatusResultFromManagementCanister> for CanisterStatusResult {
fn from(value: CanisterStatusResultFromManagementCanister) -> Self {
let CanisterStatusResultFromManagementCanister {
Expand All @@ -127,9 +153,11 @@ impl From<CanisterStatusResultFromManagementCanister> for CanisterStatusResult {
cycles,
idle_cycles_burned_per_day,
reserved_cycles,
query_stats,
} = value;

let settings = DefiniteCanisterSettings::from(settings);
let query_stats = Some(QueryStats::from(query_stats));

let idle_cycles_burned_per_day = Some(idle_cycles_burned_per_day);
let reserved_cycles = Some(reserved_cycles);
Expand All @@ -142,6 +170,7 @@ impl From<CanisterStatusResultFromManagementCanister> for CanisterStatusResult {
cycles,
idle_cycles_burned_per_day,
reserved_cycles,
query_stats,
}
}
}
Expand Down Expand Up @@ -180,6 +209,29 @@ impl From<DefiniteCanisterSettingsFromManagementCanister> for DefiniteCanisterSe
}
}

impl From<QueryStatsFromManagementCanister> for QueryStats {
fn from(value: QueryStatsFromManagementCanister) -> Self {
let QueryStatsFromManagementCanister {
num_calls_total,
num_instructions_total,
request_payload_bytes_total,
response_payload_bytes_total,
} = value;

let num_calls_total = Some(num_calls_total);
let num_instructions_total = Some(num_instructions_total);
let request_payload_bytes_total = Some(request_payload_bytes_total);
let response_payload_bytes_total = Some(response_payload_bytes_total);

QueryStats {
num_calls_total,
num_instructions_total,
request_payload_bytes_total,
response_payload_bytes_total,
}
}
}

impl CanisterStatusResultFromManagementCanister {
pub fn controllers(&self) -> &[PrincipalId] {
self.settings.controllers.as_slice()
Expand All @@ -202,6 +254,12 @@ impl CanisterStatusResultFromManagementCanister {
log_visibility: LogVisibility::Controllers,
wasm_memory_threshold: candid::Nat::from(49_u32),
},
query_stats: QueryStatsFromManagementCanister {
num_calls_total: candid::Nat::from(50_u32),
num_instructions_total: candid::Nat::from(51_u32),
request_payload_bytes_total: candid::Nat::from(52_u32),
response_payload_bytes_total: candid::Nat::from(53_u32),
},
cycles: candid::Nat::from(47_u32),
idle_cycles_burned_per_day: candid::Nat::from(48_u32),
reserved_cycles: candid::Nat::from(49_u32),
Expand Down Expand Up @@ -230,6 +288,7 @@ pub struct CanisterStatusResultV2 {
pub cycles: candid::Nat,
// this is for compat with Spec 0.12/0.13
pub idle_cycles_burned_per_day: candid::Nat,
pub query_stats: Option<QueryStats>,
}

impl CanisterStatusResultV2 {
Expand Down Expand Up @@ -263,6 +322,12 @@ impl CanisterStatusResultV2 {
Some(wasm_memory_threshold),
),
idle_cycles_burned_per_day: candid::Nat::from(idle_cycles_burned_per_day),
query_stats: Some(QueryStats {
num_calls_total: Some(candid::Nat::from(0_u64)),
num_instructions_total: Some(candid::Nat::from(0_u64)),
request_payload_bytes_total: Some(candid::Nat::from(0_u64)),
response_payload_bytes_total: Some(candid::Nat::from(0_u64)),
}),
}
}

Expand Down Expand Up @@ -403,6 +468,12 @@ impl From<CanisterStatusResultFromManagementCanister> for CanisterStatusResultV2
memory_size: value.memory_size,
cycles: value.cycles,
idle_cycles_burned_per_day: value.idle_cycles_burned_per_day,
query_stats: Some(QueryStats {
num_calls_total: Some(value.query_stats.num_calls_total),
num_instructions_total: Some(value.query_stats.num_instructions_total),
request_payload_bytes_total: Some(value.query_stats.request_payload_bytes_total),
response_payload_bytes_total: Some(value.query_stats.response_payload_bytes_total),
}),
}
}
}
Expand Down Expand Up @@ -438,6 +509,12 @@ mod tests {
cycles: candid::Nat::from(999_u32),
idle_cycles_burned_per_day: candid::Nat::from(998_u32),
reserved_cycles: candid::Nat::from(997_u32),
query_stats: QueryStatsFromManagementCanister {
num_calls_total: candid::Nat::from(93_u32),
num_instructions_total: candid::Nat::from(92_u32),
request_payload_bytes_total: candid::Nat::from(91_u32),
response_payload_bytes_total: candid::Nat::from(90_u32),
},
};

let expected_canister_status_result = CanisterStatusResult {
Expand All @@ -457,6 +534,12 @@ mod tests {
cycles: candid::Nat::from(999_u32),
idle_cycles_burned_per_day: Some(candid::Nat::from(998_u32)),
reserved_cycles: Some(candid::Nat::from(997_u32)),
query_stats: Some(QueryStats {
num_calls_total: Some(candid::Nat::from(93_u32)),
num_instructions_total: Some(candid::Nat::from(92_u32)),
request_payload_bytes_total: Some(candid::Nat::from(91_u32)),
response_payload_bytes_total: Some(candid::Nat::from(90_u32)),
}),
};

let actual_canister_status_result = CanisterStatusResult::from(m);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::*;
use crate::canister_status::{
CanisterStatusType, DefiniteCanisterSettingsFromManagementCanister, LogVisibility,
QueryStatsFromManagementCanister,
};
use candid::Nat;
use ic_base_types::{CanisterId, PrincipalId};
Expand Down Expand Up @@ -119,6 +120,12 @@ async fn test_limit_outstanding_calls() {
},
status: CanisterStatusType::Running,
reserved_cycles: zero.clone(),
query_stats: QueryStatsFromManagementCanister {
num_calls_total: zero.clone(),
num_instructions_total: zero.clone(),
request_payload_bytes_total: zero.clone(),
response_payload_bytes_total: zero.clone(),
},
};

// Step 2: Call code under test.
Expand Down
8 changes: 8 additions & 0 deletions rs/nns/handlers/root/impl/canister/root.did
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type CanisterStatusResult = record {
idle_cycles_burned_per_day : opt nat;
module_hash : opt blob;
reserved_cycles : opt nat;
query_stats : opt QueryStats;
};

type CanisterStatusType = variant {
Expand Down Expand Up @@ -107,6 +108,13 @@ type CanisterStatusLogVisibility = variant {
allowed_viewers : vec principal;
};

type QueryStats = record {
num_calls_total : opt nat;
num_instructions_total : opt nat;
request_payload_bytes_total : opt nat;
response_payload_bytes_total : opt nat;
};

type StopOrStartCanisterRequest = record {
action : CanisterAction;
canister_id : principal;
Expand Down
7 changes: 7 additions & 0 deletions rs/nns/handlers/root/interface/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use ic_nervous_system_clients::{
canister_id_record::CanisterIdRecord,
canister_status::{
CanisterStatusResult, CanisterStatusType, DefiniteCanisterSettings, LogVisibility,
QueryStats,
},
};
use ic_nns_constants::ROOT_CANISTER_ID;
Expand Down Expand Up @@ -226,6 +227,12 @@ impl SpyNnsRootCanisterClientReply {
cycles: candid::Nat::from(42_u32),
idle_cycles_burned_per_day: Some(candid::Nat::from(43_u32)),
reserved_cycles: Some(candid::Nat::from(44_u32)),
query_stats: Some(QueryStats {
num_calls_total: Some(candid::Nat::from(45_u32)),
num_instructions_total: Some(candid::Nat::from(46_u32)),
request_payload_bytes_total: Some(candid::Nat::from(47_u32)),
response_payload_bytes_total: Some(candid::Nat::from(48_u32)),
}),
}))
}

Expand Down
4 changes: 3 additions & 1 deletion rs/nns/handlers/root/unreleased_changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ on the process that this file is part of, see

## Added

* Added the `query_stats` field for the `canister_status` method.

## Changed

- The `LogVisibility` returned from `canister_status` has one more variant `allowed_viewers`,
* The `LogVisibility` returned from `canister_status` has one more variant `allowed_viewers`,
consistent with the corresponding management canister API. Calling `canister_status` for a
canister with such a log visibility setting will no longer panic.

Expand Down
8 changes: 8 additions & 0 deletions rs/sns/governance/canister/governance.did
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type CanisterStatusResultV2 = record {
settings : DefiniteCanisterSettingsArgs;
idle_cycles_burned_per_day : nat;
module_hash : opt blob;
query_stats : opt QueryStats;
};

type CanisterStatusType = variant {
Expand Down Expand Up @@ -610,6 +611,13 @@ type ProposalId = record {
id : nat64;
};

type QueryStats = record {
num_calls_total : opt nat;
num_instructions_total : opt nat;
request_payload_bytes_total : opt nat;
response_payload_bytes_total : opt nat;
};

type RegisterDappCanisters = record {
canister_ids : vec principal;
};
Expand Down
8 changes: 8 additions & 0 deletions rs/sns/governance/canister/governance_test.did
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ type CanisterStatusResultV2 = record {
settings : DefiniteCanisterSettingsArgs;
idle_cycles_burned_per_day : nat;
module_hash : opt blob;
query_stats : opt QueryStats;
};

type CanisterStatusType = variant {
Expand Down Expand Up @@ -624,6 +625,13 @@ type ProposalId = record {
id : nat64;
};

type QueryStats = record {
num_calls_total : opt nat;
num_instructions_total : opt nat;
request_payload_bytes_total : opt nat;
response_payload_bytes_total : opt nat;
};

type RegisterDappCanisters = record {
canister_ids : vec principal;
};
Expand Down
1 change: 1 addition & 0 deletions rs/sns/governance/unreleased_changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ proposal, e.g.:
```

* Do not redact chunked Wasm data in `ProposalInfo` served from `SnsGov.list_proposals`.
* Added the `query_stats` field for `canister_status`/`get_sns_canisters_summary` methods.

## Changed

Expand Down
9 changes: 9 additions & 0 deletions rs/sns/root/canister/root.did
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type CanisterStatusResult = record {
idle_cycles_burned_per_day : opt nat;
module_hash : opt blob;
reserved_cycles : opt nat;
query_stats : opt QueryStats;
};

type CanisterStatusResultV2 = record {
Expand All @@ -30,6 +31,7 @@ type CanisterStatusResultV2 = record {
settings : DefiniteCanisterSettingsArgs;
idle_cycles_burned_per_day : nat;
module_hash : opt blob;
query_stats : opt QueryStats;
};

type CanisterStatusType = variant {
Expand Down Expand Up @@ -80,6 +82,13 @@ type DefiniteCanisterSettingsArgs = record {
wasm_memory_threshold : opt nat;
};

type QueryStats = record {
num_calls_total : opt nat;
num_instructions_total : opt nat;
request_payload_bytes_total : opt nat;
response_payload_bytes_total : opt nat;
};

type FailedUpdate = record {
err : opt CanisterCallError;
dapp_canister_id : opt principal;
Expand Down
4 changes: 3 additions & 1 deletion rs/sns/root/unreleased_changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ on the process that this file is part of, see

## Added

* Added the `query_stats` field for `canister_status`/`get_sns_canisters_summary` methods.

## Changed

- The `LogVisibility` returned from `canister_status` has one more variant `allowed_viewers`,
* The `LogVisibility` returned from `canister_status` has one more variant `allowed_viewers`,
consistent with the corresponding management canister API. Calling `canister_status` for a
canister with such a log visibility setting will no longer panic.

Expand Down
2 changes: 1 addition & 1 deletion rs/sns/sns.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ but other things could be added here later.
CANISTER_NAME_TO_MAX_COMPRESSED_WASM_SIZE_E5_BYTES = {
"sns-governance-canister.wasm.gz": "13",
"sns-governance-canister_test.wasm.gz": "13",
"sns-root-canister.wasm.gz": "4",
"sns-root-canister.wasm.gz": "5",
"sns-swap-canister.wasm.gz": "7",
}
Loading