Do not prefix Thanos objstore client metrics and fix instrumentation in querier (#2568)

* Do not prefix Thanos objstore client metrics and fix instrumentation in querier

Signed-off-by: Marco Pracucci <[email protected]>

* Updated CHANGELOG

Signed-off-by: Marco Pracucci <[email protected]>

* Added missing file

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed tests

Signed-off-by: Marco Pracucci <[email protected]>

* Moved bucket client instrumentation to factory

Signed-off-by: Marco Pracucci <[email protected]>

* Fixed tests

Signed-off-by: Marco Pracucci <[email protected]>

Author: pracucci

CHANGELOG.md

@@ -22,6 +22,11 @@
 * [CHANGE] Slow query log has a different output now. Previously used `url` field has been replaced with `host` and `path`, and query parameters are logged as individual log fields with `qs_` prefix. #2520
 * [CHANGE] Experimental WAL: WAL and checkpoint compression is now disabled. #2436
 * [CHANGE] Update in dependency `go-kit/kit` from `v0.9.0` to `v0.10.0`. HTML escaping disabled in JSON Logger. #2535
+* [CHANGE] Experimental TSDB: Removed `cortex_<service>_` prefix from Thanos objstore metrics and added `component` label to distinguish which Cortex component is doing API calls to the object storage when running in single-binary mode: #2568
+  - `cortex_<service>_thanos_objstore_bucket_operations_total` renamed to `thanos_objstore_bucket_operations_total{component="<name>"}`
+  - `cortex_<service>_thanos_objstore_bucket_operation_failures_total` renamed to `thanos_objstore_bucket_operation_failures_total{component="<name>"}`
+  - `cortex_<service>_thanos_objstore_bucket_operation_duration_seconds` renamed to `thanos_objstore_bucket_operation_duration_seconds{component="<name>"}`
+  - `cortex_<service>_thanos_objstore_bucket_last_successful_upload_time` renamed to `thanos_objstore_bucket_last_successful_upload_time{component="<name>"}`
 * [FEATURE] Ruler: The `-ruler.evaluation-delay` flag was added to allow users to configure a default evaluation delay for all rules in cortex. The default value is 0 which is the current behavior. #2423
 * [FEATURE] Experimental: Added a new object storage client for OpenStack Swift. #2440
 * [FEATURE] Update in dependency `weaveworks/common`. TLS config options added to the Server. #2535

development/tsdb-blocks-storage-s3/config/grafana-agent.yaml

@@ -46,6 +46,16 @@ prometheus:
             - targets: ['query-frontend:8007']
               labels:
                 container: 'query-frontend'
+        - job_name: cortex-store-gateway-1
+          static_configs:
+            - targets: ['store-gateway-1:8008']
+              labels:
+                container: 'store-gateway-1'
+        - job_name: cortex-store-gateway-2
+          static_configs:
+            - targets: ['store-gateway-2:8009']
+              labels:
+                container: 'store-gateway-2'
 
       remote_write:
         - url: http://distributor:8001/api/prom/push

development/tsdb-blocks-storage-s3/config/prometheus.yaml

@@ -39,6 +39,16 @@ scrape_configs:
       - targets: ['query-frontend:8007']
         labels:
           container: 'query-frontend'
+- job_name: cortex-store-gateway-1
+  static_configs:
+    - targets: ['store-gateway-1:8008']
+      labels:
+        container: 'store-gateway-1'
+- job_name: cortex-store-gateway-2
+  static_configs:
+    - targets: ['store-gateway-2:8009']
+      labels:
+        container: 'store-gateway-2'
 
 remote_write:
   - url: http://distributor:8001/api/prom/push

pkg/compactor/compactor.go

@@ -112,15 +112,11 @@ type Compactor struct {
 // NewCompactor makes a new Compactor.
 func NewCompactor(compactorCfg Config, storageCfg cortex_tsdb.Config, logger log.Logger, registerer prometheus.Registerer) (*Compactor, error) {
 	createBucketClientAndTsdbCompactor := func(ctx context.Context) (objstore.Bucket, tsdb.Compactor, error) {
-		bucketClient, err := cortex_tsdb.NewBucketClient(ctx, storageCfg, "compactor", logger)
+		bucketClient, err := cortex_tsdb.NewBucketClient(ctx, storageCfg, "compactor", logger, registerer)
 		if err != nil {
 			return nil, nil, errors.Wrap(err, "failed to create the bucket client")
 		}
 
-		if registerer != nil {
-			bucketClient = objstore.BucketWithMetrics( /* bucket label value */ "", bucketClient, prometheus.WrapRegistererWithPrefix("cortex_compactor_", registerer))
-		}
-
 		compactor, err := tsdb.NewLeveledCompactor(ctx, registerer, logger, compactorCfg.BlockRanges.ToMilliseconds(), downsample.NewPool())
 		return bucketClient, compactor, err
 	}

pkg/ingester/ingester_v2.go

@@ -82,15 +82,11 @@ type TSDBState struct {
 // NewV2 returns a new Ingester that uses prometheus block storage instead of chunk storage
 func NewV2(cfg Config, clientConfig client.Config, limits *validation.Overrides, registerer prometheus.Registerer) (*Ingester, error) {
 	util.WarnExperimentalUse("Blocks storage engine")
-	bucketClient, err := cortex_tsdb.NewBucketClient(context.Background(), cfg.TSDBConfig, "cortex", util.Logger)
+	bucketClient, err := cortex_tsdb.NewBucketClient(context.Background(), cfg.TSDBConfig, "ingester", util.Logger, registerer)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to create the bucket client")
 	}
 
-	if registerer != nil {
-		bucketClient = objstore.BucketWithMetrics( /* bucket label value */ "", bucketClient, prometheus.WrapRegistererWithPrefix("cortex_ingester_", registerer))
-	}
-
 	i := &Ingester{
 		cfg:           cfg,
 		clientConfig:  clientConfig,

pkg/querier/block.go

@@ -11,7 +11,6 @@ import (
 	"github.com/prometheus/prometheus/promql"
 	"github.com/prometheus/prometheus/storage"
 	"github.com/prometheus/prometheus/tsdb/chunkenc"
-	"github.com/thanos-io/thanos/pkg/objstore"
 	"github.com/thanos-io/thanos/pkg/store/storepb"
 	"github.com/weaveworks/common/logging"
 	"github.com/weaveworks/common/user"
@@ -33,15 +32,11 @@ type BlockQueryable struct {
 // NewBlockQueryable returns a client to query a block store
 func NewBlockQueryable(cfg tsdb.Config, logLevel logging.Level, registerer prometheus.Registerer) (*BlockQueryable, error) {
 	util.WarnExperimentalUse("Blocks storage engine")
-	bucketClient, err := tsdb.NewBucketClient(context.Background(), cfg, "cortex-bucket-stores", util.Logger)
+	bucketClient, err := tsdb.NewBucketClient(context.Background(), cfg, "querier", util.Logger, registerer)
 	if err != nil {
 		return nil, err
 	}
 
-	if registerer != nil {
-		bucketClient = objstore.BucketWithMetrics( /* bucket label value */ "", bucketClient, prometheus.WrapRegistererWithPrefix("cortex_querier_", registerer))
-	}
-
 	us, err := NewBucketStoresService(cfg, bucketClient, logLevel, util.Logger, registerer)
 	if err != nil {
 		return nil, err

pkg/querier/blocks_store_queryable.go

@@ -96,7 +96,7 @@ func NewBlocksStoreQueryable(stores BlocksStoreSet, finder BlocksFinder, reg pro
 func NewBlocksStoreQueryableFromConfig(querierCfg Config, gatewayCfg storegateway.Config, storageCfg cortex_tsdb.Config, logger log.Logger, reg prometheus.Registerer) (*BlocksStoreQueryable, error) {
 	var stores BlocksStoreSet
 
-	bucketClient, err := cortex_tsdb.NewBucketClient(context.Background(), storageCfg, "querier", logger)
+	bucketClient, err := cortex_tsdb.NewBucketClient(context.Background(), storageCfg, "querier", logger, reg)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to create bucket client")
 	}

pkg/storage/tsdb/bucket_client.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"github.com/go-kit/kit/log"
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/thanos-io/thanos/pkg/objstore"
 
 	"github.com/cortexproject/cortex/pkg/storage/backend/azure"
@@ -13,17 +14,34 @@ import (
 )
 
 // NewBucketClient creates a new bucket client based on the configured backend
-func NewBucketClient(ctx context.Context, cfg Config, name string, logger log.Logger) (objstore.Bucket, error) {
+func NewBucketClient(ctx context.Context, cfg Config, name string, logger log.Logger, reg prometheus.Registerer) (client objstore.Bucket, err error) {
 	switch cfg.Backend {
 	case BackendS3:
-		return s3.NewBucketClient(cfg.S3, name, logger)
+		client, err = s3.NewBucketClient(cfg.S3, name, logger)
 	case BackendGCS:
-		return gcs.NewBucketClient(ctx, cfg.GCS, name, logger)
+		client, err = gcs.NewBucketClient(ctx, cfg.GCS, name, logger)
 	case BackendAzure:
-		return azure.NewBucketClient(cfg.Azure, name, logger)
+		client, err = azure.NewBucketClient(cfg.Azure, name, logger)
 	case BackendFilesystem:
-		return filesystem.NewBucketClient(cfg.Filesystem)
+		client, err = filesystem.NewBucketClient(cfg.Filesystem)
 	default:
 		return nil, errUnsupportedStorageBackend
 	}
+
+	if err != nil {
+		return nil, err
+	}
+
+	return bucketWithMetrics(client, name, reg), nil
+}
+
+func bucketWithMetrics(bucketClient objstore.Bucket, name string, reg prometheus.Registerer) objstore.Bucket {
+	if reg == nil {
+		return bucketClient
+	}
+
+	return objstore.BucketWithMetrics(
+		"", // bucket label value
+		bucketClient,
+		prometheus.WrapRegistererWith(prometheus.Labels{"component": name}, reg))
 }

pkg/storage/tsdb/bucket_client_test.go

@@ -86,7 +86,7 @@ func TestNewBucketClient(t *testing.T) {
 			require.NoError(t, err)
 
 			// Instance a new bucket client from the config
-			bucketClient, err := NewBucketClient(context.Background(), cfg, "test", util.Logger)
+			bucketClient, err := NewBucketClient(context.Background(), cfg, "test", util.Logger, nil)
 			require.Equal(t, testData.expectedErr, err)
 
 			if testData.expectedErr == nil {

pkg/storegateway/gateway.go

@@ -304,14 +304,10 @@ func (g *StoreGateway) OnRingInstanceHeartbeat(_ *ring.BasicLifecycler, _ *ring.
 }
 
 func createBucketClient(cfg cortex_tsdb.Config, logger log.Logger, reg prometheus.Registerer) (objstore.Bucket, error) {
-	bucketClient, err := cortex_tsdb.NewBucketClient(context.Background(), cfg, "cortex-bucket-stores", logger)
+	bucketClient, err := cortex_tsdb.NewBucketClient(context.Background(), cfg, "store-gateway", logger, reg)
 	if err != nil {
 		return nil, errors.Wrap(err, "create bucket client")
 	}
 
-	if reg != nil {
-		bucketClient = objstore.BucketWithMetrics( /* bucket label value */ "", bucketClient, prometheus.WrapRegistererWithPrefix("cortex_storegateway_", reg))
-	}
-
 	return bucketClient, nil
 }