Skip to content

Commit

Permalink
Disable Contains metrics
Browse files Browse the repository at this point in the history
It is hard to make any conclusions from the Contains metrics because
they are increased not only directly from externa requests, but also
by internal invocations, such as the grpc_bytestream pre-existing
check. The inlining logic in grpc_ac also affects the metrics, and
I'm not sure if I think that is good or bad.

Disabling the Contains metrics, at least temporary while considering
alternatives, to avoid confusion and potential overhead of counters
that are not useful at the moment.
  • Loading branch information
ulrfa committed Apr 3, 2022
1 parent 78bea4a commit a58de1c
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 52 deletions.
8 changes: 4 additions & 4 deletions cache/disk/disk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1263,8 +1263,8 @@ func TestMetricsUnvalidatedAC(t *testing.T) {
}

acHits := count(testCache.counter, acKind, hitStatus)
if acHits != 1 {
t.Fatalf("Expected acHit counter to be 1, found %f", acHits)
if acHits != 0 {
t.Fatalf("Expected acHit counter to be 0, found %f", acHits)
}

acMiss := count(testCache.counter, acKind, missStatus)
Expand Down Expand Up @@ -1301,8 +1301,8 @@ func TestMetricsUnvalidatedAC(t *testing.T) {
}

acHits = count(testCache.counter, acKind, hitStatus)
if acHits != 2 {
t.Fatalf("Expected acHit counter to be 2, found %f", acHits)
if acHits != 1 {
t.Fatalf("Expected acHit counter to be 1, found %f", acHits)
}
acMiss = count(testCache.counter, acKind, missStatus)
if acMiss != 0 {
Expand Down
48 changes: 0 additions & 48 deletions cache/disk/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,54 +95,6 @@ func (m *metricsDecorator) GetZstd(ctx context.Context, hash string, size int64,
return rc, size, nil
}

func (m *metricsDecorator) Contains(ctx context.Context, kind cache.EntryKind, hash string, size int64) (bool, int64) {
ok, size := m.diskCache.Contains(ctx, kind, hash, size)

lbls := prometheus.Labels{"method": containsMethod, "kind": kind.String()}
if ok {
lbls["status"] = hitStatus
} else {
lbls["status"] = missStatus
}
m.addCategoryLabels(ctx, lbls)
m.counter.With(lbls).Inc()

return ok, size
}

func (m *metricsDecorator) FindMissingCasBlobs(ctx context.Context, blobs []*pb.Digest) ([]*pb.Digest, error) {
numLooking := len(blobs)
digests, err := m.diskCache.FindMissingCasBlobs(ctx, blobs)
if err != nil {
return digests, err
}

numMissing := len(digests)

numFound := numLooking - numMissing

hitLabels := prometheus.Labels{
"method": containsMethod,
"kind": "cas",
"status": hitStatus,
}
m.addCategoryLabels(ctx, hitLabels)
hits := m.counter.With(hitLabels)

missLabels := prometheus.Labels{
"method": containsMethod,
"kind": "cas",
"status": missStatus,
}
m.addCategoryLabels(ctx, missLabels)
misses := m.counter.With(missLabels)

hits.Add(float64(numFound))
misses.Add(float64(numMissing))

return digests, nil
}

func (m *metricsDecorator) Put(ctx context.Context, kind cache.EntryKind, hash string, size int64, r io.Reader) error {
err := m.diskCache.Put(ctx, kind, hash, size, r)
if err != nil {
Expand Down

0 comments on commit a58de1c

Please sign in to comment.