Skip to content

Conversation

@shekhirin
Copy link
Collaborator

@shekhirin shekhirin commented Oct 13, 2025

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 from Backlog to In Progress in Reth Tracker Oct 13, 2025
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 shekhirin requested a review from joshieDo as a code owner October 16, 2025 11:12
Copy link
Member

@yongkangc yongkangc left a comment

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

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

@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
emhane pushed a commit to op-rs/op-reth that referenced this pull request Oct 20, 2025
emhane pushed a commit to op-rs/op-reth that referenced this pull request Oct 20, 2025
mediocregopher added a commit that referenced this pull request Oct 20, 2025
mediocregopher added a commit that referenced this pull request Oct 20, 2025
@jenpaff jenpaff moved this from Done to Completed in Reth Tracker Oct 30, 2025
shekhirin added a commit that referenced this pull request Oct 30, 2025
shekhirin added a commit that referenced this pull request Oct 30, 2025
shekhirin added a commit that referenced this pull request Oct 30, 2025
shekhirin added a commit that referenced this pull request Oct 30, 2025
shekhirin added a commit that referenced this pull request Oct 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

Status: Completed

Development

Successfully merging this pull request may close these issues.

Cleanup payload validation tracing spans

5 participants