ateom: add GetWorkloadStats RPC and retain actor attribution - #667
ateom: add GetWorkloadStats RPC and retain actor attribution#667Tim Bai (baizhenyu) wants to merge 2 commits into
Conversation
60c4024 to
59407c2
Compare
e591f8b to
3e687b3
Compare
Tim Bai (baizhenyu)
changed the title
Tim Bai (baizhenyu)
changed the title
Adds ateom.Ateom/GetWorkloadStats, the RPC atelet will poll for per-actor
resource usage, plus the attribution retention both ateom runtimes need to
label a sample. The measurement half of each runtime lands in the two
follow-ups; GetWorkloadStats returns Unimplemented until then, so this change
puts nothing half-populated on the wire.
GetWorkloadStats is a pure read: unlike Run/Checkpoint/Restore it does not
move the ateom between "available" and "executing", so it is safe to call on
a timer for a workload's whole lifetime. The request carries the actor UID
the caller believes is executing here, so a recycled worker is rejected with
FAILED_PRECONDITION rather than reporting a different actor's numbers under
the requested actor's name.
sandbox_class and source are enums (SandboxClass, StatsSource) rather than
strings: both are closed sets the ateom binary picks from, and a typo in a
free-form string would silently split a metric in two downstream.
Neither runtime kept the actor's identifying fields past the call that
started the workload -- they arrive on Run/Restore and nothing downstream
needed them. A usage sample is only useful once attributed, so both now hold
them for as long as they are executing:
* ateom-gvisor gains AteomService.activeActor, set by RunWorkload and
RestoreWorkload and cleared by CheckpointWorkload and by both boot-failure
paths, tracking exactly the available/executing state machine.
* ateom-microvm gains runningActor.activeActor, populated from the existing
actorBootParams on both the cold-boot and restore paths; the existing
delete from s.running in teardownActor clears it.
The extraction from the request is shared in internal/ateomstats since both
binaries need it. The type is ActorAttribution, not ActorIdentity: "actor
identity" already means a credential in this repo (ateapi's ActorIdentity
service, substratex509, ateompath.ActorIdentityDirPath), and nothing here is
a secret or is presented as proof of anything.
Part of agent-substrate#594
3e687b3 to
be84808
Compare
|
LGTM for the o11y side but please also get a LGTM from Benjamin Elder (@BenTheElder) on the ateom side |
Benjamin Elder (BenTheElder)
left a comment
Benjamin Elder (BenTheElder)
left a comment
There was a problem hiding this comment.
claude raises a good question:
| } | ||
|
|
||
| ra := &runningActor{chCmd: chCmd, vfsdCmd: vfsdCmd, durableVfsdCmd: durableVfsdCmd, apiSocket: apiSocket, baseID: actorUID, logAgent: ac} | ||
| ra := &runningActor{chCmd: chCmd, vfsdCmd: vfsdCmd, durableVfsdCmd: durableVfsdCmd, apiSocket: apiSocket, baseID: actorUID, logAgent: ac, activeActor: p.actorAttribution()} |
There was a problem hiding this comment.
🤖 question 🟢 – The two runtimes disagree about when a booting actor becomes attributable. Attribution attaches here, on the runningActor built after readyz, so nothing is registered until the boot succeeds. The gVisor side sets s.activeActor before its boot and says why: "so a sample taken against a workload that dies mid-boot is still attributable" (cmd/ateom-gvisor/main.go:287).
Against the proto's state machine that difference is visible: a poll during a micro-VM boot finds no runningActor and gets FAILED_PRECONDITION, i.e. "the ateom is available (nothing to measure)", when it is really mid-activation. The RPC is documented as "safe to call on a timer for the whole lifetime of a workload", so a caller polling across a resume would see that blip on micro-VM and not on gVisor.
Nothing observable yet, since both return Unimplemented — but this PR is the foundation the measurement half builds on, and the semantics are cheaper to settle now. Is the gVisor rationale meant to apply to both, or is post-boot-only the intended contract?
There was a problem hiding this comment.
Both should attach before the boot — the gVisor reason applies to micro-VM too. Post-boot-only wasn't a decision. Fixed in 9bc7fb6e.
It followed from where the field lived. gVisor has a slot on AteomService whose only job is attribution, so RunWorkload sets it before the boot. Micro-VM put attribution on runningActor, which is assembled from the boot's outputs (chCmd, vfsdCmd, apiSocket, logAgent) and so isn't constructed until just after readyz.WaitAll. The gVisor field's comment already claimed micro-VM "holds the same field", so the code asserted a symmetry it didn't have.
Nothing is observable while both handlers return Unimplemented. It bites once measurement lands: an actor that dies mid-boot or loops never reaches readyz, and that's the actor whose usage you'd most want.
Change. Micro-VM gains AteomService.activeActor, set before the boot in RunWorkload and RestoreWorkload and cleared by a deferred check on the error paths — the same points as gVisor. runningActor.activeActor is gone, so there's one source of truth. A single slot because an ateom serves one actor at a time; running is keyed by UID for lookup, not concurrency.
The alternative I rejected was inserting a half-built runningActor into running before the boot and filling it in after. It wouldn't break anything today — teardownActor and snapshotVMState nil-guard every field, because ra can already be nil after an ateom restart — and that's the problem. ra != nil currently means "we own this actor's processes"; a half-built entry takes that branch and silently no-ops. The guards that save it are per-field and incidental, not enforced. It's also only unreachable because s.lock spans both RPCs, and shrinking that hold (the readiness wait) is a follow-up I've already flagged.
Checkpoint clears at the teardown, not the snapshot — the one place the runtimes should differ. runsc's checkpoint takes the sandbox down, so gVisor clears as soon as it returns. The micro-VM guest is only paused until teardownActor, so a checkpoint that failed earlier has left it present, and reporting its usage is the honest answer. Clearing next to the delete from running keeps the two views of "is an actor here" in agreement.
Proto. The contract sentence equated FAILED_PRECONDITION with "available". There's now a third state: executing and attributable, but no sandbox to read yet. Same code on purpose — a timer-driven caller skips the sample either way — so what needed saying is that it means "no numbers right now", not "the actor is gone".
Neither field is atomic yet; each becomes an atomic.Pointer when its runtime's measurement half lands. Keeping the retention points identical now is what makes both mechanical.
Review catch on the previous commit: the two runtimes disagreed about when a booting actor becomes attributable. ateom-gvisor sets AteomService.activeActor before the boot, so a workload that dies mid-boot is still attributable; ateom-microvm only built its runningActor after readyz, so nothing was retained until the boot succeeded. That was not a decision about the contract, it followed from where the field lived. runningActor also holds chCmd, vfsdCmd, apiSocket, and logAgent -- none of which exist until the guest is up -- so it cannot be constructed before the boot. The attribution had been put on the one struct that structurally could not hold it early. The gVisor field's own comment already claimed the two ateoms worked the same way, so the code asserted a symmetry it did not have. Nothing was observable yet, since both handlers return Unimplemented. It shows up once the measurement halves land: an actor that never reaches readyz is one whose usage is most worth having, and micro-VM would have been the runtime that reported nothing for it. ateom-microvm gains AteomService.activeActor, set at the top of RunWorkload and RestoreWorkload and cleared by a deferred check on the error paths, matching the gVisor ateom point for point. runningActor.activeActor goes away rather than leaving two sources of truth. The service holds a single slot because an ateom serves one actor at a time; running is keyed by UID for lookup, not because several can be live at once. Deliberately not done by publishing a half-built runningActor into running early. That is the smaller diff, but CheckpointWorkload reads that map and would find an entry with a nil chCmd and logAgent. The lock makes it unreachable today, and that is the problem: it turns "an entry means a live VM" into a rule with an exception that every teardown path then has to know about. CheckpointWorkload clears at the teardown rather than at the snapshot, which is where the two runtimes legitimately differ. runsc's checkpoint takes the sandbox down, so gVisor clears as soon as it returns; the micro-VM guest is only paused until teardownActor, so a checkpoint that failed before that point has left it present and reporting its usage is the honest answer. Clearing alongside the delete from running keeps the two views of "is an actor here" from disagreeing. The proto sentence equated FAILED_PRECONDITION with the ateom being available. With attribution attached at activation intent there is a third state it has to cover: executing and attributable, but with no sandbox to read yet. Both are the same code on purpose -- a caller polling on a timer skips the sample either way -- so what needed writing down is that it means "no numbers right now" and not "the actor is gone". Neither field is atomic yet. The gVisor one becomes an atomic.Pointer when its cgroup read lands and the micro-VM one when its guest-agent read does; keeping the retention points identical now is what makes each of those a mechanical change. Part of agent-substrate#594
3 participants
Phase 0 of #550, tracked by #594. First of three stacked PRs.
Adds
ateom.Ateom/GetWorkloadStats, the RPC atelet will poll for per-actor resourceusage, plus the actor attribution both ateom runtimes need to label a sample. The measurement half of each runtime lands in the two follow-ups;
GetWorkloadStatsreturnsUnimplementeduntil then, so this change puts nothinghalf-populated on the wire.
The RPC
GetWorkloadStatsis a pure read: unlikeRunWorkload/CheckpointWorkload/RestoreWorkloadit does not move the ateom between "available" and "executing",so it is safe to call on a timer for a workload's whole lifetime.
The request carries the actor UID the caller believes is executing here. A worker
can be recycled between atelet's view of the world and the call landing, so a
mismatch is rejected with
FAILED_PRECONDITIONrather than reporting a differentactor's numbers under the requested actor's identity.
FAILED_PRECONDITIONisalso the answer when the ateom is available — there is nothing to measure.
The response is one sample, measured at sandbox granularity (which today
equals the actor; per-container attribution would need the gVisor sentry's own
accounting). It echoes the measured actor's identity so the caller can attribute
the sample without holding its own worker→actor mapping, and carries
sandbox_classandsourceso two differently-measured numbers are not silentlycompared as though they were the same thing. Both are enums — closed sets the
ateom binary picks from, where a typo in a free-form string would silently split
a metric in two downstream.
Attribution retention
Neither runtime kept the actor's identifying fields past the call that started the
workload — they arrive on Run/Restore and nothing downstream needed them. A usage
sample is only useful once attributed, so both now hold them for as long as they
are executing:
AteomService.activeActor, set byRunWorkloadandRestoreWorkload, cleared byCheckpointWorkloadand by both boot-failurepaths — tracking exactly the available/executing state machine on the service.
runningActor.activeActor, populated from the existingactorBootParamson both the cold-boot and restore paths; the existing deletefrom
s.runninginteardownActorclears it.The extraction from the request is shared in
internal/ateomstats, since bothbinaries need the same mapping.
ActorAttributioncomposes the existingresources.ActorRefrather than repeating its two fields, so the actor'sAtespacestays visibly attached to the actor — worth noting becauseTemplateNamespacebeside it is an unrelated namespace (a Kubernetes one;ActorTemplateis a namespaced CRD, while an atespace is Substrate's owntenancy unit).
It is deliberately not called
ActorIdentity: "actor identity" already meansa credential in this repo — the
ateapi.ActorIdentityservice,substratex509,ateompath.ActorIdentityDirPath, and #670 building on all three. Nothing in thistype is a secret or is presented as proof of anything; it is the tuple a usage
sample is labeled with.
Reviewer notes
activeActorhas no reader in this PR. It is written and cleared but neverread, because both
GetWorkloadStatsbodies are still stubs. The readers arrivewith the measurement in the two follow-ups; the stub comments point there.
identity, but that is a sample, not a datapoint. Per the decisions in Observability At-Scale #174,
actor/atespace identity belongs in logs and traces rather than as TSDB labels,
and only template-level dimensions become metric labels. That conversion is
atelet's, in Phase 1/2.
cpu_usage_usecresets on restore. A restore recreates the sandbox, so thecounter restarts at zero while the actor UID stays the same. Documented on the
field; callers computing deltas must treat a decrease as a reset. If that proves
too subtle in practice, an explicit epoch marker is a backward-compatible
addition later.
workerpool_nameis deliberately absent. atelet does not know it today;adding the field later is backward compatible.
Testing
internal/ateomstats—TestActorAttributionFromRequestpins the mapping fromboth request types, including the empty and nil cases (the callers are not
defensive about the request pointer, so the nil-safety of the generated getters
is load-bearing). Five deliberately distinct placeholder values, so a field
wired to the wrong source is visible.
cmd/ateom-microvm—TestActorBootParamsAttributionpins theactorBootParams→ActorAttributionmapping, andTestActorBootParamsAttributionMatchesRequestchecks that the two hops (request→ boot params → attribution, written in different files) compose back into what
the caller sent.
TestGetWorkloadStatsUnimplementedpins the stub's advertisedcontract, and
TestAteomServiceStartsAvailablechecks a fresh gVisor serviceretains no attribution (a non-nil zero value there would make an idle ateom
report an empty actor's usage instead of refusing).
RunWorkload,RestoreWorkloadandCheckpointWorkloadeach reach for netlink, runsc and theworker pod's netns within a few lines of entry, so there is no seam to drive
them from
go test. Noted incmd/ateom-gvisor/stats_test.gorather thancovered with a fake; the transitions get verified end to end once
GetWorkloadStatsreturns real data.Both ateom packages are
//go:build linux, so their tests do not execute ondarwin. They were run natively on a Linux host in addition to the local
compile-check, along with
hack/verify/shellcheck.sh(which needs docker).Still ahead
/sys/fs/cgroup/pause, notmain/: every application process runs inside the sentry, which is a singleprocess in
pause/.StatsContainerover the existingvsock connection, not the host cgroup (guest RAM is a fixed allocation, so the
host cgroup barely moves with the workload). Closes Phase 0: StatsWorkload RPC on ateom (proto, both runtime reads, identity retention) #594.