metrics: expose Prometheus exposition at /metrics

[aterga]

Summary

Prometheus instrumentation for this crate, usable from the library and not only from the bundled binary, plus a GET /metrics exposition in that binary.

Pinned to prometheus = "0.14" with the process feature — the version and features dfinity/ic already depends on, so these series land in the same estate without a second exposition dialect.

Related issues

Complements the hosted status dashboard and #113's off-host probe rather than replacing either — those answer "is it up" from outside; this answers "what is it doing" from inside.

Changes

• src/metrics.rs — collectors, label bounds, the two middlewares, tests.
• src/lib.rs — pub mod metrics.
• src/main.rs — owns a Registry, applies the two layers, serves /metrics.
• deploy/native/Caddyfile — the path is not published publicly.
• Cargo.toml / Cargo.lock — the dependency.

Built for embedders, after consumer feedback

The first version of this put everything in main.rs, which made it invisible to anyone depending on imcp2 as a library — mod metrics; in a bin target is private to that target. Four changes came out of that feedback:

• Exported from the crate. pub mod metrics, exposing Metrics, both middlewares, route_label, method_label, register_process_collector.
• log_request split into write_request_metrics and write_request_logs. Their constraints genuinely differ — metrics must bound every label, while a log line can afford the full path and is more useful carrying it — so one combined layer forced the stricter rule on both. Split, an embedder can also take either alone.
• The log moved from info to debug. It fires on every request including the noise floor of an internet-facing service; at info it drowns the handful of lines an operator wants.
• Metrics::new borrows the caller's Registry rather than owning one. A host embedding this crate already has a registry and already exposes it; series published into a private one would never be seen.

Two consequences of that last one, both deliberate:

• render() is gone. Exposition follows the registry, so the binary gathers its own. The scrape-duration signal survives as observe_scrape, recorded by whoever renders.
• The library no longer registers ProcessCollector. process_* is un-namespaced and describes the whole OS process — the embedding application's process, not this crate's — and would collide with a host that already has one. It is now an explicit register_process_collector that the standalone binary calls, because the binary is the application.

The metric prefix is also factored into a concat! macro rather than repeated seven times. Chosen over const + format! because it keeps the names &'static str and greppable in full: searching an alert rule's imcp2_http_requests_total still lands on the defining line. Deliberately fixed rather than caller-configurable — a metric name identifies the software emitting it, and one dashboard working across every deployment depends on that.

Label cardinality is the design problem

Every series is a row Prometheus holds in memory, so a label whose value an outsider picks is a memory-exhaustion primitive. Two labels are outsider-chosen and both are bounded: route is axum's MatchedPath (the route template, never the requested path), and method is allow-listed to the standard nine, since HTTP permits arbitrary extension tokens. status needs no bound — the server chooses it from a small closed set.

Testing

• cargo test — 178 pass; cargo clippy --all-targets — 0 errors.
• Proven usable from outside the crate. A scratch consumer crate depending on imcp2 registers its own metric and imcp2's into one caller-owned Registry, does its own exposition, and asserts no process_* leaked in. That is the thing that was impossible before, and it is checked rather than claimed.
• Log level verified in both directions: 0 http request lines at default RUST_LOG, present under RUST_LOG=imcp2=debug.
• Cardinality bounds tested end to end through a router with the real middleware — not by calling the recorder with a pre-computed label, which only proves the recorder is deterministic. Confirmed by mutation: raw path instead of MatchedPath fails the path test and nothing else; method_label as pass-through fails the two method tests and nothing else.
• The method test also pins the histogram bucket count, where a cardinality bug does ~14× the damage a counter shows.
• Two library-safety properties pinned by tests: a second Metrics::new against one registry returns AlreadyReg rather than panicking, and Metrics::new registers no process_*.
• Binary re-verified live: /metrics serves, 32 imcp2_* series, process collector present because the binary registers it.

Known limitation, not addressed here

nest_service("/mcp", …) means axum omits MatchedPath for everything below the mount — verified against a running server:

/version              → route="/version"
/mcp/oauth/token      → route="other"
/mcp/oauth/authorize  → route="other"
/wp-login.php         → route="other"

So all 26 MCP tools and all five OAuth endpoints currently share one label with scanner noise. The HTTP middleware structurally cannot see inside the mount, which is the real argument for instrumenting McpServer itself — deliberately left to a follow-up rather than expanding this change.

Open question for the reviewer

Does the scraper have a network path to the host's private address on :8000? I could not verify that from here. If not, a blackbox probe of the already-public /status/api/status needs no path at all — and this endpoint stays useful for embedders and local debugging regardless.

---

Generated by Claude Code

[Copilot]

🟡 Changes recommended

Arbitrary HTTP methods can still create unbounded metric series, and the cardinality test does not exercise actual requests.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Adds Prometheus observability for HTTP traffic, sessions, build details, and process health.

Changes:

• Adds a Prometheus registry and /metrics endpoint.
• Records request latency/counts and session gauges.
• Blocks public /metrics access through Caddy.

File summaries

File	Description
src/metrics.rs	Defines collectors, exposition, and tests.
src/main.rs	Adds metrics routing and request instrumentation.
deploy/native/Caddyfile	Returns 404 for public metrics requests.
Cargo.toml	Adds the Prometheus dependency.
Cargo.lock	Locks Prometheus and transitive dependencies.

Review details

• Files reviewed: 4/5 changed files
• Comments generated: 2
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

🟡 Changes recommended

Concurrent scrapes can expose an inconsistent session gauge pair that violates the documented invariant.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Suppressed comments (1)

src/metrics.rs:214

• live and active are separate collectors and are updated independently while Axum can run another /metrics scrape concurrently. When counts decrease, a concurrent registry.gather() can observe the new live value and the old active value, exporting active > live despite the metric's stated invariant. Serialize each session-pair update with gathering (or publish both values from one synchronized collector/snapshot).

        self.live_sessions.with_label_values(&[instance]).set(live);
        self.active_sessions
            .with_label_values(&[instance])
            .set(active);

• Files reviewed: 4/5 changed files
• Comments generated: 0 new
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/metrics.rs:221

• This gauge is named imcp2_process_start_time_seconds because metric! always adds the prefix, so it is not the conventional process_start_time_seconds claimed here. The binary also immediately registers ProcessCollector::for_self(), which already exports the conventional series using the OS process start; this extra gauge instead records a later point after application initialization. Remove the duplicate custom gauge/constructor parameter and use the process collector's series, or rename and document it as an application-ready timestamp.

        let start_time = IntGauge::new(
            metric!("process_start_time_seconds"),
            "Unix epoch seconds at which this process started, i.e. when the deployment \
             last restarted. Every deploy restarts the service.",
        )?;
        registry.register(Box::new(start_time.clone()))?;
        start_time.set(started_at as i64);

src/metrics.rs:245

• observe_request is public for embedders, but it writes method directly as a label. Only the provided middleware calls method_label, so a host using this recording API with a request method can reintroduce the unbounded attacker-controlled cardinality that this module promises to prevent. Normalize the method inside this method; the middleware's existing normalization is harmless if retained.

    pub fn observe_request(&self, route: &str, method: &str, status: u16, elapsed_secs: f64) {

src/metrics.rs:351

• This changes the existing request log from info to debug. Both deploy/native/imcp2.service and the Docker image default to RUST_LOG=info, so deployed request lines—including the full paths that metrics intentionally discard—will disappear, contrary to the PR's claim that the existing log remains. Preserve the info level, or update the runtime filters to enable imcp2::metrics=debug and document the behavior change.

    tracing::debug!(%method, %path, status, elapsed_ms, "http request");

[Copilot]

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/metrics.rs:226

• This metric is named and documented as a process start time, but the bundled binary passes a timestamp captured only after agent/server construction and session-reaper startup (src/main.rs:233-273); the surrounding comment here instead calls it the time serving began, which is also later than that timestamp. Consumers computing process uptime will therefore get a value with the wrong semantics. Either capture the actual process/service start at the beginning of main, or rename and document the metric to match the event being recorded.

        let start_time = IntGauge::new(
            metric!("process_start_time_seconds"),
            "Unix epoch seconds at which this process started, i.e. when the deployment \
             last restarted. Every deploy restarts the service.",
        )?;

src/metrics.rs:151

• Registration is not atomic across this constructor. If an embedder's registry already contains a later name such as imcp2_live_sessions, this call first registers the request collectors and then returns AlreadyReg, leaving inaccessible collectors behind in the caller-owned registry and preventing a clean retry or fallback. Roll back every collector registered by this invocation when any subsequent registration fails (and add a test where the collision occurs after the first collector).

        registry.register(Box::new(requests.clone()))?;

src/main.rs:413

• The scrape timer starts only after both session_gauges() calls, even though each call scans the full session map and is part of producing the scrape. If those scans become the source of scrape latency or timeouts, imcp2_metrics_scrape_duration_seconds will stay low and miss exactly the degradation it is intended to expose. Start the timer at the beginning of the handler, before refreshing the gauges.

                        let started = std::time::Instant::now();