From 661cd58ec2b5d20acd84fdda521508435c236225 Mon Sep 17 00:00:00 2001 From: Fraser Hutchison Date: Tue, 4 Jun 2024 12:28:10 +0100 Subject: [PATCH 1/2] restrict metrics crate usage to astria-telemetry --- Cargo.lock | 243 ++++++++++---- Cargo.toml | 1 - crates/astria-bridge-withdrawer/Cargo.toml | 1 - crates/astria-bridge-withdrawer/src/main.rs | 23 +- .../astria-bridge-withdrawer/src/metrics.rs | 131 ++++---- .../src/withdrawer/mod.rs | 10 +- .../src/withdrawer/submitter/tests.rs | 21 +- crates/astria-composer/Cargo.toml | 1 - crates/astria-composer/src/collectors/geth.rs | 2 +- crates/astria-composer/src/composer.rs | 9 +- crates/astria-composer/src/config.rs | 14 +- crates/astria-composer/src/executor/tests.rs | 44 ++- crates/astria-composer/src/lib.rs | 11 +- crates/astria-composer/src/main.rs | 15 +- crates/astria-composer/src/metrics.rs | 296 +++++++++--------- crates/astria-composer/src/rollup.rs | 2 +- .../tests/blackbox/helper/mod.rs | 53 +++- crates/astria-conductor/Cargo.toml | 1 - crates/astria-conductor/src/conductor.rs | 6 +- crates/astria-conductor/src/lib.rs | 1 + crates/astria-conductor/src/main.rs | 15 +- crates/astria-conductor/src/metrics.rs | 206 ++++++------ .../tests/blackbox/helpers/mod.rs | 34 +- crates/astria-sequencer-relayer/Cargo.toml | 1 - crates/astria-sequencer-relayer/src/lib.rs | 1 + crates/astria-sequencer-relayer/src/main.rs | 15 +- .../astria-sequencer-relayer/src/metrics.rs | 243 +++++++------- .../src/relayer/write/conversion.rs | 3 +- .../src/sequencer_relayer.rs | 6 +- .../helpers/test_sequencer_relayer.rs | 36 ++- .../tests/blackbox/main.rs | 4 +- crates/astria-sequencer/Cargo.toml | 1 - crates/astria-sequencer/src/app/test_utils.rs | 3 +- crates/astria-sequencer/src/lib.rs | 1 + crates/astria-sequencer/src/main.rs | 15 +- crates/astria-sequencer/src/metrics.rs | 253 ++++++++------- crates/astria-sequencer/src/sequencer.rs | 7 +- .../astria-sequencer/src/service/consensus.rs | 3 +- crates/astria-telemetry/Cargo.toml | 5 +- crates/astria-telemetry/src/lib.rs | 150 ++++----- .../astria-telemetry/src/metrics/builders.rs | 277 ++++++++++++++++ .../astria-telemetry/src/metrics/counter.rs | 25 ++ crates/astria-telemetry/src/metrics/error.rs | 79 +++++ .../astria-telemetry/src/metrics/factories.rs | 210 +++++++++++++ crates/astria-telemetry/src/metrics/gauge.rs | 32 ++ crates/astria-telemetry/src/metrics/handle.rs | 17 + .../astria-telemetry/src/metrics/histogram.rs | 22 ++ .../astria-telemetry/src/metrics/into_f64.rs | 59 ++++ crates/astria-telemetry/src/metrics/mod.rs | 68 ++++ 49 files changed, 1805 insertions(+), 871 deletions(-) create mode 100644 crates/astria-telemetry/src/metrics/builders.rs create mode 100644 crates/astria-telemetry/src/metrics/counter.rs create mode 100644 crates/astria-telemetry/src/metrics/error.rs create mode 100644 crates/astria-telemetry/src/metrics/factories.rs create mode 100644 crates/astria-telemetry/src/metrics/gauge.rs create mode 100644 crates/astria-telemetry/src/metrics/handle.rs create mode 100644 crates/astria-telemetry/src/metrics/histogram.rs create mode 100644 crates/astria-telemetry/src/metrics/into_f64.rs create mode 100644 crates/astria-telemetry/src/metrics/mod.rs diff --git a/Cargo.lock b/Cargo.lock index db687d3a4c..efef480d00 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -507,11 +507,10 @@ dependencies = [ "ethers", "futures", "hex", - "http", + "http 0.2.12", "humantime", - "hyper", + "hyper 0.14.28", "ibc-types", - "metrics", "once_cell", "pin-project-lite", "prost", @@ -573,10 +572,9 @@ dependencies = [ "futures", "hex", "humantime", - "hyper", + "hyper 0.14.28", "insta", "itertools 0.12.1", - "metrics", "once_cell", "pin-project-lite", "prost", @@ -623,14 +621,13 @@ dependencies = [ "futures", "futures-bounded", "hex", - "http", + "http 0.2.12", "humantime", "indexmap 2.2.6", "insta", "itertools 0.12.1", "itoa", "jsonrpsee", - "metrics", "moka", "once_cell", "pbjson-types", @@ -777,7 +774,6 @@ dependencies = [ "ibc-types", "insta", "matchit", - "metrics", "penumbra-ibc", "penumbra-proto", "penumbra-tower-trace", @@ -846,13 +842,12 @@ dependencies = [ "const_format", "futures", "hex", - "http", + "http 0.2.12", "humantime", - "hyper", + "hyper 0.14.28", "itertools 0.12.1", "itoa", "k256", - "metrics", "once_cell", "pbjson-types", "pin-project-lite", @@ -909,6 +904,8 @@ dependencies = [ "base64 0.21.7", "base64-serde", "const_format", + "itertools 0.12.1", + "metrics 0.23.0", "metrics-exporter-prometheus", "opentelemetry", "opentelemetry-otlp", @@ -919,6 +916,7 @@ dependencies = [ "serde_json", "serde_with", "thiserror", + "tokio", "tracing", "tracing-opentelemetry", "tracing-subscriber 0.3.18", @@ -1026,6 +1024,12 @@ dependencies = [ "bytemuck", ] +[[package]] +name = "atomic-waker" +version = "1.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1505bd5d3d116872e7271a6d4e16d81d0c8570876c8de68093a09ac269d8aac0" + [[package]] name = "auto_impl" version = "1.2.0" @@ -1054,9 +1058,9 @@ dependencies = [ "bitflags 1.3.2", "bytes", "futures-util", - "http", - "http-body", - "hyper", + "http 0.2.12", + "http-body 0.4.6", + "hyper 0.14.28", "itoa", "matchit", "memchr", @@ -1084,8 +1088,8 @@ dependencies = [ "async-trait", "bytes", "futures-util", - "http", - "http-body", + "http 0.2.12", + "http-body 0.4.6", "mime", "rustversion", "tower-layer", @@ -1131,6 +1135,12 @@ version = "0.21.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" +[[package]] +name = "base64" +version = "0.22.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" + [[package]] name = "base64-serde" version = "0.7.0" @@ -1548,7 +1558,7 @@ checksum = "4f4c948ab3cd9562d256b752d874d573c836ec8b200bba87d1154bbf662d3a00" dependencies = [ "async-trait", "celestia-types", - "http", + "http 0.2.12", "jsonrpsee", "serde", "thiserror", @@ -1808,7 +1818,7 @@ dependencies = [ "ibc-types", "ics23", "jmt", - "metrics", + "metrics 0.22.3", "once_cell", "parking_lot", "pin-project", @@ -2980,7 +2990,7 @@ dependencies = [ "futures-timer", "futures-util", "hashers", - "http", + "http 0.2.12", "instant", "jsonwebtoken", "once_cell", @@ -3483,7 +3493,26 @@ dependencies = [ "futures-core", "futures-sink", "futures-util", - "http", + "http 0.2.12", + "indexmap 2.2.6", + "slab", + "tokio", + "tokio-util 0.7.10", + "tracing", +] + +[[package]] +name = "h2" +version = "0.4.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fa82e28a107a8cc405f0839610bdc9b15f1e25ec7d696aa5cf173edbcb1486ab" +dependencies = [ + "atomic-waker", + "bytes", + "fnv", + "futures-core", + "futures-sink", + "http 1.1.0", "indexmap 2.2.6", "slab", "tokio", @@ -3612,6 +3641,17 @@ dependencies = [ "itoa", ] +[[package]] +name = "http" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "21b9ddb458710bc376481b842f5da65cdf31522de232c1ca8146abce2a358258" +dependencies = [ + "bytes", + "fnv", + "itoa", +] + [[package]] name = "http-body" version = "0.4.6" @@ -3619,7 +3659,30 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ceab25649e9960c0311ea418d17bee82c0dcec1bd053b5f9a66e265a693bed2" dependencies = [ "bytes", - "http", + "http 0.2.12", + "pin-project-lite", +] + +[[package]] +name = "http-body" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1cac85db508abc24a2e48553ba12a996e87244a0395ce011e62b37158745d643" +dependencies = [ + "bytes", + "http 1.1.0", +] + +[[package]] +name = "http-body-util" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "793429d76616a256bcb62c2a2ec2bed781c8307e797e2598c50010f2bee2544f" +dependencies = [ + "bytes", + "futures-util", + "http 1.1.0", + "http-body 1.0.0", "pin-project-lite", ] @@ -3639,7 +3702,7 @@ dependencies = [ "async-channel", "base64 0.13.1", "futures-lite", - "http", + "http 0.2.12", "infer", "pin-project-lite", "rand 0.7.3", @@ -3678,9 +3741,9 @@ dependencies = [ "futures-channel", "futures-core", "futures-util", - "h2", - "http", - "http-body", + "h2 0.3.26", + "http 0.2.12", + "http-body 0.4.6", "httparse", "httpdate", "itoa", @@ -3692,6 +3755,27 @@ dependencies = [ "want", ] +[[package]] +name = "hyper" +version = "1.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe575dd17d0862a9a33781c8c4696a55c320909004a67a00fb286ba8b1bc496d" +dependencies = [ + "bytes", + "futures-channel", + "futures-util", + "h2 0.4.5", + "http 1.1.0", + "http-body 1.0.0", + "httparse", + "httpdate", + "itoa", + "pin-project-lite", + "smallvec", + "tokio", + "want", +] + [[package]] name = "hyper-rustls" version = "0.24.2" @@ -3699,8 +3783,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ec3efd23720e2049821a693cbc7e65ea87c72f1c58ff2f9522ff332b1491e590" dependencies = [ "futures-util", - "http", - "hyper", + "http 0.2.12", + "hyper 0.14.28", "log", "rustls", "rustls-native-certs", @@ -3714,12 +3798,32 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbb958482e8c7be4bc3cf272a766a2b0bf1a6755e7a6ae777f017a31d11b13b1" dependencies = [ - "hyper", + "hyper 0.14.28", "pin-project-lite", "tokio", "tokio-io-timeout", ] +[[package]] +name = "hyper-util" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b875924a60b96e5d7b9ae7b066540b1dd1cbd90d1828f54c92e02a283351c56" +dependencies = [ + "bytes", + "futures-channel", + "futures-util", + "http 1.1.0", + "http-body 1.0.0", + "hyper 1.3.1", + "pin-project-lite", + "socket2", + "tokio", + "tower", + "tower-service", + "tracing", +] + [[package]] name = "iana-time-zone" version = "0.1.60" @@ -4316,7 +4420,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b5b005c793122d03217da09af68ba9383363caa950b90d3436106df8cabce935" dependencies = [ "futures-util", - "http", + "http 0.2.12", "jsonrpsee-core", "pin-project", "rustls-native-certs", @@ -4341,7 +4445,7 @@ dependencies = [ "beef", "futures-timer", "futures-util", - "hyper", + "hyper 0.14.28", "jsonrpsee-types", "parking_lot", "rand 0.8.5", @@ -4361,7 +4465,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5f80c17f62c7653ce767e3d7288b793dfec920f97067ceb189ebdd3570f2bc20" dependencies = [ "async-trait", - "hyper", + "hyper 0.14.28", "hyper-rustls", "jsonrpsee-core", "jsonrpsee-types", @@ -4394,8 +4498,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "82c39a00449c9ef3f50b84fc00fc4acba20ef8f559f07902244abf4c15c5ab9c" dependencies = [ "futures-util", - "http", - "hyper", + "http 0.2.12", + "hyper 0.14.28", "jsonrpsee-core", "jsonrpsee-types", "route-recognizer", @@ -4430,7 +4534,7 @@ version = "0.20.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bca9cb3933ccae417eb6b08c3448eb1cb46e39834e5b503e395e5e5bd08546c0" dependencies = [ - "http", + "http 0.2.12", "jsonrpsee-client-transport", "jsonrpsee-core", "jsonrpsee-types", @@ -4719,33 +4823,46 @@ dependencies = [ "portable-atomic", ] +[[package]] +name = "metrics" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "884adb57038347dfbaf2d5065887b6cf4312330dc8e94bc30a1a839bd79d3261" +dependencies = [ + "ahash", + "portable-atomic", +] + [[package]] name = "metrics-exporter-prometheus" -version = "0.13.1" +version = "0.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9bf4e7146e30ad172c42c39b3246864bd2d3c6396780711a1baf749cfe423e21" +checksum = "26eb45aff37b45cff885538e1dcbd6c2b462c04fe84ce0155ea469f325672c98" dependencies = [ - "base64 0.21.7", - "hyper", + "base64 0.22.1", + "http-body-util", + "hyper 1.3.1", + "hyper-util", "indexmap 2.2.6", "ipnet", - "metrics", + "metrics 0.23.0", "metrics-util", "quanta", "thiserror", "tokio", + "tracing", ] [[package]] name = "metrics-util" -version = "0.16.3" +version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b07a5eb561b8cbc16be2d216faf7757f9baf3bfb94dbb0fae3df8387a5bb47f" +checksum = "4259040465c955f9f2f1a4a8a16dc46726169bca0f88e8fb2dbeced487c3e828" dependencies = [ "crossbeam-epoch", "crossbeam-utils", "hashbrown 0.14.3", - "metrics", + "metrics 0.23.0", "num_cpus", "quanta", "sketches-ddsketch", @@ -5139,7 +5256,7 @@ checksum = "1a016b8d9495c639af2145ac22387dcb88e44118e45320d9238fbf4e7889abcb" dependencies = [ "async-trait", "futures-core", - "http", + "http 0.2.12", "opentelemetry", "opentelemetry-proto", "opentelemetry-semantic-conventions", @@ -5488,7 +5605,7 @@ dependencies = [ "ibc-proto", "ibc-types", "ics23", - "metrics", + "metrics 0.22.3", "num-traits", "once_cell", "pbjson-types", @@ -5638,7 +5755,7 @@ dependencies = [ "decaf377-rdsa", "hex", "im", - "metrics", + "metrics 0.22.3", "once_cell", "penumbra-keys", "penumbra-proto", @@ -5687,7 +5804,7 @@ source = "git+https://github.com/penumbra-zone/penumbra.git?tag=v0.77.2#76aab4b8 dependencies = [ "futures", "hex", - "http", + "http 0.2.12", "pin-project", "pin-project-lite", "sha2 0.10.8", @@ -6410,10 +6527,10 @@ dependencies = [ "encoding_rs", "futures-core", "futures-util", - "h2", - "http", - "http-body", - "hyper", + "h2 0.3.26", + "http 0.2.12", + "http-body 0.4.6", + "hyper 0.14.28", "hyper-rustls", "ipnet", "js-sys", @@ -7249,7 +7366,7 @@ dependencies = [ "base64 0.13.1", "bytes", "futures", - "http", + "http 0.2.12", "httparse", "log", "rand 0.8.5", @@ -7948,10 +8065,10 @@ dependencies = [ "axum", "base64 0.21.7", "bytes", - "h2", - "http", - "http-body", - "hyper", + "h2 0.3.26", + "http 0.2.12", + "http-body 0.4.6", + "hyper 0.14.28", "hyper-timeout", "percent-encoding", "pin-project", @@ -7980,10 +8097,10 @@ dependencies = [ "base64 0.21.7", "bytes", "flate2", - "h2", - "http", - "http-body", - "hyper", + "h2 0.3.26", + "http 0.2.12", + "http-body 0.4.6", + "hyper 0.14.28", "hyper-timeout", "percent-encoding", "pin-project", @@ -8087,8 +8204,8 @@ dependencies = [ "bytes", "futures-core", "futures-util", - "http", - "http-body", + "http 0.2.12", + "http-body 0.4.6", "http-range-header", "pin-project-lite", "tower-layer", @@ -8296,7 +8413,7 @@ dependencies = [ "byteorder", "bytes", "data-encoding", - "http", + "http 0.2.12", "httparse", "log", "rand 0.8.5", @@ -8880,7 +8997,7 @@ dependencies = [ "futures", "futures-timer", "http-types", - "hyper", + "hyper 0.14.28", "log", "once_cell", "regex", diff --git a/Cargo.toml b/Cargo.toml index 48a4b6ad67..77f4312530 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -74,7 +74,6 @@ pin-project-lite = "0.2.13" sha2 = "0.10" serde = "1" serde_json = "1" -metrics = "0.22.1" pbjson-types = "0.6" # Note that when updating the penumbra versions, vendored types in `proto/sequencerapis/astria_vendored` may need to be updated as well. penumbra-ibc = { git = "https://github.com/penumbra-zone/penumbra.git", tag = "v0.77.2", default-features = false } diff --git a/crates/astria-bridge-withdrawer/Cargo.toml b/crates/astria-bridge-withdrawer/Cargo.toml index 8a9cf78792..9703ec8477 100644 --- a/crates/astria-bridge-withdrawer/Cargo.toml +++ b/crates/astria-bridge-withdrawer/Cargo.toml @@ -21,7 +21,6 @@ ethers = { workspace = true, features = ["ethers-solc", "ws"] } hyper = { workspace = true } humantime = { workspace = true } ibc-types = { workspace = true } -metrics = { workspace = true } once_cell = { workspace = true } pin-project-lite = { workspace = true } prost = { workspace = true } diff --git a/crates/astria-bridge-withdrawer/src/main.rs b/crates/astria-bridge-withdrawer/src/main.rs index a3b87a36df..5b070bbcf6 100644 --- a/crates/astria-bridge-withdrawer/src/main.rs +++ b/crates/astria-bridge-withdrawer/src/main.rs @@ -29,21 +29,23 @@ async fn main() -> ExitCode { .set_no_otel(cfg.no_otel) .set_force_stdout(cfg.force_stdout) .set_pretty_print(cfg.pretty_print) - .filter_directives(&cfg.log); + .set_filter_directives(&cfg.log); if !cfg.no_metrics { - telemetry_conf = telemetry_conf - .metrics_addr(&cfg.metrics_http_listener_addr) - .service_name(env!("CARGO_PKG_NAME")); + telemetry_conf = + telemetry_conf.set_metrics(&cfg.metrics_http_listener_addr, env!("CARGO_PKG_NAME")); } - if let Err(e) = telemetry_conf - .try_init() + let (metrics, _telemetry_guard) = match telemetry_conf + .try_init(&()) .wrap_err("failed to setup telemetry") { - eprintln!("initializing conductor failed:\n{e:?}"); - return ExitCode::FAILURE; - } + Err(e) => { + eprintln!("initializing conductor failed:\n{e:?}"); + return ExitCode::FAILURE; + } + Ok(metrics_and_guard) => metrics_and_guard, + }; info!( config = serde_json::to_string(&cfg).expect("serializing to a string cannot fail"), @@ -52,7 +54,8 @@ async fn main() -> ExitCode { let mut sigterm = signal(SignalKind::terminate()) .expect("setting a SIGTERM listener should always work on Unix"); - let (withdrawer, shutdown_handle) = Service::new(cfg).expect("could not initialize withdrawer"); + let (withdrawer, shutdown_handle) = + Service::new(cfg, metrics).expect("could not initialize withdrawer"); let withdrawer_handle = tokio::spawn(withdrawer.run()); let shutdown_token = shutdown_handle.token(); diff --git a/crates/astria-bridge-withdrawer/src/metrics.rs b/crates/astria-bridge-withdrawer/src/metrics.rs index a409768d8e..b05fcd7a36 100644 --- a/crates/astria-bridge-withdrawer/src/metrics.rs +++ b/crates/astria-bridge-withdrawer/src/metrics.rs @@ -1,20 +1,17 @@ use std::time::Duration; -use metrics::{ - counter, - describe_counter, - describe_gauge, - describe_histogram, - gauge, - histogram, - Counter, - Gauge, - Histogram, - Unit, +use telemetry::{ + metric_names, + metrics::{ + self, + Counter, + Gauge, + Histogram, + RegisteringBuilder, + }, }; -use telemetry::metric_names; -pub(crate) struct Metrics { +pub struct Metrics { nonce_fetch_count: Counter, nonce_fetch_failure_count: Counter, nonce_fetch_latency: Histogram, @@ -24,56 +21,6 @@ pub(crate) struct Metrics { } impl Metrics { - #[must_use] - pub(crate) fn new() -> Self { - describe_counter!( - NONCE_FETCH_COUNT, - Unit::Count, - "The number of times we have attempted to fetch the nonce" - ); - let nonce_fetch_count = counter!(NONCE_FETCH_COUNT); - - describe_counter!( - NONCE_FETCH_FAILURE_COUNT, - Unit::Count, - "The number of times we have failed to fetch the nonce" - ); - let nonce_fetch_failure_count = counter!(NONCE_FETCH_FAILURE_COUNT); - - describe_histogram!( - NONCE_FETCH_LATENCY, - Unit::Seconds, - "The latency of nonce fetch" - ); - let nonce_fetch_latency = histogram!(NONCE_FETCH_LATENCY); - - describe_gauge!(CURRENT_NONCE, Unit::Count, "The current nonce"); - let current_nonce = gauge!(CURRENT_NONCE); - - describe_counter!( - SEQUENCER_SUBMISSION_FAILURE_COUNT, - Unit::Count, - "The number of failed transaction submissions to the sequencer" - ); - let sequencer_submission_failure_count = counter!(SEQUENCER_SUBMISSION_FAILURE_COUNT); - - describe_histogram!( - SEQUENCER_SUBMISSION_LATENCY, - Unit::Seconds, - "The latency of submitting a transaction to the sequencer" - ); - let sequencer_submission_latency = histogram!(SEQUENCER_SUBMISSION_LATENCY); - - Self { - nonce_fetch_count, - nonce_fetch_failure_count, - nonce_fetch_latency, - current_nonce, - sequencer_submission_failure_count, - sequencer_submission_latency, - } - } - pub(crate) fn increment_nonce_fetch_count(&self) { self.nonce_fetch_count.increment(1); } @@ -99,11 +46,65 @@ impl Metrics { } } -metric_names!(pub const METRICS_NAMES: - CURRENT_NONCE, +impl metrics::Metrics for Metrics { + type Config = (); + + fn register( + builder: &mut RegisteringBuilder, + _config: &Self::Config, + ) -> Result { + let nonce_fetch_count = builder + .new_counter_factory( + NONCE_FETCH_COUNT, + "The number of times we have attempted to fetch the nonce", + )? + .register()?; + + let nonce_fetch_failure_count = builder + .new_counter_factory( + NONCE_FETCH_FAILURE_COUNT, + "The number of times we have failed to fetch the nonce", + )? + .register()?; + + let nonce_fetch_latency = builder + .new_histogram_factory(NONCE_FETCH_LATENCY, "The latency of nonce fetch")? + .register()?; + + let current_nonce = builder + .new_gauge_factory(CURRENT_NONCE, "The current nonce")? + .register()?; + + let sequencer_submission_failure_count = builder + .new_counter_factory( + SEQUENCER_SUBMISSION_FAILURE_COUNT, + "The number of failed transaction submissions to the sequencer", + )? + .register()?; + + let sequencer_submission_latency = builder + .new_histogram_factory( + SEQUENCER_SUBMISSION_LATENCY, + "The latency of submitting a transaction to the sequencer", + )? + .register()?; + + Ok(Self { + nonce_fetch_count, + nonce_fetch_failure_count, + nonce_fetch_latency, + current_nonce, + sequencer_submission_failure_count, + sequencer_submission_latency, + }) + } +} + +metric_names!(const METRICS_NAMES: NONCE_FETCH_COUNT, NONCE_FETCH_FAILURE_COUNT, NONCE_FETCH_LATENCY, + CURRENT_NONCE, SEQUENCER_SUBMISSION_FAILURE_COUNT, SEQUENCER_SUBMISSION_LATENCY ); diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs index fd85ce447d..761ca93cac 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/mod.rs @@ -1,9 +1,6 @@ use std::{ net::SocketAddr, - sync::{ - Arc, - OnceLock, - }, + sync::Arc, time::Duration, }; @@ -65,10 +62,7 @@ impl Service { /// # Errors /// /// - If the provided `api_addr` string cannot be parsed as a socket address. - pub fn new(cfg: Config) -> eyre::Result<(Self, ShutdownHandle)> { - static METRICS: OnceLock = OnceLock::new(); - let metrics = METRICS.get_or_init(Metrics::new); - + pub fn new(cfg: Config, metrics: &'static Metrics) -> eyre::Result<(Self, ShutdownHandle)> { let shutdown_handle = ShutdownHandle::new(); let Config { api_addr, diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs index 9d94048b27..9f6ede40d3 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs @@ -48,6 +48,7 @@ use sequencer_client::{ SignedTransaction, }; use serde_json::json; +use telemetry::metrics::Metrics as _; use tempfile::NamedTempFile; use tendermint::{ abci::{ @@ -100,18 +101,18 @@ const DEFAULT_IBC_DENOM: &str = "transfer/channel-0/utia"; static TELEMETRY: Lazy<()> = Lazy::new(|| { if std::env::var_os("TEST_LOG").is_some() { let filter_directives = std::env::var("RUST_LOG").unwrap_or_else(|_| "info".into()); - telemetry::configure() - .no_otel() - .stdout_writer(std::io::stdout) + let _ = telemetry::configure() + .set_no_otel(true) + .set_stdout_writer(std::io::stdout) .set_pretty_print(true) - .filter_directives(&filter_directives) - .try_init() + .set_filter_directives(&filter_directives) + .try_init::(&()) .unwrap(); } else { - telemetry::configure() - .no_otel() - .stdout_writer(std::io::sink) - .try_init() + let _ = telemetry::configure() + .set_no_otel(true) + .set_stdout_writer(std::io::sink) + .try_init::(&()) .unwrap(); } }); @@ -148,7 +149,7 @@ impl TestSubmitter { // not testing watcher here so just set it to ready state.set_watcher_ready(); - let metrics = Box::leak(Box::new(Metrics::new())); + let metrics = Box::leak(Box::new(Metrics::noop_metrics(&()).unwrap())); let (submitter, submitter_handle) = submitter::Builder { shutdown_token: shutdown_token.clone(), diff --git a/crates/astria-composer/Cargo.toml b/crates/astria-composer/Cargo.toml index d37f07078b..7758b05145 100644 --- a/crates/astria-composer/Cargo.toml +++ b/crates/astria-composer/Cargo.toml @@ -53,7 +53,6 @@ tracing = { workspace = true, features = ["attributes"] } tryhard = { workspace = true } tonic = { workspace = true } tokio-stream = { workspace = true, features = ["net"] } -metrics = { workspace = true } [dependencies.sequencer-client] package = "astria-sequencer-client" diff --git a/crates/astria-composer/src/collectors/geth.rs b/crates/astria-composer/src/collectors/geth.rs index 0256822236..c04560401e 100644 --- a/crates/astria-composer/src/collectors/geth.rs +++ b/crates/astria-composer/src/collectors/geth.rs @@ -33,7 +33,7 @@ use ethers::providers::{ ProviderError, Ws, }; -use metrics::Counter; +use telemetry::metrics::Counter; use tokio::{ select, sync::{ diff --git a/crates/astria-composer/src/composer.rs b/crates/astria-composer/src/composer.rs index 9dcea0582a..c9fd105d93 100644 --- a/crates/astria-composer/src/composer.rs +++ b/crates/astria-composer/src/composer.rs @@ -1,7 +1,6 @@ use std::{ collections::HashMap, net::SocketAddr, - sync::OnceLock, time::Duration, }; @@ -115,12 +114,7 @@ impl Composer { /// /// An error is returned if the composer fails to be initialized. /// See `[from_config]` for its error scenarios. - pub async fn from_config(cfg: &Config) -> eyre::Result { - static METRICS: OnceLock = OnceLock::new(); - - let rollups = cfg.parse_rollups()?; - let metrics = METRICS.get_or_init(|| Metrics::new(rollups.keys())); - + pub async fn from_config(cfg: &Config, metrics: &'static Metrics) -> eyre::Result { let (composer_status_sender, _) = watch::channel(Status::default()); let shutdown_token = CancellationToken::new(); @@ -159,6 +153,7 @@ impl Composer { "API server listening" ); + let rollups = cfg.parse_rollups()?; let geth_collectors = rollups .iter() .map(|(rollup_name, url)| { diff --git a/crates/astria-composer/src/config.rs b/crates/astria-composer/src/config.rs index 07a4a5d09c..dea20df16d 100644 --- a/crates/astria-composer/src/config.rs +++ b/crates/astria-composer/src/config.rs @@ -3,13 +3,15 @@ use std::{ net::SocketAddr, }; -use astria_eyre::eyre::WrapErr; use serde::{ Deserialize, Serialize, }; -use crate::rollup::Rollup; +use crate::rollup::{ + ParseError, + Rollup, +}; // this is a config, may have many boolean values #[allow(clippy::struct_excessive_bools)] @@ -65,13 +67,17 @@ pub struct Config { } impl Config { - pub(crate) fn parse_rollups(&self) -> astria_eyre::eyre::Result> { + /// Returns a map of rollup names to rollup URLs. + /// + /// # Errors + /// + /// Returns an error if parsing fails. + pub fn parse_rollups(&self) -> Result, ParseError> { self.rollups .split(',') .filter(|s| !s.is_empty()) .map(|s| Rollup::parse(s).map(Rollup::into_parts)) .collect::, _>>() - .wrap_err("failed parsing provided :: pairs as rollups") } } diff --git a/crates/astria-composer/src/executor/tests.rs b/crates/astria-composer/src/executor/tests.rs index 900b884299..f18fbb281b 100644 --- a/crates/astria-composer/src/executor/tests.rs +++ b/crates/astria-composer/src/executor/tests.rs @@ -1,5 +1,9 @@ use std::{ io::Write, + net::{ + IpAddr, + SocketAddr, + }, time::Duration, }; @@ -17,6 +21,7 @@ use once_cell::sync::Lazy; use prost::Message; use sequencer_client::SignedTransaction; use serde_json::json; +use telemetry::metrics::Metrics as _; use tempfile::NamedTempFile; use tendermint_rpc::{ endpoint::broadcast::tx_sync, @@ -49,19 +54,38 @@ use crate::{ }; static TELEMETRY: Lazy<()> = Lazy::new(|| { + // This config can be meaningless - it's only used inside `try_init` to init the metrics, but we + // haven't configured telemetry to provide metrics here. + let config = Config { + log: String::new(), + api_listen_addr: SocketAddr::new(IpAddr::from([0, 0, 0, 0]), 0), + sequencer_url: String::new(), + sequencer_chain_id: String::new(), + rollups: String::new(), + private_key_file: String::new(), + block_time_ms: 0, + max_bytes_per_bundle: 0, + bundle_queue_capacity: 0, + force_stdout: false, + no_otel: false, + no_metrics: false, + metrics_http_listener_addr: String::new(), + pretty_print: false, + grpc_addr: SocketAddr::new(IpAddr::from([0, 0, 0, 0]), 0), + }; if std::env::var_os("TEST_LOG").is_some() { let filter_directives = std::env::var("RUST_LOG").unwrap_or_else(|_| "info".into()); telemetry::configure() - .no_otel() - .stdout_writer(std::io::stdout) - .filter_directives(&filter_directives) - .try_init() + .set_no_otel(true) + .set_stdout_writer(std::io::stdout) + .set_filter_directives(&filter_directives) + .try_init::(&config) .unwrap(); } else { telemetry::configure() - .no_otel() - .stdout_writer(std::io::sink) - .try_init() + .set_no_otel(true) + .set_stdout_writer(std::io::sink) + .try_init::(&config) .unwrap(); } }); @@ -209,7 +233,7 @@ async fn full_bundle() { // set up the executor, channel for writing seq actions, and the sequencer mock let (sequencer, nonce_guard, cfg, _keyfile) = setup().await; let shutdown_token = CancellationToken::new(); - let metrics = Box::leak(Box::new(Metrics::new(cfg.parse_rollups().unwrap().keys()))); + let metrics = Box::leak(Box::new(Metrics::noop_metrics(&cfg).unwrap())); let (executor, executor_handle) = executor::Builder { sequencer_url: cfg.sequencer_url.clone(), sequencer_chain_id: cfg.sequencer_chain_id.clone(), @@ -302,7 +326,7 @@ async fn bundle_triggered_by_block_timer() { // set up the executor, channel for writing seq actions, and the sequencer mock let (sequencer, nonce_guard, cfg, _keyfile) = setup().await; let shutdown_token = CancellationToken::new(); - let metrics = Box::leak(Box::new(Metrics::new(cfg.parse_rollups().unwrap().keys()))); + let metrics = Box::leak(Box::new(Metrics::noop_metrics(&cfg).unwrap())); let (executor, executor_handle) = executor::Builder { sequencer_url: cfg.sequencer_url.clone(), sequencer_chain_id: cfg.sequencer_chain_id.clone(), @@ -388,7 +412,7 @@ async fn two_seq_actions_single_bundle() { // set up the executor, channel for writing seq actions, and the sequencer mock let (sequencer, nonce_guard, cfg, _keyfile) = setup().await; let shutdown_token = CancellationToken::new(); - let metrics = Box::leak(Box::new(Metrics::new(cfg.parse_rollups().unwrap().keys()))); + let metrics = Box::leak(Box::new(Metrics::noop_metrics(&cfg).unwrap())); let (executor, executor_handle) = executor::Builder { sequencer_url: cfg.sequencer_url.clone(), sequencer_chain_id: cfg.sequencer_chain_id.clone(), diff --git a/crates/astria-composer/src/lib.rs b/crates/astria-composer/src/lib.rs index 881c2ebc96..a59ae3b7a3 100644 --- a/crates/astria-composer/src/lib.rs +++ b/crates/astria-composer/src/lib.rs @@ -16,7 +16,7 @@ //! # Composer, //! # Config, //! # telemetry, -//! }; +//! # }; //! # use tracing::info; //! # tokio_test::block_on(async { //! let cfg: Config = config::get().expect("failed to read configuration"); @@ -24,13 +24,13 @@ //! .expect("the json serializer should never fail when serializing to a string"); //! eprintln!("config:\n{cfg_ser}"); //! -//! telemetry::configure() -//! .filter_directives(&cfg.log) -//! .try_init() +//! let (metrics, _telemetry_guard) = telemetry::configure() +//! .set_filter_directives(&cfg.log) +//! .try_init(&cfg) //! .expect("failed to setup telemetry"); //! info!(config = cfg_ser, "initializing composer",); //! -//! let _composer = Composer::from_config(&cfg) +//! let _composer = Composer::from_config(&cfg, metrics) //! .await //! .expect("failed creating composer") //! .run_until_stopped() @@ -51,4 +51,5 @@ mod rollup; pub use build_info::BUILD_INFO; pub use composer::Composer; pub use config::Config; +pub use metrics::Metrics; pub use telemetry; diff --git a/crates/astria-composer/src/main.rs b/crates/astria-composer/src/main.rs index 803301886b..37e1631755 100644 --- a/crates/astria-composer/src/main.rs +++ b/crates/astria-composer/src/main.rs @@ -32,23 +32,22 @@ async fn main() -> ExitCode { .set_no_otel(cfg.no_otel) .set_force_stdout(cfg.force_stdout) .set_pretty_print(cfg.pretty_print) - .filter_directives(&cfg.log); + .set_filter_directives(&cfg.log); if !cfg.no_metrics { - telemetry_conf = telemetry_conf - .metrics_addr(&cfg.metrics_http_listener_addr) - .service_name(env!("CARGO_PKG_NAME")); + telemetry_conf = + telemetry_conf.set_metrics(&cfg.metrics_http_listener_addr, env!("CARGO_PKG_NAME")); } - let _telemetry_guard = match telemetry_conf - .try_init() + let (metrics, _telemetry_guard) = match telemetry_conf + .try_init(&cfg) .wrap_err("failed to setup telemetry") { Err(e) => { eprintln!("initializing composer failed:\n{e:?}"); return ExitCode::FAILURE; } - Ok(guard) => guard, + Ok(metrics_and_guard) => metrics_and_guard, }; let cfg_ser = serde_json::to_string(&cfg) @@ -56,7 +55,7 @@ async fn main() -> ExitCode { eprintln!("config:\n{cfg_ser}"); info!(config = cfg_ser, "initializing composer",); - let composer = match Composer::from_config(&cfg).await { + let composer = match Composer::from_config(&cfg, metrics).await { Err(error) => { error!(%error, "failed initializing Composer"); return ExitCode::FAILURE; diff --git a/crates/astria-composer/src/metrics.rs b/crates/astria-composer/src/metrics.rs index f400356a13..1362815722 100644 --- a/crates/astria-composer/src/metrics.rs +++ b/crates/astria-composer/src/metrics.rs @@ -4,30 +4,31 @@ use std::{ }; use astria_core::primitive::v1::RollupId; -use metrics::{ - counter, - describe_counter, - describe_gauge, - describe_histogram, - gauge, - histogram, - Counter, - Gauge, - Histogram, - Unit, +use telemetry::{ + metric_names, + metrics::{ + self, + Counter, + Error, + Gauge, + Histogram, + RegisteringBuilder, + }, }; -use telemetry::metric_names; use tracing::error; +type GethCounters = HashMap; +type GrpcCounters = HashMap; + const ROLLUP_CHAIN_NAME_LABEL: &str = "rollup_chain_name"; const ROLLUP_ID_LABEL: &str = "rollup_id"; const COLLECTOR_TYPE_LABEL: &str = "collector_type"; -pub(crate) struct Metrics { - geth_txs_received: HashMap, - geth_txs_dropped: HashMap, - grpc_txs_received: HashMap, - grpc_txs_dropped: HashMap, +pub struct Metrics { + geth_txs_received: GethCounters, + geth_txs_dropped: GethCounters, + grpc_txs_received: GrpcCounters, + grpc_txs_dropped: GrpcCounters, txs_dropped_too_large: HashMap, nonce_fetch_count: Counter, nonce_fetch_failure_count: Counter, @@ -40,83 +41,6 @@ pub(crate) struct Metrics { } impl Metrics { - #[must_use] - pub(crate) fn new<'a>(rollup_chain_names: impl Iterator + Clone) -> Self { - let (geth_txs_received, grpc_txs_received) = - register_txs_received(rollup_chain_names.clone()); - let (geth_txs_dropped, grpc_txs_dropped) = register_txs_dropped(rollup_chain_names.clone()); - let txs_dropped_too_large = register_txs_dropped_too_large(rollup_chain_names); - - describe_counter!( - NONCE_FETCH_COUNT, - Unit::Count, - "The number of times we have attempted to fetch the nonce" - ); - let nonce_fetch_count = counter!(NONCE_FETCH_COUNT); - - describe_counter!( - NONCE_FETCH_FAILURE_COUNT, - Unit::Count, - "The number of times we have failed to fetch the nonce" - ); - let nonce_fetch_failure_count = counter!(NONCE_FETCH_FAILURE_COUNT); - - describe_histogram!( - NONCE_FETCH_LATENCY, - Unit::Seconds, - "The latency of fetching the nonce, in seconds" - ); - let nonce_fetch_latency = histogram!(NONCE_FETCH_LATENCY); - - describe_gauge!(CURRENT_NONCE, Unit::Count, "The current nonce"); - let current_nonce = gauge!(CURRENT_NONCE); - - describe_histogram!( - SEQUENCER_SUBMISSION_LATENCY, - Unit::Seconds, - "The latency of submitting a transaction to the sequencer, in seconds" - ); - let sequencer_submission_latency = histogram!(SEQUENCER_SUBMISSION_LATENCY); - - describe_counter!( - SEQUENCER_SUBMISSION_FAILURE_COUNT, - Unit::Count, - "The number of failed transaction submissions to the sequencer" - ); - let sequencer_submission_failure_count = counter!(SEQUENCER_SUBMISSION_FAILURE_COUNT); - - describe_histogram!( - TRANSACTIONS_PER_SUBMISSION, - Unit::Count, - "The number of rollup transactions successfully sent to the sequencer in a single \ - submission" - ); - let txs_per_submission = histogram!(TRANSACTIONS_PER_SUBMISSION); - - describe_histogram!( - BYTES_PER_SUBMISSION, - Unit::Bytes, - "The total bytes successfully sent to the sequencer in a single submission" - ); - let bytes_per_submission = histogram!(BYTES_PER_SUBMISSION); - - Self { - geth_txs_received, - geth_txs_dropped, - grpc_txs_received, - grpc_txs_dropped, - txs_dropped_too_large, - nonce_fetch_count, - nonce_fetch_failure_count, - nonce_fetch_latency, - current_nonce, - sequencer_submission_latency, - sequencer_submission_failure_count, - txs_per_submission, - bytes_per_submission, - } - } - pub(crate) fn geth_txs_received(&self, id: &String) -> Option<&Counter> { self.geth_txs_received.get(id) } @@ -186,15 +110,102 @@ impl Metrics { } } +impl metrics::Metrics for Metrics { + type Config = crate::Config; + + fn register(builder: &mut RegisteringBuilder, config: &Self::Config) -> Result + where + Self: Sized, + { + let rollups = config + .parse_rollups() + .map_err(|error| Error::External(Box::new(error)))?; + let (geth_txs_received, grpc_txs_received) = + register_txs_received(builder, rollups.keys())?; + let (geth_txs_dropped, grpc_txs_dropped) = register_txs_dropped(builder, rollups.keys())?; + let txs_dropped_too_large = register_txs_dropped_too_large(builder, rollups.keys())?; + + let nonce_fetch_count = builder + .new_counter_factory( + NONCE_FETCH_COUNT, + "The number of times we have attempted to fetch the nonce", + )? + .register()?; + + let nonce_fetch_failure_count = builder + .new_counter_factory( + NONCE_FETCH_FAILURE_COUNT, + "The number of times we have failed to fetch the nonce", + )? + .register()?; + + let nonce_fetch_latency = builder + .new_histogram_factory( + NONCE_FETCH_LATENCY, + "The latency of fetching the nonce, in seconds", + )? + .register()?; + + let current_nonce = builder + .new_gauge_factory(CURRENT_NONCE, "The current nonce")? + .register()?; + + let sequencer_submission_latency = builder + .new_histogram_factory( + SEQUENCER_SUBMISSION_LATENCY, + "The latency of submitting a transaction to the sequencer, in seconds", + )? + .register()?; + + let sequencer_submission_failure_count = builder + .new_counter_factory( + SEQUENCER_SUBMISSION_FAILURE_COUNT, + "The number of failed transaction submissions to the sequencer", + )? + .register()?; + + let txs_per_submission = builder + .new_histogram_factory( + TRANSACTIONS_PER_SUBMISSION, + "The number of rollup transactions successfully sent to the sequencer in a single \ + submission", + )? + .register()?; + + let bytes_per_submission = builder + .new_histogram_factory( + BYTES_PER_SUBMISSION, + "The total bytes successfully sent to the sequencer in a single submission", + )? + .register()?; + + Ok(Self { + geth_txs_received, + geth_txs_dropped, + grpc_txs_received, + grpc_txs_dropped, + txs_dropped_too_large, + nonce_fetch_count, + nonce_fetch_failure_count, + nonce_fetch_latency, + current_nonce, + sequencer_submission_latency, + sequencer_submission_failure_count, + txs_per_submission, + bytes_per_submission, + }) + } +} + fn register_txs_received<'a>( + builder: &mut RegisteringBuilder, rollup_chain_names: impl Iterator, -) -> (HashMap, HashMap) { - describe_counter!( +) -> Result<(GethCounters, GrpcCounters), Error> { + let mut factory = builder.new_counter_factory( TRANSACTIONS_RECEIVED, - Unit::Count, "The number of transactions successfully received from collectors and bundled, labelled \ - by rollup and collector type" - ); + by rollup and collector type", + )?; let mut geth_counters = HashMap::new(); let mut grpc_counters = HashMap::new(); @@ -202,34 +213,32 @@ fn register_txs_received<'a>( for chain_name in rollup_chain_names { let rollup_id = RollupId::from_unhashed_bytes(chain_name.as_bytes()); - let geth_counter = counter!( - TRANSACTIONS_RECEIVED, - ROLLUP_CHAIN_NAME_LABEL => chain_name.clone(), - ROLLUP_ID_LABEL => rollup_id.to_string(), - COLLECTOR_TYPE_LABEL => "geth", - ); - geth_counters.insert(chain_name.clone(), geth_counter.clone()); - - let grpc_counter = counter!( - TRANSACTIONS_RECEIVED, - ROLLUP_CHAIN_NAME_LABEL => chain_name.clone(), - ROLLUP_ID_LABEL => rollup_id.to_string(), - COLLECTOR_TYPE_LABEL => "grpc", - ); + let geth_counter = factory.register_with_labels(&[ + (ROLLUP_CHAIN_NAME_LABEL, chain_name.clone()), + (ROLLUP_ID_LABEL, rollup_id.to_string()), + (COLLECTOR_TYPE_LABEL, "geth".to_string()), + ])?; + geth_counters.insert(chain_name.clone(), geth_counter); + + let grpc_counter = factory.register_with_labels(&[ + (ROLLUP_CHAIN_NAME_LABEL, chain_name.clone()), + (ROLLUP_ID_LABEL, rollup_id.to_string()), + (COLLECTOR_TYPE_LABEL, "grpc".to_string()), + ])?; grpc_counters.insert(rollup_id, grpc_counter); } - (geth_counters, grpc_counters) + Ok((geth_counters, grpc_counters)) } fn register_txs_dropped<'a>( + builder: &mut RegisteringBuilder, rollup_chain_names: impl Iterator, -) -> (HashMap, HashMap) { - describe_counter!( +) -> Result<(GethCounters, GrpcCounters), Error> { + let mut factory = builder.new_counter_factory( TRANSACTIONS_DROPPED, - Unit::Count, "The number of transactions dropped by the collectors before bundling, labelled by rollup \ - and collector type" - ); + and collector type", + )?; let mut geth_counters = HashMap::new(); let mut grpc_counters = HashMap::new(); @@ -237,47 +246,44 @@ fn register_txs_dropped<'a>( for chain_name in rollup_chain_names { let rollup_id = RollupId::from_unhashed_bytes(chain_name.as_bytes()); - let geth_counter = counter!( - TRANSACTIONS_DROPPED, - ROLLUP_CHAIN_NAME_LABEL => chain_name.clone(), - ROLLUP_ID_LABEL => rollup_id.to_string(), - COLLECTOR_TYPE_LABEL => "geth", - ); - geth_counters.insert(chain_name.clone(), geth_counter.clone()); - - let grpc_counter = counter!( - TRANSACTIONS_DROPPED, - ROLLUP_CHAIN_NAME_LABEL => chain_name.clone(), - ROLLUP_ID_LABEL => rollup_id.to_string(), - COLLECTOR_TYPE_LABEL => "grpc", - ); + let geth_counter = factory.register_with_labels(&[ + (ROLLUP_CHAIN_NAME_LABEL, chain_name.clone()), + (ROLLUP_ID_LABEL, rollup_id.to_string()), + (COLLECTOR_TYPE_LABEL, "geth".to_string()), + ])?; + geth_counters.insert(chain_name.clone(), geth_counter); + + let grpc_counter = factory.register_with_labels(&[ + (ROLLUP_CHAIN_NAME_LABEL, chain_name.clone()), + (ROLLUP_ID_LABEL, rollup_id.to_string()), + (COLLECTOR_TYPE_LABEL, "grpc".to_string()), + ])?; grpc_counters.insert(rollup_id, grpc_counter); } - (geth_counters, grpc_counters) + Ok((geth_counters, grpc_counters)) } fn register_txs_dropped_too_large<'a>( + builder: &mut RegisteringBuilder, rollup_chain_names: impl Iterator, -) -> HashMap { - describe_counter!( +) -> Result, Error> { + let mut factory = builder.new_counter_factory( TRANSACTIONS_DROPPED_TOO_LARGE, - Unit::Count, - "The number of transactions dropped because they were too large, labelled by rollup" - ); + "The number of transactions dropped because they were too large, labelled by rollup", + )?; let mut counters = HashMap::new(); for chain_name in rollup_chain_names { let rollup_id = RollupId::from_unhashed_bytes(chain_name.as_bytes()); - let counter = counter!( - TRANSACTIONS_DROPPED_TOO_LARGE, - ROLLUP_CHAIN_NAME_LABEL => chain_name.clone(), - ROLLUP_ID_LABEL => rollup_id.to_string(), - ); + let counter = factory.register_with_labels(&[ + (ROLLUP_CHAIN_NAME_LABEL, chain_name.clone()), + (ROLLUP_ID_LABEL, rollup_id.to_string()), + ])?; counters.insert(rollup_id, counter); } - counters + Ok(counters) } metric_names!(pub const METRICS_NAMES: diff --git a/crates/astria-composer/src/rollup.rs b/crates/astria-composer/src/rollup.rs index a5ebe9425c..310fcb98b3 100644 --- a/crates/astria-composer/src/rollup.rs +++ b/crates/astria-composer/src/rollup.rs @@ -12,7 +12,7 @@ pub(super) struct Rollup { } #[derive(Debug)] -pub(super) struct ParseError {} +pub struct ParseError {} impl ParseError { fn new() -> Self { diff --git a/crates/astria-composer/tests/blackbox/helper/mod.rs b/crates/astria-composer/tests/blackbox/helper/mod.rs index ea802b2896..f93b726f57 100644 --- a/crates/astria-composer/tests/blackbox/helper/mod.rs +++ b/crates/astria-composer/tests/blackbox/helper/mod.rs @@ -1,13 +1,17 @@ use std::{ collections::HashMap, io::Write, - net::SocketAddr, + net::{ + IpAddr, + SocketAddr, + }, time::Duration, }; use astria_composer::{ config::Config, Composer, + Metrics, }; use astria_core::{ primitive::v1::RollupId, @@ -19,6 +23,7 @@ use astria_core::{ use astria_eyre::eyre; use ethers::prelude::Transaction; use once_cell::sync::Lazy; +use telemetry::metrics; use tempfile::NamedTempFile; use tendermint_rpc::{ endpoint::broadcast::tx_sync, @@ -40,19 +45,40 @@ use wiremock::{ pub mod mock_sequencer; static TELEMETRY: Lazy<()> = Lazy::new(|| { + // This config can be meaningless - it's only used inside `try_init` to init the metrics, but we + // haven't configured telemetry to provide metrics here. + let config = Config { + log: String::new(), + api_listen_addr: SocketAddr::new(IpAddr::from([0, 0, 0, 0]), 0), + sequencer_url: String::new(), + sequencer_chain_id: String::new(), + rollups: String::new(), + private_key_file: String::new(), + block_time_ms: 0, + max_bytes_per_bundle: 0, + bundle_queue_capacity: 0, + force_stdout: false, + no_otel: false, + no_metrics: false, + metrics_http_listener_addr: String::new(), + pretty_print: false, + grpc_addr: SocketAddr::new(IpAddr::from([0, 0, 0, 0]), 0), + }; if std::env::var_os("TEST_LOG").is_some() { let filter_directives = std::env::var("RUST_LOG").unwrap_or_else(|_| "info".into()); telemetry::configure() - .no_otel() - .stdout_writer(std::io::stdout) - .filter_directives(&filter_directives) - .try_init() + .set_no_otel(true) + .set_stdout_writer(std::io::stdout) + .set_force_stdout(true) + .set_pretty_print(true) + .set_filter_directives(&filter_directives) + .try_init::(&config) .unwrap(); } else { telemetry::configure() - .no_otel() - .stdout_writer(std::io::sink) - .try_init() + .set_no_otel(true) + .set_stdout_writer(std::io::sink) + .try_init::(&config) .unwrap(); } }); @@ -64,6 +90,7 @@ pub struct TestComposer { pub sequencer: wiremock::MockServer, pub setup_guard: MockGuard, pub grpc_collector_addr: SocketAddr, + pub metrics_handle: metrics::Handle, } /// Spawns composer in a test environment. @@ -105,8 +132,15 @@ pub async fn spawn_composer(rollup_ids: &[&str]) -> TestComposer { pretty_print: true, grpc_addr: "127.0.0.1:0".parse().unwrap(), }; + + let (metrics, metrics_handle) = metrics::ConfigBuilder::new() + .with_global_recorder(false) + .build(&config) + .unwrap(); + let metrics = Box::leak(Box::new(metrics)); + let (composer_addr, grpc_collector_addr, composer_handle) = { - let composer = Composer::from_config(&config).await.unwrap(); + let composer = Composer::from_config(&config, metrics).await.unwrap(); let composer_addr = composer.local_addr(); let grpc_collector_addr = composer.grpc_local_addr().unwrap(); let task = tokio::spawn(composer.run_until_stopped()); @@ -121,6 +155,7 @@ pub async fn spawn_composer(rollup_ids: &[&str]) -> TestComposer { sequencer, setup_guard: sequencer_setup_guard, grpc_collector_addr, + metrics_handle, } } diff --git a/crates/astria-conductor/Cargo.toml b/crates/astria-conductor/Cargo.toml index 32ac1b936d..5271ba2cbd 100644 --- a/crates/astria-conductor/Cargo.toml +++ b/crates/astria-conductor/Cargo.toml @@ -69,7 +69,6 @@ tower = { version = "0.4.13", features = ["limit"] } celestia-rpc = "0.1.1" celestia-types = { workspace = true } jsonrpsee = { version = "0.20", features = ["client-core", "macros"] } -metrics.workspace = true [dev-dependencies] astria-core = { path = "../astria-core", features = [ diff --git a/crates/astria-conductor/src/conductor.rs b/crates/astria-conductor/src/conductor.rs index b924294db2..16e9ea0700 100644 --- a/crates/astria-conductor/src/conductor.rs +++ b/crates/astria-conductor/src/conductor.rs @@ -1,6 +1,5 @@ use std::{ future::Future, - sync::OnceLock, time::Duration, }; @@ -95,10 +94,7 @@ impl Conductor { /// Returns an error in the following cases if one of its constituent /// actors could not be spawned (executor, sequencer reader, or data availability reader). /// This usually happens if the actors failed to connect to their respective endpoints. - pub fn new(cfg: Config) -> eyre::Result { - static METRICS: OnceLock = OnceLock::new(); - let metrics = METRICS.get_or_init(Metrics::new); - + pub fn new(cfg: Config, metrics: &'static Metrics) -> eyre::Result { let mut tasks = JoinMap::new(); let sequencer_cometbft_client = HttpClient::new(&*cfg.sequencer_cometbft_url) diff --git a/crates/astria-conductor/src/lib.rs b/crates/astria-conductor/src/lib.rs index ff40fef07f..eb9e88c0db 100644 --- a/crates/astria-conductor/src/lib.rs +++ b/crates/astria-conductor/src/lib.rs @@ -21,3 +21,4 @@ mod utils; pub use build_info::BUILD_INFO; pub use conductor::Conductor; pub use config::Config; +pub use metrics::Metrics; diff --git a/crates/astria-conductor/src/main.rs b/crates/astria-conductor/src/main.rs index 7468e0ee2a..4732194369 100644 --- a/crates/astria-conductor/src/main.rs +++ b/crates/astria-conductor/src/main.rs @@ -48,23 +48,22 @@ async fn main() -> ExitCode { .set_no_otel(cfg.no_otel) .set_force_stdout(cfg.force_stdout) .set_pretty_print(cfg.pretty_print) - .filter_directives(&cfg.log); + .set_filter_directives(&cfg.log); if !cfg.no_metrics { - telemetry_conf = telemetry_conf - .metrics_addr(&cfg.metrics_http_listener_addr) - .service_name(env!("CARGO_PKG_NAME")); + telemetry_conf = + telemetry_conf.set_metrics(&cfg.metrics_http_listener_addr, env!("CARGO_PKG_NAME")); } - let _telemetry_guard = match telemetry_conf - .try_init() + let (metrics, _telemetry_guard) = match telemetry_conf + .try_init(&()) .wrap_err("failed to setup telemetry") { Err(e) => { eprintln!("initializing conductor failed:\n{e:?}"); return ExitCode::FAILURE; } - Ok(guard) => guard, + Ok(metrics_and_guard) => metrics_and_guard, }; info!( @@ -72,7 +71,7 @@ async fn main() -> ExitCode { "initializing conductor" ); - let conductor = match Conductor::new(cfg) { + let conductor = match Conductor::new(cfg, metrics) { Err(error) => { error!(%error, "failed initializing conductor"); return ExitCode::FAILURE; diff --git a/crates/astria-conductor/src/metrics.rs b/crates/astria-conductor/src/metrics.rs index d992611e23..165ef0282d 100644 --- a/crates/astria-conductor/src/metrics.rs +++ b/crates/astria-conductor/src/metrics.rs @@ -1,19 +1,16 @@ -use metrics::{ - counter, - describe_counter, - describe_histogram, - histogram, - Counter, - Histogram, - Unit, +use telemetry::{ + metric_names, + metrics::{ + self, + Counter, + Histogram, + RegisteringBuilder, + }, }; -use telemetry::metric_names; const NAMESPACE_TYPE_LABEL: &str = "namespace_type"; -const NAMESPACE_TYPE_METADATA: &str = "metadata"; -const NAMESPACE_TYPE_ROLLUP_DATA: &str = "rollup_data"; -pub(crate) struct Metrics { +pub struct Metrics { metadata_blobs_per_celestia_fetch: Histogram, rollup_data_blobs_per_celestia_fetch: Histogram, celestia_blob_fetch_error_count: Counter, @@ -27,97 +24,6 @@ pub(crate) struct Metrics { } impl Metrics { - #[must_use] - pub(crate) fn new() -> Self { - describe_histogram!( - BLOBS_PER_CELESTIA_FETCH, - Unit::Count, - "The number of Celestia blobs received per request sent" - ); - let metadata_blobs_per_celestia_fetch = histogram!( - BLOBS_PER_CELESTIA_FETCH, - NAMESPACE_TYPE_LABEL => NAMESPACE_TYPE_METADATA, - ); - let rollup_data_blobs_per_celestia_fetch = histogram!( - BLOBS_PER_CELESTIA_FETCH, - NAMESPACE_TYPE_LABEL => NAMESPACE_TYPE_ROLLUP_DATA, - ); - - describe_counter!( - CELESTIA_BLOB_FETCH_ERROR_COUNT, - Unit::Count, - "The number of calls made to fetch a blob from Celestia which have failed" - ); - let celestia_blob_fetch_error_count = counter!(CELESTIA_BLOB_FETCH_ERROR_COUNT); - - describe_histogram!( - DECODED_ITEMS_PER_CELESTIA_FETCH, - Unit::Count, - "The number of items decoded from the Celestia blobs received per request sent" - ); - let decoded_metadata_items_per_celestia_fetch = histogram!( - DECODED_ITEMS_PER_CELESTIA_FETCH, - NAMESPACE_TYPE_LABEL => NAMESPACE_TYPE_METADATA, - ); - let decoded_rollup_data_items_per_celestia_fetch = histogram!( - DECODED_ITEMS_PER_CELESTIA_FETCH, - NAMESPACE_TYPE_LABEL => NAMESPACE_TYPE_ROLLUP_DATA, - ); - - describe_histogram!( - SEQUENCER_BLOCKS_METADATA_VERIFIED_PER_CELESTIA_FETCH, - Unit::Count, - "The number of sequencer blocks in a single Celestia blob fetch whose metadata was \ - verified" - ); - let sequencer_blocks_metadata_verified_per_celestia_fetch = - histogram!(SEQUENCER_BLOCKS_METADATA_VERIFIED_PER_CELESTIA_FETCH); - - describe_histogram!( - SEQUENCER_BLOCK_INFORMATION_RECONSTRUCTED_PER_CELESTIA_FETCH, - Unit::Count, - "The number of sequencer blocks (or specifically, the subset pertaining to the \ - rollup) reconstructed from a single Celestia blob fetch" - ); - let sequencer_block_information_reconstructed_per_celestia_fetch = - histogram!(SEQUENCER_BLOCK_INFORMATION_RECONSTRUCTED_PER_CELESTIA_FETCH); - - describe_counter!( - EXECUTED_FIRM_BLOCK_NUMBER, - Unit::Count, - "The number/rollup height of the last executed or confirmed firm block" - ); - let executed_firm_block_number = counter!(EXECUTED_FIRM_BLOCK_NUMBER); - - describe_counter!( - EXECUTED_SOFT_BLOCK_NUMBER, - Unit::Count, - "The number/rollup height of the last executed soft block" - ); - let executed_soft_block_number = counter!(EXECUTED_SOFT_BLOCK_NUMBER); - - describe_histogram!( - TRANSACTIONS_PER_EXECUTED_BLOCK, - Unit::Count, - "The number of transactions that were included in the latest block executed against \ - the rollup" - ); - let transactions_per_executed_block = histogram!(TRANSACTIONS_PER_EXECUTED_BLOCK); - - Self { - metadata_blobs_per_celestia_fetch, - rollup_data_blobs_per_celestia_fetch, - celestia_blob_fetch_error_count, - decoded_metadata_items_per_celestia_fetch, - decoded_rollup_data_items_per_celestia_fetch, - sequencer_blocks_metadata_verified_per_celestia_fetch, - sequencer_block_information_reconstructed_per_celestia_fetch, - executed_firm_block_number, - executed_soft_block_number, - transactions_per_executed_block, - } - } - pub(crate) fn record_metadata_blobs_per_celestia_fetch(&self, blob_count: usize) { // allow: precision loss is unlikely (values too small) but also unimportant in histograms. #[allow(clippy::cast_precision_loss)] @@ -187,7 +93,95 @@ impl Metrics { } } -metric_names!(pub const METRICS_NAMES: +impl metrics::Metrics for Metrics { + type Config = (); + + fn register( + builder: &mut RegisteringBuilder, + _config: &Self::Config, + ) -> Result { + let metadata = "metadata".to_string(); + let rollup_data = "rollup_data".to_string(); + + let mut factory = builder.new_histogram_factory( + BLOBS_PER_CELESTIA_FETCH, + "The number of Celestia blobs received per request sent", + )?; + let metadata_blobs_per_celestia_fetch = + factory.register_with_labels(&[(NAMESPACE_TYPE_LABEL, metadata.clone())])?; + let rollup_data_blobs_per_celestia_fetch = + factory.register_with_labels(&[(NAMESPACE_TYPE_LABEL, rollup_data.clone())])?; + + let celestia_blob_fetch_error_count = builder + .new_counter_factory( + CELESTIA_BLOB_FETCH_ERROR_COUNT, + "The number of calls made to fetch a blob from Celestia which have failed", + )? + .register()?; + + let mut factory = builder.new_histogram_factory( + DECODED_ITEMS_PER_CELESTIA_FETCH, + "The number of items decoded from the Celestia blobs received per request sent", + )?; + let decoded_metadata_items_per_celestia_fetch = + factory.register_with_labels(&[(NAMESPACE_TYPE_LABEL, metadata)])?; + let decoded_rollup_data_items_per_celestia_fetch = + factory.register_with_labels(&[(NAMESPACE_TYPE_LABEL, rollup_data)])?; + + let sequencer_blocks_metadata_verified_per_celestia_fetch = builder + .new_histogram_factory( + SEQUENCER_BLOCKS_METADATA_VERIFIED_PER_CELESTIA_FETCH, + "The number of sequencer blocks in a single Celestia blob fetch whose metadata \ + was verified", + )? + .register()?; + + let sequencer_block_information_reconstructed_per_celestia_fetch = builder + .new_histogram_factory( + SEQUENCER_BLOCK_INFORMATION_RECONSTRUCTED_PER_CELESTIA_FETCH, + "The number of sequencer blocks (or specifically, the subset pertaining to the \ + rollup) reconstructed from a single Celestia blob fetch", + )? + .register()?; + + let executed_firm_block_number = builder + .new_counter_factory( + EXECUTED_FIRM_BLOCK_NUMBER, + "The number/rollup height of the last executed or confirmed firm block", + )? + .register()?; + + let executed_soft_block_number = builder + .new_counter_factory( + EXECUTED_SOFT_BLOCK_NUMBER, + "The number/rollup height of the last executed soft block", + )? + .register()?; + + let transactions_per_executed_block = builder + .new_histogram_factory( + TRANSACTIONS_PER_EXECUTED_BLOCK, + "The number of transactions that were included in the latest block executed \ + against the rollup", + )? + .register()?; + + Ok(Self { + metadata_blobs_per_celestia_fetch, + rollup_data_blobs_per_celestia_fetch, + celestia_blob_fetch_error_count, + decoded_metadata_items_per_celestia_fetch, + decoded_rollup_data_items_per_celestia_fetch, + sequencer_blocks_metadata_verified_per_celestia_fetch, + sequencer_block_information_reconstructed_per_celestia_fetch, + executed_firm_block_number, + executed_soft_block_number, + transactions_per_executed_block, + }) + } +} + +metric_names!(const METRICS_NAMES: BLOBS_PER_CELESTIA_FETCH, CELESTIA_BLOB_FETCH_ERROR_COUNT, DECODED_ITEMS_PER_CELESTIA_FETCH, @@ -201,8 +195,7 @@ metric_names!(pub const METRICS_NAMES: #[cfg(test)] mod tests { - use super::TRANSACTIONS_PER_EXECUTED_BLOCK; - use crate::metrics::{ + use super::{ BLOBS_PER_CELESTIA_FETCH, CELESTIA_BLOB_FETCH_ERROR_COUNT, DECODED_ITEMS_PER_CELESTIA_FETCH, @@ -210,6 +203,7 @@ mod tests { EXECUTED_SOFT_BLOCK_NUMBER, SEQUENCER_BLOCKS_METADATA_VERIFIED_PER_CELESTIA_FETCH, SEQUENCER_BLOCK_INFORMATION_RECONSTRUCTED_PER_CELESTIA_FETCH, + TRANSACTIONS_PER_EXECUTED_BLOCK, }; #[track_caller] diff --git a/crates/astria-conductor/tests/blackbox/helpers/mod.rs b/crates/astria-conductor/tests/blackbox/helpers/mod.rs index 66f415c2b3..2fb482b98e 100644 --- a/crates/astria-conductor/tests/blackbox/helpers/mod.rs +++ b/crates/astria-conductor/tests/blackbox/helpers/mod.rs @@ -5,6 +5,7 @@ use astria_conductor::{ config::CommitLevel, Conductor, Config, + Metrics, }; use astria_core::{ brotli::compress_bytes, @@ -30,6 +31,7 @@ use sequencer_client::{ tendermint_proto, tendermint_rpc, }; +use telemetry::metrics; #[macro_use] mod macros; @@ -53,19 +55,19 @@ static TELEMETRY: Lazy<()> = Lazy::new(|| { if std::env::var_os("TEST_LOG").is_some() { let filter_directives = std::env::var("RUST_LOG").unwrap_or_else(|_| "info".into()); println!("initializing telemetry"); - telemetry::configure() - .no_otel() - .stdout_writer(std::io::stdout) - .force_stdout() - .pretty_print() - .filter_directives(&filter_directives) - .try_init() + let _ = telemetry::configure() + .set_no_otel(true) + .set_stdout_writer(std::io::stdout) + .set_force_stdout(true) + .set_pretty_print(true) + .set_filter_directives(&filter_directives) + .try_init::(&()) .unwrap(); } else { - telemetry::configure() - .no_otel() - .stdout_writer(std::io::sink) - .try_init() + let _ = telemetry::configure() + .set_no_otel(true) + .set_stdout_writer(std::io::sink) + .try_init::(&()) .unwrap(); } }); @@ -92,8 +94,14 @@ pub async fn spawn_conductor(execution_commit_level: CommitLevel) -> TestConduct ..make_config() }; + let (metrics, metrics_handle) = metrics::ConfigBuilder::new() + .with_global_recorder(false) + .build(&()) + .unwrap(); + let metrics = Box::leak(Box::new(metrics)); + let conductor = { - let conductor = Conductor::new(config).unwrap(); + let conductor = Conductor::new(config, metrics).unwrap(); conductor.spawn() }; @@ -101,6 +109,7 @@ pub async fn spawn_conductor(execution_commit_level: CommitLevel) -> TestConduct conductor, mock_grpc, mock_http, + metrics_handle, } } @@ -108,6 +117,7 @@ pub struct TestConductor { pub conductor: conductor::Handle, pub mock_grpc: MockGrpc, pub mock_http: wiremock::MockServer, + pub metrics_handle: metrics::Handle, } impl Drop for TestConductor { diff --git a/crates/astria-sequencer-relayer/Cargo.toml b/crates/astria-sequencer-relayer/Cargo.toml index 4aebb55c37..cf9f950c8f 100644 --- a/crates/astria-sequencer-relayer/Cargo.toml +++ b/crates/astria-sequencer-relayer/Cargo.toml @@ -27,7 +27,6 @@ hex = { workspace = true, features = ["serde"] } humantime = { workspace = true } hyper = { workspace = true } itoa = { workspace = true } -metrics = { workspace = true } pbjson-types = { workspace = true } pin-project-lite = { workspace = true } prost = { workspace = true } diff --git a/crates/astria-sequencer-relayer/src/lib.rs b/crates/astria-sequencer-relayer/src/lib.rs index 52d922708e..67ec83894d 100644 --- a/crates/astria-sequencer-relayer/src/lib.rs +++ b/crates/astria-sequencer-relayer/src/lib.rs @@ -11,6 +11,7 @@ pub use config::{ Config, IncludeRollup, }; +pub use metrics::Metrics; pub use sequencer_relayer::{ SequencerRelayer, ShutdownHandle, diff --git a/crates/astria-sequencer-relayer/src/main.rs b/crates/astria-sequencer-relayer/src/main.rs index abd203a253..817bd82c1e 100644 --- a/crates/astria-sequencer-relayer/src/main.rs +++ b/crates/astria-sequencer-relayer/src/main.rs @@ -29,23 +29,22 @@ async fn main() -> ExitCode { .set_no_otel(cfg.no_otel) .set_force_stdout(cfg.force_stdout) .set_pretty_print(cfg.pretty_print) - .filter_directives(&cfg.log); + .set_filter_directives(&cfg.log); if !cfg.no_metrics { - telemetry_conf = telemetry_conf - .metrics_addr(&cfg.metrics_http_listener_addr) - .service_name(env!("CARGO_PKG_NAME")); + telemetry_conf = + telemetry_conf.set_metrics(&cfg.metrics_http_listener_addr, env!("CARGO_PKG_NAME")); } - let _telemetry_guard = match telemetry_conf - .try_init() + let (metrics, _telemetry_guard) = match telemetry_conf + .try_init(&()) .wrap_err("failed to setup telemetry") { Err(e) => { eprintln!("initializing sequencer-relayer failed:\n{e:?}"); return ExitCode::FAILURE; } - Ok(guard) => guard, + Ok(metrics_and_guard) => metrics_and_guard, }; info!( @@ -56,7 +55,7 @@ async fn main() -> ExitCode { let mut sigterm = signal(SignalKind::terminate()) .expect("setting a SIGTERM listener should always work on Unix"); let (sequencer_relayer, shutdown_handle) = - SequencerRelayer::new(cfg).expect("could not initialize sequencer relayer"); + SequencerRelayer::new(cfg, metrics).expect("could not initialize sequencer relayer"); let sequencer_relayer_handle = tokio::spawn(sequencer_relayer.run()); let shutdown_token = shutdown_handle.token(); diff --git a/crates/astria-sequencer-relayer/src/metrics.rs b/crates/astria-sequencer-relayer/src/metrics.rs index 1ab32eb3ab..ebba14b585 100644 --- a/crates/astria-sequencer-relayer/src/metrics.rs +++ b/crates/astria-sequencer-relayer/src/metrics.rs @@ -1,20 +1,17 @@ use std::time::Duration; -use metrics::{ - counter, - describe_counter, - describe_gauge, - describe_histogram, - gauge, - histogram, - Counter, - Gauge, - Histogram, - Unit, +use telemetry::{ + metric_names, + metrics::{ + self, + Counter, + Gauge, + Histogram, + RegisteringBuilder, + }, }; -use telemetry::metric_names; -pub(crate) struct Metrics { +pub struct Metrics { celestia_submission_height: Counter, celestia_submission_count: Counter, celestia_submission_failure_count: Counter, @@ -30,111 +27,6 @@ pub(crate) struct Metrics { } impl Metrics { - #[must_use] - pub(crate) fn new() -> Self { - describe_counter!( - CELESTIA_SUBMISSION_HEIGHT, - Unit::Count, - "The height of the last blob successfully submitted to Celestia" - ); - let celestia_submission_height = counter!(CELESTIA_SUBMISSION_HEIGHT); - - describe_counter!( - CELESTIA_SUBMISSION_COUNT, - Unit::Count, - "The number of calls made to submit to Celestia" - ); - let celestia_submission_count = counter!(CELESTIA_SUBMISSION_COUNT); - - describe_counter!( - CELESTIA_SUBMISSION_FAILURE_COUNT, - Unit::Count, - "The number of calls made to submit to Celestia which have failed" - ); - let celestia_submission_failure_count = counter!(CELESTIA_SUBMISSION_FAILURE_COUNT); - - describe_histogram!( - BLOCKS_PER_CELESTIA_TX, - Unit::Count, - "The number of Astria blocks per Celestia submission" - ); - let blocks_per_celestia_tx = histogram!(BLOCKS_PER_CELESTIA_TX); - - describe_histogram!( - BLOBS_PER_CELESTIA_TX, - Unit::Count, - "The number of blobs (Astria Sequencer blocks converted to Celestia blobs) per \ - Celestia submission" - ); - let blobs_per_celestia_tx = histogram!(BLOBS_PER_CELESTIA_TX); - - describe_histogram!( - BYTES_PER_CELESTIA_TX, - Unit::Bytes, - "The total number of payload bytes (Astria Sequencer blocks converted to Celestia \ - blobs) per Celestia submission" - ); - let bytes_per_celestia_tx = histogram!(BYTES_PER_CELESTIA_TX); - - describe_histogram!( - CELESTIA_PAYLOAD_CREATION_LATENCY, - Unit::Seconds, - "The time it takes to create a new payload for submitting to Celestia (encoding to \ - protobuf, compression, creating blobs)" - ); - let celestia_payload_creation_latency = histogram!(CELESTIA_PAYLOAD_CREATION_LATENCY); - - describe_histogram!( - CELESTIA_SUBMISSION_LATENCY, - Unit::Seconds, - "The time it takes to submit a blob to Celestia" - ); - let celestia_submission_latency = histogram!(CELESTIA_SUBMISSION_LATENCY); - - describe_counter!( - SEQUENCER_BLOCK_FETCH_FAILURE_COUNT, - Unit::Count, - "The number of calls made to fetch a block from sequencer which have failed" - ); - let sequencer_block_fetch_failure_count = counter!(SEQUENCER_BLOCK_FETCH_FAILURE_COUNT); - - describe_counter!( - SEQUENCER_HEIGHT_FETCH_FAILURE_COUNT, - Unit::Count, - "The number of calls made to fetch the current height from sequencer which have failed" - ); - let sequencer_height_fetch_failure_count = counter!(SEQUENCER_HEIGHT_FETCH_FAILURE_COUNT); - - describe_counter!( - SEQUENCER_SUBMISSION_HEIGHT, - Unit::Count, - "The height of the highest sequencer block successfully submitted to Celestia" - ); - let sequencer_submission_height = counter!(SEQUENCER_SUBMISSION_HEIGHT); - - describe_gauge!( - COMPRESSION_RATIO_FOR_ASTRIA_BLOCK, - Unit::Count, - "Ratio of uncompressed:compressed data size for all `blob.data`s in an Astria block" - ); - let compression_ratio_for_astria_block = gauge!(COMPRESSION_RATIO_FOR_ASTRIA_BLOCK); - - Self { - celestia_submission_height, - celestia_submission_count, - celestia_submission_failure_count, - blocks_per_celestia_tx, - blobs_per_celestia_tx, - bytes_per_celestia_tx, - celestia_payload_creation_latency, - celestia_submission_latency, - sequencer_block_fetch_failure_count, - sequencer_height_fetch_failure_count, - sequencer_submission_height, - compression_ratio_for_astria_block, - } - } - pub(crate) fn absolute_set_celestia_submission_height(&self, height: u64) { self.celestia_submission_height.absolute(height); } @@ -190,7 +82,120 @@ impl Metrics { } } -metric_names!(pub const METRICS_NAMES: +impl metrics::Metrics for Metrics { + type Config = (); + + fn register( + builder: &mut RegisteringBuilder, + _config: &Self::Config, + ) -> Result { + let celestia_submission_height = builder + .new_counter_factory( + CELESTIA_SUBMISSION_HEIGHT, + "The height of the last blob successfully submitted to Celestia", + )? + .register()?; + + let celestia_submission_count = builder + .new_counter_factory( + CELESTIA_SUBMISSION_COUNT, + "The number of calls made to submit to Celestia", + )? + .register()?; + + let celestia_submission_failure_count = builder + .new_counter_factory( + CELESTIA_SUBMISSION_FAILURE_COUNT, + "The number of calls made to submit to Celestia which have failed", + )? + .register()?; + + let blocks_per_celestia_tx = builder + .new_histogram_factory( + BLOCKS_PER_CELESTIA_TX, + "The number of Astria blocks per Celestia submission", + )? + .register()?; + + let blobs_per_celestia_tx = builder + .new_histogram_factory( + BLOBS_PER_CELESTIA_TX, + "The number of blobs (Astria Sequencer blocks converted to Celestia blobs) per \ + Celestia submission", + )? + .register()?; + + let bytes_per_celestia_tx = builder + .new_histogram_factory( + BYTES_PER_CELESTIA_TX, + "The total number of payload bytes (Astria Sequencer blocks converted to Celestia \ + blobs) per Celestia submission", + )? + .register()?; + + let celestia_payload_creation_latency = builder + .new_histogram_factory( + CELESTIA_PAYLOAD_CREATION_LATENCY, + "The time it takes to create a new payload for submitting to Celestia (encoding \ + to protobuf, compression, creating blobs)", + )? + .register()?; + + let celestia_submission_latency = builder + .new_histogram_factory( + CELESTIA_SUBMISSION_LATENCY, + "The time it takes to submit a blob to Celestia", + )? + .register()?; + + let sequencer_block_fetch_failure_count = builder + .new_counter_factory( + SEQUENCER_BLOCK_FETCH_FAILURE_COUNT, + "The number of calls made to fetch a block from sequencer which have failed", + )? + .register()?; + + let sequencer_height_fetch_failure_count = builder + .new_counter_factory( + SEQUENCER_HEIGHT_FETCH_FAILURE_COUNT, + "The number of calls made to fetch the current height from sequencer which have \ + failed", + )? + .register()?; + + let sequencer_submission_height = builder + .new_counter_factory( + SEQUENCER_SUBMISSION_HEIGHT, + "The height of the highest sequencer block successfully submitted to Celestia", + )? + .register()?; + + let compression_ratio_for_astria_block = builder + .new_gauge_factory( + COMPRESSION_RATIO_FOR_ASTRIA_BLOCK, + "Ratio of uncompressed:compressed data size for all `blob.data`s in an Astria \ + block", + )? + .register()?; + + Ok(Self { + celestia_submission_height, + celestia_submission_count, + celestia_submission_failure_count, + blocks_per_celestia_tx, + blobs_per_celestia_tx, + bytes_per_celestia_tx, + celestia_payload_creation_latency, + celestia_submission_latency, + sequencer_block_fetch_failure_count, + sequencer_height_fetch_failure_count, + sequencer_submission_height, + compression_ratio_for_astria_block, + }) + } +} + +metric_names!(const METRICS_NAMES: CELESTIA_SUBMISSION_HEIGHT, CELESTIA_SUBMISSION_COUNT, CELESTIA_SUBMISSION_FAILURE_COUNT, diff --git a/crates/astria-sequencer-relayer/src/relayer/write/conversion.rs b/crates/astria-sequencer-relayer/src/relayer/write/conversion.rs index 9bead7e0c2..34bb99bfcb 100644 --- a/crates/astria-sequencer-relayer/src/relayer/write/conversion.rs +++ b/crates/astria-sequencer-relayer/src/relayer/write/conversion.rs @@ -487,6 +487,7 @@ mod tests { ChaChaRng, }; use sequencer_client::SequencerBlock; + use telemetry::metrics::Metrics as _; use super::{ Input, @@ -506,7 +507,7 @@ mod tests { } fn metrics() -> &'static Metrics { - Box::leak(Box::new(Metrics::new())) + Box::leak(Box::new(Metrics::noop_metrics(&()).unwrap())) } fn block(height: u32) -> SequencerBlock { diff --git a/crates/astria-sequencer-relayer/src/sequencer_relayer.rs b/crates/astria-sequencer-relayer/src/sequencer_relayer.rs index 5250297317..9ebd3db7b7 100644 --- a/crates/astria-sequencer-relayer/src/sequencer_relayer.rs +++ b/crates/astria-sequencer-relayer/src/sequencer_relayer.rs @@ -1,6 +1,5 @@ use std::{ net::SocketAddr, - sync::OnceLock, time::Duration, }; @@ -45,10 +44,7 @@ impl SequencerRelayer { /// # Errors /// /// Returns an error if constructing the inner relayer type failed. - pub fn new(cfg: Config) -> eyre::Result<(Self, ShutdownHandle)> { - static METRICS: OnceLock = OnceLock::new(); - let metrics = METRICS.get_or_init(Metrics::new); - + pub fn new(cfg: Config, metrics: &'static Metrics) -> eyre::Result<(Self, ShutdownHandle)> { let shutdown_handle = ShutdownHandle::new(); let rollup_filter = cfg.only_include_rollups()?; let Config { diff --git a/crates/astria-sequencer-relayer/tests/blackbox/helpers/test_sequencer_relayer.rs b/crates/astria-sequencer-relayer/tests/blackbox/helpers/test_sequencer_relayer.rs index 55fbd232c5..ab5b73f3ec 100644 --- a/crates/astria-sequencer-relayer/tests/blackbox/helpers/test_sequencer_relayer.rs +++ b/crates/astria-sequencer-relayer/tests/blackbox/helpers/test_sequencer_relayer.rs @@ -20,6 +20,7 @@ use astria_core::{ use astria_grpc_mock::MockGuard as GrpcMockGuard; use astria_sequencer_relayer::{ config::Config, + Metrics, SequencerRelayer, ShutdownHandle, }; @@ -32,6 +33,7 @@ use reqwest::{ }; use serde::Deserialize; use serde_json::json; +use telemetry::metrics; use tempfile::NamedTempFile; use tendermint_config::PrivValidatorKey; use tendermint_rpc::{ @@ -128,19 +130,19 @@ static TELEMETRY: Lazy<()> = Lazy::new(|| { if std::env::var_os("TEST_LOG").is_some() { let filter_directives = std::env::var("RUST_LOG").unwrap_or_else(|_| "info".into()); println!("initializing telemetry"); - telemetry::configure() - .no_otel() - .stdout_writer(std::io::stdout) - .force_stdout() - .pretty_print() - .filter_directives(&filter_directives) - .try_init() + let _ = telemetry::configure() + .set_no_otel(true) + .set_stdout_writer(std::io::stdout) + .set_force_stdout(true) + .set_pretty_print(true) + .set_filter_directives(&filter_directives) + .try_init::(&()) .unwrap(); } else { - telemetry::configure() - .no_otel() - .stdout_writer(std::io::sink) - .try_init() + let _ = telemetry::configure() + .set_no_otel(true) + .set_stdout_writer(std::io::sink) + .try_init::(&()) .unwrap(); } }); @@ -177,6 +179,7 @@ pub struct TestSequencerRelayer { /// The Celestia chain ID which will be returned by the mock `celestia_app` instance, and set /// via `TestSequencerRelayerConfig`. pub actual_celestia_chain_id: String, + pub metrics_handle: metrics::Handle, } impl Drop for TestSequencerRelayer { @@ -728,15 +731,21 @@ impl TestSequencerRelayerConfig { force_stdout: false, no_otel: false, no_metrics: false, - metrics_http_listener_addr: String::new(), + metrics_http_listener_addr: "127.0.0.1:9000".to_string(), pretty_print: true, pre_submit_path: pre_submit_file.path().to_owned(), post_submit_path: post_submit_file.path().to_owned(), }; + let (metrics, metrics_handle) = metrics::ConfigBuilder::new() + .with_global_recorder(false) + .build(&()) + .unwrap(); + let metrics = Box::leak(Box::new(metrics)); + info!(config = serde_json::to_string(&config).unwrap()); let (sequencer_relayer, relayer_shutdown_handle) = - SequencerRelayer::new(config.clone()).unwrap(); + SequencerRelayer::new(config.clone(), metrics).unwrap(); let api_address = sequencer_relayer.local_addr(); let sequencer_relayer = tokio::task::spawn(sequencer_relayer.run()); @@ -753,6 +762,7 @@ impl TestSequencerRelayerConfig { post_submit_file, actual_sequencer_chain_id: self.sequencer_chain_id, actual_celestia_chain_id: self.celestia_chain_id, + metrics_handle, }; test_sequencer_relayer diff --git a/crates/astria-sequencer-relayer/tests/blackbox/main.rs b/crates/astria-sequencer-relayer/tests/blackbox/main.rs index e97888d546..a74df154d3 100644 --- a/crates/astria-sequencer-relayer/tests/blackbox/main.rs +++ b/crates/astria-sequencer-relayer/tests/blackbox/main.rs @@ -239,6 +239,7 @@ async fn should_filter_rollup() { let excluded_rollup_ids: HashSet<_> = (0..5).map(|x| RollupId::new([100 + x; 32])).collect(); let sequencer_relayer = TestSequencerRelayerConfig { + last_written_sequencer_height: None, only_include_rollups: included_rollup_ids.clone(), ..TestSequencerRelayerConfig::default() } @@ -317,7 +318,8 @@ async fn should_shut_down() { ) .await; - // Send the shutdown signal - equivalent to sigkill being issued to sequencer-relayer process. + // Send the shutdown signal - equivalent to sigkill being issued to sequencer-relayer + // process. sequencer_relayer.relayer_shutdown_handle.take(); let get_tx_guard = sequencer_relayer diff --git a/crates/astria-sequencer/Cargo.toml b/crates/astria-sequencer/Cargo.toml index bd282d7fb0..9a30176965 100644 --- a/crates/astria-sequencer/Cargo.toml +++ b/crates/astria-sequencer/Cargo.toml @@ -39,7 +39,6 @@ futures = { workspace = true } hex = { workspace = true, features = ["serde"] } ibc-types = { workspace = true, features = ["with_serde"] } penumbra-ibc = { workspace = true, features = ["component", "rpc"] } -metrics = { workspace = true } penumbra-proto = { workspace = true } penumbra-tower-trace = { workspace = true } prost = { workspace = true } diff --git a/crates/astria-sequencer/src/app/test_utils.rs b/crates/astria-sequencer/src/app/test_utils.rs index 15072f61d5..c6a857d83c 100644 --- a/crates/astria-sequencer/src/app/test_utils.rs +++ b/crates/astria-sequencer/src/app/test_utils.rs @@ -18,6 +18,7 @@ use astria_core::{ }; use cnidarium::Storage; use penumbra_ibc::params::IBCParameters; +use telemetry::metrics::Metrics as _; use crate::{ app::App, @@ -104,7 +105,7 @@ pub(crate) async fn initialize_app_with_storage( .expect("failed to create temp storage backing chain state"); let snapshot = storage.latest_snapshot(); let mempool = Mempool::new(); - let metrics = Box::leak(Box::new(Metrics::new())); + let metrics = Box::leak(Box::new(Metrics::noop_metrics(&()).unwrap())); let mut app = App::new(snapshot, mempool, metrics).await.unwrap(); let genesis_state = genesis_state.unwrap_or_else(|| GenesisState { diff --git a/crates/astria-sequencer/src/lib.rs b/crates/astria-sequencer/src/lib.rs index 0425796406..d11f8bad86 100644 --- a/crates/astria-sequencer/src/lib.rs +++ b/crates/astria-sequencer/src/lib.rs @@ -28,6 +28,7 @@ use astria_core::primitive::v1::{ pub use build_info::BUILD_INFO; pub use config::Config; pub(crate) use config::ADDRESS_PREFIX; +pub use metrics::Metrics; pub use sequencer::Sequencer; pub use telemetry; diff --git a/crates/astria-sequencer/src/main.rs b/crates/astria-sequencer/src/main.rs index dc6dd1266f..0c45a9d53e 100644 --- a/crates/astria-sequencer/src/main.rs +++ b/crates/astria-sequencer/src/main.rs @@ -31,22 +31,21 @@ async fn main() -> ExitCode { .set_no_otel(cfg.no_otel) .set_force_stdout(cfg.force_stdout) .set_pretty_print(cfg.pretty_print) - .filter_directives(&cfg.log); + .set_filter_directives(&cfg.log); if !cfg.no_metrics { - telemetry_conf = telemetry_conf - .metrics_addr(&cfg.metrics_http_listener_addr) - .service_name(env!("CARGO_PKG_NAME")); + telemetry_conf = + telemetry_conf.set_metrics(&cfg.metrics_http_listener_addr, env!("CARGO_PKG_NAME")); } - let _telemetry_guard = match telemetry_conf - .try_init() + let (metrics, _telemetry_guard) = match telemetry_conf + .try_init(&()) .context("failed to setup telemetry") { Err(e) => { eprintln!("initializing sequencer failed:\n{e:?}"); return ExitCode::FAILURE; } - Ok(guard) => guard, + Ok(metrics_and_guard) => metrics_and_guard, }; info!( @@ -54,7 +53,7 @@ async fn main() -> ExitCode { "initializing sequencer" ); - Sequencer::run_until_stopped(cfg) + Sequencer::run_until_stopped(cfg, metrics) .await .expect("failed to run sequencer"); diff --git a/crates/astria-sequencer/src/metrics.rs b/crates/astria-sequencer/src/metrics.rs index b170b8dcd3..9b7e9b59c3 100644 --- a/crates/astria-sequencer/src/metrics.rs +++ b/crates/astria-sequencer/src/metrics.rs @@ -1,18 +1,15 @@ -use metrics::{ - counter, - describe_counter, - describe_gauge, - describe_histogram, - gauge, - histogram, - Counter, - Gauge, - Histogram, - Unit, +use telemetry::{ + metric_names, + metrics::{ + self, + Counter, + Gauge, + Histogram, + RegisteringBuilder, + }, }; -use telemetry::metric_names; -pub(crate) struct Metrics { +pub struct Metrics { prepare_proposal_excluded_transactions_decode_failure: Counter, prepare_proposal_excluded_transactions_cometbft_space: Counter, prepare_proposal_excluded_transactions_sequencer_space: Counter, @@ -28,120 +25,6 @@ pub(crate) struct Metrics { } impl Metrics { - #[must_use] - pub(crate) fn new() -> Self { - describe_counter!( - PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_DECODE_FAILURE, - Unit::Count, - "The number of transactions that have been excluded from blocks due to failing to \ - decode" - ); - let prepare_proposal_excluded_transactions_decode_failure = - counter!(PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_DECODE_FAILURE); - - describe_counter!( - PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_COMETBFT_SPACE, - Unit::Count, - "The number of transactions that have been excluded from blocks due to running out of \ - space in the cometbft block" - ); - let prepare_proposal_excluded_transactions_cometbft_space = - counter!(PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_COMETBFT_SPACE); - - describe_counter!( - PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_SEQUENCER_SPACE, - Unit::Count, - "The number of transactions that have been excluded from blocks due to running out of \ - space in the sequencer block" - ); - let prepare_proposal_excluded_transactions_sequencer_space = - counter!(PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_SEQUENCER_SPACE); - - describe_counter!( - PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_FAILED_EXECUTION, - Unit::Count, - "The number of transactions that have been excluded from blocks due to failing to \ - execute" - ); - let prepare_proposal_excluded_transactions_failed_execution = - counter!(PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_FAILED_EXECUTION); - - describe_gauge!( - PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS, - Unit::Count, - "The number of excluded transactions in a proposal being prepared" - ); - let prepare_proposal_excluded_transactions = gauge!(PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS); - - describe_histogram!( - PROPOSAL_DEPOSITS, - Unit::Count, - "The number of deposits in a proposal" - ); - let proposal_deposits = histogram!(PROPOSAL_DEPOSITS); - - describe_histogram!( - PROPOSAL_TRANSACTIONS, - Unit::Count, - "The number of transactions in a proposal" - ); - let proposal_transactions = histogram!(PROPOSAL_TRANSACTIONS); - - describe_counter!( - PROCESS_PROPOSAL_SKIPPED_PROPOSAL, - Unit::Count, - "The number of times our submitted prepared proposal was skipped in process proposal" - ); - let process_proposal_skipped_proposal = counter!(PROCESS_PROPOSAL_SKIPPED_PROPOSAL); - - describe_counter!( - CHECK_TX_REMOVED_TOO_LARGE, - Unit::Count, - "The number of transactions that have been removed from the mempool due to being too \ - large" - ); - let check_tx_removed_too_large = counter!(CHECK_TX_REMOVED_TOO_LARGE); - - describe_counter!( - CHECK_TX_REMOVED_FAILED_STATELESS, - Unit::Count, - "The number of transactions that have been removed from the mempool due to failing \ - the stateless check" - ); - let check_tx_removed_failed_stateless = counter!(CHECK_TX_REMOVED_FAILED_STATELESS); - - describe_counter!( - CHECK_TX_REMOVED_STALE_NONCE, - Unit::Count, - "The number of transactions that have been removed from the mempool due to having a \ - stale nonce" - ); - let check_tx_removed_stale_nonce = counter!(CHECK_TX_REMOVED_STALE_NONCE); - - describe_counter!( - CHECK_TX_REMOVED_ACCOUNT_BALANCE, - Unit::Count, - "The number of transactions that have been removed from the mempool due to having not \ - enough account balance" - ); - let check_tx_removed_account_balance = counter!(CHECK_TX_REMOVED_ACCOUNT_BALANCE); - - Self { - prepare_proposal_excluded_transactions_decode_failure, - prepare_proposal_excluded_transactions_cometbft_space, - prepare_proposal_excluded_transactions_sequencer_space, - prepare_proposal_excluded_transactions_failed_execution, - prepare_proposal_excluded_transactions, - proposal_deposits, - proposal_transactions, - process_proposal_skipped_proposal, - check_tx_removed_too_large, - check_tx_removed_failed_stateless, - check_tx_removed_stale_nonce, - check_tx_removed_account_balance, - } - } - pub(crate) fn increment_prepare_proposal_excluded_transactions_decode_failure(&self) { self.prepare_proposal_excluded_transactions_decode_failure .increment(1); @@ -199,7 +82,121 @@ impl Metrics { } } -metric_names!(pub const METRICS_NAMES: +impl metrics::Metrics for Metrics { + type Config = (); + + fn register( + builder: &mut RegisteringBuilder, + _config: &Self::Config, + ) -> Result { + let prepare_proposal_excluded_transactions_decode_failure = builder + .new_counter_factory( + PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_DECODE_FAILURE, + "The number of transactions that have been excluded from blocks due to failing to \ + decode", + )? + .register()?; + + let prepare_proposal_excluded_transactions_cometbft_space = builder + .new_counter_factory( + PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_COMETBFT_SPACE, + "The number of transactions that have been excluded from blocks due to running \ + out of space in the cometbft block", + )? + .register()?; + + let prepare_proposal_excluded_transactions_sequencer_space = builder + .new_counter_factory( + PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_SEQUENCER_SPACE, + "The number of transactions that have been excluded from blocks due to running \ + out of space in the sequencer block", + )? + .register()?; + + let prepare_proposal_excluded_transactions_failed_execution = builder + .new_counter_factory( + PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_FAILED_EXECUTION, + "The number of transactions that have been excluded from blocks due to failing to \ + execute", + )? + .register()?; + + let prepare_proposal_excluded_transactions = builder + .new_gauge_factory( + PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS, + "The number of excluded transactions in a proposal being prepared", + )? + .register()?; + + let proposal_deposits = builder + .new_histogram_factory(PROPOSAL_DEPOSITS, "The number of deposits in a proposal")? + .register()?; + + let proposal_transactions = builder + .new_histogram_factory( + PROPOSAL_TRANSACTIONS, + "The number of transactions in a proposal", + )? + .register()?; + + let process_proposal_skipped_proposal = builder + .new_counter_factory( + PROCESS_PROPOSAL_SKIPPED_PROPOSAL, + "The number of times our submitted prepared proposal was skipped in process \ + proposal", + )? + .register()?; + + let check_tx_removed_too_large = builder + .new_counter_factory( + CHECK_TX_REMOVED_TOO_LARGE, + "The number of transactions that have been removed from the mempool due to being \ + too large", + )? + .register()?; + + let check_tx_removed_failed_stateless = builder + .new_counter_factory( + CHECK_TX_REMOVED_FAILED_STATELESS, + "The number of transactions that have been removed from the mempool due to \ + failing the stateless check", + )? + .register()?; + + let check_tx_removed_stale_nonce = builder + .new_counter_factory( + CHECK_TX_REMOVED_STALE_NONCE, + "The number of transactions that have been removed from the mempool due to having \ + a stale nonce", + )? + .register()?; + + let check_tx_removed_account_balance = builder + .new_counter_factory( + CHECK_TX_REMOVED_ACCOUNT_BALANCE, + "The number of transactions that have been removed from the mempool due to having \ + not enough account balance", + )? + .register()?; + + Ok(Self { + prepare_proposal_excluded_transactions_decode_failure, + prepare_proposal_excluded_transactions_cometbft_space, + prepare_proposal_excluded_transactions_sequencer_space, + prepare_proposal_excluded_transactions_failed_execution, + prepare_proposal_excluded_transactions, + proposal_deposits, + proposal_transactions, + process_proposal_skipped_proposal, + check_tx_removed_too_large, + check_tx_removed_failed_stateless, + check_tx_removed_stale_nonce, + check_tx_removed_account_balance, + }) + } +} + +metric_names!(const METRICS_NAMES: PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_DECODE_FAILURE, PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_COMETBFT_SPACE, PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_SEQUENCER_SPACE, diff --git a/crates/astria-sequencer/src/sequencer.rs b/crates/astria-sequencer/src/sequencer.rs index 9ca0ffd5a6..15173e498b 100644 --- a/crates/astria-sequencer/src/sequencer.rs +++ b/crates/astria-sequencer/src/sequencer.rs @@ -1,5 +1,3 @@ -use std::sync::OnceLock; - use anyhow::{ anyhow, Context as _, @@ -45,10 +43,7 @@ pub struct Sequencer; impl Sequencer { #[instrument(skip_all)] - pub async fn run_until_stopped(config: Config) -> Result<()> { - static METRICS: OnceLock = OnceLock::new(); - let metrics = METRICS.get_or_init(Metrics::new); - + pub async fn run_until_stopped(config: Config, metrics: &'static Metrics) -> Result<()> { if config .db_filepath .try_exists() diff --git a/crates/astria-sequencer/src/service/consensus.rs b/crates/astria-sequencer/src/service/consensus.rs index d099c04176..91c96c658c 100644 --- a/crates/astria-sequencer/src/service/consensus.rs +++ b/crates/astria-sequencer/src/service/consensus.rs @@ -239,6 +239,7 @@ mod test { use bytes::Bytes; use prost::Message as _; use rand::rngs::OsRng; + use telemetry::metrics::Metrics as _; use tendermint::{ account::Id, Hash, @@ -493,7 +494,7 @@ mod test { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mempool = Mempool::new(); - let metrics = Box::leak(Box::new(Metrics::new())); + let metrics = Box::leak(Box::new(Metrics::noop_metrics(&()).unwrap())); let mut app = App::new(snapshot, mempool.clone(), metrics).await.unwrap(); app.init_chain(storage.clone(), genesis_state, vec![], "test".to_string()) .await diff --git a/crates/astria-telemetry/Cargo.toml b/crates/astria-telemetry/Cargo.toml index aaec8de6f8..0b7e04e8dd 100644 --- a/crates/astria-telemetry/Cargo.toml +++ b/crates/astria-telemetry/Cargo.toml @@ -14,8 +14,10 @@ homepage = "https://astria.org" base64 = { workspace = true, optional = true } base64-serde = { workspace = true, optional = true } const_format = { workspace = true } +itertools = { workspace = true } -metrics-exporter-prometheus = { version = "0.13.1", default-features = false, features = [ +metrics = "0.23.0" +metrics-exporter-prometheus = { version = "0.15.0", default-features = false, features = [ "http-listener", ] } # When updating ensure that `opentelemetry-semantic-conventions` matches @@ -29,6 +31,7 @@ serde = { workspace = true, optional = true } serde_json = { workspace = true, optional = true } serde_with = { version = "3.7.0", optional = true } thiserror = { workspace = true } +tokio = { workspace = true } tracing-opentelemetry = "0.23.0" tracing-subscriber = { version = "0.3.17", features = [ "fmt", diff --git a/crates/astria-telemetry/src/lib.rs b/crates/astria-telemetry/src/lib.rs index d63680016d..6c899eb981 100644 --- a/crates/astria-telemetry/src/lib.rs +++ b/crates/astria-telemetry/src/lib.rs @@ -2,24 +2,24 @@ //! //! # Examples //! ```no_run +//! # struct Metrics; +//! # impl astria_telemetry::metrics::Metrics for Metrics { +//! # type Config = (); +//! # fn register( +//! # _: &mut astria_telemetry::metrics::RegisteringBuilder, +//! # _: &Self::Config +//! # ) -> Result { Ok(Self) } +//! # } +//! let metrics_config = (); //! astria_telemetry::configure() -//! .filter_directives("info") -//! .try_init() +//! .set_filter_directives("info") +//! .try_init::(&metrics_config) //! .expect("must be able to initialize telemetry"); //! tracing::info!("telemetry initialized"); //! ``` -use std::{ - io::IsTerminal as _, - net::{ - AddrParseError, - SocketAddr, - }, -}; +use std::io::IsTerminal as _; -use metrics_exporter_prometheus::{ - BuildError, - PrometheusBuilder, -}; +use metrics::Metrics; use opentelemetry::{ global, trace::TracerProvider as _, @@ -44,9 +44,9 @@ use tracing_subscriber::{ #[cfg(feature = "display")] pub mod display; - #[doc(hidden)] pub mod macros; +pub mod metrics; /// The errors that can occur when initializing telemetry. #[derive(Debug, thiserror::Error)] @@ -65,13 +65,11 @@ impl Error { fn init_subscriber(source: TryInitError) -> Self { Self(ErrorKind::InitSubscriber(source)) } +} - fn metrics_addr(source: AddrParseError) -> Self { - Self(ErrorKind::MetricsAddr(source)) - } - - fn exporter_install(source: BuildError) -> Self { - Self(ErrorKind::ExporterInstall(source)) +impl From for Error { + fn from(source: metrics::Error) -> Self { + Self(ErrorKind::Metrics(source)) } } @@ -83,16 +81,15 @@ enum ErrorKind { FilterDirectives(#[source] ParseError), #[error("failed installing global tracing subscriber")] InitSubscriber(#[source] TryInitError), - #[error("failed to parse metrics address")] - MetricsAddr(#[source] AddrParseError), - #[error("failed installing prometheus metrics exporter")] - ExporterInstall(#[source] BuildError), + #[error(transparent)] + Metrics(#[from] metrics::Error), } #[must_use = "the otel config must be initialized to be useful"] pub fn configure() -> Config { Config::new() } + struct BoxedMakeWriter(Box); impl BoxedMakeWriter { @@ -130,8 +127,7 @@ pub struct Config { no_otel: bool, pretty_print: bool, stdout_writer: BoxedMakeWriter, - metrics_addr: Option, - service_name: String, + metrics_config_builder: Option, } impl Config { @@ -143,85 +139,52 @@ impl Config { no_otel: false, pretty_print: false, stdout_writer: BoxedMakeWriter::new(std::io::stdout), - metrics_addr: None, - service_name: String::new(), + metrics_config_builder: None, } } } impl Config { #[must_use = "telemetry must be initialized to be useful"] - pub fn filter_directives(self, filter_directives: &str) -> Self { - Self { - filter_directives: filter_directives.to_string(), - ..self - } - } - - #[must_use = "telemetry must be initialized to be useful"] - pub fn force_stdout(self) -> Self { - self.set_force_stdout(true) - } - - #[must_use = "telemetry must be initialized to be useful"] - pub fn set_force_stdout(self, force_stdout: bool) -> Self { - Self { - force_stdout, - ..self - } - } - - #[must_use = "telemetry must be initialized to be useful"] - pub fn no_otel(self) -> Self { - self.set_no_otel(true) + pub fn set_filter_directives(mut self, filter_directives: &str) -> Self { + self.filter_directives = filter_directives.to_string(); + self } #[must_use = "telemetry must be initialized to be useful"] - pub fn set_no_otel(self, no_otel: bool) -> Self { - Self { - no_otel, - ..self - } + pub fn set_force_stdout(mut self, force_stdout: bool) -> Self { + self.force_stdout = force_stdout; + self } #[must_use = "telemetry must be initialized to be useful"] - pub fn pretty_print(self) -> Self { - self.set_pretty_print(true) + pub fn set_no_otel(mut self, no_otel: bool) -> Self { + self.no_otel = no_otel; + self } #[must_use = "telemetry must be initialized to be useful"] - pub fn set_pretty_print(self, pretty_print: bool) -> Self { - Self { - pretty_print, - ..self - } + pub fn set_pretty_print(mut self, pretty_print: bool) -> Self { + self.pretty_print = pretty_print; + self } #[must_use = "telemetry must be initialized to be useful"] - pub fn stdout_writer(self, stdout_writer: M) -> Self + pub fn set_stdout_writer(mut self, stdout_writer: M) -> Self where M: MakeWriter + Send + Sync + 'static, { - Self { - stdout_writer: BoxedMakeWriter::new(stdout_writer), - ..self - } - } - - #[must_use = "telemetry must be initialized to be useful"] - pub fn metrics_addr(self, metrics_addr: &str) -> Self { - Self { - metrics_addr: Some(metrics_addr.to_string()), - ..self - } + self.stdout_writer = BoxedMakeWriter::new(stdout_writer); + self } #[must_use = "telemetry must be initialized to be useful"] - pub fn service_name(self, service_name: &str) -> Self { - Self { - service_name: service_name.to_string(), - ..self - } + pub fn set_metrics(mut self, listening_addr: &str, service_name: &str) -> Self { + let config_builder = metrics::ConfigBuilder::new() + .with_service_name(service_name) + .with_listening_address(listening_addr); + self.metrics_config_builder = Some(config_builder); + self } /// Initialize telemetry, consuming the config. @@ -229,15 +192,14 @@ impl Config { /// # Errors /// Fails if the filter directives could not be parsed, if communication with the OTLP /// endpoint failed, or if the global tracing subscriber could not be installed. - pub fn try_init(self) -> Result { + pub fn try_init(self, config: &T::Config) -> Result<(&'static T, Guard), Error> { let Self { filter_directives, force_stdout, no_otel, pretty_print, stdout_writer, - metrics_addr, - service_name, + metrics_config_builder, } = self; let env_filter = { @@ -294,20 +256,16 @@ impl Config { .try_init() .map_err(Error::init_subscriber)?; - if let Some(metrics_addr) = metrics_addr { - let addr: SocketAddr = metrics_addr.parse().map_err(Error::metrics_addr)?; - let mut metrics_builder = PrometheusBuilder::new().with_http_listener(addr); - - if !service_name.is_empty() { - metrics_builder = metrics_builder.add_global_label("service", service_name); - } - - metrics_builder.install().map_err(Error::exporter_install)?; - } + let metrics = match metrics_config_builder { + Some(config_builder) => config_builder.build(config)?.0, + None => T::noop_metrics(config)?, + }; - Ok(Guard { + let guard = Guard { run_otel_shutdown: !no_otel, - }) + }; + + Ok((Box::leak(Box::new(metrics)), guard)) } } diff --git a/crates/astria-telemetry/src/metrics/builders.rs b/crates/astria-telemetry/src/metrics/builders.rs new file mode 100644 index 0000000000..e8059536a6 --- /dev/null +++ b/crates/astria-telemetry/src/metrics/builders.rs @@ -0,0 +1,277 @@ +use std::{ + collections::{ + hash_map::Entry, + HashMap, + HashSet, + }, + mem, + net::SocketAddr, +}; + +use metrics::Recorder as _; +use metrics_exporter_prometheus::{ + ExporterFuture, + Matcher, + PrometheusBuilder, + PrometheusRecorder, +}; + +#[cfg(docs)] +use super::{ + Counter, + Gauge, + Histogram, +}; +use super::{ + CounterFactory, + Error, + GaugeFactory, + Handle, + HistogramFactory, + Metrics, +}; + +/// A builder used to gather metrics settings, register metrics, start the exporter server and +/// register the global metrics recorder. +pub struct ConfigBuilder { + service_name: String, + listening_address: Option, + use_global_recorder: bool, +} + +impl ConfigBuilder { + /// Returns a new `ConfigBuilder`. + /// + /// If [`Self::with_listener_address`] is not called, no http server will be started, meaning + /// the metrics' values can only be rendered via the [`Handle`] returned by [`Self::build`]. + /// + /// By default, a global metrics recorder will be set when calling `Self::build`. This can be + /// disabled by calling [`Self::with_global_recorder(false)`]. + #[must_use] + pub fn new() -> Self { + Self { + service_name: String::new(), + listening_address: None, + use_global_recorder: true, + } + } + + /// All metrics will have a label applied of `service=""`. + /// + /// If `service_name` is empty, the label is not applied. + #[must_use] + pub fn with_service_name(mut self, service_name: &str) -> Self { + self.service_name = service_name.to_string(); + self + } + + /// Sets the listening address of the exporter server. + #[must_use] + pub fn with_listening_address(mut self, listening_address: &str) -> Self { + self.listening_address = Some(listening_address.to_string()); + self + } + + /// Enables or disables setting the global metrics recorder. + #[must_use] + pub fn with_global_recorder(mut self, use_global_recorder: bool) -> Self { + self.use_global_recorder = use_global_recorder; + self + } + + /// Registers the buckets and metrics as specified in `T::set_buckets` and `T::register` + /// respectively, starts the http server if enabled, sets the global metrics recorder if + /// requested and returns a new metrics object of type `T` along with a handle for rendering + /// current metrics. + // allow: no useful error info can be added without writing excessive details. + #[allow(clippy::missing_errors_doc)] + pub fn build(self, config: &T::Config) -> Result<(T, Handle), Error> { + // Apply settings to the prometheus builder. + let mut prometheus_builder = PrometheusBuilder::new(); + if !self.service_name.is_empty() { + prometheus_builder = prometheus_builder.add_global_label("service", self.service_name); + } + if let Some(listening_address) = &self.listening_address { + let addr: SocketAddr = listening_address.parse()?; + prometheus_builder = prometheus_builder.with_http_listener(addr); + } + + // Set the histogram buckets. + let mut bucket_builder = BucketBuilder { + builder: prometheus_builder, + buckets: HashMap::new(), + }; + T::set_buckets(&mut bucket_builder, config)?; + let histograms_with_buckets = bucket_builder.histogram_names(); + + // Consume the prometheus builder, yielding a recorder and a future for running the exporter + // server (this will be a no-op if the server isn't configured to run). + let (recorder, exporter_fut) = if self.listening_address.is_some() { + bucket_builder + .builder + .build() + .map_err(|error| Error::StartListening(error.into()))? + } else { + let recorder = bucket_builder.builder.build_recorder(); + let fut: ExporterFuture = Box::pin(async move { Ok(()) }); + (recorder, fut) + }; + let handle = Handle::new(recorder.handle()); + + // Register individual metrics. + let mut registering_builder = RegisteringBuilder::new(recorder); + let metrics = T::register(&mut registering_builder, config)?; + + // Ensure no histogram buckets were left unassigned. + let unassigned: HashSet<_> = histograms_with_buckets + .difference(®istering_builder.histograms) + .cloned() + .collect(); + if !unassigned.is_empty() { + return Err(Error::BucketsNotAssigned(unassigned)); + } + + // Run the exporter server and set the global recorder if requested. + tokio::spawn(exporter_fut); + if self.use_global_recorder { + metrics::set_global_recorder(registering_builder.recorder) + .map_err(|_| Error::GlobalMetricsRecorderAlreadySet)?; + } + Ok((metrics, handle)) + } +} + +impl Default for ConfigBuilder { + fn default() -> Self { + Self::new() + } +} + +/// A builder used to set histogram buckets. +/// +/// It is constructed in [`ConfigBuilder::build`] and passed to [`Metrics::set_buckets`]. +pub struct BucketBuilder { + builder: PrometheusBuilder, + buckets: HashMap<&'static str, Vec>, +} + +impl BucketBuilder { + /// Sets the buckets for the given histogram. + /// + /// # Errors + /// + /// Returns an error if `values` is empty, or if `histogram_name` has already had buckets set. + /// + /// If the given histogram is not registered later via the `RegisteringBuilder`, then + /// `RegisteringBuilder::build` will return an error. + pub fn set_buckets( + &mut self, + histogram_name: &'static str, + values: &[f64], + ) -> Result<(), Error> { + match self.buckets.entry(histogram_name) { + Entry::Occupied(_) => return Err(Error::BucketsAlreadySet(histogram_name)), + Entry::Vacant(entry) => { + let _ = entry.insert(values.to_vec()); + } + } + // Swap out the builder temporarily to call `set_buckets_for_metric` which consumes the + // builder, then swap it back into `self.builder`. + let mut builder = mem::take(&mut self.builder) + .set_buckets_for_metric(Matcher::Full(histogram_name.to_string()), values) + .map_err(|_| Error::EmptyBuckets(histogram_name))?; + mem::swap(&mut builder, &mut self.builder); + Ok(()) + } + + fn histogram_names(&self) -> HashSet { + self.buckets.keys().map(ToString::to_string).collect() + } +} + +/// A builder used to register individual metrics. +/// +/// It is constructed in [`ConfigBuilder::build`] and passed to [`Metrics::register`]. +pub struct RegisteringBuilder { + recorder: PrometheusRecorder, + counters: HashSet, + gauges: HashSet, + histograms: HashSet, +} + +impl RegisteringBuilder { + /// Returns a new `CounterFactory` for registering [`Counter`]s under the given name. + /// + /// # Errors + /// + /// Returns an error if a counter has already been registered under this name. + pub fn new_counter_factory( + &mut self, + name: &'static str, + description: &'static str, + ) -> Result { + if !self.counters.insert(name.to_string()) { + return Err(Error::MetricAlreadyRegistered { + metric_type: CounterFactory::metric_type(), + metric_name: name, + }); + } + + self.recorder + .describe_counter(name.into(), None, description.into()); + Ok(CounterFactory::new(name, &self.recorder)) + } + + /// Returns a new `GaugeFactory` for registering [`Gauge`]s under the given name. + /// + /// # Errors + /// + /// Returns an error if a gauge has already been registered under this name. + pub fn new_gauge_factory( + &mut self, + name: &'static str, + description: &'static str, + ) -> Result { + if !self.gauges.insert(name.to_string()) { + return Err(Error::MetricAlreadyRegistered { + metric_type: GaugeFactory::metric_type(), + metric_name: name, + }); + } + + self.recorder + .describe_gauge(name.into(), None, description.into()); + Ok(GaugeFactory::new(name, &self.recorder)) + } + + /// Returns a new `HistogramFactory` for registering [`Histogram`]s under the given name. + /// + /// # Errors + /// + /// Returns an error if a histogram has already been registered under this name. + pub fn new_histogram_factory( + &mut self, + name: &'static str, + description: &'static str, + ) -> Result { + if !self.histograms.insert(name.to_string()) { + return Err(Error::MetricAlreadyRegistered { + metric_type: HistogramFactory::metric_type(), + metric_name: name, + }); + } + + self.recorder + .describe_histogram(name.into(), None, description.into()); + Ok(HistogramFactory::new(name, &self.recorder)) + } + + pub(super) fn new(recorder: PrometheusRecorder) -> Self { + RegisteringBuilder { + recorder, + counters: HashSet::new(), + gauges: HashSet::new(), + histograms: HashSet::new(), + } + } +} diff --git a/crates/astria-telemetry/src/metrics/counter.rs b/crates/astria-telemetry/src/metrics/counter.rs new file mode 100644 index 0000000000..da27c022f5 --- /dev/null +++ b/crates/astria-telemetry/src/metrics/counter.rs @@ -0,0 +1,25 @@ +/// A counter. +#[derive(Clone)] +pub struct Counter(metrics::Counter); + +impl Counter { + /// Increments the counter. + pub fn increment(&self, value: u64) { + self.0.increment(value); + } + + /// Sets the counter to an absolute value. + pub fn absolute(&self, value: u64) { + self.0.absolute(value); + } + + /// Creates a no-op counter that does nothing. + #[must_use] + pub fn noop() -> Self { + Self(metrics::Counter::noop()) + } + + pub(super) fn new(counter: metrics::Counter) -> Self { + Self(counter) + } +} diff --git a/crates/astria-telemetry/src/metrics/error.rs b/crates/astria-telemetry/src/metrics/error.rs new file mode 100644 index 0000000000..1e12ba2bd7 --- /dev/null +++ b/crates/astria-telemetry/src/metrics/error.rs @@ -0,0 +1,79 @@ +use std::{ + collections::HashSet, + net::AddrParseError, +}; + +use itertools::Itertools; +use thiserror::Error; + +#[cfg(doc)] +use super::Metrics; + +/// An error related to registering or initializing metrics. +#[derive(Error, Debug)] +#[non_exhaustive] +pub enum Error { + /// The metric has already been registered. + #[error("{metric_type} `{metric_name}` has already been registered")] + MetricAlreadyRegistered { + metric_type: &'static str, + metric_name: &'static str, + }, + + /// The metric with the given labels has already been registered. + #[error("{metric_type} `{metric_name}` has already been registered with the given labels")] + MetricWithLabelsAlreadyRegistered { + metric_type: &'static str, + metric_name: &'static str, + }, + + /// The metric has a duplicate label. + #[error( + "{metric_type} `{metric_name}` has a duplicate of label `{label_name}=\"{label_value}\"`" + )] + DuplicateLabel { + metric_type: &'static str, + metric_name: &'static str, + label_name: String, + label_value: String, + }, + + /// Failed to set the given histogram's buckets. + #[error("the buckets for histogram `{0}` have already been set")] + BucketsAlreadySet(&'static str), + + /// The given histogram's buckets are empty. + #[error("the buckets for histogram `{0}` must have at least one value")] + EmptyBuckets(&'static str), + + /// The given histograms were assigned buckets, but never registered. + #[error( + "histogram(s) [{}] had buckets assigned via `Metrics::set_buckets` but were never \ + registered via `Metrics::register`", + .0.iter().join(", ") + )] + BucketsNotAssigned(HashSet), + + /// Failed to parse the metrics exporter listening address. + #[error("failed to parse metrics exporter listening address")] + ParseListeningAddress(#[from] AddrParseError), + + /// Failed to start the metrics exporter server. + #[error("failed to start the metrics exporter server")] + StartListening(#[source] StartListeningError), + + /// Failed to set the global metrics recorder. + #[error("the global metrics recorder has already been set")] + GlobalMetricsRecorderAlreadySet, + + /// External error, intended for use in implementations of [`Metrics`]. + #[error(transparent)] + External(Box), +} + +/// An error while starting the metrics exporter server. +#[derive(Error, Debug)] +#[error(transparent)] +// allow: the name correctly reflects the type. +#[allow(clippy::module_name_repetitions)] +pub struct StartListeningError(#[from] metrics_exporter_prometheus::BuildError); diff --git a/crates/astria-telemetry/src/metrics/factories.rs b/crates/astria-telemetry/src/metrics/factories.rs new file mode 100644 index 0000000000..a3abc3b85c --- /dev/null +++ b/crates/astria-telemetry/src/metrics/factories.rs @@ -0,0 +1,210 @@ +use std::{ + collections::BTreeSet, + marker::PhantomData, +}; + +use metrics::{ + Key, + Label, + Metadata, + Recorder as _, +}; +use metrics_exporter_prometheus::PrometheusRecorder; + +use super::{ + Counter, + Error, + Gauge, + Histogram, +}; + +pub struct CounterFactory<'a>(Factory<'a, Counter>); + +impl<'a> CounterFactory<'a> { + /// Registers and returns a counter with no labels applied. + /// + /// # Errors + /// + /// Returns an error if this metric has already been registered with no labels. + pub fn register(&mut self) -> Result { + self.0.register() + } + + /// Registers and returns a counter with the given labels applied. + /// + /// # Errors + /// + /// Returns an error if this metric has already been registered with the same labels (regardless + /// of order of the labels) or if any of the label pairs are duplicates. + pub fn register_with_labels( + &mut self, + labels: &[(&'static str, String)], + ) -> Result { + self.0.register_with_labels(labels) + } + + pub(super) fn new(name: &'static str, recorder: &'a PrometheusRecorder) -> Self { + Self(Factory::new(name, recorder)) + } + + pub(super) fn metric_type() -> &'static str { + Factory::<'a, Counter>::metric_type() + } +} + +pub struct GaugeFactory<'a>(Factory<'a, Gauge>); + +impl<'a> GaugeFactory<'a> { + /// Registers and returns a gauge with no labels applied. + /// + /// # Errors + /// + /// Returns an error if this metric has already been registered with no labels. + pub fn register(&mut self) -> Result { + self.0.register() + } + + /// Registers and returns a gauge with the given labels applied. + /// + /// # Errors + /// + /// Returns an error if this metric has already been registered with the same labels (regardless + /// of order of the labels) or if any of the label pairs are duplicates. + pub fn register_with_labels( + &mut self, + labels: &[(&'static str, String)], + ) -> Result { + self.0.register_with_labels(labels) + } + + pub(super) fn new(name: &'static str, recorder: &'a PrometheusRecorder) -> Self { + Self(Factory::new(name, recorder)) + } + + pub(super) fn metric_type() -> &'static str { + Factory::<'a, Gauge>::metric_type() + } +} + +pub struct HistogramFactory<'a>(Factory<'a, Histogram>); + +impl<'a> HistogramFactory<'a> { + /// Registers and returns a histogram with no labels applied. + /// + /// # Errors + /// + /// Returns an error if this metric has already been registered with no labels. + pub fn register(&mut self) -> Result { + self.0.register() + } + + /// Registers and returns a histogram with the given labels applied. + /// + /// # Errors + /// + /// Returns an error if this metric has already been registered with the same labels (regardless + /// of order of the labels) or if any of the label pairs are duplicates. + pub fn register_with_labels( + &mut self, + labels: &[(&'static str, String)], + ) -> Result { + self.0.register_with_labels(labels) + } + + pub(super) fn new(name: &'static str, recorder: &'a PrometheusRecorder) -> Self { + Self(Factory::new(name, recorder)) + } + + pub(super) fn metric_type() -> &'static str { + Factory::<'a, Histogram>::metric_type() + } +} + +struct Factory<'a, T> { + name: &'static str, + recorder: &'a PrometheusRecorder, + labels: BTreeSet>, + _phantom: PhantomData, +} + +impl<'a, T> Factory<'a, T> +where + Factory<'a, T>: RegisterMetric, +{ + fn register(&mut self) -> Result { + self.register_with_labels(&[]) + } + + fn register_with_labels(&mut self, labels: &[(&'static str, String)]) -> Result { + let key = Key::from_parts(self.name, labels); + + let mut unique_labels = BTreeSet::new(); + for label in key.labels() { + if !unique_labels.insert(label.clone()) { + return Err(Error::DuplicateLabel { + metric_type: Self::metric_type(), + metric_name: self.name, + label_name: label.key().to_string(), + label_value: label.value().to_string(), + }); + } + } + + if !self.labels.insert(unique_labels) { + return Err(Error::MetricAlreadyRegistered { + metric_type: Self::metric_type(), + metric_name: self.name, + }); + } + + Ok(self.register_metric(&key)) + } + + fn new(name: &'static str, recorder: &'a PrometheusRecorder) -> Self { + Self { + name, + recorder, + labels: BTreeSet::new(), + _phantom: PhantomData, + } + } +} + +trait RegisterMetric { + fn register_metric(&self, key: &Key) -> T; + + fn metric_type() -> &'static str; +} + +impl<'a> RegisterMetric for Factory<'a, Counter> { + fn register_metric(&self, key: &Key) -> Counter { + let ignored_metadata = Metadata::new("", metrics::Level::ERROR, None); + Counter::new(self.recorder.register_counter(key, &ignored_metadata)) + } + + fn metric_type() -> &'static str { + "counter" + } +} + +impl<'a> RegisterMetric for Factory<'a, Gauge> { + fn register_metric(&self, key: &Key) -> Gauge { + let ignored_metadata = Metadata::new("", metrics::Level::ERROR, None); + Gauge::new(self.recorder.register_gauge(key, &ignored_metadata)) + } + + fn metric_type() -> &'static str { + "gauge" + } +} + +impl<'a> RegisterMetric for Factory<'a, Histogram> { + fn register_metric(&self, key: &Key) -> Histogram { + let ignored_metadata = Metadata::new("", metrics::Level::ERROR, None); + Histogram::new(self.recorder.register_histogram(key, &ignored_metadata)) + } + + fn metric_type() -> &'static str { + "histogram" + } +} diff --git a/crates/astria-telemetry/src/metrics/gauge.rs b/crates/astria-telemetry/src/metrics/gauge.rs new file mode 100644 index 0000000000..ad7ccd9d9d --- /dev/null +++ b/crates/astria-telemetry/src/metrics/gauge.rs @@ -0,0 +1,32 @@ +use super::IntoF64; + +/// A gauge. +#[derive(Clone)] +pub struct Gauge(metrics::Gauge); + +impl Gauge { + /// Increments the gauge. + pub fn increment(&self, value: T) { + self.0.increment(value.into_f64()); + } + + /// Decrements the gauge. + pub fn decrement(&self, value: T) { + self.0.decrement(value.into_f64()); + } + + /// Sets the gauge. + pub fn set(&self, value: T) { + self.0.set(value.into_f64()); + } + + /// Creates a no-op gauge that does nothing. + #[must_use] + pub fn noop() -> Self { + Self(metrics::Gauge::noop()) + } + + pub(super) fn new(gauge: metrics::Gauge) -> Self { + Self(gauge) + } +} diff --git a/crates/astria-telemetry/src/metrics/handle.rs b/crates/astria-telemetry/src/metrics/handle.rs new file mode 100644 index 0000000000..2f2587dd89 --- /dev/null +++ b/crates/astria-telemetry/src/metrics/handle.rs @@ -0,0 +1,17 @@ +use metrics_exporter_prometheus::PrometheusHandle; + +/// A handle for rendering a snapshot of current metrics. +#[derive(Clone)] +pub struct Handle(PrometheusHandle); + +impl Handle { + /// Renders the current metrics in the same form as that of the metrics http server. + #[must_use] + pub fn render(&self) -> String { + self.0.render() + } + + pub(super) fn new(handle: PrometheusHandle) -> Self { + Self(handle) + } +} diff --git a/crates/astria-telemetry/src/metrics/histogram.rs b/crates/astria-telemetry/src/metrics/histogram.rs new file mode 100644 index 0000000000..8828c9a6bc --- /dev/null +++ b/crates/astria-telemetry/src/metrics/histogram.rs @@ -0,0 +1,22 @@ +use super::IntoF64; + +/// A histogram. +#[derive(Clone)] +pub struct Histogram(metrics::Histogram); + +impl Histogram { + /// Records a value in the histogram. + pub fn record(&self, value: T) { + self.0.record(value.into_f64()); + } + + /// Creates a no-op histogram that does nothing. + #[must_use] + pub fn noop() -> Self { + Self(metrics::Histogram::noop()) + } + + pub(super) fn new(histogram: metrics::Histogram) -> Self { + Self(histogram) + } +} diff --git a/crates/astria-telemetry/src/metrics/into_f64.rs b/crates/astria-telemetry/src/metrics/into_f64.rs new file mode 100644 index 0000000000..fcb357dc55 --- /dev/null +++ b/crates/astria-telemetry/src/metrics/into_f64.rs @@ -0,0 +1,59 @@ +/// A trait to safely convert to `f64`. +pub trait IntoF64 { + /// Converts `self` to `f64`. + fn into_f64(self) -> f64; +} + +impl IntoF64 for f64 { + fn into_f64(self) -> f64 { + self + } +} + +impl IntoF64 for std::time::Duration { + fn into_f64(self) -> f64 { + self.as_secs_f64() + } +} + +impl IntoF64 for i8 { + fn into_f64(self) -> f64 { + f64::from(self) + } +} + +impl IntoF64 for u8 { + fn into_f64(self) -> f64 { + f64::from(self) + } +} + +impl IntoF64 for i16 { + fn into_f64(self) -> f64 { + f64::from(self) + } +} + +impl IntoF64 for u16 { + fn into_f64(self) -> f64 { + f64::from(self) + } +} + +impl IntoF64 for i32 { + fn into_f64(self) -> f64 { + f64::from(self) + } +} + +impl IntoF64 for u32 { + fn into_f64(self) -> f64 { + f64::from(self) + } +} + +impl IntoF64 for f32 { + fn into_f64(self) -> f64 { + f64::from(self) + } +} diff --git a/crates/astria-telemetry/src/metrics/mod.rs b/crates/astria-telemetry/src/metrics/mod.rs new file mode 100644 index 0000000000..ef80dc914d --- /dev/null +++ b/crates/astria-telemetry/src/metrics/mod.rs @@ -0,0 +1,68 @@ +mod builders; +mod counter; +mod error; +mod factories; +mod gauge; +mod handle; +mod histogram; +mod into_f64; + +use metrics_exporter_prometheus::PrometheusBuilder; + +pub use self::{ + builders::{ + BucketBuilder, + ConfigBuilder, + RegisteringBuilder, + }, + counter::Counter, + error::Error, + factories::{ + CounterFactory, + GaugeFactory, + HistogramFactory, + }, + gauge::Gauge, + handle::Handle, + histogram::Histogram, + into_f64::IntoF64, +}; + +pub trait Metrics { + type Config; + + /// Sets the histograms' buckets as required. + /// + /// If not set for a given histogram, it will be rendered as a Prometheus summary rather than a + /// histogram. + /// + /// # Errors + /// + /// Implementations should return an error if setting buckets fails. + fn set_buckets(_builder: &mut BucketBuilder, _config: &Self::Config) -> Result<(), Error> { + Ok(()) + } + + /// Registers the individual metrics as required and returns an instance of `Self`. + /// + /// # Errors + /// + /// Implementations should return an error if registering metrics fails. + fn register(builder: &mut RegisteringBuilder, config: &Self::Config) -> Result + where + Self: Sized; + + /// Returns an instance of `Self` where the metrics are registered to a recorder that is + /// dropped immediately, meaning metrics aren't recorded. + /// + /// # Errors + /// + /// Implementations should return an error if setting buckets fails. + fn noop_metrics(config: &Self::Config) -> Result + where + Self: Sized, + { + let mut builder = RegisteringBuilder::new(PrometheusBuilder::new().build_recorder()); + Self::register(&mut builder, config) + } +} From 7432b9c7029550a2604fda7ce5ad55d694315e4b Mon Sep 17 00:00:00 2001 From: Fraser Hutchison Date: Fri, 30 Aug 2024 22:56:44 +0100 Subject: [PATCH 2/2] minor renames and re-export --- .../helpers/test_bridge_withdrawer.rs | 2 +- crates/astria-composer/src/executor/tests.rs | 2 +- crates/astria-composer/src/metrics.rs | 11 ++---- .../tests/blackbox/helper/mod.rs | 2 +- crates/astria-conductor/src/metrics.rs | 35 +++++-------------- .../tests/blackbox/helpers/mod.rs | 2 +- .../astria-sequencer-relayer/src/metrics.rs | 17 +++------ .../src/relayer/write/conversion.rs | 2 +- .../helpers/test_sequencer_relayer.rs | 2 +- crates/astria-sequencer/src/app/test_utils.rs | 2 +- crates/astria-sequencer/src/metrics.rs | 28 +++++---------- .../astria-sequencer/src/service/consensus.rs | 2 +- crates/astria-telemetry/src/lib.rs | 6 ++-- .../astria-telemetry/src/metrics/builders.rs | 6 ++-- .../astria-telemetry/src/metrics/into_f64.rs | 8 +++++ 15 files changed, 47 insertions(+), 80 deletions(-) diff --git a/crates/astria-bridge-withdrawer/tests/blackbox/helpers/test_bridge_withdrawer.rs b/crates/astria-bridge-withdrawer/tests/blackbox/helpers/test_bridge_withdrawer.rs index 40fcab8b4c..0847aa77cc 100644 --- a/crates/astria-bridge-withdrawer/tests/blackbox/helpers/test_bridge_withdrawer.rs +++ b/crates/astria-bridge-withdrawer/tests/blackbox/helpers/test_bridge_withdrawer.rs @@ -289,7 +289,7 @@ impl TestBridgeWithdrawerConfig { info!(config = serde_json::to_string(&config).unwrap()); let (metrics, metrics_handle) = metrics::ConfigBuilder::new() - .with_global_recorder(false) + .set_global_recorder(false) .build(&()) .unwrap(); let metrics = Box::leak(Box::new(metrics)); diff --git a/crates/astria-composer/src/executor/tests.rs b/crates/astria-composer/src/executor/tests.rs index cd2a385eb2..bb0e4af537 100644 --- a/crates/astria-composer/src/executor/tests.rs +++ b/crates/astria-composer/src/executor/tests.rs @@ -27,7 +27,7 @@ use prost::{ }; use sequencer_client::SignedTransaction; use serde_json::json; -use telemetry::metrics::Metrics as _; +use telemetry::Metrics as _; use tempfile::NamedTempFile; use tendermint::{ consensus::{ diff --git a/crates/astria-composer/src/metrics.rs b/crates/astria-composer/src/metrics.rs index 1362815722..28aff1dde8 100644 --- a/crates/astria-composer/src/metrics.rs +++ b/crates/astria-composer/src/metrics.rs @@ -7,7 +7,6 @@ use astria_core::primitive::v1::RollupId; use telemetry::{ metric_names, metrics::{ - self, Counter, Error, Gauge, @@ -98,19 +97,15 @@ impl Metrics { } pub(crate) fn record_txs_per_submission(&self, count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] - self.txs_per_submission.record(count as f64); + self.txs_per_submission.record(count); } pub(crate) fn record_bytes_per_submission(&self, byte_count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] - self.bytes_per_submission.record(byte_count as f64); + self.bytes_per_submission.record(byte_count); } } -impl metrics::Metrics for Metrics { +impl telemetry::Metrics for Metrics { type Config = crate::Config; fn register(builder: &mut RegisteringBuilder, config: &Self::Config) -> Result diff --git a/crates/astria-composer/tests/blackbox/helper/mod.rs b/crates/astria-composer/tests/blackbox/helper/mod.rs index b90d9daded..8b564b3001 100644 --- a/crates/astria-composer/tests/blackbox/helper/mod.rs +++ b/crates/astria-composer/tests/blackbox/helper/mod.rs @@ -144,7 +144,7 @@ pub async fn spawn_composer(rollup_ids: &[&str]) -> TestComposer { }; let (metrics, metrics_handle) = metrics::ConfigBuilder::new() - .with_global_recorder(false) + .set_global_recorder(false) .build(&config) .unwrap(); let metrics = Box::leak(Box::new(metrics)); diff --git a/crates/astria-conductor/src/metrics.rs b/crates/astria-conductor/src/metrics.rs index 165ef0282d..208bc2bbc4 100644 --- a/crates/astria-conductor/src/metrics.rs +++ b/crates/astria-conductor/src/metrics.rs @@ -1,7 +1,6 @@ use telemetry::{ metric_names, metrics::{ - self, Counter, Histogram, RegisteringBuilder, @@ -25,17 +24,11 @@ pub struct Metrics { impl Metrics { pub(crate) fn record_metadata_blobs_per_celestia_fetch(&self, blob_count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] - self.metadata_blobs_per_celestia_fetch - .record(blob_count as f64); + self.metadata_blobs_per_celestia_fetch.record(blob_count); } pub(crate) fn record_rollup_data_blobs_per_celestia_fetch(&self, blob_count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] - self.rollup_data_blobs_per_celestia_fetch - .record(blob_count as f64); + self.rollup_data_blobs_per_celestia_fetch.record(blob_count); } pub(crate) fn increment_celestia_blob_fetch_error_count(&self) { @@ -43,37 +36,29 @@ impl Metrics { } pub(crate) fn record_decoded_metadata_items_per_celestia_fetch(&self, item_count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] self.decoded_metadata_items_per_celestia_fetch - .record(item_count as f64); + .record(item_count); } pub(crate) fn record_decoded_rollup_data_items_per_celestia_fetch(&self, item_count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] self.decoded_rollup_data_items_per_celestia_fetch - .record(item_count as f64); + .record(item_count); } pub(crate) fn record_sequencer_blocks_metadata_verified_per_celestia_fetch( &self, block_count: usize, ) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] self.sequencer_blocks_metadata_verified_per_celestia_fetch - .record(block_count as f64); + .record(block_count); } pub(crate) fn record_sequencer_block_information_reconstructed_per_celestia_fetch( &self, block_count: usize, ) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] self.sequencer_block_information_reconstructed_per_celestia_fetch - .record(block_count as f64); + .record(block_count); } pub(crate) fn absolute_set_executed_firm_block_number(&self, block_number: u32) { @@ -87,19 +72,17 @@ impl Metrics { } pub(crate) fn record_transactions_per_executed_block(&self, tx_count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] - self.transactions_per_executed_block.record(tx_count as f64); + self.transactions_per_executed_block.record(tx_count); } } -impl metrics::Metrics for Metrics { +impl telemetry::Metrics for Metrics { type Config = (); fn register( builder: &mut RegisteringBuilder, _config: &Self::Config, - ) -> Result { + ) -> Result { let metadata = "metadata".to_string(); let rollup_data = "rollup_data".to_string(); diff --git a/crates/astria-conductor/tests/blackbox/helpers/mod.rs b/crates/astria-conductor/tests/blackbox/helpers/mod.rs index 0a6933647b..3dfdf28c4c 100644 --- a/crates/astria-conductor/tests/blackbox/helpers/mod.rs +++ b/crates/astria-conductor/tests/blackbox/helpers/mod.rs @@ -96,7 +96,7 @@ pub async fn spawn_conductor(execution_commit_level: CommitLevel) -> TestConduct }; let (metrics, metrics_handle) = metrics::ConfigBuilder::new() - .with_global_recorder(false) + .set_global_recorder(false) .build(&()) .unwrap(); let metrics = Box::leak(Box::new(metrics)); diff --git a/crates/astria-sequencer-relayer/src/metrics.rs b/crates/astria-sequencer-relayer/src/metrics.rs index ebba14b585..5665fed4ea 100644 --- a/crates/astria-sequencer-relayer/src/metrics.rs +++ b/crates/astria-sequencer-relayer/src/metrics.rs @@ -3,7 +3,6 @@ use std::time::Duration; use telemetry::{ metric_names, metrics::{ - self, Counter, Gauge, Histogram, @@ -40,21 +39,15 @@ impl Metrics { } pub(crate) fn record_blocks_per_celestia_tx(&self, block_count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] - self.blocks_per_celestia_tx.record(block_count as f64); + self.blocks_per_celestia_tx.record(block_count); } pub(crate) fn record_blobs_per_celestia_tx(&self, blob_count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] - self.blobs_per_celestia_tx.record(blob_count as f64); + self.blobs_per_celestia_tx.record(blob_count); } pub(crate) fn record_bytes_per_celestia_tx(&self, byte_count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] - self.bytes_per_celestia_tx.record(byte_count as f64); + self.bytes_per_celestia_tx.record(byte_count); } pub(crate) fn record_celestia_payload_creation_latency(&self, latency: Duration) { @@ -82,13 +75,13 @@ impl Metrics { } } -impl metrics::Metrics for Metrics { +impl telemetry::Metrics for Metrics { type Config = (); fn register( builder: &mut RegisteringBuilder, _config: &Self::Config, - ) -> Result { + ) -> Result { let celestia_submission_height = builder .new_counter_factory( CELESTIA_SUBMISSION_HEIGHT, diff --git a/crates/astria-sequencer-relayer/src/relayer/write/conversion.rs b/crates/astria-sequencer-relayer/src/relayer/write/conversion.rs index 34bb99bfcb..c8dc9d354f 100644 --- a/crates/astria-sequencer-relayer/src/relayer/write/conversion.rs +++ b/crates/astria-sequencer-relayer/src/relayer/write/conversion.rs @@ -487,7 +487,7 @@ mod tests { ChaChaRng, }; use sequencer_client::SequencerBlock; - use telemetry::metrics::Metrics as _; + use telemetry::Metrics as _; use super::{ Input, diff --git a/crates/astria-sequencer-relayer/tests/blackbox/helpers/test_sequencer_relayer.rs b/crates/astria-sequencer-relayer/tests/blackbox/helpers/test_sequencer_relayer.rs index a88fe8e054..041e6a488b 100644 --- a/crates/astria-sequencer-relayer/tests/blackbox/helpers/test_sequencer_relayer.rs +++ b/crates/astria-sequencer-relayer/tests/blackbox/helpers/test_sequencer_relayer.rs @@ -729,7 +729,7 @@ impl TestSequencerRelayerConfig { }; let (metrics, metrics_handle) = metrics::ConfigBuilder::new() - .with_global_recorder(false) + .set_global_recorder(false) .build(&()) .unwrap(); let metrics = Box::leak(Box::new(metrics)); diff --git a/crates/astria-sequencer/src/app/test_utils.rs b/crates/astria-sequencer/src/app/test_utils.rs index 0d0a80c401..63bbaf885b 100644 --- a/crates/astria-sequencer/src/app/test_utils.rs +++ b/crates/astria-sequencer/src/app/test_utils.rs @@ -23,7 +23,7 @@ use astria_core::{ }; use bytes::Bytes; use cnidarium::Storage; -use telemetry::metrics::Metrics as _; +use telemetry::Metrics as _; use crate::{ app::App, diff --git a/crates/astria-sequencer/src/metrics.rs b/crates/astria-sequencer/src/metrics.rs index 33ba5591b9..66a018ac2f 100644 --- a/crates/astria-sequencer/src/metrics.rs +++ b/crates/astria-sequencer/src/metrics.rs @@ -3,7 +3,6 @@ use std::time::Duration; use telemetry::{ metric_names, metrics::{ - self, Counter, Gauge, Histogram, @@ -56,21 +55,15 @@ impl Metrics { } pub(crate) fn set_prepare_proposal_excluded_transactions(&self, count: usize) { - #[allow(clippy::cast_precision_loss)] - self.prepare_proposal_excluded_transactions - .set(count as f64); + self.prepare_proposal_excluded_transactions.set(count); } pub(crate) fn record_proposal_deposits(&self, count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] - self.proposal_deposits.record(count as f64); + self.proposal_deposits.record(count); } pub(crate) fn record_proposal_transactions(&self, count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] - self.proposal_transactions.record(count as f64); + self.proposal_transactions.record(count); } pub(crate) fn increment_process_proposal_skipped_proposal(&self) { @@ -138,24 +131,19 @@ impl Metrics { } pub(crate) fn record_actions_per_transaction_in_mempool(&self, count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] - self.actions_per_transaction_in_mempool.record(count as f64); + self.actions_per_transaction_in_mempool.record(count); } pub(crate) fn record_transaction_in_mempool_size_bytes(&self, count: usize) { - // allow: precision loss is unlikely (values too small) but also unimportant in histograms. - #[allow(clippy::cast_precision_loss)] - self.transaction_in_mempool_size_bytes.record(count as f64); + self.transaction_in_mempool_size_bytes.record(count); } pub(crate) fn set_transactions_in_mempool_total(&self, count: usize) { - #[allow(clippy::cast_precision_loss)] - self.transactions_in_mempool_total.set(count as f64); + self.transactions_in_mempool_total.set(count); } } -impl metrics::Metrics for Metrics { +impl telemetry::Metrics for Metrics { type Config = (); // allow: this is reasonable as we have a lot of metrics to register; the function is not @@ -164,7 +152,7 @@ impl metrics::Metrics for Metrics { fn register( builder: &mut RegisteringBuilder, _config: &Self::Config, - ) -> Result { + ) -> Result { let prepare_proposal_excluded_transactions_cometbft_space = builder .new_counter_factory( PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_COMETBFT_SPACE, diff --git a/crates/astria-sequencer/src/service/consensus.rs b/crates/astria-sequencer/src/service/consensus.rs index 1dfc7685fb..4a4b290cda 100644 --- a/crates/astria-sequencer/src/service/consensus.rs +++ b/crates/astria-sequencer/src/service/consensus.rs @@ -220,7 +220,7 @@ mod test { use bytes::Bytes; use prost::Message as _; use rand::rngs::OsRng; - use telemetry::metrics::Metrics as _; + use telemetry::Metrics as _; use tendermint::{ account::Id, Hash, diff --git a/crates/astria-telemetry/src/lib.rs b/crates/astria-telemetry/src/lib.rs index 6c899eb981..4028a513dd 100644 --- a/crates/astria-telemetry/src/lib.rs +++ b/crates/astria-telemetry/src/lib.rs @@ -19,7 +19,7 @@ //! ``` use std::io::IsTerminal as _; -use metrics::Metrics; +pub use metrics::Metrics; use opentelemetry::{ global, trace::TracerProvider as _, @@ -181,8 +181,8 @@ impl Config { #[must_use = "telemetry must be initialized to be useful"] pub fn set_metrics(mut self, listening_addr: &str, service_name: &str) -> Self { let config_builder = metrics::ConfigBuilder::new() - .with_service_name(service_name) - .with_listening_address(listening_addr); + .set_service_name(service_name) + .set_listening_address(listening_addr); self.metrics_config_builder = Some(config_builder); self } diff --git a/crates/astria-telemetry/src/metrics/builders.rs b/crates/astria-telemetry/src/metrics/builders.rs index e8059536a6..d713b7ff48 100644 --- a/crates/astria-telemetry/src/metrics/builders.rs +++ b/crates/astria-telemetry/src/metrics/builders.rs @@ -60,21 +60,21 @@ impl ConfigBuilder { /// /// If `service_name` is empty, the label is not applied. #[must_use] - pub fn with_service_name(mut self, service_name: &str) -> Self { + pub fn set_service_name(mut self, service_name: &str) -> Self { self.service_name = service_name.to_string(); self } /// Sets the listening address of the exporter server. #[must_use] - pub fn with_listening_address(mut self, listening_address: &str) -> Self { + pub fn set_listening_address(mut self, listening_address: &str) -> Self { self.listening_address = Some(listening_address.to_string()); self } /// Enables or disables setting the global metrics recorder. #[must_use] - pub fn with_global_recorder(mut self, use_global_recorder: bool) -> Self { + pub fn set_global_recorder(mut self, use_global_recorder: bool) -> Self { self.use_global_recorder = use_global_recorder; self } diff --git a/crates/astria-telemetry/src/metrics/into_f64.rs b/crates/astria-telemetry/src/metrics/into_f64.rs index fcb357dc55..078d6211c8 100644 --- a/crates/astria-telemetry/src/metrics/into_f64.rs +++ b/crates/astria-telemetry/src/metrics/into_f64.rs @@ -57,3 +57,11 @@ impl IntoF64 for f32 { f64::from(self) } } + +impl IntoF64 for usize { + // allow: precision loss is unlikely (values too small) but also unimportant in metrics. + #[allow(clippy::cast_precision_loss)] + fn into_f64(self) -> f64 { + self as f64 + } +}