From ddceb6c5bd90a259159e0f27405a635458bf3fda Mon Sep 17 00:00:00 2001 From: ethanoroshiba Date: Mon, 28 Oct 2024 16:06:50 -0500 Subject: [PATCH 1/5] add instrumentation --- .../astria-sequencer/src/accounts/action.rs | 7 +++ .../src/accounts/component.rs | 2 +- crates/astria-sequencer/src/accounts/query.rs | 7 ++- .../src/accounts/state_ext.rs | 6 +- .../astria-sequencer/src/address/state_ext.rs | 7 ++- crates/astria-sequencer/src/app/mod.rs | 21 ++++--- crates/astria-sequencer/src/app/state_ext.rs | 10 +-- crates/astria-sequencer/src/assets/query.rs | 2 + .../astria-sequencer/src/assets/state_ext.rs | 4 +- .../astria-sequencer/src/authority/action.rs | 7 +++ .../src/authority/component.rs | 6 +- .../src/authority/state_ext.rs | 6 +- .../src/bridge/bridge_lock_action.rs | 5 ++ .../src/bridge/bridge_sudo_change_action.rs | 5 ++ .../src/bridge/bridge_unlock_action.rs | 6 ++ .../src/bridge/init_bridge_account_action.rs | 5 ++ crates/astria-sequencer/src/bridge/query.rs | 6 +- .../astria-sequencer/src/bridge/state_ext.rs | 12 ++-- crates/astria-sequencer/src/fees/action.rs | 6 ++ crates/astria-sequencer/src/fees/component.rs | 2 +- crates/astria-sequencer/src/fees/query.rs | 5 +- crates/astria-sequencer/src/fees/state_ext.rs | 62 +++++++++---------- crates/astria-sequencer/src/grpc/state_ext.rs | 16 ++--- crates/astria-sequencer/src/ibc/component.rs | 6 +- .../src/ibc/host_interface.rs | 5 ++ .../src/ibc/ibc_relayer_change.rs | 5 ++ .../src/ibc/ics20_transfer.rs | 10 +++ .../src/ibc/ics20_withdrawal.rs | 8 +++ crates/astria-sequencer/src/ibc/state_ext.rs | 4 +- .../src/mempool/mempool_state.rs | 2 +- crates/astria-sequencer/src/mempool/mod.rs | 10 +-- .../src/mempool/transactions_container.rs | 7 ++- crates/astria-sequencer/src/sequencer.rs | 41 +++++++----- .../astria-sequencer/src/service/consensus.rs | 9 +-- .../src/service/mempool/mod.rs | 4 ++ .../src/transaction/checks.rs | 6 +- .../astria-sequencer/src/transaction/mod.rs | 7 ++- 37 files changed, 226 insertions(+), 113 deletions(-) diff --git a/crates/astria-sequencer/src/accounts/action.rs b/crates/astria-sequencer/src/accounts/action.rs index dce040abf1..4d1669d1a7 100644 --- a/crates/astria-sequencer/src/accounts/action.rs +++ b/crates/astria-sequencer/src/accounts/action.rs @@ -8,6 +8,10 @@ use cnidarium::{ StateRead, StateWrite, }; +use tracing::{ + instrument, + Level, +}; use super::AddressBytes; use crate::{ @@ -27,6 +31,7 @@ impl ActionHandler for Transfer { Ok(()) } + #[instrument(skip_all, err(level = Level::WARN))] async fn check_and_execute(&self, state: S) -> Result<()> { let from = state .get_transaction_context() @@ -49,6 +54,7 @@ impl ActionHandler for Transfer { } } +#[instrument(skip_all, err(level = Level::WARN))] pub(crate) async fn execute_transfer( action: &Transfer, from: &TAddress, @@ -71,6 +77,7 @@ where Ok(()) } +#[instrument(skip_all, err(level = Level::WARN))] pub(crate) async fn check_transfer( action: &Transfer, from: &TAddress, diff --git a/crates/astria-sequencer/src/accounts/component.rs b/crates/astria-sequencer/src/accounts/component.rs index c9d515ec45..b8869d4a7c 100644 --- a/crates/astria-sequencer/src/accounts/component.rs +++ b/crates/astria-sequencer/src/accounts/component.rs @@ -25,7 +25,7 @@ pub(crate) struct AccountsComponent; impl Component for AccountsComponent { type AppState = GenesisAppState; - #[instrument(name = "AccountsComponent::init_chain", skip_all)] + #[instrument(name = "AccountsComponent::init_chain", skip_all, err)] async fn init_chain(mut state: S, app_state: &Self::AppState) -> Result<()> where S: accounts::StateWriteExt + assets::StateReadExt, diff --git a/crates/astria-sequencer/src/accounts/query.rs b/crates/astria-sequencer/src/accounts/query.rs index 00c23a4095..5c76cf6aab 100644 --- a/crates/astria-sequencer/src/accounts/query.rs +++ b/crates/astria-sequencer/src/accounts/query.rs @@ -36,6 +36,7 @@ use crate::{ assets::StateReadExt as _, }; +#[instrument(skip_all, fields(%asset), err)] async fn ibc_to_trace( state: S, asset: &asset::IbcPrefixed, @@ -47,7 +48,7 @@ async fn ibc_to_trace( .ok_or_eyre("asset not found when user has balance of it; this is a bug") } -#[instrument(skip_all, fields(%address))] +#[instrument(skip_all, fields(%address), err)] async fn get_trace_prefixed_account_balances( state: &S, address: &Address, @@ -70,6 +71,7 @@ async fn get_trace_prefixed_account_balances( /// Returns a list of [`AssetBalance`]s for the provided address. `AssetBalance`s are sorted /// alphabetically by [`asset::Denom`]. +#[instrument(skip_all)] pub(crate) async fn balance_request( storage: Storage, request: request::Query, @@ -112,6 +114,7 @@ pub(crate) async fn balance_request( } } +#[instrument(skip_all)] pub(crate) async fn nonce_request( storage: Storage, request: request::Query, @@ -150,6 +153,7 @@ pub(crate) async fn nonce_request( } } +#[instrument(skip_all, fields(%height), err)] async fn get_snapshot_and_height(storage: &Storage, height: Height) -> Result<(Snapshot, Height)> { let snapshot = match height.value() { 0 => storage.latest_snapshot(), @@ -173,6 +177,7 @@ async fn get_snapshot_and_height(storage: &Storage, height: Height) -> Result<(S Ok((snapshot, height)) } +#[instrument(skip_all)] async fn preprocess_request( storage: &Storage, request: &request::Query, diff --git a/crates/astria-sequencer/src/accounts/state_ext.rs b/crates/astria-sequencer/src/accounts/state_ext.rs index 3811dfa54d..c25f840c0f 100644 --- a/crates/astria-sequencer/src/accounts/state_ext.rs +++ b/crates/astria-sequencer/src/accounts/state_ext.rs @@ -165,7 +165,7 @@ pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt { .wrap_err("invalid balance bytes") } - #[instrument(skip_all)] + #[instrument(skip_all, err)] async fn get_account_nonce(&self, address: &T) -> Result { let bytes = self .get_raw(&keys::nonce(address)) @@ -205,7 +205,7 @@ pub(crate) trait StateWriteExt: StateWrite { Ok(()) } - #[instrument(skip_all)] + #[instrument(skip_all, fields(address = %address.display_address(), nonce), err)] fn put_account_nonce(&mut self, address: &T, nonce: u32) -> Result<()> { let bytes = StoredValue::from(storage::Nonce::from(nonce)) .serialize() @@ -241,7 +241,7 @@ pub(crate) trait StateWriteExt: StateWrite { Ok(()) } - #[instrument(skip_all, fields(address = %address.display_address(), %asset, amount))] + #[instrument(skip_all, fields(address = %address.display_address(), %asset, amount), err)] async fn decrease_balance<'a, TAddress, TAsset>( &mut self, address: &TAddress, diff --git a/crates/astria-sequencer/src/address/state_ext.rs b/crates/astria-sequencer/src/address/state_ext.rs index 0f5cd5bf30..308bdfc21e 100644 --- a/crates/astria-sequencer/src/address/state_ext.rs +++ b/crates/astria-sequencer/src/address/state_ext.rs @@ -22,10 +22,14 @@ use super::storage::{ self, keys, }; -use crate::storage::StoredValue; +use crate::{ + accounts::AddressBytes as _, + storage::StoredValue, +}; #[async_trait] pub(crate) trait StateReadExt: StateRead { + #[instrument(skip_all, fields(address = %address.display_address()), err)] async fn ensure_base_prefix(&self, address: &Address) -> Result<()> { let prefix = self .get_base_prefix() @@ -39,6 +43,7 @@ pub(crate) trait StateReadExt: StateRead { Ok(()) } + #[instrument(skip_all, err)] async fn try_base_prefixed(&self, slice: &[u8]) -> Result
{ let prefix = self .get_base_prefix() diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index 2231b3ab64..1bee46b645 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -234,6 +234,7 @@ pub(crate) struct App { } impl App { + #[instrument(name = "App::new", skip_all, err)] pub(crate) async fn new( snapshot: Snapshot, mempool: Mempool, @@ -267,7 +268,7 @@ impl App { }) } - #[instrument(name = "App:init_chain", skip_all)] + #[instrument(name = "App:init_chain", skip_all, err)] pub(crate) async fn init_chain( &mut self, storage: Storage, @@ -354,7 +355,7 @@ impl App { /// It puts this special "commitment" as the first transaction in a block. /// When other validators receive the block, they know the first transaction is /// supposed to be the commitment, and verifies that is it correct. - #[instrument(name = "App::prepare_proposal", skip_all)] + #[instrument(name = "App::prepare_proposal", skip_all, err)] pub(crate) async fn prepare_proposal( &mut self, prepare_proposal: abci::request::PrepareProposal, @@ -404,7 +405,7 @@ impl App { /// Generates a commitment to the `sequence::Actions` in the block's transactions /// and ensures it matches the commitment created by the proposer, which /// should be the first transaction in the block. - #[instrument(name = "App::process_proposal", skip_all)] + #[instrument(name = "App::process_proposal", skip_all, err)] pub(crate) async fn process_proposal( &mut self, process_proposal: abci::request::ProcessProposal, @@ -564,7 +565,7 @@ impl App { /// /// As a result, all transactions in a sequencer block are guaranteed to execute /// successfully. - #[instrument(name = "App::execute_transactions_prepare_proposal", skip_all)] + #[instrument(name = "App::execute_transactions_prepare_proposal", skip_all, err)] async fn execute_transactions_prepare_proposal( &mut self, block_size_constraints: &mut BlockSizeConstraints, @@ -744,7 +745,7 @@ impl App { /// /// As a result, all transactions in a sequencer block are guaranteed to execute /// successfully. - #[instrument(name = "App::execute_transactions_process_proposal", skip_all)] + #[instrument(name = "App::execute_transactions_process_proposal", skip_all, err)] async fn execute_transactions_process_proposal( &mut self, txs: Vec, @@ -877,7 +878,7 @@ impl App { /// `SequencerBlock`. /// /// this must be called after a block's transactions are executed. - #[instrument(name = "App::post_execute_transactions", skip_all)] + #[instrument(name = "App::post_execute_transactions", skip_all, err)] async fn post_execute_transactions( &mut self, block_hash: Hash, @@ -961,7 +962,7 @@ impl App { /// /// This is called by cometbft after the block has already been /// committed by the network's consensus. - #[instrument(name = "App::finalize_block", skip_all)] + #[instrument(name = "App::finalize_block", skip_all, err)] pub(crate) async fn finalize_block( &mut self, finalize_block: abci::request::FinalizeBlock, @@ -1112,7 +1113,7 @@ impl App { Ok(app_hash) } - #[instrument(name = "App::begin_block", skip_all)] + #[instrument(name = "App::begin_block", skip_all, err)] async fn begin_block( &mut self, begin_block: &abci::request::BeginBlock, @@ -1148,7 +1149,7 @@ impl App { } /// Executes a signed transaction. - #[instrument(name = "App::execute_transaction", skip_all)] + #[instrument(name = "App::execute_transaction", skip_all, err)] async fn execute_transaction(&mut self, signed_tx: Arc) -> Result> { signed_tx .check_stateless() @@ -1176,7 +1177,7 @@ impl App { Ok(state_tx.apply().1) } - #[instrument(name = "App::end_block", skip_all)] + #[instrument(name = "App::end_block", skip_all, err)] async fn end_block( &mut self, height: u64, diff --git a/crates/astria-sequencer/src/app/state_ext.rs b/crates/astria-sequencer/src/app/state_ext.rs index b9a2e3b78e..d4ef0f1de6 100644 --- a/crates/astria-sequencer/src/app/state_ext.rs +++ b/crates/astria-sequencer/src/app/state_ext.rs @@ -22,7 +22,7 @@ use crate::storage::StoredValue; #[async_trait] pub(crate) trait StateReadExt: StateRead { - #[instrument(skip_all)] + #[instrument(skip_all, err)] async fn get_chain_id(&self) -> Result { let Some(bytes) = self .get_raw(keys::CHAIN_ID) @@ -37,7 +37,7 @@ pub(crate) trait StateReadExt: StateRead { .wrap_err("invalid chain id bytes") } - #[instrument(skip_all)] + #[instrument(skip_all, err)] async fn get_revision_number(&self) -> Result { let Some(bytes) = self .get_raw(keys::REVISION_NUMBER) @@ -52,7 +52,7 @@ pub(crate) trait StateReadExt: StateRead { .wrap_err("invalid revision number bytes") } - #[instrument(skip_all)] + #[instrument(skip_all, err)] async fn get_block_height(&self) -> Result { let Some(bytes) = self .get_raw(keys::BLOCK_HEIGHT) @@ -67,7 +67,7 @@ pub(crate) trait StateReadExt: StateRead { .context("invalid block height bytes") } - #[instrument(skip_all)] + #[instrument(skip_all, err)] async fn get_block_timestamp(&self) -> Result