Count hits, misses, fetches and transforms as Prometheus counters (closes #311)

The five counts the cache_stats table held are now Prometheus counters in
memory, as fields of the cache. NewCache makes them and registers them on
the registry it is given. pixad gives it Prometheus' default registry,
which /metrics serves; each test gives it a new one. The Increment methods
add to their cache's counters, so counting makes no database call.

cache_stats, Cache.Stats, Service.Stats, CacheStats and the pending counts
are gone, with their tests. The background goroutine now writes only access
times, and its names say so: StartPendingAccessTimeWrites,
StopPendingAccessTimeWrites and pendingaccesstimes.go. The two stop tests
whose write under way was a count now use access times.

Model: opus-5-5

README.md

@@ -90,9 +90,8 @@ which answers 200 whenever pixa is running, in maintenance mode too (see
 
 On SIGTERM or SIGINT pixa stops accepting connections. The requests in progress
 and the images being processed get 5 seconds to finish. A request not finished
-by then is cut off. pixa then writes to the database the times cached images
-were last used that are not written yet (see Storage), and exits. It exits with
-1 when:
+by then is cut off. pixa then writes the waiting last-used times of cached
+images to the database (see Storage) and exits. It exits with 1 when:
 
 - images were still being processed after those 5 seconds,
 - some of those times could not be written, or
@@ -213,14 +212,14 @@ Test). Toward it, a cache hit opens the transformed image's file and nothing
 else, and makes no database call. The content type comes from the file's first
 bytes, which are those of the format pixa wrote it in; a file whose first bytes
 are not those of a format pixa writes is not served, and the image is converted
-again. The time the image was last served, which eviction orders by, is handed
-to a background writer without waiting for it. The hit is counted in memory. A
-miss also hands that writer the time it last used a cached source image. Once a
-second that writer writes the waiting times in one transaction. While 10,000
-times of transformed images, or 10,000 of source images, wait to be written, a
-further one is dropped, and that image only looks less recently used to
-eviction. `script/bench` measures how many hits pixa serves per second (see Load
-Test).
+again. A hit hands the time it was served to a background writer and does not
+wait for the writer. Eviction orders by that time. The hit is counted in memory.
+A miss that uses a cached source image also hands that writer the time it used
+the image. Once a second that writer writes the waiting times in one
+transaction. While 10,000 times of transformed images, or 10,000 of source
+images, wait to be written, a further one is dropped, and that image only looks
+less recently used to eviction. `script/bench` measures how many hits pixa
+serves per second (see Load Test).
 
 ### Routes
 
@@ -819,13 +818,8 @@ See `configs/config.example.yml` for all options with defaults.
 - **Database**: SQLite via modernc.org/sqlite
 - **Static assets**: embedded via `//go:embed`
 - **Metrics**: Prometheus, at `/metrics`. pixa serves `/metrics` and measures
-  HTTP requests only when `metrics.username` and `metrics.password` are set. A
-  request's `handler` label is the pattern of the route it matches, such as
-  `/v1/image/*`, never its path. Requests that match no route share the label
-  `unmatched`: those the router answers 404 or 405, and CORS preflights. A
-  request's `method` label is its method when that is `GET`, `HEAD`, `POST`,
-  `PUT`, `PATCH`, `DELETE`, `OPTIONS`, `CONNECT` or `TRACE`, and `other` for any
-  other method. `/metrics` holds:
+  HTTP requests only when `metrics.username` and `metrics.password` are set.
+  `/metrics` holds:
     - the duration of HTTP requests
     - the size of HTTP responses
     - the HTTP requests in flight
@@ -836,6 +830,14 @@ See `configs/config.example.yml` for all options with defaults.
       bytes
     - `pixa_upstream_fetch_bytes_total`: the bytes those fetches read
     - `pixa_transforms_total`: images converted
+
+    A request's `handler` label is the pattern of the route it matches, such as
+    `/v1/image/*`, never its path. Requests that match no route share the label
+    `unmatched`: those the router answers 404 or 405, and CORS preflights. A
+    request's `method` label is its method when that is `GET`, `HEAD`, `POST`,
+    `PUT`, `PATCH`, `DELETE`, `OPTIONS`, `CONNECT` or `TRACE`, and `other` for
+    any other method.
+
 - **Logging**: stdlib slog
 
 ## Entrypoints

TODO.md

@@ -34,13 +34,14 @@ P2: security: per-origin rate limiting
   and transforms are counted as Prometheus counters in memory (closes #311).
   They are `pixa_cache_hits_total`, `pixa_cache_misses_total`,
   `pixa_upstream_fetches_total`, `pixa_upstream_fetch_bytes_total` and
-  `pixa_transforms_total`, in `internal/imgcache/counters.go`, on the default
-  registry that `/metrics` serves. The `Increment` methods add to them.
-  `cache_stats`, `Cache.Stats`, `Service.Stats`, `CacheStats` and the pending
-  counts are gone. The background goroutine writes only access times now:
-  `StartPendingAccessTimeWrites` starts it and `StopPendingAccessTimeWrites`
-  stops it, in `pendingaccesstimes.go`. The counters count for the whole
-  process, so the tests that check them do not call `t.Parallel`.
+  `pixa_transforms_total`, fields of the cache. `NewCache` makes them and
+  registers them on the registry it is given (`internal/imgcache/counters.go`).
+  pixad gives it Prometheus' default registry, which `/metrics` serves; each
+  test gives it a new one. The `Increment` methods add to their cache's
+  counters. `cache_stats`, `Cache.Stats`, `Service.Stats`, `CacheStats` and the
+  pending counts are gone. The background goroutine writes only access times
+  now: `StartPendingAccessTimeWrites` starts it and
+  `StopPendingAccessTimeWrites` stops it, in `pendingaccesstimes.go`.
 - 2026-10-21 the schema file declares only the tables and columns pixa writes
   and reads (closes #310): `output_content` and `request_cache` are gone, and so
   are the columns nothing read: the host, path, query, `content_type` and

internal/app/app.go

@@ -7,6 +7,7 @@ import (
 	"os/signal"
 	"syscall"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/spf13/cobra"
 	"go.uber.org/fx"
 	"sneak.berlin/go/pixa/internal/config"
@@ -61,6 +62,8 @@ func run(_ *cobra.Command, _ []string) {
 			server.New,
 			middleware.New,
 			healthcheck.New,
+			// The registry /metrics serves
+			func() prometheus.Registerer { return prometheus.DefaultRegisterer },
 		),
 		fx.Invoke(
 			func(log *logger.Logger) { log.Identify() },

internal/handlers/cache_hit_internal_test.go

@@ -17,6 +17,7 @@ import (
 	"time"
 
 	"github.com/go-chi/chi/v5"
+	"github.com/prometheus/client_golang/prometheus"
 
 	"sneak.berlin/go/pixa/internal/database"
 	"sneak.berlin/go/pixa/internal/encurl"
@@ -120,7 +121,7 @@ func newCountingDatabaseServer(
 	cache, err := imgcache.NewCache(db, imgcache.CacheConfig{
 		StateDir:    tb.TempDir(),
 		NegativeTTL: 5 * time.Minute,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
 		tb.Fatalf("imgcache.NewCache() error = %v", err)
 	}

internal/handlers/cache_max_bytes_internal_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/fx/fxtest"
 
 	"sneak.berlin/go/pixa/internal/config"
@@ -127,7 +128,10 @@ func TestDiskCacheOffOnlyForExplicitZeroCacheMaxBytes(t *testing.T) {
 				t.Fatalf("database.New() error = %v", err)
 			}
 
-			_, err = New(lc, Params{Logger: log, Database: db, Config: cfg})
+			_, err = New(lc, Params{
+				Logger: log, Database: db, Config: cfg,
+				MetricsRegistry: prometheus.NewRegistry(),
+			})
 			if err != nil {
 				t.Fatalf("New() error = %v", err)
 			}

internal/handlers/fetcher_internal_test.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/go-chi/chi/v5"
+	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/fx"
 	"go.uber.org/fx/fxtest"
 
@@ -47,7 +48,8 @@ func TestHandlersBuildTheirOwnFetcherWhenNoneIsProvided(t *testing.T) {
 
 	app := fxtest.New(t,
 		fx.Supply(cfg),
-		fx.Provide(globals.New, logger.New, database.New, healthcheck.New, New),
+		fx.Provide(globals.New, logger.New, database.New, healthcheck.New, New,
+			func() prometheus.Registerer { return prometheus.NewRegistry() }),
 		fx.Populate(&h),
 	)
 	app.RequireStart()

internal/handlers/first_bytes_internal_test.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/go-chi/chi/v5"
+	"github.com/prometheus/client_golang/prometheus"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imagerequest"
@@ -35,7 +36,7 @@ func newRoutesFetchingThrough(
 	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
 		StateDir:    t.TempDir(),
 		NegativeTTL: 5 * time.Minute,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
 		t.Fatalf("imgcache.NewCache() error = %v", err)
 	}

internal/handlers/handlers.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/fx"
 	"sneak.berlin/go/pixa/internal/allowlist"
 	"sneak.berlin/go/pixa/internal/config"
@@ -30,6 +31,11 @@ type Params struct {
 	Database    *database.Database
 	Config      *config.Config
 
+	// MetricsRegistry is the registry the image cache registers its
+	// counters on. pixad provides Prometheus' default registry, the one
+	// /metrics serves.
+	MetricsRegistry prometheus.Registerer
+
 	// Fetcher, when provided, fetches upstream images in place of the
 	// fetcher the handlers build from the config. Only tests provide one;
 	// pixad does not.
@@ -52,6 +58,10 @@ type Handlers struct {
 	// refererBlocklist matches the hosts of referer_blocklist; its IsAllowed
 	// reports whether a URL's host is on that list.
 	refererBlocklist *allowlist.HostAllowList
+
+	// metricsRegistry is the registry the image cache registers its
+	// counters on.
+	metricsRegistry prometheus.Registerer
 }
 
 // New creates a new Handlers instance.
@@ -69,6 +79,7 @@ func New(lc fx.Lifecycle, params Params) (*Handlers, error) {
 		fetcher:          params.Fetcher,
 		csrfProtect:      csrfProtect,
 		refererBlocklist: allowlist.New(params.Config.RefererBlocklist),
+		metricsRegistry:  params.MetricsRegistry,
 	}
 
 	lc.Append(fx.Hook{
@@ -116,7 +127,8 @@ func newCacheConfig(cfg *config.Config, log *slog.Logger) imgcache.CacheConfig {
 
 // initImageService initializes the image cache and service.
 func (s *Handlers) initImageService() error {
-	cache, err := imgcache.NewCache(s.db.DB(), newCacheConfig(s.config, s.log))
+	cache, err := imgcache.NewCache(
+		s.db.DB(), newCacheConfig(s.config, s.log), s.metricsRegistry)
 	if err != nil {
 		return err
 	}

internal/handlers/handlers_internal_test.go

@@ -18,6 +18,7 @@ import (
 	"time"
 
 	"github.com/go-chi/chi/v5"
+	"github.com/prometheus/client_golang/prometheus"
 	"sneak.berlin/go/pixa/internal/database"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imgcache"
@@ -48,7 +49,7 @@ func setupTestHandler(t *testing.T) *testFixtures {
 	cache, err := imgcache.NewCache(db, imgcache.CacheConfig{
 		StateDir:    tmpDir,
 		NegativeTTL: 5 * time.Minute,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
 		t.Fatalf("failed to create cache: %v", err)
 	}

internal/handlers/image_cache_control_internal_test.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/go-chi/chi/v5"
+	"github.com/prometheus/client_golang/prometheus"
 
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/imagerequest"
@@ -33,7 +34,7 @@ func newSignedHostServer(
 	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
 		StateDir:    t.TempDir(),
 		NegativeTTL: 5 * time.Minute,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
 		t.Fatalf("imgcache.NewCache() error = %v", err)
 	}

internal/handlers/image_errors_internal_test.go

@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/go-chi/chi/v5"
+	"github.com/prometheus/client_golang/prometheus"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/imgcache"
@@ -31,7 +32,7 @@ func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
 	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
 		StateDir:    t.TempDir(),
 		NegativeTTL: 5 * time.Minute,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
 		t.Fatalf("failed to create cache: %v", err)
 	}

internal/handlers/referer_blocklist_internal_test.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/go-chi/chi/v5"
+	"github.com/prometheus/client_golang/prometheus"
 	"sneak.berlin/go/pixa/internal/allowlist"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
@@ -50,7 +51,7 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher)
 	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
 		StateDir:    t.TempDir(),
 		NegativeTTL: 5 * time.Minute,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
 		t.Fatalf("imgcache.NewCache() error = %v", err)
 	}

internal/imgcache/cache.go

@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
@@ -93,6 +94,14 @@ type Cache struct {
 	pendingAccessTimesDone    chan struct{}
 	pendingAccessTimesCancel  context.CancelFunc
 
+	// The counts of what the cache does, made and registered by
+	// registerCounters (see counters.go)
+	cacheHits          prometheus.Counter
+	cacheMisses        prometheus.Counter
+	upstreamFetches    prometheus.Counter
+	upstreamFetchBytes prometheus.Counter
+	transforms         prometheus.Counter
+
 	// reconciliationPageSize is the most rows one read of a content table
 	// returns in the reconciliation pass. newCache sets it to
 	// defaultReconciliationPageSize; tests set it smaller.
@@ -105,15 +114,22 @@ type Cache struct {
 	reconciliationReadTestHook func(rows int)
 }
 
-// NewCache creates a new cache instance.
-func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
-	return newCache(db, config, defaultFreeSpaceProbe)
+// NewCache creates a new cache instance and registers its counters on
+// metricsRegistry. pixad passes Prometheus' default registry, the one
+// /metrics serves.
+func NewCache(
+	db *sql.DB, config CacheConfig, metricsRegistry prometheus.Registerer,
+) (*Cache, error) {
+	return newCache(db, config, metricsRegistry, defaultFreeSpaceProbe)
 }
 
 // newCache is NewCache with the free-space probe passed in, so tests
 // can fake the free space the default limit is worked out from.
 func newCache(
-	db *sql.DB, config CacheConfig, probe FreeSpaceProbeFunc,
+	db *sql.DB,
+	config CacheConfig,
+	metricsRegistry prometheus.Registerer,
+	probe FreeSpaceProbeFunc,
 ) (*Cache, error) {
 	log := config.Logger
 	if log == nil {
@@ -135,6 +151,11 @@ func newCache(
 		reconciliationPageSize: defaultReconciliationPageSize,
 	}
 
+	err := c.registerCounters(metricsRegistry)
+	if err != nil {
+		return nil, err
+	}
+
 	if c.disabled {
 		return c, nil
 	}
@@ -362,13 +383,13 @@ func (c *Cache) CleanExpired(ctx context.Context) error {
 
 // IncrementStats counts a cache hit or miss, and an upstream fetch that read
 // fetchBytes bytes, as IncrementUpstreamFetch does. Like the other Increment
-// methods, it only adds to the Prometheus counters in memory (see
+// methods, it only adds to the cache's Prometheus counters in memory (see
 // counters.go), so it never waits for the database or fails.
 func (c *Cache) IncrementStats(hit bool, fetchBytes int64) {
 	if hit {
-		cacheHits.Inc()
+		c.cacheHits.Inc()
 	} else {
-		cacheMisses.Inc()
+		c.cacheMisses.Inc()
 	}
 
 	c.IncrementUpstreamFetch(fetchBytes)
@@ -381,13 +402,13 @@ func (c *Cache) IncrementUpstreamFetch(fetchBytes int64) {
 		return
 	}
 
-	upstreamFetches.Inc()
-	upstreamFetchBytes.Add(float64(fetchBytes))
+	c.upstreamFetches.Inc()
+	c.upstreamFetchBytes.Add(float64(fetchBytes))
 }
 
 // IncrementTransformCount counts one image transcoded by the image processor.
 func (c *Cache) IncrementTransformCount() {
-	transforms.Inc()
+	c.transforms.Inc()
 }
 
 // touchVariant hands the access time of a variant served from the cache to

internal/imgcache/cache_internal_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
 	_ "modernc.org/sqlite"
 )
 
@@ -52,7 +53,7 @@ func TestCache_CleanExpired(t *testing.T) {
 	cache, _ := NewCache(db, CacheConfig{
 		StateDir:    tmpDir,
 		NegativeTTL: 1 * time.Millisecond,
-	})
+	}, prometheus.NewRegistry())
 
 	ctx := context.Background()
 
@@ -103,7 +104,7 @@ func TestCache_StorageDirectoriesCreated(t *testing.T) {
 
 	_, err := NewCache(db, CacheConfig{
 		StateDir: tmpDir,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
 		t.Fatalf("NewCache() error = %v", err)
 	}

internal/imgcache/cachesize_internal_test.go

@@ -6,6 +6,8 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+
+	"github.com/prometheus/client_golang/prometheus"
 )
 
 // Static errors returned by the stub free-space probes below.
@@ -144,7 +146,7 @@ func TestNewCacheComputesDefaultMaxBytesWhenAsked(t *testing.T) {
 	cache, err := newCache(evictionTestDB(t), CacheConfig{
 		StateDir:           stateDir,
 		UseDefaultMaxBytes: true,
-	}, probe)
+	}, prometheus.NewRegistry(), probe)
 	if err != nil {
 		t.Fatalf("newCache returned error: %v", err)
 	}
@@ -175,7 +177,7 @@ func TestNewCacheKeepsExplicitMaxBytes(t *testing.T) {
 	cache, err := newCache(evictionTestDB(t), CacheConfig{
 		StateDir: t.TempDir(),
 		MaxBytes: 1024,
-	}, probe)
+	}, prometheus.NewRegistry(), probe)
 	if err != nil {
 		t.Fatalf("newCache returned error: %v", err)
 	}

internal/imgcache/concurrent_misses_internal_test.go

@@ -143,15 +143,13 @@ func jpegSize(data []byte) (int, int) {
 // one uncached variant while the first one's fetch is held. Between them they
 // must fetch the source once and transcode it once, every one must be answered
 // with the same 50x25 JPEG, and each must count one miss.
-//
-//nolint:paralleltest // it reads counters that other tests add to
 func TestService_Get_ConcurrentMissesShareOneFetch(t *testing.T) {
+	t.Parallel()
+
 	svc, fixtures, fetcher := setupHeldFetchService(t)
 
 	const requests = 8
 
-	before := readCounters()
-
 	pending := make([]<-chan getResult, 0, requests)
 	for range requests {
 		pending = append(pending,
@@ -195,8 +193,8 @@ func TestService_Get_ConcurrentMissesShareOneFetch(t *testing.T) {
 
 	want := counterValues{0, requests, 1, int64(len(photo)), 1}
 
-	if got := readCounters().minus(before); got != want {
-		t.Errorf("the requests added %+v to the counters, want %+v", got, want)
+	if got := readCounters(svc.cache); got != want {
+		t.Errorf("counters = %+v, want %+v", got, want)
 	}
 }
 

internal/imgcache/counters.go

@@ -1,35 +1,45 @@
 package imgcache
 
 import (
+	"fmt"
+
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/prometheus/client_golang/prometheus/promauto"
 )
 
-// The counts of what the image cache does, kept in memory as Prometheus
-// counters. They are registered on Prometheus' default registry, which
-// /metrics serves. Adding to one waits for nothing and makes no database
-// call. Each counts for the whole process since it started.
-//
-//nolint:gochecknoglobals // Prometheus registers each counter once per process
-var (
-	cacheHits = promauto.NewCounter(prometheus.CounterOpts{
+// registerCounters makes the cache's counters and registers them on
+// metricsRegistry. The counters are kept in memory, so adding to one waits
+// for nothing and makes no database call. It fails when metricsRegistry
+// already holds counters of the same names.
+func (c *Cache) registerCounters(metricsRegistry prometheus.Registerer) error {
+	c.cacheHits = prometheus.NewCounter(prometheus.CounterOpts{
 		Name: "pixa_cache_hits_total",
 		Help: "Image requests served from the cache.",
 	})
-	cacheMisses = promauto.NewCounter(prometheus.CounterOpts{
+	c.cacheMisses = prometheus.NewCounter(prometheus.CounterOpts{
 		Name: "pixa_cache_misses_total",
 		Help: "Image requests not served from the cache.",
 	})
-	upstreamFetches = promauto.NewCounter(prometheus.CounterOpts{
+	c.upstreamFetches = prometheus.NewCounter(prometheus.CounterOpts{
 		Name: "pixa_upstream_fetches_total",
 		Help: "Fetches of a source image from upstream that read any bytes.",
 	})
-	upstreamFetchBytes = promauto.NewCounter(prometheus.CounterOpts{
+	c.upstreamFetchBytes = prometheus.NewCounter(prometheus.CounterOpts{
 		Name: "pixa_upstream_fetch_bytes_total",
 		Help: "Bytes read from upstream by fetches of source images.",
 	})
-	transforms = promauto.NewCounter(prometheus.CounterOpts{
+	c.transforms = prometheus.NewCounter(prometheus.CounterOpts{
 		Name: "pixa_transforms_total",
 		Help: "Images the image processor transformed.",
 	})
-)
+
+	for _, counter := range []prometheus.Counter{
+		c.cacheHits, c.cacheMisses, c.upstreamFetches, c.upstreamFetchBytes, c.transforms,
+	} {
+		err := metricsRegistry.Register(counter)
+		if err != nil {
+			return fmt.Errorf("failed to register the cache's counters: %w", err)
+		}
+	}
+
+	return nil
+}

internal/imgcache/counters_internal_test.go

@@ -1,13 +1,14 @@
 package imgcache
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/testutil"
 )
 
-// counterValues holds the values of the counters in counters.go.
+// counterValues holds the values of a cache's counters.
 type counterValues struct {
 	hitCount           int64
 	missCount          int64
@@ -16,37 +17,30 @@ type counterValues struct {
 	transformCount     int64
 }
 
-// readCounters reads the counters in counters.go. They count for the whole
-// process, and every test that serves an image adds to them, so a test that
-// checks how much they rose does not call t.Parallel. Go runs the tests that
-// do not call it one at a time, and the tests that do only after them.
-func readCounters() counterValues {
+// readCounters reads cache's counters.
+func readCounters(cache *Cache) counterValues {
 	return counterValues{
-		hitCount:           int64(testutil.ToFloat64(cacheHits)),
-		missCount:          int64(testutil.ToFloat64(cacheMisses)),
-		upstreamFetchCount: int64(testutil.ToFloat64(upstreamFetches)),
-		upstreamFetchBytes: int64(testutil.ToFloat64(upstreamFetchBytes)),
-		transformCount:     int64(testutil.ToFloat64(transforms)),
+		hitCount:           int64(testutil.ToFloat64(cache.cacheHits)),
+		missCount:          int64(testutil.ToFloat64(cache.cacheMisses)),
+		upstreamFetchCount: int64(testutil.ToFloat64(cache.upstreamFetches)),
+		upstreamFetchBytes: int64(testutil.ToFloat64(cache.upstreamFetchBytes)),
+		transformCount:     int64(testutil.ToFloat64(cache.transforms)),
 	}
 }
 
-// minus returns how much each counter rose from before to v.
-func (v counterValues) minus(before counterValues) counterValues {
-	return counterValues{
-		hitCount:           v.hitCount - before.hitCount,
-		missCount:          v.missCount - before.missCount,
-		upstreamFetchCount: v.upstreamFetchCount - before.upstreamFetchCount,
-		upstreamFetchBytes: v.upstreamFetchBytes - before.upstreamFetchBytes,
-		transformCount:     v.transformCount - before.transformCount,
-	}
-}
-
-// TestCounters_AreOnTheRegistryMetricsServes checks that the counters in
-// counters.go are on Prometheus' default registry, which /metrics serves,
-// under the names README.md gives them.
-func TestCounters_AreOnTheRegistryMetricsServes(t *testing.T) {
+// TestNewCache_RegistersItsCountersOnItsRegistry checks that NewCache
+// registers the cache's counters on the registry it is given, under the
+// names README.md gives them.
+func TestNewCache_RegistersItsCountersOnItsRegistry(t *testing.T) {
 	t.Parallel()
 
+	registry := prometheus.NewRegistry()
+
+	_, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()}, registry)
+	if err != nil {
+		t.Fatalf("NewCache() error = %v", err)
+	}
+
 	names := []string{
 		"pixa_cache_hits_total",
 		"pixa_cache_misses_total",
@@ -55,13 +49,36 @@ func TestCounters_AreOnTheRegistryMetricsServes(t *testing.T) {
 		"pixa_transforms_total",
 	}
 
-	count, err := testutil.GatherAndCount(prometheus.DefaultGatherer, names...)
+	count, err := testutil.GatherAndCount(registry, names...)
 	if err != nil {
 		t.Fatalf("testutil.GatherAndCount() error = %v", err)
 	}
 
 	if count != len(names) {
-		t.Errorf("%d of the counters %v are on the default registry, want %d",
+		t.Errorf("%d of the counters %v are on the registry, want %d",
 			count, names, len(names))
 	}
 }
+
+// TestNewCache_FailsWhenItsCountersCannotBeRegistered makes a second cache
+// on the registry of a first. Its counters have the same names, so the
+// registry refuses them, and NewCache must return that error.
+func TestNewCache_FailsWhenItsCountersCannotBeRegistered(t *testing.T) {
+	t.Parallel()
+
+	registry := prometheus.NewRegistry()
+
+	_, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()}, registry)
+	if err != nil {
+		t.Fatalf("NewCache() of the first cache error = %v", err)
+	}
+
+	_, err = NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()}, registry)
+	t.Logf("NewCache() of the second cache error = %v", err)
+
+	var alreadyRegistered prometheus.AlreadyRegisteredError
+	if !errors.As(err, &alreadyRegistered) {
+		t.Errorf("NewCache() of the second cache error = %v, want %T",
+			err, alreadyRegistered)
+	}
+}

internal/imgcache/eviction_internal_test.go

@@ -14,6 +14,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"modernc.org/sqlite"
 	"sneak.berlin/go/pixa/internal/database"
 	"sneak.berlin/go/pixa/internal/imagerequest"
@@ -74,7 +75,7 @@ func newEvictionTestCache(t *testing.T, maxBytes int64) (*Cache, string) {
 		NegativeTTL:      5 * time.Minute,
 		MaxBytes:         maxBytes,
 		DisableDiskCache: maxBytes == 0,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
 		t.Fatalf("failed to create cache: %v", err)
 	}

internal/imgcache/negative_cache_internal_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
@@ -19,7 +20,7 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) {
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:    dir,
 		NegativeTTL: 5 * time.Minute,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -68,7 +69,7 @@ func TestNegativeCache_Expired(t *testing.T) {
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:    dir,
 		NegativeTTL: 1 * time.Millisecond, // very short TTL
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -191,7 +192,7 @@ func TestEvictionLoopRemovesExpiredNegativeCacheRows(t *testing.T) {
 				StateDir:         t.TempDir(),
 				NegativeTTL:      time.Second,
 				DisableDiskCache: tt.disableDiskCache,
-			})
+			}, prometheus.NewRegistry())
 			if err != nil {
 				t.Fatalf("failed to create cache: %v", err)
 			}
@@ -265,7 +266,8 @@ func negativeCacheRowCount(tb testing.TB, cache *Cache, sourcePath string) int {
 func TestCleanExpiredDeletesInBatches(t *testing.T) {
 	t.Parallel()
 
-	cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()})
+	cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()},
+		prometheus.NewRegistry())
 	if err != nil {
 		t.Fatalf("failed to create cache: %v", err)
 	}
@@ -319,7 +321,8 @@ func TestCleanExpiredDeletesInBatches(t *testing.T) {
 func TestCleanExpiredRemovesMoreThanOneBatch(t *testing.T) {
 	t.Parallel()
 
-	cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()})
+	cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()},
+		prometheus.NewRegistry())
 	if err != nil {
 		t.Fatalf("failed to create cache: %v", err)
 	}
@@ -349,7 +352,8 @@ func TestCleanExpiredRemovesMoreThanOneBatch(t *testing.T) {
 func TestCleanExpiredStopsOnceContextEnds(t *testing.T) {
 	t.Parallel()
 
-	cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()})
+	cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()},
+		prometheus.NewRegistry())
 	if err != nil {
 		t.Fatalf("failed to create cache: %v", err)
 	}

internal/imgcache/partial_cache_files_internal_test.go

@@ -112,12 +112,10 @@ func assertWarningNames(t *testing.T, logs, path string) {
 // pass, which must remove it with a warning naming it. It then asks for two
 // new sizes of the source: the first must fetch it again and cache it whole,
 // so that the second is made from the cache without a third fetch.
-//
-//nolint:paralleltest // it reads counters that other tests add to
 func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
-	svc, fixtures := SetupTestService(t)
+	t.Parallel()
 
-	before := readCounters()
+	svc, fixtures := SetupTestService(t)
 
 	getImage(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
 
@@ -130,7 +128,7 @@ func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
 	getImage(t, svc, photoVariant(fixtures, 40, imagerequest.FitCover))
 	getImage(t, svc, photoVariant(fixtures, 30, imagerequest.FitCover))
 
-	fetches := readCounters().minus(before).upstreamFetchCount
+	fetches := readCounters(svc.cache).upstreamFetchCount
 	t.Logf("upstream fetches = %d", fetches)
 
 	if fetches != 2 {
@@ -150,12 +148,10 @@ func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
 // It then asks for two new sizes of the source: the first must fetch it
 // again rather than fail to decode the partial file, and the second is
 // made from the cache.
-//
-//nolint:paralleltest // it reads counters that other tests add to
 func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) {
-	svc, fixtures := SetupTestService(t)
+	t.Parallel()
 
-	before := readCounters()
+	svc, fixtures := SetupTestService(t)
 
 	getImage(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
 
@@ -168,7 +164,7 @@ func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) {
 	getImage(t, svc, photoVariant(fixtures, 40, imagerequest.FitCover))
 	getImage(t, svc, photoVariant(fixtures, 30, imagerequest.FitCover))
 
-	fetches := readCounters().minus(before).upstreamFetchCount
+	fetches := readCounters(svc.cache).upstreamFetchCount
 	t.Logf("upstream fetches = %d", fetches)
 
 	if fetches != 2 {

internal/imgcache/pendingaccesstimes_internal_test.go

@@ -19,6 +19,10 @@ import (
 // for StopPendingAccessTimeWrites to return.
 const pendingAccessTimesWait = 5 * time.Second
 
+// otherWritesWait is how long a test gives other writes, once one write waits
+// for the database's connection, to start waiting for it too.
+const otherWritesWait = 100 * time.Millisecond
+
 // holdDatabase takes the one connection of cache's database, so that every
 // other query waits for it, and returns the func that frees it.
 func holdDatabase(t *testing.T, cache *Cache) func() {
@@ -98,6 +102,84 @@ func TestService_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy(t *testing.T) {
 	}
 }
 
+// TestStopPendingAccessTimeWrites_LetsTheWriteUnderWayFinish holds the
+// database's one connection while the goroutine that writes the pending
+// access times waits for it, hands over another access time, then stops that
+// goroutine. The stop must leave the write under way to finish rather than
+// cut it off, and once the connection is free it must write the access time
+// that write left.
+func TestStopPendingAccessTimeWrites_LetsTheWriteUnderWayFinish(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+	anHourAgo := time.Now().Add(-time.Hour)
+
+	for _, key := range []imagerequest.VariantKey{testVariantKeyOne, testVariantKeyTwo} {
+		storeEvictionTestVariant(t, cache, key, []byte("variant bytes"))
+		setVariantLastAccessed(t, cache, key, anHourAgo)
+	}
+
+	cache.StartPendingAccessTimeWrites()
+	defer func() { _ = cache.StopPendingAccessTimeWrites(t.Context()) }()
+
+	releaseDatabase := holdDatabase(t, cache)
+	defer releaseDatabase()
+
+	waitsBefore := cache.db.Stats().WaitCount
+
+	cache.touchVariant(testVariantKeyOne)
+
+	// The goroutine's write waits for the connection.
+	waitForConnectionWaits(t, cache.db, waitsBefore+1)
+
+	// Handed over after that write took the pending access times
+	cache.touchVariant(testVariantKeyTwo)
+
+	// The goroutine is told to finish here as well as by the stop, so that it
+	// is told before its write under way ends, however late the stop runs.
+	// The access time above then reaches the database only through the
+	// stop's own write.
+	cache.pendingAccessTimesCancel()
+
+	stopped := make(chan error, 1)
+
+	go func() {
+		stopped <- cache.StopPendingAccessTimeWrites(t.Context())
+	}()
+
+	// Had the stop cut off the write under way, its own write would wait for
+	// the connection too.
+	time.Sleep(otherWritesWait)
+
+	waits := cache.db.Stats().WaitCount - waitsBefore
+	t.Logf("%d writes waited for the database", waits)
+
+	if waits > 1 {
+		t.Errorf("%d writes waited for the database, want 1: "+
+			"the stop cut off the write under way", waits)
+	}
+
+	releaseDatabase()
+
+	select {
+	case err := <-stopped:
+		if err != nil {
+			t.Fatalf("StopPendingAccessTimeWrites() error = %v", err)
+		}
+	case <-time.After(pendingAccessTimesWait):
+		t.Fatal("StopPendingAccessTimeWrites() did not return once the database was free")
+	}
+
+	for _, key := range []imagerequest.VariantKey{testVariantKeyOne, testVariantKeyTwo} {
+		if countRows(t, cache, `
+			SELECT COUNT(*) FROM variant_content
+			WHERE cache_key = ? AND last_accessed_at > ?
+		`, string(key), anHourAgo.UTC().Format(sqliteTimestampFormat)) != 1 {
+			t.Errorf("the access time of variant %s was not written", key)
+		}
+	}
+}
+
 // TestStopPendingAccessTimeWrites_ReturnsWhenItsContextEnds holds the
 // database's one connection while the goroutine that writes the pending
 // access times waits for it, then stops that goroutine with a context that

internal/imgcache/service_internal_test.go

@@ -558,14 +558,12 @@ func TestService_Get_Caching(t *testing.T) {
 // Left at the present: the files' status change and birth times, which a
 // program cannot set, and the times of the directories holding the files,
 // which other entries share.
-//
-//nolint:paralleltest // it reads counters that other tests add to
 func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) {
+	t.Parallel()
+
 	svc, fixtures := SetupTestService(t)
 	variant := photoVariant(fixtures, 85, imagerequest.FitCover)
 
-	before := readCounters()
-
 	resp, err := svc.Get(t.Context(), variant)
 	if err != nil {
 		t.Fatalf("Get() first request error = %v", err)
@@ -597,7 +595,7 @@ func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) {
 
 	_ = resp.Content.Close()
 
-	fetches := readCounters().minus(before).upstreamFetchCount
+	fetches := readCounters(svc.cache).upstreamFetchCount
 	if fetches != 1 {
 		t.Errorf("upstream fetches = %d, want 1", fetches)
 	}

internal/imgcache/stats_internal_test.go

@@ -14,11 +14,10 @@ import (
 
 // TestService_Get_CountsStats walks Get through a miss that fetches the
 // source, a hit, a miss that reuses the cached source, and two misses whose
-// source cannot be used, checking how much each request added to every
-// counter in counters.go.
-//
-//nolint:paralleltest // it reads counters that other tests add to
+// source cannot be used, checking every counter of the cache after each.
 func TestService_Get_CountsStats(t *testing.T) {
+	t.Parallel()
+
 	svc, fixtures := SetupTestService(t)
 
 	// NewTestFS builds the same files the test service's fetcher serves.
@@ -36,8 +35,7 @@ func TestService_Get_CountsStats(t *testing.T) {
 
 	photoBytes, fakeBytes := int64(len(photo)), int64(len(fake))
 
-	// want is what the request adds to hits, misses, upstream fetches,
-	// upstream bytes and transforms.
+	// want is hits, misses, upstream fetches, upstream bytes, transforms.
 	steps := []struct {
 		name    string
 		path    string
@@ -48,18 +46,16 @@ func TestService_Get_CountsStats(t *testing.T) {
 		{"miss that fetches the source", testPathPhoto, 50, false,
 			counterValues{0, 1, 1, photoBytes, 1}},
 		{"hit", testPathPhoto, 50, false,
-			counterValues{1, 0, 0, 0, 0}},
+			counterValues{1, 1, 1, photoBytes, 1}},
 		{"miss that reuses the cached source", testPathPhoto, 25, false,
-			counterValues{0, 1, 0, 0, 1}},
+			counterValues{1, 2, 1, photoBytes, 2}},
 		{"miss whose source fails the magic byte check", "/images/fake.jpg", 50, true,
-			counterValues{0, 1, 1, fakeBytes, 0}},
+			counterValues{1, 3, 2, photoBytes + fakeBytes, 2}},
 		{"miss whose source is not found", "/images/nonexistent.jpg", 50, true,
-			counterValues{0, 1, 0, 0, 0}},
+			counterValues{1, 4, 2, photoBytes + fakeBytes, 2}},
 	}
 
 	for _, step := range steps {
-		before := readCounters()
-
 		resp, err := svc.Get(t.Context(), &imagerequest.Request{
 			Source: imagerequest.Source{
 				Host: fixtures.GoodHost,
@@ -80,10 +76,9 @@ func TestService_Get_CountsStats(t *testing.T) {
 			_ = resp.Content.Close()
 		}
 
-		got := readCounters().minus(before)
+		got := readCounters(svc.cache)
 		if got != step.want {
-			t.Fatalf("the %s added %+v to the counters, want %+v",
-				step.name, got, step.want)
+			t.Fatalf("after the %s: counters = %+v, want %+v", step.name, got, step.want)
 		}
 	}
 }
@@ -119,17 +114,16 @@ func (u *fakeUpstream) Read(p []byte) (int, error) {
 	return 0, u.err
 }
 
-// TestService_Get_CountsInterruptedMisses checks how much a miss whose
-// request context ends during or after the upstream fetch, and a miss whose
-// upstream body is over the size limit, add to every counter in counters.go.
-//
-//nolint:paralleltest // it reads counters that other tests add to
+// TestService_Get_CountsInterruptedMisses checks every counter of the cache
+// after a miss whose request context ends during or after the upstream
+// fetch, and after a miss whose upstream body is over the size limit.
 func TestService_Get_CountsInterruptedMisses(t *testing.T) {
+	t.Parallel()
+
 	photo := generateTestJPEG(t, 100, 100, color.RGBA{255, 0, 0, 255})
 	half := len(photo) / 2
 
-	// want is what the request adds to hits, misses, upstream fetches,
-	// upstream bytes and transforms.
+	// want is hits, misses, upstream fetches, upstream bytes, transforms.
 	tests := []struct {
 		name    string
 		served  int   // bytes of the photo the upstream body serves
@@ -149,6 +143,8 @@ func TestService_Get_CountsInterruptedMisses(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
 			svc, fixtures := SetupTestService(t)
 
 			ctx, cancel := context.WithCancel(t.Context())
@@ -164,8 +160,6 @@ func TestService_Get_CountsInterruptedMisses(t *testing.T) {
 
 			svc.fetcher = upstream
 
-			before := readCounters()
-
 			resp, err := svc.Get(ctx, &imagerequest.Request{
 				Source: imagerequest.Source{
 					Host: fixtures.GoodHost,
@@ -188,20 +182,20 @@ func TestService_Get_CountsInterruptedMisses(t *testing.T) {
 				_ = resp.Content.Close()
 			}
 
-			got := readCounters().minus(before)
+			got := readCounters(svc.cache)
 			if got != tc.want {
-				t.Errorf("the miss added %+v to the counters, want %+v", got, tc.want)
+				t.Errorf("counters = %+v, want %+v", got, tc.want)
 			}
 		})
 	}
 }
 
-// TestService_Get_CountsHitAfterRequestEnds checks every counter in
-// counters.go after a hit served with a request context that has already
-// ended: only the hit count moves.
-//
-//nolint:paralleltest // it reads counters that other tests add to
+// TestService_Get_CountsHitAfterRequestEnds checks every counter of the
+// cache after a hit served with a request context that has already ended:
+// only the hit count moves.
 func TestService_Get_CountsHitAfterRequestEnds(t *testing.T) {
+	t.Parallel()
+
 	svc, fixtures := SetupTestService(t)
 
 	req := &imagerequest.Request{
@@ -225,7 +219,7 @@ func TestService_Get_CountsHitAfterRequestEnds(t *testing.T) {
 
 	_ = resp.Content.Close()
 
-	want := readCounters()
+	want := readCounters(svc.cache)
 	want.hitCount++
 
 	ctx, cancel := context.WithCancel(t.Context())
@@ -242,7 +236,7 @@ func TestService_Get_CountsHitAfterRequestEnds(t *testing.T) {
 		t.Fatalf("CacheStatus = %v, want %v", resp.CacheStatus, CacheHit)
 	}
 
-	got := readCounters()
+	got := readCounters(svc.cache)
 	if got != want {
 		t.Errorf("counters = %+v, want %+v", got, want)
 	}

internal/imgcache/testutil_internal_test.go

@@ -14,6 +14,7 @@ import (
 	"testing/fstest"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"sneak.berlin/go/pixa/internal/database"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 )
@@ -184,7 +185,7 @@ func SetupTestService(
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:    tmpDir,
 		NegativeTTL: 5 * time.Minute,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
 		t.Fatalf("failed to create cache: %v", err)
 	}

internal/server/image_proxy_internal_test.go

@@ -19,6 +19,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/fx"
 	"go.uber.org/fx/fxtest"
 
@@ -170,6 +171,7 @@ func startImageProxy(
 			handlers.New,
 			middleware.New,
 			func() httpfetcher.Fetcher { return fetcher },
+			func() prometheus.Registerer { return prometheus.NewRegistry() },
 		),
 		fx.Populate(&h, &mw, &db),
 	)
@@ -223,6 +225,7 @@ func startImageProxyWithBlockedNetworks(
 			healthcheck.New,
 			handlers.New,
 			middleware.New,
+			func() prometheus.Registerer { return prometheus.NewRegistry() },
 		),
 		fx.Populate(&h, &mw),
 	)

internal/server/login_rate_limit_internal_test.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/fx/fxtest"
 
 	"sneak.berlin/go/pixa/internal/config"
@@ -81,6 +82,7 @@ func newTestServer(t *testing.T) *Server {
 
 	h, err := handlers.New(lc, handlers.Params{
 		Logger: log, Healthcheck: hc, Database: db, Config: cfg,
+		MetricsRegistry: prometheus.NewRegistry(),
 	})
 	if err != nil {
 		t.Fatalf("handlers.New() error = %v", err)

internal/server/logout_internal_test.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/fx"
 	"go.uber.org/fx/fxtest"
 
@@ -46,6 +47,7 @@ func startPixa(t *testing.T, cfg *config.Config) *Server {
 			healthcheck.New,
 			handlers.New,
 			middleware.New,
+			func() prometheus.Registerer { return prometheus.NewRegistry() },
 		),
 		fx.Populate(&h, &mw),
 	)