Correct cache metrics

internal/handler/handler.go

@@ -151,6 +151,7 @@ func (p *Proxy) GetOrFetchArtifact(ctx context.Context, ecosystem, name, version
 	} else if cached != nil {
 		return cached, nil
 	}
+	metrics.RecordCacheMiss(ecosystem)
 
 	pkgPURL := purl.MakePURLString(ecosystem, name, "")
 	versionPURL := purl.MakePURLString(ecosystem, name, version)
@@ -241,13 +242,10 @@ func rewriteSignedURLHost(signed, baseURL string) string {
 
 func (p *Proxy) recordCacheHit(ecosystem, versionPURL, filename string) {
 	_ = p.DB.RecordArtifactHit(versionPURL, filename)
-	metrics.RecordCacheHit(purl.NormalizeEcosystem(ecosystem))
+	metrics.RecordCacheHit(ecosystem)
 }
 
 func (p *Proxy) fetchAndCache(ctx context.Context, ecosystem, name, version, filename, pkgPURL, versionPURL string) (*CacheResult, error) {
-	// Record cache miss
-	metrics.RecordCacheMiss(ecosystem)
-
 	// Resolve download URL
 	info, err := p.Resolver.Resolve(ctx, ecosystem, name, version)
 	if err != nil {
@@ -521,12 +519,14 @@ func (p *Proxy) FetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u
 					if entry.ContentType.Valid {
 						ct = entry.ContentType.String
 					}
+					metrics.RecordCacheHit(ecosystem)
 					return data, ct, nil
 				}
 			}
 			// Cache file missing/unreadable, fall through to upstream
 		}
 	}
+	p.recordMetadataCacheMiss(ecosystem)
 
 	accept := contentTypeJSON
 	if len(acceptHeaders) > 0 && acceptHeaders[0] != "" {
@@ -574,6 +574,12 @@ func (p *Proxy) FetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u
 	return data, ct, nil
 }
 
+func (p *Proxy) recordMetadataCacheMiss(ecosystem string) {
+	if p.CacheMetadata {
+		metrics.RecordCacheMiss(ecosystem)
+	}
+}
+
 // fetchUpstreamMetadata fetches metadata from upstream, using ETag for conditional revalidation.
 // Returns the body, content type, ETag, upstream Last-Modified time, and any error.
 func (p *Proxy) fetchUpstreamMetadata(ctx context.Context, upstreamURL string, entry *database.MetadataCacheEntry, accept string) ([]byte, string, string, time.Time, error) {
@@ -824,6 +830,7 @@ func (p *Proxy) GetOrFetchArtifactFromURLWithHeaders(ctx context.Context, ecosys
 	} else if cached != nil {
 		return cached, nil
 	}
+	metrics.RecordCacheMiss(ecosystem)
 
 	pkgPURL := purl.MakePURLString(ecosystem, name, "")
 	versionPURL := purl.MakePURLString(ecosystem, name, version)

internal/handler/handler_test.go

@@ -15,9 +15,11 @@ import (
 
 	"github.com/git-pkgs/proxy/internal/config"
 	"github.com/git-pkgs/proxy/internal/database"
+	"github.com/git-pkgs/proxy/internal/metrics"
 	"github.com/git-pkgs/proxy/internal/storage"
 	"github.com/git-pkgs/purl"
 	"github.com/git-pkgs/registries/fetch"
+	"github.com/prometheus/client_golang/prometheus/testutil"
 )
 
 // mockStorage implements storage.Storage for testing.
@@ -273,6 +275,7 @@ func TestGetOrFetchArtifact_CacheHit(t *testing.T) {
 
 func TestGetOrFetchArtifact_CacheMiss_NoPackage(t *testing.T) {
 	proxy, _, _, fetcher := setupTestProxy(t)
+	missesBefore := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues("npm"))
 
 	// The resolver will fail because "nonexistent" isn't a real package,
 	// but we're testing that it tries to fetch (doesn't return from cache).
@@ -282,6 +285,10 @@ func TestGetOrFetchArtifact_CacheMiss_NoPackage(t *testing.T) {
 	if err == nil {
 		t.Fatal("expected error for uncached package")
 	}
+	missesAfter := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues("npm"))
+	if diff := missesAfter - missesBefore; diff != 1 {
+		t.Errorf("cache misses delta = %.0f, want 1", diff)
+	}
 }
 
 func TestGetOrFetchArtifactFromURL_CacheMiss_StorageMissing(t *testing.T) {
@@ -540,6 +547,7 @@ func TestServeArtifact_Stream(t *testing.T) {
 func TestGetOrFetchArtifactFromURL_CacheHit(t *testing.T) {
 	proxy, db, store, fetcher := setupTestProxy(t)
 	seedPackage(t, db, store, "pypi", "requests", "2.28.0", "requests-2.28.0.tar.gz", "pypi content")
+	missesBefore := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues("pypi"))
 
 	result, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "pypi", "requests", "2.28.0", "requests-2.28.0.tar.gz", "https://pypi.org/files/requests-2.28.0.tar.gz")
 	if err != nil {
@@ -553,10 +561,15 @@ func TestGetOrFetchArtifactFromURL_CacheHit(t *testing.T) {
 	if fetcher.fetchCalled {
 		t.Error("fetcher should not be called on cache hit")
 	}
+	missesAfter := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues("pypi"))
+	if diff := missesAfter - missesBefore; diff != 0 {
+		t.Errorf("cache misses delta = %.0f, want 0", diff)
+	}
 }
 
 func TestGetOrFetchArtifactFromURL_CacheMiss(t *testing.T) {
 	proxy, _, store, fetcher := setupTestProxy(t)
+	missesBefore := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues("pypi"))
 
 	fetcher.artifact = &fetch.Artifact{
 		Body:        io.NopCloser(strings.NewReader("fetched content")),
@@ -589,6 +602,10 @@ func TestGetOrFetchArtifactFromURL_CacheMiss(t *testing.T) {
 	if _, ok := store.files[storagePath]; !ok {
 		t.Error("artifact was not stored in storage")
 	}
+	missesAfter := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues("pypi"))
+	if diff := missesAfter - missesBefore; diff != 1 {
+		t.Errorf("cache misses delta = %.0f, want 1", diff)
+	}
 }
 
 func TestGetOrFetchArtifactFromURL_FetchError(t *testing.T) {
@@ -878,6 +895,8 @@ func TestProxyCached_NoValidators_OmitsHeaders(t *testing.T) {
 }
 
 func TestFetchOrCacheMetadata_TTL_ServesFreshFromCache(t *testing.T) {
+	hitsBefore := testutil.ToFloat64(metrics.CacheHits.WithLabelValues("test"))
+	missesBefore := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues("test"))
 	upstreamHits := 0
 	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		upstreamHits++
@@ -904,6 +923,12 @@ func TestFetchOrCacheMetadata_TTL_ServesFreshFromCache(t *testing.T) {
 	if upstreamHits != 1 {
 		t.Fatalf("expected 1 upstream hit, got %d", upstreamHits)
 	}
+	if diff := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues("test")) - missesBefore; diff != 1 {
+		t.Errorf("cache misses delta after first request = %.0f, want 1", diff)
+	}
+	if diff := testutil.ToFloat64(metrics.CacheHits.WithLabelValues("test")) - hitsBefore; diff != 0 {
+		t.Errorf("cache hits delta after first request = %.0f, want 0", diff)
+	}
 
 	// Second request within TTL should serve from cache without hitting upstream
 	body, _, err = proxy.FetchOrCacheMetadata(ctx, "test", "ttl-pkg", upstream.URL+"/pkg")
@@ -916,9 +941,16 @@ func TestFetchOrCacheMetadata_TTL_ServesFreshFromCache(t *testing.T) {
 	if upstreamHits != 1 {
 		t.Errorf("expected upstream to still be hit only once, got %d", upstreamHits)
 	}
+	if diff := testutil.ToFloat64(metrics.CacheHits.WithLabelValues("test")) - hitsBefore; diff != 1 {
+		t.Errorf("cache hits delta after second request = %.0f, want 1", diff)
+	}
+	if diff := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues("test")) - missesBefore; diff != 1 {
+		t.Errorf("cache misses delta after second request = %.0f, want 1", diff)
+	}
 }
 
 func TestFetchOrCacheMetadata_TTL_Zero_AlwaysRevalidates(t *testing.T) {
+	missesBefore := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues("test"))
 	upstreamHits := 0
 	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		upstreamHits++
@@ -947,6 +979,40 @@ func TestFetchOrCacheMetadata_TTL_Zero_AlwaysRevalidates(t *testing.T) {
 	if upstreamHits != 2 {
 		t.Errorf("expected 2 upstream hits with TTL=0, got %d", upstreamHits)
 	}
+	missesAfter := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues("test"))
+	if diff := missesAfter - missesBefore; diff != 2 {
+		t.Errorf("cache misses delta = %.0f, want 2", diff)
+	}
+}
+
+func TestFetchOrCacheMetadata_CacheDisabledDoesNotRecordMetrics(t *testing.T) {
+	const ecosystem = "metadata-disabled"
+
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"v":1}`))
+	}))
+	t.Cleanup(upstream.Close)
+
+	proxy, _, _, _ := setupTestProxy(t)
+	proxy.HTTPClient = upstream.Client()
+
+	hitsBefore := testutil.ToFloat64(metrics.CacheHits.WithLabelValues(ecosystem))
+	missesBefore := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues(ecosystem))
+
+	_, _, err := proxy.FetchOrCacheMetadata(context.Background(), ecosystem, "pkg", upstream.URL+"/pkg")
+	if err != nil {
+		t.Fatalf("fetch metadata: %v", err)
+	}
+
+	hitsAfter := testutil.ToFloat64(metrics.CacheHits.WithLabelValues(ecosystem))
+	missesAfter := testutil.ToFloat64(metrics.CacheMisses.WithLabelValues(ecosystem))
+	if diff := hitsAfter - hitsBefore; diff != 0 {
+		t.Errorf("cache hits delta = %.0f, want 0", diff)
+	}
+	if diff := missesAfter - missesBefore; diff != 0 {
+		t.Errorf("cache misses delta = %.0f, want 0", diff)
+	}
 }
 
 func TestProxyCached_StaleWarningHeader(t *testing.T) {

internal/metrics/metrics.go

@@ -6,6 +6,7 @@ import (
 	"strconv"
 	"time"
 
+	"github.com/git-pkgs/purl"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 )
@@ -173,12 +174,12 @@ func RecordRequest(ecosystem string, status int, duration time.Duration) {
 
 // RecordCacheHit increments cache hit counter.
 func RecordCacheHit(ecosystem string) {
-	CacheHits.WithLabelValues(ecosystem).Inc()
+	CacheHits.WithLabelValues(purl.NormalizeEcosystem(ecosystem)).Inc()
 }
 
 // RecordCacheMiss increments cache miss counter.
 func RecordCacheMiss(ecosystem string) {
-	CacheMisses.WithLabelValues(ecosystem).Inc()
+	CacheMisses.WithLabelValues(purl.NormalizeEcosystem(ecosystem)).Inc()
 }
 
 // RecordUpstreamFetch tracks upstream fetch duration.

internal/metrics/metrics_test.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/testutil"
 	dto "github.com/prometheus/client_model/go"
 )
 
@@ -191,22 +192,45 @@ func TestMetricsEndpointOutput(t *testing.T) {
 
 func TestMetricsLabeling(t *testing.T) {
 	// Test that different ecosystems are properly labeled
-	ecosystems := []string{"npm", "pypi", "cargo", "gem"}
+	ecosystems := []struct {
+		input string
+		label string
+	}{
+		{input: "npm", label: "npm"},
+		{input: "pypi", label: "pypi"},
+		{input: "cargo", label: "cargo"},
+		{input: "gem", label: "rubygems"},
+	}
 
 	for _, eco := range ecosystems {
-		RecordRequest(eco, 200, 10*time.Millisecond)
-		RecordCacheHit(eco)
+		RecordRequest(eco.input, 200, 10*time.Millisecond)
+		RecordCacheHit(eco.input)
 	}
 
 	// Verify each ecosystem has metrics
 	for _, eco := range ecosystems {
-		val := getMetricValue(t, CacheHits, eco)
+		val := getMetricValue(t, CacheHits, eco.label)
 		if val == 0 {
-			t.Errorf("no cache hits recorded for %s", eco)
+			t.Errorf("no cache hits recorded for %s", eco.label)
 		}
 	}
 }
 
+func TestCacheMetricLabelsAreNormalized(t *testing.T) {
+	rubyHitsBefore := testutil.ToFloat64(CacheHits.WithLabelValues("rubygems"))
+	composerMissesBefore := testutil.ToFloat64(CacheMisses.WithLabelValues("packagist"))
+
+	RecordCacheHit("gem")
+	RecordCacheMiss("composer")
+
+	if diff := testutil.ToFloat64(CacheHits.WithLabelValues("rubygems")) - rubyHitsBefore; diff != 1 {
+		t.Errorf("rubygems cache hits delta = %.0f, want 1", diff)
+	}
+	if diff := testutil.ToFloat64(CacheMisses.WithLabelValues("packagist")) - composerMissesBefore; diff != 1 {
+		t.Errorf("packagist cache misses delta = %.0f, want 1", diff)
+	}
+}
+
 func TestMetricNames(t *testing.T) {
 	// Verify metric names follow Prometheus naming conventions
 	expectedMetrics := []string{