-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[ENH]: add tracing support for tokio console #5239
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
d7770af
to
c3c0fd9
Compare
@@ -0,0 +1,3 @@ | |||
[build] | |||
# Needed to enable tracing support for tokio | |||
rustflags = ["--cfg", "tokio_unstable"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my understanding of this flag is that it only enables opt-in features that are subject to change without corresponding semver changes
2 Jobs Failed: PR checks / Python tests / test-cluster-rust-frontend (3.9, chromadb/test/property/test_add.py)No logs available for this step. PR checks / all-required-pr-checks-passedNo logs available for this step. Summary: 1 successful workflow, 1 failed workflow
Last updated: 2025-08-12 00:03:05 UTC |
c3c0fd9
to
07903b0
Compare
Add Tokio Console Tracing Support and Refactor Tracing Initialization This PR adds support for the Key Changes• Adds Affected Areas• Rust workspace dependency management (Cargo.toml, Cargo.lock) This summary was automatically generated by @propel-code-bot |
07903b0
to
5d802d2
Compare
|
||
let subscriber = tracing_subscriber::registry() | ||
.with(layers.with_filter(get_env_filter(custom_filters))) | ||
.with( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be config? we don't really use env vars elsewhere but i could see it maybe being fine here. is this a standardized env var?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong preference
it's only standardized in the sense it's the same across all our services
common debug config loosely makes sense to me to have in env vars, but maybe we should just define a new common tracing config struct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, in that case i would ask that it is config since our config system supports env var overrides, and having a single source of truth is preferable.
fmt::layer().pretty().with_target(false).boxed() | ||
} | ||
|
||
pub fn init_tracing(layers: Vec<Box<dyn Layer<Registry> + Send + Sync>>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does hosted/ depend on this code? It should be checked for dep issues, with a fix PR being up before this is merged
|
||
tracing::subscriber::set_global_default(subscriber) | ||
.expect("Should be able to set global tracing subscriber"); | ||
tracing::info!("Global tracing subscriber set"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any parent span that will actually capture this info?
Description of changes
Adds support for tokio-console to all services. This tool is useful when debugging, e.g. it should make it easy to see what futures are "stuck" / waiting on locks. It's disabled by default and can be enabled with an env var.
Test plan
How are these changes tested?
I port-forwarded 6669 from a service in Tilt to my host, then ran the
tokio-console
application and observed that data was flowing from the service to the tokio console.Migration plan
Are there any migrations, or any forwards/backwards compatibility changes needed in order to make sure this change deploys reliably?
Observability plan
What is the plan to instrument and monitor this change?
Documentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs section?