metrics: stop counting client-cancelled requests as server errors

[rdimitrov]

Summary

• Remap recorded status to 499 (NGINX-style "client closed") when ctx.Context().Err() is context.Canceled after the handler returns
• Skip the http_errors_total increment for 499s so the availability metric reflects server-visible errors only

Why

Under scraper-driven load, clients with short timeouts paginate /v0.1/servers, give up while PG is busy, and close the TCP connection. The handler's DB iteration returns context canceled, the handler converts that to huma.Error500InternalServerError, and tries to write a response to the now-closed socket. NGINX records 499 (client closed) and never delivers anything to the client — but the registry's middleware records status_code=500 and bumps mcp_registry_http_errors_total.

That's what's been firing the Availability dropped below 95% alert during the daily 17:00 UTC bursts. Recent prod numbers from one pod (~19h uptime):

mcp_registry_http_errors_total{path="/v0.1/servers", status_code="500"} 7396
mcp_registry_http_errors_total{path="/v0.1/servers/{serverName}/versions", status_code="500"} 283
mcp_registry_http_errors_total{path="/v0/servers", status_code="500"} 352

…against zero 5xx in NGINX ingress logs over the same window. The alert was correct given the data it had; the data was misclassifying client cancellations as server errors.

What this does

In internal/api/router/router.go (the metric middleware): after next(ctx), check ctx.Context().Err(). If it's context.Canceled, override statusCode to 499 and skip the error counter. Requests counter and duration histogram still record so the cancellation rate stays visible.

context.DeadlineExceeded is intentionally not remapped — that would indicate a server-side timeout if we ever add per-request deadlines, and should still count as a server error.

What this does not do

• Doesn't reduce the underlying load (ServiceNow + notion catalog scrapers driving the daily peak — separate work, see the updated_since outreach we're doing).
• Doesn't change handler behaviour — handlers still call huma.Error500… on context.Canceled. We could short-circuit earlier (return without writing) to save the work, but that's a bigger refactor and the metric fix unblocks the alert immediately.

Companion change (no code, just a Grafana annotation tweak)

The Availability dropped below 95% alert (UID bexrc60etvhmoa) currently has the annotation:

No. of 5xx are increased from acceptable limit i.e. 5% of total throughput

The query is on mcp_registry_http_errors_total, not 5xx specifically. After this change the count is meaningful again, but the wording is still misleading. Suggest updating to:

Internal error rate exceeded 5% of total throughput

(no PromQL change required.)

Test plan

• go test ./internal/api/router/... — new test passes
• golangci-lint run ./internal/... — clean
• After merge, watch a daily 17:00 UTC burst — mcp_registry_http_errors_total{status_code="500", path="/v0.1/servers"} should stay flat while mcp_registry_http_requests_total{status_code="499", path="/v0.1/servers"} grows. Availability alert should not fire on cancellation-only bursts.

🤖 Generated with Claude Code