diff --git a/Cargo.lock b/Cargo.lock index da23ee14f..edcac97fb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -528,11 +528,10 @@ dependencies = [ "ethers", "futures", "hex", - "http", + "http 0.2.12", "humantime", - "hyper", + "hyper 0.14.30", "ibc-types", - "metrics", "once_cell", "pin-project-lite", "prost", @@ -604,10 +603,9 @@ dependencies = [ "futures", "hex", "humantime", - "hyper", + "hyper 0.14.30", "insta", "itertools 0.12.1", - "metrics", "once_cell", "pin-project-lite", "prost", @@ -654,14 +652,13 @@ dependencies = [ "futures", "futures-bounded", "hex", - "http", + "http 0.2.12", "humantime", "indexmap 2.4.0", "insta", "itertools 0.12.1", "itoa", "jsonrpsee", - "metrics", "moka", "once_cell", "pbjson-types", @@ -811,7 +808,6 @@ dependencies = [ "ibc-types", "insta", "matchit", - "metrics", "penumbra-ibc", "penumbra-proto", "penumbra-tower-trace", @@ -881,15 +877,14 @@ dependencies = [ "const_format", "futures", "hex", - "http", + "http 0.2.12", "humantime", "humantime-serde", - "hyper", + "hyper 0.14.30", "isahc", "itertools 0.12.1", "itoa", "k256", - "metrics", "once_cell", "pbjson-types", "pin-project-lite", @@ -945,6 +940,8 @@ dependencies = [ "base64 0.21.7", "base64-serde", "const_format", + "itertools 0.12.1", + "metrics 0.23.0", "metrics-exporter-prometheus", "opentelemetry", "opentelemetry-otlp", @@ -955,6 +952,7 @@ dependencies = [ "serde_json", "serde_with", "thiserror", + "tokio", "tracing", "tracing-opentelemetry", "tracing-subscriber 0.3.18", @@ -1073,6 +1071,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" @@ -1101,9 +1105,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.30", "itoa", "matchit", "memchr", @@ -1131,8 +1135,8 @@ dependencies = [ "async-trait", "bytes", "futures-util", - "http", - "http-body", + "http 0.2.12", + "http-body 0.4.6", "mime", "rustversion", "tower-layer", @@ -1564,7 +1568,7 @@ checksum = "4f4c948ab3cd9562d256b752d874d573c836ec8b200bba87d1154bbf662d3a00" dependencies = [ "async-trait", "celestia-types", - "http", + "http 0.2.12", "jsonrpsee", "serde", "thiserror", @@ -1824,7 +1828,7 @@ dependencies = [ "ibc-types", "ics23", "jmt", - "metrics", + "metrics 0.22.3", "once_cell", "parking_lot", "pin-project", @@ -3007,7 +3011,7 @@ dependencies = [ "futures-timer", "futures-util", "hashers", - "http", + "http 0.2.12", "instant", "jsonwebtoken", "once_cell", @@ -3489,7 +3493,26 @@ dependencies = [ "futures-core", "futures-sink", "futures-util", - "http", + "http 0.2.12", + "indexmap 2.4.0", + "slab", + "tokio", + "tokio-util 0.7.11", + "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.4.0", "slab", "tokio", @@ -3624,6 +3647,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" @@ -3631,7 +3665,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", ] @@ -3651,7 +3708,7 @@ dependencies = [ "async-channel", "base64 0.13.1", "futures-lite", - "http", + "http 0.2.12", "infer", "pin-project-lite", "rand 0.7.3", @@ -3700,9 +3757,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", @@ -3714,6 +3771,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" @@ -3721,8 +3799,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.30", "log", "rustls", "rustls-native-certs", @@ -3736,12 +3814,32 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbb958482e8c7be4bc3cf272a766a2b0bf1a6755e7a6ae777f017a31d11b13b1" dependencies = [ - "hyper", + "hyper 0.14.30", "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" @@ -4262,7 +4360,7 @@ dependencies = [ "encoding_rs", "event-listener 2.5.3", "futures-lite", - "http", + "http 0.2.12", "log", "mime", "once_cell", @@ -4373,7 +4471,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", @@ -4398,7 +4496,7 @@ dependencies = [ "beef", "futures-timer", "futures-util", - "hyper", + "hyper 0.14.30", "jsonrpsee-types", "parking_lot", "rand 0.8.5", @@ -4418,7 +4516,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5f80c17f62c7653ce767e3d7288b793dfec920f97067ceb189ebdd3570f2bc20" dependencies = [ "async-trait", - "hyper", + "hyper 0.14.30", "hyper-rustls", "jsonrpsee-core", "jsonrpsee-types", @@ -4451,8 +4549,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.30", "jsonrpsee-core", "jsonrpsee-types", "route-recognizer", @@ -4487,7 +4585,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", @@ -4726,33 +4824,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.4.0", "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.5", - "metrics", + "metrics 0.23.0", "num_cpus", "quanta", "sketches-ddsketch", @@ -5103,7 +5214,7 @@ checksum = "1a016b8d9495c639af2145ac22387dcb88e44118e45320d9238fbf4e7889abcb" dependencies = [ "async-trait", "futures-core", - "http", + "http 0.2.12", "opentelemetry", "opentelemetry-proto", "opentelemetry-semantic-conventions", @@ -5432,7 +5543,7 @@ dependencies = [ "ibc-proto", "ibc-types", "ics23", - "metrics", + "metrics 0.22.3", "num-traits", "once_cell", "pbjson-types", @@ -5583,7 +5694,7 @@ dependencies = [ "decaf377-rdsa", "hex", "im", - "metrics", + "metrics 0.22.3", "once_cell", "pbjson-types", "penumbra-keys", @@ -5633,7 +5744,7 @@ source = "git+https://github.com/penumbra-zone/penumbra.git?rev=87adc8d6b15f6081 dependencies = [ "futures", "hex", - "http", + "http 0.2.12", "pin-project", "pin-project-lite", "sha2 0.10.8", @@ -6271,10 +6382,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.30", "hyper-rustls", "ipnet", "js-sys", @@ -7101,7 +7212,7 @@ dependencies = [ "base64 0.13.1", "bytes", "futures", - "http", + "http 0.2.12", "httparse", "log", "rand 0.8.5", @@ -7723,10 +7834,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.30", "hyper-timeout", "percent-encoding", "pin-project", @@ -7755,10 +7866,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.30", "hyper-timeout", "percent-encoding", "pin-project", @@ -7862,8 +7973,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", @@ -8047,7 +8158,7 @@ dependencies = [ "byteorder", "bytes", "data-encoding", - "http", + "http 0.2.12", "httparse", "log", "rand 0.8.5", @@ -8645,7 +8756,7 @@ dependencies = [ "futures", "futures-timer", "http-types", - "hyper", + "hyper 0.14.30", "log", "once_cell", "regex", diff --git a/Cargo.toml b/Cargo.toml index a05d327ad..649ba90f0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -73,7 +73,6 @@ insta = "1.36.1" itertools = "0.12.1" itoa = "1.0.10" jsonrpsee = { version = "0.20" } -metrics = "0.22.1" once_cell = "1.17.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. diff --git a/charts/sequencer-relayer/Chart.yaml b/charts/sequencer-relayer/Chart.yaml index 52ee27d5b..9e94ed690 100644 --- a/charts/sequencer-relayer/Chart.yaml +++ b/charts/sequencer-relayer/Chart.yaml @@ -15,7 +15,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.11.2 +version: 0.12.0 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to diff --git a/charts/sequencer-relayer/files/scripts/start-relayer.sh b/charts/sequencer-relayer/files/scripts/start-relayer.sh index 746d71022..83036fca5 100644 --- a/charts/sequencer-relayer/files/scripts/start-relayer.sh +++ b/charts/sequencer-relayer/files/scripts/start-relayer.sh @@ -4,16 +4,6 @@ set -o errexit -o nounset -o pipefail echo "Starting the Astria Sequencer Relayer..." -if ! [ -z ${ASTRIA_SEQUENCER_RELAYER_PRE_SUBMIT_PATH+x} ] && ! [ -f "$ASTRIA_SEQUENCER_RELAYER_PRE_SUBMIT_PATH" ]; then - echo "Pre-submit storage file not found, instantiating with ignore state. Post submit storage file will be created on first submit." - echo "{\"state\": \"ignore\"}" > $ASTRIA_SEQUENCER_RELAYER_PRE_SUBMIT_PATH -fi - -if ! [ -z ${ASTRIA_SEQUENCER_RELAYER_POST_SUBMIT_PATH+x} ] && ! [ -f "$ASTRIA_SEQUENCER_RELAYER_POST_SUBMIT_PATH" ]; then - echo "Post-submit storage file does not exist, instantiating with fresh state. Will start relaying from first sequencer block." - echo "{\"state\": \"fresh\"}" > $ASTRIA_SEQUENCER_RELAYER_POST_SUBMIT_PATH -fi - if ! [ -z ${ASTRIA_SEQUENCER_RELAYER_SUBMISSION_STATE_PATH+x} ] && ! [ -f "$ASTRIA_SEQUENCER_RELAYER_SUBMISSION_STATE_PATH" ]; then echo "Submission state file does not exist, instantiating with fresh state. Will start relaying from first sequencer block." echo "{\"state\": \"fresh\"}" > $ASTRIA_SEQUENCER_RELAYER_SUBMISSION_STATE_PATH diff --git a/charts/sequencer-relayer/templates/configmaps.yaml b/charts/sequencer-relayer/templates/configmaps.yaml index b76f6c402..c59838009 100644 --- a/charts/sequencer-relayer/templates/configmaps.yaml +++ b/charts/sequencer-relayer/templates/configmaps.yaml @@ -5,6 +5,7 @@ metadata: namespace: {{ include "sequencer-relayer.namespace" . }} data: ASTRIA_SEQUENCER_RELAYER_LOG: "astria_sequencer_relayer=debug" + ASTRIA_SEQUENCER_RELAYER_SUBMISSION_STATE_PATH: "{{ include "sequencer-relayer.storage.submissionStatePath" . }}" ASTRIA_SEQUENCER_RELAYER_BLOCK_TIME: "1000" ASTRIA_SEQUENCER_RELAYER_COMETBFT_ENDPOINT: "{{ .Values.config.relayer.cometbftRpc }}" ASTRIA_SEQUENCER_RELAYER_SEQUENCER_GRPC_ENDPOINT: "{{ .Values.config.relayer.sequencerGrpc }}" @@ -28,10 +29,7 @@ data: ASTRIA_SEQUENCER_RELAYER_SEQUENCER_CHAIN_ID: "{{ .Values.config.relayer.sequencerChainId }}" ASTRIA_SEQUENCER_RELAYER_CELESTIA_CHAIN_ID: "{{ .Values.config.relayer.celestiaChainId }}" {{- if not .Values.global.dev }} - ASTRIA_SEQUENCER_RELAYER_PRE_SUBMIT_PATH: "{{ include "sequencer-relayer.storage.preSubmitPath" . }}" - ASTRIA_SEQUENCER_RELAYER_POST_SUBMIT_PATH: "{{ include "sequencer-relayer.storage.postSubmitPath" . }}" {{- else }} - ASTRIA_SEQUENCER_RELAYER_SUBMISSION_STATE_PATH: "{{ include "sequencer-relayer.storage.submissionStatePath" . }}" {{- end }} --- apiVersion: v1 diff --git a/charts/sequencer/Chart.lock b/charts/sequencer/Chart.lock index f53df9a22..cbbd856d4 100644 --- a/charts/sequencer/Chart.lock +++ b/charts/sequencer/Chart.lock @@ -1,6 +1,6 @@ dependencies: - name: sequencer-relayer repository: file://../sequencer-relayer - version: 0.11.2 -digest: sha256:76454e176c237a3099be9d76ffd20a1a640cfc3d48c7598c9de8a9332ac802ad -generated: "2024-08-22T13:56:23.979747-07:00" + version: 0.12.0 +digest: sha256:e7eaa26032e2e92f8df8eaea11580ad9e464866360d4e5d4c14bae4eec8a090b +generated: "2024-08-30T12:12:47.838866-07:00" diff --git a/charts/sequencer/Chart.yaml b/charts/sequencer/Chart.yaml index cd77e6335..99346c5cb 100644 --- a/charts/sequencer/Chart.yaml +++ b/charts/sequencer/Chart.yaml @@ -15,7 +15,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.21.0 +version: 0.22.0 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. @@ -24,7 +24,7 @@ appVersion: "0.16.0" dependencies: - name: sequencer-relayer - version: "0.11.2" + version: "0.12.0" repository: "file://../sequencer-relayer" condition: sequencer-relayer.enabled diff --git a/crates/astria-bridge-contracts/src/lib.rs b/crates/astria-bridge-contracts/src/lib.rs index 49b4b0161..157e80921 100644 --- a/crates/astria-bridge-contracts/src/lib.rs +++ b/crates/astria-bridge-contracts/src/lib.rs @@ -332,7 +332,7 @@ where pub async fn get_for_block_hash( &self, block_hash: H256, - ) -> Result, GetWithdrawalActionsError> { + ) -> Result>, GetWithdrawalActionsError> { use futures::FutureExt as _; let get_ics20_logs = if self.configured_for_ics20_withdrawals() { get_logs::(&self.provider, self.contract_address, block_hash) @@ -358,7 +358,7 @@ where // XXX: The calls to `log_to_*_action` rely on only be called if `GetWithdrawalActions` // is configured for either ics20 or sequencer withdrawals (or both). They would panic // otherwise. - ics20_logs + Ok(ics20_logs .into_iter() .map(|log| self.log_to_ics20_withdrawal_action(log)) .chain( @@ -366,7 +366,7 @@ where .into_iter() .map(|log| self.log_to_sequencer_withdrawal_action(log)), ) - .collect() + .collect()) } fn log_to_ics20_withdrawal_action( @@ -378,7 +378,7 @@ where .ok_or_else(|| GetWithdrawalActionsError::log_without_block_number(&log))? .as_u64(); - let rollup_transaction_hash = log + let rollup_withdrawal_event_id = log .transaction_hash .ok_or_else(|| GetWithdrawalActionsError::log_without_transaction_hash(&log))? .to_string(); @@ -400,7 +400,7 @@ where memo: event.memo.clone(), rollup_block_number, rollup_return_address: event.sender.to_string(), - rollup_transaction_hash, + rollup_withdrawal_event_id, }) .map_err(GetWithdrawalActionsError::encode_memo)?; @@ -433,7 +433,7 @@ where .ok_or_else(|| GetWithdrawalActionsError::log_without_block_number(&log))? .as_u64(); - let rollup_transaction_hash = log + let rollup_withdrawal_event_id = log .transaction_hash .ok_or_else(|| GetWithdrawalActionsError::log_without_transaction_hash(&log))? .to_string(); @@ -441,12 +441,6 @@ where let event = decode_log::(log) .map_err(GetWithdrawalActionsError::decode_log)?; - let memo = memo_to_json(&memos::v1alpha1::BridgeUnlock { - rollup_block_number, - rollup_transaction_hash, - }) - .map_err(GetWithdrawalActionsError::encode_memo)?; - let amount = calculate_amount(&event, self.asset_withdrawal_divisor) .map_err(GetWithdrawalActionsError::calculate_withdrawal_amount)?; @@ -456,7 +450,9 @@ where let action = astria_core::protocol::transaction::v1alpha1::action::BridgeUnlockAction { to, amount, - memo, + rollup_block_number, + rollup_withdrawal_event_id, + memo: String::new(), fee_asset: self.fee_asset.clone(), bridge_address: self.bridge_address, }; diff --git a/crates/astria-bridge-withdrawer/Cargo.toml b/crates/astria-bridge-withdrawer/Cargo.toml index bc6e68498..e403dabca 100644 --- a/crates/astria-bridge-withdrawer/Cargo.toml +++ b/crates/astria-bridge-withdrawer/Cargo.toml @@ -21,7 +21,6 @@ ethers = { workspace = true, features = ["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/api.rs b/crates/astria-bridge-withdrawer/src/api.rs index 81eb2fa13..2a1abe248 100644 --- a/crates/astria-bridge-withdrawer/src/api.rs +++ b/crates/astria-bridge-withdrawer/src/api.rs @@ -20,6 +20,7 @@ use http::status::StatusCode; use hyper::server::conn::AddrIncoming; use serde::Serialize; use tokio::sync::watch; +use tracing::instrument; use crate::bridge_withdrawer::StateSnapshot; @@ -51,6 +52,7 @@ pub(crate) fn start(socket_addr: SocketAddr, withdrawer_state: WithdrawerState) } #[allow(clippy::unused_async)] // Permit because axum handlers must be async +#[instrument(skip_all)] async fn get_healthz(State(withdrawer_state): State) -> Healthz { if withdrawer_state.borrow().is_healthy() { Healthz::Ok @@ -66,6 +68,7 @@ async fn get_healthz(State(withdrawer_state): State) -> Healthz /// + there is a current sequencer height (implying a block from sequencer was received) /// + there is a current data availability height (implying a height was received from the DA) #[allow(clippy::unused_async)] // Permit because axum handlers must be async +#[instrument(skip_all)] async fn get_readyz(State(withdrawer_state): State) -> Readyz { let is_withdrawer_online = withdrawer_state.borrow().is_ready(); if is_withdrawer_online { @@ -76,6 +79,7 @@ async fn get_readyz(State(withdrawer_state): State) -> Readyz { } #[allow(clippy::unused_async)] // Permit because axum handlers must be async +#[instrument(skip_all)] async fn get_status(State(withdrawer_state): State) -> Json { Json(withdrawer_state.borrow().clone()) } diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs index 3de1a631f..a188fbd59 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/ethereum/watcher.rs @@ -7,9 +7,12 @@ use astria_bridge_contracts::{ GetWithdrawalActions, GetWithdrawalActionsBuilder, }; -use astria_core::primitive::v1::{ - asset, - Address, +use astria_core::{ + primitive::v1::{ + asset, + Address, + }, + protocol::transaction::v1alpha1::Action, }; use astria_eyre::{ eyre::{ @@ -38,6 +41,7 @@ use tokio_util::sync::CancellationToken; use tracing::{ debug, info, + info_span, instrument, warn, }; @@ -293,11 +297,13 @@ async fn watch_for_blocks( bail!("current rollup block missing block number") }; - info!( - block.height = current_rollup_block_height.as_u64(), - block.hash = current_rollup_block.hash.map(tracing::field::display), - "got current block" - ); + info_span!("watch_for_blocks").in_scope(|| { + info!( + block.height = current_rollup_block_height.as_u64(), + block.hash = current_rollup_block.hash.map(tracing::field::display), + "got current block" + ); + }); // sync any blocks missing between `next_rollup_block_height` and the current latest // (inclusive). @@ -314,7 +320,7 @@ async fn watch_for_blocks( loop { select! { () = shutdown_token.cancelled() => { - info!("block watcher shutting down"); + info_span!("watch_for_blocks").in_scope(|| info!("block watcher shutting down")); return Ok(()); } block = block_rx.next() => { @@ -348,10 +354,21 @@ async fn get_and_forward_block_events( .number .ok_or_eyre("block did not contain a rollup height")? .as_u64(); - let actions = actions_fetcher + let actions: Vec = actions_fetcher .get_for_block_hash(block_hash) .await - .wrap_err("failed getting actions for block")?; + .wrap_err("failed getting actions for block")? + .into_iter() + .filter_map(|r| { + r.map_err(|e| { + warn!( + error = %eyre::Report::new(e), + "failed to convert rollup withdrawal event to sequencer action; dropping" + ); + }) + .ok() + }) + .collect(); if actions.is_empty() { info!( diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs index d886331a3..202fd031e 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/mod.rs @@ -1,9 +1,6 @@ use std::{ net::SocketAddr, - sync::{ - Arc, - OnceLock, - }, + sync::Arc, time::Duration, }; @@ -11,9 +8,20 @@ use astria_eyre::eyre::{ self, WrapErr as _, }; +use axum::{ + routing::IntoMakeService, + Router, + Server, +}; +use ethereum::watcher::Watcher; +use hyper::server::conn::AddrIncoming; +use startup::Startup; use tokio::{ select, - sync::oneshot, + sync::oneshot::{ + self, + Receiver, + }, task::{ JoinError, JoinHandle, @@ -24,6 +32,7 @@ use tokio_util::sync::CancellationToken; use tracing::{ error, info, + instrument, }; pub(crate) use self::state::StateSnapshot; @@ -60,10 +69,7 @@ impl BridgeWithdrawer { /// # 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, @@ -167,34 +173,29 @@ impl BridgeWithdrawer { // Separate the API shutdown signal from the cancellation token because we want it to live // until the very end. let (api_shutdown_signal, api_shutdown_signal_rx) = oneshot::channel::<()>(); - let mut api_task = tokio::spawn(async move { - api_server - .with_graceful_shutdown(async move { - let _ = api_shutdown_signal_rx.await; - }) - .await - .wrap_err("api server ended unexpectedly") - }); - info!("spawned API server"); - - let mut startup_task = Some(tokio::spawn(startup.run())); - info!("spawned startup task"); - - let mut submitter_task = tokio::spawn(submitter.run()); - info!("spawned submitter task"); - let mut ethereum_watcher_task = tokio::spawn(ethereum_watcher.run()); - info!("spawned ethereum watcher task"); + let TaskHandles { + mut api_task, + mut startup_task, + mut submitter_task, + mut ethereum_watcher_task, + } = spawn_tasks( + api_server, + api_shutdown_signal_rx, + startup, + submitter, + ethereum_watcher, + ); let shutdown = loop { select!( o = async { startup_task.as_mut().unwrap().await }, if startup_task.is_none() => { match o { Ok(_) => { - info!(task = "startup", "task has exited"); + report_exit("startup", Ok(Ok(()))); startup_task = None; }, Err(error) => { - error!(task = "startup", %error, "task returned with error"); + report_exit("startup", Err(error)); break Shutdown { api_task: Some(api_task), submitter_task: Some(submitter_task), @@ -245,6 +246,48 @@ impl BridgeWithdrawer { } } +#[allow(clippy::struct_field_names)] // allow: for parity with the `Shutdown` struct. +struct TaskHandles { + api_task: JoinHandle>, + startup_task: Option>>, + submitter_task: JoinHandle>, + ethereum_watcher_task: JoinHandle>, +} + +#[instrument(skip_all)] +fn spawn_tasks( + api_server: Server>, + api_shutdown_signal_rx: Receiver<()>, + startup: Startup, + submitter: Submitter, + ethereum_watcher: Watcher, +) -> TaskHandles { + let api_task = tokio::spawn(async move { + api_server + .with_graceful_shutdown(async move { + let _ = api_shutdown_signal_rx.await; + }) + .await + .wrap_err("api server ended unexpectedly") + }); + info!("spawned API server"); + + let startup_task = Some(tokio::spawn(startup.run())); + info!("spawned startup task"); + + let submitter_task = tokio::spawn(submitter.run()); + info!("spawned submitter task"); + let ethereum_watcher_task = tokio::spawn(ethereum_watcher.run()); + info!("spawned ethereum watcher task"); + + TaskHandles { + api_task, + startup_task, + submitter_task, + ethereum_watcher_task, + } +} + /// A handle for instructing the [`Service`] to shut down. /// /// It is returned along with its related `Service` from [`Service::new`]. The @@ -275,6 +318,7 @@ impl ShutdownHandle { } impl Drop for ShutdownHandle { + #[instrument(skip_all)] fn drop(&mut self) { if !self.token.is_cancelled() { info!("shutdown handle dropped, issuing shutdown to all services"); @@ -283,6 +327,7 @@ impl Drop for ShutdownHandle { } } +#[instrument(skip_all)] fn report_exit(task_name: &str, outcome: Result, JoinError>) { match outcome { Ok(Ok(())) => info!(task = task_name, "task has exited"), @@ -314,6 +359,7 @@ impl Shutdown { const STARTUP_SHUTDOWN_TIMEOUT_SECONDS: u64 = 1; const SUBMITTER_SHUTDOWN_TIMEOUT_SECONDS: u64 = 19; + #[instrument(skip_all)] async fn run(self) { let Self { api_task, diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs index a040347ec..1d5f353b9 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs @@ -47,6 +47,7 @@ use tracing::{ instrument, warn, Instrument as _, + Level, Span, }; use tryhard::backoff_strategies::ExponentialBackoff; @@ -120,6 +121,7 @@ impl InfoHandle { } } + #[instrument(skip_all, err)] pub(super) async fn get_info(&mut self) -> eyre::Result { let state = self .rx @@ -202,6 +204,7 @@ impl Startup { /// - `self.chain_id` does not match the value returned from the sequencer node /// - `self.fee_asset` is not a valid fee asset on the sequencer node /// - `self.sequencer_bridge_address` does not have a sufficient balance of `self.fee_asset`. + #[instrument(skip_all, err)] async fn confirm_sequencer_config(&self) -> eyre::Result<()> { // confirm the sequencer chain id let actual_chain_id = @@ -250,6 +253,7 @@ impl Startup { /// in the sequencer logic). /// 5. Failing to convert the transaction data from bytes to proto. /// 6. Failing to convert the transaction data from proto to `SignedTransaction`. + #[instrument(skip_all, err)] async fn get_last_transaction(&self) -> eyre::Result> { // get last transaction hash by the bridge account, if it exists let last_transaction_hash_resp = get_bridge_account_last_transaction_hash( @@ -323,6 +327,7 @@ impl Startup { /// the sequencer logic) /// 3. The last transaction by the bridge account did not contain a withdrawal action /// 4. The memo of the last transaction by the bridge account could not be parsed + #[instrument(skip_all, err)] async fn get_starting_rollup_height(&mut self) -> eyre::Result { let signed_transaction = self .get_last_transaction() @@ -347,6 +352,7 @@ impl Startup { } } +#[instrument(skip_all, err(level = Level::WARN))] async fn ensure_mempool_empty( cometbft_client: sequencer_client::HttpClient, sequencer_client: sequencer_service_client::SequencerServiceClient, @@ -391,6 +397,7 @@ async fn ensure_mempool_empty( /// 2. Failing to get the latest nonce from cometBFT's mempool. /// 3. The pending nonce from the Sequencer's app-side mempool does not match the latest nonce from /// cometBFT's mempool after the exponential backoff times out. +#[instrument(skip_all, err)] async fn wait_for_empty_mempool( cometbft_client: sequencer_client::HttpClient, sequencer_grpc_endpoint: String, @@ -461,11 +468,7 @@ fn rollup_height_from_signed_transaction( .ok_or_eyre("last transaction by the bridge account did not contain a withdrawal action")?; let last_batch_rollup_height = match withdrawal_action { - Action::BridgeUnlock(action) => { - let memo: memos::v1alpha1::BridgeUnlock = serde_json::from_str(&action.memo) - .wrap_err("failed to parse memo from last transaction by the bridge account")?; - Some(memo.rollup_block_number) - } + Action::BridgeUnlock(action) => Some(action.rollup_block_number), Action::Ics20Withdrawal(action) => { let memo: memos::v1alpha1::Ics20WithdrawalFromRollup = serde_json::from_str(&action.memo) @@ -485,7 +488,7 @@ fn rollup_height_from_signed_transaction( Ok(last_batch_rollup_height) } -#[instrument(skip_all)] +#[instrument(skip_all, err)] async fn get_bridge_account_last_transaction_hash( client: sequencer_client::HttpClient, state: Arc, @@ -507,7 +510,7 @@ async fn get_bridge_account_last_transaction_hash( res } -#[instrument(skip_all)] +#[instrument(skip_all, err)] async fn get_sequencer_transaction_at_hash( client: sequencer_client::HttpClient, state: Arc, @@ -525,7 +528,7 @@ async fn get_sequencer_transaction_at_hash( res } -#[instrument(skip_all)] +#[instrument(skip_all, err)] async fn get_sequencer_chain_id( client: sequencer_client::HttpClient, state: Arc, @@ -542,7 +545,7 @@ async fn get_sequencer_chain_id( Ok(genesis.chain_id) } -#[instrument(skip_all)] +#[instrument(skip_all, err)] async fn get_allowed_fee_assets( client: sequencer_client::HttpClient, state: Arc, @@ -559,7 +562,7 @@ async fn get_allowed_fee_assets( res } -#[instrument(skip_all)] +#[instrument(skip_all, err)] async fn get_latest_nonce( client: sequencer_client::HttpClient, state: Arc, diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs index e93551138..e20335cbc 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/builder.rs @@ -6,7 +6,10 @@ use astria_eyre::eyre::{ }; use tokio::sync::mpsc; use tokio_util::sync::CancellationToken; -use tracing::info; +use tracing::{ + info, + instrument, +}; use super::state::State; use crate::{ @@ -30,6 +33,7 @@ impl Handle { } } + #[instrument(skip_all, err)] pub(crate) async fn send_batch(&self, batch: Batch) -> eyre::Result<()> { self.batches_tx .send(batch) diff --git a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs index 4febe56ec..21ffba98b 100644 --- a/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/bridge_withdrawer/submitter/mod.rs @@ -74,7 +74,7 @@ impl Submitter { pub(super) async fn run(mut self) -> eyre::Result<()> { let (sequencer_chain_id, sequencer_grpc_client) = select! { () = self.shutdown_token.cancelled() => { - info!("submitter received shutdown signal while waiting for startup"); + report_exit(Ok("submitter received shutdown signal while waiting for startup")); return Ok(()); } @@ -96,7 +96,6 @@ impl Submitter { biased; () = self.shutdown_token.cancelled() => { - info!("received shutdown signal"); break Ok("shutdown requested"); } @@ -124,17 +123,12 @@ impl Submitter { // close the channel to signal to batcher that the submitter is shutting down self.batches_rx.close(); - match reason { - Ok(reason) => info!(reason, "submitter shutting down"), - Err(reason) => { - error!(%reason, "submitter shutting down"); - } - } + report_exit(reason); Ok(()) } - #[instrument(skip_all)] + #[instrument(skip_all, err)] async fn process_batch( &self, sequencer_grpc_client: SequencerServiceClient, @@ -182,24 +176,18 @@ impl Submitter { .await .context("failed to submit transaction to cometbft")?; if let tendermint::abci::Code::Err(check_tx_code) = rsp.check_tx.code { - error!( - abci.code = check_tx_code, - abci.log = rsp.check_tx.log, - rollup.height = rollup_height, - "transaction failed to be included in the mempool, aborting." - ); Err(eyre!( - "check_tx failure upon submitting transaction to sequencer" + "check_tx failure upon submitting transaction to sequencer: transaction failed to \ + be included in the mempool, aborting. abci.code = {check_tx_code}, abci.log = \ + {}, rollup.height = {rollup_height}", + rsp.check_tx.log )) } else if let tendermint::abci::Code::Err(deliver_tx_code) = rsp.tx_result.code { - error!( - abci.code = deliver_tx_code, - abci.log = rsp.tx_result.log, - rollup.height = rollup_height, - "transaction failed to be executed in a block, aborting." - ); Err(eyre!( - "deliver_tx failure upon submitting transaction to sequencer" + "deliver_tx failure upon submitting transaction to sequencer: transaction failed \ + to be executed in a block, aborting. abci.code = {deliver_tx_code}, abci.log = \ + {}, rollup.height = {rollup_height}", + rsp.tx_result.log, )) } else { // update state after successful submission @@ -217,6 +205,16 @@ impl Submitter { } } +#[instrument(skip_all)] +fn report_exit(reason: eyre::Result<&str>) { + match reason { + Ok(reason) => info!(%reason, "submitter shutting down"), + Err(reason) => { + error!(%reason, "submitter shutting down"); + } + } +} + /// Submits a `SignedTransaction` to the sequencer with an exponential backoff #[instrument( name = "submit_tx", @@ -224,7 +222,8 @@ impl Submitter { fields( nonce = tx.nonce(), transaction.hash = %telemetry::display::hex(&tx.sha256_of_proto_encoding()), - ) + ), + err )] async fn submit_tx( client: sequencer_client::HttpClient, @@ -279,6 +278,7 @@ async fn submit_tx( res } +#[instrument(skip_all, err)] pub(crate) async fn get_pending_nonce( client: sequencer_service_client::SequencerServiceClient, address: Address, diff --git a/crates/astria-bridge-withdrawer/src/lib.rs b/crates/astria-bridge-withdrawer/src/lib.rs index 867015602..e215e1bd3 100644 --- a/crates/astria-bridge-withdrawer/src/lib.rs +++ b/crates/astria-bridge-withdrawer/src/lib.rs @@ -7,3 +7,4 @@ pub(crate) mod metrics; pub use bridge_withdrawer::BridgeWithdrawer; pub use build_info::BUILD_INFO; pub use config::Config; +pub use metrics::Metrics; diff --git a/crates/astria-bridge-withdrawer/src/main.rs b/crates/astria-bridge-withdrawer/src/main.rs index 89e6f6893..546ca608b 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,7 @@ async fn main() -> ExitCode { let mut sigterm = signal(SignalKind::terminate()) .expect("setting a SIGTERM listener should always work on Unix"); - let (withdrawer, shutdown_handle) = match BridgeWithdrawer::new(cfg) { + let (withdrawer, shutdown_handle) = match BridgeWithdrawer::new(cfg, metrics) { Err(error) => { error!(%error, "failed initializing bridge withdrawer"); return ExitCode::FAILURE; diff --git a/crates/astria-bridge-withdrawer/src/metrics.rs b/crates/astria-bridge-withdrawer/src/metrics.rs index 28548007d..0da7143af 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 { current_nonce: Gauge, nonce_fetch_count: Counter, nonce_fetch_failure_count: Counter, @@ -24,56 +21,6 @@ pub(crate) struct Metrics { } impl Metrics { - #[must_use] - pub(crate) fn new() -> Self { - describe_gauge!(CURRENT_NONCE, Unit::Count, "The current nonce"); - let current_nonce = gauge!(CURRENT_NONCE); - - describe_counter!( - NONCE_FETCH_COUNT, - Unit::Count, - "The number of times a nonce was fetched from the sequencer" - ); - let nonce_fetch_count = counter!(NONCE_FETCH_COUNT); - - describe_counter!( - NONCE_FETCH_FAILURE_COUNT, - Unit::Count, - "The number of failed attempts to fetch nonce from sequencer" - ); - let nonce_fetch_failure_count = counter!(NONCE_FETCH_FAILURE_COUNT); - - describe_histogram!( - NONCE_FETCH_LATENCY, - Unit::Seconds, - "The latency of fetching nonce from sequencer" - ); - let nonce_fetch_latency = histogram!(NONCE_FETCH_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!( - SEQUENCER_SUBMISSION_LATENCY, - Unit::Seconds, - "The latency of submitting a transaction to the sequencer" - ); - let sequencer_submission_latency = histogram!(SEQUENCER_SUBMISSION_LATENCY); - - Self { - current_nonce, - nonce_fetch_count, - nonce_fetch_failure_count, - nonce_fetch_latency, - sequencer_submission_failure_count, - sequencer_submission_latency, - } - } - pub(crate) fn set_current_nonce(&self, nonce: u32) { self.current_nonce.set(nonce); } @@ -99,11 +46,68 @@ 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 current_nonce = builder + .new_gauge_factory(CURRENT_NONCE, "The current nonce")? + .register()?; + + let nonce_fetch_count = builder + .new_counter_factory( + NONCE_FETCH_COUNT, + "The number of times a nonce was fetched from the sequencer", + )? + .register()?; + + let nonce_fetch_failure_count = builder + .new_counter_factory( + NONCE_FETCH_FAILURE_COUNT, + "The number of failed attempts to fetch nonce from sequencer", + )? + .register()?; + + let nonce_fetch_latency = builder + .new_histogram_factory( + NONCE_FETCH_LATENCY, + "The latency of fetching nonce from sequencer", + )? + .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 { + current_nonce, + nonce_fetch_count, + nonce_fetch_failure_count, + nonce_fetch_latency, + 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/tests/blackbox/helpers/test_bridge_withdrawer.rs b/crates/astria-bridge-withdrawer/tests/blackbox/helpers/test_bridge_withdrawer.rs index 012988fef..0847aa77c 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 @@ -9,6 +9,7 @@ use astria_bridge_withdrawer::{ bridge_withdrawer::ShutdownHandle, BridgeWithdrawer, Config, + Metrics, }; use astria_core::{ primitive::v1::asset::{ @@ -17,10 +18,7 @@ use astria_core::{ }, protocol::{ bridge::v1alpha1::BridgeAccountLastTxHashResponse, - memos::v1alpha1::{ - BridgeUnlock, - Ics20WithdrawalFromRollup, - }, + memos::v1alpha1::Ics20WithdrawalFromRollup, transaction::v1alpha1::{ action::{ BridgeUnlockAction, @@ -41,6 +39,7 @@ use sequencer_client::{ Address, NonceResponse, }; +use telemetry::metrics; use tempfile::NamedTempFile; use tokio::task::JoinHandle; use tracing::{ @@ -76,17 +75,17 @@ 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) + .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() + .set_no_otel(true) + .set_stdout_writer(std::io::sink) + .try_init::(&()) .unwrap(); } }); @@ -112,6 +111,9 @@ pub struct TestBridgeWithdrawer { /// The config used to initialize the bridge withdrawer. pub config: Config, + + /// A handle to the metrics. + pub metrics_handle: metrics::Handle, } impl Drop for TestBridgeWithdrawer { @@ -285,8 +287,15 @@ impl TestBridgeWithdrawerConfig { }; info!(config = serde_json::to_string(&config).unwrap()); + + let (metrics, metrics_handle) = metrics::ConfigBuilder::new() + .set_global_recorder(false) + .build(&()) + .unwrap(); + let metrics = Box::leak(Box::new(metrics)); + let (bridge_withdrawer, bridge_withdrawer_shutdown_handle) = - BridgeWithdrawer::new(config.clone()).unwrap(); + BridgeWithdrawer::new(config.clone(), metrics).unwrap(); let api_address = bridge_withdrawer.local_addr(); let bridge_withdrawer = tokio::task::spawn(bridge_withdrawer.run()); @@ -298,6 +307,7 @@ impl TestBridgeWithdrawerConfig { bridge_withdrawer_shutdown_handle: Some(bridge_withdrawer_shutdown_handle), bridge_withdrawer, config, + metrics_handle, }; test_bridge_withdrawer.mount_startup_responses().await; @@ -423,11 +433,9 @@ pub fn make_bridge_unlock_action(receipt: &TransactionReceipt) -> Action { let inner = BridgeUnlockAction { to: default_sequencer_address(), amount: 1_000_000u128, - memo: serde_json::to_string(&BridgeUnlock { - rollup_block_number: receipt.block_number.unwrap().as_u64(), - rollup_transaction_hash: receipt.transaction_hash.to_string(), - }) - .unwrap(), + rollup_block_number: receipt.block_number.unwrap().as_u64(), + rollup_withdrawal_event_id: receipt.transaction_hash.to_string(), + memo: String::new(), fee_asset: denom, bridge_address: default_bridge_address(), }; @@ -448,7 +456,7 @@ pub fn make_ics20_withdrawal_action(receipt: &TransactionReceipt) -> Action { memo: "nootwashere".to_string(), rollup_return_address: receipt.from.to_string(), rollup_block_number: receipt.block_number.unwrap().as_u64(), - rollup_transaction_hash: receipt.transaction_hash.to_string(), + rollup_withdrawal_event_id: receipt.transaction_hash.to_string(), }) .unwrap(), fee_asset: denom, diff --git a/crates/astria-cli/src/commands/bridge/collect.rs b/crates/astria-cli/src/commands/bridge/collect.rs index c060f7c5f..531107a23 100644 --- a/crates/astria-cli/src/commands/bridge/collect.rs +++ b/crates/astria-cli/src/commands/bridge/collect.rs @@ -202,7 +202,10 @@ async fn block_to_actions( "failed getting actions for block; block hash: `{block_hash}`, block height: \ `{rollup_height}`" ) - })?; + })? + .into_iter() + .collect::>()?; + actions_by_rollup_height.insert(rollup_height, actions) } diff --git a/crates/astria-composer/Cargo.toml b/crates/astria-composer/Cargo.toml index d381d8d03..9dad92be7 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 2a658da7c..c2e4e4601 100644 --- a/crates/astria-composer/src/collectors/geth.rs +++ b/crates/astria-composer/src/collectors/geth.rs @@ -37,7 +37,7 @@ use ethers::{ }, types::Transaction, }; -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 7577bb88b..a4b82f27b 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, }; @@ -120,12 +119,7 @@ impl Composer { /// An error is returned if the composer fails to be initialized. /// See `[from_config]` for its error scenarios. #[instrument(skip_all, err)] - 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(); @@ -166,6 +160,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 0816f723a..c8e125d96 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)] @@ -34,7 +36,7 @@ pub struct Config { /// Path to private key for the sequencer account used for signing transactions pub private_key_file: String, - // The address prefix to use when constructing sequencer addresses using the signing key. + /// The address prefix to use when constructing sequencer addresses using the signing key. pub sequencer_address_prefix: String, /// Sequencer block time in milliseconds @@ -71,13 +73,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 9aa0ea265..bb0e4af53 100644 --- a/crates/astria-composer/src/executor/tests.rs +++ b/crates/astria-composer/src/executor/tests.rs @@ -1,11 +1,19 @@ use std::{ io::Write, + net::{ + IpAddr, + SocketAddr, + }, time::Duration, }; use astria_core::{ generated::protocol::accounts::v1alpha1::NonceResponse, primitive::v1::{ + asset::{ + Denom, + IbcPrefixed, + }, RollupId, ROLLUP_ID_LEN, }, @@ -19,6 +27,7 @@ use prost::{ }; use sequencer_client::SignedTransaction; use serde_json::json; +use telemetry::Metrics as _; use tempfile::NamedTempFile; use tendermint::{ consensus::{ @@ -64,19 +73,40 @@ 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(), + sequencer_address_prefix: 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), + fee_asset: Denom::IbcPrefixed(IbcPrefixed::new([0; 32])), + }; 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(); } }); @@ -294,7 +324,7 @@ async fn full_bundle() { // set up the executor, channel for writing seq actions, and the sequencer mock let (sequencer, 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())); mount_genesis(&sequencer, &cfg.sequencer_chain_id).await; let (executor, executor_handle) = executor::Builder { sequencer_url: cfg.sequencer_url.clone(), @@ -385,7 +415,7 @@ async fn bundle_triggered_by_block_timer() { // set up the executor, channel for writing seq actions, and the sequencer mock let (sequencer, 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())); mount_genesis(&sequencer, &cfg.sequencer_chain_id).await; let (executor, executor_handle) = executor::Builder { sequencer_url: cfg.sequencer_url.clone(), @@ -473,7 +503,7 @@ async fn two_seq_actions_single_bundle() { // set up the executor, channel for writing seq actions, and the sequencer mock let (sequencer, 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())); mount_genesis(&sequencer, &cfg.sequencer_chain_id).await; let (executor, executor_handle) = executor::Builder { sequencer_url: cfg.sequencer_url.clone(), @@ -572,7 +602,7 @@ async fn chain_id_mismatch_returns_error() { // set up sequencer mock let (sequencer, 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())); // mount a status response with an incorrect chain_id mount_genesis(&sequencer, "bad-chain-id").await; diff --git a/crates/astria-composer/src/lib.rs b/crates/astria-composer/src/lib.rs index 4341513c1..cbf59addc 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() @@ -54,4 +54,5 @@ pub(crate) mod utils; 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 803301886..37e163175 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 f400356a1..28aff1dde 100644 --- a/crates/astria-composer/src/metrics.rs +++ b/crates/astria-composer/src/metrics.rs @@ -4,30 +4,30 @@ 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::{ + 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 +40,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) } @@ -174,27 +97,110 @@ 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 telemetry::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 +208,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 +241,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 a5ebe9425..310fcb98b 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 8747680d9..8b564b300 100644 --- a/crates/astria-composer/tests/blackbox/helper/mod.rs +++ b/crates/astria-composer/tests/blackbox/helper/mod.rs @@ -1,16 +1,26 @@ 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, + primitive::v1::{ + asset::{ + Denom, + IbcPrefixed, + }, + RollupId, + }, protocol::{ abci::AbciErrorCode, transaction::v1alpha1::SignedTransaction, @@ -19,6 +29,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 +51,42 @@ 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(), + sequencer_address_prefix: 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), + fee_asset: Denom::IbcPrefixed(IbcPrefixed::new([0; 32])), + }; 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 +98,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. @@ -107,8 +142,15 @@ pub async fn spawn_composer(rollup_ids: &[&str]) -> TestComposer { grpc_addr: "127.0.0.1:0".parse().unwrap(), fee_asset: "nria".parse().unwrap(), }; + + let (metrics, metrics_handle) = metrics::ConfigBuilder::new() + .set_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()); @@ -123,6 +165,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 3c94b521f..717e1edbd 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/celestia/fetch.rs b/crates/astria-conductor/src/celestia/fetch.rs index cf45cf202..cb8eb5f34 100644 --- a/crates/astria-conductor/src/celestia/fetch.rs +++ b/crates/astria-conductor/src/celestia/fetch.rs @@ -52,6 +52,7 @@ impl RawBlobs { celestia_height, sequencer_namespace = %base64(sequencer_namespace.as_ref()), rollup_namespace = %base64(rollup_namespace.as_ref()), + err, ))] pub(super) async fn fetch_new_blobs( client: CelestiaClient, @@ -85,6 +86,7 @@ pub(super) async fn fetch_new_blobs( }) } +#[instrument(skip_all, err)] async fn fetch_blobs_with_retry( client: CelestiaClient, height: u64, diff --git a/crates/astria-conductor/src/celestia/mod.rs b/crates/astria-conductor/src/celestia/mod.rs index 7c209802e..5de1d5b09 100644 --- a/crates/astria-conductor/src/celestia/mod.rs +++ b/crates/astria-conductor/src/celestia/mod.rs @@ -51,6 +51,7 @@ use tracing::{ info_span, instrument, trace, + trace_span, warn, }; @@ -148,7 +149,9 @@ impl Reader { pub(crate) async fn run_until_stopped(mut self) -> eyre::Result<()> { let (executor, sequencer_chain_id) = select!( () = self.shutdown.clone().cancelled_owned() => { - info!("received shutdown signal while waiting for Celestia reader task to initialize"); + info_span!("conductor::celestia::Reader::run_until_stopped").in_scope(|| + info!("received shutdown signal while waiting for Celestia reader task to initialize") + ); return Ok(()); } @@ -163,6 +166,7 @@ impl Reader { .await } + #[instrument(skip_all, err)] async fn initialize( &mut self, ) -> eyre::Result<(executor::Handle, tendermint::chain::Id)> { @@ -303,18 +307,19 @@ impl RunningReader { }) } - #[instrument(skip(self))] async fn run_until_stopped(mut self) -> eyre::Result<()> { - info!( - initial_celestia_height = self.celestia_next_height, - initial_max_celestia_height = self.max_permitted_celestia_height(), - celestia_variance = self.celestia_variance, - rollup_namespace = %base64(&self.rollup_namespace.as_bytes()), - rollup_id = %self.rollup_id, - sequencer_chain_id = %self.sequencer_chain_id, - sequencer_namespace = %base64(&self.sequencer_namespace.as_bytes()), - "starting firm block read loop", - ); + info_span!("conductor::celestia::RunningReader::run_until_stopped").in_scope(|| { + info!( + initial_celestia_height = self.celestia_next_height, + initial_max_celestia_height = self.max_permitted_celestia_height(), + celestia_variance = self.celestia_variance, + rollup_namespace = %base64(&self.rollup_namespace.as_bytes()), + rollup_id = %self.rollup_id, + sequencer_chain_id = %self.sequencer_chain_id, + sequencer_namespace = %base64(&self.sequencer_namespace.as_bytes()), + "starting firm block read loop", + ); + }); let reason = loop { self.schedule_new_blobs(); @@ -329,7 +334,9 @@ impl RunningReader { res = &mut self.enqueued_block, if self.waiting_for_executor_capacity() => { match res { Ok(celestia_height_of_forwarded_block) => { - trace!("submitted enqueued block to executor, resuming normal operation"); + trace_span!("conductor::celestia::RunningReader::run_until_stopped") + .in_scope(|| + trace!("submitted enqueued block to executor, resuming normal operation")); self.advance_reference_celestia_height(celestia_height_of_forwarded_block); } Err(err) => break Err(err).wrap_err("failed sending enqueued block to executor"), @@ -353,18 +360,7 @@ impl RunningReader { } Some(res) = self.latest_heights.next() => { - match res { - Ok(height) => { - info!(height, "observed latest height from Celestia"); - self.record_latest_celestia_height(height); - } - Err(error) => { - warn!( - %error, - "failed fetching latest height from sequencer; waiting until next tick", - ); - } - } + self.handle_latest_height(res); } ); @@ -372,18 +368,26 @@ impl RunningReader { // XXX: explicitly setting the event message (usually implicitly set by tracing) let message = "shutting down"; - match reason { - Ok(reason) => { - info!(reason, message); - Ok(()) + report_exit(reason, message) + } + + #[instrument(skip_all)] + fn handle_latest_height(&mut self, res: eyre::Result) { + match res { + Ok(height) => { + info!(height, "observed latest height from Celestia"); + self.record_latest_celestia_height(height); } - Err(reason) => { - error!(%reason, message); - Err(reason) + Err(error) => { + warn!( + %error, + "failed fetching latest height from sequencer; waiting until next tick", + ); } } } + #[instrument(skip_all)] fn cache_reconstructed_blocks(&mut self, reconstructed: ReconstructedBlocks) { for block in reconstructed.blocks { let block_hash = block.block_hash; @@ -413,6 +417,7 @@ impl RunningReader { is_next_below_head && is_next_in_window && is_capacity_in_task_set } + #[instrument(skip_all)] fn schedule_new_blobs(&mut self) { let mut scheduled = vec![]; while self.can_schedule_blobs() { @@ -444,6 +449,7 @@ impl RunningReader { *reference_height = max(*reference_height, candidate); } + #[instrument(skip_all)] fn forward_block_to_executor(&mut self, block: ReconstructedBlock) -> eyre::Result<()> { let celestia_height = block.celestia_height; match self.executor.try_send_firm_block(block) { @@ -507,6 +513,7 @@ impl FetchConvertVerifyAndReconstruct { celestia_height = self.celestia_height, rollup_namespace = %base64(self.rollup_namespace.as_bytes()), sequencer_namespace = %base64(self.sequencer_namespace.as_bytes()), + err, ))] async fn execute(self) -> eyre::Result { let Self { @@ -592,6 +599,7 @@ impl FetchConvertVerifyAndReconstruct { } } +#[instrument(skip_all, err)] async fn enqueue_block( executor: executor::Handle, block: ReconstructedBlock, @@ -601,6 +609,7 @@ async fn enqueue_block( Ok(celestia_height) } +#[instrument(skip_all, err)] async fn get_sequencer_chain_id(client: SequencerClient) -> eyre::Result { use sequencer_client::Client as _; @@ -633,3 +642,17 @@ async fn get_sequencer_chain_id(client: SequencerClient) -> eyre::Result u64 { reference.saturating_add(variance.saturating_mul(6)) } + +#[instrument(skip_all)] +fn report_exit(exit_reason: eyre::Result<&str>, message: &str) -> eyre::Result<()> { + match exit_reason { + Ok(reason) => { + info!(%reason, message); + Ok(()) + } + Err(reason) => { + error!(%reason, message); + Err(reason) + } + } +} diff --git a/crates/astria-conductor/src/celestia/verify.rs b/crates/astria-conductor/src/celestia/verify.rs index 1f44662da..67b964ed9 100644 --- a/crates/astria-conductor/src/celestia/verify.rs +++ b/crates/astria-conductor/src/celestia/verify.rs @@ -38,6 +38,7 @@ use tracing::{ instrument, warn, Instrument, + Level, }; use tryhard::{ backoff_strategies::BackoffStrategy, @@ -206,6 +207,7 @@ struct VerificationMeta { } impl VerificationMeta { + #[instrument(skip_all, err(level = Level::WARN))] async fn fetch( client: RateLimitedVerificationClient, height: SequencerHeight, @@ -298,6 +300,7 @@ impl BlobVerifier { } } +#[instrument(skip_all, err)] async fn fetch_commit_with_retry( client: SequencerClient, height: SequencerHeight, @@ -332,6 +335,7 @@ async fn fetch_commit_with_retry( }) } +#[instrument(skip_all, err)] async fn fetch_validators_with_retry( client: SequencerClient, prev_height: SequencerHeight, @@ -447,6 +451,7 @@ struct RateLimitedVerificationClient { } impl RateLimitedVerificationClient { + #[instrument(skip_all, err)] async fn get_commit( mut self, height: SequencerHeight, @@ -468,6 +473,7 @@ impl RateLimitedVerificationClient { } } + #[instrument(skip_all, err)] async fn get_validators( mut self, prev_height: SequencerHeight, diff --git a/crates/astria-conductor/src/conductor.rs b/crates/astria-conductor/src/conductor.rs index ff857257f..2fc3ac7b9 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, }; @@ -23,6 +22,7 @@ use tokio_util::{ use tracing::{ error, info, + info_span, instrument, warn, }; @@ -52,6 +52,7 @@ impl Handle { /// /// # Panics /// Panics if called twice. + #[instrument(skip_all, err)] pub async fn shutdown(&mut self) -> Result<(), tokio::task::JoinError> { self.shutdown_token.cancel(); let task = self.task.take().expect("shutdown must not be called twice"); @@ -95,10 +96,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) @@ -173,9 +171,8 @@ impl Conductor { /// /// # Panics /// Panics if it could not install a signal handler. - #[instrument(skip_all)] async fn run_until_stopped(mut self) { - info!("conductor is running"); + info_span!("Conductor::run_until_stopped").in_scope(|| info!("conductor is running")); let exit_reason = select! { biased; @@ -193,10 +190,7 @@ impl Conductor { }; let message = "initiating shutdown"; - match exit_reason { - Ok(reason) => info!(reason, message), - Err(reason) => error!(%reason, message), - } + report_exit(exit_reason, message); self.shutdown().await; } @@ -220,6 +214,7 @@ impl Conductor { /// Waits 25 seconds for all tasks to shut down before aborting them. 25 seconds /// because kubernetes issues SIGKILL 30 seconds after SIGTERM, giving 5 seconds /// to abort the remaining tasks. + #[instrument(skip_all)] async fn shutdown(mut self) { self.shutdown.cancel(); @@ -251,3 +246,11 @@ impl Conductor { info!("shutting down"); } } + +#[instrument(skip_all)] +fn report_exit(exit_reason: eyre::Result<&str>, message: &str) { + match exit_reason { + Ok(reason) => info!(%reason, message), + Err(reason) => error!(%reason, message), + } +} diff --git a/crates/astria-conductor/src/executor/channel.rs b/crates/astria-conductor/src/executor/channel.rs index c4348bd08..450dfb23d 100644 --- a/crates/astria-conductor/src/executor/channel.rs +++ b/crates/astria-conductor/src/executor/channel.rs @@ -20,6 +20,7 @@ use tokio::sync::{ Semaphore, TryAcquireError, }; +use tracing::instrument; /// Creates an mpsc channel for sending soft blocks between asynchronous task. /// @@ -92,6 +93,7 @@ impl Sender { /// Sends a block, waiting until the channel has permits. /// /// Returns an error if the channel is closed. + #[instrument(skip_all, err)] pub(super) async fn send(&self, block: T) -> Result<(), SendError> { let sem = self.sem.upgrade().ok_or(SendError)?; let permit = sem.acquire().await?; @@ -151,6 +153,7 @@ impl Receiver { } /// Receives a block over the channel. + #[instrument(skip_all)] pub(super) async fn recv(&mut self) -> Option { self.chan.recv().await } diff --git a/crates/astria-conductor/src/executor/client.rs b/crates/astria-conductor/src/executor/client.rs index 46d784b4a..6f9f9fcdb 100644 --- a/crates/astria-conductor/src/executor/client.rs +++ b/crates/astria-conductor/src/executor/client.rs @@ -86,7 +86,7 @@ impl Client { } /// Calls remote procedure `astria.execution.v1alpha2.GetGenesisInfo` - #[instrument(skip_all, fields(uri = %self.uri))] + #[instrument(skip_all, fields(uri = %self.uri), err)] pub(crate) async fn get_genesis_info_with_retry(&mut self) -> eyre::Result { let response = tryhard::retry_fn(|| { let mut client = self.inner.clone(); @@ -182,7 +182,7 @@ impl Client { /// /// * `firm` - The firm block /// * `soft` - The soft block - #[instrument(skip_all, fields(uri = %self.uri))] + #[instrument(skip_all, fields(uri = %self.uri), err)] pub(super) async fn update_commitment_state_with_retry( &mut self, commitment_state: CommitmentState, diff --git a/crates/astria-conductor/src/executor/mod.rs b/crates/astria-conductor/src/executor/mod.rs index 2ef56213d..916a951e2 100644 --- a/crates/astria-conductor/src/executor/mod.rs +++ b/crates/astria-conductor/src/executor/mod.rs @@ -32,6 +32,7 @@ use tokio::{ use tokio_util::sync::CancellationToken; use tracing::{ debug, + debug_span, error, info, instrument, @@ -142,6 +143,7 @@ impl Handle { } impl Handle { + #[instrument(skip_all, err)] pub(crate) async fn send_firm_block( self, block: ReconstructedBlock, @@ -162,6 +164,7 @@ impl Handle { Ok(()) } + #[instrument(skip_all, err)] pub(crate) async fn send_soft_block_owned( self, block: FilteredSequencerBlock, @@ -197,6 +200,7 @@ impl Handle { self.state.next_expected_soft_sequencer_height() } + #[instrument(skip_all)] pub(crate) async fn next_expected_soft_height_if_changed( &mut self, ) -> Result { @@ -252,15 +256,13 @@ pub(crate) struct Executor { } impl Executor { - #[instrument(skip_all, err)] pub(crate) async fn run_until_stopped(mut self) -> eyre::Result<()> { select!( () = self.shutdown.clone().cancelled_owned() => { - info!( + return report_exit(Ok( "received shutdown signal while initializing task; \ aborting intialization and exiting" - ); - return Ok(()); + ), ""); } res = self.init() => { res.wrap_err("initialization failed")?; @@ -285,11 +287,11 @@ impl Executor { Some(block) = async { self.firm_blocks.as_mut().unwrap().recv().await }, if self.firm_blocks.is_some() => { - debug!( + debug_span!("conductor::Executor::run_until_stopped").in_scope(||debug!( block.height = %block.sequencer_height(), block.hash = %telemetry::display::base64(&block.block_hash), "received block from celestia reader", - ); + )); if let Err(error) = self.execute_firm(block).await { break Err(error).wrap_err("failed executing firm block"); } @@ -298,11 +300,11 @@ impl Executor { Some(block) = async { self.soft_blocks.as_mut().unwrap().recv().await }, if self.soft_blocks.is_some() && spread_not_too_large => { - debug!( + debug_span!("conductor::Executor::run_until_stopped").in_scope(||debug!( block.height = %block.height(), block.hash = %telemetry::display::base64(&block.block_hash()), "received block from sequencer reader", - ); + )); if let Err(error) = self.execute_soft(block).await { break Err(error).wrap_err("failed executing soft block"); } @@ -312,19 +314,11 @@ impl Executor { // XXX: explicitly setting the message (usually implicitly set by tracing) let message = "shutting down"; - match reason { - Ok(reason) => { - info!(reason, message); - Ok(()) - } - Err(reason) => { - error!(%reason, message); - Err(reason) - } - } + report_exit(reason, message) } /// Runs the init logic that needs to happen before [`Executor`] can enter its main loop. + #[instrument(skip_all, err)] async fn init(&mut self) -> eyre::Result<()> { self.set_initial_node_state() .await @@ -400,6 +394,7 @@ impl Executor { #[instrument(skip_all, fields( block.hash = %telemetry::display::base64(&block.block_hash()), block.height = block.height().value(), + err, ))] async fn execute_soft(&mut self, block: FilteredSequencerBlock) -> eyre::Result<()> { // TODO(https://github.com/astriaorg/astria/issues/624): add retry logic before failing hard. @@ -463,6 +458,7 @@ impl Executor { #[instrument(skip_all, fields( block.hash = %telemetry::display::base64(&block.block_hash), block.height = block.sequencer_height().value(), + err, ))] async fn execute_firm(&mut self, block: ReconstructedBlock) -> eyre::Result<()> { let celestia_height = block.celestia_height; @@ -544,6 +540,7 @@ impl Executor { block.height = block.height.value(), block.num_of_transactions = block.transactions.len(), rollup.parent_hash = %telemetry::display::base64(&parent_hash), + err ))] async fn execute_block( &mut self, @@ -576,7 +573,7 @@ impl Executor { Ok(executed_block) } - #[instrument(skip_all)] + #[instrument(skip_all, err)] async fn set_initial_node_state(&mut self) -> eyre::Result<()> { let genesis_info = { async { @@ -613,7 +610,7 @@ impl Executor { Ok(()) } - #[instrument(skip_all)] + #[instrument(skip_all, err)] async fn update_commitment_state(&mut self, update: Update) -> eyre::Result<()> { use Update::{ OnlyFirm, @@ -675,6 +672,20 @@ impl Executor { } } +#[instrument(skip_all)] +fn report_exit(reason: eyre::Result<&str>, message: &str) -> eyre::Result<()> { + match reason { + Ok(reason) => { + info!(%reason, message); + Ok(()) + } + Err(error) => { + error!(%error, message); + Err(error) + } + } +} + enum Update { OnlyFirm(Block, CelestiaHeight), OnlySoft(Block), diff --git a/crates/astria-conductor/src/executor/state.rs b/crates/astria-conductor/src/executor/state.rs index bf6309834..ccaa49a6b 100644 --- a/crates/astria-conductor/src/executor/state.rs +++ b/crates/astria-conductor/src/executor/state.rs @@ -20,6 +20,7 @@ use tokio::sync::watch::{ self, error::RecvError, }; +use tracing::instrument; pub(super) fn channel() -> (StateSender, StateReceiver) { let (tx, rx) = watch::channel(None); @@ -50,6 +51,7 @@ pub(super) struct StateReceiver { } impl StateReceiver { + #[instrument(skip_all, err)] pub(super) async fn wait_for_init(&mut self) -> eyre::Result<()> { self.inner .wait_for(Option::is_some) @@ -82,6 +84,7 @@ impl StateReceiver { ) } + #[instrument(skip_all)] pub(crate) async fn next_expected_soft_height_if_changed( &mut self, ) -> Result { diff --git a/crates/astria-conductor/src/lib.rs b/crates/astria-conductor/src/lib.rs index ff40fef07..eb9e88c0d 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 7468e0ee2..473219436 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 d992611e2..208bc2bbc 100644 --- a/crates/astria-conductor/src/metrics.rs +++ b/crates/astria-conductor/src/metrics.rs @@ -1,19 +1,15 @@ -use metrics::{ - counter, - describe_counter, - describe_histogram, - histogram, - Counter, - Histogram, - Unit, +use telemetry::{ + metric_names, + metrics::{ + 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,109 +23,12 @@ 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)] - 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) { @@ -137,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) { @@ -181,13 +72,99 @@ 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 telemetry::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!(pub const METRICS_NAMES: +metric_names!(const METRICS_NAMES: BLOBS_PER_CELESTIA_FETCH, CELESTIA_BLOB_FETCH_ERROR_COUNT, DECODED_ITEMS_PER_CELESTIA_FETCH, @@ -201,8 +178,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 +186,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/src/sequencer/mod.rs b/crates/astria-conductor/src/sequencer/mod.rs index 535c61d4f..719fb7205 100644 --- a/crates/astria-conductor/src/sequencer/mod.rs +++ b/crates/astria-conductor/src/sequencer/mod.rs @@ -28,10 +28,13 @@ use tokio::select; use tokio_util::sync::CancellationToken; use tracing::{ debug, + debug_span, error, info, + instrument, trace, warn, + warn_span, }; use crate::{ @@ -82,8 +85,7 @@ impl Reader { pub(crate) async fn run_until_stopped(mut self) -> eyre::Result<()> { let executor = select!( () = self.shutdown.clone().cancelled_owned() => { - info!("received shutdown signal while waiting for Sequencer reader task to initialize"); - return Ok(()); + return report_exit(Ok("received shutdown signal while waiting for Sequencer reader task to initialize"), ""); } res = self.initialize() => { res? @@ -95,6 +97,7 @@ impl Reader { .await } + #[instrument(skip_all, err)] async fn initialize(&mut self) -> eyre::Result> { self.executor .wait_for_init() @@ -174,16 +177,7 @@ impl RunningReader { // XXX: explicitly setting the message (usually implicitly set by tracing) let message = "shutting down"; - match stop_reason { - Ok(stop_reason) => { - info!(stop_reason, message); - Ok(()) - } - Err(stop_reason) => { - error!(%stop_reason, message); - Err(stop_reason) - } - } + report_exit(stop_reason, message) } async fn run_loop(&mut self) -> eyre::Result<&'static str> { @@ -200,7 +194,9 @@ impl RunningReader { // Process block execution which was enqueued due to executor channel being full. res = &mut self.enqueued_block, if !self.enqueued_block.is_terminated() => { res.wrap_err("failed sending enqueued block to executor")?; - debug!("submitted enqueued block to executor, resuming normal operation"); + debug_span!("conductor::sequencer::RunningReader::run_loop").in_scope(|| + debug!("submitted enqueued block to executor, resuming normal operation") + ); } // Skip heights that executor has already executed (e.g. firm blocks from Celestia) @@ -220,29 +216,37 @@ impl RunningReader { // otherwise recover from a failed block fetch. let block = block.wrap_err("the stream of new blocks returned a catastrophic error")?; if let Err(error) = self.block_cache.insert(block) { - warn!(%error, "failed pushing block into sequential cache, dropping it"); + warn_span!("conductor::sequencer::RunningReader::run_loop").in_scope(|| + warn!(%error, "failed pushing block into sequential cache, dropping it") + ); } } // Record the latest height of the Sequencer network, allowing `blocks_from_heights` to progress. Some(res) = self.latest_height_stream.next() => { - match res { - Ok(height) => { - debug!(%height, "received latest height from sequencer"); - self.blocks_from_heights.set_latest_observed_height_if_greater(height); - } - Err(error) => { - warn!( - error = %Report::new(error), - "failed fetching latest height from sequencer; waiting until next tick", - ); - } - } + self.handle_latest_height(res); } } } } + #[instrument(skip_all)] + fn handle_latest_height(&mut self, res: Result) { + match res { + Ok(height) => { + debug!(%height, "received latest height from sequencer"); + self.blocks_from_heights + .set_latest_observed_height_if_greater(height); + } + Err(error) => { + warn!( + error = %Report::new(error), + "failed fetching latest height from sequencer; waiting until next tick", + ); + } + } + } + /// Sends `block` to the executor task. /// /// Enqueues the block is the channel to the executor is full, sending it once @@ -295,3 +299,17 @@ impl RunningReader { self.block_cache.drop_obsolete(next_height); } } + +#[instrument(skip_all)] +fn report_exit(reason: eyre::Result<&str>, message: &str) -> eyre::Result<()> { + match reason { + Ok(reason) => { + info!(%reason, message); + Ok(()) + } + Err(reason) => { + error!(%reason, message); + Err(reason) + } + } +} diff --git a/crates/astria-conductor/tests/blackbox/helpers/mod.rs b/crates/astria-conductor/tests/blackbox/helpers/mod.rs index ff07a3b0f..3dfdf28c4 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; @@ -54,19 +56,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(); } }); @@ -93,8 +95,14 @@ pub async fn spawn_conductor(execution_commit_level: CommitLevel) -> TestConduct ..make_config() }; + let (metrics, metrics_handle) = metrics::ConfigBuilder::new() + .set_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() }; @@ -102,6 +110,7 @@ pub async fn spawn_conductor(execution_commit_level: CommitLevel) -> TestConduct conductor, mock_grpc, mock_http, + metrics_handle, } } @@ -109,6 +118,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-core/src/generated/astria.protocol.memos.v1alpha1.rs b/crates/astria-core/src/generated/astria.protocol.memos.v1alpha1.rs index bd066f2ab..ead1e0919 100644 --- a/crates/astria-core/src/generated/astria.protocol.memos.v1alpha1.rs +++ b/crates/astria-core/src/generated/astria.protocol.memos.v1alpha1.rs @@ -1,25 +1,3 @@ -#[allow(clippy::derive_partial_eq_without_eq)] -#[derive(Clone, PartialEq, ::prost::Message)] -pub struct BridgeUnlock { - /// The block number on the rollup that triggered the transaction underlying - /// this bridge unlock memo. - #[prost(uint64, tag = "1")] - pub rollup_block_number: u64, - /// The hash of the original rollup transaction that triggered a bridge unlock - /// and that is underlying this bridge unlock memo. - /// - /// This field is of type `string` so that it can be formatted in the preferred - /// format of the rollup when targeting plain text encoding. - #[prost(string, tag = "2")] - pub rollup_transaction_hash: ::prost::alloc::string::String, -} -impl ::prost::Name for BridgeUnlock { - const NAME: &'static str = "BridgeUnlock"; - const PACKAGE: &'static str = "astria.protocol.memos.v1alpha1"; - fn full_name() -> ::prost::alloc::string::String { - ::prost::alloc::format!("astria.protocol.memos.v1alpha1.{}", Self::NAME) - } -} /// Memo for an ICS20 withdrawal from the rollup which is sent to /// an external IBC-enabled chain. #[allow(clippy::derive_partial_eq_without_eq)] @@ -29,13 +7,14 @@ pub struct Ics20WithdrawalFromRollup { /// this ics20 withdrawal memo. #[prost(uint64, tag = "1")] pub rollup_block_number: u64, - /// The hash of the original rollup transaction that triggered this ics20 - /// withdrawal and that is underlying this bridge unlock memo. + /// An identifier of the original rollup withdrawal event that triggered this ics20 + /// withdrawal and that is underlying this bridge unlock memo. For general EVM + /// this is typically a transaction hash. /// /// This field is of type `string` so that it can be formatted in the preferred /// format of the rollup when targeting plain text encoding. #[prost(string, tag = "2")] - pub rollup_transaction_hash: ::prost::alloc::string::String, + pub rollup_withdrawal_event_id: ::prost::alloc::string::String, /// The return address on the rollup to which funds should returned in case of /// failure. This field exists so that the rollup can identify which account /// the returned funds originated from. diff --git a/crates/astria-core/src/generated/astria.protocol.memos.v1alpha1.serde.rs b/crates/astria-core/src/generated/astria.protocol.memos.v1alpha1.serde.rs index 315161ec8..a7efd8ee1 100644 --- a/crates/astria-core/src/generated/astria.protocol.memos.v1alpha1.serde.rs +++ b/crates/astria-core/src/generated/astria.protocol.memos.v1alpha1.serde.rs @@ -1,116 +1,3 @@ -impl serde::Serialize for BridgeUnlock { - #[allow(deprecated)] - fn serialize(&self, serializer: S) -> std::result::Result - where - S: serde::Serializer, - { - use serde::ser::SerializeStruct; - let mut len = 0; - if self.rollup_block_number != 0 { - len += 1; - } - if !self.rollup_transaction_hash.is_empty() { - len += 1; - } - let mut struct_ser = serializer.serialize_struct("astria.protocol.memos.v1alpha1.BridgeUnlock", len)?; - if self.rollup_block_number != 0 { - #[allow(clippy::needless_borrow)] - struct_ser.serialize_field("rollupBlockNumber", ToString::to_string(&self.rollup_block_number).as_str())?; - } - if !self.rollup_transaction_hash.is_empty() { - struct_ser.serialize_field("rollupTransactionHash", &self.rollup_transaction_hash)?; - } - struct_ser.end() - } -} -impl<'de> serde::Deserialize<'de> for BridgeUnlock { - #[allow(deprecated)] - fn deserialize(deserializer: D) -> std::result::Result - where - D: serde::Deserializer<'de>, - { - const FIELDS: &[&str] = &[ - "rollup_block_number", - "rollupBlockNumber", - "rollup_transaction_hash", - "rollupTransactionHash", - ]; - - #[allow(clippy::enum_variant_names)] - enum GeneratedField { - RollupBlockNumber, - RollupTransactionHash, - } - impl<'de> serde::Deserialize<'de> for GeneratedField { - fn deserialize(deserializer: D) -> std::result::Result - where - D: serde::Deserializer<'de>, - { - struct GeneratedVisitor; - - impl<'de> serde::de::Visitor<'de> for GeneratedVisitor { - type Value = GeneratedField; - - fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(formatter, "expected one of: {:?}", &FIELDS) - } - - #[allow(unused_variables)] - fn visit_str(self, value: &str) -> std::result::Result - where - E: serde::de::Error, - { - match value { - "rollupBlockNumber" | "rollup_block_number" => Ok(GeneratedField::RollupBlockNumber), - "rollupTransactionHash" | "rollup_transaction_hash" => Ok(GeneratedField::RollupTransactionHash), - _ => Err(serde::de::Error::unknown_field(value, FIELDS)), - } - } - } - deserializer.deserialize_identifier(GeneratedVisitor) - } - } - struct GeneratedVisitor; - impl<'de> serde::de::Visitor<'de> for GeneratedVisitor { - type Value = BridgeUnlock; - - fn expecting(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - formatter.write_str("struct astria.protocol.memos.v1alpha1.BridgeUnlock") - } - - fn visit_map(self, mut map_: V) -> std::result::Result - where - V: serde::de::MapAccess<'de>, - { - let mut rollup_block_number__ = None; - let mut rollup_transaction_hash__ = None; - while let Some(k) = map_.next_key()? { - match k { - GeneratedField::RollupBlockNumber => { - if rollup_block_number__.is_some() { - return Err(serde::de::Error::duplicate_field("rollupBlockNumber")); - } - rollup_block_number__ = - Some(map_.next_value::<::pbjson::private::NumberDeserialize<_>>()?.0) - ; - } - GeneratedField::RollupTransactionHash => { - if rollup_transaction_hash__.is_some() { - return Err(serde::de::Error::duplicate_field("rollupTransactionHash")); - } - rollup_transaction_hash__ = Some(map_.next_value()?); - } - } - } - Ok(BridgeUnlock { - rollup_block_number: rollup_block_number__.unwrap_or_default(), - rollup_transaction_hash: rollup_transaction_hash__.unwrap_or_default(), - }) - } - } - deserializer.deserialize_struct("astria.protocol.memos.v1alpha1.BridgeUnlock", FIELDS, GeneratedVisitor) - } -} impl serde::Serialize for Ics20TransferDeposit { #[allow(deprecated)] fn serialize(&self, serializer: S) -> std::result::Result @@ -214,7 +101,7 @@ impl serde::Serialize for Ics20WithdrawalFromRollup { if self.rollup_block_number != 0 { len += 1; } - if !self.rollup_transaction_hash.is_empty() { + if !self.rollup_withdrawal_event_id.is_empty() { len += 1; } if !self.rollup_return_address.is_empty() { @@ -228,8 +115,8 @@ impl serde::Serialize for Ics20WithdrawalFromRollup { #[allow(clippy::needless_borrow)] struct_ser.serialize_field("rollupBlockNumber", ToString::to_string(&self.rollup_block_number).as_str())?; } - if !self.rollup_transaction_hash.is_empty() { - struct_ser.serialize_field("rollupTransactionHash", &self.rollup_transaction_hash)?; + if !self.rollup_withdrawal_event_id.is_empty() { + struct_ser.serialize_field("rollupWithdrawalEventId", &self.rollup_withdrawal_event_id)?; } if !self.rollup_return_address.is_empty() { struct_ser.serialize_field("rollupReturnAddress", &self.rollup_return_address)?; @@ -249,8 +136,8 @@ impl<'de> serde::Deserialize<'de> for Ics20WithdrawalFromRollup { const FIELDS: &[&str] = &[ "rollup_block_number", "rollupBlockNumber", - "rollup_transaction_hash", - "rollupTransactionHash", + "rollup_withdrawal_event_id", + "rollupWithdrawalEventId", "rollup_return_address", "rollupReturnAddress", "memo", @@ -259,7 +146,7 @@ impl<'de> serde::Deserialize<'de> for Ics20WithdrawalFromRollup { #[allow(clippy::enum_variant_names)] enum GeneratedField { RollupBlockNumber, - RollupTransactionHash, + RollupWithdrawalEventId, RollupReturnAddress, Memo, } @@ -284,7 +171,7 @@ impl<'de> serde::Deserialize<'de> for Ics20WithdrawalFromRollup { { match value { "rollupBlockNumber" | "rollup_block_number" => Ok(GeneratedField::RollupBlockNumber), - "rollupTransactionHash" | "rollup_transaction_hash" => Ok(GeneratedField::RollupTransactionHash), + "rollupWithdrawalEventId" | "rollup_withdrawal_event_id" => Ok(GeneratedField::RollupWithdrawalEventId), "rollupReturnAddress" | "rollup_return_address" => Ok(GeneratedField::RollupReturnAddress), "memo" => Ok(GeneratedField::Memo), _ => Err(serde::de::Error::unknown_field(value, FIELDS)), @@ -307,7 +194,7 @@ impl<'de> serde::Deserialize<'de> for Ics20WithdrawalFromRollup { V: serde::de::MapAccess<'de>, { let mut rollup_block_number__ = None; - let mut rollup_transaction_hash__ = None; + let mut rollup_withdrawal_event_id__ = None; let mut rollup_return_address__ = None; let mut memo__ = None; while let Some(k) = map_.next_key()? { @@ -320,11 +207,11 @@ impl<'de> serde::Deserialize<'de> for Ics20WithdrawalFromRollup { Some(map_.next_value::<::pbjson::private::NumberDeserialize<_>>()?.0) ; } - GeneratedField::RollupTransactionHash => { - if rollup_transaction_hash__.is_some() { - return Err(serde::de::Error::duplicate_field("rollupTransactionHash")); + GeneratedField::RollupWithdrawalEventId => { + if rollup_withdrawal_event_id__.is_some() { + return Err(serde::de::Error::duplicate_field("rollupWithdrawalEventId")); } - rollup_transaction_hash__ = Some(map_.next_value()?); + rollup_withdrawal_event_id__ = Some(map_.next_value()?); } GeneratedField::RollupReturnAddress => { if rollup_return_address__.is_some() { @@ -342,7 +229,7 @@ impl<'de> serde::Deserialize<'de> for Ics20WithdrawalFromRollup { } Ok(Ics20WithdrawalFromRollup { rollup_block_number: rollup_block_number__.unwrap_or_default(), - rollup_transaction_hash: rollup_transaction_hash__.unwrap_or_default(), + rollup_withdrawal_event_id: rollup_withdrawal_event_id__.unwrap_or_default(), rollup_return_address: rollup_return_address__.unwrap_or_default(), memo: memo__.unwrap_or_default(), }) diff --git a/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs b/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs index fd8f2cf0a..a65b8a8b2 100644 --- a/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs +++ b/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs @@ -386,7 +386,8 @@ pub struct BridgeUnlockAction { /// the asset used to pay the transaction fee #[prost(string, tag = "3")] pub fee_asset: ::prost::alloc::string::String, - /// memo for double spend prevention + /// The memo field can be used to provide unique identifying additional + /// information about the bridge unlock transaction. #[prost(string, tag = "4")] pub memo: ::prost::alloc::string::String, /// the address of the bridge account to transfer from @@ -394,6 +395,19 @@ pub struct BridgeUnlockAction { pub bridge_address: ::core::option::Option< super::super::super::primitive::v1::Address, >, + /// The block number on the rollup that triggered the transaction underlying + /// this bridge unlock memo. + #[prost(uint64, tag = "6")] + pub rollup_block_number: u64, + /// An identifier of the original rollup event, such as a transaction hash which + /// triggered a bridge unlock and is underlying event that led to this bridge + /// unlock. This can be utilized for tracing from the bridge back to + /// distinct rollup events. + /// + /// This field is of type `string` so that it can be formatted in the preferred + /// format of the rollup when targeting plain text encoding. + #[prost(string, tag = "7")] + pub rollup_withdrawal_event_id: ::prost::alloc::string::String, } impl ::prost::Name for BridgeUnlockAction { const NAME: &'static str = "BridgeUnlockAction"; diff --git a/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.serde.rs b/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.serde.rs index 9c89346b6..f6bad8ea5 100644 --- a/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.serde.rs +++ b/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.serde.rs @@ -593,6 +593,12 @@ impl serde::Serialize for BridgeUnlockAction { if self.bridge_address.is_some() { len += 1; } + if self.rollup_block_number != 0 { + len += 1; + } + if !self.rollup_withdrawal_event_id.is_empty() { + len += 1; + } let mut struct_ser = serializer.serialize_struct("astria.protocol.transactions.v1alpha1.BridgeUnlockAction", len)?; if let Some(v) = self.to.as_ref() { struct_ser.serialize_field("to", v)?; @@ -609,6 +615,13 @@ impl serde::Serialize for BridgeUnlockAction { if let Some(v) = self.bridge_address.as_ref() { struct_ser.serialize_field("bridgeAddress", v)?; } + if self.rollup_block_number != 0 { + #[allow(clippy::needless_borrow)] + struct_ser.serialize_field("rollupBlockNumber", ToString::to_string(&self.rollup_block_number).as_str())?; + } + if !self.rollup_withdrawal_event_id.is_empty() { + struct_ser.serialize_field("rollupWithdrawalEventId", &self.rollup_withdrawal_event_id)?; + } struct_ser.end() } } @@ -626,6 +639,10 @@ impl<'de> serde::Deserialize<'de> for BridgeUnlockAction { "memo", "bridge_address", "bridgeAddress", + "rollup_block_number", + "rollupBlockNumber", + "rollup_withdrawal_event_id", + "rollupWithdrawalEventId", ]; #[allow(clippy::enum_variant_names)] @@ -635,6 +652,8 @@ impl<'de> serde::Deserialize<'de> for BridgeUnlockAction { FeeAsset, Memo, BridgeAddress, + RollupBlockNumber, + RollupWithdrawalEventId, } impl<'de> serde::Deserialize<'de> for GeneratedField { fn deserialize(deserializer: D) -> std::result::Result @@ -661,6 +680,8 @@ impl<'de> serde::Deserialize<'de> for BridgeUnlockAction { "feeAsset" | "fee_asset" => Ok(GeneratedField::FeeAsset), "memo" => Ok(GeneratedField::Memo), "bridgeAddress" | "bridge_address" => Ok(GeneratedField::BridgeAddress), + "rollupBlockNumber" | "rollup_block_number" => Ok(GeneratedField::RollupBlockNumber), + "rollupWithdrawalEventId" | "rollup_withdrawal_event_id" => Ok(GeneratedField::RollupWithdrawalEventId), _ => Err(serde::de::Error::unknown_field(value, FIELDS)), } } @@ -685,6 +706,8 @@ impl<'de> serde::Deserialize<'de> for BridgeUnlockAction { let mut fee_asset__ = None; let mut memo__ = None; let mut bridge_address__ = None; + let mut rollup_block_number__ = None; + let mut rollup_withdrawal_event_id__ = None; while let Some(k) = map_.next_key()? { match k { GeneratedField::To => { @@ -717,6 +740,20 @@ impl<'de> serde::Deserialize<'de> for BridgeUnlockAction { } bridge_address__ = map_.next_value()?; } + GeneratedField::RollupBlockNumber => { + if rollup_block_number__.is_some() { + return Err(serde::de::Error::duplicate_field("rollupBlockNumber")); + } + rollup_block_number__ = + Some(map_.next_value::<::pbjson::private::NumberDeserialize<_>>()?.0) + ; + } + GeneratedField::RollupWithdrawalEventId => { + if rollup_withdrawal_event_id__.is_some() { + return Err(serde::de::Error::duplicate_field("rollupWithdrawalEventId")); + } + rollup_withdrawal_event_id__ = Some(map_.next_value()?); + } } } Ok(BridgeUnlockAction { @@ -725,6 +762,8 @@ impl<'de> serde::Deserialize<'de> for BridgeUnlockAction { fee_asset: fee_asset__.unwrap_or_default(), memo: memo__.unwrap_or_default(), bridge_address: bridge_address__, + rollup_block_number: rollup_block_number__.unwrap_or_default(), + rollup_withdrawal_event_id: rollup_withdrawal_event_id__.unwrap_or_default(), }) } } diff --git a/crates/astria-core/src/protocol/memos/snapshots/astria_core__protocol__memos__v1alpha1__test__bridge_unlock_memo_snapshot.snap b/crates/astria-core/src/protocol/memos/snapshots/astria_core__protocol__memos__v1alpha1__test__bridge_unlock_memo_snapshot.snap deleted file mode 100644 index 03fd3880e..000000000 --- a/crates/astria-core/src/protocol/memos/snapshots/astria_core__protocol__memos__v1alpha1__test__bridge_unlock_memo_snapshot.snap +++ /dev/null @@ -1,8 +0,0 @@ ---- -source: crates/astria-core/src/protocol/memos/v1alpha1.rs -expression: memo ---- -{ - "rollupBlockNumber": "42", - "rollupTransactionHash": "a-rollup-defined-hash" -} diff --git a/crates/astria-core/src/protocol/memos/snapshots/astria_core__protocol__memos__v1alpha1__test__ics20_withdrawal_from_rollup_memo_snapshot.snap b/crates/astria-core/src/protocol/memos/snapshots/astria_core__protocol__memos__v1alpha1__test__ics20_withdrawal_from_rollup_memo_snapshot.snap index 8fdb7038e..0f7388fcd 100644 --- a/crates/astria-core/src/protocol/memos/snapshots/astria_core__protocol__memos__v1alpha1__test__ics20_withdrawal_from_rollup_memo_snapshot.snap +++ b/crates/astria-core/src/protocol/memos/snapshots/astria_core__protocol__memos__v1alpha1__test__ics20_withdrawal_from_rollup_memo_snapshot.snap @@ -4,7 +4,7 @@ expression: memo --- { "rollupBlockNumber": "1", - "rollupTransactionHash": "a-rollup-defined-hash", + "rollupWithdrawalEventId": "a-rollup-defined-hash", "rollupReturnAddress": "a-rollup-defined-address", "memo": "hello" } diff --git a/crates/astria-core/src/protocol/memos/v1alpha1.rs b/crates/astria-core/src/protocol/memos/v1alpha1.rs index 6de9472aa..9bc1d794a 100644 --- a/crates/astria-core/src/protocol/memos/v1alpha1.rs +++ b/crates/astria-core/src/protocol/memos/v1alpha1.rs @@ -1,5 +1,4 @@ pub use crate::generated::protocol::memos::v1alpha1::{ - BridgeUnlock, Ics20TransferDeposit, Ics20WithdrawalFromRollup, }; @@ -8,22 +7,12 @@ pub use crate::generated::protocol::memos::v1alpha1::{ mod test { use super::*; - #[test] - fn bridge_unlock_memo_snapshot() { - let memo = BridgeUnlock { - rollup_block_number: 42, - rollup_transaction_hash: "a-rollup-defined-hash".to_string(), - }; - - insta::assert_json_snapshot!(memo); - } - #[test] fn ics20_withdrawal_from_rollup_memo_snapshot() { let memo = Ics20WithdrawalFromRollup { rollup_block_number: 1, rollup_return_address: "a-rollup-defined-address".to_string(), - rollup_transaction_hash: "a-rollup-defined-hash".to_string(), + rollup_withdrawal_event_id: "a-rollup-defined-hash".to_string(), memo: "hello".to_string(), }; diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs index 6fb4f3d2c..fe23ffd23 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs @@ -1544,10 +1544,14 @@ pub struct BridgeUnlockAction { pub amount: u128, // asset to use for fee payment. pub fee_asset: asset::Denom, - // memo for double spend protection. - pub memo: String, // the address of the bridge account to transfer from. pub bridge_address: Address, + // A field for users to additional identifying information + pub memo: String, + // The block number of the rollup block containing the withdrawal event. + pub rollup_block_number: u64, + // The identifier of the withdrawal event in the rollup block. + pub rollup_withdrawal_event_id: String, } impl Protobuf for BridgeUnlockAction { @@ -1562,6 +1566,8 @@ impl Protobuf for BridgeUnlockAction { fee_asset: self.fee_asset.to_string(), memo: self.memo, bridge_address: Some(self.bridge_address.into_raw()), + rollup_block_number: self.rollup_block_number, + rollup_withdrawal_event_id: self.rollup_withdrawal_event_id, } } @@ -1573,6 +1579,8 @@ impl Protobuf for BridgeUnlockAction { fee_asset: self.fee_asset.to_string(), memo: self.memo.clone(), bridge_address: Some(self.bridge_address.to_raw()), + rollup_block_number: self.rollup_block_number, + rollup_withdrawal_event_id: self.rollup_withdrawal_event_id.clone(), } } @@ -1592,6 +1600,8 @@ impl Protobuf for BridgeUnlockAction { fee_asset, memo, bridge_address, + rollup_block_number, + rollup_withdrawal_event_id, } = proto; let to = to .ok_or_else(|| BridgeUnlockActionError::field_not_set("to")) @@ -1612,6 +1622,8 @@ impl Protobuf for BridgeUnlockAction { fee_asset, memo, bridge_address, + rollup_block_number, + rollup_withdrawal_event_id, }) } diff --git a/crates/astria-sequencer-relayer/Cargo.toml b/crates/astria-sequencer-relayer/Cargo.toml index 5b4e97f85..43f5f9f85 100644 --- a/crates/astria-sequencer-relayer/Cargo.toml +++ b/crates/astria-sequencer-relayer/Cargo.toml @@ -28,7 +28,6 @@ humantime = { workspace = true } humantime-serde = "1.1.1" 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 52d922708..67ec83894 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 fe85558de..a072bf06f 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()); tokio::select!( diff --git a/crates/astria-sequencer-relayer/src/metrics.rs b/crates/astria-sequencer-relayer/src/metrics.rs index 1ab32eb3a..5665fed4e 100644 --- a/crates/astria-sequencer-relayer/src/metrics.rs +++ b/crates/astria-sequencer-relayer/src/metrics.rs @@ -1,20 +1,16 @@ use std::time::Duration; -use metrics::{ - counter, - describe_counter, - describe_gauge, - describe_histogram, - gauge, - histogram, - Counter, - Gauge, - Histogram, - Unit, +use telemetry::{ + metric_names, + metrics::{ + 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 +26,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); } @@ -148,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) { @@ -190,7 +75,120 @@ impl Metrics { } } -metric_names!(pub const METRICS_NAMES: +impl telemetry::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 9bead7e0c..c8dc9d354 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 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 d04b7c159..a8c63de35 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, }; @@ -48,10 +47,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 15aa52fa9..041e6a488 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 @@ -21,6 +21,7 @@ use astria_core::{ use astria_grpc_mock::MockGuard as GrpcMockGuard; use astria_sequencer_relayer::{ config::Config, + Metrics, SequencerRelayer, ShutdownHandle, }; @@ -30,6 +31,7 @@ use itertools::Itertools; use once_cell::sync::Lazy; use serde::Deserialize; use serde_json::json; +use telemetry::metrics; use tempfile::NamedTempFile; use tendermint_config::PrivValidatorKey; use tendermint_rpc::{ @@ -127,19 +129,19 @@ static TELEMETRY: Lazy<()> = Lazy::new(|| { let filter_directives = std::env::var("RUST_LOG") .unwrap_or_else(|_| "astria_sequencer_relayer=trace,blackbox=trace,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(); } }); @@ -175,6 +177,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 { @@ -720,14 +723,20 @@ 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, submission_state_path: submission_state_file.path().to_owned(), }; + let (metrics, metrics_handle) = metrics::ConfigBuilder::new() + .set_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()); @@ -743,6 +752,7 @@ impl TestSequencerRelayerConfig { submission_state_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 1cb3716b7..4598c6a82 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 5f58d6645..5efcf0af6 100644 --- a/crates/astria-sequencer/Cargo.toml +++ b/crates/astria-sequencer/Cargo.toml @@ -42,7 +42,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/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap index ce51df7d2..887dcec2b 100644 --- a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap +++ b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap @@ -3,36 +3,36 @@ source: crates/astria-sequencer/src/app/tests_breaking_changes.rs expression: app.app_hash.as_bytes() --- [ - 126, - 205, - 191, - 0, + 241, + 61, + 69, + 211, 65, - 191, - 147, - 26, - 88, - 230, - 39, - 60, - 192, - 5, - 177, - 11, - 248, - 83, - 115, - 225, - 203, - 17, - 58, - 245, - 71, - 172, - 232, + 119, + 7, + 14, + 200, + 183, 75, - 203, - 163, - 32, - 75 + 232, + 68, + 126, + 56, + 5, + 8, + 220, + 199, + 173, + 193, + 135, + 169, + 26, + 36, + 119, + 144, + 174, + 76, + 108, + 221, + 225 ] diff --git a/crates/astria-sequencer/src/app/test_utils.rs b/crates/astria-sequencer/src/app/test_utils.rs index 05ffb9b7e..63bbaf885 100644 --- a/crates/astria-sequencer/src/app/test_utils.rs +++ b/crates/astria-sequencer/src/app/test_utils.rs @@ -23,6 +23,7 @@ use astria_core::{ }; use bytes::Bytes; use cnidarium::Storage; +use telemetry::Metrics as _; use crate::{ app::App, @@ -134,7 +135,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(self::genesis_state); diff --git a/crates/astria-sequencer/src/app/tests_breaking_changes.rs b/crates/astria-sequencer/src/app/tests_breaking_changes.rs index 6909cc833..f18739a5e 100644 --- a/crates/astria-sequencer/src/app/tests_breaking_changes.rs +++ b/crates/astria-sequencer/src/app/tests_breaking_changes.rs @@ -271,8 +271,10 @@ async fn app_execute_transaction_with_every_action_snapshot() { to: bob_address, amount: 10, fee_asset: nria().into(), - memo: "{}".into(), + memo: String::new(), bridge_address: astria_address(&bridge.address_bytes()), + rollup_block_number: 1, + rollup_withdrawal_event_id: "a-rollup-defined-hash".to_string(), } .into(), BridgeSudoChangeAction { diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index c433c48fc..2c60e06f8 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -1042,6 +1042,8 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { fee_asset: nria().into(), memo: "{ \"msg\": \"lilywashere\" }".into(), bridge_address, + rollup_block_number: 1, + rollup_withdrawal_event_id: "id-from-rollup".to_string(), }; let tx = UnsignedTransaction { diff --git a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs index a49e90c31..615f77b9e 100644 --- a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs @@ -17,17 +17,35 @@ use crate::{ }, address::StateReadExt as _, app::ActionHandler, - bridge::StateReadExt as _, + bridge::{ + StateReadExt as _, + StateWriteExt as _, + }, transaction::StateReadExt as _, }; #[async_trait::async_trait] impl ActionHandler for BridgeUnlockAction { + // TODO(https://github.com/astriaorg/astria/issues/1430): move checks to the `BridgeUnlock` parsing. async fn check_stateless(&self) -> Result<()> { + ensure!(self.amount > 0, "amount must be greater than zero",); + ensure!(self.memo.len() <= 64, "memo must not be more than 64 bytes"); + ensure!( + !self.rollup_withdrawal_event_id.is_empty(), + "rollup withdrawal event id must be non-empty", + ); + ensure!( + self.rollup_withdrawal_event_id.len() <= 64, + "rollup withdrawal event id must not be more than 64 bytes", + ); + ensure!( + self.rollup_block_number > 0, + "rollup block number must be greater than zero", + ); Ok(()) } - async fn check_and_execute(&self, state: S) -> Result<()> { + async fn check_and_execute(&self, mut state: S) -> Result<()> { let from = state .get_current_source() .expect("transaction source must be present in state when executing an action") @@ -68,6 +86,14 @@ impl ActionHandler for BridgeUnlockAction { }; check_transfer(&transfer_action, self.bridge_address, &state).await?; + state + .check_and_set_withdrawal_event_block_for_bridge_account( + self.bridge_address, + &self.rollup_withdrawal_event_id, + self.rollup_block_number, + ) + .await + .context("withdrawal event already processed")?; execute_transfer(&transfer_action, self.bridge_address, state).await?; Ok(()) @@ -130,8 +156,10 @@ mod tests { to: to_address, amount: transfer_amount, fee_asset: asset.clone(), - memo: "{}".into(), + memo: String::new(), bridge_address, + rollup_block_number: 1, + rollup_withdrawal_event_id: "a-rollup-defined-hash".to_string(), }; // invalid sender, doesn't match action's `from`, should fail @@ -167,8 +195,10 @@ mod tests { to: to_address, amount: transfer_amount, fee_asset: asset, - memo: "{}".into(), + memo: String::new(), bridge_address, + rollup_block_number: 1, + rollup_withdrawal_event_id: "a-rollup-defined-hash".to_string(), }; // invalid sender, doesn't match action's bridge account's withdrawer, should fail @@ -179,7 +209,7 @@ mod tests { } #[tokio::test] - async fn execute_with_bridge_address_unset() { + async fn execute_with_bridge_address_set() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); @@ -209,8 +239,10 @@ mod tests { to: to_address, amount: transfer_amount, fee_asset: asset.clone(), - memo: "{}".into(), + memo: String::new(), bridge_address, + rollup_block_number: 1, + rollup_withdrawal_event_id: "a-rollup-defined-hash-3".to_string(), }; // not enough balance; should fail @@ -233,7 +265,7 @@ mod tests { } #[tokio::test] - async fn execute_with_bridge_address_set() { + async fn execute_with_duplicated_withdrawal_event_id() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); @@ -258,31 +290,36 @@ mod tests { .unwrap(); state.put_bridge_account_withdrawer_address(bridge_address, bridge_address); state.put_allowed_fee_asset(&asset); + // Put plenty of balance + state + .put_account_balance(bridge_address, &asset, 3 * transfer_amount) + .unwrap(); - let bridge_unlock = BridgeUnlockAction { + let bridge_unlock_first = BridgeUnlockAction { to: to_address, amount: transfer_amount, fee_asset: asset.clone(), - memo: "{}".into(), + memo: String::new(), bridge_address, + rollup_block_number: 1, + rollup_withdrawal_event_id: "a-rollup-defined-hash".to_string(), + }; + let bridge_unlock_second = BridgeUnlockAction { + rollup_block_number: 10, + ..bridge_unlock_first.clone() }; - // not enough balance; should fail - state - .put_account_balance(bridge_address, &asset, transfer_amount) + // first should succeed, next should fail due to duplicate event. + bridge_unlock_first + .check_and_execute(&mut state) + .await .unwrap(); assert_anyhow_error( - &bridge_unlock + &bridge_unlock_second .check_and_execute(&mut state) .await .unwrap_err(), - "insufficient funds for transfer and fee payment", + "withdrawal event already processed", ); - - // enough balance; should pass - state - .put_account_balance(bridge_address, &asset, transfer_amount + transfer_fee) - .unwrap(); - bridge_unlock.check_and_execute(&mut state).await.unwrap(); } } diff --git a/crates/astria-sequencer/src/bridge/state_ext.rs b/crates/astria-sequencer/src/bridge/state_ext.rs index 8e12ba694..5da4f9bf8 100644 --- a/crates/astria-sequencer/src/bridge/state_ext.rs +++ b/crates/astria-sequencer/src/bridge/state_ext.rs @@ -5,6 +5,7 @@ use std::collections::{ use anyhow::{ anyhow, + bail, Context, Result, }; @@ -135,6 +136,20 @@ fn bridge_account_withdrawer_address_storage_key(address: &T) - ) } +fn bridge_account_withdrawal_event_storage_key( + address: &T, + withdrawal_event_id: &str, +) -> String { + format!( + "{}/withdrawalevent/{}", + BridgeAccountKey { + prefix: BRIDGE_ACCOUNT_PREFIX, + address + }, + withdrawal_event_id + ) +} + fn last_transaction_hash_for_bridge_account_storage_key(address: &T) -> Vec { format!( "{}/lasttx", @@ -419,6 +434,37 @@ pub(crate) trait StateWriteExt: StateWrite { ); } + #[instrument(skip_all)] + async fn check_and_set_withdrawal_event_block_for_bridge_account( + &mut self, + address: T, + withdrawal_event_id: &str, + block_num: u64, + ) -> Result<()> { + let key = bridge_account_withdrawal_event_storage_key(&address, withdrawal_event_id); + + // Check if the withdrawal ID has already been used, if so return an error. + let bytes = self + .get_raw(&key) + .await + .context("failed reading raw withdrawal event from state")?; + if let Some(bytes) = bytes { + let existing_block_num = u64::from_be_bytes( + bytes + .try_into() + .expect("all block numbers stored should be 8 bytes; this is a bug"), + ); + + bail!( + "withdrawal event ID {withdrawal_event_id} used by block number \ + {existing_block_num}" + ); + } + + self.put_raw(key, block_num.to_be_bytes().to_vec()); + Ok(()) + } + // the deposit "nonce" for a given rollup ID during a given block. // this is only used to generate storage keys for each of the deposits within a block, // and is reset to 0 at the beginning of each block. diff --git a/crates/astria-sequencer/src/ibc/ics20_transfer.rs b/crates/astria-sequencer/src/ibc/ics20_transfer.rs index b632474bc..914985945 100644 --- a/crates/astria-sequencer/src/ibc/ics20_transfer.rs +++ b/crates/astria-sequencer/src/ibc/ics20_transfer.rs @@ -1052,7 +1052,7 @@ mod test { memo: String::new(), rollup_block_number: 1, rollup_return_address: "rollup-defined".to_string(), - rollup_transaction_hash: hex::encode([1u8; 32]), + rollup_withdrawal_event_id: "a-rollup-defined-id".into(), }) .unwrap(), }; diff --git a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs index cf5d77221..515befa6b 100644 --- a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs +++ b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs @@ -9,7 +9,10 @@ use astria_core::{ asset::Denom, Address, }, - protocol::transaction::v1alpha1::action, + protocol::{ + memos::v1alpha1::Ics20WithdrawalFromRollup, + transaction::v1alpha1::action, + }, Protobuf as _, }; use cnidarium::{ @@ -35,7 +38,10 @@ use crate::{ address::StateReadExt as _, app::ActionHandler, assets::StateWriteExt as _, - bridge::StateReadExt as _, + bridge::{ + StateReadExt as _, + StateWriteExt as _, + }, ibc::{ StateReadExt as _, StateWriteExt as _, @@ -63,49 +69,81 @@ fn withdrawal_to_unchecked_ibc_packet( /// Establishes the withdrawal target. /// /// The function returns the following addresses under the following conditions: -/// 1. `from` if `action.bridge_address` is unset and `from` is *not* a bridge account; -/// 2. `from` if `action.bridge_address` is unset and `from` is a bridge account and `from` is its -/// stored withdrawer address. -/// 3. `action.bridge_address` if `action.bridge_address` is set and a bridge account and `from` is -/// its stored withdrawer address. +/// 1. `action.bridge_address` if `action.bridge_address` is set and `from` is its stored withdrawer +/// address. +/// 2. `from` if `action.bridge_address` is unset and `from` is *not* a bridge account. +/// +/// Errors if: +/// 1. Errors reading from DB +/// 2. `action.bridge_address` is set, but `from` is not the withdrawer address. +/// 3. `action.bridge_address` is unset, but `from` is a bridge account. async fn establish_withdrawal_target( action: &action::Ics20Withdrawal, state: &S, from: [u8; 20], ) -> Result<[u8; 20]> { - if action.bridge_address.is_none() - && !state - .is_a_bridge_account(from) + // If the bridge address is set, the withdrawer on that address must match + // the from address. + if let Some(bridge_address) = action.bridge_address { + let Some(withdrawer) = state + .get_bridge_account_withdrawer_address(bridge_address) .await - .context("failed to get bridge account rollup id")? - { - return Ok(from); - } + .context("failed to get bridge withdrawer")? + else { + bail!("bridge address must have a withdrawer address set"); + }; + + ensure!( + withdrawer == from.address_bytes(), + "sender does not match bridge withdrawer address; unauthorized" + ); - // if `action.bridge_address` is set, but it's not a valid bridge account, - // the `get_bridge_account_withdrawer_address` step will fail. - let bridge_address = action.bridge_address.map_or(from, Address::bytes); + return Ok(bridge_address.address_bytes()); + } - let Some(withdrawer) = state - .get_bridge_account_withdrawer_address(bridge_address) + // If the bridge address is not set, the sender must not be a bridge account. + if state + .is_a_bridge_account(from) .await - .context("failed to get bridge withdrawer")? - else { - bail!("bridge address must have a withdrawer address set"); - }; - - ensure!( - withdrawer == from.address_bytes(), - "sender does not match bridge withdrawer address; unauthorized" - ); + .context("failed to establish whether the sender is a bridge account")? + { + bail!("sender cannot be a bridge address if bridge address is not set"); + } - Ok(bridge_address) + Ok(from) } #[async_trait::async_trait] impl ActionHandler for action::Ics20Withdrawal { + // TODO(https://github.com/astriaorg/astria/issues/1430): move checks to the `Ics20Withdrawal` parsing. async fn check_stateless(&self) -> Result<()> { ensure!(self.timeout_time() != 0, "timeout time must be non-zero",); + ensure!(self.amount() > 0, "amount must be greater than zero",); + if self.bridge_address.is_some() { + let parsed_bridge_memo: Ics20WithdrawalFromRollup = serde_json::from_str(&self.memo) + .context("failed to parse memo for ICS bound bridge withdrawal")?; + + ensure!( + !parsed_bridge_memo.rollup_return_address.is_empty(), + "rollup return address must be non-empty", + ); + ensure!( + parsed_bridge_memo.rollup_return_address.len() <= 256, + "rollup return address must be no more than 256 bytes", + ); + ensure!( + !parsed_bridge_memo.rollup_withdrawal_event_id.is_empty(), + "rollup withdrawal event id must be non-empty", + ); + ensure!( + parsed_bridge_memo.rollup_withdrawal_event_id.len() <= 64, + "rollup withdrawal event id must be no more than 64 bytes", + ); + ensure!( + parsed_bridge_memo.rollup_block_number != 0, + "rollup block number must be non-zero", + ); + } // NOTE (from penumbra): we could validate the destination chain address as bech32 to // prevent mistyped addresses, but this would preclude sending to chains that don't @@ -128,6 +166,17 @@ impl ActionHandler for action::Ics20Withdrawal { state.ensure_base_prefix(bridge_address).await.context( "failed to verify that bridge address address has permitted base prefix", )?; + let parsed_bridge_memo: Ics20WithdrawalFromRollup = serde_json::from_str(&self.memo) + .context("failed to parse memo for ICS bound bridge withdrawal")?; + + state + .check_and_set_withdrawal_event_block_for_bridge_account( + self.bridge_address.map_or(from, Address::bytes), + &parsed_bridge_memo.rollup_withdrawal_event_id, + parsed_bridge_memo.rollup_block_number, + ) + .await + .context("withdrawal event already processed")?; } let withdrawal_target = establish_withdrawal_target(self, &state, from) @@ -207,7 +256,6 @@ mod tests { use super::*; use crate::{ address::StateWriteExt as _, - bridge::StateWriteExt as _, test_utils::{ assert_anyhow_error, astria_address, @@ -274,11 +322,11 @@ mod tests { memo: String::new(), }; - assert_eq!( - establish_withdrawal_target(&action, &state, bridge_address) + assert_anyhow_error( + &establish_withdrawal_target(&action, &state, bridge_address) .await - .unwrap(), - bridge_address, + .unwrap_err(), + "sender cannot be a bridge address if bridge address is not set", ); } @@ -333,13 +381,6 @@ mod tests { ); } - #[tokio::test] - async fn bridge_unset() { - let mut action = action(); - action.bridge_address = None; - run_test(action).await; - } - #[tokio::test] async fn bridge_set() { let mut action = action(); diff --git a/crates/astria-sequencer/src/lib.rs b/crates/astria-sequencer/src/lib.rs index ca41a1473..d35e9e078 100644 --- a/crates/astria-sequencer/src/lib.rs +++ b/crates/astria-sequencer/src/lib.rs @@ -28,5 +28,6 @@ mod utils; pub use build_info::BUILD_INFO; pub use config::Config; +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 dc6dd1266..0c45a9d53 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 eadd2365f..66a018ac2 100644 --- a/crates/astria-sequencer/src/metrics.rs +++ b/crates/astria-sequencer/src/metrics.rs @@ -1,22 +1,18 @@ use std::time::Duration; -use metrics::{ - counter, - describe_counter, - describe_gauge, - describe_histogram, - gauge, - histogram, - Counter, - Gauge, - Histogram, - Unit, +use telemetry::{ + metric_names, + metrics::{ + Counter, + Gauge, + Histogram, + RegisteringBuilder, + }, }; -use telemetry::metric_names; const CHECK_TX_STAGE: &str = "stage"; -pub(crate) struct Metrics { +pub struct Metrics { prepare_proposal_excluded_transactions_cometbft_space: Counter, prepare_proposal_excluded_transactions_sequencer_space: Counter, prepare_proposal_excluded_transactions_failed_execution: Counter, @@ -43,195 +39,6 @@ pub(crate) struct Metrics { } impl Metrics { - #[must_use] - #[allow(clippy::too_many_lines)] - pub(crate) fn new() -> Self { - 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); - - describe_counter!( - CHECK_TX_REMOVED_FAILED_EXECUTION, - Unit::Count, - "The number of transactions that have been removed from the mempool due to failing \ - execution in prepare_proposal()" - ); - let check_tx_removed_failed_execution = counter!(CHECK_TX_REMOVED_FAILED_EXECUTION); - - describe_counter!( - CHECK_TX_REMOVED_EXPIRED, - Unit::Count, - "The number of transactions that have been removed from the mempool due to expiring \ - in the app's mempool" - ); - let check_tx_removed_expired = counter!(CHECK_TX_REMOVED_EXPIRED); - - describe_histogram!( - CHECK_TX_DURATION_SECONDS, - Unit::Seconds, - "The amount of time taken in seconds to successfully complete the various stages of \ - check_tx" - ); - let check_tx_duration_seconds_parse_tx = histogram!( - CHECK_TX_DURATION_SECONDS, - CHECK_TX_STAGE => "length check and parse raw tx" - ); - let check_tx_duration_seconds_check_stateless = histogram!( - CHECK_TX_DURATION_SECONDS, - CHECK_TX_STAGE => "stateless check" - ); - let check_tx_duration_seconds_check_nonce = histogram!( - CHECK_TX_DURATION_SECONDS, - CHECK_TX_STAGE => "nonce check" - ); - let check_tx_duration_seconds_check_chain_id = histogram!( - CHECK_TX_DURATION_SECONDS, - CHECK_TX_STAGE => "chain id check" - ); - let check_tx_duration_seconds_check_balance = histogram!( - CHECK_TX_DURATION_SECONDS, - CHECK_TX_STAGE => "balance check" - ); - let check_tx_duration_seconds_check_removed = histogram!( - CHECK_TX_DURATION_SECONDS, - CHECK_TX_STAGE => "check for removal" - ); - let check_tx_duration_seconds_insert_to_app_mempool = histogram!( - CHECK_TX_DURATION_SECONDS, - CHECK_TX_STAGE => "insert to app mempool" - ); - - describe_histogram!( - ACTIONS_PER_TRANSACTION_IN_MEMPOOL, - Unit::Count, - "The number of actions in a transaction added to the app mempool" - ); - let actions_per_transaction_in_mempool = histogram!(ACTIONS_PER_TRANSACTION_IN_MEMPOOL); - - describe_histogram!( - TRANSACTION_IN_MEMPOOL_SIZE_BYTES, - Unit::Bytes, - "The number of bytes in a transaction added to the app mempool" - ); - let transaction_in_mempool_size_bytes = histogram!(TRANSACTION_IN_MEMPOOL_SIZE_BYTES); - - describe_gauge!( - TRANSACTIONS_IN_MEMPOOL_TOTAL, - Unit::Count, - "The number of transactions in the app mempool" - ); - let transactions_in_mempool_total = gauge!(TRANSACTIONS_IN_MEMPOOL_TOTAL); - - Self { - 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_expired, - check_tx_removed_failed_execution, - check_tx_removed_failed_stateless, - check_tx_removed_stale_nonce, - check_tx_removed_account_balance, - check_tx_duration_seconds_parse_tx, - check_tx_duration_seconds_check_stateless, - check_tx_duration_seconds_check_nonce, - check_tx_duration_seconds_check_chain_id, - check_tx_duration_seconds_check_balance, - check_tx_duration_seconds_check_removed, - check_tx_duration_seconds_insert_to_app_mempool, - actions_per_transaction_in_mempool, - transaction_in_mempool_size_bytes, - transactions_in_mempool_total, - } - } - pub(crate) fn increment_prepare_proposal_excluded_transactions_cometbft_space(&self) { self.prepare_proposal_excluded_transactions_cometbft_space .increment(1); @@ -248,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) { @@ -330,24 +131,197 @@ 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 telemetry::Metrics for Metrics { + type Config = (); + + // allow: this is reasonable as we have a lot of metrics to register; the function is not + // complex, just long. + #[allow(clippy::too_many_lines)] + fn register( + builder: &mut RegisteringBuilder, + _config: &Self::Config, + ) -> Result { + 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_expired = builder + .new_counter_factory( + CHECK_TX_REMOVED_EXPIRED, + "The number of transactions that have been removed from the mempool due to \ + expiring in the app's mempool", + )? + .register()?; + + let check_tx_removed_failed_execution = builder + .new_counter_factory( + CHECK_TX_REMOVED_FAILED_EXECUTION, + "The number of transactions that have been removed from the mempool due to \ + failing execution in prepare_proposal", + )? + .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()?; + + let mut check_tx_duration_factory = builder.new_histogram_factory( + CHECK_TX_DURATION_SECONDS, + "The amount of time taken in seconds to successfully complete the various stages of \ + check_tx", + )?; + let check_tx_duration_seconds_parse_tx = check_tx_duration_factory.register_with_labels( + &[(CHECK_TX_STAGE, "length check and parse raw tx".to_string())], + )?; + let check_tx_duration_seconds_check_stateless = check_tx_duration_factory + .register_with_labels(&[(CHECK_TX_STAGE, "stateless check".to_string())])?; + let check_tx_duration_seconds_check_nonce = check_tx_duration_factory + .register_with_labels(&[(CHECK_TX_STAGE, "nonce check".to_string())])?; + let check_tx_duration_seconds_check_chain_id = check_tx_duration_factory + .register_with_labels(&[(CHECK_TX_STAGE, "chain id check".to_string())])?; + let check_tx_duration_seconds_check_balance = check_tx_duration_factory + .register_with_labels(&[(CHECK_TX_STAGE, "balance check".to_string())])?; + let check_tx_duration_seconds_check_removed = check_tx_duration_factory + .register_with_labels(&[(CHECK_TX_STAGE, "check for removal".to_string())])?; + let check_tx_duration_seconds_insert_to_app_mempool = check_tx_duration_factory + .register_with_labels(&[(CHECK_TX_STAGE, "insert to app mempool".to_string())])?; + + let actions_per_transaction_in_mempool = builder + .new_histogram_factory( + ACTIONS_PER_TRANSACTION_IN_MEMPOOL, + "The number of actions in a transaction added to the app mempool", + )? + .register()?; + + let transaction_in_mempool_size_bytes = builder + .new_histogram_factory( + TRANSACTION_IN_MEMPOOL_SIZE_BYTES, + "The number of bytes in a transaction added to the app mempool", + )? + .register()?; + + let transactions_in_mempool_total = builder + .new_gauge_factory( + TRANSACTIONS_IN_MEMPOOL_TOTAL, + "The number of transactions in the app mempool", + )? + .register()?; + + Ok(Self { + 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_expired, + check_tx_removed_failed_execution, + check_tx_removed_failed_stateless, + check_tx_removed_stale_nonce, + check_tx_removed_account_balance, + check_tx_duration_seconds_parse_tx, + check_tx_duration_seconds_check_stateless, + check_tx_duration_seconds_check_nonce, + check_tx_duration_seconds_check_chain_id, + check_tx_duration_seconds_check_balance, + check_tx_duration_seconds_check_removed, + check_tx_duration_seconds_insert_to_app_mempool, + actions_per_transaction_in_mempool, + transaction_in_mempool_size_bytes, + transactions_in_mempool_total, + }) } } -metric_names!(pub const METRICS_NAMES: +metric_names!(const METRICS_NAMES: PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_COMETBFT_SPACE, PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_SEQUENCER_SPACE, PREPARE_PROPOSAL_EXCLUDED_TRANSACTIONS_FAILED_EXECUTION, diff --git a/crates/astria-sequencer/src/sequencer.rs b/crates/astria-sequencer/src/sequencer.rs index 3468832bb..ac5661ed7 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 _, @@ -10,6 +8,7 @@ use penumbra_tower_trace::{ trace::request_span, v038::RequestExt as _, }; +use telemetry::metrics::register_histogram_global; use tendermint::v0_38::abci::ConsensusRequest; use tokio::{ select, @@ -44,12 +43,10 @@ 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<()> { cnidarium::register_metrics(); - metrics::histogram!("cnidarium_get_raw_duration_seconds"); - metrics::histogram!("cnidarium_nonverifiable_get_raw_duration_seconds"); + register_histogram_global("cnidarium_get_raw_duration_seconds"); + register_histogram_global("cnidarium_nonverifiable_get_raw_duration_seconds"); if config .db_filepath diff --git a/crates/astria-sequencer/src/service/consensus.rs b/crates/astria-sequencer/src/service/consensus.rs index f5b6c9d56..4a4b290cd 100644 --- a/crates/astria-sequencer/src/service/consensus.rs +++ b/crates/astria-sequencer/src/service/consensus.rs @@ -220,6 +220,7 @@ mod test { use bytes::Bytes; use prost::Message as _; use rand::rngs::OsRng; + use telemetry::Metrics as _; use tendermint::{ account::Id, Hash, @@ -463,7 +464,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 aaec8de6f..0b7e04e8d 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 d63680016..4028a513d 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, -}; +pub 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() + .set_service_name(service_name) + .set_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 000000000..d713b7ff4 --- /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 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 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 set_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 000000000..da27c022f --- /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 000000000..1e12ba2bd --- /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 000000000..a3abc3b85 --- /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 000000000..ad7ccd9d9 --- /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 000000000..2f2587dd8 --- /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 000000000..8828c9a6b --- /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 000000000..078d6211c --- /dev/null +++ b/crates/astria-telemetry/src/metrics/into_f64.rs @@ -0,0 +1,67 @@ +/// 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) + } +} + +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 + } +} diff --git a/crates/astria-telemetry/src/metrics/mod.rs b/crates/astria-telemetry/src/metrics/mod.rs new file mode 100644 index 000000000..e029b5a0b --- /dev/null +++ b/crates/astria-telemetry/src/metrics/mod.rs @@ -0,0 +1,76 @@ +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) + } +} + +/// Registers the given histogram to the global recorder, if it has been set. +/// +/// This should only be used to register a histogram from a third party crate. All Astria metrics +/// should be added to a struct that implements `Metrics` and registered via `Metrics::register`. +pub fn register_histogram_global(name: &'static str) { + let _ = metrics::histogram!(name); +} diff --git a/proto/protocolapis/astria/protocol/memos/v1alpha1/types.proto b/proto/protocolapis/astria/protocol/memos/v1alpha1/types.proto index 4e8b2e493..00435b669 100644 --- a/proto/protocolapis/astria/protocol/memos/v1alpha1/types.proto +++ b/proto/protocolapis/astria/protocol/memos/v1alpha1/types.proto @@ -9,30 +9,19 @@ syntax = "proto3"; package astria.protocol.memos.v1alpha1; -message BridgeUnlock { - // The block number on the rollup that triggered the transaction underlying - // this bridge unlock memo. - uint64 rollup_block_number = 1; - // The hash of the original rollup transaction that triggered a bridge unlock - // and that is underlying this bridge unlock memo. - // - // This field is of type `string` so that it can be formatted in the preferred - // format of the rollup when targeting plain text encoding. - string rollup_transaction_hash = 2; -} - // Memo for an ICS20 withdrawal from the rollup which is sent to // an external IBC-enabled chain. message Ics20WithdrawalFromRollup { // The block number on the rollup that triggered the transaction underlying // this ics20 withdrawal memo. uint64 rollup_block_number = 1; - // The hash of the original rollup transaction that triggered this ics20 - // withdrawal and that is underlying this bridge unlock memo. + // An identifier of the original rollup withdrawal event that triggered this ics20 + // withdrawal and that is underlying this bridge unlock memo. For general EVM + // this is typically a transaction hash. // // This field is of type `string` so that it can be formatted in the preferred // format of the rollup when targeting plain text encoding. - string rollup_transaction_hash = 2; + string rollup_withdrawal_event_id = 2; // The return address on the rollup to which funds should returned in case of // failure. This field exists so that the rollup can identify which account // the returned funds originated from. diff --git a/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto b/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto index 19069f44a..05f40dc21 100644 --- a/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto +++ b/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto @@ -209,10 +209,22 @@ message BridgeUnlockAction { astria.primitive.v1.Uint128 amount = 2; // the asset used to pay the transaction fee string fee_asset = 3; - // memo for double spend prevention + // The memo field can be used to provide unique identifying additional + // information about the bridge unlock transaction. string memo = 4; // the address of the bridge account to transfer from astria.primitive.v1.Address bridge_address = 5; + // The block number on the rollup that triggered the transaction underlying + // this bridge unlock memo. + uint64 rollup_block_number = 6; + // An identifier of the original rollup event, such as a transaction hash which + // triggered a bridge unlock and is underlying event that led to this bridge + // unlock. This can be utilized for tracing from the bridge back to + // distinct rollup events. + // + // This field is of type `string` so that it can be formatted in the preferred + // format of the rollup when targeting plain text encoding. + string rollup_withdrawal_event_id = 7; } message BridgeSudoChangeAction {