Skip to content

Commit

Permalink
Fix overlapping metric registrations
Browse files Browse the repository at this point in the history
This fixes a problem when multiple metrics bucket are regsitered with
different names. Also adds a test so this won't be happening in the
future.
  • Loading branch information
simonswine committed Jul 27, 2023
1 parent 89475d4 commit 79413f7
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 7 deletions.
14 changes: 7 additions & 7 deletions objstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,10 +431,11 @@ func WrapWithMetrics(b Bucket, reg prometheus.Registerer, name string) *metricBu
Buckets: []float64{0.001, 0.01, 0.1, 0.3, 0.6, 1, 3, 6, 9, 20, 30, 60, 90, 120},
}, []string{"operation"}),

lastSuccessfulUploadTime: promauto.With(reg).NewGaugeVec(prometheus.GaugeOpts{
Name: "objstore_bucket_last_successful_upload_time",
Help: "Second timestamp of the last successful upload to the bucket.",
}, []string{"bucket"}),
lastSuccessfulUploadTime: promauto.With(reg).NewGauge(prometheus.GaugeOpts{
Name: "objstore_bucket_last_successful_upload_time",
Help: "Second timestamp of the last successful upload to the bucket.",
ConstLabels: prometheus.Labels{"bucket": name},
}),
}
for _, op := range []string{
OpIter,
Expand All @@ -450,7 +451,6 @@ func WrapWithMetrics(b Bucket, reg prometheus.Registerer, name string) *metricBu
bkt.opsDuration.WithLabelValues(op)
bkt.opsFetchedBytes.WithLabelValues(op)
}
bkt.lastSuccessfulUploadTime.WithLabelValues(b.Name())
return bkt
}

Expand All @@ -464,7 +464,7 @@ type metricBucket struct {
opsFetchedBytes *prometheus.CounterVec

opsDuration *prometheus.HistogramVec
lastSuccessfulUploadTime *prometheus.GaugeVec
lastSuccessfulUploadTime prometheus.Gauge
}

func (b *metricBucket) WithExpectedErrs(fn IsOpFailureExpectedFunc) Bucket {
Expand Down Expand Up @@ -581,7 +581,7 @@ func (b *metricBucket) Upload(ctx context.Context, name string, r io.Reader) err
}
return err
}
b.lastSuccessfulUploadTime.WithLabelValues(b.bkt.Name()).SetToCurrentTime()
b.lastSuccessfulUploadTime.SetToCurrentTime()
b.opsDuration.WithLabelValues(op).Observe(time.Since(start).Seconds())
return nil
}
Expand Down
8 changes: 8 additions & 0 deletions objstore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ func TestMetricBucket_Close(t *testing.T) {
testutil.Assert(t, promtest.ToFloat64(bkt.lastSuccessfulUploadTime) > lastUpload)
}

// TestMetricBucket_Close_WithErrs tests that the metrics from two different buckets clients are not conflicting with each other

Check failure on line 75 in objstore_test.go

View workflow job for this annotation

GitHub Actions / Linters (Static Analysis) for Go

Comment should end in a period (godot)
func TestMetricBucket_Multiple_Clients(t *testing.T) {
reg := prometheus.NewPedanticRegistry()

WrapWithMetrics(NewInMemBucket(), reg, "abc")
WrapWithMetrics(NewInMemBucket(), reg, "def")
}

func TestDownloadUploadDirConcurrency(t *testing.T) {
r := prometheus.NewRegistry()
m := WrapWithMetrics(NewInMemBucket(), r, "")
Expand Down

0 comments on commit 79413f7

Please sign in to comment.