Skip to content

feat(agent): collect and store host diagnostic bundles - #20

Open
memetics19 wants to merge 9 commits into
mainfrom
feat/agent-diagnostics
Open

feat(agent): collect and store host diagnostic bundles#20
memetics19 wants to merge 9 commits into
mainfrom
feat/agent-diagnostics

Conversation

@memetics19

@memetics19 memetics19 commented Aug 18, 2026

Copy link
Copy Markdown
Owner

Summary

Adds read-only diagnostic bundles: pulse-agent --diagnose collects evidence about why a host failed — OOM kills, disk usage, busiest processes, container and systemd state, Proxmox guest status — and Pulse stores it. The goal is answering "why did this break" on a remote host without opening an SSH session.

Collection is on demand only in this PR. Attaching bundles to incidents automatically, and acting on them, are follow-up work.

Type of change

  • feat — new feature
  • fix — bug fix
  • refactor — no behaviour change
  • docs
  • ci / build — pipeline or tooling
  • test
  • chore
  • Breaking change (! / BREAKING CHANGE: footer)

Changes

agent

  • New internal/diagnostics package: collectors for dmesg OOM kills, df usage, ps top processes, docker ps -a, systemctl list-units --failed, and qm list.
  • Collectors take an injected Runner, so they are tested against recorded command output rather than the host the tests run on.
  • Each command is individually timeout-bounded — a wedged host is exactly when diagnostics matter most.
  • Sections degrade independently: a host that denies dmesg, has no Docker, or is not a Proxmox node still produces a useful bundle, with unavailable collectors recording their own error.
  • Captures the recent journal for units already in the failed state and recent output for containers already stopped. An OOM kill says a service died; the log says why. Targets are derived from what the host reports as broken, never taken from the server. Bounded to 200 lines, 5 targets per section, 32 KiB each, so the bundle stays under the 1 MiB request cap.
  • A failing command's own output is folded into its section error. A bare exit status 1 is useless here; cannot connect to the docker daemon and usage: sudo dmesg are the actual diagnoses.
  • --diagnose flag for one-shot collection. Prints the bundle with no server configured, which is the only mode available when Pulse itself is unreachable.
  • postJSON extracted from Push so both ingest paths share one request path.

api

  • Migration 11 adds incident_diagnostics. incident_id is optional — an on-demand bundle describes a host at a moment, not necessarily an incident — and cascades when set.
  • POST /api/ingest/diagnostics, authenticated with the same agent bearer token as metric ingest.
  • The bundle is stored verbatim as JSON: the agent owns the bundle schema, so collectors can change without a server-side migration.
  • collected_at records receipt time, keeping ordering consistent across agents with skewed clocks. The agent's own timestamp survives inside the payload.

docs

  • New docs/diagnostics.md, added to the nav; architecture.md links to it.

chore

  • Per-module coverage.* artifacts added to .gitignore.

Breaking changes

None.

Test plan

  • cd api && go test ./... -count=1
  • cd agent && go test ./... -count=1
  • cd cli && go test ./... -count=1
  • cd ui && npx tsc --noEmit (UI unchanged)
  • gofmt -l clean, go vet ./... clean in all three modules
  • Manual check: ran pulse-agent --diagnose on macOS. disk and docker collected; kernel, processes, systemd, and proxmox degraded to per-section errors as designed, and the bundle stayed usable.

Running it live also caught a bug the unit tests had not: /dev was flagged as a full disk, because pseudo filesystems permanently report 100% capacity. A false "disk full" would drive a wrong diagnosis, so pseudo filesystems are now listed but never flagged. Covered by a regression test.

Known limitations

  • The parsers are tested against representative command output, not against output captured from a real Proxmox host. qm list, systemctl list-units, and the dmesg OOM line should be checked against a live host before relying on them.
  • ps -eo ... --sort=-pcpu is procps-specific. Fine on Debian/Proxmox; it degrades on macOS, so the processes section cannot be smoke-tested on a Mac.
  • Reading the kernel ring buffer needs root. Fine for the systemd unit; the kernel section degrades when the agent runs unprivileged.

Known gaps, deliberately left for follow-ups

  • The agent binary is not shipped in releases. release.yml cross-compiles only api/cmd/pulse, and install.sh never mentions the agent, so pulse-agent --diagnose is only reachable by building from source. Pre-existing, but this PR is the first change that depends on it.
  • Diagnostics are never pruned. PruneIncidentDiagnostics exists but nothing calls it; pruner.Run only prunes check results. The table grows without bound.
  • An unknown incident_id returns 500 and leaks the SQL error rather than a 400.

Adds a diagnostics package that gathers evidence about why a host failed:
OOM kills from dmesg, filesystem usage, the busiest processes, container
and systemd unit state, and Proxmox guest status.

Collectors take an injected Runner, so they are tested against recorded
command output rather than the host the tests run on. Each command is
individually timeout-bounded, since a wedged host is exactly when
diagnostics matter most.

Sections degrade independently: a host that denies dmesg, has no Docker,
or is not a Proxmox node still produces a useful bundle, with the
unavailable collectors recording their own error.

Pseudo filesystems are listed but never reported full — they permanently
report 100% capacity, and a false "disk full" would drive a wrong
diagnosis.
Adds migration 11 with an incident_diagnostics table and a
POST /api/ingest/diagnostics endpoint authenticated with the same agent
bearer token as metric ingest.

The bundle is stored verbatim as JSON: the agent owns the bundle schema,
so collectors can be added or changed without a matching server-side
migration.

incident_id is optional. An on-demand bundle describes a host at a moment
in time and need not belong to an incident; when set, diagnostics cascade
with the incident.

collected_at records receipt time so ordering stays consistent across
agents with skewed clocks. The agent's own timestamp survives inside the
stored payload.
Collects a single diagnostic bundle and prints it. When --server and
--token are supplied it uploads the bundle as well.

Printing works with no server configured at all, which is the only mode
available when Pulse itself is unreachable — the case where a bundle is
most needed.

Extracts the shared postJSON helper from Push so both ingest paths use
one request path.

@memetics19 memetics19 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review decision: Request changes

The fixed-command design, per-section degradation, authenticated agent ownership, and test seams are solid. I found seven actionable correctness/lifecycle issues in the inline comments. The highest-risk path is a degraded host: sequential timeouts can consume the full parent deadline, and the configured upload then starts with an expired context.

I also verified the three-dot PR diff with git diff --check, gofmt, go vet ./..., full tests, and race-enabled tests in agent, api, and cli; all passed. Current GitHub checks are green. One important coverage gap remains: .github/workflows/ci.yml vets/tests api but not the newly changed agent module. Please add the agent module to CI and include blocking-runner, real stderr, malformed-output, CLI flag/exit, bundle-shape, and retention tests with the fixes.

Comment thread agent/internal/diagnostics/collect.go
Comment thread agent/internal/diagnostics/exec.go Outdated
Comment thread agent/internal/diagnostics/disk.go Outdated
Comment thread api/internal/db/queries/diagnostics.sql
Comment thread api/internal/handlers/diagnostics.go Outdated
Comment thread agent/cmd/agent/main.go
Comment thread agent/internal/diagnostics/docker.go
An OOM kill says a service died; the log says why. The agent now pulls
the recent journal for units already in the failed state and recent output
for containers already stopped.

Log targets are derived from what the host reports as broken, never taken
from the server, so this adds no new input the server can influence.

Capture is bounded so a bundle stays under the server's 1 MiB request cap:
the last 200 lines, at most 5 targets per section, truncated to 32 KiB
each. Truncation keeps the tail, where the failure is.

Also folds a failing command's own output into its section error. A bare
"exit status 1" is useless in a tool whose whole job is explaining
failures — "cannot connect to the docker daemon" and "usage: sudo dmesg"
are the actual diagnoses.
… credentials

Collection could consume the whole diagnose deadline, so PushDiagnostics
inherited an expired context and the upload failed on exactly the degraded
hosts this feature targets. The upload now runs on a context detached from
collection; Pusher still bounds the request with its own client timeout.
A wedged host therefore yields a partial bundle that still reaches Pulse.

Suppressing tmpfs and overlay hid real incidents: a full tmpfs is
memory-backed exhaustion and a full overlay is a container's writable layer
filling up. Suppression is now limited to mounts that read 100% by design.
Because df -P names the device rather than the filesystem type, read-only
image mounts are matched by /dev/loop* — a snap squashfs was being flagged
as a full disk.

Supplying only one of --server or --token silently fell back to local-only
mode and exited 0, so automation could believe evidence reached the server
when it never did. It now fails with a message.
PruneIncidentDiagnostics was generated but never called, so bundles
accumulated without bound while the pruner trimmed only check results.
Diagnostics now fall under the same retention window.

The ingest handler accepted any non-empty JSON, storing null, numbers,
strings, and arrays as diagnostic evidence. It now requires a JSON object.
Section contents stay unvalidated — the agent owns that schema.
A killed process surfaces as "signal: killed", which does not tell an
operator the command hit its time limit — the likeliest failure on the
wedged hosts this feature exists to diagnose.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant