feat(otel): set ate.* actor telemetry identity on ateapi spans by krisztianfekete · Pull Request #412 · agent-substrate/substrate

Krisztian F (krisztianfekete) · 2026-07-09T16:01:53Z

This PR sets consistent, namespaced actor-identity set on the spans we already emit:

ate.atespace, ate.actor.id, ate.actor.template.{name,namespace}, ate.actor.version
ateapi: the RPC server span for create/resume/suspend/pause/delete

We do this, so platform traces are queryable by actor, atespace, and template. Cross-hop correlation and per-tenant/per-template filtering, all on traces (not TSDB labels, as discussed in #174). This is a general, workload-neutral telemetry identity plumbing. We could add gen_ai specific stuff on top of this later.

This is how golden snapshots get build:

This is a run:

Notes:

Left as it was on purpose: metric attribute names and the stdout log labels (ate.dev/*), but we should probably do this as well. Maybe in this PR?
Bikeshed is welcomed on the ate.* spelling before merge, renaming span attrs later is painful.
Tests pass
Appropriate changes to documentation are included in the PR

Dmitry Berkovich (dberkov) · 2026-07-11T04:44:06Z

+	return m
+}
+
+func TestActorIdentity(t *testing.T) {


could you please rewrite test based on data driven pattern?

tests := []struct {....}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
....

Sure, done!

Dmitry Berkovich (dberkov) · 2026-07-11T04:50:49Z

+	}
+}
+
+func TestCreateActor_StampsFullSpanIdentity(t *testing.T) {


the "cmd/ateapi/internal/controlapi/span_identity_test.go" supposed to concentrate on methods exposed in the cmd/ateapi/internal/controlapi/span_identity.go:

setSpanActorIdentity

setSpanActorRefIdentity

The "TestCreateActor_StampsFullSpanIdentity" method needs to be tested in create_actor_test.go. that does not exist today :)

Makes sense, I split it up.

Dmitry Berkovich (dberkov) · 2026-07-11T04:55:23Z

 	if err := validatePauseActorRequest(req); err != nil {
 		return nil, err
 	}
+	setSpanActorRefIdentity(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName())


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.

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?

+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.

Dmitry Berkovich (dberkov) · 2026-07-11T05:00:08Z

Left as it was on purpose: metric attribute names and the stdout log labels (ate.dev/*), but we should probably do this as well. Maybe in this PR?

Bikeshed is welcomed on the ate.* spelling before merge, renaming span attrs later is painful.

What do you mean by it?

Krisztian F (krisztianfekete) · 2026-07-11T07:03:26Z

Left as it was on purpose: metric attribute names and the stdout log labels (ate.dev/*), but we should probably do this as well. Maybe in this PR?

Bikeshed is welcomed on the ate.* spelling before merge, renaming span attrs later is painful.

What do you mean by it?

This PR only covers span attributes. I left the stdout log labels and metric attribute names as-is to keep the scope small. Would you prefer to do the reconsiliation in this PR, or as a follow-up?
The ate.* bikeshed was just about deciding on the namespace before merge. I haven't seen an explicit decision on ate.* formally (might have just missed it), so wanted to bring it up.

Thanks for the review!

Zoe Zhao (zoez7) · 2026-07-11T16:48:50Z

+
+// 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, actorID string) {


the parameter name is actorID but the callsites are passing in actor name, can we change it to actorName?

Zoe Zhao (zoez7) · 2026-07-11T16:51:55Z

+// 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, actorID string) {
+	trace.SpanFromContext(ctx).SetAttributes(ateattr.ActorRefIdentity(atespace, actorID)...)


Similarly here, can we rename ActorRefIdentity to ActorObjectReference to be consistent with pkg/proto/ateapipb/ateapi.proto

Zoe Zhao (zoez7) · 2026-07-11T16:56:56Z

+}
+
+// ActorIdentity is nil-safe; a nil Actor yields zero-valued attributes.
+func ActorIdentity(a *ateapipb.Actor) []attribute.KeyValue {


Nit: I think this should be ActorAttributes since it logs several fields that I would not consider as the "identity" of the actor.

Zoe Zhao (zoez7) · 2026-07-11T16:59:34Z

+	})
+
+	assertSpanStr(t, attrs, ateattr.AtespaceKey, testAtespace)
+	assertSpanStr(t, attrs, ateattr.ActorIDKey, "id1")


nit: also create constants for "id" similar to testAtespace above?

Zoe Zhao (zoez7) · 2026-07-11T17:24:43Z

 	if err := validatePauseActorRequest(req); err != nil {
 		return nil, err
 	}
+	setSpanActorRefIdentity(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName())


+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.

Zoe Zhao (zoez7) · 2026-07-11T17:25:05Z

+// Dotted ate.* matches the metric-instrument naming (atenet.*, atelet.*), not the
+// ate.dev/ slash form used for k8s labels and stdout log fields.
+const (
+	AtespaceKey               = attribute.Key("ate.atespace")


ate.actor.atespace?

Zoe Zhao (zoez7) · 2026-07-11T17:30:43Z

Add a test in pause_actor_test.go?

Zoe Zhao (zoez7) · 2026-07-11T17:44:05Z

+	"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
+)
+
+// installSpanRecorder swaps in a recording global TracerProvider. Span-producing


Let's have each test create its own trace provider to avoid this global provider shared by all tests? We will want to run tests in parallel later to speed up testing.

Zoe Zhao (zoez7) · 2026-07-11T18:15:08Z

While reviewing this I realized today the atenet router uses different keys as trace attribute
https://github.com/agent-substrate/substrate/blob/c1ab0958f85202faf9f87ea66cd98903a9de763b/cmd/atenet/internal/router/resumer.go#L49-L50C22

and the traces are not connected with the ateapi one, for the same ResumeActor request, we have one trace which traces the request through AteAPI-Atelet -Ateom

but another, detached one with just the atenet router:

We don't need to address it in this PR. Could you help us create a tracking issue for this?

feat(otel): set ate.* actor identity on router and ateapi spans

5ba9da5

Krisztian F (krisztianfekete) force-pushed the feat/actor-identity-spans branch from 776de1e to 5ba9da5 Compare July 9, 2026 17:26

Krisztian F (krisztianfekete) changed the title ~~feat(otel): set ate.* actor telemetry identity on router and ateapi spans~~ feat(otel): set ate.* actor telemetry identity on ateapi spans Jul 9, 2026

Merge main into feat/actor-identity-spans

5230263

Krisztian F (krisztianfekete) marked this pull request as ready for review July 10, 2026 15:59

Dmitry Berkovich (dberkov) reviewed Jul 11, 2026

View reviewed changes

Dmitry Berkovich (dberkov) requested a review from Zoe Zhao (zoez7) July 11, 2026 05:00

address review comments, split span identity tests by method

8067f63

Zoe Zhao (zoez7) reviewed Jul 11, 2026

View reviewed changes

Uh oh!

Conversation

Krisztian F (krisztianfekete) commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dmitry Berkovich (dberkov) commented Jul 11, 2026

Uh oh!

Krisztian F (krisztianfekete) commented Jul 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zoe Zhao (zoez7) commented Jul 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Krisztian F (krisztianfekete) commented Jul 9, 2026 •

edited

Loading