Don't trace tasks spawned through the console server #314
Conversation
|
(Now that tokio-rs/tracing#2001 is in a release, console could also require that release instead of the NoSubscriber wrapper) |
|
I kinda think the task being shown is useful, since it could help you realize how much the console is affecting your app, or reveal potential bugs in the console server. But I understand wanting to hide it usually. I don't know what the best solution is. Maybe we can dim it, or have a thing at the bottom "2 tasks hidden by default", or some other idea? |
|
I could provide a way to make this configurable, I would put it in Aside: Having hyper tasks coming from the subscriber definitely made the console unusable for me at release. Currently tasks like this cannot be distinguished if the app also uses hyper, though better context annotations will hopefully be provided at some point. Figuring out console bugs or overheads is more relevant to a tokio-console developer, someone also more able to find and configure an "enable_self_trace" flag. |
|
I agree that it feels a bit philosophically wrong to totally exclude the console's tasks, because then we are not reporting a completely accurate view of the application's real state. But, pragmatically, if the console tasks make using the console more difficult, I'm willing to consider excluding them --- doing so may also improve performance and/or reduce dropped events. So, I agree with @seanmonstar --- we should allow the user to choose whether the console's tasks are recorded or not. I think in an ideal world, we would want to configure whether or not the console's tasks are recorded in the console UI, using a general-purpose runtime filtering system as described in #131. However, implementing that is a pretty big project --- it would require UI work as well as potential changes to the wire protocol and |
There was a problem hiding this comment.
I think that, rather than always disabling recording on the console-subscriber thread, we should add a builder option to control whether or not it's disabled, and only set NoSubscriber when that builder option is set. I would be fine with making the console-subscriber thread disabled by default, but I think it ought to be possible to enable recording it.
Also, I commented with a couple smaller suggestions, but neither of those are hard blockers for merging this branch.
g2p
force-pushed
the
no-self-trace
branch
3 times, most recently
from
206d196 to
42a8afe
Compare
|
Addressed comments and made this configurable. |
|
Edit: never mind, I took care of it. |
Enabling the console subscriber starts a server that generates a lot of async activity. Introspecting or logging that can drown out the activity the program has outside of instrumentation. Provide a new builder method, enable_self_trace, to control whether to also trace events that come with enabling the tokio console. If self trace is not enabled, set a thread-local subscriber that drops events coming from the server thread. Raise the requirement on tracing-core so that setting a per-thread NoSubscriber is effective.
|
(I just rebased to appease CI. Feel free to merge when that works) |
Head branch was pushed to by a user without write access
<a name="0.1.4"></a> ## 0.1.4 (2022-04-11) #### Bug Fixes * fix memory leak from historical `PollOp`s (#311) ([9178ecf](9178ecf), closes [#256](256)) #### Features * **console-api:** Update `tonic` to `0.7` (#318) ([83d8a87](83d8a87)) * don't trace tasks spawned through the console server (#314) ([0045e9b](0045e9b))
The console subscriber's server generates a lot of async activity,
introspecting or logging that drowns out the activity the program
has outside of instrumentation.
Set a thread-local subscriber that drops events coming from the server thread.
Implementation requires a simple wrapper for NoSubscriber, because the dispatch implementation uses it as a sentinel value (tokio-rs/tracing#2001 explains the history) and logs through the global dispatcher when a thread-local NoSubscriber is set.