Skip to content
Open
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
4 changes: 3 additions & 1 deletion cmd/ateapi/internal/controlapi/create_actor.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@ func (s *Service) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequ
if err := validateCreateActorRequest(req); err != nil {
return nil, err
}

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

setSpanActorRefIdentity(ctx, in.GetMetadata().GetAtespace(), in.GetMetadata().GetName())

_, err := s.actorTemplateLister.ActorTemplates(templateNamespace).Get(templateName)
if err != nil {
if k8serrors.IsNotFound(err) {
Expand Down Expand Up @@ -76,6 +77,7 @@ func (s *Service) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequ
return nil, fmt.Errorf("while recording actor: %w", err)
}

setSpanActorIdentity(ctx, stored)
return stored, nil
}

Expand Down
59 changes: 59 additions & 0 deletions cmd/ateapi/internal/controlapi/create_actor_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// 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"
"testing"

"go.opentelemetry.io/otel/attribute"

"github.com/agent-substrate/substrate/internal/ateattr"
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
)

// CreateActor is the only lifecycle op with the full identity (incl. version)
// available in the request, so the whole ate.* set should land on its span.
func TestCreateActor_StampsFullSpanIdentity(t *testing.T) {
ns := namespaceForTest("ns-span-create")
tc := setupTest(t, ns)
defer tc.cleanup()
createTemplate(t, tc, ns)

attrs := recordRootSpanAttrs(t, func(ctx context.Context) {
if _, err := tc.service.CreateActor(ctx, &ateapipb.CreateActorRequest{
Actor: &ateapipb.Actor{
Metadata: &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: testActorID},
ActorTemplateNamespace: ns,
ActorTemplateName: "tmpl1",
},
}); err != nil {
t.Fatalf("CreateActor: %v", err)
}
})

assertSpanStr(t, attrs, ateattr.AtespaceKey, testAtespace)
assertSpanStr(t, attrs, ateattr.ActorNameKey, testActorID)
assertSpanStr(t, attrs, ateattr.ActorTemplateNameKey, "tmpl1")
assertSpanStr(t, attrs, ateattr.ActorTemplateNamespaceKey, ns)
// uid is server-assigned on create, so assert it is present and non-empty
// rather than a fixed value.
if v, ok := attrs[ateattr.ActorUIDKey]; !ok || v.Type() != attribute.STRING || v.AsString() == "" {
t.Errorf("%s = %v, want non-empty server-assigned uid", ateattr.ActorUIDKey, v.Emit())
}
if v, ok := attrs[ateattr.ActorVersionKey]; !ok || v.Type() != attribute.INT64 || v.AsInt64() != 1 {
t.Errorf("%s = %v, want int64 1", ateattr.ActorVersionKey, v.Emit())
}
}
1 change: 1 addition & 0 deletions cmd/ateapi/internal/controlapi/delete_actor.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func (s *Service) DeleteActor(ctx context.Context, req *ateapipb.DeleteActorRequ
if err := validateDeleteActorRequest(req); err != nil {
return nil, err
}
setSpanActorRefIdentity(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName())

deleted, err := s.persistence.DeleteActor(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName())
if err != nil {
Expand Down
52 changes: 52 additions & 0 deletions cmd/ateapi/internal/controlapi/delete_actor_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// 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"
"testing"

"github.com/agent-substrate/substrate/internal/ateattr"
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
)

// Delete addresses the actor by ref (atespace + id) and does not resolve the
// template/version, so only the ref identity is stamped.
func TestDeleteActor_StampsRefSpanIdentity(t *testing.T) {
ns := namespaceForTest("ns-span-delete")
tc := setupTest(t, ns)
defer tc.cleanup()
createTemplate(t, tc, ns)
if _, err := tc.service.CreateActor(context.Background(), &ateapipb.CreateActorRequest{
Actor: &ateapipb.Actor{
Metadata: &ateapipb.ResourceMetadata{Atespace: testAtespace, Name: testActorID},
ActorTemplateNamespace: ns,
ActorTemplateName: "tmpl1",
},
}); err != nil {
t.Fatalf("seed CreateActor: %v", err)
}

attrs := recordRootSpanAttrs(t, func(ctx context.Context) {
if _, err := tc.service.DeleteActor(ctx, &ateapipb.DeleteActorRequest{
Actor: &ateapipb.ObjectRef{Atespace: testAtespace, Name: testActorID},
}); err != nil {
t.Fatalf("DeleteActor: %v", err)
}
})

assertSpanStr(t, attrs, ateattr.AtespaceKey, testAtespace)
assertSpanStr(t, attrs, ateattr.ActorNameKey, testActorID)
}
5 changes: 4 additions & 1 deletion cmd/ateapi/internal/controlapi/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ var (
fakeAtelet = &FakeAteletServer{}
)

const testAtespace = "test-atespace"
const (
testAtespace = "test-atespace"
testActorID = "id1"
)

var (
ignoreUID = protocmp.IgnoreFields(&ateapipb.ResourceMetadata{}, "uid")
Expand Down
2 changes: 2 additions & 0 deletions cmd/ateapi/internal/controlapi/pause_actor.go

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Add a test in pause_actor_test.go?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, added this!

Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func (s *Service) PauseActor(ctx context.Context, req *ateapipb.PauseActorReques
if err := validatePauseActorRequest(req); err != nil {
return nil, err
}
setSpanActorRefIdentity(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

out of curiosity, what will be happened if instead of setSpanActorRefIdentity, the "setSpanActorIdentity" will be called.
The only value that is changing between line 33 to 46 is version.
I guess the intention is to keep the latest version right? Is it possible to call to setSpanActorIdentity twice. before s.actorWorkflow.PauseActor and after? The latest version will be submitted in successful case and previous version will be submitted in case of error.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can't call the full one early. At line 33 we only have the ObjectRef from the request and setSpanActorIdentity needs the resolved *Actor that we don't have until the workflow returns it.

Attributes overwrite by key, so the later wins and we get the latest version on success. On error we return early with only the ref, so no version.

I guess we could also add the version on mid-workflow failures if we move it inside the workflow. Maybe that's better as a follow-up if it's worth it. What do you think?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

+1 without looking at the code, it's not obvious which "version" is in the attribute (is it the version before or after the pause?) we can update the key to "new_version" or "updated_version" for clarification.

Image

@krisztianfekete Krisztian F (krisztianfekete) Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd keep it as ate.actor.version. It means the same thing on every op, which is the version the op resulted in. Forking it to new_version on pause but version on create breaks querying for spans that have actor A at version X.

Happy to add a line to the attr doc spelling out "resulting version" or similar if that makes it better?


actor, err := s.actorWorkflow.PauseActor(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName())
if err != nil {
Expand All @@ -42,6 +43,7 @@ func (s *Service) PauseActor(ctx context.Context, req *ateapipb.PauseActorReques
return nil, err
}

setSpanActorIdentity(ctx, actor)
return &ateapipb.PauseActorResponse{Actor: actor}, nil
}

Expand Down
52 changes: 52 additions & 0 deletions cmd/ateapi/internal/controlapi/pause_actor_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// 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"
"testing"

"go.opentelemetry.io/otel/attribute"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/agent-substrate/substrate/internal/ateattr"
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
)

// Pause stamps the ref identity before resolving the Actor record, so a failed
// lookup still carries who/where; it must not invent template/version, which are
// known only once the record resolves (and stamped on success).
func TestPauseActor_FailedLookupStampsRefIdentityOnly(t *testing.T) {
ns := namespaceForTest("ns-span-pause-err")
tc := setupTest(t, ns)
defer tc.cleanup()

attrs := recordRootSpanAttrs(t, func(ctx context.Context) {
if _, err := tc.service.PauseActor(ctx, &ateapipb.PauseActorRequest{
Actor: &ateapipb.ObjectRef{Atespace: testAtespace, Name: testActorID},
}); status.Code(err) != codes.NotFound {
t.Fatalf("PauseActor(missing) error = %v, want code NotFound", err)
}
})

assertSpanStr(t, attrs, ateattr.AtespaceKey, testAtespace)
assertSpanStr(t, attrs, ateattr.ActorNameKey, testActorID)
for _, k := range []attribute.Key{ateattr.ActorUIDKey, ateattr.ActorTemplateNameKey, ateattr.ActorTemplateNamespaceKey, ateattr.ActorVersionKey} {
if _, ok := attrs[k]; ok {
t.Errorf("unexpected %s on failed-pause span", k)
}
}
}
2 changes: 2 additions & 0 deletions cmd/ateapi/internal/controlapi/resume_actor.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func (s *Service) ResumeActor(ctx context.Context, req *ateapipb.ResumeActorRequ
if err := validateResumeActorRequest(req); err != nil {
return nil, err
}
setSpanActorRefIdentity(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName())

actor, err := s.actorWorkflow.ResumeActor(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName(), req.GetBoot())
if err != nil {
Expand All @@ -42,6 +43,7 @@ func (s *Service) ResumeActor(ctx context.Context, req *ateapipb.ResumeActorRequ
return nil, err
}

setSpanActorIdentity(ctx, actor)
return &ateapipb.ResumeActorResponse{Actor: actor}, nil
}

Expand Down
42 changes: 42 additions & 0 deletions cmd/ateapi/internal/controlapi/resume_actor_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// 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"
"testing"

"github.com/agent-substrate/substrate/internal/ateattr"
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
)

// The early ref stamp must land on the span even when the op fails, so a failed
// resume is still attributable to who/where.
func TestResumeActor_ErrorStillStampsRefSpanIdentity(t *testing.T) {
ns := namespaceForTest("ns-span-resume-err")
tc := setupTest(t, ns)
defer tc.cleanup()

attrs := recordRootSpanAttrs(t, func(ctx context.Context) {
if _, err := tc.service.ResumeActor(ctx, &ateapipb.ResumeActorRequest{
Actor: &ateapipb.ObjectRef{Atespace: testAtespace, Name: "missing"},
}); err == nil {
t.Fatal("expected error resuming missing actor")
}
})

assertSpanStr(t, attrs, ateattr.AtespaceKey, testAtespace)
assertSpanStr(t, attrs, ateattr.ActorNameKey, "missing")
}
36 changes: 36 additions & 0 deletions cmd/ateapi/internal/controlapi/span_identity.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// 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"

"go.opentelemetry.io/otel/trace"

"github.com/agent-substrate/substrate/internal/ateattr"
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
)

// setSpanActorIdentity annotates the RPC's server span (from ctx) with the
// actor's full identity. A no-op when ctx carries no recording span.
func setSpanActorIdentity(ctx context.Context, a *ateapipb.Actor) {
trace.SpanFromContext(ctx).SetAttributes(ateattr.ActorAttributes(a)...)
}

// setSpanActorRefIdentity is setSpanActorIdentity for the identity subset known
// before the Actor record resolves, so a failed lookup still carries who/where.
func setSpanActorRefIdentity(ctx context.Context, atespace, name string) {
trace.SpanFromContext(ctx).SetAttributes(ateattr.ActorRefAttributes(atespace, name)...)
}
Loading
Loading