-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Add support for SpanTrace capture in ICE reports #103993
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,20 +45,32 @@ | |
use std::env::{self, VarError}; | ||
use std::fmt::{self, Display}; | ||
use std::io::{self, IsTerminal}; | ||
use std::str::FromStr; | ||
use tracing_core::{Event, Subscriber}; | ||
use tracing_subscriber::filter::{Directive, EnvFilter, LevelFilter}; | ||
use tracing_subscriber::fmt::{ | ||
format::{self, FormatEvent, FormatFields}, | ||
FmtContext, | ||
}; | ||
use tracing_subscriber::layer::SubscriberExt; | ||
use tracing_subscriber::Layer; | ||
|
||
pub fn init_env_logger(env: &str) -> Result<(), Error> { | ||
let filter = match env::var(env) { | ||
/// In contrast to `init_rustc_env_logger` this allows you to choose an env var | ||
/// other than `RUSTC_LOG`. | ||
pub fn init_env_logger(env: &str, ice_env: &str) -> Result<(), Error> { | ||
let log_filter = match env::var(env) { | ||
Ok(env) => EnvFilter::new(env), | ||
_ => EnvFilter::default().add_directive(Directive::from(LevelFilter::WARN)), | ||
}; | ||
|
||
let error_filter = match env::var(ice_env) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should just enable all levels by default for the spantrace. if people see it as a problem we can always offer ways to reduce it. My use case would always unconditionally enable trace spans for ICEs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was just worried about causing perf issues, but that seems easy enough to test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, so I tried this a while back and ran into some errors and ended up putting it down. Picked it back up, rebased, dealt with merge conflicts and got it working again and looked at the error in some more detail with @jyn514 and we came to the conclusion that it seems like it's a pre-existing error that I'm triggering but not causing. Jyn felt like they recall encountering the same error in the past. I want to do some more testing to verify this theory and see if I can repro the issue without having tracing_error installed by enabling TRACE capture on all spans with a custom registry layer. I recall testing this in november and it works at trace level when reporting errors during compilation of other crates, it's only when it's compiling itself with trace enabled on spans that it's running into a panic from a diagnostic not being emitted in time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh, then let's land it as is, so we can debug it on master. |
||
Ok(env) => tracing::Level::from_str(&env).unwrap(), | ||
_ => tracing::Level::WARN, | ||
}; | ||
let error_filter = tracing_subscriber::filter::filter_fn(move |metadata| { | ||
metadata.is_span() && metadata.level() <= &error_filter | ||
}); | ||
|
||
let color_logs = match env::var(String::from(env) + "_COLOR") { | ||
Ok(value) => match value.as_ref() { | ||
"always" => true, | ||
|
@@ -75,7 +87,7 @@ pub fn init_env_logger(env: &str) -> Result<(), Error> { | |
Some(v) => &v != "0", | ||
}; | ||
|
||
let layer = tracing_tree::HierarchicalLayer::default() | ||
let log_layer = tracing_tree::HierarchicalLayer::default() | ||
.with_writer(io::stderr) | ||
.with_indent_lines(true) | ||
.with_ansi(color_logs) | ||
|
@@ -84,9 +96,12 @@ pub fn init_env_logger(env: &str) -> Result<(), Error> { | |
.with_verbose_entry(verbose_entry_exit) | ||
.with_indent_amount(2); | ||
#[cfg(parallel_compiler)] | ||
let layer = layer.with_thread_ids(true).with_thread_names(true); | ||
let log_layer = log_layer.with_thread_ids(true).with_thread_names(true); | ||
let error_layer = tracing_error::ErrorLayer::default(); | ||
let subscriber = tracing_subscriber::Registry::default() | ||
.with(error_layer.with_filter(error_filter)) | ||
.with(log_layer.with_filter(log_filter)); | ||
|
||
let subscriber = tracing_subscriber::Registry::default().with(filter).with(layer); | ||
match env::var(format!("{env}_BACKTRACE")) { | ||
Ok(str) => { | ||
let fmt_layer = tracing_subscriber::fmt::layer() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -677,6 +677,7 @@ impl Session { | |
|| self.opts.unstable_opts.unpretty.is_some() | ||
|| self.opts.output_types.contains_key(&OutputType::Mir) | ||
|| std::env::var_os("RUSTC_LOG").is_some() | ||
|| std::env::var_os("RUSTC_ICE_LOG").is_some() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jyn514 identified the issue with defaulting the log level to This way we can leave actually increasing the usage of this functionality to later PRs where it can be based on more practical usage experience. |
||
{ | ||
return; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
// edition:2021 | ||
// known-bug: unknown | ||
// unset-rustc-env:RUST_BACKTRACE | ||
// rustc-env:RUSTC_ICE_LOG=trace | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted to see one that actually captured some SpanTrace output just to make sure it's working and i understood why the other ones are empty. I'm going to add some logic to skip adding the "SpanTrace:\n" bit if the SpanTrace itself is empty. I'm also wondering if I should add some There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test suddenly started failing after rebasing on master and I do not yet know why I tried building latest master to see if it was a bug that just happened to get through and it failed two entirely unrelated tests instead... |
||
// compile-flags:-Z trait-solver=chalk | ||
// error-pattern:internal compiler error | ||
// failure-status:101 | ||
|
Uh oh!
There was an error while loading. Please reload this page.