ateom-gvisor: measure the sandbox cgroup for GetWorkloadStats - #739
Merged
Da Huang (git286) merged 1 commit intoAug 10, 2026
Merged
Conversation
Tim Bai (baizhenyu)
force-pushed
the
stats-gvisor-cgroup
branch
from
August 10, 2026 14:25
3687951 to
05ee5dd
Compare
Tim Bai (baizhenyu)
requested review from
Jeff Luo (JeffLuoo) and
Da Huang (git286)
August 10, 2026 15:32
Fills in the measurement half of GetWorkloadStats for the gVisor runtime, so it returns real numbers instead of Unimplemented. The micro-VM runtime keeps its stub until the guest-agent read lands. The sample comes from the sandbox's cgroup v2 leaf at /sys/fs/cgroup/pause. gVisor runs every container of a sandbox inside one host process, the sentry, and runsc places that process in the cgroup of the container that created the sandbox -- "pause", the first container RunWorkload and RestoreWorkload create. The actor's own containers get leaves too, but their memory and CPU are the sentry's and are charged there instead, which is why a sample is attributed to the actor rather than to a container. The path follows the "/" + containerName convention runsc.ensureContainerCgroupsPath writes into the OCI spec, resolved against the pod's own cgroup scope that setupCgroupDelegation prepares. The read lives in a new cmd/ateom-gvisor/internal/cgroupstats. It takes the cgroup directory as an argument rather than reaching for an absolute path, so the parsing is testable from a fixture tree without root or a live sandbox, and it carries no build tag so those tests run everywhere. It fails only when memory.current is missing or unparseable -- wrapping fs.ErrNotExist in the first case, so the handler can tell "no cgroup to measure" from "the format is not what we parse". Every other file degrades to zero on its own field, because each has a legitimate reason to be absent on a healthy node: memory.peak predates kernel 5.19, and setupCgroupDelegation enables controllers best-effort, so a cgroup with memory but no cpu is reachable. Reporting no memory numbers because the node could not report CPU would be the wrong trade. Working set is memory.current less memory.stat's inactive_file, saturating at zero rather than wrapping: the two files are read a moment apart and are not a consistent snapshot, so inactive_file can legitimately exceed the memory.current read just before it. The proto's measurement block gains the epoch caveat it was missing. It already said cpu_usage_usec is per-epoch; memory_peak_bytes accumulates the same way and said nothing, so a caller reading it as a lifetime peak would silently under-report -- this source recreates the sandbox cgroup on every restore, and memory.peak restarts with it. The note is phrased per source rather than as one rule, because the two sources do not agree: the guest agent reads counters the guest kernel keeps in its own RAM, and a restored guest brings those back, so an epoch there does not end where it ends here. It also drops the suggestion that watching for a decrease is enough to spot a boundary. An epoch can begin above the value last reported, and then no decrease ever appears. The handler is the first user of the NOT_FOUND / FAILED_PRECONDITION split the previous commit documented, and follows it: available and a UID mismatch are both NOT_FOUND, since each tells the caller the actor is not here and its worker-to-actor mapping wants re-resolving. A missing cgroup under a matching UID is the transient FAILED_PRECONDITION -- usually a poll landing in the boot, since attribution is now retained from the moment the ateom accepts the actor, before runsc has created the leaf. A malformed cgroup is Internal: not a routine race, so it must not be reported as one. The handler takes no lock, which is what the atomic on activeActor bought and what TestGetWorkloadStatsDoesNotTakeLock pins: it holds s.lock across the call, so a handler that reached for it deadlocks. After the read it reloads activeActor and compares pointer identity, so a checkpoint plus a fresh run completing underneath the read is NOT_FOUND rather than misattributed -- the same answer a retry would get, rather than a second code for one state. Deliberately not here: a panic-recovery interceptor. grpc-go serves each RPC on its own goroutine and does not recover handler panics, and the Go runtime kills the process on an unrecovered panic in any goroutine, so a nil dereference in any handler today ends every other RPC the ateom is serving, including an in-flight checkpoint. That gap predates this change and is not made reachable by it: nothing calls GetWorkloadStats yet, and neither the handler (which nil-checks before every dereference) nor the parser (which length-checks before indexing) has a panic path. It wants its own PR, covering every server rather than the two ateoms a telemetry change happens to touch, and it needs to land before the phase that adds a caller polling on a timer. Not covered here: that the sentry lands in /sys/fs/cgroup/pause on a live node is a runsc placement behavior, derived from the spec convention above but not verified from a unit test. Process listings in agent-substrate#161 confirm runsc-sandbox and both gofers sit in the "pause" cgroup, but those runs predate agent-substrate#496, so they establish the leaf name rather than the absolute path. The leaf holds the sentry's own overhead and the gofers alongside the actor's work, which is what the proto means by measuring the SANDBOX; splitting the actor's share out would need the sentry's own accounting. Part of agent-substrate#594
Tim Bai (baizhenyu)
force-pushed
the
stats-gvisor-cgroup
branch
from
August 10, 2026 18:36
05ee5dd to
e2986ef
Compare
Da Huang (git286)
approved these changes
Aug 10, 2026
Da Huang (git286)
merged commit Aug 10, 2026
b746fb4
into
agent-substrate:main
13 of 15 checks passed
Da Huang (git286)
pushed a commit
that referenced
this pull request
Aug 12, 2026
Third and last of the PRs for Phase 0 of #550, and what completes #594. #739 does the gVisor half; this one is independent of it and does not touch it, so the two can land in either order. Fills in the measurement half of `GetWorkloadStats` for the micro-VM runtime, so it returns real numbers instead of `Unimplemented`. ## Where the numbers come from Inside the guest, not from a host cgroup. The host cgroup here holds cloud-hypervisor, and its memory is the guest RAM allocation it took at boot — near-constant, and near-identical for an idle actor and a saturated one. The numbers that move with the workload are the ones the guest kernel keeps, and the kata-agent's `StatsContainer` is what reads them out. `AgentClient` grows that call; it is safe alongside the stdout/stderr forwarding, since ttrpc multiplexes the one connection, which is already what those goroutines rely on. What gets summed is the actor's overlay **workloads**, one per container. Their carriers are deliberately absent: a carrier is created and never started (see `CreateCarrier`), so it runs no process and its cgroup has nothing in it to add. Summing the containers is what turns per-container guest accounting into the one per-actor figure the proto reports. Summing the peaks is an upper bound on the peak of the sum rather than the figure itself — two containers need not have peaked at the same moment — and for the single-container actors this runtime mostly serves it is exact. Flagging it in case you'd rather report the largest single peak instead; I think the bound is the more useful of two imperfect answers. ## The conversion New `cmd/ateom-microvm/internal/agentstats`. Pure: it takes an already-fetched `CgroupStats` and never talks to a guest, which keeps it testable without a live micro-VM and — unlike the rest of the micro-VM ateom — without the `linux` build tag. It never fails. Every field the agent left out reads as zero, and nil stats (what the agent answers for a container it has no accounting for) is a zero sample rather than a panic on a path polled for the life of every workload. Working set subtracts the guest's reclaimable page cache, saturating at zero, and accepts both the v2 name (`inactive_file`) and the v1 hierarchical one (`total_inactive_file`). CPU time is divided by 1000: the agent reports nanoseconds, matching the runc stats struct its own is modeled on, and the proto wants microseconds. A container the agent cannot report contributes nothing instead of failing the sample, and for the common way that happens zero is the *correct* contribution rather than a fallback — a container that has exited took its guest cgroup with it and consumes nothing from here on. Failing an actor's telemetry because one sidecar is gone would be the wrong answer. The sample fails only when no container could be read at all, which is the guest as a whole not answering rather than one container being gone. ## Status codes A deliberate mirror of the gVisor handler, so the two runtimes answer a poller the same way. **`NOT_FOUND`** — available, or a UID mismatch. Each says the actor is not here, and the caller's worker-to-actor mapping wants re-resolving. **`FAILED_PRECONDITION`** — no guest to ask yet. A poll landing in the boot or the restore (attribution is retained from the moment the ateom accepts the actor), or one landing mid-teardown. A guest that answers nothing at all is this code and not `Internal`: the sandbox going away is a routine state here, and the next `CheckpointWorkload` turns it into the `NOT_FOUND` above. ## Locking The handler takes no lock, and that is a stronger constraint here than on the gVisor side, where the cgroup path is a constant. The agent client lives in `AteomService.running`, which `lock` guards, so reading it from the handler would be a data race whatever the read is for. Hence `AteomService.guestStats`: an atomic holding the agent client and the container ids, published once the containers are up and cleared by `teardownActor` before it closes anything. Clearing it there rather than alongside the attribution is what keeps a poll landing mid-teardown on the "no numbers right now" path instead of surfacing a closed connection as a failed read. `TestGetWorkloadStatsDoesNotTakeLock` pins the whole property: it holds `s.lock` across the call, so a handler that reached for it — or that looked the agent up in `running` — deadlocks. After the read the handler reloads `activeActor` and compares pointer identity, so a checkpoint plus a fresh run completing underneath it is `NOT_FOUND` rather than misattributed. ## Epoch semantics differ from the cgroup source `memory_peak_bytes` and `cpu_usage_usec` accumulate, and the epoch they accumulate over does not begin where the gVisor source's does. This is the source the proto's epoch note (landing in #739) is being careful about. There, a restore ends the epoch: `runsc delete` destroys the sandbox cgroup and its counters with it. Here the counters live in the guest kernel's own memory, so `restoreFullScope` — relaunch cloud-hypervisor with `--restore`, then resume — brings them back with the guest RAM rather than restarting them, and a **FULL** restore continues the epoch across what the caller sees as a gap. **DATA** has no guest to resume and cold-boots, so that scope does restart at zero. **DATA_ON_GOLDEN** is the case that defeats the obvious detection: it resumes the *template's* golden guest, so an actor's first sample can begin at whatever the golden had accumulated when it was snapshotted — an epoch beginning *above* the value last reported, with no decrease anywhere for a caller to notice. Nothing here can hide that. A caller wanting a lifetime figure has to accumulate one itself and treat a scope transition as a boundary. ## Proto Two comment-only edits, on different lines than #739 touches: - `STATS_SOURCE_GUEST_AGENT` now says what it counts and what it cannot see — the guest kernel, the agent, and the host VMM process are overhead outside the workload's own containers. The cgroup source is the other way round, since the sandbox's host process is one process and its runtime's overhead is charged along with the workload's. The two sources are not comparable figures and the enum should say so. - The message doc now explains *why* there is no per-container attribution rather than just stating it. This source could give it; the gVisor source cannot split one at all. A field only one runtime could ever fill would be worse than none. ## A gap worth knowing about A restore whose post-restore agent dial fails answers `FAILED_PRECONDITION` for the rest of that activation. Telemetry rides on the connection log forwarding already keeps open, and that dial is best-effort by design — a failed dial must not fail a restore whose actor is already running. A second dial of its own would not help: whatever kept the agent from answering a 15s retry loop would keep it from answering that one too. ## Testing - `agentstats`: a fifteen-case table over `FromCgroupStats` (v2 guest, v1 `total_inactive_file`, both keys present, reclaimable cache at and above usage, missing `memory.stat`, no peak, sub-microsecond truncation, memory-without-cpu and the converse, nil and empty stats), plus `TestSamplePlus` for the summation including saturation. - `GetWorkloadStats`: happy path, `TestGetWorkloadStatsSumsContainers`, `TestGetWorkloadStatsSkipsUnreadableContainer`, `TestGetWorkloadStatsCountsAnsweredContainer`, a seven-case error table, and the no-lock regression test. - The `cmd/ateom-microvm` tests are `//go:build linux` and were run rather than only compile-checked: `go test ./...` is green for the whole repo inside a `golang:1.26.3` container, and the `GetWorkloadStats` tests pass under `-race`. **Draft, because one thing is measured rather than exercised.** The field mapping is a property of the guest kernel and image rather than of this code, and a live guest answering `StatsContainer` needs a micro-VM worker, which needs a nested-virtualization node — neither the GKE dev cluster (`c3-standard-8` with `advancedMachineFeatures` absent) nor a local Mac provides one today. So it was measured against the pinned assets themselves — the kata-static 4.0.0 kernel and rootfs that `hack/microvm-assets/assemble.sh` stages and ateom fetches: - **Which cgroup version a container gets** is not a question that guest can answer two ways. Its kernel is 6.18.35, built `CONFIG_MEMCG=y` with `CONFIG_MEMCG_V1` unset, so a v1 memory controller cannot be mounted there at all. Its init is systemd 255.4, which reports `default-hierarchy=unified` when run from that rootfs, and `buildVMConfig` passes no hierarchy override on the cmdline. Containers get v2, and `inactive_file` is the spelling that appears. - **Whether a high-water mark is reported** resolves the same way. The agent links cgroups-rs 0.5.1, whose v2 path reads `memory.current` into `usage` and `memory.peak` into `max_usage`, and passes `memory.stat` through verbatim. `memory.peak` has existed since 5.19, so on a 6.18 guest the peak is a real figure rather than the zero this defends against. - **The unit conversion** falls out of the same source: v2 has no `cpuacct` controller, so the agent falls through to `cpu.stat` and multiplies `usage_usec` by 1000 — the nanosecond reading the division here turns back into microseconds. The v1 spellings stay anyway. The guest image is a `SandboxConfig` asset, so a cluster can be pointed at a different one, and a wrong guess there should cost a low working-set figure rather than a failed sample. Happy to take it out of draft as-is if reviewers are content with the assets being measured instead of a guest being run; otherwise it waits on a nested-virt node. Part of #594
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.
Second of the three PRs for Phase 0 of #550. #667 has merged, so this is now rebased onto
mainand stands alone as a single commit.Fills in the measurement half of
GetWorkloadStatsfor the gVisor runtime, so it returns real numbers instead ofUnimplemented. The micro-VM runtime keeps its stub until the guest-agent read lands in PR 3, which closes #594.Where the numbers come from
/sys/fs/cgroup/pause, the sandbox's cgroup v2 leaf.gVisor runs every container of a sandbox inside one host process — the sentry — and runsc places that process in the cgroup of the container that created the sandbox. That is
pause, the first containerRunWorkloadandRestoreWorkloadcreate. The actor's own containers get leaves too, but their memory and CPU are the sentry's and are charged there instead. This is why a sample is attributed to the actor and not to a container, as the proto already says.The path follows the
"/" + containerNameconventionrunsc.ensureContainerCgroupsPathwrites into the OCI spec, resolved against the pod's own cgroup scope thatsetupCgroupDelegationprepares (the worker runs in a private cgroup namespace, so/sys/fs/cgroupis the pod's cgroup, not the host root).The read
New
cmd/ateom-gvisor/internal/cgroupstats. It takes the cgroup directory as an argument rather than reaching for an absolute path, so the parsing is testable from a fixture tree without root or a live sandbox — and it carries no build tag, so unlike the rest ofcmd/ateom-gvisorthose tests run on every platform.It fails only when
memory.currentis missing or unparseable, wrappingfs.ErrNotExistin the first case so the handler can tell "no cgroup to measure" from "the format is not what we parse".Every other file degrades to zero on its own field, because each has a legitimate reason to be absent on a healthy node:
memory.peakpredates kernel 5.19, andsetupCgroupDelegationenables controllers best-effort, so a cgroup withmemorybut nocpuis reachable. Reporting no memory numbers because the node could not report CPU seemed like the wrong trade — pushback welcome if you'd rather it be all-or-nothing.Working set is
memory.current − memory.stat:inactive_file, saturating at zero rather than wrapping. The two files are read a moment apart and are not a consistent snapshot, soinactive_filecan legitimately exceed thememory.currentread just before it; onuint64the naive subtraction gives ~1.8e19 instead of ~0.Proto: the epoch caveat (added after review)
The measurement block gains a note it was missing. It already said
cpu_usage_usecis per-epoch;memory_peak_bytesaccumulates the same way and said nothing, so a caller reading it as a lifetime peak would silently under-report after every restore — this source recreates the sandbox cgroup, andmemory.peakrestarts with it.The note is phrased per source rather than as one rule, because the two sources do not agree.
STATS_SOURCE_GUEST_AGENTreads counters the guest kernel keeps in its own RAM, andrestoreFullScopebrings those back from the memory snapshot, so an epoch there does not end where it ends here.It also drops the old suggestion that watching for a decrease is enough to spot a boundary. An epoch can begin above the value last reported —
DATA_ON_GOLDENresumes the template's golden guest — and then no decrease ever appears.Comment-only, plus the regenerated
ateom.pb.go.Status codes
This is the first user of the
NOT_FOUND/FAILED_PRECONDITIONsplit #667 documented, and the split is by what the caller has to do about it.NOT_FOUND— the ateom is not executing the requested actor: it is available, or it was recycled onto a different one. Retrying on the same timer will never change that; the caller's worker-to-actor mapping wants re-resolving.FAILED_PRECONDITION— the requested actor is the one here, but there is nothing to read yet. Since #667 the attribution is retained from the moment the ateom accepts the actor, so a poll landing in the boot arrives before runsc has created the leaf. Transient: skip the sample and take the next one.Internal— the cgroup is there and does not parse. Not a routine race, so it must not be reported as one.The post-read pointer recheck returns
NOT_FOUNDfor the same reason the first two do: it catches exactly the state a retry would land on, and one condition should not report two different codes depending on where in the handler it was noticed.Locking
AteomService.activeActorbecomes anatomic.Pointer. The three lifecycle RPCs still holdAteomService.lockfor their whole bodies and keep doing so — the change is for the reader.lockis not what would make the read slow; it is the only thing that would make a plain field read legal while a writer is running. And it is held across entire boots, restores, and checkpoints (including the readiness wait), so "take the lock to read one pointer" becomes "wait out the boot", with pollers piling up behind it.atomic.Pointermakes the read safe on its own, so the reader never queues. The field is only ever assigned or cleared as a whole pointer, never mutated in place, which is what the type is for.The type makes a lock-free read possible; it does not make one happen.
TestGetWorkloadStatsDoesNotTakeLockpins it: it holdss.lockacross the call, so a handler that ever reaches for the lock deadlocks.The handler reloads the pointer after the read and compares identity, so a checkpoint plus a fresh run completing underneath the read is reported rather than misattributed to the wrong actor.
Deliberately not here: panic recovery
grpc-go serves each RPC on its own goroutine and does not recover handler panics, and the Go runtime kills the process on an unrecovered panic in any goroutine.
grep -rn "recover()"finds nothing in our non-test code, so today a nil dereference in any handler ends every other RPC the ateom is serving — including an in-flight checkpoint.An earlier revision of this PR fixed that for the two ateoms. I have pulled it back out. The gap predates this change and is not made reachable by it: nothing calls
GetWorkloadStatsyet, and neither the handler (which nil-checks before every dereference) nor the parser (which length-checks before indexing) has a panic path. It deserves its own PR covering every server rather than the two ateoms a telemetry change happens to touch, and the real constraint is that it lands before the phase that adds a caller polling on a timer — not before this one.Follow-up worth filing separately
The readiness wait is held under
s.lockin all four boot paths (main.go:364,:639, and the micro-VM equivalents). It is not a runsc subcommand, so it sits outside what the lock's own comment claims to protect, and it is the longest hold in the file. Shrinking it changes lifecycle behavior, so it does not belong in a telemetry PR — but it is the highest-leverage locking fix here.Testing
cgroupstats: nine-case table over fixture trees — all files present, missingmemory.peak, missingcpu.stat, missingmemory.stat,memory.statwithoutinactive_file,inactive_fileabovememory.current, all-zero, malformed/blank/over-long lines, unparseable optional files — plus missing-cgroup and malformed-memory.currentcases that pin which one matchesfs.ErrNotExist.GetWorkloadStats: happy path against a fixture cgroup root, a five-case error table (emptyactor_uid→InvalidArgument; available and UID mismatch →NotFound; vanished cgroup →FailedPrecondition; malformed cgroup →Internal), and the no-lock regression test.shellcheck.shandproto-fmt.shincluded. No shell scripts changed.The
cmd/ateom-gvisorhandler tests are//go:build linux, and they were run rather than only compile-checked:go test ./...is green for the whole repo inside agolang:1.26.3container (linux/arm64), and theGetWorkloadStatstests pass under-race.cgroupstats.Readwas also pointed at that container's live cgroup v2 tree as a one-off check that the parser agrees with a real kernel and not just with its own fixtures: all four files present, and the working set matchingmemory.currentlessinactive_file.The one thing a container cannot cover — that the sentry lands in
/sys/fs/cgroup/pause— was measured on a live worker. The path is derived fromensureContainerCgroupsPath, but the placement is a runsc behavior, so it needed a real node. This branch's own build was deployed to a GKE cluster, an actor was run on it from the counter ActorTemplate, and the worker's processes were read through an ephemeral container sharing the ateom's PID namespace:Both halves of the assumption hold. The sentry is in the
pauseleaf, and the gofer for the counter container is in that same leaf rather than one of its own — which is the one-cgroup-per-sandbox property the handler relies on to report a whole actor from a single read.setupCgroupDelegation's private namespace is visible too: ateom's own processes sit in the siblingateomleaf, so the sandbox's usage is not mixed with the ateom's.The kubelet's cadvisor endpoint reads the same leaf independently, and reports the four values this PR parses:
memory.current39,854,080, peak 41,914,368, working set 38,596,608, cpu 0.238517 s. The working set is consistent with the parser's definition — 39,854,080 less 38,596,608 leaves 1,257,472 ofinactive_file.Not covered:
GetWorkloadStatsanswering over the wire. The ateom serves its control API on a unix socket inside a distroless image, so a live RPC needs a client mounted alongside it — which is the phase that adds the caller, not this one.