Prometheus /metrics endpoint for proxy and proposal observability

[wankhede04]

Closes #329.

What

Adds an opt-in GET /metrics endpoint (AGENT_VAULT_METRICS_ENABLED) exposing the standard Prometheus text exposition format:

• agent_vault_proxy_requests_total{service,status} — proxied request outcomes
• agent_vault_proxy_request_duration_seconds — latency histogram
• agent_vault_proposals{status} — current proposal backlog by lifecycle status (pending, applied, rejected, expired), across all vaults — e.g. agent_vault_proposals{status="pending"} for the pending backlog

Disabled by default; unauthenticated when enabled (consistent with /health and /v1/status) — meant to be scraped from a trusted network or behind a reverse proxy that adds auth, and clearly documented as such.

Why

Agent Vault is meant to run as its own trust boundary / infrastructure (per the README), but there was no metrics export of any kind — only a debug log line per request or querying the request-log table directly. Neither wires into Prometheus/Grafana/Datadog, making it hard to alert on things operators actually care about (proposal backlog growing, latency regressions, error-rate spikes by service).

Design notes

• Proxy metrics are recorded via a requestlog.Sink that stacks into the existing requestlog.MultiSink alongside the audit-log sink — the package's docstring already describes this as the intended extension point ("Later sinks... stack here without touching the proxy"), so the hot path itself is untouched.
• Proposal gauge is queried live from the store (CountProposalsByStatus) on every scrape rather than tracked via incremented counters at each transition call site. This is deliberate: proposal status changes happen from several places (approve, reject, the lazy expiry sweep), and a live query can never drift from the proposals table regardless of which path fired — whereas scattering counter increments across those call sites risks silently undercounting if a path is missed (now or in a future change).
• Metric names/labels follow the shape proposed in the issue.

Changes

• internal/metrics: Metrics, EnabledFromEnv, the requestlog.Sink adapter, and the live proposals collector.
• internal/store: CountProposalsByStatus(ctx) — a single global GROUP BY status query.
• internal/server: GET /metrics, gated by AttachMetrics (404 until attached, same pattern as other optional server behavior).
• cmd: wiring in both the foreground and detached server-start paths.
• Docs: .env.example, environment-variables.mdx (new Prometheus metrics section + metric reference table), reference/cli.mdx, README.md.

Scoped to proxy + proposal metrics for this first pass, per the phased approach from the issue — netguard-blocked and rate-limit-rejected counters are natural follow-ups, noted in the docs as such. Happy to take those on separately.

Testing

• New unit tests: internal/metrics (env parsing, sink recording, proposals gauge including the "store errors don't break the scrape" and "all four statuses always reported, even at zero" cases), internal/server (/metrics 404-when-unattached vs 200-when-attached, live gauge values), internal/store (CountProposalsByStatus across multiple vaults/statuses), cmd (env-gated wiring).
• Manual smoke test against the real built binary: /metrics 404s by default, returns proper Prometheus output with AGENT_VAULT_METRICS_ENABLED=true.
• go build ./..., go vet ./..., go test ./... all pass (one pre-existing, unrelated failure in internal/isolation — a Docker-socket-path test that also fails on main in this sandbox).
• go.mod/go.sum diff is minimal — only github.com/prometheus/client_golang and its own transitive deps.

[greptile-apps]

Greptile Summary

This PR adds an opt-in Prometheus metrics endpoint for proxy and proposal observability. The main changes are:

• Proxy request counters and latency histograms recorded through the request-log sink.
• Live proposal gauges backed by a global status-count query.
• Optional /metrics wiring for foreground and detached servers.
• Configuration documentation and tests for the new metrics flow.

Confidence Score: 4/5

The database-backed scrape path needs concurrency and query bounds before merging.

• The registry, sink wiring, and aggregate query are consistent.
• Concurrent unauthenticated scrapes can consume the database pool.
• Client cancellation does not stop the proposal query.

internal/metrics/metrics.go and internal/metrics/proposals.go

Security Review

The unauthenticated metrics endpoint allows unbounded concurrent database-backed scrapes. Since proposal collection uses a background context, abandoned requests can continue waiting for or using database connections and cause resource exhaustion.

Important Files Changed

Filename	Overview
internal/metrics/metrics.go	Adds the Prometheus registry, proxy metrics, scrape handler, and request-log sink adapter; concurrent gathers are not limited.
internal/metrics/proposals.go	Adds a live proposal collector whose database query uses an unbounded background context.
internal/store/sql_store.go	Adds a global grouped query for proposal counts with row and scan error handling.
cmd/server.go	Wires metrics into both server startup paths and combines the metrics and audit-log sinks.
internal/server/server.go	Adds optional metrics attachment and registers the public /metrics route.
internal/server/handle_metrics.go	Returns 404 when metrics are disabled and delegates enabled requests to the Prometheus handler.

_{Reviews (1): Last reviewed commit: "feat: Prometheus /metrics endpoint for p..." | Re-trigger Greptile}

[greptile-apps]

Unbounded Database-Backed Scrapes

When metrics are enabled, unauthenticated callers can start unlimited concurrent gathers, each running the proposal count query with a background context. Repeated requests or disconnected clients can keep queries waiting for database connections and exhaust the pool, blocking normal application traffic.

[wankhede04]

Thanks for the review, @greptile-apps! Addressed the unbounded database-backed scrapes issue in fb0ce55:

/metrics is unauthenticated (same as /health), so without a bound, a flood of concurrent scrapes could each open their own CountProposalsByStatus query and exhaust the database connection pool. The proposals collector now:

• Caches the result for 5s behind a mutex, so concurrent/rapid scrapes serialize on one in-flight query and share its result instead of each issuing their own.
• Caps the query itself with a 3s timeout, so a slow/hanging database can't pin a scrape (and a connection) indefinitely.
• Serves the last good cached value on a transient query failure instead of nothing, so a brief blip doesn't blank the gauge.

Added TestProposalsCollector_CachesWithinTTL, TestProposalsCollector_ConcurrentScrapesShareOneQuery, and TestProposalsCollector_QueryTimesOutRatherThanBlockingForever (all pass with -race) plus reran the full existing suite.

[greptile-apps]

The implementation is clean and correct. The key design choices all hold up:

Mutex-held-for-duration (rather than a singleflight or separate "in-flight" flag) is the right call here — the 19 concurrent waiters serialise on one lock acquisition, each sees cachedAt within the TTL on their turn, and the store is hit exactly once. TestProposalsCollector_ConcurrentScrapesShareOneQuery proves this correctly: one goroutine acquires the lock and blocks on release; the other 19 wait on the mutex; after close(counter.release) the first completes and populates c.cached; each subsequent waiter finds a fresh cache hit and returns without calling the store.

Stale-on-error semantics (return c.cached, c.cached != nil) are exactly right — a transient blip shouldn't zero out the gauge on dashboards, and the ok=false-only-on-never-succeeded path matches the documented "skip on error" contract from before the cache was added.

Timeout vs. TTL ordering (3s < 5s) is sound — the query always gives up before the next scrape could legitimately bypass the cache.

One minor note: TestProposalsCollector_QueryTimesOutRatherThanBlockingForever waits up to proposalsQueryTimeout + 2s = 5s in the worst case, which will slow down the test suite on every run (not just failures). Since the test only needs to verify the query eventually unblocks, the outer deadline can be tighter — something like proposalsQueryTimeout + 500*time.Millisecond gives the same failure-detection guarantee without adding unnecessary wall-clock time on the happy path. Not a blocker, just worth tidying.

Everything else looks good — the concern is addressed.

[wankhede04]

All Greptile review comments have been addressed and fixed. I take look on concerned Reported by greptile but that's not a major one, Would appreciate it if someone from the team could take a look and merge if it looks good. Thanks!