feat(server): expose host disk usage in /_hawser/info (Standard mode) - #92
Open
strausmann wants to merge 2 commits into
Open
feat(server): expose host disk usage in /_hawser/info (Standard mode)#92strausmann wants to merge 2 commits into
strausmann wants to merge 2 commits into
Conversation
Collect() reported DiskTotal/DiskUsed/DiskFree as a plain 0 whenever the Docker data-root statfs call failed (e.g. the path isn't visible in the agent's mount namespace), because HostMetrics used bare uint64 fields with no omitempty. On the wire, "disk stat failed" was indistinguishable from "disk is full" or "0 bytes total". Change the three fields to *uint64 with omitempty: a nil pointer omits the field entirely on a stat failure, a non-nil pointer marshals the real value -- including a legitimate 0 -- so success and failure no longer look alike. This is wire-additive: an unchanged JSON consumer that still expects a plain "number" gets undefined/absent instead of 0, which is a strictly more useful signal and does not break decoding. collectDisk() is split into a standalone, exported DiskUsage(path) that no longer resolves the data root itself, so the statfs call is unit-testable without a live Docker client. Collect() now resolves the data root and calls DiskUsage() directly, applying the result via a small applyDiskMetrics() helper that is also independently tested.
Standard mode's /_hawser/info previously reported only uptime -- no CPU, memory, or disk -- because it has no running metrics.Collector; Edge mode's periodic disk/CPU/memory push simply never applies here. Disk space running out unnoticed is a real, distinct problem for single-node Standard deployments (see Finsys/dockhand#976, #1397), so this adds a one-shot diskTotal/diskUsed/diskFree to the info response using the same field names Edge mode sends on the metrics wire, without pulling in the full Collector (its CPU-delta state and periodic push loop are Edge-specific and not needed here). Reuses metrics.DiskUsage(path), exported for this purpose in the prior commit. addDiskInfo() is split out from handleInfo() so the disk lookup is unit-testable directly against a real and a deliberately missing path, without a live Docker client or an HTTP round trip. On a stat failure the disk* keys are simply left out of the response, matching the "absent means unavailable" convention HostMetrics uses on the Edge wire format -- not a misleading 0. Respects SKIP_DF_COLLECTION for the same reason Edge mode does (statfs can be slow on hosts with many mounts). CPU and memory are out of scope here: adding them would mean either duplicating Collector's CPU-delta/mutex state per-request or instantiating a long-lived Collector in the Standard-mode server, which is a larger design decision left for a separate change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Standard mode's
/_hawser/infoonly ever reporteduptime-- no CPU, memory, or disk -- becausemetrics.Collector(the thing that gathers all of that for Edge mode's periodic metrics push) isnever instantiated in the Standard-mode server. Disk space running out unnoticed is a real,
distinct problem for single-node Standard deployments, independent of the Edge-mode metrics
channel (see Finsys/dockhand#976, Finsys/dockhand#1397).
Change
Add a one-shot
diskTotal/diskUsed/diskFree(bytes) to the/_hawser/inforesponse, usingthe same field names Edge mode sends on the metrics wire. Reuses
metrics.DiskUsage(path)(exported in #91) rather than duplicating the
statfslogic or instantiating a fullCollector-- Standard mode already holds a
*docker.Client(s.dockerClient), so resolving the data rootand statting it is a cheap, stateless per-request call.
On a stat failure the three keys are simply left out of the response, matching the
"absent means unavailable" convention from #91 -- not a misleading
0. RespectsSKIP_DF_COLLECTIONfor the same reason Edge mode does (hosts with many mounted volumes canmake
statfsslow).Out of scope (deliberately)
CPU and memory are not added here. Doing so would mean either duplicating
Collector'sCPU-delta/mutex state per HTTP request, or instantiating a long-lived
Collectorinside theStandard-mode server -- both larger design decisions (does Standard mode get a periodic metrics
loop of its own? does
/_hawser/infobecome a snapshot of that, or stay request-driven?) thatare better made as their own change/issue rather than folded into a disk-only fix. Disk is
uniquely simple here because a single
statfscall is stateless; CPU usage needs two samplesover time.
Tests
New
internal/server/http_test.go(package had no tests before):addDiskInfo()success path: real temp dir,used + free == total, unrelated map keysuntouched
addDiskInfo()error path: missing path → all threedisk*keys absent, not0go build ./...,go vet ./...,go test ./... -raceall pass.