atenet: report "unknown" when a resume does not complete by JeffLuoo · Pull Request #1482 · agent-substrate/substrate

Jeff Luo (JeffLuoo) · 2026-09-04T16:06:46Z

atenet.router.route.duration carries two labels that the router filled in wrongly on a failure path.

`ate.router.resume` said "none" for a failed resume

none means "the resume found the actor already running" — the warm route. The router also gave none to every failed resume, every canceled caller, and every caller shed by a full parking lot. Those failures landed in the warm-route series, so they did not show in the cold-start rate and they polluted the warm-route latency.

A gRPC code cannot recover the fact. A canceled leader's flight outlives its request and keeps restoring the actor, and a DeadlineExceeded can land in the middle of a restore. So the PR does not classify the error. It reserves none, triggered and joined for a resume that completed, and adds a fourth value, unknown, for every resume that did not:

Case	Before	After
Actor already running	`none`	`none`
Leader completed a cold activation	`triggered`	`triggered`
Joiner waited on a completed cold activation	`joined`	`joined`
Resume failed (leader and joiners)	`none`	`unknown`
Caller canceled before the flight finished	`none`	`unknown`
Caller shed by a full parking lot	`none`	`unknown`
Egress, which never resumes an actor	`none`	`unknown`

`ate.template.*` was the empty string on a failure

The registry marks ate.template.atespace and ate.template.name required on this metric, but the router sent an empty string when it failed before it resolved a template. ateattr.NormalizeTemplateDimension now sends the constant "unknown" instead. The constant does not come from the request, thus it adds no caller-controlled label value — the cardinality rule in docs/metrics/substrate.yaml still holds, and the PR records the exception there.

How to read the change on a dashboard

A query that already splits by ate.router.resume gains an unknown series and loses the failures that used to hide inside none. A none rate reads lower after the change and a cold-start rate is unaffected. Do not read the time in the unknown series as an activation time.

Documentation

docs/metrics/registry/metrics.yaml: the unknown member of ate.router.resume, the "unknown" fallback on the two template labels, and a note on the metric.
docs/metrics/substrate.yaml: the cardinality-rule exception.
docs/observability.md: the label list for the metric.

Tests

resumer_test.go: a failed flight gives unknown to the leader and to all nine joiners; a canceled caller gives unknown; a shed caller and a shed joiner give unknown.
metrics_test.go: empty template dimensions record as "unknown"; Result.resume() defaults to unknown.
ateattr_test.go: NormalizeTemplateDimension and the four RouterResume* values.
Tests pass
Appropriate changes to documentation are included in the PR

Krisztian F (krisztianfekete) · 2026-09-07T08:46:35Z

+
+// NormalizeTemplateDimension ensures a template dimension (atespace or name) is
+// non-empty, falling back to TemplateUnknown if unset.
+func NormalizeTemplateDimension(dim string) string {


Nice, but it's still only called from the router though, and the group brief now promises unknown for all 7 metrics that ref these attrs.

Thanks for catching it. This change would touch 7 metrics. I would prefer to do it in a follow-up PR for a cleaner chagne.

Krisztian F (krisztianfekete) · 2026-09-07T08:49:28Z

+// isDefinitiveResumeError reports whether err represents a failure where no cold
+// activation could be attempted (e.g. the actor does not exist, bad request, or
+// permission denied), as opposed to in-flight capacity or transient failures.
+func isDefinitiveResumeError(err error) bool {
+	switch status.Code(err) {
+	case codes.NotFound, codes.InvalidArgument, codes.PermissionDenied, codes.Unauthenticated:
+		return true
+	default:
+		return false
+	}
+}


Can we flip the default? Right now Internal, Unimplemented, Unknown etc. fall through to triggered, so an unrecognized code is as an activation in the latency series.
Unknown means we don't know, so unattempted seems safer.

Also wondering about FailedPrecondition, its registry brief is "the state of the actor did not permit a route" and with parking off it fails immediately, which sounds definitive. And retryable's comment just above already groups DeadlineExceeded with NotFound and PermissionDenied, so it's a bit weird to have two classifiers for the same codes I think.

Wdyt?

Agreed on it. But I want to discuss more on the flipping bit.

If I flip it, I still need a list of codes that mean "an activation was in flight". This is not quite maintainable by looking at the code now.

My proposed change will be:

Delete isDefinitiveResumeError. This removes the classifier.

Make err == nil a condition for triggered and joined. A resume that fails reports the fourth value (added in this PR). FailedPrecondition and ResourceExhausted both go there. It also corrects the words "initiated cold activation" that you found.

Change the name unattempted to unknown. The value includes requests where an activation did operate. One example is a canceled leader. Its flight is separate from the request context, and it continues to restore the actor. A second example is DeadlineExceeded during a restore. Thus unattempted is not true for these requests, and new words in the brief cannot correct this.

WDYT?

Yes, to all of these, you can just make the brief say "resume didn't complete, can't say if an activation ran" or similar.

Thanks. Updated all three I mentioned above.

Krisztian F (krisztianfekete) · 2026-09-07T08:52:04Z

@@ -197,6 +212,9 @@ const (
 	RouterResumeTriggered = "triggered"


initiated cold activation

a request that is ResourceExhausted gets triggered without activating anything. Same wording in the registry brief.

Updated.

A request with ResourceExhausted now gets unknown, not triggered. Only a resume that completed can get trigger.

Krisztian F (krisztianfekete) · 2026-09-07T09:01:31Z

+                No cold activation was in flight. The request was invalid, the
+                actor did not exist, the direction does not resume, or the
+                request canceled before activation was attempted.


"No cold activation was in flight" and "request canceled" contradict each other, a canceled joiner was waiting on someone else's activation. Maybe "this request neither attempted nor observed an activation" is more accurate?

Agreed. The two statements cannot both be true.

The value has the new name unknown, and the brief no longer makes a statement about the activation.

Krisztian F (krisztianfekete) · 2026-09-09T16:16:27Z

+		mock := &resumerMockClient{
+			resumeFn: func(ctx context.Context, in *ateapipb.ResumeActorRequest, opts ...grpc.CallOption) (*ateapipb.ResumeActorResponse, error) {
+				mu.Lock()
+				resumeCalled++


Should we assert this?

The router tagged every failed resume with ate.router.resume="none", the same label a warm route on an already-running actor gets. That put the failures into the warm-route series and hid them from the cold-start rate and the activation-time distribution. A gRPC code cannot say whether an activation ran. A canceled leader's flight outlives its request and keeps restoring the actor, and a DeadlineExceeded can land in the middle of a restore. So reserve "none", "triggered" and "joined" for a resume that completed, and add a fourth value, "unknown", for every resume that did not: a failed flight, a caller that went away, a caller shed by a full parking lot, and a direction that never resumes an actor. atenet.router.route.duration also sent an empty string for ate.template.atespace and ate.template.name when the router failed before it resolved a template. The registry marks both labels required. Send the constant "unknown" instead. The constant does not come from the request, thus it adds no caller-controlled label value. Update the metric registry, the cardinality rules and the observability guide for the new value and the template fallback.

Jeff Luo (JeffLuoo) · 2026-09-18T14:17:15Z

Krisztian F (@krisztianfekete) gentle ping

Krisztian F (krisztianfekete)

Added one comment, the rest looks good to me now, thanks!

The test held the flight open until every caller had started, then slept 20 ms so each one could attach. A caller that had started but not yet reached the flight makes its own RPC once the first one completes, thus a slow runner failed the count on scheduling alone. Run the test in a synctest bubble. synctest.Wait returns only after every caller parks on the flight, so the release cannot race them.

The caller selects on its own context and on the flight's done channel. The mock returned at once, so both cases could be ready and the select picked one at random. The test then saw a triggered resume and no error. It failed within 5000 runs. Hold the flight open for the whole call, so only the cancellation is ever ready. A synctest bubble does not help here, because the race is the select, not the scheduling.

Jeff Luo (JeffLuoo) requested review from Da Huang (git286) and Krisztian F (krisztianfekete) September 4, 2026 16:06

Krisztian F (krisztianfekete) reviewed Sep 4, 2026

View reviewed changes

Jeff Luo (JeffLuoo) force-pushed the fix/issue-1474-router-route-duration-labels branch from 1cbcde7 to 9546f66 Compare September 4, 2026 18:19

Jeff Luo (JeffLuoo) requested a review from Krisztian F (krisztianfekete) September 4, 2026 18:23

Krisztian F (krisztianfekete) reviewed Sep 7, 2026

View reviewed changes

Krisztian F (krisztianfekete) reviewed Sep 9, 2026

View reviewed changes

Jeff Luo (JeffLuoo) requested a review from Krisztian F (krisztianfekete) September 14, 2026 18:17

Jeff Luo (JeffLuoo) force-pushed the fix/issue-1474-router-route-duration-labels branch from 9546f66 to 7cd3661 Compare September 14, 2026 18:20

Bowei Du (bowei) added kind/bug Something isn't working / bugfixes area/network area/observability labels Sep 17, 2026

Jeff Luo (JeffLuoo) force-pushed the fix/issue-1474-router-route-duration-labels branch from 7cd3661 to dfdfc66 Compare September 18, 2026 13:51

Jeff Luo (JeffLuoo) changed the title ~~atenet: preserve cold resume outcome and fallback empty template dimension~~ atenet: report "unknown" when a resume does not complete Sep 18, 2026

Krisztian F (krisztianfekete) reviewed Sep 21, 2026

View reviewed changes

Comment thread cmd/atenet/internal/router/ingress/resumer_test.go

Jeff Luo (JeffLuoo) requested a review from Krisztian F (krisztianfekete) September 21, 2026 13:27

Krisztian F (krisztianfekete) approved these changes Sep 22, 2026

View reviewed changes

Jeff Luo (JeffLuoo) added this pull request to the merge queue Sep 22, 2026

Merged via the queue into agent-substrate:main with commit b374862 Sep 22, 2026
11 checks passed

		@@ -197,6 +212,9 @@ const (
		RouterResumeTriggered = "triggered"

Conversation

Jeff Luo (JeffLuoo) commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ate.router.resume said "none" for a failed resume

ate.template.* was the empty string on a failure

How to read the change on a dashboard

Documentation

Tests

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jeff Luo (JeffLuoo) commented Sep 18, 2026

Uh oh!

Krisztian F (krisztianfekete) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Jeff Luo (JeffLuoo) commented Sep 4, 2026 •

edited

Loading

`ate.router.resume` said "none" for a failed resume

`ate.template.*` was the empty string on a failure