[pve-devel] [PATCH proxmox v2] worker_task: write result message manually, bypassing tracing

* [pve-devel] [PATCH proxmox v2] worker_task: write result message manually, bypassing tracing
@ 2025-02-18 14:52 Gabriel Goller
  2025-02-18 14:57 ` Gabriel Goller
  2025-02-18 15:05 ` Wolfgang Bumiller
  0 siblings, 2 replies; 4+ messages in thread
From: Gabriel Goller @ 2025-02-18 14:52 UTC (permalink / raw)
  To: pve-devel

The workertasks currently get their status from parsing the log
messages in the task-log file. The problem is that if these messages are
filtered – which is now possible using the PBS_LOG env variable – some
workertasks will end up with a "stopped: unknown" status. This is not
desirable so write the message manually to the workertask file and
bypass tracing.

This way we are guaranteed that, regardless of the max logging level the
user sets, the final message (and status) is written.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---

Changelog:
v2:
 * remove jank (detect workertask failure based on return value instead
   of message)

 proxmox-log/src/lib.rs                 | 17 +++++++++++++++++
 proxmox-rest-server/src/worker_task.rs | 16 +++++++++++++---
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/proxmox-log/src/lib.rs b/proxmox-log/src/lib.rs
index 8c74e42b618d..755d1b4a850c 100644
--- a/proxmox-log/src/lib.rs
+++ b/proxmox-log/src/lib.rs
@@ -191,3 +191,20 @@ pub fn init_cli_logger(
     LogTracer::init_with_filter(log_level.as_log())?;
     Ok(())
 }
+
+/// Write manually to the current tasklog bypassing the whole tracing infrastructure. Note that this
+/// will also bypass all the filtering and writing to journald or elsewhere. If has_failed is true,
+/// print to stderr as well.
+pub fn log_manually_to_tasklog(msg: &str, has_failed: bool) -> Result<(), anyhow::Error> {
+    if let Some(ctx) = LogContext::current() {
+        let mut logger = ctx.logger.lock().unwrap();
+        // If the message is an error, print to stderr (which will end up in journald) as well.
+        if has_failed {
+            eprintln!("{msg}");
+        }
+        logger.logger.log(msg);
+        Ok(())
+    } else {
+        anyhow::bail!("Unable to manually write message to workertask log file: No workertask exists in this task.");
+    }
+}
diff --git a/proxmox-rest-server/src/worker_task.rs b/proxmox-rest-server/src/worker_task.rs
index 24e2676e69c7..06d986f41dbb 100644
--- a/proxmox-rest-server/src/worker_task.rs
+++ b/proxmox-rest-server/src/worker_task.rs
@@ -7,14 +7,14 @@ use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::sync::{Arc, LazyLock, Mutex, OnceLock};
 use std::time::{Duration, SystemTime};
 
-use anyhow::{bail, format_err, Error};
+use anyhow::{bail, format_err, Context, Error};
 use futures::*;
 use nix::fcntl::OFlag;
 use serde::{Deserialize, Serialize};
 use serde_json::{json, Value};
 use tokio::signal::unix::SignalKind;
 use tokio::sync::{oneshot, watch};
-use tracing::{info, warn};
+use tracing::{error, info, warn};
 
 use proxmox_daemon::command_socket::CommandSocket;
 use proxmox_lang::try_block;
@@ -1025,7 +1025,17 @@ impl WorkerTask {
     /// Log task result, remove task from running list
     pub fn log_result(&self, result: &Result<(), Error>) {
         let state = self.create_state(result);
-        self.log_message(state.result_text());
+
+        // Write the result manually to the workertask file. We don't want to filter or process
+        // this message by the logging system. This also guarantees the result message will be in
+        // the file, regardless of the logging level.
+        proxmox_log::log_manually_to_tasklog(&state.result_text(), result.is_err())
+            .context("Error writing final result message")
+            .unwrap_or_else(|err| {
+                // Note: this will probably also fail to be written to the tasklog, but at least it
+                // ends up in the journal.
+                error!("{err:#}");
+            });
 
         WORKER_TASK_LIST.lock().unwrap().remove(&self.upid.task_id);
         // this wants to access WORKER_TASK_LIST, so we need to drop the lock above
-- 
2.39.5



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

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