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 optional support for tokio-console #318
Conversation
9576383
to
1eac31b
Compare
Fixed the first CI failure by downgrading the compile error when you try to combine the The second CI failure is on |
Huh... I did not expect CI to pass on this branch. The error that I get locally looks like this:
|
This turned out to be quite hairy, mostly because we need to apply the config's log level filter to the actual logs (stdout and, optionally sentry), but do not want to filter out the tokio tracing events needed by the console_subscriber. I hit several edge cases in tracing getting this to work, and we now depend on a git version of tracing with a backported patch :(
1eac31b
to
2029b89
Compare
Rebased on current |
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.
If we find ourselves maintaining the tracing fork in a month now, we'll look into alternatives
@girlbossceo you may want to copy my tracing branch into your github account to match the other git dependencies. The tracing change that we need is tokio-rs/tracing#2956. I made this PR against the master branch, and don't know how likely it is that it will be backported to the 0.1.x branch by upstream tracing. If not, we might be maintaining the fork for a long time. If we don't want to do this, there are alternative ways to get the
LogLevelReloadHandles
wrapper to work, but they are ugly.The refactor of how the reload handle works also enables fixing the current behavior where enabling
allow_jaeger
ortracing_flame
will disable the normal (and sentry) logging, but I'd like to make that change in a separate PR.