Uh oh!
There was an error while loading. Please reload this page.
Move from log to tracing - #74726
Conversation
rust-highfive
commented
Jul 24, 2020
(rust_highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
The replacements seem like they could be pretty confusing, so we should try to move off of them pretty quickly, but I agree that for now this is probably the right way to do things, presuming it doesn't cause problems with tracing's own macros expanding to ::tracing::foo and that not resolving.
There was a problem hiding this comment.
I agree, I'll open an E-easy issue with checkboxes for each of the renames
There was a problem hiding this comment.
Tracing's macros should all use $crate::, so this should be fine.
Mark-Simulacrum
commented
Jul 24, 2020
Once CI is passing here, could you kick off a perf run? I want to make sure this isn't a performance regression (which would be unexpected, but we should check). |
jyn514
commented
Jul 24, 2020
Looks like rustdoc had a dependency on |
Mark-Simulacrum
commented
Jul 24, 2020
(that is actually good - any tools that have shared dependencies with rustc should ideally not declare them to save time in CI as loading from the sysroot is free, compared to recompiling) |
oli-obk
commented
Jul 25, 2020
@bors try @rust-timer queue |
rust-timer
commented
Jul 25, 2020
Awaiting bors try build completion |
bors
commented
Jul 25, 2020
⌛ Trying commit 2f7afb6d9058b3ebf7a059517ddc231da059ef0d with merge ec225facb960eff32c5290531b2dced3da219bdb... |
rust-log-analyzer
commented
Jul 25, 2020
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
bors
commented
Jul 25, 2020
💔 Test failed - checks-actions |
oli-obk
commented
Jul 25, 2020
@bors try @rust-timer queue this time |
rust-timer
commented
Jul 25, 2020
Awaiting bors try build completion |
bors
commented
Jul 25, 2020
⌛ Trying commit b9a3fd38bef4e62704cfefda80ccf67a815700d1 with merge 46c33324e985772632898d0f4c1b6d4c246496ec... |
bors
commented
Jul 25, 2020
☀️ Try build successful - checks-actions, checks-azure |
rust-timer
commented
Jul 25, 2020
Queued 46c33324e985772632898d0f4c1b6d4c246496ec with parent 037d8e7, future comparison URL. |
mark-i-m
commented
Jul 25, 2020
Could you also make a PR to update the rustc-dev-guide, specifically with how to turn on and use logging? There is some existing content on how to enable the current logging system. |
rust-timer
commented
Jul 25, 2020
Finished benchmarking try commit (46c33324e985772632898d0f4c1b6d4c246496ec): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
@hawkw any tips on debugging perf regressions due to migrating from EDIT 1: I believe this is all working correctly, as looking at a const-eval heavy test's perf result (https://perf.rust-lang.org/detailed-query.html?commit=46c33324e985772632898d0f4c1b6d4c246496ec&base_commit=037d8e747de5056f0202f29aa2b0353bdbbf5cfe&benchmark=wf-projection-stress-65510-check&run_name=full) still has a significant number of EDIT 2: I'm doing what you suggested originally now (I think?), and enabling the EDIT 3: Oh, that doesn't work with EDIT 4: Maybe we're losing perf because we stay compatible to |
oli-obk
commented
Jul 26, 2020
@bors try @rust-timer queue |
rust-timer
commented
Jul 26, 2020
Awaiting bors try build completion |
bors
commented
Jul 26, 2020
⌛ Trying commit e301587481705f5dd14df480a9f76c2432e3ad8e with merge 1b5adbdbb4a5be604061874ffc2dade72bf7ede0... |
bors
commented
Jul 26, 2020
☀️ Try build successful - checks-actions, checks-azure |
| /// other than `RUSTC_LOG`. | ||
| pub fn init_env_logger(env: &str) { | ||
| // Don't register a dispatcher if there's no filter to print anything | ||
| match std::env::var(env) { |
There was a problem hiding this comment.
what would be the difference in this case?
There was a problem hiding this comment.
hmm true, let's change it
jyn514
commented
Aug 3, 2020
This broke my logging :( How do I get back the behavior of |
|
jyn514
commented
Aug 3, 2020
hmm, this works for |
jyn514
commented
Aug 3, 2020
This fixes it for me: diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs
index 002c5f96710..4f08452767a 100644
--- a/src/librustdoc/lib.rs+++ b/src/librustdoc/lib.rs@@ -43,7 +43,7 @@ extern crate rustc_trait_selection;
extern crate rustc_typeck;
extern crate test as testing;
#[macro_use]
-extern crate log;+extern crate tracing as log;
use std::default::Default;
use std::env;Other tools might need a similar fix. |
memoryruins
commented
Aug 3, 2020
Sure, #74747 is the end goal and if tools want to do that first that's fine. But in the meantime logging is completely broken for rustdoc. |
memoryruins
commented
Aug 3, 2020
👍 #75081 is a reasonable patch until rustdoc is ready for source renaming |
Mark-Simulacrum
commented
Aug 3, 2020
hawkw
commented
Aug 3, 2020
Yay! I'm really glad we were able to get this merged, and the optimizations should benefit other Please feel free to ping me if anyone has additional questions about migrating to |
hawkw
commented
Aug 11, 2020
FWIW, it might be worth trying this again with the latest |
The only visible change is that we now get timestamps in our logs:
This PR was explicitly designed to be as low-impact as possible. We can now move to using the name
tracinginsteads oflogon a crate-by-crate basis and use any of the other tracing features where desirable.As far as I can tell this will allow tools to seamlessly keep working (since they are using
rustc_driver::init_log...).This is the first half of step 1 of the accepted
tracingMCP (rust-lang/compiler-team#331)