From bf933f546f013c815d14e5a8cc9cbc2036509eb7 Mon Sep 17 00:00:00 2001 From: Dimitar Kostadinov Date: Tue, 5 Sep 2023 16:47:54 +0300 Subject: [PATCH] Fix proxy statistics Signed-off-by: Dimitar Kostadinov --- registry/proxy/proxyblobstore.go | 5 +- registry/proxy/proxyblobstore_test.go | 84 +++++++++++++++++++++++ registry/proxy/proxymanifeststore.go | 2 +- registry/proxy/proxymanifeststore_test.go | 51 ++++++++++++++ registry/proxy/proxymetrics.go | 20 ++++-- 5 files changed, 153 insertions(+), 9 deletions(-) diff --git a/registry/proxy/proxyblobstore.go b/registry/proxy/proxyblobstore.go index f83ef329..0265a07b 100644 --- a/registry/proxy/proxyblobstore.go +++ b/registry/proxy/proxyblobstore.go @@ -62,7 +62,8 @@ func (pbs *proxyBlobStore) copyContent(ctx context.Context, dgst digest.Digest, return distribution.Descriptor{}, err } - proxyMetrics.BlobPush(uint64(desc.Size)) + proxyMetrics.BlobPull(uint64(desc.Size)) + proxyMetrics.BlobPush(uint64(desc.Size), false) return desc, nil } @@ -75,7 +76,7 @@ func (pbs *proxyBlobStore) serveLocal(ctx context.Context, w http.ResponseWriter return false, nil } - proxyMetrics.BlobPush(uint64(localDesc.Size)) + proxyMetrics.BlobPush(uint64(localDesc.Size), true) return true, pbs.localStore.ServeBlob(ctx, w, r, dgst) } diff --git a/registry/proxy/proxyblobstore_test.go b/registry/proxy/proxyblobstore_test.go index 6a4324e7..85142dda 100644 --- a/registry/proxy/proxyblobstore_test.go +++ b/registry/proxy/proxyblobstore_test.go @@ -329,6 +329,90 @@ func TestProxyStoreServeBig(t *testing.T) { testProxyStoreServe(t, te, numClients) } +func TestProxyStoreServeMetrics(t *testing.T) { + te := makeTestEnv(t, "foo/bar") + + blobSize := 200 + blobCount := 10 + numUnique := 10 + populate(t, te, blobCount, blobSize, numUnique) + + numClients := 1 + proxyMetrics = &proxyMetricsCollector{} + testProxyStoreServe(t, te, numClients) + + expected := &proxyMetricsCollector{ + blobMetrics: Metrics{ + Requests: uint64(blobCount*numClients + blobCount), + Hits: uint64(blobCount), + Misses: uint64(blobCount), + BytesPushed: uint64(blobSize*blobCount*numClients + blobSize*blobCount), + BytesPulled: uint64(blobSize * blobCount), + }, + } + + if proxyMetrics.blobMetrics.Requests != expected.blobMetrics.Requests { + t.Errorf("Expected blobMetrics.Requests %d but got %d", expected.blobMetrics.Requests, proxyMetrics.blobMetrics.Requests) + } + if proxyMetrics.blobMetrics.Hits != expected.blobMetrics.Hits { + t.Errorf("Expected blobMetrics.Hits %d but got %d", expected.blobMetrics.Hits, proxyMetrics.blobMetrics.Hits) + } + if proxyMetrics.blobMetrics.Misses != expected.blobMetrics.Misses { + t.Errorf("Expected blobMetrics.Misses %d but got %d", expected.blobMetrics.Misses, proxyMetrics.blobMetrics.Misses) + } + if proxyMetrics.blobMetrics.BytesPushed != expected.blobMetrics.BytesPushed { + t.Errorf("Expected blobMetrics.BytesPushed %d but got %d", expected.blobMetrics.BytesPushed, proxyMetrics.blobMetrics.BytesPushed) + } + if proxyMetrics.blobMetrics.BytesPulled != expected.blobMetrics.BytesPulled { + t.Errorf("Expected blobMetrics.BytesPulled %d but got %d", expected.blobMetrics.BytesPulled, proxyMetrics.blobMetrics.BytesPulled) + } +} + +func TestProxyStoreServeMetricsConcurrent(t *testing.T) { + te := makeTestEnv(t, "foo/bar") + + blobSize := 200 + blobCount := 10 + numUnique := 10 + populate(t, te, blobCount, blobSize, numUnique) + + numClients := 4 + proxyMetrics = &proxyMetricsCollector{} + testProxyStoreServe(t, te, numClients) + + expected := &proxyMetricsCollector{ + blobMetrics: Metrics{ + Requests: uint64(blobCount*numClients + blobCount), + Hits: uint64(blobCount), + Misses: uint64(blobCount), + BytesPushed: uint64(blobSize*blobCount*numClients + blobSize*blobCount), + BytesPulled: uint64(blobSize * blobCount), + }, + } + + if proxyMetrics.blobMetrics.Requests != expected.blobMetrics.Requests { + t.Errorf("Expected blobMetrics.Requests %d but got %d", expected.blobMetrics.Requests, proxyMetrics.blobMetrics.Requests) + } + if proxyMetrics.blobMetrics.Hits+proxyMetrics.blobMetrics.Misses != expected.blobMetrics.Requests { + t.Errorf("Expected blobMetrics.Hits + blobMetrics.Misses %d but got %d", expected.blobMetrics.Requests, proxyMetrics.blobMetrics.Hits+proxyMetrics.blobMetrics.Misses) + } + if proxyMetrics.blobMetrics.Hits < expected.blobMetrics.Hits { + t.Errorf("Expect blobMetrics.Hits %d to be >= %d", proxyMetrics.blobMetrics.Hits, expected.blobMetrics.Hits) + } + if proxyMetrics.blobMetrics.Misses < expected.blobMetrics.Misses { + t.Errorf("Expect blobMetrics.Misses %d to be >= %d", proxyMetrics.blobMetrics.Misses, expected.blobMetrics.Misses) + } + if proxyMetrics.blobMetrics.BytesPushed != expected.blobMetrics.BytesPushed { + t.Errorf("Expected blobMetrics.BytesPushed %d but got %d", expected.blobMetrics.BytesPushed, proxyMetrics.blobMetrics.BytesPushed) + } + if proxyMetrics.blobMetrics.BytesPulled < expected.blobMetrics.BytesPulled { + t.Errorf("Expect blobMetrics.BytesPulled %d to be >= %d", proxyMetrics.blobMetrics.BytesPulled, expected.blobMetrics.BytesPulled) + } + if proxyMetrics.blobMetrics.BytesPulled > expected.blobMetrics.BytesPushed-expected.blobMetrics.BytesPulled { + t.Errorf("Expect blobMetrics.BytesPulled %d to be <= %d", proxyMetrics.blobMetrics.BytesPulled, expected.blobMetrics.BytesPushed-expected.blobMetrics.BytesPulled) + } +} + // testProxyStoreServe will create clients to consume all blobs // populated in the truth store func testProxyStoreServe(t *testing.T, te *testEnv, numClients int) { diff --git a/registry/proxy/proxymanifeststore.go b/registry/proxy/proxymanifeststore.go index fa60a07e..d86c93ac 100644 --- a/registry/proxy/proxymanifeststore.go +++ b/registry/proxy/proxymanifeststore.go @@ -60,7 +60,7 @@ func (pms proxyManifestStore) Get(ctx context.Context, dgst digest.Digest, optio return nil, err } - proxyMetrics.ManifestPush(uint64(len(payload))) + proxyMetrics.ManifestPush(uint64(len(payload)), !fromRemote) if fromRemote { proxyMetrics.ManifestPull(uint64(len(payload))) diff --git a/registry/proxy/proxymanifeststore_test.go b/registry/proxy/proxymanifeststore_test.go index 026cb069..67313f66 100644 --- a/registry/proxy/proxymanifeststore_test.go +++ b/registry/proxy/proxymanifeststore_test.go @@ -280,3 +280,54 @@ func TestProxyManifestsWithoutScheduler(t *testing.T) { t.Fatal(err) } } + +func TestProxyManifestsMetrics(t *testing.T) { + proxyMetrics = &proxyMetricsCollector{} + name := "foo/bar" + env := newManifestStoreTestEnv(t, name, "latest") + + ctx := context.Background() + // Get - should succeed and pull manifest into local + _, err := env.manifests.Get(ctx, env.manifestDigest) + if err != nil { + t.Fatal(err) + } + + if proxyMetrics.manifestMetrics.Requests != 1 { + t.Errorf("Expected manifestMetrics.Requests %d but got %d", 1, proxyMetrics.manifestMetrics.Requests) + } + if proxyMetrics.manifestMetrics.Hits != 0 { + t.Errorf("Expected manifestMetrics.Hits %d but got %d", 0, proxyMetrics.manifestMetrics.Hits) + } + if proxyMetrics.manifestMetrics.Misses != 1 { + t.Errorf("Expected manifestMetrics.Misses %d but got %d", 1, proxyMetrics.manifestMetrics.Misses) + } + if proxyMetrics.manifestMetrics.BytesPulled != 257 { + t.Errorf("Expected manifestMetrics.BytesPulled %d but got %d", 257, proxyMetrics.manifestMetrics.BytesPulled) + } + if proxyMetrics.manifestMetrics.BytesPushed != 257 { + t.Errorf("Expected manifestMetrics.BytesPushed %d but got %d", 257, proxyMetrics.manifestMetrics.BytesPushed) + } + + // Get proxied - manifest comes from local + _, err = env.manifests.Get(ctx, env.manifestDigest) + if err != nil { + t.Fatal(err) + } + + if proxyMetrics.manifestMetrics.Requests != 2 { + t.Errorf("Expected manifestMetrics.Requests %d but got %d", 2, proxyMetrics.manifestMetrics.Requests) + } + if proxyMetrics.manifestMetrics.Hits != 1 { + t.Errorf("Expected manifestMetrics.Hits %d but got %d", 1, proxyMetrics.manifestMetrics.Hits) + } + if proxyMetrics.manifestMetrics.Misses != 1 { + t.Errorf("Expected manifestMetrics.Misses %d but got %d", 1, proxyMetrics.manifestMetrics.Misses) + } + if proxyMetrics.manifestMetrics.BytesPulled != 257 { + t.Errorf("Expected manifestMetrics.BytesPulled %d but got %d", 257, proxyMetrics.manifestMetrics.BytesPulled) + } + if proxyMetrics.manifestMetrics.BytesPushed != 514 { + t.Errorf("Expected manifestMetrics.BytesPushed %d but got %d", 514, proxyMetrics.manifestMetrics.BytesPushed) + } +} diff --git a/registry/proxy/proxymetrics.go b/registry/proxy/proxymetrics.go index eff86694..4eb89569 100644 --- a/registry/proxy/proxymetrics.go +++ b/registry/proxy/proxymetrics.go @@ -46,14 +46,18 @@ func (pmc *proxyMetricsCollector) BlobPull(bytesPulled uint64) { } // BlobPush tracks metrics about blobs pushed to clients -func (pmc *proxyMetricsCollector) BlobPush(bytesPushed uint64) { +func (pmc *proxyMetricsCollector) BlobPush(bytesPushed uint64, isHit bool) { atomic.AddUint64(&pmc.blobMetrics.Requests, 1) - atomic.AddUint64(&pmc.blobMetrics.Hits, 1) atomic.AddUint64(&pmc.blobMetrics.BytesPushed, bytesPushed) requests.WithValues("blob").Inc(1) - hits.WithValues("blob").Inc(1) pushedBytes.WithValues("blob").Inc(float64(bytesPushed)) + + if isHit { + atomic.AddUint64(&pmc.blobMetrics.Hits, 1) + + hits.WithValues("blob").Inc(1) + } } // ManifestPull tracks metrics related to Manifests pulled into the cache @@ -66,14 +70,18 @@ func (pmc *proxyMetricsCollector) ManifestPull(bytesPulled uint64) { } // ManifestPush tracks metrics about manifests pushed to clients -func (pmc *proxyMetricsCollector) ManifestPush(bytesPushed uint64) { +func (pmc *proxyMetricsCollector) ManifestPush(bytesPushed uint64, isHit bool) { atomic.AddUint64(&pmc.manifestMetrics.Requests, 1) - atomic.AddUint64(&pmc.manifestMetrics.Hits, 1) atomic.AddUint64(&pmc.manifestMetrics.BytesPushed, bytesPushed) requests.WithValues("manifest").Inc(1) - hits.WithValues("manifest").Inc(1) pushedBytes.WithValues("manifest").Inc(float64(bytesPushed)) + + if isHit { + atomic.AddUint64(&pmc.manifestMetrics.Hits, 1) + + hits.WithValues("manifest").Inc(1) + } } // proxyMetrics tracks metrics about the proxy cache. This is