From 3367b95b917ac5f442d63ad8c33ef767bcdb4f4e Mon Sep 17 00:00:00 2001
From: Carsten <carsten@ravensburg.it>
Date: Wed, 22 Apr 2026 22:29:26 +0200
Subject: [PATCH] chore(agent): log /proc reads, log diagnostics enable,
 comment trap_exit

Addresses final code review:
- Host collector's /proc reads now go through Diagnostics.log_read/3,
  appearing in commands.log formatted as `$ cat /proc/loadavg`
- configure/1 logs an info line on successful enable so the operator
  has a breadcrumb in the journal
- Writer.init/1 documents the deliberate trap_exit omission
---
 agent/lib/proxmox_agent/collectors/host.ex    | 18 +++++++++++++++---
 agent/lib/proxmox_agent/diagnostics.ex        |  6 ++++++
 agent/lib/proxmox_agent/diagnostics/writer.ex |  3 +++
 agent/test/proxmox_agent/diagnostics_test.exs | 10 ++++++++++
 4 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/agent/lib/proxmox_agent/collectors/host.ex b/agent/lib/proxmox_agent/collectors/host.ex
index cc5dcc6..99c2a07 100644
--- a/agent/lib/proxmox_agent/collectors/host.ex
+++ b/agent/lib/proxmox_agent/collectors/host.ex
@@ -51,13 +51,13 @@ defmodule ProxmoxAgent.Collectors.Host do
   end
 
   defp read_loadavg(proc_dir) do
-    body = File.read!(Path.join(proc_dir, "loadavg"))
+    body = read_and_log(Path.join(proc_dir, "loadavg"))
     [l1, l5, l15 | _] = String.split(body, ~r/\s+/, trim: true)
     {to_float(l1), to_float(l5), to_float(l15)}
   end
 
   defp read_meminfo(proc_dir) do
-    body = File.read!(Path.join(proc_dir, "meminfo"))
+    body = read_and_log(Path.join(proc_dir, "meminfo"))
 
     parsed =
       body
@@ -76,11 +76,23 @@ defmodule ProxmoxAgent.Collectors.Host do
   end
 
   defp read_uptime(proc_dir) do
-    body = File.read!(Path.join(proc_dir, "uptime"))
+    body = read_and_log(Path.join(proc_dir, "uptime"))
     [secs | _] = String.split(body, " ", trim: true)
     secs |> to_float() |> trunc()
   end
 
+  defp read_and_log(path) do
+    start = System.monotonic_time(:microsecond)
+    result = File.read(path)
+    duration_us = System.monotonic_time(:microsecond) - start
+    ProxmoxAgent.Diagnostics.log_read(path, result, duration_us)
+
+    case result do
+      {:ok, body} -> body
+      {:error, reason} -> raise File.Error, reason: reason, action: "read file", path: to_string(path)
+    end
+  end
+
   defp kb_to_bytes(nil), do: nil
 
   defp kb_to_bytes(str) do
diff --git a/agent/lib/proxmox_agent/diagnostics.ex b/agent/lib/proxmox_agent/diagnostics.ex
index 6a3a71d..4f2a3da 100644
--- a/agent/lib/proxmox_agent/diagnostics.ex
+++ b/agent/lib/proxmox_agent/diagnostics.ex
@@ -22,6 +22,7 @@ defmodule ProxmoxAgent.Diagnostics do
     case File.mkdir_p(dir) do
       :ok ->
         Application.put_env(:agent, :dump_dir, dir)
+        Logger.info("diagnostics: enabled, writing to #{dir}")
         :ok
 
       {:error, reason} ->
@@ -46,6 +47,11 @@ defmodule ProxmoxAgent.Diagnostics do
     cast({:sample, kind, payload})
   end
 
+  @spec log_read(Path.t(), command_result(), non_neg_integer()) :: :ok
+  def log_read(path, result, duration_us) do
+    log_command("cat", [to_string(path)], result, duration_us)
+  end
+
   defp disable do
     Application.delete_env(:agent, :dump_dir)
     :ok
diff --git a/agent/lib/proxmox_agent/diagnostics/writer.ex b/agent/lib/proxmox_agent/diagnostics/writer.ex
index 24a89c7..328031c 100644
--- a/agent/lib/proxmox_agent/diagnostics/writer.ex
+++ b/agent/lib/proxmox_agent/diagnostics/writer.ex
@@ -19,6 +19,9 @@ defmodule ProxmoxAgent.Diagnostics.Writer do
   def init(opts) do
     dir = Keyword.fetch!(opts, :dir)
 
+    # No trap_exit: the Writer owns no linked processes other than the supervisor.
+    # If you add a linked port/task, restore Process.flag(:trap_exit, true) and
+    # add a matching handle_info({:EXIT, _, _}, state) clause.
     with {:ok, commands} <- open(Path.join(dir, "commands.log")),
          {:ok, samples} <- open(Path.join(dir, "samples.log")) do
       {:ok, %{dir: dir, commands: commands, samples: samples}}
diff --git a/agent/test/proxmox_agent/diagnostics_test.exs b/agent/test/proxmox_agent/diagnostics_test.exs
index 000bfe2..00692ae 100644
--- a/agent/test/proxmox_agent/diagnostics_test.exs
+++ b/agent/test/proxmox_agent/diagnostics_test.exs
@@ -96,5 +96,15 @@ defmodule ProxmoxAgent.DiagnosticsTest do
       assert body =~ "kind=fast"
       assert body =~ "\"zfs_pools\""
     end
+
+    test "log_read/3 writes to commands.log formatted as cat", %{dir: dir} do
+      assert :ok = Diagnostics.log_read("/proc/loadavg", {:ok, "0.12 0.34 0.56 1/200 3000"}, 150)
+      :ok = GenServer.call(ProxmoxAgent.Diagnostics.Writer, :flush)
+
+      body = File.read!(Path.join(dir, "commands.log"))
+      assert body =~ "$ cat /proc/loadavg"
+      assert body =~ "0.12 0.34 0.56"
+      assert body =~ "exit=0"
+    end
   end
 end