diff --git a/Cargo.lock b/Cargo.lock index 055bfd21060..af6b9d7f2c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -18143,8 +18143,9 @@ version = "0.9.0" dependencies = [ "anyhow", "candid", + "candid_parser", "getrandom", - "hex", + "ic-canister-log 0.2.0", "ic-canisters-http-types", "ic-cdk 0.16.0", "ic-cdk-macros 0.9.0", diff --git a/rs/boundary_node/ic_boundary/src/rate_limiting/fetcher.rs b/rs/boundary_node/ic_boundary/src/rate_limiting/fetcher.rs index 453e3c01618..983c95569d4 100644 --- a/rs/boundary_node/ic_boundary/src/rate_limiting/fetcher.rs +++ b/rs/boundary_node/ic_boundary/src/rate_limiting/fetcher.rs @@ -130,13 +130,13 @@ impl FetchesRules for CanisterFetcher { let Some(raw) = x.rule_raw else { return Err(anyhow!( "rule with id {} ({:?}) is None", - x.id, + x.rule_id, x.description )); }; let rule = RateLimitRule::from_bytes_yaml(&raw) - .context(format!("unable to decode raw rule with id {}", x.id))?; + .context(format!("unable to decode raw rule with id {}", x.rule_id))?; Ok(rule) }) @@ -172,7 +172,7 @@ mod test { is_redacted: false, rules: vec![ OutputRule { - id: "foobar".into(), + rule_id: "foobar".into(), incident_id: "barfoo".into(), rule_raw: Some(indoc! {" canister_id: aaaaa-aa @@ -183,7 +183,7 @@ mod test { description: None }, OutputRule { - id: "foobaz".into(), + rule_id: "foobaz".into(), incident_id: "barfoo".into(), rule_raw: Some(indoc! {" canister_id: 5s2ji-faaaa-aaaaa-qaaaq-cai @@ -194,7 +194,7 @@ mod test { description: None }, OutputRule { - id: "deadbeef".into(), + rule_id: "deadbeef".into(), incident_id: "barfoo".into(), rule_raw: Some(indoc! {" canister_id: aaaaa-aa @@ -242,7 +242,7 @@ mod test { schema_version: SCHEMA_VERSION, is_redacted: false, rules: vec![OutputRule { - id: "foobar".into(), + rule_id: "foobar".into(), incident_id: "barfoo".into(), rule_raw: None, description: None, diff --git a/rs/boundary_node/rate_limits/BUILD.bazel b/rs/boundary_node/rate_limits/BUILD.bazel index 51c9bcadcb1..a9bb1930784 100644 --- a/rs/boundary_node/rate_limits/BUILD.bazel +++ b/rs/boundary_node/rate_limits/BUILD.bazel @@ -7,14 +7,13 @@ DEPENDENCIES = [ # Keep sorted. "//rs/boundary_node/rate_limits/api:rate_limits_api", "//rs/nns/constants", + "//rs/rust_canisters/canister_log", "//rs/rust_canisters/http_types", "@crate_index//:anyhow", "@crate_index//:candid", "@crate_index//:getrandom", - "@crate_index//:hex", "@crate_index//:ic-cdk", "@crate_index//:ic-cdk-timers", - "@crate_index//:ic-metrics-encoder", "@crate_index//:ic-stable-structures", "@crate_index//:mockall", "@crate_index//:prometheus", @@ -47,6 +46,7 @@ rust_test( srcs = glob(["canister/**/*.rs"]), crate_name = "rate_limit_canister", crate_root = "canister/lib.rs", + data = ["canister/interface.did"], proc_macro_deps = MACRO_DEPENDENCIES, - deps = DEPENDENCIES, + deps = DEPENDENCIES + ["@crate_index//:candid_parser"], ) diff --git a/rs/boundary_node/rate_limits/Cargo.toml b/rs/boundary_node/rate_limits/Cargo.toml index b88fca8cf01..247157ec494 100644 --- a/rs/boundary_node/rate_limits/Cargo.toml +++ b/rs/boundary_node/rate_limits/Cargo.toml @@ -10,8 +10,8 @@ documentation.workspace = true anyhow = { workspace = true } candid = { workspace = true } getrandom = { version = "0.2", features = ["custom"] } -hex = { workspace = true } ic-canisters-http-types = { path = "../../rust_canisters/http_types" } +ic-canister-log = { path = "../../rust_canisters/canister_log" } ic-cdk = { workspace = true } ic-cdk-macros = { workspace = true } ic-cdk-timers = { workspace = true } @@ -29,6 +29,7 @@ thiserror = { workspace = true } uuid = { workspace = true, features = ['serde', 'v4'] } [dev-dependencies] +candid_parser = { workspace = true } rate-limit-canister-integration-tests = { path = "./integration_tests" } [lib] diff --git a/rs/boundary_node/rate_limits/api/src/lib.rs b/rs/boundary_node/rate_limits/api/src/lib.rs index 0a9dc300efd..9f79729f01f 100644 --- a/rs/boundary_node/rate_limits/api/src/lib.rs +++ b/rs/boundary_node/rate_limits/api/src/lib.rs @@ -108,7 +108,7 @@ pub struct InputRule { #[derive(CandidType, Deserialize, Debug, PartialEq)] pub struct OutputRule { - pub id: RuleId, + pub rule_id: RuleId, pub incident_id: IncidentId, pub rule_raw: Option>, pub description: Option, @@ -116,7 +116,7 @@ pub struct OutputRule { #[derive(CandidType, Deserialize, Debug, PartialEq)] pub struct OutputRuleMetadata { - pub id: RuleId, + pub rule_id: RuleId, pub incident_id: IncidentId, pub rule_raw: Option>, pub description: Option, @@ -158,7 +158,7 @@ impl std::fmt::Display for OutputConfig { writeln!(f, "{INDENT}Is redacted: {}", self.is_redacted)?; for (i, rule) in self.rules.iter().enumerate() { writeln!(f, "{DOUBLE_INDENT}Rule {}:", i + 1)?; - writeln!(f, "{DOUBLE_INDENT}ID: {}", rule.id)?; + writeln!(f, "{DOUBLE_INDENT}ID: {}", rule.rule_id)?; writeln!(f, "{DOUBLE_INDENT}Incident ID: {}", rule.incident_id)?; if let Some(ref description) = rule.description { writeln!(f, "{DOUBLE_INDENT}Description: {description}")?; @@ -175,7 +175,7 @@ impl std::fmt::Display for OutputConfig { impl std::fmt::Display for OutputRuleMetadata { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { writeln!(f, "\nOutputRuleMetadata")?; - writeln!(f, "{INDENT}ID: {}", self.id)?; + writeln!(f, "{INDENT}ID: {}", self.rule_id)?; writeln!( f, "{INDENT}Disclosed at: {}", diff --git a/rs/boundary_node/rate_limits/canister/add_config.rs b/rs/boundary_node/rate_limits/canister/add_config.rs index 6450b9f0767..35e201c68f9 100644 --- a/rs/boundary_node/rate_limits/canister/add_config.rs +++ b/rs/boundary_node/rate_limits/canister/add_config.rs @@ -90,7 +90,11 @@ impl AddsConfig for ConfigAdder { AddConfigError::Internal(anyhow!("No config for version={current_version} found")) })?; - let next_version = current_version + 1; + let next_version = current_version.checked_add(1).ok_or_else(|| { + AddConfigError::Internal(anyhow!( + "Overflow occurred while incrementing the current version {current_version}" + )) + })?; // Ordered IDs of all rules in the submitted config let mut rule_ids = Vec::::new(); diff --git a/rs/boundary_node/rate_limits/canister/canister.rs b/rs/boundary_node/rate_limits/canister/canister.rs index c8648f09c16..3c53e1b89ac 100644 --- a/rs/boundary_node/rate_limits/canister/canister.rs +++ b/rs/boundary_node/rate_limits/canister/canister.rs @@ -5,12 +5,13 @@ use crate::confidentiality_formatting::{ }; use crate::disclose::{DisclosesRules, RulesDiscloser}; use crate::getter::{ConfigGetter, EntityGetter, IncidentGetter, RuleGetter}; +use crate::logs::{self, Log, LogEntry, Priority, P0}; use crate::metrics::{ - export_metrics_as_http_response, with_metrics_registry, WithMetrics, LAST_CANISTER_CHANGE_TIME, - LAST_SUCCESSFUL_REGISTRY_POLL_TIME, REGISTRY_POLL_CALLS_COUNTER, + export_metrics_as_http_response, with_metrics_registry, WithMetrics, METRICS, }; use crate::state::{init_version_and_config, with_canister_state, CanisterApi}; use candid::Principal; +use ic_canister_log::{export as export_logs, log}; use ic_canisters_http_types::{HttpRequest, HttpResponse, HttpResponseBuilder}; use ic_cdk::api::call::call; use ic_cdk_macros::{init, inspect_message, post_upgrade, query, update}; @@ -20,7 +21,7 @@ use rate_limits_api::{ GetApiBoundaryNodeIdsRequest, GetConfigResponse, GetRuleByIdResponse, GetRulesByIncidentIdResponse, IncidentId, InitArg, InputConfig, RuleId, Version, }; -use std::{sync::Arc, time::Duration}; +use std::{borrow::BorrowMut, str::FromStr, sync::Arc, time::Duration}; const REGISTRY_CANISTER_METHOD: &str = "get_api_boundary_node_ids"; const UPDATE_METHODS: [&str; 2] = ["add_config", "disclose_rules"]; @@ -84,8 +85,11 @@ fn init(init_arg: InitArg) { ); }); // Update metric. - LAST_CANISTER_CHANGE_TIME.with(|cell| { - cell.borrow_mut().set(current_time as i64); + METRICS.with(|cell| { + let mut cell = cell.borrow_mut(); + cell.last_canister_change_time + .borrow_mut() + .set(current_time as i64); }); } @@ -146,14 +150,14 @@ fn get_rules_by_incident_id(incident_id: IncidentId) -> GetRulesByIncidentIdResp fn add_config(config: InputConfig) -> AddConfigResponse { let caller_id = ic_cdk::api::caller(); let current_time = ic_cdk::api::time(); - let result = with_canister_state(|state| { + with_canister_state(|state| { let access_resolver = AccessLevelResolver::new(caller_id, state.clone()); let adder = ConfigAdder::new(state); let adder = WithAuthorization::new(adder, access_resolver); let adder = WithMetrics::new(adder); adder.add_config(config, current_time) })?; - Ok(result) + Ok(()) } /// Makes specified rules publicly accessible for viewing @@ -164,14 +168,14 @@ fn add_config(config: InputConfig) -> AddConfigResponse { fn disclose_rules(args: DiscloseRulesArg) -> DiscloseRulesResponse { let caller_id = ic_cdk::api::caller(); let disclose_time = ic_cdk::api::time(); - let result = with_canister_state(|state| { + with_canister_state(|state| { let access_resolver = AccessLevelResolver::new(caller_id, state.clone()); let discloser = RulesDiscloser::new(state); let discloser = WithAuthorization::new(discloser, access_resolver); let discloser = WithMetrics::new(discloser); discloser.disclose_rules(args, disclose_time) })?; - Ok(result) + Ok(()) } #[query(decoding_quota = 10000)] @@ -180,6 +184,50 @@ fn http_request(request: HttpRequest) -> HttpResponse { "/metrics" => with_canister_state(|state| { with_metrics_registry(|registry| export_metrics_as_http_response(registry, state)) }), + "/logs" => { + use serde_json; + + let max_skip_timestamp = match request.raw_query_param("time") { + Some(arg) => match u64::from_str(arg) { + Ok(value) => value, + Err(_) => { + return HttpResponseBuilder::bad_request() + .with_body_and_content_length("failed to parse the 'time' parameter") + .build() + } + }, + None => 0, + }; + + let mut entries: Log = Default::default(); + for entry in export_logs(&logs::P0) { + entries.entries.push(LogEntry { + timestamp: entry.timestamp, + counter: entry.counter, + priority: Priority::P0, + file: entry.file.to_string(), + line: entry.line, + message: entry.message, + }); + } + for entry in export_logs(&logs::P1) { + entries.entries.push(LogEntry { + timestamp: entry.timestamp, + counter: entry.counter, + priority: Priority::P1, + file: entry.file.to_string(), + line: entry.line, + message: entry.message, + }); + } + entries + .entries + .retain(|entry| entry.timestamp >= max_skip_timestamp); + HttpResponseBuilder::ok() + .header("Content-Type", "application/json; charset=utf-8") + .with_body_and_content_length(serde_json::to_string(&entries).unwrap_or_default()) + .build() + } _ => HttpResponseBuilder::not_found().build(), } } @@ -205,22 +253,98 @@ fn periodically_poll_api_boundary_nodes(interval: u64, canister_api: Arc ("failure", "calling_canister_method_failed"), - Err(_) => ("failure", "canister_call_rejected"), + Ok((Err(err),)) => { + log!( + P0, + "[poll_api_boundary_nodes]: failed to fetch nodes from registry {err:?}", + ); + ("failure", "calling_canister_method_failed") + } + Err(err) => { + log!( + P0, + "[poll_api_boundary_nodes]: failed to fetch nodes from registry {err:?}", + ); + ("failure", "canister_call_rejected") + } }; // Update metric. - REGISTRY_POLL_CALLS_COUNTER.with(|cell| { - let metric = cell.borrow_mut(); - metric.with_label_values(&[call_status, message]).inc(); + METRICS.with(|cell| { + let mut cell = cell.borrow_mut(); + cell.registry_poll_calls + .borrow_mut() + .with_label_values(&[call_status, message]) + .inc(); }); }); }); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn check_candid_interface_compatibility() { + use candid_parser::utils::{service_equal, CandidSource}; + + fn source_to_str(source: &CandidSource) -> String { + match source { + CandidSource::File(f) => { + std::fs::read_to_string(f).unwrap_or_else(|_| "".to_string()) + } + CandidSource::Text(t) => t.to_string(), + } + } + + fn check_service_equal( + new_name: &str, + new: CandidSource, + old_name: &str, + old: CandidSource, + ) { + let new_str = source_to_str(&new); + let old_str = source_to_str(&old); + match service_equal(new, old) { + Ok(_) => {} + Err(e) => { + eprintln!( + "{} is not compatible with {}!\n\n\ + {}:\n\ + {}\n\n\ + {}:\n\ + {}\n", + new_name, old_name, new_name, new_str, old_name, old_str + ); + panic!("{:?}", e); + } + } + } + + candid::export_service!(); + + let new_interface = __export_service(); + + // check the public interface against the actual one + let old_interface = std::path::PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap()) + .join("canister/interface.did"); + + check_service_equal( + "actual rate-limit candid interface", + candid_parser::utils::CandidSource::Text(&new_interface), + "declared candid interface in interface.did file", + candid_parser::utils::CandidSource::File(old_interface.as_path()), + ); + } +} diff --git a/rs/boundary_node/rate_limits/canister/getter.rs b/rs/boundary_node/rate_limits/canister/getter.rs index afe365e7953..038d24d6656 100644 --- a/rs/boundary_node/rate_limits/canister/getter.rs +++ b/rs/boundary_node/rate_limits/canister/getter.rs @@ -339,13 +339,13 @@ mod tests { is_redacted: false, rules: vec![ api::OutputRule { - id: rule_id_1.0.to_string(), + rule_id: rule_id_1.0.to_string(), incident_id: incident_id.0.to_string(), rule_raw: Some(b"{\"a\": 1}".to_vec()), description: Some("verbose description 1".to_string()), }, api::OutputRule { - id: rule_id_2.0.to_string(), + rule_id: rule_id_2.0.to_string(), incident_id: incident_id.0.to_string(), rule_raw: Some(b"{\"b\": 2}".to_vec()), description: Some("verbose description 2".to_string()), @@ -367,13 +367,13 @@ mod tests { is_redacted: true, rules: vec![ api::OutputRule { - id: rule_id_1.0.to_string(), + rule_id: rule_id_1.0.to_string(), incident_id: incident_id.0.to_string(), rule_raw: None, description: None, }, api::OutputRule { - id: rule_id_2.0.to_string(), + rule_id: rule_id_2.0.to_string(), incident_id: incident_id.0.to_string(), rule_raw: Some(b"{\"b\": 2}".to_vec()), description: Some("verbose description 2".to_string()), @@ -421,7 +421,7 @@ mod tests { assert_eq!( response, api::OutputRuleMetadata { - id: rule_id.0.to_string(), + rule_id: rule_id.0.to_string(), incident_id: incident_id.0.to_string(), rule_raw: Some(b"{\"a\": 1}".to_vec()), description: Some("verbose description".to_string()), @@ -435,7 +435,7 @@ mod tests { assert_eq!( response, api::OutputRuleMetadata { - id: rule_id.0.to_string(), + rule_id: rule_id.0.to_string(), incident_id: incident_id.0.to_string(), rule_raw: None, description: None, @@ -498,7 +498,7 @@ mod tests { let response = getter_unauthorized.get(&incident_id.0.to_string()).unwrap(); let rule_1 = api::OutputRuleMetadata { - id: rule_id_1.0.to_string(), + rule_id: rule_id_1.0.to_string(), incident_id: incident_id.0.to_string(), rule_raw: None, description: None, @@ -507,7 +507,7 @@ mod tests { removed_in_version: Some(3), }; let rule_2 = api::OutputRuleMetadata { - id: rule_id_2.0.to_string(), + rule_id: rule_id_2.0.to_string(), incident_id: incident_id.0.to_string(), rule_raw: Some(b"{\"b\": 2}".to_vec()), description: Some("verbose description 2".to_string()), diff --git a/rs/boundary_node/rate_limits/canister/interface.did b/rs/boundary_node/rate_limits/canister/interface.did index d1f60823a0b..e87ad544923 100644 --- a/rs/boundary_node/rate_limits/canister/interface.did +++ b/rs/boundary_node/rate_limits/canister/interface.did @@ -29,7 +29,7 @@ type OutputConfig = record { }; // Response structure for returning the requested configuration and associated metadata -type OutputConfigResponse = record { +type ConfigResponse = record { version: Version; // Version of the configuration active_since: Timestamp; // Time when this configuration was added (became active) config: OutputConfig; // Contains the list of rules @@ -94,14 +94,14 @@ type GetConfigError = variant { }; type GetRuleByIdError = variant { - NotFound; // Indicates that a rule with the specified ID does not exist - InvalidUuidFormat: text; // Indicates that the provided ID is not a valid UUID + NotFound; // Indicates that a rule with the specified ID does not exist + InvalidUuidFormat; // Indicates that the provided ID is not a valid UUID Internal: text; // Captures all unexpected internal errors }; type GetRulesByIncidentIdError = variant { - NotFound; // Indicates that an incident with the specified ID does not exist - InvalidUuidFormat: text; // Indicates that the provided ID is not a valid UUID + NotFound; // Indicates that an incident with the specified ID does not exist + InvalidUuidFormat; // Indicates that the provided ID is not a valid UUID Internal: text; // Captures all unexpected internal errors }; @@ -117,7 +117,7 @@ type DiscloseRulesResponse = variant { }; type GetConfigResponse = variant { - Ok: OutputConfigResponse; + Ok: ConfigResponse; Err: GetConfigError; }; diff --git a/rs/boundary_node/rate_limits/canister/lib.rs b/rs/boundary_node/rate_limits/canister/lib.rs index 7e6ee975e84..83adf10f42b 100644 --- a/rs/boundary_node/rate_limits/canister/lib.rs +++ b/rs/boundary_node/rate_limits/canister/lib.rs @@ -2,7 +2,7 @@ mod access_control; #[allow(dead_code)] mod add_config; -#[cfg(target_family = "wasm")] +#[cfg(any(target_family = "wasm", test))] mod canister; #[allow(dead_code)] mod confidentiality_formatting; @@ -10,6 +10,7 @@ mod confidentiality_formatting; mod disclose; #[allow(dead_code)] mod getter; +mod logs; #[allow(dead_code)] mod metrics; mod random; diff --git a/rs/boundary_node/rate_limits/canister/logs.rs b/rs/boundary_node/rate_limits/canister/logs.rs new file mode 100644 index 00000000000..a470a72c57c --- /dev/null +++ b/rs/boundary_node/rate_limits/canister/logs.rs @@ -0,0 +1,29 @@ +use candid::Deserialize; +use ic_canister_log::declare_log_buffer; + +// High-priority messages. +declare_log_buffer!(name = P0, capacity = 1000); + +// Low-priority info messages. +declare_log_buffer!(name = P1, capacity = 1000); + +#[derive(Clone, Debug, Default, Deserialize, serde::Serialize)] +pub struct Log { + pub entries: Vec, +} + +#[derive(Clone, Debug, Deserialize, serde::Serialize)] +pub struct LogEntry { + pub timestamp: u64, + pub priority: Priority, + pub file: String, + pub line: u32, + pub message: String, + pub counter: u64, +} + +#[derive(Clone, Debug, Deserialize, serde::Serialize)] +pub enum Priority { + P0, + P1, +} diff --git a/rs/boundary_node/rate_limits/canister/metrics.rs b/rs/boundary_node/rate_limits/canister/metrics.rs index a2c9ec0d865..2f95b9b285a 100644 --- a/rs/boundary_node/rate_limits/canister/metrics.rs +++ b/rs/boundary_node/rate_limits/canister/metrics.rs @@ -6,136 +6,114 @@ use crate::{ }; use ic_canisters_http_types::{HttpResponse, HttpResponseBuilder}; use ic_cdk::api::stable::WASM_PAGE_SIZE_IN_BYTES; -use prometheus::{CounterVec, Encoder, Gauge, IntGauge, Opts, Registry, TextEncoder}; -use std::cell::RefCell; +use prometheus::{ + CounterVec, Encoder, Gauge, IntGauge, Opts, Registry, Result as PrometheusResult, TextEncoder, +}; +use std::{borrow::BorrowMut, cell::RefCell}; thread_local! { - static STABLE_MEMORY_SIZE: RefCell = RefCell::new( - Gauge::new( + pub static METRICS: RefCell = RefCell::new(CanisterMetrics::new().expect("failed to create Prometheus metrics")); +} + +/// Represents all metrics collected in the canister +pub struct CanisterMetrics { + pub registry: Registry, // Prometheus registry + pub active_rate_limit_rules_count: IntGauge, + pub active_version: IntGauge, + pub api_boundary_nodes_count: IntGauge, + pub canister_api_calls: CounterVec, + pub configs_count: IntGauge, + pub incidents_count: IntGauge, + pub last_canister_change_time: IntGauge, + pub last_successful_registry_poll_time: IntGauge, + pub registry_poll_calls: CounterVec, + pub stable_memory_size: Gauge, +} + +impl CanisterMetrics { + pub fn new() -> PrometheusResult { + let registry = Registry::new(); + + let stable_memory_size = Gauge::new( "stable_memory_bytes", "Size of the stable memory allocated by this canister in bytes.", - ) - .unwrap(), - ); + )?; - static CANISTER_API_CALLS_COUNTER: RefCell = RefCell::new(CounterVec::new(Opts::new( - "canister_api_calls", - "Number of calls to the canister methods with the status and message (in case of error)", - ), &["method", "status", "message"]).unwrap()); + let canister_api_calls = CounterVec::new( + Opts::new( + "canister_api_calls", + "Number of calls to the canister methods with the status and message (in case of error)", + ), + &["method", "status", "message"], + )?; - static API_BOUNDARY_NODES_COUNT: RefCell = RefCell::new( - IntGauge::new( + let api_boundary_nodes_count = IntGauge::new( "api_boundary_nodes_count", "Number of API boundary nodes with full read access permission to rate-limit config.", - ) - .unwrap(), - ); + )?; - static ACTIVE_VERSION: RefCell = RefCell::new( - IntGauge::new( + let active_version = IntGauge::new( "active_config_version", "Version of the currently active configuration", - ) - .unwrap(), - ); + )?; - static ACTIVE_RATE_LIMIT_RULES_COUNT: RefCell = RefCell::new( - IntGauge::new( + let active_rate_limit_rules_count = IntGauge::new( "active_rules_count", "Number of rate-limit rules in the active configuration", - ) - .unwrap(), - ); + )?; - static INCIDENTS_COUNT: RefCell = - RefCell::new(IntGauge::new("stored_incidents_count", "Number of stored incidents").unwrap()); + let incidents_count = + IntGauge::new("stored_incidents_count", "Number of stored incidents")?; - static CONFIGS_COUNT: RefCell = RefCell::new( - IntGauge::new( + let configs_count = IntGauge::new( "stored_configs_count", "Number of stored rate-limit configurations", - ) - .unwrap(), - ); + )?; - pub static LAST_SUCCESSFUL_REGISTRY_POLL_TIME: RefCell = RefCell::new(IntGauge::new( - "last_successful_registry_poll", - "The Unix timestamp of the last successful poll of the API boundary nodes from registry canister").unwrap()); + let last_successful_registry_poll_time = IntGauge::new( + "last_successful_registry_poll", + "The Unix timestamp of the last successful poll of the API boundary nodes from registry canister", + )?; - pub static REGISTRY_POLL_CALLS_COUNTER: RefCell = RefCell::new( - CounterVec::new( + let registry_poll_calls = CounterVec::new( Opts::new( "registry_poll_calls", "Number of registry polling calls with the status and message (in case of error)", ), &["status", "message"], - ) - .unwrap(), - ); + )?; - pub static LAST_CANISTER_CHANGE_TIME: RefCell = RefCell::new( - IntGauge::new( + let last_canister_change_time = IntGauge::new( "last_successful_canister_upgrade", "The Unix timestamp of the last successful canister upgrade", - ) - .unwrap(), - ); - - static METRICS_REGISTRY: RefCell = RefCell::new({ - let registry = Registry::new(); - - STABLE_MEMORY_SIZE.with(|cell| { - let cell = Box::new(cell.borrow().clone()); - registry.register(cell).unwrap(); - }); - - CANISTER_API_CALLS_COUNTER.with(|cell| { - let cell = Box::new(cell.borrow().clone()); - registry.register(cell).unwrap(); - }); - - API_BOUNDARY_NODES_COUNT.with(|cell| { - let cell = Box::new(cell.borrow().clone()); - registry.register(cell).unwrap(); - }); - - ACTIVE_VERSION.with(|cell| { - let cell = Box::new(cell.borrow().clone()); - registry.register(cell).unwrap(); - }); - - ACTIVE_RATE_LIMIT_RULES_COUNT.with(|cell| { - let cell = Box::new(cell.borrow().clone()); - registry.register(cell).unwrap(); - }); - - INCIDENTS_COUNT.with(|cell| { - let cell = Box::new(cell.borrow().clone()); - registry.register(cell).unwrap(); - }); - - CONFIGS_COUNT.with(|cell| { - let cell = Box::new(cell.borrow().clone()); - registry.register(cell).unwrap(); - }); - - LAST_SUCCESSFUL_REGISTRY_POLL_TIME.with(|cell| { - let cell = Box::new(cell.borrow().clone()); - registry.register(cell).unwrap(); - }); - - REGISTRY_POLL_CALLS_COUNTER.with(|cell| { - let cell = Box::new(cell.borrow().clone()); - registry.register(cell).unwrap(); - }); - - LAST_CANISTER_CHANGE_TIME.with(|cell| { - let cell = Box::new(cell.borrow().clone()); - registry.register(cell).unwrap(); - }); - - registry - }); + )?; + + // Register all metrics in the registry + registry.register(Box::new(stable_memory_size.clone()))?; + registry.register(Box::new(canister_api_calls.clone()))?; + registry.register(Box::new(api_boundary_nodes_count.clone()))?; + registry.register(Box::new(active_version.clone()))?; + registry.register(Box::new(active_rate_limit_rules_count.clone()))?; + registry.register(Box::new(incidents_count.clone()))?; + registry.register(Box::new(configs_count.clone()))?; + registry.register(Box::new(last_successful_registry_poll_time.clone()))?; + registry.register(Box::new(registry_poll_calls.clone()))?; + registry.register(Box::new(last_canister_change_time.clone()))?; + + Ok(Self { + registry, + active_rate_limit_rules_count, + active_version, + api_boundary_nodes_count, + canister_api_calls, + configs_count, + incidents_count, + last_canister_change_time, + last_successful_registry_poll_time, + registry_poll_calls, + stable_memory_size, + }) + } } pub fn export_metrics_as_http_response( @@ -163,31 +141,20 @@ pub fn export_metrics_as_http_response( } pub fn recompute_metrics(canister_api: impl CanisterApi) { - STABLE_MEMORY_SIZE.with(|cell| { + METRICS.with(|cell| { + let mut cell = cell.borrow_mut(); + let memory = (ic_cdk::api::stable::stable_size() * WASM_PAGE_SIZE_IN_BYTES) as f64; - cell.borrow_mut().set(memory); - }); - API_BOUNDARY_NODES_COUNT.with(|cell| { - cell.borrow_mut() + cell.stable_memory_size.borrow_mut().set(memory); + cell.api_boundary_nodes_count .set(canister_api.api_boundary_nodes_count() as i64); - }); - - ACTIVE_VERSION.with(|cell| { - cell.borrow_mut() + cell.active_version .set(canister_api.get_version().unwrap_or(0) as i64); - }); - - CONFIGS_COUNT.with(|cell| { - cell.borrow_mut().set(canister_api.configs_count() as i64); - }); - - INCIDENTS_COUNT.with(|cell| { - cell.borrow_mut().set(canister_api.incidents_count() as i64); - }); - - ACTIVE_RATE_LIMIT_RULES_COUNT.with(|cell| { - cell.borrow_mut() + cell.configs_count.set(canister_api.configs_count() as i64); + cell.incidents_count + .set(canister_api.incidents_count() as i64); + cell.active_rate_limit_rules_count .set(canister_api.active_rules_count() as i64); }); } @@ -239,8 +206,8 @@ impl DisclosesRules for WithMetrics { } fn update_canister_call_metrics(method_name: &str, status: &str, message: &str) { - CANISTER_API_CALLS_COUNTER.with(|cell| { - let metric = cell.borrow_mut(); + METRICS.with(|cell| { + let metric = &cell.borrow_mut().canister_api_calls; metric .with_label_values(&[method_name, status, message]) .inc(); @@ -251,8 +218,8 @@ pub fn with_metrics_registry(f: F) -> T where F: FnOnce(&Registry) -> T, { - METRICS_REGISTRY.with(|cell| { - let registry = cell.borrow().clone(); + METRICS.with(|cell| { + let registry = cell.borrow().registry.clone(); f(®istry) }) } diff --git a/rs/boundary_node/rate_limits/canister/types.rs b/rs/boundary_node/rate_limits/canister/types.rs index 672a8b0e954..0b579194632 100644 --- a/rs/boundary_node/rate_limits/canister/types.rs +++ b/rs/boundary_node/rate_limits/canister/types.rs @@ -79,7 +79,7 @@ impl From for api::OutputRule { fn from(value: OutputRule) -> Self { api::OutputRule { description: value.description, - id: value.id.to_string(), + rule_id: value.id.to_string(), incident_id: value.incident_id.to_string(), rule_raw: value.rule_raw, } @@ -299,7 +299,7 @@ pub struct OutputRuleMetadata { impl From for api::OutputRuleMetadata { fn from(value: OutputRuleMetadata) -> Self { api::OutputRuleMetadata { - id: value.id.0.to_string(), + rule_id: value.id.0.to_string(), incident_id: value.incident_id.0.to_string(), rule_raw: value.rule_raw, description: value.description,