Fix proxy statistics

Signed-off-by: Dimitar Kostadinov <dimitar.kostadinov@sap.com>
distribution · Oct 31, 2023 · d2b369f · d2b369f
1 parent 6c694cb
commit d2b369f
Show file tree

Hide file tree

Showing 5 changed files with 153 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 blobMetrics.Requests %d but got %d", 1, proxyMetrics.manifestMetrics.Requests)
+	}
+	if proxyMetrics.manifestMetrics.Hits != 0 {
+		t.Errorf("Expected blobMetrics.Hits %d but got %d", 0, proxyMetrics.manifestMetrics.Hits)
+	}
+	if proxyMetrics.manifestMetrics.Misses != 1 {
+		t.Errorf("Expected blobMetrics.Misses %d but got %d", 1, proxyMetrics.manifestMetrics.Misses)
+	}
+	if proxyMetrics.manifestMetrics.BytesPulled != 257 {
+		t.Errorf("Expected blobMetrics.BytesPulled %d but got %d", 257, proxyMetrics.manifestMetrics.BytesPulled)
+	}
+	if proxyMetrics.manifestMetrics.BytesPushed != 257 {
+		t.Errorf("Expected blobMetrics.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 blobMetrics.Requests %d but got %d", 2, proxyMetrics.manifestMetrics.Requests)
+	}
+	if proxyMetrics.manifestMetrics.Hits != 1 {
+		t.Errorf("Expected blobMetrics.Hits %d but got %d", 1, proxyMetrics.manifestMetrics.Hits)
+	}
+	if proxyMetrics.manifestMetrics.Misses != 1 {
+		t.Errorf("Expected blobMetrics.Misses %d but got %d", 1, proxyMetrics.manifestMetrics.Misses)
+	}
+	if proxyMetrics.manifestMetrics.BytesPulled != 257 {
+		t.Errorf("Expected blobMetrics.BytesPulled %d but got %d", 257, proxyMetrics.manifestMetrics.BytesPulled)
+	}
+	if proxyMetrics.manifestMetrics.BytesPushed != 514 {
+		t.Errorf("Expected blobMetrics.BytesPushed %d but got %d", 514, proxyMetrics.manifestMetrics.BytesPushed)
+	}
+}
diff --git 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