From 48b8f3d1adb9530abb905d14fc003c9d26be38c5 Mon Sep 17 00:00:00 2001 From: gregorydemay <112856886+gregorydemay@users.noreply.github.com> Date: Wed, 18 Sep 2024 15:03:39 +0200 Subject: [PATCH] refactor: remove `cketh_eth_rpc_call_retry_count` metrics (#277) Follow-up on #275 to remove the `cketh_eth_rpc_call_retry_count` metrics imported from the [`ic-cketh-minter`](https://github.com/dfinity/ic/blob/1f551bea63354370b6e7a5037e96d464bdab3b41/rs/ethereum/cketh/minter/Cargo.toml) dependency. --- src/rpc_client/eth_rpc/mod.rs | 107 ++------------------------------ src/rpc_client/eth_rpc/tests.rs | 95 ---------------------------- 2 files changed, 4 insertions(+), 198 deletions(-) diff --git a/src/rpc_client/eth_rpc/mod.rs b/src/rpc_client/eth_rpc/mod.rs index 6c4ee935..b9702f7c 100644 --- a/src/rpc_client/eth_rpc/mod.rs +++ b/src/rpc_client/eth_rpc/mod.rs @@ -646,7 +646,10 @@ where loop { rpc_request.id = next_request_id(); let payload = serde_json::to_string(&rpc_request).unwrap(); - log!(TRACE_HTTP, "Calling url: {}, with payload: {payload}", url); + log!( + TRACE_HTTP, + "Calling url (retries={retries}): {url}, with payload: {payload}" + ); let effective_size_estimate = response_size_estimate.get(); let transform_op = O::response_transform() @@ -702,8 +705,6 @@ where response.status ); - metrics::observe_retry_count(eth_method.clone(), retries); - // JSON-RPC responses over HTTP should have a 2xx status code, // even if the contained JsonRpcResult is an error. // If the server is not available, it will sometimes (wrongly) return HTML that will fail parsing as JSON. @@ -750,103 +751,3 @@ fn sort_by_hash(to_sort: &mut [T]) { a_hash.cmp(&b_hash) }); } - -#[allow(dead_code)] //TODO 243: hook-up to existing metrics -pub(super) mod metrics { - use ic_metrics_encoder::MetricsEncoder; - use std::cell::RefCell; - use std::collections::BTreeMap; - - /// The max number of RPC call retries we expect to see (plus one). - const MAX_EXPECTED_RETRIES: usize = 20; - - #[derive(Default)] - struct RetryHistogram { - /// The histogram of HTTP call retry counts. - /// The last bucket corresponds to the "infinite" value that exceeds the maximum number we - /// expect to see in practice. - retry_buckets: [u64; MAX_EXPECTED_RETRIES + 1], - retry_count: u64, - } - - impl RetryHistogram { - fn observe_retry_count(&mut self, count: usize) { - self.retry_buckets[count.min(MAX_EXPECTED_RETRIES)] += 1; - self.retry_count += count as u64; - } - - /// Returns a iterator over the histrogram buckets in the format that ic-metrics-encoder - /// expects. - fn iter(&self) -> impl Iterator + '_ { - (0..MAX_EXPECTED_RETRIES) - .zip(self.retry_buckets[0..MAX_EXPECTED_RETRIES].iter().cloned()) - .map(|(k, v)| (k as f64, v as f64)) - .chain(std::iter::once(( - f64::INFINITY, - self.retry_buckets[MAX_EXPECTED_RETRIES] as f64, - ))) - } - } - - #[derive(Default)] - pub struct HttpMetrics { - /// Retry counts histograms indexed by the ETH RCP method name. - retry_histogram_per_method: BTreeMap, - } - - impl HttpMetrics { - pub fn observe_retry_count(&mut self, method: String, count: usize) { - self.retry_histogram_per_method - .entry(method) - .or_default() - .observe_retry_count(count); - } - - #[cfg(test)] - pub fn count_retries_in_bucket(&self, method: &str, count: usize) -> u64 { - match self.retry_histogram_per_method.get(method) { - Some(histogram) => histogram.retry_buckets[count.min(MAX_EXPECTED_RETRIES)], - None => 0, - } - } - - pub fn encode( - &self, - encoder: &mut MetricsEncoder, - ) -> std::io::Result<()> { - if self.retry_histogram_per_method.is_empty() { - return Ok(()); - } - - let mut histogram_vec = encoder.histogram_vec( - "cketh_eth_rpc_call_retry_count", - "The number of ETH RPC call retries by method.", - )?; - - for (method, histogram) in &self.retry_histogram_per_method { - histogram_vec = histogram_vec.histogram( - &[("method", method.as_str())], - histogram.iter(), - histogram.retry_count as f64, - )?; - } - - Ok(()) - } - } - - //TODO XC-243: use existing METRICS declared in memory.rs - thread_local! { - static METRICS: RefCell = RefCell::default(); - } - - /// Record the retry count for the specified ETH RPC method. - pub fn observe_retry_count(method: String, count: usize) { - METRICS.with(|metrics| metrics.borrow_mut().observe_retry_count(method, count)); - } - - /// Encodes the metrics related to ETH RPC method calls. - pub fn encode(encoder: &mut MetricsEncoder) -> std::io::Result<()> { - METRICS.with(|metrics| metrics.borrow().encode(encoder)) - } -} diff --git a/src/rpc_client/eth_rpc/tests.rs b/src/rpc_client/eth_rpc/tests.rs index 856a5399..35c45333 100644 --- a/src/rpc_client/eth_rpc/tests.rs +++ b/src/rpc_client/eth_rpc/tests.rs @@ -405,98 +405,3 @@ fn transaction_receipt_normalization() { }"#, ); } - -#[test] -fn http_metrics_should_aggregate_retry_counts() { - use super::metrics::HttpMetrics; - - let mut metrics = HttpMetrics::default(); - - for count in [0, 1, 2, 3, 0, 0, 2, 5, 100, 200, 300] { - metrics.observe_retry_count("eth_test".to_string(), count); - } - - for count in [0, 1, 2, 3] { - metrics.observe_retry_count("eth_test2".to_string(), count); - } - - assert_eq!(3, metrics.count_retries_in_bucket("eth_test", 0)); - assert_eq!(1, metrics.count_retries_in_bucket("eth_test", 1)); - assert_eq!(2, metrics.count_retries_in_bucket("eth_test", 2)); - assert_eq!(1, metrics.count_retries_in_bucket("eth_test", 3)); - assert_eq!(0, metrics.count_retries_in_bucket("eth_test", 4)); - assert_eq!(1, metrics.count_retries_in_bucket("eth_test", 5)); - assert_eq!(1, metrics.count_retries_in_bucket("eth_test", 5)); - assert_eq!(3, metrics.count_retries_in_bucket("eth_test", 100)); - assert_eq!(3, metrics.count_retries_in_bucket("eth_test", 200)); - assert_eq!(3, metrics.count_retries_in_bucket("eth_test", 300)); - assert_eq!(3, metrics.count_retries_in_bucket("eth_test", usize::MAX)); - - assert_eq!(1, metrics.count_retries_in_bucket("eth_test2", 0)); - assert_eq!(1, metrics.count_retries_in_bucket("eth_test2", 1)); - assert_eq!(1, metrics.count_retries_in_bucket("eth_test2", 2)); - assert_eq!(1, metrics.count_retries_in_bucket("eth_test2", 3)); - assert_eq!(0, metrics.count_retries_in_bucket("eth_test2", 100)); - - assert_eq!(0, metrics.count_retries_in_bucket("eth_unknown", 0)); - - let mut encoder = ic_metrics_encoder::MetricsEncoder::new(Vec::new(), 12346789); - metrics.encode(&mut encoder).unwrap(); - let bytes = encoder.into_inner(); - let metrics_text = String::from_utf8(bytes).unwrap(); - - assert_eq!( - metrics_text.trim(), - r#" -# HELP cketh_eth_rpc_call_retry_count The number of ETH RPC call retries by method. -# TYPE cketh_eth_rpc_call_retry_count histogram -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="0"} 3 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="1"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="2"} 6 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="3"} 7 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="4"} 7 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="5"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="6"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="7"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="8"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="9"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="10"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="11"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="12"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="13"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="14"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="15"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="16"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="17"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="18"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="19"} 8 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test",le="+Inf"} 11 12346789 -cketh_eth_rpc_call_retry_count_sum{method="eth_test"} 613 12346789 -cketh_eth_rpc_call_retry_count_count{method="eth_test"} 11 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="0"} 1 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="1"} 2 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="2"} 3 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="3"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="4"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="5"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="6"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="7"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="8"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="9"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="10"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="11"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="12"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="13"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="14"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="15"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="16"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="17"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="18"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="19"} 4 12346789 -cketh_eth_rpc_call_retry_count_bucket{method="eth_test2",le="+Inf"} 4 12346789 -cketh_eth_rpc_call_retry_count_sum{method="eth_test2"} 6 12346789 -cketh_eth_rpc_call_retry_count_count{method="eth_test2"} 4 12346789 -"# - .trim() - ); -}