Skip to content

fix(telemetry): expose tool call success/failure in tool_call span metadata - #246

Merged
zoedsoupe merged 1 commit into
zoedsoupe:mainfrom
acollado-g2:fix/tool-call-error-status-telemetry
Jul 29, 2026
Merged

zoedsoupe merged 1 commit into
zoedsoupe:mainfrom
acollado-g2:fix/tool-call-error-status-telemetry

Conversation

@acollado-g2

Copy link
Copy Markdown
Contributor

Problem

The tool_call telemetry span (do_handle_request/4 for "tools/call" requests) always emits the same :stop metadata, %{tool: tool_name}, regardless of whether the wrapped call succeeded, returned a protocol-level error (e.g. tool not found, invalid params), or returned a CallToolResult with isError: true.

The actual result of module.handle_request(request, frame) is available inside the span function, but it is discarded before being turned into span metadata — a telemetry handler attached to this event has no way to tell a successful tool call apart from a failed one without independently re-deriving that information elsewhere (e.g. by separately handling [:anubis_mcp, :server, :error], which only fires for protocol-level errors, not for tool-level isError: true results).

Solution

Bind the result of module.handle_request(request, frame) and derive an is_error boolean from it before it becomes span metadata:

  • {:error, _reason, _frame} (protocol-level error) → is_error: true
  • {:reply, %{"isError" => true}, _frame} (tool-level error via Response.error/2) → is_error: true
  • anything else → is_error: false

Added three regression tests covering all three branches (successful call, tool-level isError: true, protocol-level tool-not-found), attached directly to the tool_call :stop telemetry event.

Rationale

This was the smallest possible change that surfaces the missing signal: the result is already computed and in scope inside the span closure, so no additional work is done, only a cheap pattern match on a value that already exists. No public API, return value, or behavior changes — this only adds a new key to telemetry metadata, which is additive and non-breaking for existing handlers.

I kept the derivation as a private function (tool_call_error?/1) rather than inlining it, since the two failure shapes it distinguishes (protocol error vs. tool-level isError) are exactly the two failure modes the MCP spec itself defines, and a named predicate makes that mapping explicit rather than embedding it in the span call.

Related to #243 (unprefixed event name) but independent — this PR does not touch the event namespace, only the :stop metadata shape. Happy to rebase on top of #244 if that merges first.

…tadata

The tool_call telemetry span's :stop event metadata only ever carried
%{tool: tool_name}, regardless of whether the wrapped handle_request/2
call succeeded, returned a protocol-level error, or returned a
CallToolResult with isError: true. Consumers attaching handlers to this
event have no way to distinguish a successful tool call from a failed
one without re-deriving it from the (discarded) result.

Add an is_error boolean to the span's stop metadata, derived from the
actual result: true for {:error, _, _} protocol errors and for
{:reply, %{"isError" => true}, _} tool-level errors, false otherwise.
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4058a4b6-1044-4c05-a210-49ae0399123a

📥 Commits

Reviewing files that changed from the base of the PR and between 2451bb7 and 559fc00.

📒 Files selected for processing (2)
  • lib/anubis/server/session.ex
  • test/anubis/server/session_test.exs

📝 Walkthrough

Problem

tool_call telemetry did not indicate whether a request resulted in an error.

Solution

Add an is_error flag to span stop metadata for protocol-level and tool-level errors, with regression tests covering error and success cases.

Rationale

Improves telemetry visibility without changing public APIs or request behavior.

Walkthrough

Anubis.Server.Session now adds is_error to tools/call telemetry metadata. Error detection covers {:error, ...} handler results and JSON-RPC replies with "isError" => true, while non-tool requests retain direct delegation. Tests verify telemetry classification for successful calls, tool execution errors, and missing tools.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely matches the PR’s main change: adding tool-call success/failure metadata to telemetry.
Description check ✅ Passed The description follows the template with Problem, Solution, and Rationale and includes the key implementation details.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@acollado-g2

Copy link
Copy Markdown
Contributor Author

QA — verified against a live MCP deployment

In addition to the unit tests in this PR, I ran a broad manual QA pass against a real, running MCP server deployment fronted by this library (multi-pod, behind a load balancer, real network hops) to make sure nothing about the change's assumptions breaks under real traffic:

  • Full protocol smoke suite: handshake, tools/list, tools/call across all registered tools, resources/list/read, prompts/list/get — 22/22 requests, 63/63 assertions passed.
  • Concurrent stress: two separate bursts of 30 and 20 fully-parallel initialize → notifications/initialized → tools/list handshakes against fresh sessions — 50/50 passed, 0% failure rate.
  • Sequential stress: 20 iterations of a 4-step handshake+tool-call cycle run back-to-back — 80/80 requests passed.
  • Varied tools/call traffic: ~17 real calls across all registered tools with different arguments, to generate broad, realistic tool_call span coverage.
  • Deliberate failure cases (the actual scenarios this PR's is_error metadata targets):
    • 3× calls to nonexistent tool names → protocol-level {:error, ...} (invalid_params, "Tool not found").
    • 1× call with a required argument omitted → protocol-level {:error, ...} (invalid_params).
    • 1× tool call whose own logic returned a result-level error → {:reply, %{"isError" => true, ...}, frame}.

All five failure cases returned exactly the response shapes this PR's tool_call_error?/1 pattern-matches on, confirming the classification logic covers both real-world failure paths (not just the two shapes constructed in the unit tests). No unexpected 5xx responses, no regressions in unrelated request/tool-call volume, observed across the whole test window.

The deployment I tested against is still on the pre-fix version of this library, so I could not observe is_error show up in that host's own live metrics yet — but this pass validates that the underlying success/failure code paths this PR taps into behave exactly as assumed under real concurrent load, not just in the isolated test harness.

@zoedsoupe
zoedsoupe merged commit ace5ecb into zoedsoupe:main Jul 29, 2026
13 checks passed
@zoedsoupe zoedsoupe mentioned this pull request Jul 29, 2026
zoedsoupe added a commit that referenced this pull request Jul 29, 2026
🚀 Want to release this?
---


##
[1.11.0](v1.10.0...v1.11.0)
(2026-07-29)


### Features

* **session_store:** supervise Redis store subtree to make restarts
race-free ([#242](#242))
([48c8c1a](48c8c1a))


### Bug Fixes

* **prompts:** wrap prompt content objects and map system_message to
user role ([#234](#234))
([2451bb7](2451bb7))
* **server:** make title option compile in use Anubis.Server.Component
([b04feed](b04feed))
* **server:** use restart :temporary for session processes
([#240](#240))
([30bc4f5](30bc4f5))
* **sse:** buffer partial events across Finch chunks
([#245](#245))
([beea2f6](beea2f6))
* Stream the client SSE GET instead of buffering it (server push never
delivered) ([#231](#231))
([a722c1b](a722c1b))
* **telemetry:** expose tool call success/failure in tool_call span
metadata ([#246](#246))
([ace5ecb](ace5ecb))
* **telemetry:** include client_info in initialize response event
metadata ([#248](#248))
([74ed457](74ed457))
* **telemetry:** namespace tool_call span under :anubis_mcp
([#244](#244))
([561a96b](561a96b))


### Tests

* **server:** fix stale tool_call event name and function_exported?
loading races
([7247d2c](7247d2c))
* **transport:** synchronize held SSE plug with Bypass teardown
([93c5a34](93c5a34))


### Continuous Integration

* fix dialyzer plt caching
([8424439](8424439))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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.

2 participants