From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 6D7CD1FF09C for ; Mon, 21 Sep 2026 11:53:29 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id F1FD8215C1; Mon, 21 Sep 2026 11:53:19 +0200 (CEST) From: Thomas Ellmenreich To: pdm-devel@lists.proxmox.com Subject: [PATCH proxmox 4/5] log: return the logger configuration after initialisation Date: Mon, 21 Sep 2026 11:51:57 +0200 Message-ID: <20260921095210.229315-6-t.ellmenreich@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260921095210.229315-2-t.ellmenreich@proxmox.com> References: <20260921095210.229315-2-t.ellmenreich@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1789984393499 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.572 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) KAM_SHORT 0.001 Use of a URL Shortener for very short URL 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: VOSUZNE5V36PM2JOO7XWPNKSZSOQVW57 X-Message-ID-Hash: VOSUZNE5V36PM2JOO7XWPNKSZSOQVW57 X-MailFrom: t.ellmenreich@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 CC: Thomas Ellmenreich 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: The logger configuration is returned when the logger is initialised, so that it can be referenced for other configurations or loggers. Signed-off-by: Thomas Ellmenreich --- proxmox-log/src/builder.rs | 135 +++++++++++++++++++++++++++++++++---- proxmox-log/src/lib.rs | 6 +- 2 files changed, 126 insertions(+), 15 deletions(-) diff --git a/proxmox-log/src/builder.rs b/proxmox-log/src/builder.rs index 05da3643..e591fecc 100644 --- a/proxmox-log/src/builder.rs +++ b/proxmox-log/src/builder.rs @@ -57,7 +57,7 @@ impl Filter for NoWorkerTask { /// # func().expect("failed to init logger"); /// ``` pub struct Logger { - global_log_level: EnvFilter, + config: LoggerConfig, layer: Vec + Send + Sync + 'static>>, } @@ -66,10 +66,8 @@ impl Logger { /// 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 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, + config: LoggerConfig::from_env(env_var, default_log_level), layer: vec![], } } @@ -136,13 +134,14 @@ impl Logger { /// Inits the tracing logger with the previously configured layers. /// /// Also configures the `LogTracer` which will convert all `log` events to tracing events. - pub fn init(self) -> Result<(), anyhow::Error> { + pub fn init(self) -> Result { + let config = self.config.clone(); let registry = self.create_subscriber(); tracing::subscriber::set_global_default(registry)?; LogTracer::init()?; - Ok(()) + Ok(config) } /// Creates the subscriber to be used for the Logger @@ -151,7 +150,76 @@ impl Logger { fn create_subscriber(self) -> impl Subscriber { tracing_subscriber::registry() .with(self.layer) - .with(self.global_log_level) + .with(self.config.global_log_level) + } +} + +/// Struct containing the configuration of the logger +pub struct LoggerConfig { + global_log_level: EnvFilter, +} + +/// TODO: With version 0.3.20 of tracing_subscriber this [`Clone`] +/// implementation can be replaced with a simple derive +impl Clone for LoggerConfig { + fn clone(&self) -> Self { + let global_log_level = self.clone_global_filter(); + Self { global_log_level } + } +} + +impl LoggerConfig { + /// Tries to read the environemnt variable with the provided name and + /// parse its contents as a [`EnvFilter`] + pub fn from_env(env_var: &str, default_log_level: LevelFilter) -> LoggerConfig { + let var_content = std::env::var(env_var).ok(); + Self::from_string(var_content, default_log_level) + } + + /// Tries to parse the provided string as a [`EnvFilter`] if present. Falls + /// back to the default in all other cases + pub fn from_string( + env_filter_str: Option, + default_log_level: LevelFilter, + ) -> LoggerConfig { + let global_log_level = Self::apply_default_log_level(env_filter_str, default_log_level); + Self { global_log_level } + } + + pub fn global_filter(&self) -> EnvFilter { + self.clone_global_filter() + } + + /// Returns an hint of the highest [verbosity level](https://docs.rs/tracing-core/0.1.32/tracing_core/metadata/struct.Level.html) + /// that this `EnvFilter` will enable. + /// + /// # Panics + /// + /// Panics if no max can be found, although that should never be the case + /// as we force a default log level and thus always have a max. + pub fn global_filter_max_level_hint(&self) -> LevelFilter { + self.global_log_level + .max_level_hint() + .expect("because we force a default log level there should always be a hint") + } + + /// Clones the global_log_filter and returns said clone. Since the clone + /// implementation is not yet available, serializes the filter back into + /// a string and then parses it again. + /// + /// TODO: the current version of EnvFilter provided by the debian package + /// does not implement clone although the following [0] version already + /// does. So once the version is bumped, replace this hack with a `.clone()` + /// + /// [0]: https://docs.rs/tracing-subscriber/0.3.20/src/tracing_subscriber/filter/env/mod.rs.html#211-223 + /// + /// # Panics + /// + /// Panics if, while serializing into a string and deserializing back into + /// a [`EnvFilter`], the parsing fails (which should never happen). + fn clone_global_filter(&self) -> EnvFilter { + EnvFilter::try_new(format!("{}", self.global_log_level)) + .expect("creating a new envfilter from a existing filter should always be possible") } /// If present, tries to parse the `env_filter_str` as a [`EnvFilter`], @@ -184,9 +252,9 @@ mod tests { use tracing::level_filters::LevelFilter; use tracing_log::log; - use tracing_subscriber::{Layer, util::SubscriberInitExt}; + use tracing_subscriber::{EnvFilter, Layer, util::SubscriberInitExt}; - use crate::Logger; + use crate::{Logger, builder::LoggerConfig}; /// Modules created for testing purposes. Specifically, to test filtering /// of logs in different modules. @@ -209,6 +277,47 @@ mod tests { }}; } + // TODO: delete once [`EnvFilter`] implements [`Clone`] + #[test] + fn check_env_filter_display_contains_expected_modules() { + // Arrange + let filter_str = "warn,proxmox_log::builder::tests::test_module=info,proxmox_log::builder::tests::test_module::nested_module=error"; + + // Act + let formatted_filter = EnvFilter::try_new(filter_str).unwrap().to_string(); + + // Assert + let sort_parts = |filter: &str| -> String { + let mut parts = filter.split(',').collect::>(); + parts.sort(); + parts.join(",") + }; + assert_eq!(sort_parts(formatted_filter.as_str()), sort_parts(filter_str)); + } + + // TODO: delete once [`EnvFilter`] implements [`Clone`] + #[test] + fn check_env_filter_default_formats_as_expected() { + // Arrange + let default = LevelFilter::WARN; + + // Act + let formatted_filter = EnvFilter::default().add_directive(default.into()).to_string(); + + // Assert + assert_eq!(formatted_filter.as_str(), "warn"); + } + + // TODO: delete once [`EnvFilter`] implements [`Clone`] + #[test] + fn check_empty_env_filter_formats_as_expected() { + // Act + let formatted_filter = EnvFilter::try_new("").unwrap().to_string(); + + // Assert + assert_eq!(formatted_filter.as_str(), ""); + } + #[test] fn logger_builder_correctly_applies_filter() { // Arrange @@ -348,7 +457,7 @@ mod tests { ( events, Logger { - global_log_level: Logger::apply_default_log_level( + config: LoggerConfig::from_string( var_filter.map(str::to_string), default_log_level, ), @@ -371,8 +480,10 @@ mod tests { ) -> (Arc>>, tracing::subscriber::DefaultGuard) { use tracing_subscriber::layer::SubscriberExt; - let filter = - Logger::apply_default_log_level(var_filter.map(str::to_string), default_log_level); + let filter = LoggerConfig::apply_default_log_level( + var_filter.map(str::to_string), + default_log_level, + ); let (events, filter) = TestLayer::new(filter); let guard = tracing_subscriber::Registry::default() diff --git a/proxmox-log/src/lib.rs b/proxmox-log/src/lib.rs index 32c10e27..3c2e4fdd 100644 --- a/proxmox-log/src/lib.rs +++ b/proxmox-log/src/lib.rs @@ -13,7 +13,7 @@ mod pve_task_formatter; mod tasklog_layer; pub mod builder; -pub use builder::Logger; +pub use builder::{Logger, LoggerConfig}; pub use file_logger::{FileLogOptions, FileLogger}; pub use tracing::Level; @@ -154,7 +154,7 @@ where pub fn init_logger( env_var_name: &str, default_log_level: LevelFilter, -) -> Result<(), anyhow::Error> { +) -> Result { Logger::from_env(env_var_name, default_log_level) .journald_on_no_workertask() .tasklog_pbs() @@ -168,7 +168,7 @@ pub fn init_logger( pub fn init_cli_logger( env_var_name: &str, default_log_level: LevelFilter, -) -> Result<(), anyhow::Error> { +) -> Result { Logger::from_env(env_var_name, default_log_level) .stderr_on_no_workertask() .tasklog_pbs() -- 2.47.3