feat: expose live Prometheus tool metrics - #244
Merged
kerlenton merged 3 commits intoAug 26, 2026
Conversation
Review of the Prometheus listener. The gate passes here, race detector included, which the PR body says could not be run on the author's machine. Five things it does not prove. A tool label is whatever a peer put in params.name, and Write repeats it on seventeen lines per series. Nothing bounded its length or the number of distinct names, and neither peer has to cooperate: the store recognises a tools/call by its method, so a server writing them on its own stdout mints one series each. Measured on this branch, one 4 MiB name rendered a 71 MB scrape body, which Prometheus drops whole against body_size_limit, taking every real series for that target with it, and 50,000 names held 50,000 series for the life of the process. Names are now truncated at 128 bytes before the cardinality check, so long variants fold together rather than each minting a series, and past two thousand series the rest are counted under tool="(over-series-cap)". Folding rather than evicting, because evicting resets a counter and Prometheus reads that as a target restart. requestKey dropped the direction that store.callKey carries and explains. JSON-RPC scopes id uniqueness to the sender, so a server may legally issue sampling/createMessage, roots/list or elicitation/create carrying the id of a tool call still in flight. Observe read that as a retry and finished the live call, so its error and its latency were never counted. Verified against all three methods before and after. A whole class of failure was invisible. A 502 from a gateway, or a 401 challenge, arrives as a status and a body that is not JSON-RPC, so the store counts it against the session and attaches no call. Nothing said which request it answered, since the connection id is the client's address and a client may have several in flight, so it was counted as a call with no error and no latency. An outage read as traffic stopping rather than traffic failing, while a default check run over the same capture failed on it. Those now have their own series, mcpsnoop_transport_errors_total by status, exported even when empty so a graph of it on a healthy hub is flat rather than absent. Nothing is guessed onto a tool. A headless hub could not be stopped. Run closes the listener on cancel and then waits on connection goroutines parked in Decode, which return only when their shim disconnects, so SIGTERM did nothing for as long as a wrapped server lived. The TUI never noticed because it does not wait for Run to return. Accepted connections are now closed with the listener. Shutdown also reported the listener's timeout ahead of the hub's error, so an ordinary stop during a scrape exited 1 and hid what the hub had to say. Deriving the server labels walked every session in the store, allocating a header for each, once per observed envelope while the write lock was held. A session's identity is fixed once its first frames land, so it is memoised. The README section gains what it left out: that the endpoint has no authentication, that the tool label is bounded and what the overflow series means, where a transport failure is counted, and that a restart resets the counters. It also carried the file's only semicolon. Not changed, having been checked and found sound: escapeLabel matches the three escapes the exposition format requires, Write copies under a read lock and formats without it, and the growth of operations and inflight for unanswered calls is dominated by store retention that predates this change and costs less here than the TUI path does today.
kerlenton
force-pushed
the
feat/137-prometheus-metrics
branch
from
August 26, 2026 19:03
c6a5a6b to
d3b5071
Compare
Owner
|
LGTM! Thanks! |
Contributor
Author
|
Thanks again for the review and merge! I genuinely enjoyed working on this. If there are any other issues where an extra pair of hands would be useful, feel free to tag or assign me. I’d be very happy to help. And if there’s anything on the roadmap that you’d like someone to take on, I’d be glad to work on that too. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #137
Add an opt-in hub-side Prometheus listener. Bare mcpsnoop keeps its interactive TUI; mcpsnoop --metrics-listen runs the hub headlessly so the metrics process remains available without an interactive terminal.
Startup backfill primes session metadata and correlation state but is excluded from live counters. HTTP correlation includes ConnID, and MRTR operations remain one call with one latency observation. Reused request identities also clear stale pending tool state.
Public metrics: mcpsnoop_tool_calls_total, mcpsnoop_tool_errors_total, and mcpsnoop_tool_call_duration_seconds.
The listener is independent of the MCP proxy and uses stdlib-only Prometheus text exposition, with no new runtime dependency.
Tests run: targeted metrics, hub, TUI, and CLI tests; repeated regression tests for HTTP correlation, backfill priming, reconnect identity, headless lifecycle, and default routing; go vet ./...; staticcheck; cross-platform vet for Windows, Darwin, and Linux; git diff --check. The full go test ./... run is blocked on this Windows environment by existing Unix executable, file-mode, pipe, editor, and timestamp assumptions. POSIX make check could not run because the available POSIX shell cannot see go or gofmt. Race tests could not run because CGO/GCC is unavailable.