Skip to content

fix: use spans properly in submit task #112

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

Closed
wants to merge 4 commits into from
Closed

Conversation

dylanlott
Copy link
Contributor

@dylanlott dylanlott commented Jun 16, 2025

fix: use spans properly in submit task

This PR fixes the panicking span in the submit task.

Previously, there was a panic caused by incorrect handling of the span in the retry loop.

Tested and confirmed that spans are tracing as expected in devnet with this fix.

# image and tag containing the patch under test
image sha256:932cc18313df7c894e20641a444b61a2cb541f44bc85627a30604b71219c90f9                                             0.0s
docker.io/library/quincey-builder:debug

This branch is deployed in dev-net at commit 415baae17f0194bb39f3f5825b9d5473f27a3ece.

Closes ENG-1128

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@dylanlott dylanlott self-assigned this Jun 16, 2025
@dylanlott dylanlott added the bug Something isn't working label Jun 16, 2025 — with Graphite App
@dylanlott dylanlott marked this pull request as ready for review June 16, 2025 22:22
.into_future()
.instrument(span.clone())
.await
let prev_host_number = host_block_number - 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a drive-by or was it related to the panic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's another bug, actually, just was part of the same PR. I think it's the reason why the builder isn't submitting blocks in dev-net right now. I will build and submit this into a new PR since the span fixes are addressed in #113.

Copy link
Member

@prestwich prestwich left a comment

Choose a reason for hiding this comment

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

this isnt fixing, its just removing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants