feat(telemetry): expose tool call arguments/result in tools/call span (opt-in) - #281
Conversation
… (opt-in) The tools/call telemetry span ([:anubis_mcp, :server, :tool_call]) only ever carried the tool name and an is_error flag, never the arguments a tool was called with or the result it returned. Integrators building observability around a production MCP server could measure that a tool was called and whether it errored, but never what it was asked or what was returned. Add a config flag, :telemetry_capture_tool_payload, defaulting to false, that opts a server into carrying arguments in the span's start metadata and result in its stop metadata. This mirrors the opt_in requirement level OpenTelemetry's GenAI semantic conventions assign to the equivalent gen_ai.tool.call.arguments / gen_ai.tool.call.result span attributes -- tool payloads are the highest-cardinality, highest-PII surface in the request lifecycle, so capture must be an explicit choice, never a default. Applies uniformly across protocol eras: tools/call keeps the same name/arguments shape in both the legacy version modules and the new 2026-07-28 stateless dialect, and the scheduler's span sits above the era distinction in the call stack. Adds test coverage for both the default-off and opt-in-on paths, and documents the flag in pages/testing.md alongside the PII rationale.
github-actions
Bot
added
the
needs-work
|
Thanks for the PR! Some automated quality checks didn't pass - see the |
acollado-g2
changed the title
|
Warning Review limit reached
Next review available in: 10 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughProblemTool-call telemetry did not capture request arguments or handler results. SolutionAdd opt-in payload capture through RationaleImprove tool-call observability while limiting PII exposure. Add tests and document configuration and privacy considerations. WalkthroughTool-call execution now uses Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f0dd3dd2-d7b0-4c25-8520-4b0187e939c2
📒 Files selected for processing (3)
lib/anubis/server/session/scheduler.expages/testing.mdtest/anubis/server/session_test.exs
- Instrument the task-augmented tools/call worker path (Session.Tasks.spawn_worker/6), which previously bypassed the tool_call span entirely. Extract the span logic shared by both the synchronous scheduler dispatch and the async task worker into Telemetry.span_tool_call/3, so there is one implementation instead of two diverging copies. - Move the :telemetry_capture_tool_payload default into a module attribute (@default_capture_tool_payload), matching this codebase's existing convention for configurable defaults. - Fix the pages/testing.md example: arguments only exists in :start metadata and result/is_error only in :stop metadata, so a single :stop-only handler pattern-matching on all three would never match. Attach to both events instead, and note the task-augmented path is covered too. - Restore the telemetry_capture_tool_payload application env correctly in the opt-in test: Application.get_env/2 can't distinguish 'unset' from 'explicitly nil', so an unset key was being persisted as nil on exit instead of actually deleted. Use fetch_env/2 to capture the original state and delete_env/2 to restore an absent key. - Add a dedicated test proving the task-augmented path now emits the same telemetry span as the synchronous path.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/anubis/server/session_test.exs (1)
804-805: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winP3 — Test the unset configuration path.
Line 805 sets the flag to
false. The test therefore verifies explicit disablement, not the default when the key is absent. Delete the application key before the request.Proposed fix
- Application.put_env(:anubis_mcp, :telemetry_capture_tool_payload, false) + Application.delete_env(:anubis_mcp, :telemetry_capture_tool_payload)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9483988f-9a22-4a1f-a031-9b0af8e8fab7
📒 Files selected for processing (6)
lib/anubis/server/session/scheduler.exlib/anubis/server/session/tasks.exlib/anubis/telemetry.expages/testing.mdtest/anubis/server/session_test.exstest/anubis/server/tasks_test.exs
… testing.md - Add a Examples section to span_tool_call/3's @doc, matching the convention used by other modules with iex> examples in this codebase. Kept simple (no Frame struct, no map literal) to avoid custom-inspect and key-ordering mismatches in the example output. - pages/testing.md's :telemetry.attach_many example called Logger.info/2 without require Logger; since Logger.info is a macro, copying the example as-is would emit a compile warning. Add the require.
The default-behavior test set the flag to false rather than leaving it unset, so it exercised explicit disablement instead of the true default when no one has configured anything. Delete the application env key instead.
2 participants
What
Adds tool call
argumentsandresultto the[:anubis_mcp, :server, :tool_call]telemetry span metadata, gated behind a config flag that defaults tofalse.Why
Anubis.Server.Session.Scheduler'stools/callhandler already wraps execution in a:telemetry.span, but the span only ever carried the tool name and an error flag:https://github.com/zoedsoupe/anubis-mcp/blob/main/lib/anubis/server/session/scheduler.ex#L184-L192
For anyone instrumenting a production MCP server, this means you can measure that a tool was called and whether it errored, but never what it was asked or what it returned — the two things that actually matter for debugging tool behavior or validating output quality over time. Both values are already in scope at the point the span is created (
request["params"]["arguments"]and the computedresult); they're just never surfaced.This applies uniformly across protocol eras. I checked
V2026_07_28(the new stateless dialect) against the legacy version modules —tools/callkeeps the samename/argumentsparam shape in both — and the scheduler's span sits above the era distinction in the call stack, so this single change covers legacy and stateless servers without duplication.Why opt-in, not default-on
Tool arguments and results are the highest-cardinality, highest-PII surface in the request lifecycle. This matches where the spec ecosystem has already landed: OpenTelemetry's GenAI semantic conventions define
gen_ai.tool.call.arguments/gen_ai.tool.call.resulton both the genericexecute_toolspan and MCP's ownmcp.server/mcp.clientspan types, and both are markedopt_in— OTel's strictest requirement level.So this PR follows that same posture: default off, one config flag, integrator opts in with full knowledge of the tradeoff.
Changes
lib/anubis/server/session/scheduler.ex—do_handle_request/4for"tools/call"now conditionally includesargumentsin the span's start metadata andresultin its stop metadata, controlled byApplication.get_env(:anubis_mcp, :telemetry_capture_tool_payload, false).test/anubis/server/session_test.exs— newdescribeblock with two tests: default behavior is unchanged (noarguments/resultin span metadata when the flag is unset), and with the flag enabled,:telemetry.attach_manycapturesargumentson span start andresulton span stop. Built on the existingTasksStubServer/Sessionharness, same pattern as the adjacenttool_call telemetry :is_error metadatablock.pages/testing.md— new "Observing tool calls" section documenting the flag, an example:telemetry.attach, and the PII/