From 8cb716bc22076c9d563d9578eca9828be3d59f69 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sat, 26 Sep 2020 18:07:30 -0400 Subject: [PATCH] subscriber: warn if trying to enable a statically disabled level This implements the warning in `tracing-subscriber` only, as mentioned in https://github.com/tokio-rs/tracing/issues/975#issuecomment-692903469. 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 | the static max level is info note: to enable DEBUG logging, remove the `max_level_info` feature ``` Fixes: #975 --- tracing-core/src/metadata.rs | 4 +-- tracing-subscriber/Cargo.toml | 3 +- .../src/filter/env/directive.rs | 4 +-- tracing-subscriber/src/filter/env/mod.rs | 36 +++++++++++++++++++ 4 files changed, 42 insertions(+), 5 deletions(-) diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index c575a55b45..e2617aea84 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -94,7 +94,7 @@ pub struct Metadata<'a> { pub struct Kind(KindInner); /// Describes the level of verbosity of a span or event. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] pub struct Level(LevelInner); /// A filter comparable to a verbosity `Level`. @@ -107,7 +107,7 @@ pub struct Level(LevelInner); /// addition of an `OFF` level that completely disables all trace /// instrumentation. #[repr(transparent)] -#[derive(Clone, Eq, PartialEq)] +#[derive(Copy, Clone, Eq, PartialEq)] pub struct LevelFilter(Option); /// Indicates that a string could not be parsed to a valid level. diff --git a/tracing-subscriber/Cargo.toml b/tracing-subscriber/Cargo.toml index b1bb448d44..dd177334ff 100644 --- a/tracing-subscriber/Cargo.toml +++ b/tracing-subscriber/Cargo.toml @@ -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"] @@ -34,6 +34,7 @@ json = ["tracing-serde", "serde", "serde_json"] tracing-core = { path = "../tracing-core", version = "0.1.16" } # only required by the filter feature +tracing = { optional = true, path = "../tracing", version = "0.1.16" } matchers = { optional = true, version = "0.0.1" } regex = { optional = true, version = "1", default-features = false, features = ["std"] } smallvec = { optional = true, version = "1" } diff --git a/tracing-subscriber/src/filter/env/directive.rs b/tracing-subscriber/src/filter/env/directive.rs index de8019437b..f7178f863a 100644 --- a/tracing-subscriber/src/filter/env/directive.rs +++ b/tracing-subscriber/src/filter/env/directive.rs @@ -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, + pub(crate) target: Option, in_span: Option, fields: FilterVec, - level: LevelFilter, + pub(crate) level: LevelFilter, } /// A directive which will statically enable or disable a given callsite. diff --git a/tracing-subscriber/src/filter/env/mod.rs b/tracing-subscriber/src/filter/env/mod.rs index ac3fac4bbf..6278fdd03a 100644 --- a/tracing-subscriber/src/filter/env/mod.rs +++ b/tracing-subscriber/src/filter/env/mod.rs @@ -242,6 +242,42 @@ impl EnvFilter { } fn from_directives(directives: impl IntoIterator) -> 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() { + eprintln!("warning: 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.into_level().expect("=off would not have enabled any filters"); + eprintln!(" | `{}` would enable the {} level for {}", directive, level, target); + } + eprintln!(" | 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(); + eprintln!("note: to enable {}logging, remove the `{}` feature", earlier_level, feature); + } + let (dynamics, mut statics) = Directive::make_tables(directives); let has_dynamics = !dynamics.is_empty();