-
Notifications
You must be signed in to change notification settings - Fork 136
feat(otel): set ate.* actor telemetry identity on ateapi spans #412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5ba9da5
5230263
8067f63
692d7ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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()) | ||
| } | ||
| } |
| 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) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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()) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd keep it as 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 { | ||
|
|
@@ -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 | ||
| } | ||
|
|
||
|
|
||
| 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) | ||
| } | ||
| } | ||
| } |
| 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") | ||
| } |
| 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)...) | ||
| } |

There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, added this!