Skip to content

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

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

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.

Requirements

  • The mcp.tool/<name> span must carry the same attributes buildMcpToolSpanAttributes produces for the
    completed call — including ok and, on a failure, error_code — on both the return path and the throw
    path.
  • buildMcpToolSpanAttributes must be the single source of those attributes. Do not build a second
    literal in src/mcp/dispatch-telemetry.ts; the pre-call literal at line 84 is either replaced or fed
    from the contract helper.
  • DispatchTelemetrySink.withSpan gains the seam needed to publish attributes discovered after the
    handler runs (for example, by having the wrapped function return the attributes alongside its result, or
    by passing a setter into it). Whatever the shape, NOOP_DISPATCH_SINK
    (src/mcp/dispatch-telemetry.ts:55) must remain a pure passthrough and the Worker path must stay a
    passthrough at zero cost — the reason the runner is a registry (src/mcp/dispatch-span-registry.ts:8)
    is that Workers has no collector.
  • Span attributes must stay a strict subset of the usage event: no arguments, no results, no
    payloads_excluded. buildMcpToolSpanAttributes already guarantees this and must not be widened.
  • What must NOT change: the two structured log lines and their event names (mcp_tool_call_failed,
    mcp_tool_call_threw) — the self-host Loki pipeline parses them; mcpToolSpanName's
    mcp.tool/<tool> format; withOtelSpan's existing status/exception-event handling in
    src/selfhost/otel.ts:348-375, including the deliberate hand-built exception event that routes
    through otelSafeAttributes.
  • What must NOT change: the guarantee that telemetry never turns a working tool call into a failed one —
    every new path stays inside the existing best-effort try/catch posture.

⚠️ Required pattern: src/mcp/dispatch-telemetry.ts:86-95's emit closure, which is built once and
called from both the return and the throw path with the completed call — the span attributes need the
same treatment. What does NOT satisfy this issue: (a) importing src/selfhost/otel.ts from
src/mcp/dispatch-telemetry.ts to reach the active span directly, which pulls the tracer into the
Cloudflare Worker bundle and defeats the registry indirection; (b) opening a second, nested span just to
carry the outcome; (c) adding ok/error_code to the pre-call literal at line 84, where neither value
is known yet.

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 Coverage Requirements

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.

Expected Outcome

A self-hosted operator's trace view can filter mcp.tool/* spans by outcome and by closed-set error code,
matching what the PostHog usage_event breakdown already shows, and buildMcpToolSpanAttributes is used
for the spans its name and doc describe rather than only for two log lines.

Links & Resources

  • packages/loopover-contract/src/telemetry.ts:248buildMcpToolSpanAttributes
  • src/mcp/dispatch-telemetry.ts:84 — the pre-call literal that is actually used as span attributes
  • src/mcp/dispatch-telemetry.ts:116 / :135 — the helper's only two call sites, both log lines
  • src/mcp/dispatch-telemetry.ts:51DispatchTelemetrySink.withSpan, attributes-at-open only
  • src/mcp/dispatch-telemetry-sink.ts:94 — the runner resolution chain
  • src/mcp/dispatch-span-registry.ts:8 — why the runner is a registry and not an import
  • src/selfhost/otel.ts:348withOtelSpan and otelSafeAttributes

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions