From e3c36b6567bf652ebbf7cfee453e36dca1698508 Mon Sep 17 00:00:00 2001 From: Jeff Luo Date: Fri, 14 Aug 2026 10:30:10 -0400 Subject: [PATCH 1/2] ateapi: pair ate.workerpool.namespace with ate.workerpool.name Fixes #951. ate.actor.crashes, ate.actor.lifecycle.operation.duration and ate.scheduler.assignment.duration named a WorkerPool by name alone. A WorkerPool is namespaced, so same-named pools in different namespaces merged into one series, and the three could not join the instruments that already carry both keys. ateattr.WorkerPoolAttributes now builds the pair, and omits both keys when no pool is assigned, so a crash before the actor reached a worker no longer reports an empty-string pool. This changes the series identity of the three instruments. --- cmd/ateapi/internal/controlapi/metrics.go | 15 ++-- .../internal/controlapi/metrics_test.go | 38 +++++---- .../internal/controlapi/workflow_resume.go | 4 +- docs/observability.md | 8 +- internal/ateattr/ateattr.go | 33 ++++++-- internal/ateattr/ateattr_test.go | 77 +++++++++++++++---- internal/e2e/suites/metrics/metrics_test.go | 11 ++- 7 files changed, 136 insertions(+), 50 deletions(-) diff --git a/cmd/ateapi/internal/controlapi/metrics.go b/cmd/ateapi/internal/controlapi/metrics.go index 3df184ed1..954328d96 100644 --- a/cmd/ateapi/internal/controlapi/metrics.go +++ b/cmd/ateapi/internal/controlapi/metrics.go @@ -180,14 +180,14 @@ func (i *Instruments) recordLifecycleOp(ctx context.Context, op string, start ti // an empty-string series. snapshotKind is empty for suspend/pause, which do not // restore; snapshotScope applies to all three and is what separates a restore // combined with the template's golden state from a plain one of the same kind. +// The pool keys are set together or not at all; see ateattr.WorkerPoolAttributes. func lifecycleOpAttrs(actor *ateapipb.Actor, template *atev1alpha1.ActorTemplate, snapshotKind, snapshotScope string) []attribute.KeyValue { attrs := []attribute.KeyValue{ ateattr.TemplateNameKey.String(actor.GetActorTemplateName()), ateattr.TemplateNamespaceKey.String(actor.GetActorTemplateNamespace()), } - if pool := actor.GetWorkerAssignment().GetWorkerPool(); pool != "" { - attrs = append(attrs, ateattr.WorkerPoolNameKey.String(pool)) - } + ass := actor.GetWorkerAssignment() + attrs = append(attrs, ateattr.WorkerPoolAttributes(ass.GetWorkerNamespace(), ass.GetWorkerPool())...) if template != nil { attrs = append(attrs, ateattr.SandboxClassKey.String(string(template.Spec.SandboxClass))) } @@ -205,15 +205,14 @@ func lifecycleOpAttrs(actor *ateapipb.Actor, template *atev1alpha1.ActorTemplate // no_free_worker (a capacity signal, not a failure) carries neither. class is // set on every outcome it is known for, so no_free_worker names the capacity // that ran out and stays comparable with assigned. -func (i *Instruments) recordSchedulerAssignment(ctx context.Context, start time.Time, outcome, pool, class string, err error) { +// The pool keys are set together or not at all; see ateattr.WorkerPoolAttributes. +func (i *Instruments) recordSchedulerAssignment(ctx context.Context, start time.Time, outcome, poolNamespace, pool, class string, err error) { if i == nil || i.schedulerAssignmentDuration == nil { return } - attrs := make([]attribute.KeyValue, 0, 4) + attrs := make([]attribute.KeyValue, 0, 5) attrs = append(attrs, ateattr.SchedulerOutcomeKey.String(outcome)) - if pool != "" { - attrs = append(attrs, ateattr.WorkerPoolNameKey.String(pool)) - } + attrs = append(attrs, ateattr.WorkerPoolAttributes(poolNamespace, pool)...) if class != "" { attrs = append(attrs, ateattr.SandboxClassKey.String(class)) } diff --git a/cmd/ateapi/internal/controlapi/metrics_test.go b/cmd/ateapi/internal/controlapi/metrics_test.go index 9ae491528..ad2e5b039 100644 --- a/cmd/ateapi/internal/controlapi/metrics_test.go +++ b/cmd/ateapi/internal/controlapi/metrics_test.go @@ -210,7 +210,7 @@ func TestLifecycleOpDurationShape(t *testing.T) { actor := &ateapipb.Actor{ ActorTemplateName: "support-agent", ActorTemplateNamespace: "ate-agents", - WorkerAssignment: &ateapipb.WorkerAssignment{WorkerPool: "pool-a"}, + WorkerAssignment: &ateapipb.WorkerAssignment{WorkerNamespace: "ate-workers", WorkerPool: "pool-a"}, } template := &atev1alpha1.ActorTemplate{ Spec: atev1alpha1.ActorTemplateSpec{SandboxClass: atev1alpha1.SandboxClassGvisor}, @@ -223,6 +223,7 @@ func TestLifecycleOpDurationShape(t *testing.T) { ateattr.ActorOperationNameKey, ateattr.TemplateNameKey, ateattr.TemplateNamespaceKey, + ateattr.WorkerPoolNamespaceKey, ateattr.WorkerPoolNameKey, ateattr.SandboxClassKey, ateattr.SnapshotKindKey, @@ -231,6 +232,11 @@ func TestLifecycleOpDurationShape(t *testing.T) { if op, _ := attrString(dp, ateattr.ActorOperationNameKey); op != ateattr.OperationResume { t.Errorf("operation = %q, want %q", op, ateattr.OperationResume) } + // The pool is only identified by the pair: two pools may share a name in + // different namespaces. + if ns, _ := attrString(dp, ateattr.WorkerPoolNamespaceKey); ns != "ate-workers" { + t.Errorf("worker pool namespace = %q, want %q", ns, "ate-workers") + } // Kind and scope are independent: a data_on_golden restore of the actor's // own latest snapshot must stay distinguishable from one of a local snapshot. if scope, _ := attrString(dp, ateattr.SnapshotScopeKey); scope != ateattr.SnapshotScopeDataOnGolden { @@ -243,11 +249,13 @@ func TestLifecycleOpDurationShape(t *testing.T) { // TestLifecycleOpAttrsOmitsUnknownScope guards the failure path: a resume that // dies before the restore request is built has no scope, and an empty-string -// series would be indistinguishable from a real one. +// series would be indistinguishable from a real one. The unassigned actor also +// pins the pool pair: a failure before the assign step emits neither key. func TestLifecycleOpAttrsOmitsUnknownScope(t *testing.T) { actor := &ateapipb.Actor{ActorTemplateName: "support-agent", ActorTemplateNamespace: "ate-agents"} for _, kv := range lifecycleOpAttrs(actor, nil, "", "") { - if kv.Key == ateattr.SnapshotScopeKey || kv.Key == ateattr.SnapshotKindKey { + switch kv.Key { + case ateattr.SnapshotScopeKey, ateattr.SnapshotKindKey, ateattr.WorkerPoolNamespaceKey, ateattr.WorkerPoolNameKey: t.Errorf("attribute %s must be omitted while unknown, got %q", kv.Key, kv.Value.AsString()) } } @@ -293,12 +301,13 @@ func TestRecordLifecycleOp_OutcomeClassification(t *testing.T) { } // TestSchedulerAssignmentShapeAndOutcomes asserts the assignment histogram stamps -// pool only when a worker was assigned and error.type only for the error outcome, -// so no_free_worker (a capacity signal) carries neither. +// the pool pair only when a worker was assigned and error.type only for the error +// outcome, so no_free_worker (a capacity signal) carries neither. func TestSchedulerAssignmentShapeAndOutcomes(t *testing.T) { tests := []struct { name string outcome string + poolNamespace string pool string class string err error @@ -306,15 +315,16 @@ func TestSchedulerAssignmentShapeAndOutcomes(t *testing.T) { wantErrorType string }{ { - name: "assigned stamps pool and class, no error.type", - outcome: ateattr.SchedulerOutcomeAssigned, - pool: "pool-a", - class: "gvisor", - err: nil, - wantKeys: []attribute.Key{ateattr.SchedulerOutcomeKey, ateattr.WorkerPoolNameKey, ateattr.SandboxClassKey}, + name: "assigned stamps the pool pair and class, no error.type", + outcome: ateattr.SchedulerOutcomeAssigned, + poolNamespace: "ate-workers", + pool: "pool-a", + class: "gvisor", + err: nil, + wantKeys: []attribute.Key{ateattr.SchedulerOutcomeKey, ateattr.WorkerPoolNamespaceKey, ateattr.WorkerPoolNameKey, ateattr.SandboxClassKey}, }, { - name: "no_free_worker carries class but neither pool nor error.type", + name: "no_free_worker carries class but neither pool key nor error.type", outcome: ateattr.SchedulerOutcomeNoFreeWorker, pool: "", class: "gvisor", @@ -322,7 +332,7 @@ func TestSchedulerAssignmentShapeAndOutcomes(t *testing.T) { wantKeys: []attribute.Key{ateattr.SchedulerOutcomeKey, ateattr.SandboxClassKey}, }, { - name: "error carries error.type, no pool, class omitted when unknown", + name: "error carries error.type, no pool keys, class omitted when unknown", outcome: ateattr.SchedulerOutcomeError, pool: "", class: "", @@ -334,7 +344,7 @@ func TestSchedulerAssignmentShapeAndOutcomes(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { inst, reader := newTestInstruments(t) - inst.recordSchedulerAssignment(context.Background(), time.Now(), tt.outcome, tt.pool, tt.class, tt.err) + inst.recordSchedulerAssignment(context.Background(), time.Now(), tt.outcome, tt.poolNamespace, tt.pool, tt.class, tt.err) dp := singleHistogramDP(t, reader, schedulerAssignmentMetric) assertAttrKeys(t, dp, tt.wantKeys...) diff --git a/cmd/ateapi/internal/controlapi/workflow_resume.go b/cmd/ateapi/internal/controlapi/workflow_resume.go index 6666168d0..49eb393c5 100644 --- a/cmd/ateapi/internal/controlapi/workflow_resume.go +++ b/cmd/ateapi/internal/controlapi/workflow_resume.go @@ -419,6 +419,7 @@ func schedulerRecordable(err error) bool { func (w *ActorWorkflow) assignWorkerAttempt(ctx context.Context, actorRef resources.ActorRef, actor *ateapipb.Actor, actorTemplate *atev1alpha1.ActorTemplate) (_ *ateapipb.Actor, _ *ateapipb.Worker, err error) { start := time.Now() outcome := ateattr.SchedulerOutcomeError + poolNamespace := "" pool := "" class := "" if actorTemplate != nil { @@ -426,7 +427,7 @@ func (w *ActorWorkflow) assignWorkerAttempt(ctx context.Context, actorRef resour } defer func() { if schedulerRecordable(err) { - w.instruments.recordSchedulerAssignment(ctx, start, outcome, pool, class, err) + w.instruments.recordSchedulerAssignment(ctx, start, outcome, poolNamespace, pool, class, err) } }() @@ -531,6 +532,7 @@ func (w *ActorWorkflow) assignWorkerAttempt(ctx context.Context, actorRef resour return nil, nil, status.Errorf(codes.Aborted, "actor %s is %s and can no longer be resumed", actorRef, fresh.GetStatus()) } } + poolNamespace = assignedWorker.GetWorkerNamespace() pool = assignedWorker.GetWorkerPool() outcome = ateattr.SchedulerOutcomeAssigned return storedActor, assignedWorker, nil diff --git a/docs/observability.md b/docs/observability.md index 61d92f6f6..87855278e 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -110,7 +110,7 @@ Agent Substrate emits foundational OpenTelemetry system and server metrics to mo | Metric | Emitted by | Type | Measures | |--------|------------|------|----------| | `rpc.server.call.duration` | ateapi & atelet (gRPC servers, via `otelgrpc`) | histogram | per-method gRPC latency, request rate, and errors (labels `rpc.method`, `rpc.response.status_code`) | -| `ate.actor.crashes` | ateapi | counter | Number of times actors transitioned to `STATUS_CRASHED` with failure reasons (labels `ate.actor.operation.name`, `ate.failure.reason`, `ate.template.namespace`, `ate.template.name`, `ate.workerpool.name`, `ate.sandbox.class`) | +| `ate.actor.crashes` | ateapi | counter | Number of times actors transitioned to `STATUS_CRASHED` with failure reasons (labels `ate.actor.operation.name`, `ate.failure.reason`, `ate.template.namespace`, `ate.template.name`, `ate.workerpool.namespace`, `ate.workerpool.name`, `ate.sandbox.class`) | | `atenet.router.route.duration` | atenet-router | histogram | Substrate E2E — Envoy receiving a request to Envoy forwarding it to the resolved worker, excluding actor compute and the response (labels `ate.template.namespace`, `ate.template.name`, `ate.router.outcome`, `ate.router.resume`) | | `ate.scheduler.eligible_workers` | ateapi | histogram | number of eligible unassigned workers available during scheduling given the constraint filters (labels `ate.workerpool.namespace`, `ate.workerpool.name`, `ate.sandbox.class`, `ate.scheduling.constraint`) | | `atelet.snapshot.size` | atelet | histogram | uncompressed size in bytes of each gVisor snapshot image written during checkpoint (labels `file.name`, `ate.template.namespace`, `ate.template.name`) | @@ -119,8 +119,8 @@ Agent Substrate emits foundational OpenTelemetry system and server metrics to mo | `ate.workerpool.ready_workers` | atecontroller | up/down counter | number of worker pods currently ready for a WorkerPool, from `status.readyReplicas` (labels `ate.workerpool.namespace`, `ate.workerpool.name`) | | `ate.workerpool.workers` | ateapi | up/down counter | live worker count per pool, split by state (`idle`/`assigned`) and sandbox class to provide fleet capacity and saturation at a glance | -| `ate.actor.lifecycle.operation.duration` | ateapi | histogram | how long each actor operation (create/resume/suspend/pause/delete) takes and whether it failed (`error.type` present = failure, absent = success); labeled by operation, template, pool, sandbox class, and snapshot kind and scope on resume; already-running resume no-ops are not recorded so the histogram tracks actual activations, not router traffic | -| `ate.scheduler.assignment.duration` | ateapi | histogram | time it takes for an actor to be assigned to a worker, per attempt (version-conflict retries record only the final attempt), with the outcome (`assigned` / `no_free_worker` / `error`) and sandbox class to catch scheduling latency and capacity starvation problems | +| `ate.actor.lifecycle.operation.duration` | ateapi | histogram | how long each actor operation (create/resume/suspend/pause/delete) takes and whether it failed (`error.type` present = failure, absent = success); labeled by operation, template, pool (`ate.workerpool.namespace` + `ate.workerpool.name`), sandbox class, and snapshot kind and scope on resume; already-running resume no-ops are not recorded so the histogram tracks actual activations, not router traffic | +| `ate.scheduler.assignment.duration` | ateapi | histogram | time it takes for an actor to be assigned to a worker, per attempt (version-conflict retries record only the final attempt), with the outcome (`assigned` / `no_free_worker` / `error`), the assigned pool (`ate.workerpool.namespace` + `ate.workerpool.name`) and sandbox class to catch scheduling latency and capacity starvation problems | | `ate.actor.restore.duration` | atelet | histogram | how long each phase of a restore takes on the worker node, which is where cold-start latency actually goes once ateapi hands off (labels `ate.snapshot.phase`, `ate.snapshot.kind`, `ate.snapshot.scope`, `ate.template.namespace`, `ate.template.name`, `ate.sandbox.class`, plus `ate.failure.reason` on failure) | | `ate.actor.checkpoint.duration` | atelet | histogram | the same phase breakdown for writing a snapshot, so a slow suspend can be attributed to ateom or to the upload (same labels as the restore histogram) | | `ate.imagecache.requests` | atelet | counter | image lookups in the node-local image cache, by outcome (`ate.imagecache.outcome`), with `error.type` on the `error` outcome. A miss pays for the pull and the unpack, so the hit ratio per node is a leading indicator of resume latency | @@ -142,6 +142,8 @@ For `ate.imagecache.requests`: * `ate.imagecache.outcome` is `hit` when the node holds a complete image record — every layer directory the record names is present — and `miss` when the lookup must pull. A failed lookup is neither: `error` is a failed lookup whatever the cause, and `cancelled` or `timeout` is the caller giving up, as on `ate.router.outcome`. So the hit ratio is `hit / (hit + miss)`, with failures and abandoned lookups out of the denominator. * `error.type` is present only on the `error` outcome, and carries the registry's own HTTP status for its rejection, from a fixed set: `401`, `403`, `404`, `429`, `500`, `502`, `503`, `504`. The set is an allow-list because the registry client reports whatever the remote returned. Each other status, and each failure that carries no status, reports `_OTHER`. +`ate.workerpool.namespace` and `ate.workerpool.name` identify a pool together, on every instrument that names one. A WorkerPool is a namespaced resource, so the name on its own merges same-named pools from different namespaces into one series. The pair means that capacity (`ate.workerpool.workers`, `ate.workerpool.desired_workers`, `ate.workerpool.ready_workers`) joins to demand (`ate.scheduler.assignment.duration`, `ate.actor.lifecycle.operation.duration`, `ate.actor.crashes`) by pool. On the actor-centric instruments, an operation that has no pool yet — a crash before the actor reached a worker, or the `no_free_worker` outcome — carries neither key rather than an empty-string one. + The three snapshot labels are orthogonal and mean the same thing on every histogram that carries them: * `ate.snapshot.kind`: which snapshot the operation reads or writes. `local` (node-local, written by a pause), `latest` (the actor's own durable snapshot), `golden` (the template's image), or `boot` (from scratch, so it never appears on the atelet histograms). * `ate.snapshot.scope`: what content it covers. `full`, `data`, or `data_on_golden` (restore-only: the actor's data combined with the golden guest state). diff --git a/internal/ateattr/ateattr.go b/internal/ateattr/ateattr.go index 7a938d0ca..3f4a45ecc 100644 --- a/internal/ateattr/ateattr.go +++ b/internal/ateattr/ateattr.go @@ -251,6 +251,26 @@ func NormalizeSandboxClass(class string) string { } } +// WorkerPoolAttributes returns the namespaced identity of a WorkerPool. The two +// keys are always set together, or not at all: a WorkerPool is namespaced, so +// the name alone merges same-named pools from different namespaces into one +// series, and it cannot join against the instruments that carry the pair. An +// empty name means no pool is assigned yet, which reports as an absent pair +// rather than an empty-string series. +// +// Pool-centric instruments that record a deliberate zero-valued series for "no +// pool matched" build the pair themselves; this helper is for the actor-centric +// instruments, where an unknown pool is omitted. +func WorkerPoolAttributes(namespace, name string) []attribute.KeyValue { + if name == "" { + return nil + } + return []attribute.KeyValue{ + WorkerPoolNamespaceKey.String(namespace), + WorkerPoolNameKey.String(name), + } +} + // ActorRefAttributes returns the subset knowable before the Actor record // resolves: only the (atespace, name) the request addresses. The uid and version // are server-assigned and unknown until the record loads, so they are omitted. @@ -275,6 +295,9 @@ func ActorAttributes(a *ateapipb.Actor) []attribute.KeyValue { // ActorMetricAttributes returns the metric labels for an Actor. // High-cardinality attributes (atespace, actor name, actor uid) are omitted. +// The worker-pool pair is omitted while the actor holds no assignment, so a +// crash before the actor reaches a worker reports no pool rather than an +// empty-string one. func ActorMetricAttributes(a *ateapipb.Actor, sandboxClass, operationName, reason string) []attribute.KeyValue { if a == nil { return nil @@ -286,17 +309,13 @@ func ActorMetricAttributes(a *ateapipb.Actor, sandboxClass, operationName, reaso } operationName = NormalizeOperationName(operationName) - pool := "" - if ass := a.GetWorkerAssignment(); ass != nil { - pool = ass.GetWorkerPool() - } - - return []attribute.KeyValue{ + ass := a.GetWorkerAssignment() + attrs := []attribute.KeyValue{ TemplateNamespaceKey.String(a.GetActorTemplateNamespace()), TemplateNameKey.String(a.GetActorTemplateName()), - WorkerPoolNameKey.String(pool), SandboxClassKey.String(sandboxClass), ActorOperationNameKey.String(operationName), FailureReasonKey.String(reason), } + return append(attrs, WorkerPoolAttributes(ass.GetWorkerNamespace(), ass.GetWorkerPool())...) } diff --git a/internal/ateattr/ateattr_test.go b/internal/ateattr/ateattr_test.go index 0813e3260..62422c771 100644 --- a/internal/ateattr/ateattr_test.go +++ b/internal/ateattr/ateattr_test.go @@ -244,19 +244,21 @@ func TestActorMetricAttributes(t *testing.T) { ActorTemplateNamespace: "default", ActorTemplateName: "counter-template", WorkerAssignment: &ateapipb.WorkerAssignment{ - WorkerPool: "default-pool", + WorkerNamespace: "ate-workers", + WorkerPool: "default-pool", }, } t.Run("explicit operation and reason", func(t *testing.T) { got := toMap(ActorMetricAttributes(actor, "gvisor", OperationResume, ReasonCorruptedAssignment)) want := map[attribute.Key]any{ - TemplateNamespaceKey: "default", - TemplateNameKey: "counter-template", - WorkerPoolNameKey: "default-pool", - SandboxClassKey: "gvisor", - ActorOperationNameKey: OperationResume, - FailureReasonKey: ReasonCorruptedAssignment, + TemplateNamespaceKey: "default", + TemplateNameKey: "counter-template", + WorkerPoolNamespaceKey: "ate-workers", + WorkerPoolNameKey: "default-pool", + SandboxClassKey: "gvisor", + ActorOperationNameKey: OperationResume, + FailureReasonKey: ReasonCorruptedAssignment, } assertAttrs(t, got, want) @@ -265,12 +267,13 @@ func TestActorMetricAttributes(t *testing.T) { t.Run("default unknown values", func(t *testing.T) { got := toMap(ActorMetricAttributes(actor, "gvisor", "", "")) want := map[attribute.Key]any{ - TemplateNamespaceKey: "default", - TemplateNameKey: "counter-template", - WorkerPoolNameKey: "default-pool", - SandboxClassKey: "gvisor", - ActorOperationNameKey: OperationUnknown, - FailureReasonKey: ReasonUnknown, + TemplateNamespaceKey: "default", + TemplateNameKey: "counter-template", + WorkerPoolNamespaceKey: "ate-workers", + WorkerPoolNameKey: "default-pool", + SandboxClassKey: "gvisor", + ActorOperationNameKey: OperationUnknown, + FailureReasonKey: ReasonUnknown, } assertAttrs(t, got, want) @@ -278,12 +281,33 @@ func TestActorMetricAttributes(t *testing.T) { t.Run("out of range operation name is normalized to unknown", func(t *testing.T) { got := toMap(ActorMetricAttributes(actor, "gvisor", "invalid_op", "")) + want := map[attribute.Key]any{ + TemplateNamespaceKey: "default", + TemplateNameKey: "counter-template", + WorkerPoolNamespaceKey: "ate-workers", + WorkerPoolNameKey: "default-pool", + SandboxClassKey: "gvisor", + ActorOperationNameKey: OperationUnknown, + FailureReasonKey: ReasonUnknown, + } + + assertAttrs(t, got, want) + }) + + // An actor that crashed before it reached a worker has no pool. Reporting + // one key of the pair, or an empty-string name, would put that crash in a + // series that looks like a real pool. + t.Run("unassigned actor omits both pool keys", func(t *testing.T) { + unassigned := &ateapipb.Actor{ + ActorTemplateNamespace: "default", + ActorTemplateName: "counter-template", + } + got := toMap(ActorMetricAttributes(unassigned, "gvisor", OperationCreate, ReasonUnknown)) want := map[attribute.Key]any{ TemplateNamespaceKey: "default", TemplateNameKey: "counter-template", - WorkerPoolNameKey: "default-pool", SandboxClassKey: "gvisor", - ActorOperationNameKey: OperationUnknown, + ActorOperationNameKey: OperationCreate, FailureReasonKey: ReasonUnknown, } @@ -291,6 +315,29 @@ func TestActorMetricAttributes(t *testing.T) { }) } +// TestWorkerPoolAttributes pins the both-or-neither rule. A WorkerPool is +// namespaced, so a name on its own merges same-named pools from different +// namespaces and cannot join against the instruments that carry the pair. +func TestWorkerPoolAttributes(t *testing.T) { + t.Run("known pool returns the pair", func(t *testing.T) { + got := toMap(WorkerPoolAttributes("ate-workers", "pool-a")) + want := map[attribute.Key]any{ + WorkerPoolNamespaceKey: "ate-workers", + WorkerPoolNameKey: "pool-a", + } + + assertAttrs(t, got, want) + }) + + t.Run("unknown pool returns neither key", func(t *testing.T) { + for _, namespace := range []string{"", "ate-workers"} { + if got := WorkerPoolAttributes(namespace, ""); len(got) != 0 { + t.Errorf("WorkerPoolAttributes(%q, \"\") = %v, want no attributes", namespace, got) + } + } + }) +} + // TestSnapshotScopeValue pins the enum-to-label mapping ateapi and atelet share. // An unmapped enum value must report unknown rather than its stringified form, // which would let a wire value widen the label set. diff --git a/internal/e2e/suites/metrics/metrics_test.go b/internal/e2e/suites/metrics/metrics_test.go index 0a27834fa..153b7bdfc 100644 --- a/internal/e2e/suites/metrics/metrics_test.go +++ b/internal/e2e/suites/metrics/metrics_test.go @@ -234,6 +234,7 @@ func TestPlatformMetricsEmitted(t *testing.T) { reasonVal := extractLabelValue(line, "ate_failure_reason") tmplNSVal := extractLabelValue(line, "ate_template_namespace") tmplNameVal := extractLabelValue(line, "ate_template_name") + workerPoolNSVal := extractLabelValue(line, "ate_workerpool_namespace") workerPoolVal := extractLabelValue(line, "ate_workerpool_name") sandboxVal := extractLabelValue(line, "ate_sandbox_class") @@ -256,6 +257,12 @@ func TestPlatformMetricsEmitted(t *testing.T) { if tmplNameVal == "" { crashErrs = append(crashErrs, "ate_template_name label is missing or empty") } + // The pool keys identify one WorkerPool together: the name on + // its own merges same-named pools from different namespaces. + // The suite crashes an assigned actor, so both are expected. + if workerPoolNSVal == "" { + crashErrs = append(crashErrs, "ate_workerpool_namespace label is missing or empty") + } if workerPoolVal == "" { crashErrs = append(crashErrs, "ate_workerpool_name label is missing or empty") } @@ -264,8 +271,8 @@ func TestPlatformMetricsEmitted(t *testing.T) { } if len(crashErrs) > 0 { - errs = append(errs, fmt.Sprintf("ate_actor_crashes line %q failed label validation:\n - %s\n (Extracted labels: op=%q, reason=%q, tmplNS=%q, tmplName=%q, workerPool=%q, sandboxClass=%q)", - line, strings.Join(crashErrs, "\n - "), opVal, reasonVal, tmplNSVal, tmplNameVal, workerPoolVal, sandboxVal)) + errs = append(errs, fmt.Sprintf("ate_actor_crashes line %q failed label validation:\n - %s\n (Extracted labels: op=%q, reason=%q, tmplNS=%q, tmplName=%q, workerPoolNS=%q, workerPool=%q, sandboxClass=%q)", + line, strings.Join(crashErrs, "\n - "), opVal, reasonVal, tmplNSVal, tmplNameVal, workerPoolNSVal, workerPoolVal, sandboxVal)) } } } From 1fab0aab21544b6b3e8cdaf78f0b6bd258cb56fa Mon Sep 17 00:00:00 2001 From: Jeff Luo Date: Fri, 14 Aug 2026 14:05:53 -0400 Subject: [PATCH 2/2] ateapi: drop the pool pair when the namespace is missing WorkerPoolAttributes guarded only the empty name, so a name without a namespace still emitted the pair with an empty namespace -- the same merged series the pair exists to prevent, and one that joins to nothing. Both keys are now set together or not at all. Also separate the two states the docs ran together: an absent pair means the operation has no pool, while the empty-valued pair on ate.scheduler.eligible_workers means no pool matched the constraints. --- docs/observability.md | 6 +++++- internal/ateattr/ateattr.go | 18 ++++++++---------- internal/ateattr/ateattr_test.go | 8 ++++++++ 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/docs/observability.md b/docs/observability.md index 87855278e..ad77701bd 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -142,7 +142,11 @@ For `ate.imagecache.requests`: * `ate.imagecache.outcome` is `hit` when the node holds a complete image record — every layer directory the record names is present — and `miss` when the lookup must pull. A failed lookup is neither: `error` is a failed lookup whatever the cause, and `cancelled` or `timeout` is the caller giving up, as on `ate.router.outcome`. So the hit ratio is `hit / (hit + miss)`, with failures and abandoned lookups out of the denominator. * `error.type` is present only on the `error` outcome, and carries the registry's own HTTP status for its rejection, from a fixed set: `401`, `403`, `404`, `429`, `500`, `502`, `503`, `504`. The set is an allow-list because the registry client reports whatever the remote returned. Each other status, and each failure that carries no status, reports `_OTHER`. -`ate.workerpool.namespace` and `ate.workerpool.name` identify a pool together, on every instrument that names one. A WorkerPool is a namespaced resource, so the name on its own merges same-named pools from different namespaces into one series. The pair means that capacity (`ate.workerpool.workers`, `ate.workerpool.desired_workers`, `ate.workerpool.ready_workers`) joins to demand (`ate.scheduler.assignment.duration`, `ate.actor.lifecycle.operation.duration`, `ate.actor.crashes`) by pool. On the actor-centric instruments, an operation that has no pool yet — a crash before the actor reached a worker, or the `no_free_worker` outcome — carries neither key rather than an empty-string one. +`ate.workerpool.namespace` and `ate.workerpool.name` identify a pool together, on every instrument that names one. A WorkerPool is a namespaced resource, so the name on its own merges same-named pools from different namespaces into one series. The pair means that capacity (`ate.workerpool.workers`, `ate.workerpool.desired_workers`, `ate.workerpool.ready_workers`) joins to demand (`ate.scheduler.assignment.duration`, `ate.actor.lifecycle.operation.duration`, `ate.actor.crashes`) by pool. + +Two states read differently: +* **No keys** means the operation has no pool. The actor-centric instruments omit the pair, so a crash before the actor reached a worker, or the `no_free_worker` outcome, names no pool. +* **Both keys empty** means no pool matched. Only `ate.scheduler.eligible_workers` reports it, as one zero-valued series that keeps "nothing is schedulable" on the same chart as the per-pool series. The three snapshot labels are orthogonal and mean the same thing on every histogram that carries them: * `ate.snapshot.kind`: which snapshot the operation reads or writes. `local` (node-local, written by a pause), `latest` (the actor's own durable snapshot), `golden` (the template's image), or `boot` (from scratch, so it never appears on the atelet histograms). diff --git a/internal/ateattr/ateattr.go b/internal/ateattr/ateattr.go index 3f4a45ecc..c744d0227 100644 --- a/internal/ateattr/ateattr.go +++ b/internal/ateattr/ateattr.go @@ -251,18 +251,16 @@ func NormalizeSandboxClass(class string) string { } } -// WorkerPoolAttributes returns the namespaced identity of a WorkerPool. The two -// keys are always set together, or not at all: a WorkerPool is namespaced, so -// the name alone merges same-named pools from different namespaces into one -// series, and it cannot join against the instruments that carry the pair. An -// empty name means no pool is assigned yet, which reports as an absent pair -// rather than an empty-string series. +// WorkerPoolAttributes returns the namespaced identity of a WorkerPool. A +// WorkerPool is namespaced, so half the pair identifies no pool: either key +// missing drops both, rather than emit an empty-string series that merges +// same-named pools and joins to nothing. // -// Pool-centric instruments that record a deliberate zero-valued series for "no -// pool matched" build the pair themselves; this helper is for the actor-centric -// instruments, where an unknown pool is omitted. +// This is for the actor-centric instruments, where an unknown pool is omitted. +// Pool-centric ones that record a deliberate zero-valued series for "no pool +// matched" build the pair themselves. func WorkerPoolAttributes(namespace, name string) []attribute.KeyValue { - if name == "" { + if name == "" || namespace == "" { return nil } return []attribute.KeyValue{ diff --git a/internal/ateattr/ateattr_test.go b/internal/ateattr/ateattr_test.go index 62422c771..6b1f227fd 100644 --- a/internal/ateattr/ateattr_test.go +++ b/internal/ateattr/ateattr_test.go @@ -336,6 +336,14 @@ func TestWorkerPoolAttributes(t *testing.T) { } } }) + + // The reverse of the case this helper exists for: a name without a namespace + // half-identifies the pool, which joins to nothing on the paired instruments. + t.Run("name without a namespace returns neither key", func(t *testing.T) { + if got := WorkerPoolAttributes("", "pool-a"); len(got) != 0 { + t.Errorf("WorkerPoolAttributes(\"\", \"pool-a\") = %v, want no attributes", got) + } + }) } // TestSnapshotScopeValue pins the enum-to-label mapping ateapi and atelet share.