Skip to content

Commit

Permalink
subscriber: warn if trying to enable a statically disabled level (#990)
Browse files Browse the repository at this point in the history
## Motivation


Fixes #975.

## Solution

This implements the warning in `tracing-subscriber` only, as mentioned
in
#975 (comment).
It warns whenever directives are parsed, including programmatically and
through environment variables. It does not include the suggested new API
which returns the filters that wouldn't be parsed.

- List filters that would be ignored
- Mention 'static max level'
- Describe how to enable the logging

Example output:

```
$ RUST_LOG=off,debug,silenced[x]=trace cargo run -q
warning: some trace filter directives would enable traces that are disabled statically
 | `debug` would enable the DEBUG level for all targets
 | `silenced[x]=trace` would enable the TRACE level for the `silenced` target
 = note: the static max level is info
 = note: to enable DEBUG logging, remove the `max_level_info` feature
```
![image](https://user-images.githubusercontent.com/23638587/95243324-77dc6a00-07de-11eb-8ed3-6ee2109940d4.png)
  • Loading branch information
jyn514 authored Oct 6, 2020
1 parent cb1dd95 commit 1d901b3
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 3 deletions.
3 changes: 2 additions & 1 deletion tracing-subscriber/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ keywords = ["logging", "tracing", "metrics", "subscriber"]
[features]

default = ["env-filter", "smallvec", "fmt", "ansi", "chrono", "tracing-log", "json"]
env-filter = ["matchers", "regex", "lazy_static"]
env-filter = ["matchers", "regex", "lazy_static", "tracing"]
fmt = ["registry"]
ansi = ["fmt", "ansi_term"]
registry = ["sharded-slab", "thread_local"]
Expand All @@ -34,6 +34,7 @@ json = ["tracing-serde", "serde", "serde_json"]
tracing-core = { path = "../tracing-core", version = "0.2" }

# only required by the filter feature
tracing = { optional = true, path = "../tracing", version = "0.2" }
matchers = { optional = true, version = "0.0.1" }
regex = { optional = true, version = "1", default-features = false, features = ["std"] }
smallvec = { optional = true, version = "1" }
Expand Down
4 changes: 2 additions & 2 deletions tracing-subscriber/src/filter/env/directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ use tracing_core::{span, Level, Metadata};
// TODO(eliza): add a builder for programmatically constructing directives?
#[derive(Debug, Eq, PartialEq)]
pub struct Directive {
target: Option<String>,
in_span: Option<String>,
fields: FilterVec<field::Match>,
level: LevelFilter,
pub(crate) target: Option<String>,
pub(crate) level: LevelFilter,
}

/// A directive which will statically enable or disable a given callsite.
Expand Down
90 changes: 90 additions & 0 deletions tracing-subscriber/src/filter/env/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,96 @@ impl EnvFilter {
}

fn from_directives(directives: impl IntoIterator<Item = Directive>) -> Self {
use tracing::level_filters::STATIC_MAX_LEVEL;
use tracing::Level;

let directives: Vec<_> = directives.into_iter().collect();

let disabled: Vec<_> = directives
.iter()
.filter(|directive| directive.level > STATIC_MAX_LEVEL)
.collect();

if !disabled.is_empty() {
#[cfg(feature = "ansi_term")]
use ansi_term::{Color, Style};
// NOTE: We can't use a configured `MakeWriter` because the EnvFilter
// has no knowledge of any underlying subscriber or collector, which
// may or may not use a `MakeWriter`.
let warn = |msg: &str| {
#[cfg(not(feature = "ansi_term"))]
let msg = format!("warning: {}", msg);
#[cfg(feature = "ansi_term")]
let msg = {
let bold = Style::new().bold();
let mut warning = Color::Yellow.paint("warning");
warning.style_ref_mut().is_bold = true;
format!("{}{} {}", warning, bold.clone().paint(":"), bold.paint(msg))
};
eprintln!("{}", msg);
};
let ctx_prefixed = |prefix: &str, msg: &str| {
#[cfg(not(feature = "ansi_term"))]
let msg = format!("note: {}", msg);
#[cfg(feature = "ansi_term")]
let msg = {
let mut equal = Color::Fixed(21).paint("="); // dark blue
equal.style_ref_mut().is_bold = true;
format!(" {} {} {}", equal, Style::new().bold().paint(prefix), msg)
};
eprintln!("{}", msg);
};
let ctx_help = |msg| ctx_prefixed("help:", msg);
let ctx_note = |msg| ctx_prefixed("note:", msg);
let ctx = |msg: &str| {
#[cfg(not(feature = "ansi_term"))]
let msg = format!("note: {}", msg);
#[cfg(feature = "ansi_term")]
let msg = {
let mut pipe = Color::Fixed(21).paint("|");
pipe.style_ref_mut().is_bold = true;
format!(" {} {}", pipe, msg)
};
eprintln!("{}", msg);
};
warn("some trace filter directives would enable traces that are disabled statically");
for directive in disabled {
let target = if let Some(target) = &directive.target {
format!("the `{}` target", target)
} else {
"all targets".into()
};
let level = directive
.level
.clone()
.into_level()
.expect("=off would not have enabled any filters");
ctx(&format!(
"`{}` would enable the {} level for {}",
directive, level, target
));
}
ctx_note(&format!("the static max level is `{}`", STATIC_MAX_LEVEL));
let help_msg = || {
let (feature, filter) = match STATIC_MAX_LEVEL.into_level() {
Some(Level::TRACE) => unreachable!(
"if the max level is trace, no static filtering features are enabled"
),
Some(Level::DEBUG) => ("max_level_debug", Level::TRACE),
Some(Level::INFO) => ("max_level_info", Level::DEBUG),
Some(Level::WARN) => ("max_level_warn", Level::INFO),
Some(Level::ERROR) => ("max_level_error", Level::WARN),
None => return ("max_level_off", String::new()),
};
(feature, format!("{} ", filter))
};
let (feature, earlier_level) = help_msg();
ctx_help(&format!(
"to enable {}logging, remove the `{}` feature",
earlier_level, feature
));
}

let (dynamics, mut statics) = Directive::make_tables(directives);
let has_dynamics = !dynamics.is_empty();

Expand Down

0 comments on commit 1d901b3

Please sign in to comment.