[pbs-devel] [PATCH proxmox v2 1/4] log: rename/move init functions

[Wolfgang Bumiller]

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 <l.wagner at proxmox.com>
> Signed-off-by: Gabriel Goller <g.goller at proxmox.com>
> ---
>  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::<LevelFilter>() {
> -            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::<LevelFilter>() {
>              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