Skip to content

feat(engine): improve payload validator tracing spans #18960

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

Jump to bottom
Merged
merged 25 commits into from
Oct 17, 2025
Merged

feat(engine): improve payload validator tracing spans #18960

merged 25 commits into from
Oct 17, 2025

Conversation

shekhirin
Copy link
Collaborator

@shekhirin shekhirin commented Oct 13, 2025 •
edited
Loading

Towards #18892

This PR has two main changes:

  • All instrument macro calls now have a debug level specified. This is done to prevent losing context for DEBUG level events — previously if the instrument macro created a span with level TRACE, then DEBUG events won't have this span as a parent, unless the env filter is specified to include these TRACE spans. Just creating a span doesn't emit an event itself, so it doesn't make the stdout noisier.
  • More spans with engine::tree* target is created, giving us a better overview of payload validation pipeline image

@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Oct 13, 2025
@shekhirin shekhirin marked this pull request as draft October 13, 2025 13:11
@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Oct 13, 2025
mediocregopher
mediocregopher reviewed Oct 14, 2025
View reviewed changes
Copy link
Collaborator

@mediocregopher mediocregopher left a comment

Choose a reason for hiding this comment

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

Does setting the level to trace mean that these fields won't get included in e.g. WARN logs? If so maybe it would be useful to also have these applied to all logs? Having more context in each individual log makes them easier to use imo

@shekhirin shekhirin force-pushed the alexey/payload-validator-spans branch from 4e2c21c to 2edfca4 Compare October 15, 2025 03:13
@shekhirin shekhirin force-pushed the alexey/payload-validator-spans branch from 2edfca4 to a2ca4a3 Compare October 15, 2025 03:14
@shekhirin shekhirin force-pushed the alexey/payload-validator-spans branch from 4aad039 to 32575f8 Compare October 15, 2025 03:19
@shekhirin shekhirin force-pushed the alexey/payload-validator-spans branch 3 times, most recently from 5f07b20 to a284629 Compare October 15, 2025 06:51
@shekhirin shekhirin force-pushed the alexey/payload-validator-spans branch from a284629 to d2f02a5 Compare October 15, 2025 06:52
shekhirin and others added 4 commits October 16, 2025 14:52
@shekhirin
Merge remote-tracking branch 'origin/main' into alexey/payload-valida…
413e7b6 
…tor-spans
@shekhirin @claude
chore: remove unused Block import
f076382 
Removes unused revm::context::Block import from payload_validator.rs.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@shekhirin
remove payload from try_insert_payload span
c33f51c
@shekhirin
spans.
071e028
@shekhirin shekhirin added C-enhancement New feature or request A-observability Related to tracing, metrics, logs and other observability tools labels Oct 16, 2025
shekhirin and others added 5 commits October 16, 2025 19:42
@shekhirin @claude
chore: add span names to payload processor instrumentation
c7881f3 
Added descriptive span names to instrument attributes for better
tracing visibility in the payload processor and prewarm components.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@shekhirin @claude
Merge main into alexey/payload-validator-spans
efea7e6 
Resolved conflicts in payload_validator.rs by accepting the simpler
implementation from main that always includes trie updates instead
of conditionally discarding them.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@shekhirin @claude
chore: update tracing spans to debug level
c3e65d5 
- Added level = "debug" to all instrument macros
- Changed info_span! to debug_span! across payload processor and validator
- Updated tracing imports to use debug_span instead of info_span

This ensures proper span level filtering for improved observability
in the payload processing pipeline.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@shekhirin
remove reth-bench binaries oops
79d4d7d
@shekhirin
chore: set all tracing spans to debug level
bc330b4 
- Added level = "debug" to all #[instrument(...)] macros in engine tree
- Changed all info_span! calls to debug_span!
- Updated imports to use debug_span instead of info_span

Files modified:
- crates/engine/tree/src/tree/cached_state.rs
- crates/engine/tree/src/tree/mod.rs
- crates/engine/tree/src/tree/metrics.rs
- crates/engine/tree/src/tree/payload_processor/multiproof.rs
- crates/engine/tree/src/tree/payload_processor/sparse_trie.rs
- crates/engine/tree/src/tree/payload_processor/mod.rs (already done)
- crates/engine/tree/src/tree/payload_processor/prewarm.rs (already done)
- crates/engine/tree/src/tree/payload_validator.rs (already done)
@shekhirin shekhirin marked this pull request as ready for review October 16, 2025 11:12
yongkangc
yongkangc approved these changes Oct 17, 2025
View reviewed changes
Copy link
Member

@yongkangc yongkangc left a comment •
edited
Loading

Choose a reason for hiding this comment

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

overall looks good to me 👍🏻

one comment:

  • i think span would help within build_account_multiproof

crates/engine/tree/src/tree/payload_validator.rs
let block_num_hash = input.num_hash();

trace!(target: "engine::tree", block=?block_num_hash, parent=?parent_hash, "Fetching block state provider");
trace!(target: "engine::tree::payload_validator", "Fetching block state provider");
Copy link
Member

Choose a reason for hiding this comment

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

do we not want the block num hash here?

Copy link
Member

Choose a reason for hiding this comment

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

That info will be added by the instrument call at the top of the fn

@yongkangc yongkangc mentioned this pull request Oct 17, 2025
Merged
@jenpaff jenpaff linked an issue Oct 17, 2025 that may be closed by this pull request
Cleanup payload validation tracing spans #18892
Closed
@Rjected Rjected enabled auto-merge October 17, 2025 20:39
@Rjected Rjected added this pull request to the merge queue Oct 17, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 17, 2025
@Rjected Rjected added this pull request to the merge queue Oct 17, 2025
Merged via the queue into main with commit 4a32bc0 Oct 17, 2025
41 checks passed
@Rjected Rjected deleted the alexey/payload-validator-spans branch October 17, 2025 21:35
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Oct 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mediocregopher mediocregopher mediocregopher approved these changes

@yongkangc yongkangc yongkangc approved these changes

@rkrasiuk rkrasiuk Awaiting requested review from rkrasiuk rkrasiuk is a code owner

@mattsse mattsse Awaiting requested review from mattsse mattsse is a code owner

@Rjected Rjected Awaiting requested review from Rjected Rjected is a code owner

@fgimenez fgimenez Awaiting requested review from fgimenez fgimenez is a code owner

@RomanHodulak RomanHodulak Awaiting requested review from RomanHodulak RomanHodulak is a code owner

@joshieDo joshieDo Awaiting requested review from joshieDo joshieDo is a code owner

Assignees

@Rjected Rjected

Labels

A-observability Related to tracing, metrics, logs and other observability tools C-enhancement New feature or request

Projects

Reth Tracker
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Cleanup payload validation tracing spans

4 participants

@shekhirin @mediocregopher @Rjected @yongkangc