-
Notifications
You must be signed in to change notification settings - Fork 131
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
Add console-subscriber #545
Add console-subscriber #545
Conversation
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.
Reviewable status: 0 of 1 LGTMs obtained, and pending CI: Bazel Dev / large-ubuntu-22.04, Local / large-ubuntu-22.04, Remote / large-ubuntu-22.04, asan / large-ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), large-ubuntu-20.04, large-ubuntu-22.04
a discussion (no related file):
I have played with this too and think we can enable this using a feature. We can just use #[cfg(feature = enable_tokio_console])
or something on the few additional lines of code.
f092185
to
b1602b3
Compare
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.
Reviewable status: 0 of 1 LGTMs obtained
a discussion (no related file):
Previously, allada (Nathan (Blaise) Bruer) wrote…
I have played with this too and think we can enable this using a feature. We can just use
#[cfg(feature = enable_tokio_console])
or something on the few additional lines of code.
Added feature flag. Updated the github description with an example but that isn't being reflective in reviewable for some reason.
Done.
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.
Reviewed 4 of 4 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: 0 of 1 LGTMs obtained
-- commits
line 5 at r3:
nit: Maybe this is a reviewable bug but this looks like it's a second commit.
Cargo.toml
line 50 at r2 (raw file):
tracing = "0.1.40" tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } console-subscriber = { version = "0.2.0" }
nit: The other imports are sorted alphabetically
src/bin/cas.rs
line 673 at r2 (raw file):
.with_thread_names(true) .with_filter(env_filter), )
nit: This looks like it could be deduplicated with a let fmt_layer = tracing_subscriber::fmt::layer()....with_env_filter(env_filter)
.
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.
Reviewable status: 1 of 1 LGTMs obtained
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.
Reviewable status: 1 of 1 LGTMs obtained
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.
Reviewable status: 1 of 1 LGTMs obtained
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.
Reviewable status: 1 of 1 LGTMs obtained
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: Maybe this is a reviewable bug but this looks like it's a second commit.
I foobar'd something, not a reviewable bug
src/bin/cas.rs
line 673 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: This looks like it could be deduplicated with a
let fmt_layer = tracing_subscriber::fmt::layer()....with_env_filter(env_filter)
.
I spent a lot of time trying to wedge that in, for some reason tracing_subscriber::fmt()
builder does not seem to take a Layer
or Layered
(forget which one is produced). Instead I figured we might revisit this code if/when we add span tracing or other instrumentation features from tokio. Also gives me personally a bit more time to dig into their docs, its fairly rich but also seems this side of stuff isn't near a 1.0.0 yet.
210d0e7
to
f4d989f
Compare
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.
Reviewable status: 1 of 1 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / ubuntu-22.04, Local / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), pre-commit-checks, publish-image, ubuntu-20.04, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable
Previously, adam-singer (Adam Singer) wrote…
I foobar'd something, not a reviewable bug
Done.
Cargo.toml
line 50 at r2 (raw file):
Previously, aaronmondal (Aaron Siddhartha Mondal) wrote…
nit: The other imports are sorted alphabetically
Done.
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.
Reviewable status: 1 of 1 LGTMs obtained, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / ubuntu-22.04, Cargo Dev / ubuntu-22.04, Local / ubuntu-22.04, Remote / large-ubuntu-22.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (20.04), docker-compose-compiles-nativelink (22.04), integration-tests (20.04), integration-tests (22.04), pre-commit-checks, publish-image, ubuntu-20.04, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable
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.
Reviewed 6 of 6 files at r4, all commit messages.
Reviewable status:complete! 1 of 1 LGTMs obtained
Description
console-subscriber provides an instrumentation layer for async tokio applications. console provides viewing tasks, resources and histogram information.
Tokio instrumentation is in unstable phase and requires additional build/rust flags to be enabled. These configuration are not enabled unless explicitly opted into, thus disabled by default in production/dev. Use the
tokio_unstable
cfg andenable_tokio_console
features flags to enabled.Example
Run native link with basic cas config.
New terminal (same host) to launch the tokio console application. See tokio-console for details on how to run.
NOTE: Skipping documentation on instrumentation util proper place is found and other tips/tooling available, such as spans and profiling.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
not work as expected)
How Has This Been Tested?
Please also list any relevant details for your test configuration
Checklist
bazel test //...
passes locallygit amend
see some docsThis change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)