From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 3043D1FF173 for ; Mon, 10 Feb 2025 15:38:09 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id AAF0BF9FC; Mon, 10 Feb 2025 15:38:06 +0100 (CET) Date: Mon, 10 Feb 2025 15:37:31 +0100 From: Wolfgang Bumiller To: Gabriel Goller Message-ID: <5cettqvqg6i6y2cgihynjjirl4vyl6c5l3ewbn4ydhccpgc6ac@aj5yojr624d4> References: <20241209104606.263045-1-g.goller@proxmox.com> <20241209104606.263045-2-g.goller@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20241209104606.263045-2-g.goller@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.082 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox v2 1/4] log: rename/move init functions X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Cc: Lukas Wagner , pbs-devel@lists.proxmox.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" On Mon, Dec 09, 2024 at 11:46:03AM +0100, Gabriel Goller wrote: > Rename and move init functions to different module. Make function names > product-independent. Like this we don't have to use e.g. `pve_logger` in > a shared library, which wouldn't be completely true. > > Suggested-by: Lukas Wagner > Signed-off-by: Gabriel Goller > --- > proxmox-log/src/init.rs | 116 ++++++++++++++++++++++++++++++++++++++++ > proxmox-log/src/lib.rs | 86 +++++++++++++++-------------- > 2 files changed, 161 insertions(+), 41 deletions(-) > create mode 100644 proxmox-log/src/init.rs > > diff --git a/proxmox-log/src/init.rs b/proxmox-log/src/init.rs > new file mode 100644 > index 000000000000..67efa9fcc81f > --- /dev/null > +++ b/proxmox-log/src/init.rs > @@ -0,0 +1,116 @@ > +use tracing::{level_filters::LevelFilter, Level}; > +use tracing_log::{AsLog, LogTracer}; > +use tracing_subscriber::{filter::filter_fn, layer::SubscriberExt, Layer}; > + > +use crate::{ > + get_env_variable, journald_or_stderr_layer, plain_stderr_layer, tasklog_layer::TasklogLayer, LogContext > +}; > + > +/// Inits a new tracing logger that prints to journald or tasklog with the logging level specified in the > +/// environment variable `env_var`. > +/// > +/// This logger is task-aware, which means if we are in a PBS task, we will retrieve the task-file > +/// and write to it. We'll only write to journald if we are not in a task, or if the level of the > +/// log is `ERROR`. If `env_var` doesn't exist or can't be read, use the `default_log_level`. > +/// The output will be very plain: no ansi, no timestamp, no level, just the message and it's > +/// fields. > +pub fn journald_or_tasklog( > + env_var: &str, > + default_log_level: LevelFilter, > +) -> Result<(), anyhow::Error> { > + let log_level = get_env_variable(env_var, default_log_level); > + > + let registry = tracing_subscriber::registry() > + .with( > + journald_or_stderr_layer() > + .with_filter(filter_fn(|metadata| { > + !LogContext::exists() || *metadata.level() == Level::ERROR > + })) > + .with_filter(log_level), > + ) > + .with(TasklogLayer {}.with_filter(log_level)); > + > + tracing::subscriber::set_global_default(registry)?; > + LogTracer::init_with_filter(log_level.as_log())?; > + Ok(()) > +} > + > +/// Inits a new tracing logger that prints to stderr or tasklog with the logging level specified in the > +/// environment variable `env_var`. > +/// > +/// This logger is task-aware, which means if we are in a PBS task, we will retrieve the task-file > +/// and write to it. We'll only write to stderr if we are not in a task. If `env_var` doesn't exist > +/// or can't be read, use the `default_log_level`. The output will be very plain: no ansi, no > +/// timestamp, no level, just the message and it's > +/// fields. > +pub fn stderr_or_tasklog( > + env_var: &str, > + default_log_level: LevelFilter, > +) -> Result<(), anyhow::Error> { > + let log_level = get_env_variable(env_var, default_log_level); > + > + let registry = tracing_subscriber::registry() > + .with( > + plain_stderr_layer() > + .with_filter(filter_fn(|_metadata| !LogContext::exists())) ^ This condition misses the `Level::ERROR` comparison while being suggested as a replacement for `init_cli_logger` which had it (not visible in the patch context lines, but it's there). If this is done on purpose, please explain it in the commit message. If this was an oversight, the functions in `lib.rs` could just call their recommended `init::` counterparts instead of reimplementing them. > + .with_filter(log_level), > + ) > + .with(TasklogLayer {}.with_filter(log_level)); > + > + tracing::subscriber::set_global_default(registry)?; > + LogTracer::init_with_filter(log_level.as_log())?; > + Ok(()) > +} > + > +/// Inits a new tracing logger that prints to stderr with the logging level specified in the > +/// environment variable `env_var`. > +/// > +/// If `env_var` doesn't exist or can't be read, use the `default_log_level`. The output will be > +/// very plain: no ansi, no timestamp, no level, just the message and it's fields. > +pub fn stderr(env_var: &str, default_log_level: LevelFilter) -> Result<(), anyhow::Error> { > + let log_level = get_env_variable(env_var, default_log_level); > + > + let registry = tracing_subscriber::registry().with(plain_stderr_layer().with_filter(log_level)); > + > + tracing::subscriber::set_global_default(registry)?; > + LogTracer::init_with_filter(log_level.as_log())?; > + Ok(()) > +} > + > +/// Inits a new tracing logger that prints to journald with the logging level specified in the > +/// environment variable `env_var`. > +/// > +/// Prints every message to journald. If journald cannot be opened, every message will land in > +/// stderr. If `env_var` does not exist or doesn't contain a readable log level, the > +/// `default_log_level` will be used. > +pub fn journald(env_var: &str, default_log_level: LevelFilter) -> Result<(), anyhow::Error> { > + let log_level = get_env_variable(env_var, default_log_level); > + > + let registry = > + tracing_subscriber::registry().with(journald_or_stderr_layer().with_filter(log_level)); > + > + tracing::subscriber::set_global_default(registry)?; > + LogTracer::init_with_filter(log_level.as_log())?; > + Ok(()) > +} > + > +/// Inits a new tracing logger that prints to journald and tasklog with the logging level specified in the > +/// environment variable `env_var`. > +/// > +/// This logger is task-aware, which means if we are in a PBS task, we will retrieve the task-file > +/// and write to it. We will always write to journald and tasklog, even if we are in a task. If > +/// `env_var` doesn't exist or can't be read, use the `default_log_level`. > +pub fn journald_and_tasklog( > + env_var: &str, > + default_log_level: LevelFilter, > +) -> Result<(), anyhow::Error> { > + let log_level = get_env_variable(env_var, default_log_level); > + > + let registry = tracing_subscriber::registry() > + .with(journald_or_stderr_layer().with_filter(log_level)) > + .with(TasklogLayer {}.with_filter(log_level)); > + > + tracing::subscriber::set_global_default(registry)?; > + LogTracer::init_with_filter(log_level.as_log())?; > + Ok(()) > +} > diff --git a/proxmox-log/src/lib.rs b/proxmox-log/src/lib.rs > index 8c74e42b618d..50f63699336f 100644 > --- a/proxmox-log/src/lib.rs > +++ b/proxmox-log/src/lib.rs > @@ -5,18 +5,18 @@ use std::env; > use std::future::Future; > use std::sync::{Arc, Mutex}; > > +use tasklog_layer::TasklogLayer; > use tokio::task::futures::TaskLocalFuture; > use tracing_log::{AsLog, LogTracer}; > use tracing_subscriber::filter::filter_fn; > use tracing_subscriber::prelude::*; > > -use tasklog_layer::TasklogLayer; > - > mod file_logger; > -pub use file_logger::{FileLogOptions, FileLogger}; > - > mod tasklog_layer; > > +pub mod init; > +pub use file_logger::{FileLogOptions, FileLogger}; > + > pub use tracing::debug; > pub use tracing::debug_span; > pub use tracing::enabled; > @@ -38,36 +38,6 @@ tokio::task_local! { > static LOG_CONTEXT: LogContext; > } > > -pub fn init_logger( > - env_var_name: &str, > - default_log_level: LevelFilter, > -) -> Result<(), anyhow::Error> { > - let mut log_level = default_log_level; > - if let Ok(v) = env::var(env_var_name) { > - match v.parse::() { > - Ok(l) => { > - log_level = l; > - } > - Err(e) => { > - eprintln!("env variable {env_var_name} found, but parsing failed: {e:?}"); > - } > - } > - } > - let registry = tracing_subscriber::registry() > - .with( > - journald_or_stderr_layer() > - .with_filter(filter_fn(|metadata| { > - !LogContext::exists() || *metadata.level() == Level::ERROR > - })) > - .with_filter(log_level), > - ) > - .with(TasklogLayer {}.with_filter(log_level)); > - > - tracing::subscriber::set_global_default(registry)?; > - LogTracer::init_with_filter(log_level.as_log())?; > - Ok(()) > -} > - > /// A file logger and warnings counter which can be used across a scope for separate logging. > /// Mainly used for worker-task logging. > pub struct FileLogState { > @@ -160,22 +130,56 @@ where > .with_writer(std::io::stderr) > } > > -/// Initialize default logger for CLI binaries > -pub fn init_cli_logger( > - env_var_name: &str, > - default_log_level: LevelFilter, > -) -> Result<(), anyhow::Error> { > +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_name) { > + if let Ok(v) = env::var(env_var) { > match v.parse::() { > Ok(l) => { > log_level = l; > } > Err(e) => { > - eprintln!("env variable {env_var_name} found, but parsing failed: {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. > +/// > +/// Check the (tokio) LogContext and print to either journald or the Tasklog. > +#[deprecated(note = "Use the init::journald_or_tasklog function")] > +pub fn init_logger( > + env_var_name: &str, > + default_log_level: LevelFilter, > +) -> Result<(), anyhow::Error> { > + let log_level = get_env_variable(env_var_name, default_log_level); > + > + let registry = tracing_subscriber::registry() > + .with( > + journald_or_stderr_layer() > + .with_filter(filter_fn(|metadata| { > + !LogContext::exists() || *metadata.level() == Level::ERROR > + })) > + .with_filter(log_level), > + ) > + .with(TasklogLayer {}.with_filter(log_level)); > + > + tracing::subscriber::set_global_default(registry)?; > + LogTracer::init_with_filter(log_level.as_log())?; > + Ok(()) > +} > + > +/// Initialize default tracing logger for CLI binaries. > +/// > +/// Prints to stderr and to the tasklog if we are in a pbs workertask. > +#[deprecated(note = "Use the init::stderr_or_tasklog function")] > +pub fn init_cli_logger( > + env_var_name: &str, > + default_log_level: LevelFilter, > +) -> Result<(), anyhow::Error> { > + let log_level = get_env_variable(env_var_name, default_log_level); > > let registry = tracing_subscriber::registry() > .with( > -- > 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel