public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
To: pdm-devel@lists.proxmox.com
Cc: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
Subject: [PATCH proxmox 2/5] log: replace simple level filter with env filter
Date: Mon, 21 Sep 2026 11:51:55 +0200	[thread overview]
Message-ID: <20260921095210.229315-4-t.ellmenreich@proxmox.com> (raw)
In-Reply-To: <20260921095210.229315-2-t.ellmenreich@proxmox.com>

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

[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 | 38 ++++++++++++++++++++++++++++++++------
 proxmox-log/src/lib.rs     | 16 ----------------
 3 files changed, 33 insertions(+), 23 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 3108ea29..9b545603 100644
--- a/proxmox-log/src/builder.rs
+++ b/proxmox-log/src/builder.rs
@@ -2,6 +2,7 @@ use tracing::Level;
 use tracing::Metadata;
 use tracing::level_filters::LevelFilter;
 use tracing_log::LogTracer;
+use tracing_subscriber::EnvFilter;
 use tracing_subscriber::Layer;
 use tracing_subscriber::Registry;
 use tracing_subscriber::layer::Context;
@@ -9,7 +10,7 @@ use tracing_subscriber::layer::Filter;
 use tracing_subscriber::layer::SubscriberExt;
 
 use crate::{
-    LogContext, get_env_variable, journald_or_stderr_layer, plain_stderr_layer,
+    LogContext, journald_or_stderr_layer, plain_stderr_layer,
     pve_task_formatter::PveTaskFormatter, tasklog_layer::TasklogLayer,
 };
 ///
@@ -55,16 +56,17 @@ impl<S> Filter<S> for NoWorkerTask {
 /// # func().expect("failed to init logger");
 /// ```
 pub struct Logger {
-    global_log_level: LevelFilter,
+    global_log_level: EnvFilter,
     layer: Vec<Box<dyn Layer<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 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.
     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 log_level = Self::apply_default_log_level(var_content, default_log_level);
         Logger {
             global_log_level: log_level,
             layer: vec![],
@@ -143,4 +145,28 @@ impl Logger {
         LogTracer::init()?;
         Ok(())
     }
+
+    /// If present, tries to parse the `env_filter_str` as a [`EnvFilter`],
+    /// otherwiese falls back to the provided default log level.
+    ///
+    /// The provided default log level is a application default defined by us.
+    /// The user can define their own default as part of the [`EnvFilter`]
+    ///
+    /// * `env_filter_str` - The log settings that would usually be written into
+    ///                      a environment variable and then extracted by [`get_env_variable`]
+    /// * `default_log_level` - Default log level used by the [`Logger`] in
+    ///                         case the log settings are empty.
+    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) {
+                Ok(filter) => return filter,
+                Err(e) => eprintln!("unable to parse the log env variable: {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





  parent reply	other threads:[~2026-09-21  9:53 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-21  9:51 [RFC datacenter-manager/proxmox 0/5] log: allow finegrained control logging levels Thomas Ellmenreich
2026-09-21  9:51 ` [PATCH proxmox 1/5] log: replace per layer filtering by one global filter Thomas Ellmenreich
2026-09-21  9:51 ` Thomas Ellmenreich [this message]
2026-09-21  9:51 ` [PATCH proxmox 3/5] log: add tests to the logger Thomas Ellmenreich
2026-09-21  9:51 ` [PATCH proxmox 4/5] log: return the logger configuration after initialisation Thomas Ellmenreich
2026-09-21  9:51 ` [PATCH datacenter-manager 5/5] api: set REST server debug level based on actual log level Thomas Ellmenreich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260921095210.229315-4-t.ellmenreich@proxmox.com \
    --to=t.ellmenreich@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal