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

internal/imgcache/stats_internal_test.go

@@ -3,245 +3,18 @@ package imgcache
 import (
 	"bytes"
 	"context"
-	"database/sql"
 	"image/color"
 	"io"
 	"io/fs"
-	"log/slog"
-	"math"
-	"strings"
 	"testing"
-	"time"
 
-	"sneak.berlin/go/pixa/internal/database"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
-func setupStatsTestDB(t *testing.T) *sql.DB {
-	t.Helper()
-
-	db, err := sql.Open("sqlite", ":memory:")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	err = database.ApplyMigrations(context.Background(), db, nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	t.Cleanup(func() { _ = db.Close() })
-
-	return db
-}
-
-func TestStats_HitRateIsRatio(t *testing.T) {
-	t.Parallel()
-
-	db := setupStatsTestDB(t)
-	dir := t.TempDir()
-
-	cache, err := NewCache(db, CacheConfig{
-		StateDir:    dir,
-		NegativeTTL: 5 * time.Minute,
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	ctx := context.Background()
-
-	// Set some hit/miss counts and a transform_count
-	_, err = db.ExecContext(ctx, `
-		UPDATE cache_stats
-		SET hit_count = 75, miss_count = 25, transform_count = 9999
-		WHERE id = 1
-	`)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	stats, err := cache.Stats(ctx)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if stats.HitCount != 75 {
-		t.Errorf("HitCount = %d, want 75", stats.HitCount)
-	}
-
-	if stats.MissCount != 25 {
-		t.Errorf("MissCount = %d, want 25", stats.MissCount)
-	}
-
-	// HitRate should be 0.75, NOT 9999 (transform_count)
-	expectedRate := 0.75
-	if math.Abs(stats.HitRate-expectedRate) > 0.001 {
-		t.Errorf("HitRate = %f, want %f (was it scanning transform_count?)",
-			stats.HitRate, expectedRate)
-	}
-}
-
-func TestStats_ZeroCounts(t *testing.T) {
-	t.Parallel()
-
-	db := setupStatsTestDB(t)
-	dir := t.TempDir()
-
-	cache, err := NewCache(db, CacheConfig{
-		StateDir:    dir,
-		NegativeTTL: 5 * time.Minute,
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	stats, err := cache.Stats(context.Background())
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// With zero hits and misses, HitRate should be 0, not some garbage value
-	if stats.HitRate != 0.0 {
-		t.Errorf("HitRate = %f, want 0.0 for zero counts", stats.HitRate)
-	}
-}
-
-// TestStats_LogsFailedCountQueries verifies that a failed item count query
-// and a failed size query are each logged at warn and Stats still succeeds.
-func TestStats_LogsFailedCountQueries(t *testing.T) {
-	t.Parallel()
-
-	db := setupStatsTestDB(t)
-
-	var logBuf bytes.Buffer
-
-	cache, err := NewCache(db, CacheConfig{
-		StateDir:    t.TempDir(),
-		NegativeTTL: 5 * time.Minute,
-		Logger:      slog.New(slog.NewJSONHandler(&logBuf, nil)),
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	_, err = db.ExecContext(t.Context(),
-		`DROP TABLE source_content; DROP TABLE variant_content`)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	_, err = cache.Stats(t.Context())
-	if err != nil {
-		t.Fatalf("Stats() error = %v, want nil", err)
-	}
-
-	for _, msg := range []string{
-		"failed to count cache items for stats",
-		"failed to sum cache size for stats",
-	} {
-		want := `"level":"WARN","msg":"` + msg + `"`
-		if !strings.Contains(logBuf.String(), want) {
-			t.Errorf("log missing %s; got %q", want, logBuf.String())
-		}
-	}
-}
-
-// TestStats_TotalsCountSourcesAndVariants verifies that TotalItems and
-// TotalSizeBytes cover the stored source images and processed variants.
-func TestStats_TotalsCountSourcesAndVariants(t *testing.T) {
-	t.Parallel()
-
-	cache, _ := newEvictionTestCache(t, 1<<30)
-
-	storeEvictionTestSource(t, cache, testHostCDN, testPathCat,
-		bytes.Repeat([]byte{0xAA}, 1000))
-	storeEvictionTestVariant(t, cache, testVariantKeyOne,
-		bytes.Repeat([]byte{0xAB}, 500))
-	storeEvictionTestVariant(t, cache, testVariantKeyTwo,
-		bytes.Repeat([]byte{0xAC}, 250))
-
-	stats, err := cache.Stats(t.Context())
-	if err != nil {
-		t.Fatalf("Stats() error = %v", err)
-	}
-
-	if stats.TotalItems != 3 {
-		t.Errorf("TotalItems = %d, want 3 (1 source, 2 variants)", stats.TotalItems)
-	}
-
-	if stats.TotalSizeBytes != 1750 {
-		t.Errorf("TotalSizeBytes = %d, want 1750 (1000+500+250)",
-			stats.TotalSizeBytes)
-	}
-}
-
-// TestStats_DisabledCacheReportsNoItems verifies that a disabled disk cache
-// reports no items and no size, even when its database still holds the
-// rows of an earlier run with the disk cache enabled.
-func TestStats_DisabledCacheReportsNoItems(t *testing.T) {
-	t.Parallel()
-
-	enabled, _ := newEvictionTestCache(t, 1<<30)
-
-	storeEvictionTestSource(t, enabled, testHostCDN, testPathCat,
-		bytes.Repeat([]byte{0xAA}, 1000))
-	storeEvictionTestVariant(t, enabled, testVariantKeyOne,
-		bytes.Repeat([]byte{0xAB}, 500))
-
-	disabled, err := NewCache(enabled.db, CacheConfig{
-		StateDir:         t.TempDir(),
-		NegativeTTL:      5 * time.Minute,
-		DisableDiskCache: true,
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	stats, err := disabled.Stats(t.Context())
-	if err != nil {
-		t.Fatalf("Stats() error = %v", err)
-	}
-
-	if stats.TotalItems != 0 || stats.TotalSizeBytes != 0 {
-		t.Errorf("TotalItems = %d, TotalSizeBytes = %d, want 0 and 0",
-			stats.TotalItems, stats.TotalSizeBytes)
-	}
-}
-
-// cacheStatsCounters holds the counters of the cache_stats row, in column
-// order.
-type cacheStatsCounters struct {
-	hitCount           int64
-	missCount          int64
-	upstreamFetchCount int64
-	upstreamFetchBytes int64
-	transformCount     int64
-}
-
-// readCacheStatsCounters reads the counters of the cache_stats row.
-func readCacheStatsCounters(t *testing.T, cache *Cache) cacheStatsCounters {
-	t.Helper()
-
-	var got cacheStatsCounters
-
-	err := cache.db.QueryRowContext(t.Context(), `
-		SELECT hit_count, miss_count, upstream_fetch_count,
-		       upstream_fetch_bytes, transform_count
-		FROM cache_stats WHERE id = 1
-	`).Scan(&got.hitCount, &got.missCount, &got.upstreamFetchCount,
-		&got.upstreamFetchBytes, &got.transformCount)
-	if err != nil {
-		t.Fatalf("failed to read cache_stats: %v", err)
-	}
-
-	return got
-}
-
 // 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 every cache_stats counter after each.
+// source cannot be used, checking every counter of the cache after each.
 func TestService_Get_CountsStats(t *testing.T) {
 	t.Parallel()
 
@@ -268,18 +41,18 @@ func TestService_Get_CountsStats(t *testing.T) {
 		path    string
 		size    int
 		wantErr bool
-		want    cacheStatsCounters
+		want    counterValues
 	}{
 		{"miss that fetches the source", testPathPhoto, 50, false,
-			cacheStatsCounters{0, 1, 1, photoBytes, 1}},
+			counterValues{0, 1, 1, photoBytes, 1}},
 		{"hit", testPathPhoto, 50, false,
-			cacheStatsCounters{1, 1, 1, photoBytes, 1}},
+			counterValues{1, 1, 1, photoBytes, 1}},
 		{"miss that reuses the cached source", testPathPhoto, 25, false,
-			cacheStatsCounters{1, 2, 1, photoBytes, 2}},
+			counterValues{1, 2, 1, photoBytes, 2}},
 		{"miss whose source fails the magic byte check", "/images/fake.jpg", 50, true,
-			cacheStatsCounters{1, 3, 2, photoBytes + fakeBytes, 2}},
+			counterValues{1, 3, 2, photoBytes + fakeBytes, 2}},
 		{"miss whose source is not found", "/images/nonexistent.jpg", 50, true,
-			cacheStatsCounters{1, 4, 2, photoBytes + fakeBytes, 2}},
+			counterValues{1, 4, 2, photoBytes + fakeBytes, 2}},
 	}
 
 	for _, step := range steps {
@@ -303,9 +76,7 @@ func TestService_Get_CountsStats(t *testing.T) {
 			_ = resp.Content.Close()
 		}
 
-		waitForPendingCountsWritten(t, svc.cache)
-
-		got := readCacheStatsCounters(t, svc.cache)
+		got := readCounters(svc.cache)
 		if got != step.want {
 			t.Fatalf("after the %s: counters = %+v, want %+v", step.name, got, step.want)
 		}
@@ -343,7 +114,7 @@ func (u *fakeUpstream) Read(p []byte) (int, error) {
 	return 0, u.err
 }
 
-// TestService_Get_CountsInterruptedMisses checks every cache_stats counter
+// 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) {
@@ -359,15 +130,15 @@ func TestService_Get_CountsInterruptedMisses(t *testing.T) {
 		cancel  bool  // whether the body then ends the request context
 		readErr error // what the body then returns
 		wantErr bool
-		want    cacheStatsCounters
+		want    counterValues
 	}{
 		{"request context ends during the fetch", half, true, context.Canceled, true,
-			cacheStatsCounters{0, 1, 1, int64(half), 0}},
+			counterValues{0, 1, 1, int64(half), 0}},
 		{"request context ends after the fetch", len(photo), true, io.EOF, false,
-			cacheStatsCounters{0, 1, 1, int64(len(photo)), 1}},
+			counterValues{0, 1, 1, int64(len(photo)), 1}},
 		{"upstream body over the size limit", half, false,
 			httpfetcher.ErrResponseTooLarge, true,
-			cacheStatsCounters{0, 1, 1, int64(half), 0}},
+			counterValues{0, 1, 1, int64(half), 0}},
 	}
 
 	for _, tc := range tests {
@@ -411,9 +182,7 @@ func TestService_Get_CountsInterruptedMisses(t *testing.T) {
 				_ = resp.Content.Close()
 			}
 
-			waitForPendingCountsWritten(t, svc.cache)
-
-			got := readCacheStatsCounters(t, svc.cache)
+			got := readCounters(svc.cache)
 			if got != tc.want {
 				t.Errorf("counters = %+v, want %+v", got, tc.want)
 			}
@@ -421,9 +190,9 @@ func TestService_Get_CountsInterruptedMisses(t *testing.T) {
 	}
 }
 
-// TestService_Get_CountsHitAfterRequestEnds checks every cache_stats counter
-// after a hit served with a request context that has already ended: only
-// the hit count moves.
+// 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()
 
@@ -450,9 +219,7 @@ func TestService_Get_CountsHitAfterRequestEnds(t *testing.T) {
 
 	_ = resp.Content.Close()
 
-	waitForPendingCountsWritten(t, svc.cache)
-
-	want := readCacheStatsCounters(t, svc.cache)
+	want := readCounters(svc.cache)
 	want.hitCount++
 
 	ctx, cancel := context.WithCancel(t.Context())
@@ -469,9 +236,7 @@ func TestService_Get_CountsHitAfterRequestEnds(t *testing.T) {
 		t.Fatalf("CacheStatus = %v, want %v", resp.CacheStatus, CacheHit)
 	}
 
-	waitForPendingCountsWritten(t, svc.cache)
-
-	got := readCacheStatsCounters(t, svc.cache)
+	got := readCounters(svc.cache)
 	if got != want {
 		t.Errorf("counters = %+v, want %+v", got, want)
 	}