diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 6a7aab4..e7d2e9e 100644 --- a/internal/handler/handler.go +++ b/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) diff --git a/internal/handler/handler_test.go b/internal/handler/handler_test.go index d52e7b6..cbf3aec 100644 --- a/internal/handler/handler_test.go +++ b/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) { diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index f23a5a9..df47222 100644 --- a/internal/metrics/metrics.go +++ b/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. diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 445a715..a445467 100644 --- a/internal/metrics/metrics_test.go +++ b/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{