chore: read_state metrics (#9235)

Adding `read_state` metrics before adding future optimizations, as
described in NODE-1863

---------

Co-authored-by: IDX GitHub Automation <infra+github-automation@dfinity.org>

Author: andrewbattat

rs/http_endpoints/public/src/lib.rs

@@ -350,6 +350,7 @@ pub fn start_server(
     let canister_read_state_router = |version| {
         CanisterReadStateServiceBuilder::builder(
             log.clone(),
+            metrics.clone(),
             state_reader.clone(),
             registry_client.clone(),
             ingress_verifier.clone(),
@@ -369,6 +370,7 @@ pub fn start_server(
 
     let subnet_read_state_router = |version| {
         SubnetReadStateServiceBuilder::builder(
+            metrics.clone(),
             nns_delegation_reader.clone(),
             state_reader.clone(),
             nns_subnet_id,

rs/http_endpoints/public/src/metrics.rs

@@ -71,6 +71,9 @@ pub struct HttpHandlerMetrics {
     // sync call handler metrics
     pub sync_call_early_response_trigger_total: IntCounterVec,
     pub sync_call_certificate_status_total: IntCounterVec,
+
+    // read_state metrics
+    pub read_state_path_type_total: IntCounterVec,
 }
 
 // There is a mismatch between the labels and the public spec.
@@ -207,6 +210,17 @@ impl HttpHandlerMetrics {
                 "The count of early response triggers for the /{v3,v4}/.../call endpoint.",
                 &[LABEL_SYNC_CALL_EARLY_RESPONSE_TRIGGER],
             ),
+            read_state_path_type_total: metrics_registry.int_counter_vec(
+                "replica_http_read_state_path_type_total",
+                "Count of read_state paths requested, by endpoint type and path type.",
+                &["endpoint", "path_type"],
+            ),
         }
     }
+
+    pub fn observe_read_state_path(&self, endpoint_type: &str, path_type: &str) {
+        self.read_state_path_type_total
+            .with_label_values(&[endpoint_type, path_type])
+            .inc()
+    }
 }

rs/http_endpoints/public/src/read_state/canister.rs

@@ -5,6 +5,7 @@ use super::{
 use crate::{
     HttpError, ReplicaHealthStatus,
     common::{Cbor, WithTimeout, build_validator, validation_error_to_http_error},
+    metrics::HttpHandlerMetrics,
 };
 
 use axum::{
@@ -57,6 +58,7 @@ pub enum Version {
 pub struct CanisterReadStateService {
     log: ReplicaLogger,
     health_status: Arc<AtomicCell<ReplicaHealthStatus>>,
+    metrics: HttpHandlerMetrics,
     nns_delegation_reader: NNSDelegationReader,
     state_reader: Arc<dyn StateReader<State = ReplicatedState>>,
     time_source: Arc<dyn TimeSource>,
@@ -69,6 +71,7 @@ pub struct CanisterReadStateService {
 pub struct CanisterReadStateServiceBuilder {
     log: ReplicaLogger,
     health_status: Option<Arc<AtomicCell<ReplicaHealthStatus>>>,
+    metrics: HttpHandlerMetrics,
     malicious_flags: Option<MaliciousFlags>,
     nns_delegation_reader: NNSDelegationReader,
     state_reader: Arc<dyn StateReader<State = ReplicatedState>>,
@@ -91,6 +94,7 @@ impl CanisterReadStateService {
 impl CanisterReadStateServiceBuilder {
     pub fn builder(
         log: ReplicaLogger,
+        metrics: HttpHandlerMetrics,
         state_reader: Arc<dyn StateReader<State = ReplicatedState>>,
         registry_client: Arc<dyn RegistryClient>,
         ingress_verifier: Arc<dyn IngressSigVerifier>,
@@ -101,6 +105,7 @@ impl CanisterReadStateServiceBuilder {
         Self {
             log,
             health_status: None,
+            metrics,
             malicious_flags: None,
             nns_delegation_reader,
             state_reader,
@@ -136,6 +141,7 @@ impl CanisterReadStateServiceBuilder {
             health_status: self
                 .health_status
                 .unwrap_or_else(|| Arc::new(AtomicCell::new(ReplicaHealthStatus::Healthy))),
+            metrics: self.metrics,
             nns_delegation_reader: self.nns_delegation_reader,
             state_reader: self.state_reader,
             time_source: self.time_source.unwrap_or(Arc::new(SysTimeSource::new())),
@@ -163,6 +169,7 @@ pub(crate) async fn canister_read_state(
     State(CanisterReadStateService {
         log,
         health_status,
+        metrics,
         nns_delegation_reader,
         state_reader,
         time_source,
@@ -218,6 +225,7 @@ pub(crate) async fn canister_read_state(
 
         // Verify authorization for requested paths.
         if let Err(HttpError { status, message }) = verify_paths(
+            &metrics,
             version,
             certified_state_reader.get_state(),
             &read_state.source,
@@ -257,6 +265,7 @@ pub(crate) async fn canister_read_state(
 
 // Verifies that the `user` is authorized to retrieve the `paths` requested.
 fn verify_paths(
+    metrics: &HttpHandlerMetrics,
     version: Version,
     state: &ReplicatedState,
     user: &UserId,
@@ -265,6 +274,8 @@ fn verify_paths(
     effective_principal_id: PrincipalId,
     nns_subnet_id: SubnetId,
 ) -> Result<(), HttpError> {
+    const ENDPOINT: &str = "canister";
+
     let mut last_request_status_id: Option<MessageId> = None;
 
     // Convert the paths to slices to make it easier to match below.
@@ -275,10 +286,13 @@ fn verify_paths(
 
     for path in paths {
         match path.as_slice() {
-            [b"time"] => {}
+            [b"time"] => {
+                metrics.observe_read_state_path(ENDPOINT, "time");
+            }
             [b"canister", canister_id, b"controllers" | b"module_hash"] => {
                 let canister_id = parse_principal_id(canister_id)?;
                 verify_principal_ids(&canister_id, &effective_principal_id)?;
+                metrics.observe_read_state_path(ENDPOINT, "canister_info");
             }
             [b"canister", canister_id, b"metadata", name] => {
                 let name = String::from_utf8(Vec::from(*name)).map_err(|err| HttpError {
@@ -295,26 +309,50 @@ fn verify_paths(
                     &CanisterId::unchecked_from_principal(principal_id),
                     &name,
                     state,
-                )?
+                )?;
+                metrics.observe_read_state_path(ENDPOINT, "canister_metadata");
+            }
+            [b"api_boundary_nodes"] => {
+                metrics.observe_read_state_path(ENDPOINT, "api_boundary_nodes");
             }
-            [b"api_boundary_nodes"] => {}
             [b"api_boundary_nodes", _node_id]
             | [
                 b"api_boundary_nodes",
                 _node_id,
                 b"domain" | b"ipv4_address" | b"ipv6_address",
-            ] => {}
-            [b"subnet"] => {}
-            [b"subnet", _subnet_id] | [b"subnet", _subnet_id, b"public_key" | b"node"] => {}
+            ] => {
+                metrics.observe_read_state_path(ENDPOINT, "api_boundary_nodes_info");
+            }
+            [b"subnet"] => {
+                metrics.observe_read_state_path(ENDPOINT, "subnet");
+            }
+            [b"subnet", _subnet_id] => {
+                metrics.observe_read_state_path(ENDPOINT, "subnet_info");
+            }
+            [b"subnet", _subnet_id, b"public_key"] => {
+                metrics.observe_read_state_path(ENDPOINT, "subnet_public_key");
+            }
+            [b"subnet", _subnet_id, b"node"] => {
+                metrics.observe_read_state_path(ENDPOINT, "subnet_node");
+            }
+            [b"subnet", _subnet_id, b"node", _node_id] => {
+                metrics.observe_read_state_path(ENDPOINT, "subnet_node_info");
+            }
+            [b"subnet", _subnet_id, b"node", _node_id, b"public_key"] => {
+                metrics.observe_read_state_path(ENDPOINT, "subnet_node_public_key");
+            }
             // `/subnet/<subnet_id>/canister_ranges` is always allowed on the `/api/v2` endpoint
-            [b"subnet", _subnet_id, b"canister_ranges"] if version == Version::V2 => {}
+            [b"subnet", _subnet_id, b"canister_ranges"] if version == Version::V2 => {
+                metrics.observe_read_state_path(ENDPOINT, "subnet_canister_ranges");
+            }
             // `/subnet/<subnet_id>/canister_ranges` is allowed on the `/api/v3` endpoint
             // only when `subnet_id == nns_subnet_id`.
             [b"subnet", subnet_id, b"canister_ranges"]
                 if version == Version::V3
-                    && parse_principal_id(subnet_id)? == nns_subnet_id.get() => {}
-            [b"subnet", _subnet_id, b"node", _node_id]
-            | [b"subnet", _subnet_id, b"node", _node_id, b"public_key"] => {}
+                    && parse_principal_id(subnet_id)? == nns_subnet_id.get() =>
+            {
+                metrics.observe_read_state_path(ENDPOINT, "subnet_canister_ranges");
+            }
             [b"request_status", request_id]
             | [
                 b"request_status",
@@ -365,6 +403,7 @@ fn verify_paths(
                             .to_string(),
                     });
                 }
+                metrics.observe_read_state_path(ENDPOINT, "request_status");
             }
             _ => {
                 // All other paths are unsupported.
@@ -427,6 +466,7 @@ mod test {
     };
     use hyper::StatusCode;
     use ic_crypto_tree_hash::{Digest, Label, MixedHashTree, Path};
+    use ic_metrics::MetricsRegistry;
     use ic_registry_subnet_type::SubnetType;
     use ic_replicated_state::{
         CanisterQueues, RefundPool, ReplicatedState, SystemMetadata,
@@ -442,6 +482,10 @@ mod test {
     const NNS_SUBNET_ID: SubnetId = SUBNET_0;
     const APP_SUBNET_ID: SubnetId = SUBNET_1;
 
+    fn test_metrics() -> HttpHandlerMetrics {
+        HttpHandlerMetrics::new(&MetricsRegistry::new())
+    }
+
     #[test]
     fn encoding_read_state_tree_empty() {
         let tree = MixedHashTree::Empty;
@@ -565,9 +609,11 @@ mod test {
 
     #[rstest]
     fn test_canister_ranges_are_not_allowed(#[values(Version::V2, Version::V3)] version: Version) {
+        let metrics = test_metrics();
         let state = fake_replicated_state();
 
         let error = verify_paths(
+            &metrics,
             version,
             &state,
             &user_test_id(1),
@@ -586,9 +632,11 @@ mod test {
 
     #[rstest]
     fn test_verify_path(#[values(Version::V2, Version::V3)] version: Version) {
+        let metrics = test_metrics();
         let state = fake_replicated_state();
         assert_eq!(
             verify_paths(
+                &metrics,
                 version,
                 &state,
                 &user_test_id(1),
@@ -602,6 +650,7 @@ mod test {
 
         assert_eq!(
             verify_paths(
+                &metrics,
                 version,
                 &state,
                 &user_test_id(1),
@@ -625,6 +674,7 @@ mod test {
         );
 
         let err = verify_paths(
+            &metrics,
             version,
             &state,
             &user_test_id(1),
@@ -643,8 +693,10 @@ mod test {
     #[test]
     fn deprecated_canister_ranges_path_is_not_allowed_on_the_v3_endpoint_except_for_the_nns_subnet()
     {
+        let metrics = test_metrics();
         let state = fake_replicated_state();
         let err = verify_paths(
+            &metrics,
             Version::V3,
             &state,
             &user_test_id(1),
@@ -662,6 +714,7 @@ mod test {
 
         assert!(
             verify_paths(
+                &metrics,
                 Version::V3,
                 &state,
                 &user_test_id(1),
@@ -680,9 +733,11 @@ mod test {
 
     #[test]
     fn deprecated_canister_ranges_path_is_allowed_on_the_v2_endpoint() {
+        let metrics = test_metrics();
         let state = fake_replicated_state();
         assert!(
             verify_paths(
+                &metrics,
                 Version::V2,
                 &state,
                 &user_test_id(1),
@@ -698,4 +753,77 @@ mod test {
             .is_ok()
         );
     }
+
+    #[test]
+    fn test_verify_paths_records_metrics() {
+        let metrics = test_metrics();
+        let state = fake_replicated_state();
+        let canister_id = canister_test_id(1);
+
+        verify_paths(
+            &metrics,
+            Version::V2,
+            &state,
+            &user_test_id(1),
+            &[
+                Path::from(Label::from("time")),
+                Path::new(vec![
+                    Label::from("canister"),
+                    canister_id.get().to_vec().into(),
+                    Label::from("module_hash"),
+                ]),
+                Path::new(vec![Label::from("api_boundary_nodes")]),
+                Path::new(vec![
+                    Label::from("subnet"),
+                    APP_SUBNET_ID.get().to_vec().into(),
+                    Label::from("public_key"),
+                ]),
+                Path::new(vec![
+                    Label::from("request_status"),
+                    [0; 32].into(),
+                    Label::from("status"),
+                ]),
+            ],
+            &CanisterIdSet::all(),
+            canister_id.get(),
+            NNS_SUBNET_ID,
+        )
+        .unwrap();
+
+        assert_eq!(
+            metrics
+                .read_state_path_type_total
+                .with_label_values(&["canister", "time"])
+                .get(),
+            1
+        );
+        assert_eq!(
+            metrics
+                .read_state_path_type_total
+                .with_label_values(&["canister", "canister_info"])
+                .get(),
+            1
+        );
+        assert_eq!(
+            metrics
+                .read_state_path_type_total
+                .with_label_values(&["canister", "api_boundary_nodes"])
+                .get(),
+            1
+        );
+        assert_eq!(
+            metrics
+                .read_state_path_type_total
+                .with_label_values(&["canister", "subnet_public_key"])
+                .get(),
+            1
+        );
+        assert_eq!(
+            metrics
+                .read_state_path_type_total
+                .with_label_values(&["canister", "request_status"])
+                .get(),
+            1
+        );
+    }
 }