Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(sequencer): Add instrumentation #1368

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions crates/astria-sequencer/src/accounts/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ use cnidarium::{
StateRead,
StateWrite,
};
use tracing::{
instrument,
Level,
};

use super::AddressBytes;
use crate::{
Expand All @@ -31,10 +35,12 @@ use crate::{

#[async_trait::async_trait]
impl ActionHandler for TransferAction {
#[instrument(skip_all)]
async fn check_stateless(&self) -> Result<()> {
Ok(())
}

#[instrument(skip_all, err(level = Level::WARN))]
async fn check_and_execute<S: StateWrite>(&self, state: S) -> Result<()> {
let from = state
.get_transaction_context()
Expand All @@ -57,6 +63,11 @@ impl ActionHandler for TransferAction {
}
}

#[instrument(skip_all,
fields(from = %from.display_address(),
to = %action.to.display_address(),
amount = %action.amount),
err(level = Level::WARN))]
pub(crate) async fn execute_transfer<S, TAddress>(
action: &TransferAction,
from: TAddress,
Expand Down Expand Up @@ -115,6 +126,11 @@ where
Ok(())
}

#[instrument(skip_all,
fields(from = %from.display_address(),
to = %action.to.display_address(),
amount = %action.amount),
err(level = Level::WARN))]
pub(crate) async fn check_transfer<S, TAddress>(
action: &TransferAction,
from: TAddress,
Expand Down
6 changes: 3 additions & 3 deletions crates/astria-sequencer/src/accounts/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,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<S>(mut state: S, app_state: &Self::AppState) -> Result<()>
where
S: accounts::StateWriteExt + assets::StateReadExt,
Expand All @@ -45,15 +45,15 @@ impl Component for AccountsComponent {
Ok(())
}

#[instrument(name = "AccountsComponent::begin_block", skip_all)]
#[instrument(name = "AccountsComponent::begin_block", skip_all, err)]
async fn begin_block<S: accounts::StateWriteExt + 'static>(
_state: &mut Arc<S>,
_begin_block: &BeginBlock,
) -> Result<()> {
Ok(())
}

#[instrument(name = "AccountsComponent::end_block", skip_all)]
#[instrument(name = "AccountsComponent::end_block", skip_all, err)]
async fn end_block<S: accounts::StateWriteExt + 'static>(
_state: &mut Arc<S>,
_end_block: &EndBlock,
Expand Down
5 changes: 5 additions & 0 deletions crates/astria-sequencer/src/accounts/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use crate::{
state_ext::StateReadExt as _,
};

#[instrument(skip_all, fields(%asset), err)]
async fn ibc_to_trace<S: StateRead>(
state: S,
asset: asset::IbcPrefixed,
Expand Down Expand Up @@ -77,6 +78,7 @@ async fn get_trace_prefixed_account_balances<S: StateRead>(
stream.try_collect::<Vec<_>>().await
}

#[instrument(skip_all)]
pub(crate) async fn balance_request(
storage: Storage,
request: request::Query,
Expand Down Expand Up @@ -116,6 +118,7 @@ pub(crate) async fn balance_request(
}
}

#[instrument(skip_all)]
pub(crate) async fn nonce_request(
storage: Storage,
request: request::Query,
Expand Down Expand Up @@ -154,6 +157,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(),
Expand All @@ -177,6 +181,7 @@ async fn get_snapshot_and_height(storage: &Storage, height: Height) -> Result<(S
Ok((snapshot, height))
}

#[instrument(skip_all)]
ethanoroshiba marked this conversation as resolved.
Show resolved Hide resolved
async fn preprocess_request(
storage: &Storage,
request: &request::Query,
Expand Down
12 changes: 6 additions & 6 deletions crates/astria-sequencer/src/accounts/state_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ fn extract_asset_from_key(s: &str) -> Result<asset::IbcPrefixed> {

#[async_trait]
pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt {
#[instrument(skip_all)]
#[instrument(skip_all, fields(address = %address.display_address()))]
fn account_asset_keys(
&self,
address: impl AddressBytes,
Expand All @@ -175,7 +175,7 @@ pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt {
}
}

#[instrument(skip_all)]
#[instrument(skip_all, fields(address = %address.display_address()))]
fn account_asset_balances(
&self,
address: impl AddressBytes,
Expand Down Expand Up @@ -208,7 +208,7 @@ pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt {
Ok(balance)
}

#[instrument(skip_all)]
#[instrument(skip_all, fields(address = %address.display_address()), err)]
async fn get_account_nonce<T: AddressBytes>(&self, address: T) -> Result<u32> {
let bytes = self
.get_raw(&nonce_storage_key(address))
Expand All @@ -224,7 +224,7 @@ pub(crate) trait StateReadExt: StateRead + crate::assets::StateReadExt {
Ok(nonce)
}

#[instrument(skip_all)]
#[instrument(skip_all, err)]
async fn get_transfer_base_fee(&self) -> Result<u128> {
let bytes = self
.get_raw(TRANSFER_BASE_FEE_STORAGE_KEY)
Expand Down Expand Up @@ -260,7 +260,7 @@ pub(crate) trait StateWriteExt: StateWrite {
Ok(())
}

#[instrument(skip_all)]
#[instrument(skip_all, fields(address = %address.display_address(), nonce), err)]
fn put_account_nonce<T: AddressBytes>(&mut self, address: T, nonce: u32) -> Result<()> {
let bytes = borsh::to_vec(&Nonce(nonce)).wrap_err("failed to serialize nonce")?;
self.put_raw(nonce_storage_key(address), bytes);
Expand Down Expand Up @@ -321,7 +321,7 @@ pub(crate) trait StateWriteExt: StateWrite {
Ok(())
}

#[instrument(skip_all)]
#[instrument(skip_all, fields(fee), err)]
fn put_transfer_base_fee(&mut self, fee: u128) -> Result<()> {
let bytes = borsh::to_vec(&Fee(fee)).wrap_err("failed to serialize fee")?;
self.put_raw(TRANSFER_BASE_FEE_STORAGE_KEY.to_string(), bytes);
Expand Down
2 changes: 2 additions & 0 deletions crates/astria-sequencer/src/address/state_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ fn ibc_compat_prefix_key() -> &'static str {

#[async_trait]
pub(crate) trait StateReadExt: StateRead {
#[instrument(skip_all, fields(%address), err)]
async fn ensure_base_prefix(&self, address: &Address<Bech32m>) -> Result<()> {
let prefix = self
.get_base_prefix()
Expand All @@ -41,6 +42,7 @@ pub(crate) trait StateReadExt: StateRead {
Ok(())
}

#[instrument(skip_all, err)]
async fn try_base_prefixed(&self, slice: &[u8]) -> Result<Address> {
let prefix = self
.get_base_prefix()
Expand Down
16 changes: 8 additions & 8 deletions crates/astria-sequencer/src/api_state_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl From<RollupId> for RollupIdSer {

#[async_trait]
pub(crate) trait StateReadExt: StateRead {
#[instrument(skip_all)]
#[instrument(skip_all, fields(%height), err)]
async fn get_block_hash_by_height(&self, height: u64) -> Result<[u8; 32]> {
let key = block_hash_by_height_key(height);
let Some(hash) = self
Expand All @@ -153,7 +153,7 @@ pub(crate) trait StateReadExt: StateRead {
Ok(hash)
}

#[instrument(skip_all)]
#[instrument(skip_all, fields(hash = %hex::encode(hash)), err)]
async fn get_sequencer_block_header_by_hash(
&self,
hash: &[u8],
Expand All @@ -175,7 +175,7 @@ pub(crate) trait StateReadExt: StateRead {
Ok(header)
}

#[instrument(skip_all)]
#[instrument(skip_all, fields(hash = %hex::encode(hash)), err)]
async fn get_rollup_ids_by_block_hash(&self, hash: &[u8]) -> Result<Vec<RollupId>> {
let key = rollup_ids_by_hash_key(hash);
let Some(rollup_ids_bytes) = self
Expand All @@ -192,7 +192,7 @@ pub(crate) trait StateReadExt: StateRead {
Ok(rollup_ids)
}

#[instrument(skip_all)]
#[instrument(skip_all, fields(hash = %hex::encode(hash)), err)]
async fn get_sequencer_block_by_hash(&self, hash: &[u8]) -> Result<SequencerBlock> {
let Some(header_bytes) = self
.nonverifiable_get_raw(&sequencer_block_header_by_hash_key(hash))
Expand Down Expand Up @@ -266,7 +266,7 @@ pub(crate) trait StateReadExt: StateRead {
Ok(block)
}

#[instrument(skip_all)]
#[instrument(skip_all, fields(%height), err)]
async fn get_sequencer_block_by_height(&self, height: u64) -> Result<SequencerBlock> {
let hash = self
.get_block_hash_by_height(height)
Expand All @@ -277,7 +277,7 @@ pub(crate) trait StateReadExt: StateRead {
.wrap_err("failed to get sequencer block by hash")
}

#[instrument(skip_all)]
#[instrument(skip_all, fields(hash = %hex::encode(hash), %rollup_id), err)]
async fn get_rollup_data(
&self,
hash: &[u8],
Expand All @@ -301,7 +301,7 @@ pub(crate) trait StateReadExt: StateRead {
Ok(rollup_transactions)
}

#[instrument(skip_all)]
#[instrument(skip_all, fields(hash = %hex::encode(hash)), err)]
async fn get_block_proofs_by_block_hash(
&self,
hash: &[u8],
Expand Down Expand Up @@ -338,7 +338,7 @@ pub(crate) trait StateReadExt: StateRead {
impl<T: StateRead> StateReadExt for T {}

pub(crate) trait StateWriteExt: StateWrite {
#[instrument(skip_all)]
#[instrument(skip_all, fields(height = %block.height()), err)]
fn put_sequencer_block(&mut self, block: SequencerBlock) -> Result<()> {
// split up and write the sequencer block to state in the following order:
// 1. height to block hash
Expand Down
25 changes: 14 additions & 11 deletions crates/astria-sequencer/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ use tracing::{
debug,
info,
instrument,
Level,
};

use crate::{
Expand Down Expand Up @@ -184,6 +185,7 @@ pub(crate) struct App {
}

impl App {
#[instrument(skip_all, err)]
pub(crate) async fn new(
snapshot: Snapshot,
mempool: Mempool,
Expand Down Expand Up @@ -218,7 +220,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,
Expand Down Expand Up @@ -301,7 +303,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,
Expand Down Expand Up @@ -351,7 +353,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(level = Level::WARN))]
pub(crate) async fn process_proposal(
&mut self,
process_proposal: abci::request::ProcessProposal,
Expand Down Expand Up @@ -482,7 +484,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,
Expand Down Expand Up @@ -633,7 +635,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(level = Level::WARN))]
async fn execute_transactions_process_proposal(
&mut self,
txs: Vec<SignedTransaction>,
Expand Down Expand Up @@ -765,7 +767,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,
Expand Down Expand Up @@ -836,7 +838,7 @@ impl App {
}),
Err(e) => {
// this is actually a protocol error, as only valid txs should be finalized
tracing::error!(
tracing::warn!(
error = AsRef::<dyn std::error::Error>::as_ref(&e),
"failed to finalize transaction; ignoring it",
);
Expand Down Expand Up @@ -953,8 +955,8 @@ impl App {
Ok(app_hash)
}

#[instrument(name = "App::begin_block", skip_all)]
async fn begin_block(
#[instrument(name = "App::begin_block", skip_all, err)]
pub(crate) async fn begin_block(
&mut self,
begin_block: &abci::request::BeginBlock,
) -> Result<Vec<abci::Event>> {
Expand Down Expand Up @@ -1020,8 +1022,8 @@ impl App {
Ok(state_tx.apply().1)
}

#[instrument(name = "App::end_block", skip_all)]
async fn end_block(
#[instrument(name = "App::end_block", skip_all, err)]
pub(crate) async fn end_block(
&mut self,
height: u64,
fee_recipient: [u8; 20],
Expand Down Expand Up @@ -1141,6 +1143,7 @@ impl App {
// NOTE: this function locks the mempool until all accounts have been cleaned.
// this could potentially stall consensus from moving to the next round if
// the mempool is large, especially if recosting transactions.
#[instrument(skip_all)]
async fn update_mempool_after_finalization<S: StateRead>(
mempool: &mut Mempool,
state: &S,
Expand Down
3 changes: 3 additions & 0 deletions crates/astria-sequencer/src/assets/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use tendermint::abci::{
response,
Code,
};
use tracing::instrument;

use crate::{
assets::StateReadExt as _,
Expand All @@ -24,6 +25,7 @@ use crate::{
//
// Example:
// `abci-cli query --path=asset/denom/<DENOM_ID>`
#[instrument(skip_all)]
pub(crate) async fn denom_request(
storage: Storage,
request: request::Query,
Expand Down Expand Up @@ -110,6 +112,7 @@ fn preprocess_request(params: &[(String, String)]) -> Result<asset::IbcPrefixed,
Ok(asset)
}

#[instrument(skip_all)]
pub(crate) async fn allowed_fee_assets_request(
storage: Storage,
request: request::Query,
Expand Down
Loading
Loading