diff --git a/README.md b/README.md index e41653aa..a723f2d0 100644 --- a/README.md +++ b/README.md @@ -368,7 +368,7 @@ All carry `strategy` and `registry`. Registry values come from configured host/A The HTTP server metrics of these handlers carry `cachew.cache.result` (`hit`, `miss`, `coalesced` or `error`). Hits also carry `cachew.cache.source`, the backend that served them (`disk`, `s3`, `memory`, `remote`, or `other`), after stale and conditional probes. -Origin metrics count HTTP exchanges that reach an origin through `http.DefaultTransport`, including redirect hops and transport failures. Requests rejected during an upstream 429 cooldown are not counted. `status_code=none` means no HTTP response; `error_type` is `none`, `transport`, `timeout`, or `canceled`. Duration ends at response headers or transport failure. Registry identifies the logical artifact provider, including its redirects, rather than each physical destination. These metrics do not count every low-level connection retry, Git transport, Go-module fetch, GitHub metadata lookup, OCI tag-list or referrer pass-through, or S3 operation. Internal Hermit forwarding to another Cachew route is excluded from origin/fill counts; the receiving handler measures its own work. +Origin metrics count HTTP exchanges that reach an origin through `http.DefaultTransport`, including redirect hops and transport failures. Requests rejected during an upstream 429 cooldown are not counted. `status_code=none` means no HTTP response; `error_type` is `none`, `transport`, `timeout`, or `canceled`. Duration ends at response headers or transport failure. Registry identifies the logical artifact provider, including its redirects, rather than each physical destination. These metrics do not count every low-level connection retry, Git transport, Go-module fetch, GitHub metadata lookup, OCI tag-list or referrer pass-through, or S3 operation. OCI registry ping and token exchanges are counted, because they go to the registry for the artifact. Internal Hermit forwarding to another Cachew route is excluded from origin/fill counts; the receiving handler measures its own work. Fills record a terminal outcome, rather than treating an origin 200 as successful publication. `purpose` separates `origin`, `backfill` and ranged `heal`. Results distinguish successful completion from creation, write, commit, transfer or read errors, cancellation, and superseded heals. Tier writes report backend outcomes separately: a failed S3 write can coexist with a successful disk write. Early close or canceled backfill is not a successful fill. Client responses continue normally when cache writes fail. diff --git a/internal/metrics/artifact.go b/internal/metrics/artifact.go index a816f47f..bbc8e620 100644 --- a/internal/metrics/artifact.go +++ b/internal/metrics/artifact.go @@ -61,6 +61,12 @@ func ContinueArtifactRequest(ctx, request context.Context) context.Context { return ctx } +// WithoutArtifactRequest detaches ctx from its artifact request, so auxiliary +// lookups such as release metadata are not counted as origin traffic. +func WithoutArtifactRequest(ctx context.Context) context.Context { + return context.WithValue(ctx, artifactKey{}, nil) +} + // WithCachePurpose distinguishes origin fills from background cache replication. func WithCachePurpose(ctx context.Context, purpose string) context.Context { return context.WithValue(ctx, purposeKey{}, purpose) diff --git a/internal/strategy/github_releases.go b/internal/strategy/github_releases.go index c5dcc494..ef64c131 100644 --- a/internal/strategy/github_releases.go +++ b/internal/strategy/github_releases.go @@ -15,6 +15,7 @@ import ( "github.com/block/cachew/internal/githubapp" "github.com/block/cachew/internal/httputil" "github.com/block/cachew/internal/logging" + "github.com/block/cachew/internal/metrics" "github.com/block/cachew/internal/spool" "github.com/block/cachew/internal/strategy/handler" ) @@ -172,7 +173,7 @@ func (g *GitHubReleases) downloadRelease(ctx context.Context, org, repo, release logger.DebugContext(ctx, "Using GitHub API for private release") apiURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/releases/tags/%s", url.PathEscape(org), url.PathEscape(repo), escapePathSegments(release)) - req, err := g.newGitHubRequest(ctx, apiURL, "application/vnd.github+json", org, "get_release") + req, err := g.newGitHubRequest(metrics.WithoutArtifactRequest(ctx), apiURL, "application/vnd.github+json", org, "get_release") if err != nil { return nil, httputil.Errorf(http.StatusInternalServerError, "create API request") } diff --git a/internal/strategy/github_releases_test.go b/internal/strategy/github_releases_test.go index 3a526f06..bc9bd69d 100644 --- a/internal/strategy/github_releases_test.go +++ b/internal/strategy/github_releases_test.go @@ -12,11 +12,15 @@ import ( "time" "github.com/alecthomas/assert/v2" + "go.opentelemetry.io/otel" + sdkmetric "go.opentelemetry.io/otel/sdk/metric" + "go.opentelemetry.io/otel/sdk/metric/metricdata" "github.com/block/cachew/internal/cache" "github.com/block/cachew/internal/githubapp" "github.com/block/cachew/internal/httputil" "github.com/block/cachew/internal/logging" + "github.com/block/cachew/internal/metrics" "github.com/block/cachew/internal/strategy" ) @@ -122,11 +126,11 @@ func setupTest(t *testing.T, config strategy.GitHubReleasesConfig) (*mockGitHubS t.Cleanup(mock.close) originalTransport := http.DefaultTransport - t.Cleanup(func() { http.DefaultTransport = originalTransport }) //nolint:reassign // Required for testing - http.DefaultTransport = httputil.NewBackoffTransport(&testTransport{ //nolint:reassign // Required for testing + t.Cleanup(func() { http.DefaultTransport = originalTransport }) //nolint:reassign // Required for testing + http.DefaultTransport = httputil.NewBackoffTransport(metrics.NewOriginTransport(&testTransport{ //nolint:reassign // Required for testing backend: mock.server, originalTransport: originalTransport, - }, time.Minute) + }), time.Minute) _, ctx := logging.Configure(context.Background(), logging.Config{Level: slog.LevelError}) memCache, err := cache.NewMemory(ctx, cache.MemoryConfig{MaxTTL: time.Hour}) @@ -202,6 +206,41 @@ func TestGitHubReleasesPrivateRepo(t *testing.T) { assert.Equal(t, 1, mock.downloadCallCount, "second request should be served from cache") } +func TestGitHubReleasesMetadataIsNotOriginTraffic(t *testing.T) { + previous := otel.GetMeterProvider() + reader := sdkmetric.NewManualReader() + provider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) + otel.SetMeterProvider(provider) + t.Cleanup(func() { otel.SetMeterProvider(previous); assert.NoError(t, provider.Shutdown(context.Background())) }) + mock, mux, ctx := setupTest(t, strategy.GitHubReleasesConfig{ + Token: "test-token", + PrivateOrgs: []string{"privateorg"}, + }) + + req := httptest.NewRequest(http.MethodGet, "/github.com/privateorg/repo/releases/download/v2.0.0/binary.tar.gz", nil) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req.WithContext(ctx)) + assert.Equal(t, http.StatusOK, w.Code) + assert.Equal(t, 1, mock.apiCallCount) + + var data metricdata.ResourceMetrics + assert.NoError(t, reader.Collect(ctx, &data)) + var origin int64 + for _, scope := range data.ScopeMetrics { + for _, m := range scope.Metrics { + if m.Name != "cachew.origin.requests" { + continue + } + sum, ok := m.Data.(metricdata.Sum[int64]) + assert.True(t, ok) + for _, point := range sum.DataPoints { + origin += point.Value + } + } + } + assert.Equal(t, int64(1), origin) +} + func TestGitHubReleasesPrivateRepoAssetNotFound(t *testing.T) { mock, mux, ctx := setupTest(t, strategy.GitHubReleasesConfig{ Token: "test-token",