From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 123111FF0B7 for ; Fri, 02 Oct 2026 10:10:16 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id C5CD12166E; Fri, 02 Oct 2026 10:10:15 +0200 (CEST) Content-Type: text/plain; charset=UTF-8 Date: Fri, 02 Oct 2026 10:10:12 +0200 Message-Id: Subject: Re: [PATCH proxmox v2 2/4] fix #6081: log: replace simple level filter with env filter From: "Lukas Wagner" To: "Thomas Ellmenreich" , Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260923115815.211083-1-t.ellmenreich@proxmox.com> <20260923115815.211083-3-t.ellmenreich@proxmox.com> In-Reply-To: <20260923115815.211083-3-t.ellmenreich@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1790928612807 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.397 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: XCSTSFNC2K6LQS2PSH5J5U5VGVHGVRA3 X-Message-ID-Hash: XCSTSFNC2K6LQS2PSH5J5U5VGVHGVRA3 X-MailFrom: l.wagner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed Sep 23, 2026 at 1:58 PM CEST, Thomas Ellmenreich wrote: > 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. > Looks good to me, one small note inline. > + > + /// 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 writ= ten into > + /// a environment variable and then extracted b= y [`get_env_variable`] > + /// * `default_log_level` - Default log level used by the [`Logger`]= in > + /// case the log settings are empty. Thanks for the extensive documentation! This produceds 2 clippy warnings (warning: doc list item overindented), please fix these for the next iteration of this series. > + fn apply_default_log_level( > + env_filter_str: Option, > + default_log_level: LevelFilter, > + ) -> EnvFilter { > + if let Some(env_filter_str) =3D env_filter_str { > + match EnvFilter::try_new(env_filter_str) { > + Ok(filter) =3D> return filter, > + Err(e) =3D> eprintln!("unable to parse the log env varia= ble: {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)] > =20 > -use std::env; > use std::future::Future; > use std::sync::{Arc, Mutex}; > =20 > @@ -147,21 +146,6 @@ where > .with_writer(std::io::stderr) > } > =20 > -fn get_env_variable(env_var: &str, default_log_level: LevelFilter) -> Le= velFilter { > - let mut log_level =3D default_log_level; > - if let Ok(v) =3D env::var(env_var) { > - match v.parse::() { > - Ok(l) =3D> { > - log_level =3D l; > - } > - Err(e) =3D> { > - eprintln!("env variable {env_var} found, but parsing fai= led: {e:?}"); > - } > - } > - } > - log_level > -} > - > /// Initialize tracing logger that prints to journald or stderr dependin= g on if we are in a pbs > /// task. > ///