Skip to content

Automatically pass spans into blocking handles - #8158

Merged
mergify[bot] merged 7 commits into
sigp:unstablefrom
eserilev:pass-spans-into-blocking-handles
Apr 1, 2026
Merged

Automatically pass spans into blocking handles#8158
mergify[bot] merged 7 commits into
sigp:unstablefrom
eserilev:pass-spans-into-blocking-handles

Conversation

@eserilev

@eserilev eserilev commented Oct 2, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Currently we have to manually pass spans across threads (i.e. during spawn_blocking_handle invocations). This PR ensures that the current span is always passed across threads for:

  • spawn_blocking_handle
  • spawn_blocking_with_rayon_async
  • spawn_blocking_with_rayon

If there is no current span, nothing will happen

@eserilev eserilev added work-in-progress PR is a work-in-progress code-quality ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Oct 2, 2025
@mergify

mergify Bot commented Oct 7, 2025

Copy link
Copy Markdown

Some required checks have failed. Could you please take a look @eserilev? 🙏

@mergify mergify Bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Oct 7, 2025
@eserilev
eserilev force-pushed the pass-spans-into-blocking-handles branch from 96975a7 to 1943507 Compare October 7, 2025 23:40
eserilev
eserilev commented Oct 7, 2025
View reviewed changes
Comment thread beacon_node/lighthouse_tracing/src/lib.rs Outdated
eserilev
eserilev commented Oct 7, 2025
View reviewed changes
Comment thread beacon_node/beacon_chain/src/beacon_chain.rs
@eserilev eserilev added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Oct 7, 2025
jimmygchen
jimmygchen reviewed Oct 30, 2025
View reviewed changes
Comment thread beacon_node/beacon_chain/src/beacon_chain.rs
jimmygchen
jimmygchen reviewed Oct 30, 2025
View reviewed changes
Comment thread beacon_node/beacon_chain/src/beacon_chain.rs
jimmygchen
jimmygchen reviewed Oct 30, 2025
View reviewed changes
Comment thread beacon_node/beacon_chain/src/beacon_chain.rs
jimmygchen
jimmygchen reviewed Oct 30, 2025
View reviewed changes
Comment thread beacon_node/beacon_chain/src/beacon_chain.rs
jimmygchen
jimmygchen reviewed Oct 30, 2025
View reviewed changes
Comment thread beacon_node/beacon_chain/src/beacon_chain.rs
jimmygchen
jimmygchen reviewed Oct 30, 2025
View reviewed changes
Comment thread beacon_node/beacon_chain/src/canonical_head.rs
jimmygchen
jimmygchen reviewed Oct 30, 2025
View reviewed changes
Comment thread beacon_node/http_api/src/publish_blocks.rs
jimmygchen
jimmygchen requested changes Oct 30, 2025
View reviewed changes

@jimmygchen jimmygchen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, i think this definitely makes it easier to instrument code that spawns a blocking thread!

I wasn't sure what was the intention of removing some of the spans though, were you thinking to create them in the spawn functions, because we'll lose some traces here.

@jimmygchen jimmygchen added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Oct 30, 2025
@mergify

mergify Bot commented Nov 29, 2025

Copy link
Copy Markdown

Hi @eserilev, this pull request has been closed automatically due to 30 days of inactivity. If you’d like to continue working on it, feel free to reopen at any time.

@mergify mergify Bot closed this Nov 29, 2025
@mergify mergify Bot added the stale Stale PRs that have been inactive and is now outdated label Nov 29, 2025
@jimmygchen jimmygchen reopened this Jan 8, 2026
@jimmygchen

jimmygchen commented Jan 8, 2026

Copy link
Copy Markdown
Member

Hi @eserilev would you mind looking at the review comments and let me know if they make sense?
I think it'd still be useful to automatically pass spans into blocking handles.

@jimmygchen jimmygchen self-assigned this Jan 12, 2026
@jimmygchen

jimmygchen commented Jan 12, 2026

Copy link
Copy Markdown
Member

Thanks @eserilev I'll review this today!

@jimmygchen

jimmygchen commented Feb 24, 2026

Copy link
Copy Markdown
Member

@eserilev sorry for leaving this PR stale again 😂

I'll work on getting this up to date again - there are a few calls in gloas.rs that needs update as well.

@jimmygchen
Merge remote-tracking branch 'origin/unstable' into pass-spans-into-b…
b630aa1 
…locking-handles

# Conflicts:
#	beacon_node/beacon_chain/src/beacon_chain.rs
#	beacon_node/beacon_chain/src/canonical_head.rs
#	beacon_node/beacon_chain/src/fetch_blobs/mod.rs
@jimmygchen
Remove redundant manual span passing in gloas files
80dddd3
@jimmygchen
Merge branch 'unstable' into pass-spans-into-blocking-handles
6134ace
jimmygchen
jimmygchen approved these changes Mar 30, 2026
View reviewed changes

@jimmygchen jimmygchen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've merged unstable into this branch and applied the changes on a few new code paths. The remaining changes look good!

@eserilev would you mind reviewing my latest changes before we merge this?

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Mar 31, 2026
@mergify mergify Bot added the queued label Mar 31, 2026
@mergify

mergify Bot commented Mar 31, 2026 •
edited
Loading

Copy link
Copy Markdown

Merge Queue Status

This pull request spent 58 minutes 19 seconds in the queue, including 56 minutes 20 seconds running CI.

Required conditions to merge

Reason

The merge conditions cannot be satisfied due to failing checks

Failing checks:

Hint

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

mergify Bot added a commit that referenced this pull request Mar 31, 2026
@mergify
Merge of #8158
e31d47c
@mergify mergify Bot mentioned this pull request Mar 31, 2026
Closed
10 tasks
mergify Bot added a commit that referenced this pull request Apr 1, 2026
@mergify
Merge of #8158
f3b05a1
mergify Bot added a commit that referenced this pull request Apr 1, 2026
@mergify
Merge of #8158
c7c18de
This was referenced Apr 1, 2026
Merged
Closed
Merged
Closed
@mergify mergify Bot added dequeued and removed queued labels Apr 1, 2026
@jimmygchen

jimmygchen commented Apr 1, 2026

Copy link
Copy Markdown
Member

@mergify requeue

@mergify

mergify Bot commented Apr 1, 2026 •
edited
Loading

Copy link
Copy Markdown

Merge Queue Status

This pull request spent 29 minutes 29 seconds in the queue, including 28 minutes 16 seconds running CI.

Required conditions to merge

@mergify mergify Bot added queued and removed dequeued labels Apr 1, 2026
mergify Bot added a commit that referenced this pull request Apr 1, 2026
@mergify
Merge of #8158
1138233
@mergify mergify Bot mentioned this pull request Apr 1, 2026
Closed
10 tasks
@mergify
mergify Bot merged commit 99f5a92 into sigp:unstable Apr 1, 2026
38 checks passed
@mergify mergify Bot removed the queued label Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jimmygchen jimmygchen jimmygchen approved these changes
@pawanjay176 pawanjay176 pawanjay176 approved these changes

Assignees

@jimmygchen jimmygchen

Labels

code-quality ready-for-merge This PR is ready to merge. tracing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@eserilev @jimmygchen @pawanjay176