mcp(telemetry): put the tool call's outcome on the OTel span instead of only in the log line

[phamngocquy]

Summary

packages/loopover-contract/src/telemetry.ts:248 declares the span contract:

/**
 * OTel span attributes for one tool call.
 *
 * Deliberately a STRICT SUBSET of the usage event -- no arguments, no results, not even the
 * excluded-marker. ...
 */
export function buildMcpToolSpanAttributes(call: McpToolCallTelemetry): Record<string, unknown> {
  return { tool, category, surface, transport, ok, duration_ms, ...(errorCode ? { error_code } : {}) };
}

src/mcp/dispatch-telemetry.ts:15 repeats the promise in the chokepoint's own header: "an OTel span
mcp.tool/<name> on the self-host path, whose attributes are a strict subset -- never arguments."

buildMcpToolSpanAttributes never reaches a span. Its only two call sites are the two structured log
lines — src/mcp/dispatch-telemetry.ts:116 and :135:

log("warn", "mcp_tool_call_failed", buildMcpToolSpanAttributes(call));
...
log("error", "mcp_tool_call_threw", buildMcpToolSpanAttributes(call));

The span's actual attributes are a separate literal built before the call runs, at
src/mcp/dispatch-telemetry.ts:84:

const attributes = { tool: toolName, category, surface: "remote" as const };
...
return await sink.withSpan(mcpToolSpanName(toolName), attributes, async () => { ... });

So a self-hosted operator's tracing backend receives mcp.tool/<name> spans carrying tool, category
and surface and nothing else. ok, transport and — the one that matters for triage — error_code
never reach the span, so a trace view cannot be filtered or grouped by cause the way the PostHog view
can. withOtelSpan (src/selfhost/otel.ts:348) sets SpanStatusCode.ERROR on a throw, which is the only
outcome signal the span carries today; the resolved closed-set code that the very same call object
already holds is dropped.

The gap is structural, not an oversight at one line: DispatchTelemetrySink.withSpan
(src/mcp/dispatch-telemetry.ts:51) takes its attributes once, at open, and exposes no way to add any
before the span ends — so instrumentToolDispatch has no seam through which to publish an outcome it only
learns after the handler returns.

Deliverables

• DispatchTelemetrySink.withSpan's signature gains a seam for post-hoc attributes, and
  NOOP_DISPATCH_SINK plus createDispatchTelemetrySink
  (src/mcp/dispatch-telemetry-sink.ts:75) both implement it.
• instrumentToolDispatch publishes buildMcpToolSpanAttributes(call) onto the span on the return
  path and on the throw path.
• src/selfhost/otel.ts's runner (or the closure the self-host entry registers via
  setMcpDispatchSpanRunner) applies those attributes to the real span through otelSafeAttributes,
  the same scrubber every other attribute goes through.
• A regression test at test/unit/mcp-dispatch-telemetry.test.ts named for this bug that injects a
  recording withSpan sink, runs a handler that returns normally and one that throws, and asserts the
  span for each ends with ok and — for the throw — an error_code drawn from
  MCP_TELEMETRY_ERROR_CODES.
• A test asserting NOOP_DISPATCH_SINK.withSpan is still a pure passthrough that records nothing and
  returns the handler's value unchanged.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for
example adding the attributes on the success path only, or changing the sink signature without wiring
the self-host runner — does not resolve this issue.

Test plan

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's
coverage.include covers src/**/*.ts (line 78) and packages/loopover-contract/src/**/*.ts (line 108),
so every touched path is measured and gated. Both arms of each branch need a test: the return path
versus the throw path in instrumentToolDispatch, the ...(call.errorCode ? { error_code } : {}) spread
in buildMcpToolSpanAttributes (already covered by test/unit/mcp-dispatch-telemetry.test.ts:71 and
:75 — keep both), the call.transport ?? "local" nullish arm, and the
withSpan ?? getMcpDispatchSpanRunner() ?? passthrough chain at
src/mcp/dispatch-telemetry-sink.ts:94, whose three arms must each be exercised.

Fixes #10042

[loopover-orb]

Tip

✅ LoopOver review result - approve/merge recommended

_{Review updated: 2026-07-31 13:31:35 UTC}

7 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

• safe to merge

Review summary
This PR closes the actual gap the issue describes: dispatch-telemetry.ts's withSpan now takes a `fn` that receives a `setOutcomeAttributes` callback, and the tool-dispatch chokepoint (src/mcp/dispatch-telemetry.ts) calls it with the same `buildMcpToolSpanAttributes(call)` payload already used for the log lines, on both the success and throw paths. server.ts wires the callback through `setCurrentOtelSpanAttributes` while still inside `withOtelSpan`'s active span context, and the new selfhost-otel.test.ts assertion confirms the exported span actually carries `ok`, `transport`, and `error_code` — the fields the issue says were missing. The refactor also correctly moves the try/catch that used to wrap the whole `sink.withSpan` call to live inside the `fn` passed to it, so the error still propagates out for `withOtelSpan`'s own ERROR-status handling, and the never-throws contract on the new outcome-setter is preserved and tested (test/unit/mcp-dispatch-telemetry.test.ts's 'never lets a failing setOutcomeAttributes reach the caller').

Nits — 5 non-blocking

• The span now opens with `{}` attributes (dispatch-telemetry.ts:120, `sink.withSpan(mcpToolSpanName(toolName), {}, ...)`) instead of `{tool, category, surface}` as before — if a process crash or forced flush ever exports the span before the handler resolves, it would now carry zero attributes instead of at least the three static ones; worth a one-line comment on why this tradeoff is acceptable.
• `buildMcpToolSpanAttributes(call)` is now computed twice per failed/thrown call (once in `publishSpanOutcome`, once again for the `log(...)` line at dispatch-telemetry.ts:143/152) — could be hoisted into a single local to avoid the duplicate work and keep the two payloads visibly identical by construction.
• `SetMcpSpanOutcomeAttributes` is exported from dispatch-telemetry.ts and re-exported/imported in three other files (dispatch-telemetry-sink.ts, dispatch-span-registry.ts) — consider whether it belongs in the contract package alongside `McpToolCallTelemetry` instead, since it's now part of the same cross-file wiring contract those other types live in.
• Add a short comment at dispatch-telemetry.ts:120 explaining why span-open attributes are now empty (deferred entirely to `setOutcomeAttributes`) so a future reader doesn't assume it's an oversight.
• Factor the duplicated `buildMcpToolSpanAttributes(call)` call in the success and throw branches into a single computed value passed to both `publishSpanOutcome` and `log`.

Decision drivers

• ✅ Code review — No blockers (1 reviewer)
• ✅ Gate result — Passing (No configured blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#10042
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 68 registered-repo PR(s), 19 merged, 0 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	phamngocquy; Gittensor profile; 68 PR(s), 0 issue(s).
Improvement	✅ Minor	risk: clean · value: minor · LLM: significant

Linked issue satisfaction

Addressed
The diff adds a setOutcomeAttributes seam to withSpan/NOOP_DISPATCH_SINK/createDispatchTelemetrySink/the registry, publishes buildMcpToolSpanAttributes(call) on both the return and throw paths, wires the self-host runner to apply it via setCurrentOtelSpanAttributes/otelSafeAttributes, and includes regression tests in mcp-dispatch-telemetry.test.ts and selfhost-otel.test.ts asserting ok/error_code

Review context

• Author: phamngocquy
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: Java, Python, Lua, Jupyter Notebook, C, Dockerfile, JavaScript, Shell
• Official Gittensor activity: 68 PR(s), 0 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Keep the PR focused and include validation evidence before maintainer review.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

• @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
• @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
• A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

• Re-run LoopOver review

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.50%. Comparing base (07a43da) to head (4b934df).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10230      +/-   ##
==========================================
+ Coverage   80.47%   80.50%   +0.02%     
==========================================
  Files         282      285       +3     
  Lines       58856    58925      +69     
  Branches     6978     6993      +15     
==========================================
+ Hits        47366    47435      +69     
  Misses      11199    11199              
  Partials      291      291              

Flag	Coverage Δ
backend	100.00% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/mcp/dispatch-span-registry.ts	100.00% <ø> (ø)
src/mcp/dispatch-telemetry-sink.ts	100.00% <100.00%> (ø)
src/mcp/dispatch-telemetry.ts	100.00% <100.00%> (ø)

[loopover-orb]

LoopOver approves — the gate is satisfied and CI is green.