[pve-devel] [PATCH proxmox{, -backup, -perl-rs} 0/4] Introduce builder for logging initialization

[pve-devel] [PATCH proxmox{, -backup, -perl-rs} 0/4] Introduce builder for logging initialization

[Gabriel Goller]

To move away from product-specific logging functions or overly long and
descriptive initialization functions, add a builder-type structure.
This structure will combine layers and compose a logger more descriptively.

The is backwards compatible as the old functions have not been removed and are
only deprecated.

This is the first version because the approach compared to the last series
changed quite a lot.
Previous series: https://lore.proxmox.com/pbs-devel/20241209104606.263045-1-g.goller@proxmox.com/

proxmox:

Gabriel Goller (2):
  log: introduce logging builder
  log: add layer for pve workertasks in perlmod crates

 proxmox-log/src/builder.rs            | 141 ++++++++++++++++++++++++++
 proxmox-log/src/lib.rs                |  93 +++++++----------
 proxmox-log/src/pve_task_formatter.rs |  31 ++++++
 3 files changed, 209 insertions(+), 56 deletions(-)
 create mode 100644 proxmox-log/src/builder.rs
 create mode 100644 proxmox-log/src/pve_task_formatter.rs


proxmox-backup:

Gabriel Goller (1):
  log: use new builder initializer

 pbs-tape/src/bin/pmt.rs           |  5 +++--
 pbs-tape/src/bin/pmtx.rs          |  5 +++--
 proxmox-backup-client/src/main.rs |  6 ++++--
 proxmox-file-restore/src/main.rs  |  7 +++++--
 pxar-bin/src/main.rs              |  7 +++++--
 src/bin/proxmox-backup-api.rs     |  6 ++++--
 src/bin/proxmox-backup-debug.rs   |  6 ++++--
 src/bin/proxmox-backup-manager.rs |  8 ++++++--
 src/bin/proxmox-backup-proxy.rs   |  6 ++++--
 src/bin/proxmox-daily-update.rs   | 13 ++++++-------
 src/bin/proxmox-tape.rs           |  6 ++++--
 src/bin/sg-tape-cmd.rs            |  5 +++--
 12 files changed, 51 insertions(+), 29 deletions(-)


proxmox-perl-rs:

Gabriel Goller (1):
  log: use new logging builder, print to stderr and journald

 common/src/logger.rs | 7 ++++++-
 pmg-rs/Cargo.toml    | 1 -
 pmg-rs/src/lib.rs    | 2 +-
 pve-rs/Cargo.toml    | 1 -
 4 files changed, 7 insertions(+), 4 deletions(-)


Summary over all repositories:
  19 files changed, 267 insertions(+), 89 deletions(-)

-- 
Generated by git-murpp 0.8.0


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

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

[Gabriel Goller]

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

[pve-devel] [PATCH proxmox 2/2] log: add layer for pve workertasks in perlmod crates

[Gabriel Goller]

Add a layer that outputs messages to stderr in a specific format. In
PVE, stderr is rerouted to the tasklog if the we are within a
workertask. Therefore, ensure the stderr output is formatted
appropriately.

Reported-by: Lukas Wagner <l.wagner@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-log/src/builder.rs            | 16 +++++++++++++-
 proxmox-log/src/lib.rs                |  1 +
 proxmox-log/src/pve_task_formatter.rs | 31 +++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 proxmox-log/src/pve_task_formatter.rs

diff --git a/proxmox-log/src/builder.rs b/proxmox-log/src/builder.rs
index f7db38a94982..5725b4c0f09d 100644
--- a/proxmox-log/src/builder.rs
+++ b/proxmox-log/src/builder.rs
@@ -4,7 +4,7 @@ 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,
+    pve_task_formatter::PveTaskFormatter, tasklog_layer::TasklogLayer, LogContext,
 };
 
 /// Builder-like struct to compose your logging layers.
@@ -114,6 +114,20 @@ impl Logger {
         self
     }
 
+    /// Print to stderr in the PVE format.
+    ///
+    /// The PVE format only prints the event level and messages.
+    /// e.g.: `DEBUG: event message`.
+    pub fn stderr_pve(mut self) -> Logger {
+        let layer = tracing_subscriber::fmt::layer()
+            .event_format(PveTaskFormatter {})
+            .with_writer(std::io::stderr)
+            .with_filter(self.global_log_level)
+            .boxed();
+        self.layer.push(layer);
+        self
+    }
+
     /// Inits the tracing logger with the previously configured layers.
     ///
     /// Also configures the `LogTracer` which will convert all `log` events to tracing events.
diff --git a/proxmox-log/src/lib.rs b/proxmox-log/src/lib.rs
index 51ca89acc992..2f4d4dabe23c 100644
--- a/proxmox-log/src/lib.rs
+++ b/proxmox-log/src/lib.rs
@@ -9,6 +9,7 @@ use tokio::task::futures::TaskLocalFuture;
 use tracing_subscriber::prelude::*;
 
 mod file_logger;
+mod pve_task_formatter;
 mod tasklog_layer;
 
 pub mod builder;
diff --git a/proxmox-log/src/pve_task_formatter.rs b/proxmox-log/src/pve_task_formatter.rs
new file mode 100644
index 000000000000..e9866a4b0869
--- /dev/null
+++ b/proxmox-log/src/pve_task_formatter.rs
@@ -0,0 +1,31 @@
+use std::fmt;
+use tracing::{Event, Subscriber};
+use tracing_subscriber::field::VisitOutput;
+use tracing_subscriber::fmt::format::{DefaultVisitor, Writer};
+use tracing_subscriber::fmt::{FmtContext, FormatEvent, FormatFields};
+use tracing_subscriber::registry::LookupSpan;
+
+/// This custom formatter outputs logs as they are visible in the PVE task log.
+///
+/// e.g.: "DEBUG: sample message"
+pub struct PveTaskFormatter {}
+
+impl<C, N> FormatEvent<C, N> for PveTaskFormatter
+where
+    C: Subscriber + for<'a> LookupSpan<'a>,
+    N: for<'a> FormatFields<'a> + 'static,
+{
+    fn format_event(
+        &self,
+        _ctx: &FmtContext<'_, C, N>,
+        mut writer: Writer<'_>,
+        event: &Event<'_>,
+    ) -> fmt::Result {
+        write!(writer, "{}: ", event.metadata().level().as_str())?;
+
+        let mut v = DefaultVisitor::new(writer.by_ref(), true);
+        event.record(&mut v);
+        v.finish()?;
+        writer.write_char('\n')
+    }
+}
-- 
2.39.5



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

[pve-devel] [PATCH proxmox-backup 1/1] log: use new builder initializer

[Gabriel Goller]

Use new logger builder to initialize the logging in each component.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 pbs-tape/src/bin/pmt.rs           |  5 +++--
 pbs-tape/src/bin/pmtx.rs          |  5 +++--
 proxmox-backup-client/src/main.rs |  6 ++++--
 proxmox-file-restore/src/main.rs  |  7 +++++--
 pxar-bin/src/main.rs              |  7 +++++--
 src/bin/proxmox-backup-api.rs     |  6 ++++--
 src/bin/proxmox-backup-debug.rs   |  6 ++++--
 src/bin/proxmox-backup-manager.rs |  8 ++++++--
 src/bin/proxmox-backup-proxy.rs   |  6 ++++--
 src/bin/proxmox-daily-update.rs   | 13 ++++++-------
 src/bin/proxmox-tape.rs           |  6 ++++--
 src/bin/sg-tape-cmd.rs            |  5 +++--
 12 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/pbs-tape/src/bin/pmt.rs b/pbs-tape/src/bin/pmt.rs
index 9e39dbe16b2f..d70c61e9a10e 100644
--- a/pbs-tape/src/bin/pmt.rs
+++ b/pbs-tape/src/bin/pmt.rs
@@ -15,7 +15,6 @@
 use anyhow::{bail, Error};
 use serde_json::Value;
 
-use proxmox_log::init_cli_logger;
 use proxmox_router::cli::*;
 use proxmox_router::RpcEnvironment;
 use proxmox_schema::{api, ArraySchema, IntegerSchema, Schema, StringSchema};
@@ -800,7 +799,9 @@ fn options(
 }
 
 fn main() -> Result<(), Error> {
-    init_cli_logger("PBS_LOG", proxmox_log::LevelFilter::INFO)?;
+    proxmox_log::Logger::from_env("PBS_LOG", proxmox_log::LevelFilter::INFO)
+        .stderr()
+        .init()?;
 
     let uid = nix::unistd::Uid::current();
 
diff --git a/pbs-tape/src/bin/pmtx.rs b/pbs-tape/src/bin/pmtx.rs
index 303353e6bfd3..df556a8f2706 100644
--- a/pbs-tape/src/bin/pmtx.rs
+++ b/pbs-tape/src/bin/pmtx.rs
@@ -16,7 +16,6 @@ use std::fs::File;
 use anyhow::{bail, Error};
 use serde_json::Value;
 
-use proxmox_log::init_cli_logger;
 use proxmox_router::cli::*;
 use proxmox_router::RpcEnvironment;
 use proxmox_schema::api;
@@ -388,7 +387,9 @@ fn scan(param: Value) -> Result<(), Error> {
 }
 
 fn main() -> Result<(), Error> {
-    init_cli_logger("PBS_LOG", proxmox_log::LevelFilter::INFO)?;
+    proxmox_log::Logger::from_env("PBS_LOG", proxmox_log::LevelFilter::INFO)
+        .stderr()
+        .init()?;
 
     let uid = nix::unistd::Uid::current();
 
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 632a291707c7..c49591041eb1 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -16,7 +16,6 @@ use xdg::BaseDirectories;
 use pathpatterns::{MatchEntry, MatchType, PatternFlag};
 use proxmox_async::blocking::TokioWriterAdapter;
 use proxmox_io::StdChannelWriter;
-use proxmox_log::init_cli_logger;
 use proxmox_router::{cli::*, ApiMethod, RpcEnvironment};
 use proxmox_schema::api;
 use proxmox_sys::fs::{file_get_json, image_size, replace_file, CreateOptions};
@@ -1962,7 +1961,10 @@ impl ReadAt for BufferedDynamicReadAt {
 
 fn main() {
     pbs_tools::setup_libc_malloc_opts();
-    init_cli_logger("PBS_LOG", proxmox_log::LevelFilter::INFO).expect("failed to initiate logger");
+    proxmox_log::Logger::from_env("PBS_LOG", proxmox_log::LevelFilter::INFO)
+        .stderr()
+        .init()
+        .expect("failed to initiate logger");
 
     let backup_cmd_def = CliCommand::new(&API_METHOD_CREATE_BACKUP)
         .arg_param(&["backupspec"])
diff --git a/proxmox-file-restore/src/main.rs b/proxmox-file-restore/src/main.rs
index 572e2d188b42..7888c38d64ba 100644
--- a/proxmox-file-restore/src/main.rs
+++ b/proxmox-file-restore/src/main.rs
@@ -10,7 +10,6 @@ use serde_json::{json, Value};
 use tokio::io::AsyncWriteExt;
 
 use proxmox_compression::zstd::ZstdEncoder;
-use proxmox_log::init_cli_logger;
 use proxmox_router::cli::{
     complete_file_name, default_table_format_options, format_and_print_result_full,
     get_output_format, run_cli_command, CliCommand, CliCommandMap, CliEnvironment, ColumnConfig,
@@ -629,7 +628,11 @@ fn main() {
         true => proxmox_log::LevelFilter::DEBUG,
         false => proxmox_log::LevelFilter::INFO,
     };
-    init_cli_logger("PBS_LOG", loglevel).expect("failed to initiate logger");
+
+    proxmox_log::Logger::from_env("PBS_LOG", loglevel)
+        .stderr()
+        .init()
+        .expect("failed to initiate logger");
 
     let list_cmd_def = CliCommand::new(&API_METHOD_LIST)
         .arg_param(&["snapshot", "path"])
diff --git a/pxar-bin/src/main.rs b/pxar-bin/src/main.rs
index 7dff1e38c252..cccbb3315aaa 100644
--- a/pxar-bin/src/main.rs
+++ b/pxar-bin/src/main.rs
@@ -22,7 +22,7 @@ use pbs_client::pxar::{
 use pxar::EntryKind;
 
 use proxmox_human_byte::HumanByte;
-use proxmox_log::{debug, enabled, error, init_cli_logger, Level};
+use proxmox_log::{debug, enabled, error, Level};
 use proxmox_router::cli::*;
 use proxmox_schema::api;
 
@@ -574,7 +574,10 @@ fn dump_archive(archive: String, payload_input: Option<String>) -> Result<(), Er
 }
 
 fn main() {
-    init_cli_logger("PXAR_LOG", proxmox_log::LevelFilter::INFO).expect("failed to initiate logger");
+    proxmox_log::Logger::from_env("PXAR_LOG", proxmox_log::LevelFilter::INFO)
+        .stderr()
+        .init()
+        .expect("failed to initiate logger");
 
     let cmd_def = CliCommandMap::new()
         .insert(
diff --git a/src/bin/proxmox-backup-api.rs b/src/bin/proxmox-backup-api.rs
index 7a72d49a434d..9c06f0cea1dc 100644
--- a/src/bin/proxmox-backup-api.rs
+++ b/src/bin/proxmox-backup-api.rs
@@ -8,7 +8,6 @@ use hyper::{Body, StatusCode};
 use tracing::level_filters::LevelFilter;
 
 use proxmox_lang::try_block;
-use proxmox_log::init_logger;
 use proxmox_rest_server::{ApiConfig, RestServer};
 use proxmox_router::RpcEnvironmentType;
 use proxmox_sys::fs::CreateOptions;
@@ -41,7 +40,10 @@ fn get_index() -> Pin<Box<dyn Future<Output = Response<Body>> + Send>> {
 }
 
 async fn run() -> Result<(), Error> {
-    init_logger("PBS_LOG", LevelFilter::INFO)?;
+    proxmox_log::Logger::from_env("PBS_LOG", LevelFilter::INFO)
+        .journald_on_no_workertask()
+        .tasklog_pbs()
+        .init()?;
 
     config::create_configdir()?;
 
diff --git a/src/bin/proxmox-backup-debug.rs b/src/bin/proxmox-backup-debug.rs
index 35ad11c70e7e..da5485c28e49 100644
--- a/src/bin/proxmox-backup-debug.rs
+++ b/src/bin/proxmox-backup-debug.rs
@@ -1,4 +1,3 @@
-use proxmox_log::init_cli_logger;
 use proxmox_router::{
     cli::{run_cli_command, CliCommandMap, CliEnvironment},
     RpcEnvironment,
@@ -8,7 +7,10 @@ mod proxmox_backup_debug;
 use proxmox_backup_debug::*;
 
 fn main() {
-    init_cli_logger("PBS_LOG", proxmox_log::LevelFilter::INFO).expect("failed to initiate logger");
+    proxmox_log::Logger::from_env("PBS_LOG", proxmox_log::LevelFilter::INFO)
+        .stderr()
+        .init()
+        .expect("failed to initiate logger");
 
     let cmd_def = CliCommandMap::new()
         .insert("inspect", inspect::inspect_commands())
diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index 02ca0d028225..c8719b2a23cb 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -3,7 +3,6 @@ use std::io::{self, Write};
 use std::str::FromStr;
 
 use anyhow::{format_err, Error};
-use proxmox_log::init_cli_logger;
 use serde_json::{json, Value};
 
 use proxmox_router::{cli::*, RpcEnvironment};
@@ -618,7 +617,12 @@ async fn get_versions(verbose: bool, param: Value) -> Result<Value, Error> {
 }
 
 async fn run() -> Result<(), Error> {
-    init_cli_logger("PBS_LOG", proxmox_log::LevelFilter::INFO)?;
+    // We need to use the tasklog logger here as well, because the proxmox-backup-manager can and
+    // will directly execute workertasks.
+    proxmox_log::Logger::from_env("PBS_LOG", proxmox_log::LevelFilter::INFO)
+        .stderr_on_no_workertask()
+        .tasklog_pbs()
+        .init()?;
     proxmox_backup::server::notifications::init()?;
 
     let cmd_def = CliCommandMap::new()
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index ce1be1c0d8d9..d9c9c6e03af7 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -16,7 +16,6 @@ use openssl::ssl::SslAcceptor;
 use serde_json::{json, Value};
 
 use proxmox_lang::try_block;
-use proxmox_log::init_logger;
 use proxmox_router::{RpcEnvironment, RpcEnvironmentType};
 use proxmox_sys::fs::CreateOptions;
 use proxmox_sys::logrotate::LogRotate;
@@ -179,7 +178,10 @@ async fn get_index_future(env: RestEnvironment, parts: Parts) -> Response<Body>
 }
 
 async fn run() -> Result<(), Error> {
-    init_logger("PBS_LOG", LevelFilter::INFO)?;
+    proxmox_log::Logger::from_env("PBS_LOG", LevelFilter::INFO)
+        .journald_on_no_workertask()
+        .tasklog_pbs()
+        .init()?;
 
     proxmox_backup::auth_helpers::setup_auth_context(false);
     proxmox_backup::server::notifications::init()?;
diff --git a/src/bin/proxmox-daily-update.rs b/src/bin/proxmox-daily-update.rs
index dbfee07b4cd4..97ccd973f8f7 100644
--- a/src/bin/proxmox-daily-update.rs
+++ b/src/bin/proxmox-daily-update.rs
@@ -110,13 +110,12 @@ async fn run(rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
 fn main() {
     proxmox_backup::tools::setup_safe_path_env();
 
-    if let Err(err) = syslog::init(
-        syslog::Facility::LOG_DAEMON,
-        log::LevelFilter::Info,
-        Some("proxmox-daily-update"),
-    ) {
-        eprintln!("unable to initialize syslog - {err}");
-    }
+    // We need to use the tasklog layer here because we call a workertask.
+    proxmox_log::Logger::from_env("PBS_LOG", proxmox_log::LevelFilter::INFO)
+        .journald_on_no_workertask()
+        .tasklog_pbs()
+        .init()
+        .expect("unable to initialize logger");
 
     let mut rpcenv = CliEnvironment::new();
     rpcenv.set_auth_id(Some(String::from("root@pam")));
diff --git a/src/bin/proxmox-tape.rs b/src/bin/proxmox-tape.rs
index 8e8584b35637..f23b75226390 100644
--- a/src/bin/proxmox-tape.rs
+++ b/src/bin/proxmox-tape.rs
@@ -5,7 +5,6 @@ use serde_json::{json, Value};
 
 use proxmox_human_byte::HumanByte;
 use proxmox_io::ReadExt;
-use proxmox_log::init_cli_logger;
 use proxmox_router::cli::*;
 use proxmox_router::RpcEnvironment;
 use proxmox_schema::api;
@@ -998,7 +997,10 @@ async fn catalog_media(mut param: Value) -> Result<(), Error> {
 }
 
 fn main() {
-    init_cli_logger("PBS_LOG", proxmox_log::LevelFilter::INFO).expect("failed to initiate logger");
+    proxmox_log::Logger::from_env("PBS_LOG", proxmox_log::LevelFilter::INFO)
+        .stderr()
+        .init()
+        .expect("failed to initiate logger");
 
     let cmd_def = CliCommandMap::new()
         .insert(
diff --git a/src/bin/sg-tape-cmd.rs b/src/bin/sg-tape-cmd.rs
index cd14b660a68a..221f4d4f3977 100644
--- a/src/bin/sg-tape-cmd.rs
+++ b/src/bin/sg-tape-cmd.rs
@@ -10,7 +10,6 @@ use pbs_tape::sg_tape::SgTape;
 use proxmox_backup::tape::encryption_keys::load_key;
 use serde_json::Value;
 
-use proxmox_log::init_cli_logger;
 use proxmox_router::{cli::*, RpcEnvironment};
 use proxmox_schema::api;
 use proxmox_uuid::Uuid;
@@ -125,7 +124,9 @@ fn set_encryption(
 }
 
 fn main() -> Result<(), Error> {
-    init_cli_logger("PBS_LOG", proxmox_log::LevelFilter::INFO)?;
+    proxmox_log::Logger::from_env("PBS_LOG", proxmox_log::LevelFilter::INFO)
+        .stderr()
+        .init()?;
 
     // check if we are user root or backup
     let backup_uid = pbs_config::backup_user()?.uid;
-- 
2.39.5



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

[pve-devel] [PATCH proxmox-perl-rs 1/1] log: use new logging builder, print to stderr and journald

[Gabriel Goller]

Use the new builder. Print the logs to stderr and journald always.
Remove the log dependency.

Suggested-by: Lukas Wagner <l.wagner@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 common/src/logger.rs | 7 ++++++-
 pmg-rs/Cargo.toml    | 1 -
 pmg-rs/src/lib.rs    | 2 +-
 pve-rs/Cargo.toml    | 1 -
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/common/src/logger.rs b/common/src/logger.rs
index 1c8940ba4588..946ecab022b1 100644
--- a/common/src/logger.rs
+++ b/common/src/logger.rs
@@ -5,7 +5,12 @@ pub fn init(env_var_name: &str, default_log_level: &str) {
     if let Err(e) = default_log_level
         .parse()
         .map_err(Error::from)
-        .and_then(|default_log_level| proxmox_log::init_logger(env_var_name, default_log_level))
+        .and_then(|default_log_level| {
+            proxmox_log::Logger::from_env(env_var_name, default_log_level)
+                .stderr_pve()
+                .journald()
+                .init()
+        })
     {
         eprintln!("could not set up env_logger: {e:?}");
     }
diff --git a/pmg-rs/Cargo.toml b/pmg-rs/Cargo.toml
index 12526714f060..137434d227ac 100644
--- a/pmg-rs/Cargo.toml
+++ b/pmg-rs/Cargo.toml
@@ -18,7 +18,6 @@ anyhow = "1.0"
 hex = "0.4"
 http = "0.2.7"
 libc = "0.2"
-log = "0.4.17"
 nix = "0.26"
 openssl = "0.10.40"
 serde = "1.0"
diff --git a/pmg-rs/src/lib.rs b/pmg-rs/src/lib.rs
index 3db6966ab263..0ebe2855f8c4 100644
--- a/pmg-rs/src/lib.rs
+++ b/pmg-rs/src/lib.rs
@@ -29,7 +29,7 @@ mod export {
 }
 
 pub fn send_updates_available(_updates: &[&APTUpdateInfo]) -> Result<(), Error> {
-    log::warn!("update notifications are not implemented for PMG yet");
+    tracing::warn!("update notifications are not implemented for PMG yet");
 
     Ok(())
 }
diff --git a/pve-rs/Cargo.toml b/pve-rs/Cargo.toml
index 81c641250da7..80bb034ab986 100644
--- a/pve-rs/Cargo.toml
+++ b/pve-rs/Cargo.toml
@@ -20,7 +20,6 @@ base64 = "0.13"
 hex = "0.4"
 http = "0.2.7"
 libc = "0.2"
-log = "0.4.17"
 nix = "0.26"
 openssl = "0.10.40"
 serde = "1.0"
-- 
2.39.5



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

Re: [pve-devel] [PATCH proxmox{, -backup, -perl-rs} 0/4] Introduce builder for logging initialization

[Gabriel Goller]

wrong list again.
sorry for the noise :(


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