Skip to content

metrics: DisableCompression on promhttp.HandlerOpts + pin in tests#43

Merged
mfwolffe merged 3 commits into
trunkfrom
fix/metrics-disable-promhttp-gzip
May 10, 2026
Merged

metrics: DisableCompression on promhttp.HandlerOpts + pin in tests#43
mfwolffe merged 3 commits into
trunkfrom
fix/metrics-disable-promhttp-gzip

Conversation

@espadonne
Copy link
Copy Markdown
Contributor

Summary

F1 from the post-hardening audit (2026-05-10).

PR #34 moved `/metrics` out of the chi Compress middleware group, which the audit verified by code inspection. But the live droplet still returns `Content-Encoding: gzip` when scrapers send `Accept-Encoding: gzip` — because `promhttp.HandlerFor` runs its OWN gzip layer with `DisableCompression` defaulting to `false`. PR #34's regression test passed because it used a stub handler, not the real `metrics.Handler()`.

Production is currently saved by alloy's `enable_compression = false` workaround (PR #32). This PR removes the dependency on that workaround.

Changes

  1. `internal/infra/metrics/metrics.go` — set `DisableCompression: true` on `promhttp.HandlerOpts`. Comment explains why.
  2. `internal/web/handlers/metrics_compress_test.go` — split into two tests:
    • `TestMetricsRouteBypassesCompressMiddleware` (existing, renamed) — pins layer 1 (chi middleware)
    • `TestMetricsHandlerPromhttpCompressionDisabled` (new) — pins layer 2 (promhttp); calls `metrics.Handler("", "")` directly with `Accept-Encoding: gzip` and asserts no `Content-Encoding` header

A regression in either layer now fails loud.

After this lands

Once deployed, the alloy workaround in #32 becomes redundant. Filing a follow-up issue to drop `enable_compression = false` from the alloy template + the live droplet config — but only after this binary is verified live.

Test plan

  • `go build ./...` clean
  • Both tests pass: `go test ./internal/web/handlers/ -run TestMetrics -count=1 -v`
  • After deploy: `curl -H 'Accept-Encoding: gzip' http://shithub.sh/metrics\` (basic-auth'd or otherwise reachable) returns plain text

@mfwolffe mfwolffe merged commit 0fc23b6 into trunk May 10, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants