Skip to content

[minor] stamp OpenTelemetry span attributes in UpdateActor - #512

Merged
Eitan Yarmush (EItanya) merged 1 commit into
agent-substrate:mainfrom
troychiu:fix-update-actor-span-attributes
Jul 29, 2026
Merged

[minor] stamp OpenTelemetry span attributes in UpdateActor#512
Eitan Yarmush (EItanya) merged 1 commit into
agent-substrate:mainfrom
troychiu:fix-update-actor-span-attributes

Conversation

@troychiu

Copy link
Copy Markdown
Contributor

This PR adds missing OpenTelemetry span attributes in UpdateActor and corresponding tests.

  • Tests pass
  • Appropriate changes to documentation are included in the PR

@EItanya

Copy link
Copy Markdown
Collaborator

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good to me, just added a nit test comment.

if err := validateUpdateActorRequest(req); err != nil {
return nil, err
}
setSpanActorRefAttributes(ctx, req.GetActor().GetAtespace(), req.GetActor().GetName())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you mind adding a failed lookup test case so a missing actor path is guarded? Similar to

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.TemplateNameKey, ateattr.TemplateNamespaceKey, ateattr.ActorVersionKey} {
if _, ok := attrs[k]; ok {
t.Errorf("unexpected %s on failed-pause span", k)
}
}
}

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.

Updated! Thank you

@troychiu
Troy Chiu (troychiu) force-pushed the fix-update-actor-span-attributes branch from b76b5fb to d57ea55 Compare July 27, 2026 21:06
@troychiu
Troy Chiu (troychiu) force-pushed the fix-update-actor-span-attributes branch from d57ea55 to 09d977e Compare July 28, 2026 23:09
@troychiu

Troy Chiu (troychiu) commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main.

@troychiu

Copy link
Copy Markdown
Contributor Author

Eitan Yarmush (@EItanya) can we merge this?

@EItanya

Copy link
Copy Markdown
Collaborator

Sorry, ended up with conflicts from another merge :(

@troychiu

Copy link
Copy Markdown
Contributor Author

Just rebased onto main! Thank you.

@EItanya
Eitan Yarmush (EItanya) merged commit 5bf19fc into agent-substrate:main Jul 29, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants