[pve-devel] [PATCH proxmox 1/2] log: introduce logging builder

[Gabriel Goller <g.goller@proxmox.com>]

Add a builder-like struct to compose a tracing logger using different
layers. Instead of having an init function per product/binary or
super-specific init functions that describe the logger, have a dynamic
builder. The builder improves the usability and makes the logging
initialization more self-explaining.

Suggested-by: Lukas Wagner <l.wagner@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-log/src/builder.rs | 127 +++++++++++++++++++++++++++++++++++++
 proxmox-log/src/lib.rs     |  92 +++++++++++----------------
 2 files changed, 163 insertions(+), 56 deletions(-)
 create mode 100644 proxmox-log/src/builder.rs

diff --git a/proxmox-log/src/builder.rs b/proxmox-log/src/builder.rs
new file mode 100644
index 000000000000..f7db38a94982
--- /dev/null
+++ b/proxmox-log/src/builder.rs
@@ -0,0 +1,127 @@
+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,
+};
+
+/// Builder-like struct to compose your logging layers.
+///
+/// Stores a global log level which will also be applied to all layers. The different layers can be
+/// added with the builder methods. Note that the init method can only be called once.
+///
+/// # Examples
+///
+/// ```
+/// // The default PBS daemon/proxy logger
+/// Logger::from_env("PBS_LOG", LevelFilter::INFO)
+///     .journald_on_no_workertask()
+///     .tasklog_pbs()
+///     .init()?;
+/// ```
+///
+/// ```
+/// // The default PBS cli logger
+/// Logger::from_env("PBS_LOG", LevelFilter::INFO)
+///     .stderr()
+///     .init()?;
+/// ```
+pub struct Logger {
+    global_log_level: LevelFilter,
+    layer: Vec<
+        Box<dyn tracing_subscriber::Layer<tracing_subscriber::Registry> + Send + Sync + 'static>,
+    >,
+}
+
+impl Logger {
+    /// Create a new LogBuilder with no layers and a default loglevel retrieved from an env
+    /// 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 log_level = get_env_variable(env_var, default_log_level);
+        Logger {
+            global_log_level: log_level,
+            layer: vec![],
+        }
+    }
+
+    /// Print to journald.
+    ///
+    /// If the journal cannot be opened, print to stderr instead.
+    pub fn journald(mut self) -> Logger {
+        self.layer.push(
+            journald_or_stderr_layer()
+                .with_filter(self.global_log_level)
+                .boxed(),
+        );
+        self
+    }
+
+    /// Print to journald if no LogContext (we're not in a PBS workertask) is given.
+    ///
+    /// If opening the journal fails, we fallback and print to stderr. We print to journald if
+    /// no LogContext exists – which means we are not in a PBS workertask – or the level of the
+    /// log message is 'ERROR'.
+    pub fn journald_on_no_workertask(mut self) -> Logger {
+        self.layer.push(
+            journald_or_stderr_layer()
+                .with_filter(filter_fn(|metadata| {
+                    !LogContext::exists() || *metadata.level() == Level::ERROR
+                }))
+                .with_filter(self.global_log_level)
+                .boxed(),
+        );
+        self
+    }
+
+    /// Print to the PBS tasklog if we are in a PBS workertask.
+    ///
+    /// Check if a LogContext exists and if it does, print to the corresponding task log file.
+    pub fn tasklog_pbs(mut self) -> Logger {
+        self.layer
+            .push(TasklogLayer {}.with_filter(self.global_log_level).boxed());
+        self
+    }
+
+    /// Print to stderr.
+    ///
+    /// Prints all the events to stderr with the compact format (no level, no timestamp).
+    pub fn stderr(mut self) -> Logger {
+        self.layer.push(
+            plain_stderr_layer()
+                .with_filter(self.global_log_level)
+                .boxed(),
+        );
+        self
+    }
+
+    /// Print to stderr if no workertask exists or the event level is `ERROR`.
+    ///
+    /// Print to stderr in the default compact format (no level, no timestamp). This will only be
+    /// triggered if no workertask could be found (no LogContext exists) or the event level is
+    /// `ERROR`.
+    pub fn stderr_on_no_workertask(mut self) -> Logger {
+        self.layer.push(
+            plain_stderr_layer()
+                .with_filter(filter_fn(|metadata| {
+                    !LogContext::exists() || *metadata.level() == Level::ERROR
+                }))
+                .with_filter(self.global_log_level)
+                .boxed(),
+        );
+        self
+    }
+
+    /// 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> {
+        let registry = tracing_subscriber::registry().with(self.layer);
+        tracing::subscriber::set_global_default(registry)?;
+
+        LogTracer::init_with_filter(self.global_log_level.as_log())?;
+        Ok(())
+    }
+}
diff --git a/proxmox-log/src/lib.rs b/proxmox-log/src/lib.rs
index 8c74e42b618d..51ca89acc992 100644
--- a/proxmox-log/src/lib.rs
+++ b/proxmox-log/src/lib.rs
@@ -6,17 +6,15 @@ use std::future::Future;
 use std::sync::{Arc, Mutex};
 
 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 builder;
+pub use builder::Logger;
+pub use file_logger::{FileLogOptions, FileLogger};
+
 pub use tracing::debug;
 pub use tracing::debug_span;
 pub use tracing::enabled;
@@ -38,36 +36,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,34 +128,46 @@ 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
+}
 
-    let registry = tracing_subscriber::registry()
-        .with(
-            plain_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 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 `Logger` builder instead")]
+pub fn init_logger(
+    env_var_name: &str,
+    default_log_level: LevelFilter,
+) -> Result<(), anyhow::Error> {
+    Logger::from_env(env_var_name, default_log_level)
+        .journald_on_no_workertask()
+        .tasklog_pbs()
+        .init()
+}
+
+/// 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 `Logger` builder instead")]
+pub fn init_cli_logger(
+    env_var_name: &str,
+    default_log_level: LevelFilter,
+) -> Result<(), anyhow::Error> {
+    Logger::from_env(env_var_name, default_log_level)
+        .stderr_on_no_workertask()
+        .tasklog_pbs()
+        .init()
 }
-- 
2.39.5



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