No pixa-specific Prometheus metrics, and metrics are not collected at all unless credentials are configured #85

New Issue

clawbot · 2026-08-09T03:50:01+02:00

clawbot commented

2026-08-09 03:50:01 +02:00

Verified against main at 61f42e6. Covers the "Prometheus performance metrics" item in TODO.md.

1. There are no pixa-specific metrics. What exists is generic HTTP metrics from go-http-metrics (internal/middleware/middleware.go:124-132): request counts, durations, status codes. Nothing exposes what an operator actually needs to run this service:

cache hit/miss ratio (the single most important number for a cache)
current cache size on disk vs. the configured limit, and eviction counts/bytes (newly meaningful with #51/PR #55)
upstream fetch count, bytes, duration, and error rate by outcome
transcode duration and in-flight processing count
negative-cache hits

README.md:128 lists "Metrics: Prometheus" under Architecture, which oversells what is there.

2. Metrics are not collected unless metrics.username is set. internal/server/routes.go:24-26,63-68 installs the recorder middleware only inside the same conditional that mounts the authenticated /metrics endpoint. So without credentials, metrics are not merely unexposed — they are never recorded. That may well be intentional (no collection cost when nobody is scraping), but the coupling is invisible and surprising: an operator who mounts /metrics behind their own reverse-proxy auth, without setting pixa's credentials, gets an endpoint that reports nothing.

Uncertain: whether item 2 is deliberate. Confirm intent before changing it — if deliberate, document it rather than "fixing" it.

Definition of done

Domain metrics registered and emitted for at least: cache hits/misses (by class — variant vs source), cache bytes on disk and configured limit, eviction count and bytes, upstream fetch count/bytes/duration/errors, transcode duration, in-flight processing count.
Metric names follow Prometheus conventions (pixa_ prefix, base units, _total on counters) and are documented in the README.
Item 2 resolved deliberately: either decouple collection from credential configuration, or document the coupling in README.md and config.example.yml. Say which and why in the PR.
Cardinality is bounded — do not label metrics with upstream host or path. An image proxy sees unbounded hosts and paths; a per-host label is a memory leak in the metrics registry with the same shape as the unbounded hostSems map.
Tests asserting the counters move as expected on a hit, a miss, and an eviction.
make check green.

Coordination

Do after #55 merges — the eviction and cache-size metrics only exist once that lands, and it is the source of the numbers worth exporting.

Verified against `main` at `61f42e6`. Covers the "Prometheus performance metrics" item in `TODO.md`. **1. There are no pixa-specific metrics.** What exists is generic HTTP metrics from `go-http-metrics` (`internal/middleware/middleware.go:124-132`): request counts, durations, status codes. Nothing exposes what an operator actually needs to run this service: - cache hit/miss ratio (the single most important number for a cache) - current cache size on disk vs. the configured limit, and eviction counts/bytes (newly meaningful with #51/PR #55) - upstream fetch count, bytes, duration, and error rate by outcome - transcode duration and in-flight processing count - negative-cache hits `README.md:128` lists "**Metrics**: Prometheus" under Architecture, which oversells what is there. **2. Metrics are not collected unless `metrics.username` is set.** `internal/server/routes.go:24-26,63-68` installs the recorder middleware only inside the same conditional that mounts the authenticated `/metrics` endpoint. So without credentials, metrics are not merely unexposed — they are never recorded. That may well be intentional (no collection cost when nobody is scraping), but the coupling is invisible and surprising: an operator who mounts `/metrics` behind their own reverse-proxy auth, without setting pixa's credentials, gets an endpoint that reports nothing. **Uncertain:** whether item 2 is deliberate. Confirm intent before changing it — if deliberate, document it rather than "fixing" it. ## Definition of done 1. Domain metrics registered and emitted for at least: cache hits/misses (by class — variant vs source), cache bytes on disk and configured limit, eviction count and bytes, upstream fetch count/bytes/duration/errors, transcode duration, in-flight processing count. 2. Metric names follow Prometheus conventions (`pixa_` prefix, base units, `_total` on counters) and are documented in the README. 3. Item 2 resolved deliberately: either decouple collection from credential configuration, or document the coupling in `README.md` and `config.example.yml`. Say which and why in the PR. 4. Cardinality is bounded — **do not** label metrics with upstream host or path. An image proxy sees unbounded hosts and paths; a per-host label is a memory leak in the metrics registry with the same shape as the unbounded `hostSems` map. 5. Tests asserting the counters move as expected on a hit, a miss, and an eviction. 6. `make check` green. ## Coordination Do **after** #55 merges — the eviction and cache-size metrics only exist once that lands, and it is the source of the numbers worth exporting.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:50:01 +02:00

clawbot referenced this issue

2026-08-09 16:10:39 +02:00

Tracking: 1.0.0 sequencing and critical path #103

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/pixa#85