atenet: preserve cold resume outcome and fallback empty template dimension - #1482
Conversation
| // Note: "unknown" is syntactically a legal atespace/template name; like | ||
| // ate.sandbox.class="unknown", this trades potential collision with a real | ||
| // object named "unknown" for maintaining a bounded, non-empty metric dimension. | ||
| const TemplateUnknown = "unknown" |
There was a problem hiding this comment.
Can we have a Normalize for this, like for the rest of unknowns?
There was a problem hiding this comment.
Added normalization in ateattr, following the pattern of NormalizeSandboxClass and NormalizeOperationName, and switched recordRouteDuration to use it. Added unit tests for it in ateattr_test.go as well.
| // - ResumeOutcomeJoined ("joined"): Cold activation joiner (resumed == true or attempt errored, caller's reqID != leaderID). | ||
| outcome := ResumeOutcomeNone | ||
| if callRes.resumed { | ||
| if callRes.resumed || callRes.err != nil { |
There was a problem hiding this comment.
A NotFound never triggered anything, but it now gets resume="triggered". Can we add a new value like unattempted for definitive errors, and keep triggered/joined for the capacity ones where a resume really was in flight?
There was a problem hiding this comment.
Great catch. Added RouterResumeUnattempted = "unattempted" to ateattr and registered it in metrics.yaml.
| // - ResumeOutcomeNone ("none"): resumed == false, actor was already active/running. | ||
| // - ResumeOutcomeTriggered ("triggered"): Cold activation leader (resumed == true, caller's reqID == leaderID). | ||
| // - ResumeOutcomeJoined ("joined"): Cold activation joiner (resumed == true, caller's reqID != leaderID). | ||
| // - ResumeOutcomeNone ("none"): resumed == false and err == nil, actor was already active/running. |
There was a problem hiding this comment.
Here, + the PR says it reserves none for warm hits, but we have multiple places it still returns none on a cancel.
There was a problem hiding this comment.
Updated. Set none only when actual warm hits where the actor was already running.
| The atespace of the ActorTemplate of the actor, or "unknown" if the | ||
| request failed before template resolution. |
There was a problem hiding this comment.
Two things:
- "if the request failed" misses egress. Egress succeeds and still has no template, so it gets unknown too. Result's comment in extproc/handler.go phrases it right, can we mirror that?
- 7 metrics share this brief but only the router sends unknown. And line 72 plus bounded-or-catalog-scoped in docs/metrics/substrate.yaml both say these labels only ever name something an operator made. unknown doesn't, so it needs writing down there.
There was a problem hiding this comment.
Updated both.
| } | ||
| if outcome != ResumeOutcomeNone { | ||
| t.Errorf("expected outcome %q on error, got %q", ResumeOutcomeNone, outcome) | ||
| if outcome != ResumeOutcomeTriggered { |
There was a problem hiding this comment.
Does this expect a nonexistent actor trigering a cold activation basically?
There was a problem hiding this comment.
Fixed. With unattempted introduced, ActorNotFound now expects ResumeOutcomeUnattempted (since no cold activation was ever attempted). Also added a test case for context cancellation verifying it returns unattempted.
…nsions (agent-substrate#1474) Fixes agent-substrate#1474 When atenet-router routes requests: - Mislabeled cold resume failures: Previously, failed resumes were unconditionally overwritten with ate.router.resume="none", distorting cold activation rates and durations. In-flight cold activation attempts (and joiners) now preserve "triggered" and "joined" respectively on capacity and transient errors. Definitive non-activation errors (e.g. NotFound, InvalidArgument), context cancellations, and non-resuming directions (egress) now report "unattempted", strictly reserving "none" for warm hits on running actors. - Missing template labels: Empty template namespace and name dimensions are normalized to "unknown" using ateattr.NormalizeTemplateDimension, satisfying metric registry invariants without using request-derived labels on failure paths. - Documentation: Updated registry metrics.yaml and substrate.yaml to reflect the "unattempted" resume value, mirror handler Result documentation for "unknown" template fallback (including egress), and account for platform-injected unknown dimensions.
1cbcde7 to
9546f66
Compare
2 participants
Fixes #1474
When atenet-router routes requests: