Fixed instrumented bucket interface in globalMarkersBucket

CHANGELOG.md

@@ -27,7 +27,7 @@
   * `cortex_alertmanager_sync_configs_failed_total`
   * `cortex_alertmanager_tenants_discovered`
   * `cortex_alertmanager_tenants_owned`
-* [ENHANCEMENT] Blocks storage: introduced a per-tenant bucket index, periodically updated by the compactor, used to avoid full bucket scanning done by queriers, store-gateways and rulers. The bucket index is updated by the compactor during blocks cleanup, on every `-compactor.cleanup-interval`. #3553 #3555 #3561 #3583 #3625
+* [ENHANCEMENT] Blocks storage: introduced a per-tenant bucket index, periodically updated by the compactor, used to avoid full bucket scanning done by queriers, store-gateways and rulers. The bucket index is updated by the compactor during blocks cleanup, on every `-compactor.cleanup-interval`. #3553 #3555 #3561 #3583 #3625 #3711
 * [ENHANCEMENT] Blocks storage: introduced an option `-blocks-storage.bucket-store.bucket-index.enabled` to enable the usage of the bucket index in the querier, store-gateway and ruler. When enabled, the querier, store-gateway and ruler will use the bucket index to find a tenant's blocks instead of running the periodic bucket scan. The following new metrics are exported by the querier and ruler: #3614 #3625
   * `cortex_bucket_index_loads_total`
   * `cortex_bucket_index_load_failures_total`

pkg/storage/tsdb/bucketindex/markers_bucket_client.go

@@ -110,6 +110,24 @@ func (b *globalMarkersBucket) Attributes(ctx context.Context, name string) (objs
 	return b.parent.Attributes(ctx, name)
 }
 
+// WithExpectedErrs implements objstore.InstrumentedBucket.
+func (b *globalMarkersBucket) WithExpectedErrs(fn objstore.IsOpFailureExpectedFunc) objstore.Bucket {
+	if ib, ok := b.parent.(objstore.InstrumentedBucket); ok {
+		return ib.WithExpectedErrs(fn)
+	}
+
+	return b
+}
+
+// ReaderWithExpectedErrs implements objstore.InstrumentedBucketReader.
+func (b *globalMarkersBucket) ReaderWithExpectedErrs(fn objstore.IsOpFailureExpectedFunc) objstore.BucketReader {
+	if ib, ok := b.parent.(objstore.InstrumentedBucketReader); ok {
+		return ib.ReaderWithExpectedErrs(fn)
+	}
+
+	return b
+}
+
 func (b *globalMarkersBucket) isBlockDeletionMark(name string) (ulid.ULID, bool) {
 	if path.Base(name) != metadata.DeletionMarkFilename {
 		return ulid.ULID{}, false

pkg/storage/tsdb/bucketindex/markers_bucket_client_test.go

@@ -1,14 +1,19 @@
 package bucketindex
 
 import (
+	"bytes"
 	"context"
 	"strings"
 	"testing"
 
 	"github.com/oklog/ulid"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/thanos-io/thanos/pkg/objstore"
 
+	"github.com/cortexproject/cortex/pkg/storage/bucket"
 	cortex_testutil "github.com/cortexproject/cortex/pkg/storage/tsdb/testutil"
 )
 
@@ -74,3 +79,76 @@ func TestGlobalMarkersBucket_isBlockDeletionMark(t *testing.T) {
 		})
 	}
 }
+
+func TestBucketWithGlobalMarkers_ShouldWorkCorrectlyWithBucketMetrics(t *testing.T) {
+	reg := prometheus.NewPedanticRegistry()
+	ctx := context.Background()
+
+	// We wrap the underlying filesystem bucket client with metrics,
+	// global markers (intentionally in the middle of the chain) and
+	// user prefix.
+	bkt, _ := cortex_testutil.PrepareFilesystemBucket(t)
+	bkt = objstore.BucketWithMetrics("", bkt, reg)
+	bkt = BucketWithGlobalMarkers(bkt)
+	userBkt := bucket.NewUserBucketClient("user-1", bkt)
+
+	reader, err := userBkt.Get(ctx, "does-not-exist")
+	require.Error(t, err)
+	require.Nil(t, reader)
+	assert.True(t, bkt.IsObjNotFoundErr(err))
+
+	// Should track the failure.
+	assert.NoError(t, testutil.GatherAndCompare(reg, bytes.NewBufferString(`
+		# HELP thanos_objstore_bucket_operation_failures_total Total number of operations against a bucket that failed, but were not expected to fail in certain way from caller perspective. Those errors have to be investigated.
+		# TYPE thanos_objstore_bucket_operation_failures_total counter
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="attributes"} 0
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="delete"} 0
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="exists"} 0
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="get"} 1
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="get_range"} 0
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="iter"} 0
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="upload"} 0
+		# HELP thanos_objstore_bucket_operations_total Total number of all attempted operations against a bucket.
+		# TYPE thanos_objstore_bucket_operations_total counter
+		thanos_objstore_bucket_operations_total{bucket="",operation="attributes"} 0
+		thanos_objstore_bucket_operations_total{bucket="",operation="delete"} 0
+		thanos_objstore_bucket_operations_total{bucket="",operation="exists"} 0
+		thanos_objstore_bucket_operations_total{bucket="",operation="get"} 1
+		thanos_objstore_bucket_operations_total{bucket="",operation="get_range"} 0
+		thanos_objstore_bucket_operations_total{bucket="",operation="iter"} 0
+		thanos_objstore_bucket_operations_total{bucket="",operation="upload"} 0
+	`),
+		"thanos_objstore_bucket_operations_total",
+		"thanos_objstore_bucket_operation_failures_total",
+	))
+
+	reader, err = userBkt.ReaderWithExpectedErrs(userBkt.IsObjNotFoundErr).Get(ctx, "does-not-exist")
+	require.Error(t, err)
+	require.Nil(t, reader)
+	assert.True(t, bkt.IsObjNotFoundErr(err))
+
+	// Should not track the failure.
+	assert.NoError(t, testutil.GatherAndCompare(reg, bytes.NewBufferString(`
+		# HELP thanos_objstore_bucket_operation_failures_total Total number of operations against a bucket that failed, but were not expected to fail in certain way from caller perspective. Those errors have to be investigated.
+		# TYPE thanos_objstore_bucket_operation_failures_total counter
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="attributes"} 0
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="delete"} 0
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="exists"} 0
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="get"} 1
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="get_range"} 0
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="iter"} 0
+		thanos_objstore_bucket_operation_failures_total{bucket="",operation="upload"} 0
+		# HELP thanos_objstore_bucket_operations_total Total number of all attempted operations against a bucket.
+		# TYPE thanos_objstore_bucket_operations_total counter
+		thanos_objstore_bucket_operations_total{bucket="",operation="attributes"} 0
+		thanos_objstore_bucket_operations_total{bucket="",operation="delete"} 0
+		thanos_objstore_bucket_operations_total{bucket="",operation="exists"} 0
+		thanos_objstore_bucket_operations_total{bucket="",operation="get"} 2
+		thanos_objstore_bucket_operations_total{bucket="",operation="get_range"} 0
+		thanos_objstore_bucket_operations_total{bucket="",operation="iter"} 0
+		thanos_objstore_bucket_operations_total{bucket="",operation="upload"} 0
+	`),
+		"thanos_objstore_bucket_operations_total",
+		"thanos_objstore_bucket_operation_failures_total",
+	))
+}