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

Request tracing without task local storage #3251

Merged
merged 8 commits into from
Jan 24, 2025

Conversation

MartinquaXD
Copy link
Contributor

Description

Alternative to #3247

All of the context still applies to this PR.
But instead of looking up the request id from the task local storage we are now able to look it up from the tracing span.
When I introduced the request id feature I already wanted to have it work this way but I was not able to get it working.

Finally after some more research I figured out that this can be accomplished by implementing a custom tracing layer which makes the request id available for lookup later on.

Now the request id is easily available wherever a the current span or any of it's parents contains the request id.
This means we no longer have to worry about 2 different ways of making important data available when spawning a new task.

Changes

  • reworked request_id handling to use a tracing layer instead of task local storage
  • piped the auction id as a request id into the ethrpc http transport layer on /settle calls so we can associate rpc proxy logs with the solution we tried so submit
  • since the driver now has an internal settle queue the additional indirection of spawning a new task in the /settle handler is no longer needed => I removed it to make the code simpler

How to test

Ran local_node test with extra logging to make sure the request ids arrive in the http transport layer with and without request buffering. Since I used the e2e test for multiple winners again we try to submit 2 solutions.
without buffering (applies to mevblocker):

2025-01-22T10:07:46.274Z ERROR request{id="554"}:/settle{solver="test_solver" auction_id=554}:mempool{kind="Mempool(MEVBlocker)"}: ethrpc::http: metadata="554" call="eth_sendRawTransaction"
...
2025-01-22T10:07:46.277Z ERROR request{id="554"}:/settle{solver="solver2" auction_id=554}:mempool{kind="Mempool(MEVBlocker)"}: ethrpc::http: metadata="554" call="eth_sendRawTransaction"

with buffering (applies to public mempool submissions):

2025-01-22T10:02:10.347Z ERROR ethrpc::http::metadata="548:eth_sendRawTransaction(0,1)"

Also added unit tests to show that we can correctly get the request_id from the tracing span under various conditions.

@MartinquaXD MartinquaXD requested a review from a team as a code owner January 22, 2025 10:45
Copy link
Contributor

@mstrug mstrug left a comment

Choose a reason for hiding this comment

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

Only 2 comments.

crates/observe/src/request_id.rs Show resolved Hide resolved
crates/observe/src/request_id.rs Outdated Show resolved Hide resolved
@MartinquaXD MartinquaXD merged commit 589e52d into main Jan 24, 2025
11 checks passed
@MartinquaXD MartinquaXD deleted the request-tracing-without-task-local-storage branch January 24, 2025 12:50
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants