Skip to content

feat(l1): fullsync metrics #6466

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.

Sign up for GitHub

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

Open
Arkenan wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat(l1): fullsync metrics #6466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a94044c
metrics
Arkenan Mar 31, 2026
88a37f2
moar metrics
Arkenan Mar 31, 2026
bdfef9e
add grafana dasbhoard for full sync
Arkenan Apr 13, 2026
47eda94
format
Arkenan Apr 16, 2026
23ff4dd
Merge remote-tracking branch 'origin/main' into fullsync-metrics
Arkenan Apr 16, 2026
28f3e58
fix(l1): defer Amsterdam block gas overflow check to post-execution (…
azteca1998 Apr 16, 2026
24bfd57
Merge branch 'main' into fullsync-metrics
Arkenan Apr 17, 2026
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
26 changes: 25 additions & 1 deletion crates/blockchain/blockchain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ use vm::StoreVmDatabase;

#[cfg(feature = "metrics")]
use ethrex_metrics::blocks::METRICS_BLOCKS;
#[cfg(feature = "metrics")]
use ethrex_metrics::fullsync::METRICS_FULLSYNC;

#[cfg(feature = "c-kzg")]
use ethrex_common::types::BlobsBundle;
Expand Down Expand Up @@ -2188,7 +2190,8 @@ impl Blockchain {
let mut total_gas_used = 0;
let mut transactions_count = 0;

let interval = Instant::now();
let interval = std::time::Instant::now();
let exec_start = std::time::Instant::now();
for (i, block) in blocks.iter().enumerate() {
if cancellation_token.is_cancelled() {
info!("Received shutdown signal, aborting");
Expand Down Expand Up @@ -2232,9 +2235,13 @@ impl Blockchain {
tokio::task::yield_now().await;
}

let exec_ms = exec_start.elapsed().as_millis();
Comment on lines 2235 to +2238
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 transitions_ms measured and logged but not exposed as a Prometheus metric

transitions_ms (time spent in vm.get_state_transitions()) is included in the [FULLSYNC TIMING] log and clearly matters for profiling, but there is no corresponding batch_transitions_ms gauge in MetricsFullSync. The Grafana "Batch Time Breakdown" stacked-bar chart sums body_download + execution + merkle + store, so it will consistently under-report total batch time by the transitions duration. This gap will be invisible to operators using the dashboard.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: crates/blockchain/blockchain.rs
Line: 2235-2238

Comment:
**`transitions_ms` measured and logged but not exposed as a Prometheus metric**

`transitions_ms` (time spent in `vm.get_state_transitions()`) is included in the `[FULLSYNC TIMING]` log and clearly matters for profiling, but there is no corresponding `batch_transitions_ms` gauge in `MetricsFullSync`. The Grafana "Batch Time Breakdown" stacked-bar chart sums `body_download + execution + merkle + store`, so it will consistently under-report total batch time by the transitions duration. This gap will be invisible to operators using the dashboard.

How can I resolve this? If you propose a fix, please make it concise.


let transitions_start = std::time::Instant::now();
let account_updates = vm
.get_state_transitions()
.map_err(|err| (ChainError::EvmError(err), None))?;
let transitions_ms = transitions_start.elapsed().as_millis();

let last_block = blocks
.last()
Expand All @@ -2244,11 +2251,13 @@ impl Blockchain {
let last_block_gas_limit = last_block.header.gas_limit;

// Apply the account updates over all blocks and compute the new state root
let merkle_start = std::time::Instant::now();
let account_updates_list = self
.storage
.apply_account_updates_batch(first_block_header.parent_hash, &account_updates)
.map_err(|e| (e.into(), None))?
.ok_or((ChainError::ParentStateNotFound, None))?;
let merkle_ms = merkle_start.elapsed().as_millis();

let new_state_root = account_updates_list.state_trie_hash;
let state_updates = account_updates_list.state_updates;
Expand All @@ -2258,6 +2267,7 @@ impl Blockchain {
// Check state root matches the one in block header
validate_state_root(&last_block.header, new_state_root).map_err(|e| (e, None))?;

let store_start = std::time::Instant::now();
let update_batch = UpdateBatch {
account_updates: state_updates,
storage_updates: accounts_updates,
Expand All @@ -2270,6 +2280,17 @@ impl Blockchain {
self.storage
.store_block_updates(update_batch)
.map_err(|e| (e.into(), None))?;
let store_ms = store_start.elapsed().as_millis();

info!(
"[FULLSYNC TIMING] Batch {}: exec={}ms, transitions={}ms, merkle={}ms, store={}ms, total={}ms",
blocks_len,
exec_ms,
transitions_ms,
merkle_ms,
store_ms,
interval.elapsed().as_millis()
);
Comment on lines 2190 to +2293
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Timing instrumentation always compiled regardless of metrics feature

All four timing Instants (exec_start, transitions_start, merkle_start, store_start) and the [FULLSYNC TIMING] info! log are unconditional — they run even when the metrics feature is disabled. The existing metrics! macro is specifically designed to gate this overhead. Similarly, body_download_start and its [FULLSYNC TIMING] info log in full.rs fire unconditionally. On a busy node this adds one info! line per batch and per body-download round, which can be verbose in non-metrics deployments.

Consider wrapping both the timing variables and the log under metrics!(...) / fullsync_metrics!(...), or demoting the log to debug!.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: crates/blockchain/blockchain.rs
Line: 2190-2293

Comment:
**Timing instrumentation always compiled regardless of `metrics` feature**

All four timing `Instant`s (`exec_start`, `transitions_start`, `merkle_start`, `store_start`) and the `[FULLSYNC TIMING]` `info!` log are unconditional — they run even when the `metrics` feature is disabled. The existing `metrics!` macro is specifically designed to gate this overhead. Similarly, `body_download_start` and its `[FULLSYNC TIMING]` info log in `full.rs` fire unconditionally. On a busy node this adds one `info!` line per batch and per body-download round, which can be verbose in non-metrics deployments.

Consider wrapping both the timing variables and the log under `metrics!(...)` / `fullsync_metrics!(...)`, or demoting the log to `debug!`.

How can I resolve this? If you propose a fix, please make it concise.


let elapsed_seconds = interval.elapsed().as_secs_f64();
let throughput = if elapsed_seconds > 0.0 && total_gas_used != 0 {
Expand All @@ -2285,6 +2306,9 @@ impl Blockchain {
// Set the latest gas used as the average gas used per block in the batch
METRICS_BLOCKS.set_latest_gas_used(total_gas_used as f64 / blocks_len as f64);
METRICS_BLOCKS.set_latest_gigagas(throughput);
METRICS_FULLSYNC.set_batch_execution_ms(exec_ms as f64);
METRICS_FULLSYNC.set_batch_merkle_ms(merkle_ms as f64);
METRICS_FULLSYNC.set_batch_store_ms(store_ms as f64);
);

if self.options.perf_logs_enabled {
Expand Down
10 changes: 8 additions & 2 deletions crates/blockchain/metrics/api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use axum::{Router, routing::get};

use crate::{
MetricsApiError, blocks::METRICS_BLOCKS, gather_default_metrics, node::METRICS_NODE,
p2p::METRICS_P2P, process::METRICS_PROCESS, transactions::METRICS_TX,
MetricsApiError, blocks::METRICS_BLOCKS, fullsync::METRICS_FULLSYNC, gather_default_metrics,
node::METRICS_NODE, p2p::METRICS_P2P, process::METRICS_PROCESS, transactions::METRICS_TX,
};

pub async fn start_prometheus_metrics_api(
Expand Down Expand Up @@ -54,6 +54,12 @@ pub(crate) async fn get_metrics() -> String {
Err(_) => tracing::error!("Failed to gather METRICS_P2P"),
};

ret_string.push('\n');
match METRICS_FULLSYNC.gather_metrics() {
Ok(s) => ret_string.push_str(&s),
Err(_) => tracing::error!("Failed to gather METRICS_FULLSYNC"),
};

ret_string.push('\n');
if let Some(node_metrics) = METRICS_NODE.get() {
match node_metrics.gather_metrics() {
Expand Down
Loading
Loading