Skip to content

Conversation

@squadgazzz
Copy link
Contributor

@squadgazzz squadgazzz commented Dec 8, 2025

Description

It turned out that tokio_unstable introduces a memory leak when creating spans. As @MartinquaXD noticed:

Most of the tracing related memory was allocated while holding a tokio mutex lock. When you check the tokio code you see that this only happens when we build it with tokio_unstable which is only used for tokio-console support.

Disabling the tokio_unstable helped to mitigate the memory leak issue almost completely.

Changes

  • Remove tokio_unstable from the codebase.
  • Keep it only in the playground (for now).

Further implementation

Using the updated deployment CI job from #3954, the tokio-console can be gated behind a feature in case we need to run it in the future. Will implement it in a follow-up pr.

@squadgazzz squadgazzz marked this pull request as ready for review December 9, 2025 11:14
@squadgazzz squadgazzz requested a review from a team as a code owner December 9, 2025 11:14
Copilot AI review requested due to automatic review settings December 9, 2025 11:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a memory leak issue caused by tokio_unstable by removing it from the global build configuration and restricting tokio-console support to the playground environment only. The changes introduce a feature flag approach that allows tokio-console to be enabled on-demand rather than globally.

  • Removed global tokio_unstable configuration from .cargo/config.toml
  • Converted console-subscriber to an optional dependency gated behind a tokio-console feature flag
  • Updated playground Dockerfile to explicitly enable tokio-console feature for all packages

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.cargo/config.toml Removed global tokio_unstable rustflag configuration to prevent memory leaks in production
crates/observe/Cargo.toml Made console-subscriber optional and added tokio-console feature flag
crates/observe/build.rs Added clarifying comments about tokio_unstable usage in playground
crates/observe/src/tracing.rs Refactored tokio-console initialization to use conditional compilation with both tokio_unstable cfg and tokio-console feature flag
playground/Dockerfile Added --features observe/tokio-console to all cargo build commands to enable tokio-console in playground environment

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

let enable_tokio_console: bool = std::env::var("TOKIO_CONSOLE")
.unwrap_or("false".to_string())
.parse()
.unwrap();
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The .unwrap() call could panic if the TOKIO_CONSOLE environment variable is set to an invalid value (e.g., "maybe", "1", "yes"). Consider using .unwrap_or(false) or .unwrap_or_else(|_| false) to handle parsing errors gracefully, defaulting to false when the value cannot be parsed as a boolean.

Suggested change
.unwrap();
.unwrap_or(false);

Copilot uses AI. Check for mistakes.
@squadgazzz squadgazzz added this pull request to the merge queue Dec 9, 2025
Merged via the queue into main with commit 70b17ec Dec 9, 2025
25 checks passed
@squadgazzz squadgazzz deleted the drop-tokio-console-support branch December 9, 2025 14:24
@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 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.

4 participants