[pbs-devel] [PATCH] notify: migrate from `log` to `tracing`

* [pbs-devel] [PATCH] notify: migrate from `log` to `tracing`
@ 2024-12-05 10:18 Gabriel Goller
  2024-12-05 12:02 ` Lukas Wagner
  2024-12-05 12:22 ` [pbs-devel] applied: " Thomas Lamprecht
  0 siblings, 2 replies; 3+ messages in thread
From: Gabriel Goller @ 2024-12-05 10:18 UTC (permalink / raw)
  To: pbs-devel; +Cc: Lukas Wagner

Migrated from `log` to `tracing`. Imported `tracing` only as it has a
smaller footprint (and less dependencies) than `proxmox_log`.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-notify/Cargo.toml            |  2 +-
 proxmox-notify/src/config.rs         |  6 ++++--
 proxmox-notify/src/context/common.rs |  4 +++-
 proxmox-notify/src/context/pbs.rs    |  8 +++++---
 proxmox-notify/src/endpoints/smtp.rs |  2 +-
 proxmox-notify/src/lib.rs            | 25 +++++++++++--------------
 proxmox-notify/src/matcher.rs        |  5 +++--
 proxmox-notify/src/renderer/mod.rs   |  3 ++-
 8 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index 725bd21070fb..5c700ca88fc9 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -18,7 +18,7 @@ const_format.workspace = true
 handlebars = { workspace = true }
 http = { workspace = true, optional = true }
 lettre = { workspace = true, optional = true }
-log.workspace = true
+tracing.workspace = true
 mail-parser = { workspace = true, optional = true }
 openssl.workspace = true
 percent-encoding = { workspace = true, optional = true }
diff --git a/proxmox-notify/src/config.rs b/proxmox-notify/src/config.rs
index 4d0b53f7aaf1..791e0b5fa6af 100644
--- a/proxmox-notify/src/config.rs
+++ b/proxmox-notify/src/config.rs
@@ -1,5 +1,7 @@
 use std::sync::OnceLock;
 
+use tracing::warn;
+
 use proxmox_schema::{ApiType, ObjectSchema};
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
@@ -148,7 +150,7 @@ pub fn config(raw_config: &str) -> Result<(SectionConfigData, [u8; 32]), Error>
     // This mechanism cleans out left-over entries.
     let entries: Vec<GroupConfig> = data.convert_to_typed_array("group").unwrap_or_default();
     if !entries.is_empty() {
-        log::warn!("clearing left-over 'group' entries from notifications.cfg");
+        warn!("clearing left-over 'group' entries from notifications.cfg");
     }
 
     for entry in entries {
@@ -157,7 +159,7 @@ pub fn config(raw_config: &str) -> Result<(SectionConfigData, [u8; 32]), Error>
 
     let entries: Vec<FilterConfig> = data.convert_to_typed_array("filter").unwrap_or_default();
     if !entries.is_empty() {
-        log::warn!("clearing left-over 'filter' entries from notifications.cfg");
+        warn!("clearing left-over 'filter' entries from notifications.cfg");
     }
 
     for entry in entries {
diff --git a/proxmox-notify/src/context/common.rs b/proxmox-notify/src/context/common.rs
index 7580bd1a318b..88bacf527f5c 100644
--- a/proxmox-notify/src/context/common.rs
+++ b/proxmox-notify/src/context/common.rs
@@ -1,10 +1,12 @@
 use std::path::Path;
 
+use tracing::error;
+
 pub(crate) fn attempt_file_read<P: AsRef<Path>>(path: P) -> Option<String> {
     match proxmox_sys::fs::file_read_optional_string(path) {
         Ok(contents) => contents,
         Err(err) => {
-            log::error!("{err}");
+            error!("{err}");
             None
         }
     }
diff --git a/proxmox-notify/src/context/pbs.rs b/proxmox-notify/src/context/pbs.rs
index 09c555e424cb..cd1176465543 100644
--- a/proxmox-notify/src/context/pbs.rs
+++ b/proxmox-notify/src/context/pbs.rs
@@ -1,6 +1,8 @@
-use serde::Deserialize;
 use std::path::Path;
 
+use serde::Deserialize;
+use tracing::error;
+
 use proxmox_schema::{ObjectSchema, Schema, StringSchema};
 use proxmox_section_config::{SectionConfig, SectionConfigPlugin};
 
@@ -46,13 +48,13 @@ fn lookup_mail_address(content: &str, username: &str) -> Option<String> {
             match parsed.lookup::<DummyPbsUser>("user", username) {
                 Ok(user) => common::normalize_for_return(user.email.as_deref()),
                 Err(err) => {
-                    log::error!("unable to parse {PBS_USER_CFG_FILENAME}: {err}");
+                    error!("unable to parse {PBS_USER_CFG_FILENAME}: {err}");
                     None
                 }
             }
         }
         Err(err) => {
-            log::error!("unable to parse {PBS_USER_CFG_FILENAME}: {err}");
+            error!("unable to parse {PBS_USER_CFG_FILENAME}: {err}");
             None
         }
     }
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index 973f7fadf2b1..6bb2d2d0c3e7 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -336,7 +336,7 @@ impl Endpoint for SmtpEndpoint {
                                 let header = HeaderValue::new(name, value);
                                 message.headers_mut().insert_raw(header);
                             }
-                            Err(e) => log::error!("could not set header: {e}"),
+                            Err(e) => error!("could not set header: {e}"),
                         }
                     }
                 }
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 12f3866b535d..12e59474850b 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -9,6 +9,7 @@ use context::context;
 use serde::{Deserialize, Serialize};
 use serde_json::json;
 use serde_json::Value;
+use tracing::{error, info};
 
 use proxmox_schema::api;
 use proxmox_section_config::SectionConfigData;
@@ -299,9 +300,7 @@ impl Config {
                     if let Some(obj) = value.as_object_mut() {
                         obj.insert("origin".to_string(), Value::String("builtin".into()));
                     } else {
-                        log::error!(
-                            "section config entry is not an object. This should not happen"
-                        );
+                        error!("section config entry is not an object. This should not happen");
                     }
                 } else {
                     // Entry is built-in, but it has been modified by the user.
@@ -311,9 +310,7 @@ impl Config {
                             Value::String("modified-builtin".into()),
                         );
                     } else {
-                        log::error!(
-                            "section config entry is not an object. This should not happen"
-                        );
+                        error!("section config entry is not an object. This should not happen");
                     }
                 }
             } else {
@@ -322,7 +319,7 @@ impl Config {
                 if let Some(obj) = val.as_object_mut() {
                     obj.insert("origin".to_string(), Value::String("builtin".into()));
                 } else {
-                    log::error!("section config entry is not an object. This should not happen");
+                    error!("section config entry is not an object. This should not happen");
                 }
                 config
                     .set_data(key, builtin_typename, val)
@@ -356,7 +353,7 @@ impl Config {
             if let Some(obj) = value.as_object_mut() {
                 obj.remove("origin");
             } else {
-                log::error!("section config entry is not an object. This should not happen");
+                error!("section config entry is not an object. This should not happen");
             }
         }
 
@@ -397,7 +394,7 @@ macro_rules! parse_endpoints_with_private_config {
                 match $config.private_config.sections.get(&config.name) {
                     Some((section_type_name, private_config)) => {
                         if $type_name != section_type_name {
-                            log::error!(
+                            error!(
                                 "Could not instantiate endpoint '{name}': \
                                 private config has wrong type",
                                 name = config.name
@@ -411,7 +408,7 @@ macro_rules! parse_endpoints_with_private_config {
                             private_config: private_config.clone(),
                         }));
                     }
-                    None => log::error!(
+                    None => error!(
                         "Could not instantiate endpoint '{name}': \
                             private config does not exist",
                         name = config.name
@@ -551,21 +548,21 @@ impl Bus {
 
                 if endpoint.disabled() {
                     // Skip this target if it is disabled
-                    log::info!("skipping disabled target '{name}'");
+                    info!("skipping disabled target '{name}'");
                     continue;
                 }
 
                 match endpoint.send(notification) {
                     Ok(_) => {
-                        log::info!("notified via target `{name}`");
+                        info!("notified via target `{name}`");
                     }
                     Err(e) => {
                         // Only log on errors, do not propagate fail to the caller.
-                        log::error!("could not notify via target `{name}`: {e}");
+                        error!("could not notify via target `{name}`: {e}");
                     }
                 }
             } else {
-                log::error!("could not notify via target '{target}', it does not exist");
+                error!("could not notify via target '{target}', it does not exist");
             }
         }
     }
diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs
index 7fb2c9b18b7c..083c2dbda5d5 100644
--- a/proxmox-notify/src/matcher.rs
+++ b/proxmox-notify/src/matcher.rs
@@ -6,6 +6,7 @@ use std::str::FromStr;
 use const_format::concatcp;
 use regex::Regex;
 use serde::{Deserialize, Serialize};
+use tracing::{error, info};
 
 use proxmox_schema::api_types::{COMMENT_SCHEMA, SAFE_ID_REGEX_STR};
 use proxmox_schema::{api, const_regex, ApiStringFormat, Schema, StringSchema, Updater};
@@ -445,7 +446,7 @@ pub fn check_matches<'a>(
     for matcher in matchers {
         if matcher.disable.unwrap_or_default() {
             // Skip this matcher if it is disabled
-            log::info!("skipping disabled matcher '{name}'", name = matcher.name);
+            info!("skipping disabled matcher '{name}'", name = matcher.name);
             continue;
         }
 
@@ -454,7 +455,7 @@ pub fn check_matches<'a>(
                 let t = t.unwrap_or_default();
                 targets.extend(t.iter().map(|s| s.as_str()));
             }
-            Err(err) => log::error!("matcher '{matcher}' failed: {err}", matcher = matcher.name),
+            Err(err) => error!("matcher '{matcher}' failed: {err}", matcher = matcher.name),
         }
     }
 
diff --git a/proxmox-notify/src/renderer/mod.rs b/proxmox-notify/src/renderer/mod.rs
index 393cbbf2775c..e058ea2218b0 100644
--- a/proxmox-notify/src/renderer/mod.rs
+++ b/proxmox-notify/src/renderer/mod.rs
@@ -8,6 +8,7 @@ use handlebars::{
 };
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
+use tracing::error;
 
 use proxmox_human_byte::HumanByte;
 use proxmox_time::TimeSpan;
@@ -142,7 +143,7 @@ impl ValueRenderFunction {
             ValueRenderFunction::Timestamp => value_to_timestamp(value),
         }
         .unwrap_or_else(|| {
-            log::error!("could not render value {value} with renderer {self:?}");
+            error!("could not render value {value} with renderer {self:?}");
             String::from("ERROR")
         })
     }
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 3+ messages in thread