Repository navigation
feat(server-utils): Emit execute_tool spans for Anthropic's beta.sessions.events.toolRunner() - #25554
Conversation
…ions.events.toolRunner() `beta.sessions.events.toolRunner()` runs the user's tools for a Managed Agents session, but its tool calls got no span. The runner turns a throw into an error result for the model, so nobody saw a failing tool either. #25320 only covered `beta.messages.toolRunner()`. The session runner calls the same `run(input, ctx)` as the messages runner and keeps its tools on a public `tools` field, so this reuses the wrapping from #25320. It has no `addTools()`, so it can only run the tools it was created with. On the orchestrion path the hook sits on the `SessionToolRunner` constructor instead of `toolRunner()`, because the SDK's `EnvironmentWorker` creates runners with `new SessionToolRunner()` directly. `instrumentAnthropicAiClient` can only wrap `toolRunner()`, so it doesn't see those runners. Supported from SDK 0.97.0, the first version with the session runner. Fixes #25446 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
size-limit report 📦Compared: bd1c3d5 (PR test merge using develop at 7030f21) Against: 7030f21 (develop) · base run #63715 ✅ Baseline matches: this diff contains only this PR's changes.
|
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit da32a71. Configure here.
Optional E2E testsSome optional checks were not successful. They do not block the merge. 9 not successful, 56 successful. See the run.
Successful checks
|
isaacs
left a comment
isaacs
left a comment
There was a problem hiding this comment.
Two small things that might be worth looking at, and/or posting a follow-up about. LGTM!
|
|
||
| function handleToolError(span: Span, error: unknown, context: RunContext): void { | ||
| if (context?.signal?.aborted) { | ||
| span.setStatus({ code: SPAN_STATUS_ERROR, message: 'cancelled' }); |
There was a problem hiding this comment.
(low/preexisting)
A timeout gets recorded as cancelled, and the tool's actual error is dropped. The PR body lists this as a known limitation. It has a second effect: handleToolError returns early when the signal is aborted, so captureException never runs. A tool that hangs for 120s and then throws (or any tool that throws after the per-tool timeout) produces no error event. For a tool that hangs, a timeout is the most useful failure to see.
A closure over the runner won't fix it. You could compare runner.signal.aborted with ctx.signal.aborted, but EnvironmentWorker can pass the same tools array to every session's runner. instrumentTools wraps each tool only once, so a closure would hold the first runner only.
Suggestion: keep the limitation for now. We could ask upstream to abort with a reason (for example, toolCtrl.abort(new DOMException('...', 'TimeoutError'))). Then handleToolError could map a TimeoutError reason to deadline_exceeded and still capture it.
There was a problem hiding this comment.
I agree, keeping it as the known limitation. I'll open an issue on the SDK asking for a TimeoutError reason on the per-tool abort, then mapping it to deadline_exceeded and capturing is a one line on our side
| /** Give each tool call of a Managed Agents session runner a `gen_ai.execute_tool` span. */ | ||
| export function instrumentSessionToolRunner(runner: unknown, options: AnthropicAiOptions): void { | ||
| // The session runner has no `addTools()`: it only runs the tools it was created with. | ||
| instrumentTools((runner as SessionToolRunner | undefined)?.tools, options); |
There was a problem hiding this comment.
(low)
instrumentToolRunner starts with if (!runner || typeof runner !== 'object') return;. instrumentSessionToolRunner instead does (runner as SessionToolRunner | undefined)?.tools. Both are safe, because instrumentTools rejects anything that is not an array. But the two sibling functions read differently.
Suggestion: use the same early return in both, or drop it from instrumentToolRunner if instrumentTools is the real guard.
There was a problem hiding this comment.
fixed! used the same early return in both. couldn't drop it from instrumentToolRunner, wrapInPlace(runner, 'addTools', …) reads runner[key] so it'd throw on null
| functionQuery: { className: 'Messages', methodName: 'toolRunner', kind: 'Sync' as const }, | ||
| })), | ||
| // The constructor, not `sessions.events.toolRunner()`, so runners built directly (e.g. by `EnvironmentWorker`) are covered. | ||
| ...['lib/tools/SessionToolRunner.js', 'lib/tools/SessionToolRunner.mjs'].map(filePath => ({ |
There was a problem hiding this comment.
(Should've been in my review, but I forgot to hit the button first 😅)
The constructor hook targets EnvironmentWorker. But the worker drives each runner from its own poll loop (lib/environments/worker.mjs, for await (const _ of runner)), and nothing in user code wraps that loop in a span. So each tool call becomes its own root span (its own segment under span streaming). A busy session would produce lots of separate execute_tool segments, but not tied to the session, because the agent.tool_use event has no session id, and the span sets no gen_ai.conversation.id, which may be noisy and hard to use.
Suggestion: we could consider a parent span per runner iteration? For example, wrap the runner's [Symbol.asyncIterator] in an invoke_agent-style span that sets gen_ai.conversation.id from runner.sessionId, so the tool spans nest under it. Or if that's out of scope, add a line to the PR body that says EnvironmentWorker tool calls show up as root spans, and open a follow-up issue.
There was a problem hiding this comment.
Makes sense to me! fwiw anthropic's own otel tracing puts one anthropic.environment.work span around each session and nests the tool spans under it, so a per runner span sounds right. opened #25768 for it
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
beta.sessions.events.toolRunner()runs the user's tools for a Managed Agents session, but its tool calls got no span. The runner turns a throw into an error result for the model, so nobody saw a failing tool either. #25320 only coveredbeta.messages.toolRunner().The session runner calls the same
run(input, ctx)as the messages runner and keeps its tools on a publictoolsfield, so this reuses the wrapping from #25320. It has noaddTools(), so it can only run the tools it was created with. On the orchestrion path the hook sits on theSessionToolRunnerconstructor instead oftoolRunner(), because the SDK'sEnvironmentWorkercreates runners withnew SessionToolRunner()directly.instrumentAnthropicAiClientcan only wraptoolRunner(), so it doesn't see those runners. Supported from SDK 0.97.0, the first version with the session runner.Known limitation. A tool that stops on the session runner's 120s timeout is recorded as cancelled, like a user abort, because the SDK aborts the same signal for both.
Fixes #25446
🤖 Generated with Claude Code