diff --git a/cmd/ateapi/internal/controlapi/metrics.go b/cmd/ateapi/internal/controlapi/metrics.go index be46f3b80b..e8e539515b 100644 --- a/cmd/ateapi/internal/controlapi/metrics.go +++ b/cmd/ateapi/internal/controlapi/metrics.go @@ -102,7 +102,12 @@ func RegisterWorkerCount(meter metric.Meter, workers func() ([]*ateapipb.Worker, if w.GetStatus().GetAllocated().GetActors() > 0 { state = ateattr.WorkerStateAssigned } - tally[key{w.GetWorkerNamespace(), w.GetWorkerPool(), state, w.GetSandboxClass()}]++ + // CreateWorker does not validate the class, thus a worker can have an + // empty one. Report it as unknown, not as the pool's class: the + // scheduler puts no actor on a worker that offers no class, thus the + // pool's series must not count it as free capacity. + class := ateattr.NormalizeSandboxClass(w.GetSandboxClass()) + tally[key{w.GetWorkerNamespace(), w.GetWorkerPool(), state, class}]++ } for k, n := range tally { o.ObserveInt64(counter, n, metric.WithAttributes( @@ -191,7 +196,7 @@ func lifecycleOpAttrs(actor *ateapipb.Actor, template *ateapipb.ActorTemplate, s ass := actor.GetStatus().GetWorkerAssignment() attrs = append(attrs, ateattr.WorkerPoolAttributes(ass.GetWorkerNamespace(), ass.GetWorkerPool())...) if template != nil { - attrs = append(attrs, ateattr.SandboxClassKey.String(sandboxClassString(template.GetSandboxConfig().GetSandboxClass()))) + attrs = append(attrs, ateattr.SandboxClassAttribute(sandboxClassString(template.GetSandboxConfig().GetSandboxClass()))) } if snapshotKind != "" { attrs = append(attrs, ateattr.SnapshotKindKey.String(snapshotKind)) diff --git a/cmd/ateapi/internal/controlapi/metrics_test.go b/cmd/ateapi/internal/controlapi/metrics_test.go index 58df824131..51d812c207 100644 --- a/cmd/ateapi/internal/controlapi/metrics_test.go +++ b/cmd/ateapi/internal/controlapi/metrics_test.go @@ -400,3 +400,75 @@ func TestWorkerCountSeedsZeroForKnownPools(t *testing.T) { } } } + +// TestWorkerCountEmptyClassWorker covers the workers that CreateWorker accepted +// with no sandbox class. They report as unknown, not as an empty-string class, +// and not as capacity of the pool they name: the scheduler puts no actor on +// them, thus the pool's own series stays at 0 and an idle==0 alert on it still +// fires. +func TestWorkerCountEmptyClassWorker(t *testing.T) { + const ( + gvisor = string(atev1alpha1.SandboxClassGvisor) + microvm = string(atev1alpha1.SandboxClassMicroVM) + ) + tests := []struct { + name string + pools []*atev1alpha1.WorkerPool + workers []*ateapipb.Worker + want map[series]int64 + }{ + { + name: "gvisor pool", + pools: []*atev1alpha1.WorkerPool{workerPool("ns-1", "pool-empty", "")}, + workers: []*ateapipb.Worker{ + worker("ns-1", "pool-empty", "", false), + worker("ns-1", "pool-empty", "", false), + worker("ns-1", "pool-empty", "", false), + }, + want: map[series]int64{ + {"ns-1", "pool-empty", ateattr.WorkerStateIdle, gvisor}: 0, + {"ns-1", "pool-empty", ateattr.WorkerStateAssigned, gvisor}: 0, + {"ns-1", "pool-empty", ateattr.WorkerStateIdle, ateattr.SandboxClassUnknown}: 3, + }, + }, + { + // No gvisor series shows for a pool that runs no gvisor. + name: "microvm pool", + pools: []*atev1alpha1.WorkerPool{workerPool("ns-1", "pool-micro", atev1alpha1.SandboxClassMicroVM)}, + workers: []*ateapipb.Worker{worker("ns-1", "pool-micro", "", false)}, + want: map[series]int64{ + {"ns-1", "pool-micro", ateattr.WorkerStateIdle, microvm}: 0, + {"ns-1", "pool-micro", ateattr.WorkerStateAssigned, microvm}: 0, + {"ns-1", "pool-micro", ateattr.WorkerStateIdle, ateattr.SandboxClassUnknown}: 1, + }, + }, + { + // A worker that matches no pool has no class to fall back to. + name: "orphan worker", + pools: nil, + workers: []*ateapipb.Worker{worker("ns-1", "pool-orphan", "", false)}, + want: map[series]int64{ + {"ns-1", "pool-orphan", ateattr.WorkerStateIdle, ateattr.SandboxClassUnknown}: 1, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pools := func(labels.Selector) ([]*atev1alpha1.WorkerPool, error) { return tt.pools, nil } + workers := func() ([]*ateapipb.Worker, error) { return tt.workers, nil } + reader := newWorkerCountReader(t, workers, pools) + + sum := mustMetric(t, reader, workerpoolWorkersMetric).Data.(metricdata.Sum[int64]) + got := seriesCounts(sum) + if len(got) != len(tt.want) { + t.Fatalf("got %d series, want %d: %v", len(got), len(tt.want), got) + } + for k, v := range tt.want { + if gv, ok := got[k]; !ok || gv != v { + t.Errorf("series %v = %d (present=%v), want %d", k, gv, ok, v) + } + } + }) + } +} diff --git a/cmd/ateapi/internal/scheduling/metrics.go b/cmd/ateapi/internal/scheduling/metrics.go index 93f1c2a00b..06ee8da2a4 100644 --- a/cmd/ateapi/internal/scheduling/metrics.go +++ b/cmd/ateapi/internal/scheduling/metrics.go @@ -92,7 +92,7 @@ func (s *scheduler) recordEligibleWorkers(ctx context.Context, matching []*ateap s.eligibleWorkers.Record(ctx, count, metric.WithAttributes( ateattr.WorkerPoolNamespaceKey.String(k.namespace), ateattr.WorkerPoolNameKey.String(k.pool), - ateattr.SandboxClassKey.String(constraints.SandboxClass), + ateattr.SandboxClassAttribute(constraints.SandboxClass), ateattr.SchedulingConstraintKey.String(constraintStr), )) } diff --git a/cmd/ateapi/internal/scheduling/scheduling_test.go b/cmd/ateapi/internal/scheduling/scheduling_test.go index 9365b21e0f..19dfa4fbdb 100644 --- a/cmd/ateapi/internal/scheduling/scheduling_test.go +++ b/cmd/ateapi/internal/scheduling/scheduling_test.go @@ -652,7 +652,7 @@ func TestSchedule_EligibleWorkersMetric(t *testing.T) { } s := New(flt, WithIntn(firstIntn), WithMeter(meter)) - _, err := s.Schedule(context.Background(), Constraints{SandboxClass: "kata"}) + _, err := s.Schedule(context.Background(), Constraints{SandboxClass: "microvm"}) if !errors.Is(err, ErrNoCapacity) { t.Fatalf("Schedule() error = %v, want ErrNoCapacity", err) } @@ -670,8 +670,8 @@ func TestSchedule_EligibleWorkersMetric(t *testing.T) { t.Errorf("datapoint sum = %d, want 0", dp.Sum) } class, _ := dp.Attributes.Value(ateattr.SandboxClassKey) - if class.AsString() != "kata" { - t.Errorf("got sandbox class %q, want kata", class.AsString()) + if class.AsString() != "microvm" { + t.Errorf("got sandbox class %q, want microvm", class.AsString()) } } } diff --git a/cmd/atelet/metrics.go b/cmd/atelet/metrics.go index 896c835f5b..b48815b253 100644 --- a/cmd/atelet/metrics.go +++ b/cmd/atelet/metrics.go @@ -98,7 +98,7 @@ func (o snapshotOp) attrs() []attribute.KeyValue { attrs = append(attrs, ateattr.SnapshotKindKey.String(o.kind)) } if o.sandboxClass != "" { - attrs = append(attrs, ateattr.SandboxClassKey.String(ateattr.NormalizeSandboxClass(o.sandboxClass))) + attrs = append(attrs, ateattr.SandboxClassAttribute(o.sandboxClass)) } return attrs } diff --git a/docs/metrics/registry/metrics.yaml b/docs/metrics/registry/metrics.yaml index 7548f2cb1f..45c6317310 100644 --- a/docs/metrics/registry/metrics.yaml +++ b/docs/metrics/registry/metrics.yaml @@ -140,9 +140,11 @@ groups: stability: development value: unknown brief: > - The snapshot manifest has no class, or the class is not in this - list. The default is not gvisor. Thus a bad manifest stays - visible. + The source has no class, or a class that is not in this list. + The default is not gvisor. Thus a bad record stays visible. + ateapi reports it for a Worker that CreateWorker accepted with + no class, and for an ActorTemplate with no sandbox config. + atelet reports it for a snapshot manifest with no class. - id: registry.ate.snapshot type: attribute_group @@ -620,6 +622,9 @@ groups: not write. Some moves into this state are not a loss of data. They are careful responses to a control plane problem. The ate.failure.reason key keeps these groups separate. + ate.sandbox.class is unknown when ateapi could not read the class of the + worker: the worker record is already gone, or its assignment is already + clear. annotations: substrate: emitted_by: [ateapi] @@ -660,6 +665,10 @@ groups: and not a gauge. ateapi sets both states to 0 for each known pool. Thus a full pool or an empty pool reports 0. An absent series would stop an alert on idle == 0. + A worker with the class unknown is not capacity of the pool: the scheduler + puts no actor on it. Thus a query that asks how much capacity a pool has + must keep ate.sandbox.class, because a sum across the classes adds that + worker back into the total. annotations: substrate: emitted_by: [ateapi] diff --git a/internal/ateattr/ateattr.go b/internal/ateattr/ateattr.go index 702bf1263b..160f70548e 100644 --- a/internal/ateattr/ateattr.go +++ b/internal/ateattr/ateattr.go @@ -343,6 +343,16 @@ func NormalizeSandboxClass(class string) string { } } +// SandboxClassAttribute sets ate.sandbox.class. No source of the class is +// validated, thus an emitter that sets the attribute must set it through this +// helper and not through SandboxClassKey. +// +// To omit the attribute while the class is unknown is a different choice, and +// two emitters make it: recordSchedulerAssignment and snapshotOp.attrs. +func SandboxClassAttribute(class string) attribute.KeyValue { + return SandboxClassKey.String(NormalizeSandboxClass(class)) +} + // 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 @@ -445,7 +455,7 @@ func ActorMetricAttributes(a *ateapipb.Actor, sandboxClass, operationName, reaso attrs := []attribute.KeyValue{ TemplateAtespaceKey.String(a.GetActorTemplate().GetAtespace()), TemplateNameKey.String(a.GetActorTemplate().GetName()), - SandboxClassKey.String(sandboxClass), + SandboxClassAttribute(sandboxClass), ActorOperationNameKey.String(operationName), } attrs = append(attrs, FailureAttributes(reason)...) diff --git a/internal/ateattr/ateattr_test.go b/internal/ateattr/ateattr_test.go index 0fd7bdb312..76abffb6b9 100644 --- a/internal/ateattr/ateattr_test.go +++ b/internal/ateattr/ateattr_test.go @@ -469,6 +469,24 @@ func TestActorMetricAttributes(t *testing.T) { assertAttrs(t, got, want) }) + // releaseWorker gives an empty class when the worker record is gone. + // CreateWorker can also store one. Empty is not a permitted value. + t.Run("empty sandbox class is normalized to unknown", func(t *testing.T) { + got := toMap(ActorMetricAttributes(actor, "", OperationResume, ReasonCorruptedAssignment)) + want := map[attribute.Key]any{ + TemplateAtespaceKey: "default", + TemplateNameKey: "counter-template", + WorkerPoolNamespaceKey: "ate-workers", + WorkerPoolNameKey: "default-pool", + SandboxClassKey: SandboxClassUnknown, + ActorOperationNameKey: OperationResume, + FailureReasonKey: ReasonCorruptedAssignment, + FailureDomainKey: FailureDomainInfrastructure, + } + + assertAttrs(t, got, want) + }) + 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{