[PATCH proxmox v2 2/7] fix #6081: log: replace simple level filter with env filter

[Thomas Ellmenreich <t.ellmenreich@proxmox.com>]

Instead of parsing the contents of the environment variable passed to the
Logger as a simple LevelFilter, parse it as an EnvFilter [0]. This is
backwards compatible, as a simple LevelFilter [1] string is parsable as
an EnvFilter.

The LogTracer only accepts a simple level as a filter, so pass it the
EnvFilter's max level hint. This will not avoid the construction of all
eventually discarded logs, but should avoid most of them.

[0]: https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.EnvFilter.html
[1]: https://docs.rs/tracing-subscriber/latest/tracing_subscriber/filter/struct.LevelFilter.html

Suggested-by: Gabriel Goller <g.goller@proxmox.com>
Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6081
Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
---
 proxmox-log/Cargo.toml     |  2 +-
 proxmox-log/src/builder.rs | 50 ++++++++++++++++++++++++++++++--------
 proxmox-log/src/lib.rs     | 16 ------------
 3 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/proxmox-log/Cargo.toml b/proxmox-log/Cargo.toml
index 25c36fc4..4abe1b11 100644
--- a/proxmox-log/Cargo.toml
+++ b/proxmox-log/Cargo.toml
@@ -16,7 +16,7 @@ anyhow.workspace = true
 nix.workspace = true
 tracing.workspace = true
 tracing-journald.workspace = true
-tracing-subscriber.workspace = true
+tracing-subscriber = { workspace = true, features = ["env-filter"] }
 tracing-log = { workspace = true, features = ["std"] }
 tokio = { workspace = true, features = ["rt-multi-thread"] }
 
diff --git a/proxmox-log/src/builder.rs b/proxmox-log/src/builder.rs
index bbac7797..f21983e3 100644
--- a/proxmox-log/src/builder.rs
+++ b/proxmox-log/src/builder.rs
@@ -3,14 +3,15 @@ use tracing::Metadata;
 use tracing::level_filters::LevelFilter;
 use tracing_log::AsLog;
 use tracing_log::LogTracer;
+use tracing_subscriber::EnvFilter;
 use tracing_subscriber::Layer;
 use tracing_subscriber::layer::Context;
 use tracing_subscriber::layer::Filter;
 use tracing_subscriber::layer::SubscriberExt;
 
 use crate::{
-    LogContext, get_env_variable, journald_or_stderr_layer, plain_stderr_layer,
-    pve_task_formatter::PveTaskFormatter, tasklog_layer::TasklogLayer,
+    LogContext, journald_or_stderr_layer, plain_stderr_layer, pve_task_formatter::PveTaskFormatter,
+    tasklog_layer::TasklogLayer,
 };
 ///
 /// Filter yielding `true` *outside* of worker tasks, *unless* the level is `ERROR`.
@@ -55,20 +56,21 @@ impl<S> Filter<S> for NoWorkerTask {
 /// # func().expect("failed to init logger");
 /// ```
 pub struct Logger {
-    global_log_level: LevelFilter,
+    global_log_filter: EnvFilter,
     layer: Vec<
         Box<dyn tracing_subscriber::Layer<tracing_subscriber::Registry> + Send + Sync + 'static>,
     >,
 }
 
 impl Logger {
-    /// Create a new LogBuilder with no layers and a default loglevel retrieved from an env
-    /// variable. If the env variable cannot be retrieved or the content is not parsable, fallback
-    /// to the default_log_level passed.
+    /// Create a new Logger with no layers and a log filter retrieved from an env
+    /// variable. If the env variable cannot be retrieved or the content is not
+    /// parsable, fall back to the default_log_level passed.
     pub fn from_env(env_var: &str, default_log_level: LevelFilter) -> Logger {
-        let log_level = get_env_variable(env_var, default_log_level);
+        let var_content = std::env::var(env_var).ok();
+        let global_log_filter = Self::apply_default_log_level(var_content, default_log_level);
         Logger {
-            global_log_level: log_level,
+            global_log_filter,
             layer: vec![],
         }
     }
@@ -136,13 +138,41 @@ impl Logger {
     ///
     /// Also configures the `LogTracer` which will convert all `log` events to tracing events.
     pub fn init(self) -> Result<(), anyhow::Error> {
+        let max_level = self.global_log_filter.max_level_hint();
         let registry = tracing_subscriber::registry()
             .with(self.layer)
-            .with(self.global_log_level);
+            .with(self.global_log_filter);
 
         tracing::subscriber::set_global_default(registry)?;
 
-        LogTracer::init_with_filter(self.global_log_level.as_log())?;
+        match max_level {
+            Some(max_level) => LogTracer::init_with_filter(max_level.as_log()),
+            None => LogTracer::init(),
+        }?;
+
         Ok(())
     }
+
+    /// If present, tries to parse the `env_filter_str` as an [`EnvFilter`],
+    /// otherwise falls back to the provided default log level.
+    ///
+    /// The provided default log level is an application default defined by us.
+    /// The user can define their own default as part of the [`EnvFilter`].
+    ///
+    /// * `env_filter_str` - Log settings in string format, what is usually
+    ///   written into a log variable. This supports the [`EnvFilter`] format.
+    /// * `default_log_level` - Default log level used by the [`Logger`] in
+    ///   case the log settings are not set.
+    fn apply_default_log_level(
+        env_filter_str: Option<String>,
+        default_log_level: LevelFilter,
+    ) -> EnvFilter {
+        if let Some(env_filter_str) = env_filter_str {
+            match EnvFilter::try_new(env_filter_str.as_str()) {
+                Ok(filter) => return filter,
+                Err(e) => eprintln!("unable to parse the log env variable {env_filter_str}: {e:#}"),
+            }
+        }
+        EnvFilter::default().add_directive(default_log_level.into())
+    }
 }
diff --git a/proxmox-log/src/lib.rs b/proxmox-log/src/lib.rs
index 2d321f20..32c10e27 100644
--- a/proxmox-log/src/lib.rs
+++ b/proxmox-log/src/lib.rs
@@ -1,7 +1,6 @@
 #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
 #![deny(unsafe_op_in_unsafe_fn)]
 
-use std::env;
 use std::future::Future;
 use std::sync::{Arc, Mutex};
 
@@ -147,21 +146,6 @@ where
         .with_writer(std::io::stderr)
 }
 
-fn get_env_variable(env_var: &str, default_log_level: LevelFilter) -> LevelFilter {
-    let mut log_level = default_log_level;
-    if let Ok(v) = env::var(env_var) {
-        match v.parse::<LevelFilter>() {
-            Ok(l) => {
-                log_level = l;
-            }
-            Err(e) => {
-                eprintln!("env variable {env_var} found, but parsing failed: {e:?}");
-            }
-        }
-    }
-    log_level
-}
-
 /// Initialize tracing logger that prints to journald or stderr depending on if we are in a pbs
 /// task.
 ///
-- 
2.47.3