ateom: add GetActiveWorkloadStats RPC - #899
Open
Tim Bai (baizhenyu) wants to merge 4 commits into
Open
Conversation
The first piece of agent-substrate#896: a parameterless sibling to GetWorkloadStats for a scraper that enumerates ateoms and holds no worker-to-actor mapping. An available ateom answers an empty stats list rather than an error, an executing one answers the same sample the keyed read would give, and consumers attribute solely from the identity echoed in each sample -- without an asserted uid, the response is the only statement of who was measured. Both runtimes implement it by sharing the measurement half of their existing GetWorkloadStats (extracted as sampleSandbox / sampleGuest); the per-RPC difference is confined to addressing and to what a transition underneath the read means: NOT_FOUND for the keyed read, whose caller asserted an actor that is now gone, FAILED_PRECONDITION for the discovery read, which has no requested actor to disown and should simply take the next sample. Part of agent-substrate#896, toward agent-substrate#550.
Tim Bai (baizhenyu)
changed the title
Tim Bai (baizhenyu)
requested review from
Jeff Luo (JeffLuoo) and
Da Huang (git286)
August 12, 2026 18:42
Review follow-ups on the discovery RPC, both from the same observation: a caller with no prior knowledge finds every ateom state routinely, so none of them should be shaped like an error. * The sample is its own message now. Nesting one RPC's response inside another's tied the two contracts together; WorkloadStatsSample carries the identity + measurements, GetWorkloadStatsResponse wraps one, and GetActiveWorkloadStatsResponse wraps state plus sample. Wire-breaking for GetWorkloadStats, deliberately taken now: the RPC has no callers until the atelet reader lands, so this is the last free moment. * The discovery read answers with a WorkloadState instead of error codes: AVAILABLE with no sample, EXECUTING with one, or EXECUTING with none when there are no numbers to give yet (boot, restore, teardown, transition underneath the read). Error codes on it now mean real failures only. The keyed GetWorkloadStats keeps its NOT_FOUND / FAILED_PRECONDITION contract -- that caller asserted knowledge, and the codes answer the assertion. * The sample slot is singular, like the state that describes it. A repeated field implied an arity the singular state already contradicted -- a response describing several workloads would need per-workload states, restructuring this shape regardless -- so the list bought nothing and cost every caller a loop. Message presence says "no sample" on its own. The per-runtime sample helpers now return raw errors and each RPC maps them to its own contract, which is where the difference between the two reads actually lives. Part of agent-substrate#896, toward agent-substrate#550.
Review follow-ups. The state field and the optional sample could still
express one nonsense combination (a sample on an available ateom); the
oneof cannot, and the reason enum says why there is no sample instead of
which half of the state machine to go ask about:
oneof result {
WorkloadStatsSample sample = 1;
NoSampleReason no_sample_reason = 2; // NO_WORKLOAD | NOT_MEASURABLE_YET
}
A sample being present is itself the statement that a workload is
executing, so WorkloadState is gone.
Also covers the transition re-check with tests on both runtimes -- the
one branch of the semantic split that had none. The micro-VM fake agent
grew an onCall hook and the gVisor service a readSandboxCgroup seam
(mirroring containerStatsReader), so a test can flip activeActor inside
the lock-free read: the discovery read answers with the reason that is
true NOW (NOT_MEASURABLE_YET for a new occupant, NO_WORKLOAD after a
checkpoint), the keyed read answers NOT_FOUND.
Part of agent-substrate#896, toward agent-substrate#550.
| } | ||
|
|
||
| sample, err := s.sampleGuest(ctx, active) | ||
| if err != nil { |
Collaborator
There was a problem hiding this comment.
Should we check the error type here, similar to the gvisor implementation?
Collaborator
Author
There was a problem hiding this comment.
Mostly no — vsock failures have no type structure to check (closed/timeout/refused are all routine "guest not answering", unlike local-fs ENOENT-vs-else). But the question flushed out one real case: the stale-target check is an invariant violation, not a routine state, and it was hiding in the routine bucket. 0ef14de maps it to Internal on both reads via a sentinel, mirroring the gVisor split.
Review follow-up. The routine vsock failures stay one undifferentiated class -- unlike the gVisor runtime's local file reads, a vsock call offers no error type that separates "gone" from "broken" -- but one bug-shaped error was hiding in the routine bucket: the guest target and the attribution disagreeing, which the lifecycle RPCs write together under lock and which should therefore never happen. That is an invariant violation, not a state to skip past, so both stats reads now map it to Internal via the errStaleGuestTarget sentinel, mirroring the gVisor runtime's unexpected-means-Internal split. Part of agent-substrate#896, toward agent-substrate#550.
Da Huang (git286)
approved these changes
Aug 13, 2026
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
2 participants
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.
First piece of #896 (Phase 1 of #550):
GetActiveWorkloadStats, a parameterless sibling toGetWorkloadStatsfor a scraper that enumerates ateoms and holds no worker-to-actor mapping.Contract
statslist, not an error — an idle worker is a normal thing for a scraper to find.FAILED_PRECONDITIONkeeps the meaning it has onGetWorkloadStats: executing, but no numbers yet (mid-boot, guest not answering) — skip this sample, take the next one.Implementation
Both runtimes share the measurement half of their existing
GetWorkloadStats, extracted assampleSandbox(gVisor) /sampleGuest(micro-VM) — the discovery handler is attribution-load, empty-if-nil, same helper, re-check. No change to what is measured or how.The one deliberate semantic split: a lifecycle transition underneath the lock-free read is
NOT_FOUNDon the keyed read (the caller asserted an actor that is now gone; its mapping wants re-resolving) butFAILED_PRECONDITIONon the discovery read (there is no requested actor to disown — the numbers just cannot be attributed to any single actor this tick).Naming note: #896's sketch called this
GetCurrentActiveWorkloadStats; "current" was redundant with "active", so it landed shorter.Testing
FAILED_PRECONDITION; and a cross-check pinning that the discovery sample is identical to the keyed sample against the same fixture — one measurement, two addressing modes.make verifyclean through the proto checks (go-generate.shconfirms the regeneratedateom.pb.go/ateom_grpc.pb.goare canonical).Part of #896, toward #550.