[rupesh-kumar-sah]

Summary

Previously, src/metrics.ts had no visibility into the MCP server surface, leaving tool calls, latencies, and errors unmonitored on dashboards.

This PR instruments BazaarMcpServer with Prometheus metrics using closed, bounded label sets:

• mcp_tool_calls_total (Counter): tracks tool calls by tool name (bazaar_search, paid_call, unknown) and outcome (success, error).
• mcp_tool_duration_seconds (Histogram): measures execution duration per tool with bounded buckets.
• mcp_errors_total (Counter): counts errors partitioned strictly by MCP_ERROR_CODES values.
• Exposes all metrics on the existing /metrics endpoint via the shared Prometheus register.

Changes

• Defined mcp_tool_calls_total, mcp_tool_duration_seconds, and mcp_errors_total in src/metrics.ts.
• Recorded tool execution outcomes and timers in CallToolRequestSchema handler in src/mcp/server.ts.
• Incremented mcp_errors_total on every error code emission in createErrorResponse.
• Added comprehensive assertions in tests/mcp.test.ts verifying all 3 metrics in the registry output.

Closes #425

[drips-wave]

@rupesh-kumar-sah Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[Miracle656]

Needs a rebase — #521 landed this morning and rewrote most of src/mcp/server.ts underneath this branch.

src/mcp/server.ts   3 conflicts,  525 lines
tests/mcp.test.ts   2 conflicts, 1567 lines

Those are not adjacent additions. #521 added an authenticated Streamable HTTP transport, a spend budget with a payment_budget tool, a payment ledger and bounded tool results, so the dispatch path your metrics wrap is a different shape now. I am not resolving 525 lines of it by hand: paid_call moves real money, and choosing between two versions of that handler is a behaviour decision rather than a merge.

Your #444 went in cleanly an hour ago, so the rebase should be the only obstacle here.

What to carry across

Instrumenting tool calls, durations and errors is worth having, and it composes with what #521 added rather than competing with it. Two things to line up while you rebase:

• src/mcp/server.ts now distinguishes several failure modes that deserve separate label values rather than one error bucket: ERR_UNKNOWN_TOOL (yours, from #444), ERR_PAYMENT_REJECTED when no payment keys are configured, and ERR_BUDGET_EXCEEDED when a paid call would exceed the daily ceiling. A duration histogram that cannot tell a refused payment from a failed upstream is much less useful at 2am.
• Keep the label set finite. src/metrics.ts already carries a comment on ingest_last_success_timestamp warning against labels that grow with user configuration — tool name is fine, anything carrying a URL or a caller id is not.

Rebase, get npx tsc --noEmit and npx vitest run clean, and re-request. Worth doing soon: #554 is also waiting on a rebase over the same #521 changes, and whichever lands first moves the file under the other.

[rupesh-kumar-sah]

@Miracle656 Rebased cleanly over latest main (4dc40a3 / #521) and force-pushed.

Summary of Rebase & Alignment:

1. Composed cleanly with feat(mcp): add authenticated Streamable HTTP transport alongside stdio (#419) #521 additions:
   • Metrics wrapper in CallToolRequestSchema now instruments the rewritten dispatch path, capturing tool execution durations and outcomes without interfering with budget holds, streamable HTTP transport, or payment ledger records.
   • Distinct failure modes (ERR_UNKNOWN_TOOL, ERR_PAYMENT_REJECTED, ERR_BUDGET_EXCEEDED, ERR_SERVICE_UNREACHABLE, etc.) are tracked via mcp_errors_total counter labeled by structured error code.
2. Cardinality Protection:
   • Kept label sets strictly closed and finite (tool strictly bounds to known tools or 'unknown', outcome to 'success'/'error', code to enumerated McpErrorCode). No URLs, endpoints, or caller IDs are injected as metric labels.
3. Verification:
   • npx tsc --noEmit passed with 0 errors.
   • Full test suite verified green (npx vitest run: 134/134 test files passed, 1604/1604 tests passing, including MCP server identity and Prometheus metrics assertions).

Ready for re-review!

[Miracle656]

Scanned clean. mcp_tool_calls_total, mcp_tool_duration_seconds and mcp_errors_total are the right three — count, latency and failures is what you need to answer "is the MCP server healthy", and the MCP surface has had none of it.

The branch is 79 commits behind main, and merging it as-is would revert shipped work.

Against current main the diff removes roughly 6,000 lines from src/ that are not yours — files that landed after you branched: src/reconciliation/dispersion.ts (312, #448), src/__tests__/venueReconciliation.test.ts (385), src/__tests__/observationAndSourceTime.test.ts (335, #240), and src/routes/facilitator.ts, whose x402 spend ceiling landed this morning.

Please rebase on current main and re-request.

Two things that moved under src/mcp/ specifically, and they matter for these metrics:

• #521 reworked the server: an authenticated Streamable HTTP transport, a spend budget with a payment_budget tool, a payment ledger, and bounded tool results.
• #554 landed an hour ago: fetchWithPayment is now optional, and paid_call refuses up front with ERR_PAYMENT_REJECTED when no payment keys are configured.

That second one changes what your error counter should record. A paid_call refused because the server has no keys is a configuration state, not a failure — if it lands in mcp_errors_total undifferentiated, a correctly-configured read-only server will look permanently broken on a dashboard. Worth a reason label distinguishing a refusal from a genuine tool error, with the values bounded to a small fixed set.

Also worth knowing: #434 is open and adds GraphQL operation and resolver metrics. Different namespace, so no collision, but you are both extending src/metrics.ts and whoever rebases second will meet the other there.

One general note, since it applies to any new metric here: keep every label bounded by construction. tool is a finite set of tool names, which is fine. Never label with anything caller-supplied — an unbounded label set grows /metrics until it cannot be scraped, which takes out monitoring rather than just skewing it.

https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB