Re: [pve-devel] [PATCH v2] log: add perlmod logger

[Lukas Wagner <l.wagner@proxmox.com>]

On  2024-12-05 11:23, Gabriel Goller wrote:
> Add special logger for perlmod. This one will print everything to
> stderr (which will end up in the tasklog) and the errors to journald.
> 
> Reported-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> Reported-by: Lukas Wagner <l.wagner@proxmox.com>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
> 
> v2:
>  - nothing changed
> 
>  proxmox-log/src/lib.rs | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/proxmox-log/src/lib.rs b/proxmox-log/src/lib.rs
> index 8c74e42b618d..5713c094e981 100644
> --- a/proxmox-log/src/lib.rs
> +++ b/proxmox-log/src/lib.rs
> @@ -191,3 +191,38 @@ pub fn init_cli_logger(
>      LogTracer::init_with_filter(log_level.as_log())?;
>      Ok(())
>  }
> +
> +/// Initialize logger for perlmod
> +///
> +/// This logger will log everything to stderr (which will land in the tasklog) 
> +/// and the errors to syslog as well.
> +pub fn init_perlmod_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(
> +            tracing_journald::layer().ok()
> +                .with_filter(filter_fn(|metadata| {
> +                    *metadata.level() == Level::ERROR
> +                }))
> +                .with_filter(log_level),
> +        )
> +        .with(plain_stderr_layer().with_filter(log_level));
> +
> +    tracing::subscriber::set_global_default(registry)?;
> +    LogTracer::init_with_filter(log_level.as_log())?;
> +    Ok(())
> +}

Minor nit: repo in the subject prefix is missing, also the order of patches should probably reversed.

Seems to work fine, now messages logged by proxmox-notify while being in task context show up
in the task logs again. Only downside to this approach is that we now also log to stderr
in regular daemon (non-task) context? I don't think this is an issue tho?

Small (visual) improvement would be to log the severity as well, similar to the rest of the task logs.

E.g.

INFO: notified via ...
ERROR: could not notify via ...

At the moment the prefix is not there, which is inconsistent to the other messages in the task log.

Consider this:

Tested-by: Lukas Wagner <l.wagner@proxmox.com>


-- 
- Lukas



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel