Skip to content

Conversation

@syphar
Copy link
Member

@syphar syphar commented Nov 3, 2025

Here I'm trying again to fix this.

Problem is: when we emit log-messages from inside the stream, or sentry errors, these don't contain any information from the parent spans. For example on this sentry we can't really see which crate is affected.

The initially obvious solution was #2874, where I use Span::enter and the guard object.

Some reading and testing made me understand where the memory leak was coming from. Generally the guard object coming from Span::enter should never be held across await-points. Which I did since I was holding onto it for the whole stream.

The solution is to use the async-capable alternative, which is annotating the future with .instrument(span). The new tracing-futures dependency is needed so we can directly annotate the stream.

Additionally I did forget to pass the span info into our render_in_threadpool clojures.

@syphar syphar self-assigned this Nov 3, 2025
@github-actions github-actions bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Nov 3, 2025
@syphar syphar marked this pull request as ready for review November 3, 2025 00:17
@syphar syphar requested a review from a team as a code owner November 3, 2025 00:17
@syphar syphar force-pushed the request-stream-tracing-context branch 2 times, most recently from 4904cc3 to 1b9711b Compare November 3, 2025 05:46
@GuillaumeGomez
Copy link
Member

Feel free to merge once merge conflicts have been fixed.

@syphar syphar force-pushed the request-stream-tracing-context branch from 1b9711b to b8cf593 Compare November 3, 2025 23:09
@syphar syphar merged commit de318c5 into rust-lang:master Nov 4, 2025
8 checks passed
@syphar syphar deleted the request-stream-tracing-context branch November 4, 2025 06:56
@github-actions github-actions bot added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Nov 4, 2025
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants