Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions cmd/ateapi/internal/controlapi/create_actor.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ import (
"context"
"errors"
"fmt"
"time"

"github.com/agent-substrate/substrate/cmd/ateapi/internal/store"
"github.com/agent-substrate/substrate/internal/ateattr"
"github.com/agent-substrate/substrate/internal/resources"
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
"google.golang.org/grpc/codes"
Expand All @@ -29,16 +31,24 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
)

func (s *Service) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequest) (*ateapipb.Actor, error) {
if err := validateCreateActorRequest(req); err != nil {
func (s *Service) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequest) (created *ateapipb.Actor, err error) {
start := time.Now()
in := req.GetActor()
defer func() {
s.instruments.recordLifecycleOp(ctx, ateattr.OperationCreate, start, err,
ateattr.ActorTemplateNameKey.String(in.GetActorTemplateName()),
ateattr.ActorTemplateNamespaceKey.String(in.GetActorTemplateNamespace()),
)
}()

if err = validateCreateActorRequest(req); err != nil {
return nil, err
}

in := req.GetActor()
templateNamespace := in.GetActorTemplateNamespace()
templateName := in.GetActorTemplateName()

_, err := s.actorTemplateLister.ActorTemplates(templateNamespace).Get(templateName)
_, err = s.actorTemplateLister.ActorTemplates(templateNamespace).Get(templateName)
if err != nil {
if k8serrors.IsNotFound(err) {
return nil, status.Errorf(codes.FailedPrecondition, "ActorTemplate %s/%s not found", templateNamespace, templateName)
Expand Down
21 changes: 18 additions & 3 deletions cmd/ateapi/internal/controlapi/delete_actor.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,42 @@ import (
"context"
"errors"
"fmt"
"time"

"github.com/agent-substrate/substrate/cmd/ateapi/internal/store"
"github.com/agent-substrate/substrate/internal/ateattr"
"github.com/agent-substrate/substrate/internal/resources"
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"k8s.io/apimachinery/pkg/util/validation/field"
)

func (s *Service) DeleteActor(ctx context.Context, req *ateapipb.DeleteActorRequest) (*ateapipb.Actor, error) {
if err := validateDeleteActorRequest(req); err != nil {
func (s *Service) DeleteActor(ctx context.Context, req *ateapipb.DeleteActorRequest) (deleted *ateapipb.Actor, err error) {
start := time.Now()
// tmpl is whichever loaded record carries the template dimensions; the delete
// request itself only names the actor.
var tmpl *ateapipb.Actor
defer func() {
s.instruments.recordLifecycleOp(ctx, ateattr.OperationDelete, start, err,
ateattr.ActorTemplateNameKey.String(tmpl.GetActorTemplateName()),
ateattr.ActorTemplateNamespaceKey.String(tmpl.GetActorTemplateNamespace()),
)
}()

if err = validateDeleteActorRequest(req); err != nil {
return nil, err
}

deleted, err := s.persistence.DeleteActor(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName())
deleted, err = s.persistence.DeleteActor(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName())
if err != nil {
if errors.Is(err, store.ErrNotFound) {
return nil, status.Errorf(codes.NotFound, "Actor %s not found", req.GetActor().GetName())
}
if errors.Is(err, store.ErrFailedPrecondition) {
current, getErr := s.persistence.GetActor(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName())
if getErr == nil {
tmpl = current
return nil, status.Errorf(codes.FailedPrecondition, "Actor %s is not suspended (status: %v)", req.GetActor().GetName(), current.GetStatus())
}
return nil, status.Errorf(codes.FailedPrecondition, "Actor %s is not suspended", req.GetActor().GetName())
Expand All @@ -50,6 +64,7 @@ func (s *Service) DeleteActor(ctx context.Context, req *ateapipb.DeleteActorRequ
return nil, fmt.Errorf("while deleting actor from DB: %w", err)
}

tmpl = deleted
return deleted, nil
}

Expand Down
11 changes: 10 additions & 1 deletion cmd/ateapi/internal/controlapi/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/redis/go-redis/v9"
"go.opentelemetry.io/otel"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/credentials/insecure"
Expand Down Expand Up @@ -310,7 +311,15 @@ func setupTest(t *testing.T, ns string) *testContext {
}

dialer := NewAteletDialer(workerInformer.GetIndexer(), ateletInformer.GetIndexer())
service := NewService(persistence, wc, actorTemplateLister, workerPoolLister, sandboxConfigLister, dialer, k8sClient)
// Real instruments (against the no-op global meter) so the full-stack flows
// exercise the deferred record path, not just the dedicated metric tests.
instruments, err := NewInstruments(otel.Meter("ateapi"), wc.Workers)
if err != nil {
cancel()
mr.Close()
t.Fatalf("failed to create instruments: %v", err)
}
service := NewService(persistence, wc, actorTemplateLister, workerPoolLister, sandboxConfigLister, dialer, k8sClient, instruments)

// 5. Start REAL gRPC Server for ATE API
grpcServer := grpc.NewServer(grpc.UnaryInterceptor(ateinterceptors.ServerUnaryInterceptor))
Expand Down
171 changes: 171 additions & 0 deletions cmd/ateapi/internal/controlapi/metrics.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
// Copyright 2026 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package controlapi

import (
"context"
"fmt"
"time"

"github.com/agent-substrate/substrate/internal/ateattr"
atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1"
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"google.golang.org/grpc/status"
)

const (
lifecycleOpDurationMetric = "ate.actor.lifecycle.operation.duration"
schedulerAssignmentMetric = "ate.scheduler.assignment.duration"
workerpoolWorkersMetric = "ate.workerpool.workers"
)

// Instruments holds ateapi's platform metric instruments. A nil *Instruments is
// a valid no-op, so tests and pre-init code paths need no special casing.
type Instruments struct {
lifecycleOpDuration metric.Float64Histogram
schedulerAssignmentDuration metric.Float64Histogram
}

// NewInstruments builds the instruments from meter and registers the observable
// worker-count callback against workers (workercache.Cache.Workers in prod).
func NewInstruments(meter metric.Meter, workers func() ([]*ateapipb.Worker, error)) (*Instruments, error) {
lifecycleOpDuration, err := meter.Float64Histogram(
lifecycleOpDurationMetric,
metric.WithUnit("s"),
metric.WithDescription("Duration of an actor lifecycle operation (create, resume, suspend, pause, delete) handled by ateapi."),
metric.WithExplicitBucketBoundaries(0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.15, 0.25, 0.5, 1, 2.5, 5, 10, 30),
)
if err != nil {
return nil, fmt.Errorf("create %s histogram: %w", lifecycleOpDurationMetric, err)
}

schedulerAssignmentDuration, err := meter.Float64Histogram(
schedulerAssignmentMetric,
metric.WithUnit("s"),
metric.WithDescription("Duration of the scheduler's worker-assignment step during actor resume."),
metric.WithExplicitBucketBoundaries(0.0005, 0.001, 0.0025, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1),
)
if err != nil {
return nil, fmt.Errorf("create %s histogram: %w", schedulerAssignmentMetric, err)
}

if err := registerWorkerCount(meter, workers); err != nil {
return nil, err
}

return &Instruments{
lifecycleOpDuration: lifecycleOpDuration,
schedulerAssignmentDuration: schedulerAssignmentDuration,
}, nil
}

// registerWorkerCount wires ate.workerpool.workers. Worker counts are spatially
// summable (over states = pool size, over pools = fleet), which is the
// UpDownCounter contract; a gauge would be wrong for a value meant to be summed.
func registerWorkerCount(meter metric.Meter, workers func() ([]*ateapipb.Worker, error)) error {
counter, err := meter.Int64ObservableUpDownCounter(
workerpoolWorkersMetric,
metric.WithUnit("{worker}"),
metric.WithDescription("Number of workers by pool, worker state, and sandbox class."),
)
if err != nil {
return fmt.Errorf("create %s updowncounter: %w", workerpoolWorkersMetric, err)
}

_, err = meter.RegisterCallback(func(_ context.Context, o metric.Observer) error {
ws, err := workers()
if err != nil {
// Cache not ready (warmup/reconnect): skip so no zero-valued points.
return nil
}
type key struct{ pool, state, class string }
tally := make(map[key]int64, len(ws))
for _, w := range ws {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this tally only creates a series for states present in the current worker slice. If a pool's last idle worker becomes assigned, {state="idle"} will disappear, rather than becoming 0. As such, an idle == 0 type saturation query or alert would only see an absent/stale vector and may not return results or fire.

A pool with zero registered workers disappears entirely, which also makes “all replicas are down” indistinguishable from “pool does not exist.”

Is it possible to drive the callback from the known WorkerPools/classes and emit both bounded states (idle and assigned) for each pool, including zero values, only skipping the collection when the cache itself is unavailable?

state := ateattr.WorkerStateIdle
if w.GetAssignment() != nil {
state = ateattr.WorkerStateAssigned
}
tally[key{w.GetWorkerPool(), state, w.GetSandboxClass()}]++
}
for k, n := range tally {
o.ObserveInt64(counter, n, metric.WithAttributes(
ateattr.WorkerPoolNameKey.String(k.pool),
ateattr.WorkerStateKey.String(k.state),
ateattr.SandboxClassKey.String(k.class),
))
}
return nil
}, counter)
if err != nil {
return fmt.Errorf("register %s callback: %w", workerpoolWorkersMetric, err)
}
return nil
}

// recordLifecycleOp records one observation, classifying a non-nil err onto
// error.type via its gRPC status code; error.type's absence marks success.
func (i *Instruments) recordLifecycleOp(ctx context.Context, op string, start time.Time, err error, extraAttrs ...attribute.KeyValue) {
if i == nil || i.lifecycleOpDuration == nil {
return
}
attrs := make([]attribute.KeyValue, 0, len(extraAttrs)+2)
attrs = append(attrs, ateattr.ActorOperationNameKey.String(op))
attrs = append(attrs, extraAttrs...)
if err != nil {
attrs = append(attrs, ateattr.ErrorTypeKey.String(status.Code(err).String()))
}
i.lifecycleOpDuration.Record(ctx, time.Since(start).Seconds(), metric.WithAttributes(attrs...))
}

// lifecycleOpAttrs builds the template/pool/class/kind dimensions from whatever
// workflow state has resolved so far; each is nil-safe and omitted when unknown,
// so an operation that fails before a dimension is known still records a
// well-formed (smaller) attribute set.
func lifecycleOpAttrs(actor *ateapipb.Actor, template *atev1alpha1.ActorTemplate, snapshotKind string) []attribute.KeyValue {
attrs := []attribute.KeyValue{
ateattr.ActorTemplateNameKey.String(actor.GetActorTemplateName()),
ateattr.ActorTemplateNamespaceKey.String(actor.GetActorTemplateNamespace()),
}
if pool := actor.GetWorkerPoolName(); pool != "" {
attrs = append(attrs, ateattr.WorkerPoolNameKey.String(pool))
}
if template != nil {
attrs = append(attrs, ateattr.SandboxClassKey.String(string(template.Spec.SandboxClass)))
}
if snapshotKind != "" {
attrs = append(attrs, ateattr.SnapshotKindKey.String(snapshotKind))
}
return attrs
}

// recordSchedulerAssignment stamps pool only when a worker was assigned, and
// error.type only for the error outcome, so no_free_worker (a capacity signal)
// never carries an error.type.
func (i *Instruments) recordSchedulerAssignment(ctx context.Context, start time.Time, outcome, pool string, err error) {
if i == nil || i.schedulerAssignmentDuration == nil {
return
}
attrs := make([]attribute.KeyValue, 0, 3)
attrs = append(attrs, ateattr.SchedulerOutcomeKey.String(outcome))
if pool != "" {
attrs = append(attrs, ateattr.WorkerPoolNameKey.String(pool))
}
if outcome == ateattr.SchedulerOutcomeError && err != nil {
attrs = append(attrs, ateattr.ErrorTypeKey.String(status.Code(err).String()))
}
i.schedulerAssignmentDuration.Record(ctx, time.Since(start).Seconds(), metric.WithAttributes(attrs...))
}
Loading