LabelNames API with matchers (#3)

* Upgrade prometheus to b1ed4a0a663d0c62526312311c7529471abbc565 Updated Prometheus to latest commit that includes changes in the LabelQuerier.LabelNames method signature: prometheus/prometheus#9083 Signed-off-by: Oleg Zaytsev <[email protected]> * Add matchers to all LabelNames implementations Signed-off-by: Oleg Zaytsev <[email protected]> * Pass matchers from distributor to ingester's block tsdb Signed-off-by: Oleg Zaytsev <[email protected]> * Ingester.LabelNames(w/matchers) for chunks too Signed-off-by: Oleg Zaytsev <[email protected]> * Use the old distributor query path when cfg is disabled Signed-off-by: Oleg Zaytsev <[email protected]> * Sort label names in old implementations Signed-off-by: Oleg Zaytsev <[email protected]> * BlockStoreQueryable querying both paths Signed-off-by: Oleg Zaytsev <[email protected]> * Fix pkg/querier/querier_test.go Signed-off-by: Oleg Zaytsev <[email protected]> * Use the provided ctx Signed-off-by: Oleg Zaytsev <[email protected]> * Make doc * Test distributorQueryable.LabelNames And fix missing matchers in the call to Distributor Signed-off-by: Oleg Zaytsev <[email protected]> * Test Distributor.LabelNames Signed-off-by: Oleg Zaytsev <[email protected]> * Test Ingester.LabelNames with matchers Signed-off-by: Oleg Zaytsev <[email protected]> * Add matchers to querier test Signed-off-by: Oleg Zaytsev <[email protected]> * Add CHANGELOG.md entry Signed-off-by: Oleg Zaytsev <[email protected]> * Move changes to mimir/unreleased changelog section Signed-off-by: Oleg Zaytsev <[email protected]> * Add another label to testcase to improve test If multiple matchers didn't work, we wouldn't realize as the result set was the same as for a single matcher. Now the multiple matcher doesn't match the series with status=500 which has a reason=broken label Signed-off-by: Oleg Zaytsev <[email protected]> * Update docs, add `enabled` suffix to the config. Signed-off-by: Oleg Zaytsev <[email protected]> * Remove fixme comment and feature flag from blocks_store_queryable All the StoreGateways (actually, one) are already supporting matchers in the call so we don't need to use a feature flag on this. Signed-off-by: Oleg Zaytsev <[email protected]> * Update span name Signed-off-by: Oleg Zaytsev <[email protected]> Co-authored-by: Marco Pracucci <[email protected]> * Remove feature flag from blocks_store_queryable Signed-off-by: Oleg Zaytsev <[email protected]> * Fixup changelog entry Signed-off-by: Oleg Zaytsev <[email protected]> * Remove fingerprint collision testcase At least until we find a collision between two different label names. Signed-off-by: Oleg Zaytsev <[email protected]> * Handle matchers in tenantfederation/mergeQuerier.LabelNames Signed-off-by: Oleg Zaytsev <[email protected]> * Opinionated refactor of filterValuesByMatchers Since there are three options, IMO, a switch is clearer than if/continue/if/elseif structure. Signed-off-by: Oleg Zaytsev <[email protected]> Co-authored-by: Marco Pracucci <[email protected]>
grafana · Jul 23, 2021 · 17e3423 · 17e3423
1 parent 47c4e86
commit 17e3423
Show file tree

Hide file tree

Showing 43 changed files with 911 additions and 220 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,10 +7,12 @@
 * [CHANGE] Changed `-alertmanager.storage.type` default value from `configdb` to `local`. #15
 * [CHANGE] Prevent path traversal attack from users able to control the HTTP header `X-Scope-OrgID`. (CVE-2021-36157) #20
   * Users only have control of the HTTP header when Mimir is not frontend by an auth proxy validating the tenant IDs
+* [ENHANCEMENT] Querier now can use the `LabelNames` call with matchers, if matchers are provided in the `/labels` API call, instead of using the more expensive `MetricsForLabelMatchers` call as before. This can be enabled by enabling the `-querier.query-label-names-with-matchers-enabled` flag once the ingesters are updated to this version. In the future this is expected to become the default behavior. #3
 * [BUGFIX] Upgrade Prometheus. TSDB now waits for pending readers before truncating Head block, fixing the `chunk not found` error and preventing wrong query results. #16
 
 
 ## master / unreleased
+
 * [FEATURE] Ruler: Add new `-ruler.query-stats-enabled` which when enabled will report the `cortex_ruler_query_seconds_total` as a per-user metric that tracks the sum of the wall time of executing queries in the ruler in seconds. #4317
 * [FEATURE] Query Frontend: Add `cortex_query_fetched_series_total` and `cortex_query_fetched_chunks_bytes_total` per-user counters to expose the number of series and bytes fetched as part of queries. These metrics can be enabled with the `-frontend.query-stats-enabled` flag (or its respective YAML config option `query_stats_enabled`). #4343
 * [CHANGE] Update Go version to 1.16.6. #4362

diff --git a/docs/blocks-storage/querier.md b/docs/blocks-storage/querier.md
@@ -136,6 +136,13 @@ querier:
   # CLI flag: -querier.at-modifier-enabled
   [at_modifier_enabled: <boolean> | default = false]
 
+  # True to enable queriers to use an optimized implementation which passes down
+  # to ingesters the label matchers when running the label names API. Can be
+  # enabled once all ingesters run a version >= the one where this option has
+  # been introduced.
+  # CLI flag: -querier.query-label-names-with-matchers-enabled
+  [query_label_names_with_matchers_enabled: <boolean> | default = false]
+
   # The time after which a metric should be queried from storage and not just
   # ingesters. 0 means all queries are sent to store. When running the blocks
   # storage, if this option is enabled, the time range of the query sent to the

diff --git a/docs/configuration/config-file-reference.md b/docs/configuration/config-file-reference.md
@@ -862,6 +862,13 @@ The `querier_config` configures the Cortex querier.
 # CLI flag: -querier.at-modifier-enabled
 [at_modifier_enabled: <boolean> | default = false]
 
+# True to enable queriers to use an optimized implementation which passes down
+# to ingesters the label matchers when running the label names API. Can be
+# enabled once all ingesters run a version >= the one where this option has been
+# introduced.
+# CLI flag: -querier.query-label-names-with-matchers-enabled
+[query_label_names_with_matchers_enabled: <boolean> | default = false]
+
 # The time after which a metric should be queried from storage and not just
 # ingesters. 0 means all queries are sent to store. When running the blocks
 # storage, if this option is enabled, the time range of the query sent to the

diff --git a/docs/configuration/v1-guarantees.md b/docs/configuration/v1-guarantees.md
@@ -95,3 +95,6 @@ Currently experimental features are:
   - `-alertmanager.sharding-ring.heartbeat-period=0`
   - `-compactor.ring.heartbeat-period=0`
   - `-store-gateway.sharding-ring.heartbeat-period=0`
+- `LabelNames` calls using matchers
+  - `-querier.query-label-names-with-matchers-enabled`
+
diff --git a/go.mod b/go.mod
@@ -46,7 +46,7 @@ require (
 	github.com/prometheus/client_golang v1.11.0
 	github.com/prometheus/client_model v0.2.0
 	github.com/prometheus/common v0.29.0
-	github.com/prometheus/prometheus v1.8.2-0.20210720084720-59d02b5ef003
+	github.com/prometheus/prometheus v1.8.2-0.20210720123808-b1ed4a0a663d
 	github.com/segmentio/fasthash v0.0.0-20180216231524-a72b379d632e
 	github.com/sony/gobreaker v0.4.1
 	github.com/spf13/afero v1.2.2

diff --git a/go.sum b/go.sum
@@ -1520,8 +1520,8 @@ github.com/prometheus/prometheus v1.8.2-0.20210215121130-6f488061dfb4/go.mod h1:
 github.com/prometheus/prometheus v1.8.2-0.20210315220929-1cba1741828b/go.mod h1:MS/bpdil77lPbfQeKk6OqVQ9OLnpN3Rszd0hka0EOWE=
 github.com/prometheus/prometheus v1.8.2-0.20210324152458-c7a62b95cea0/go.mod h1:sf7j/iAbhZahjeC0s3wwMmp5dksrJ/Za1UKdR+j6Hmw=
 github.com/prometheus/prometheus v1.8.2-0.20210421143221-52df5ef7a3be/go.mod h1:WbIKsp4vWCoPHis5qQfd0QimLOR7qe79roXN5O8U8bs=
-github.com/prometheus/prometheus v1.8.2-0.20210720084720-59d02b5ef003 h1:MYbsDV+OIFLkqwea5oC3jIUp/HxFyXQrvXpw/hooMRQ=
-github.com/prometheus/prometheus v1.8.2-0.20210720084720-59d02b5ef003/go.mod h1:o6V+A4iPEWjLG0rSEKeev3OzfBZwP+ay+4iS4dkfLI4=
+github.com/prometheus/prometheus v1.8.2-0.20210720123808-b1ed4a0a663d h1:UnqZFF2qXa+ctCfbss/J4yn9rTVoTiuawjrokqwt4Hg=
+github.com/prometheus/prometheus v1.8.2-0.20210720123808-b1ed4a0a663d/go.mod h1:o6V+A4iPEWjLG0rSEKeev3OzfBZwP+ay+4iS4dkfLI4=
 github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
 github.com/rafaeljusto/redigomock v0.0.0-20190202135759-257e089e14a1/go.mod h1:JaY6n2sDr+z2WTsXkOmNRUfDy6FN0L6Nk7x06ndm4tY=
 github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=

diff --git a/pkg/distributor/distributor.go b/pkg/distributor/distributor.go
@@ -881,16 +881,17 @@ func (d *Distributor) LabelValuesForLabelName(ctx context.Context, from, to mode
 }
 
 // LabelNames returns all of the label names.
-func (d *Distributor) LabelNames(ctx context.Context, from, to model.Time) ([]string, error) {
+func (d *Distributor) LabelNames(ctx context.Context, from, to model.Time, matchers ...*labels.Matcher) ([]string, error) {
 	replicationSet, err := d.GetIngestersForMetadata(ctx)
 	if err != nil {
 		return nil, err
 	}
 
-	req := &ingester_client.LabelNamesRequest{
-		StartTimestampMs: int64(from),
-		EndTimestampMs:   int64(to),
+	req, err := ingester_client.ToLabelNamesRequest(from, to, matchers)
+	if err != nil {
+		return nil, err
 	}
+
 	resps, err := d.ForReplicationSet(ctx, replicationSet, func(ctx context.Context, client ingester_client.IngesterClient) (interface{}, error) {
 		return client.LabelNames(ctx, req)
 	})

diff --git a/pkg/distributor/distributor_test.go b/pkg/distributor/distributor_test.go
@@ -1808,6 +1808,105 @@ func TestDistributor_MetricsForLabelMatchers(t *testing.T) {
 	}
 }
 
+func TestDistributor_LabelNames(t *testing.T) {
+	const numIngesters = 5
+
+	fixtures := []struct {
+		lbls      labels.Labels
+		value     float64
+		timestamp int64
+	}{
+		{labels.Labels{{Name: labels.MetricName, Value: "test_1"}, {Name: "status", Value: "200"}}, 1, 100000},
+		{labels.Labels{{Name: labels.MetricName, Value: "test_1"}, {Name: "status", Value: "500"}, {Name: "reason", Value: "broken"}}, 1, 110000},
+		{labels.Labels{{Name: labels.MetricName, Value: "test_2"}}, 2, 200000},
+	}
+
+	tests := map[string]struct {
+		shuffleShardEnabled bool
+		shuffleShardSize    int
+		matchers            []*labels.Matcher
+		expectedResult      []string
+		expectedIngesters   int
+	}{
+		"should return an empty response if no metric match": {
+			matchers: []*labels.Matcher{
+				mustNewMatcher(labels.MatchEqual, model.MetricNameLabel, "unknown"),
+			},
+			expectedResult:    []string{},
+			expectedIngesters: numIngesters,
+		},
+		"should filter metrics by single matcher": {
+			matchers: []*labels.Matcher{
+				mustNewMatcher(labels.MatchEqual, model.MetricNameLabel, "test_1"),
+			},
+			expectedResult:    []string{labels.MetricName, "reason", "status"},
+			expectedIngesters: numIngesters,
+		},
+		"should filter metrics by multiple matchers": {
+			matchers: []*labels.Matcher{
+				mustNewMatcher(labels.MatchEqual, "status", "200"),
+				mustNewMatcher(labels.MatchEqual, model.MetricNameLabel, "test_1"),
+			},
+			expectedResult:    []string{labels.MetricName, "status"},
+			expectedIngesters: numIngesters,
+		},
+		"should query only ingesters belonging to tenant's subring if shuffle sharding is enabled": {
+			shuffleShardEnabled: true,
+			shuffleShardSize:    3,
+			matchers: []*labels.Matcher{
+				mustNewMatcher(labels.MatchEqual, model.MetricNameLabel, "test_1"),
+			},
+			expectedResult:    []string{labels.MetricName, "reason", "status"},
+			expectedIngesters: 3,
+		},
+		"should query all ingesters if shuffle sharding is enabled but shard size is 0": {
+			shuffleShardEnabled: true,
+			shuffleShardSize:    0,
+			matchers: []*labels.Matcher{
+				mustNewMatcher(labels.MatchEqual, model.MetricNameLabel, "test_1"),
+			},
+			expectedResult:    []string{labels.MetricName, "reason", "status"},
+			expectedIngesters: numIngesters,
+		},
+	}
+
+	for testName, testData := range tests {
+		t.Run(testName, func(t *testing.T) {
+			now := model.Now()
+
+			// Create distributor
+			ds, ingesters, r, _ := prepare(t, prepConfig{
+				numIngesters:        numIngesters,
+				happyIngesters:      numIngesters,
+				numDistributors:     1,
+				shardByAllLabels:    true,
+				shuffleShardEnabled: testData.shuffleShardEnabled,
+				shuffleShardSize:    testData.shuffleShardSize,
+			})
+			defer stopAll(ds, r)
+
+			// Push fixtures
+			ctx := user.InjectOrgID(context.Background(), "test")
+
+			for _, series := range fixtures {
+				req := mockWriteRequest(series.lbls, series.value, series.timestamp)
+				_, err := ds[0].Push(ctx, req)
+				require.NoError(t, err)
+			}
+
+			names, err := ds[0].LabelNames(ctx, now, now, testData.matchers...)
+			require.NoError(t, err)
+			assert.ElementsMatch(t, testData.expectedResult, names)
+
+			// Check how many ingesters have been queried.
+			// Due to the quorum the distributor could cancel the last request towards ingesters
+			// if all other ones are successful, so we're good either has been queried X or X-1
+			// ingesters.
+			assert.Contains(t, []int{testData.expectedIngesters, testData.expectedIngesters - 1}, countMockIngestersCalls(ingesters, "LabelNames"))
+		})
+	}
+}
+
 func TestDistributor_MetricsMetadata(t *testing.T) {
 	const numIngesters = 5
 
@@ -2346,6 +2445,34 @@ func (i *mockIngester) MetricsForLabelMatchers(ctx context.Context, req *client.
 	return &response, nil
 }
 
+func (i *mockIngester) LabelNames(ctx context.Context, req *client.LabelNamesRequest, opts ...grpc.CallOption) (*client.LabelNamesResponse, error) {
+	i.Lock()
+	defer i.Unlock()
+
+	i.trackCall("LabelNames")
+
+	if !i.happy {
+		return nil, errFail
+	}
+
+	_, _, matchers, err := client.FromLabelNamesRequest(req)
+	if err != nil {
+		return nil, err
+	}
+
+	response := client.LabelNamesResponse{}
+	for _, ts := range i.timeseries {
+		if match(ts.Labels, matchers) {
+			for _, lbl := range ts.Labels {
+				response.LabelNames = append(response.LabelNames, lbl.Name)
+			}
+		}
+	}
+	sort.Strings(response.LabelNames)
+
+	return &response, nil
+}
+
 func (i *mockIngester) MetricsMetadata(ctx context.Context, req *client.MetricsMetadataRequest, opts ...grpc.CallOption) (*client.MetricsMetadataResponse, error) {
 	i.Lock()
 	defer i.Unlock()

diff --git a/pkg/ingester/client/compat.go b/pkg/ingester/client/compat.go
@@ -172,6 +172,35 @@ func FromLabelValuesRequest(req *LabelValuesRequest) (string, int64, int64, []*l
 	return req.LabelName, req.StartTimestampMs, req.EndTimestampMs, matchers, nil
 }
 
+// ToLabelNamesRequest builds a LabelNamesRequest proto
+func ToLabelNamesRequest(from, to model.Time, matchers []*labels.Matcher) (*LabelNamesRequest, error) {
+	ms, err := toLabelMatchers(matchers)
+	if err != nil {
+		return nil, err
+	}
+
+	return &LabelNamesRequest{
+		StartTimestampMs: int64(from),
+		EndTimestampMs:   int64(to),
+		Matchers:         &LabelMatchers{Matchers: ms},
+	}, nil
+}
+
+// FromLabelNamesRequest unpacks a LabelNamesRequest proto
+func FromLabelNamesRequest(req *LabelNamesRequest) (int64, int64, []*labels.Matcher, error) {
+	var err error
+	var matchers []*labels.Matcher
+
+	if req.Matchers != nil {
+		matchers, err = FromLabelMatchers(req.Matchers.Matchers)
+		if err != nil {
+			return 0, 0, nil, err
+		}
+	}
+
+	return req.StartTimestampMs, req.EndTimestampMs, matchers, nil
+}
+
 func toLabelMatchers(matchers []*labels.Matcher) ([]*LabelMatcher, error) {
 	result := make([]*LabelMatcher, 0, len(matchers))
 	for _, matcher := range matchers {

diff --git a/pkg/ingester/client/compat_test.go b/pkg/ingester/client/compat_test.go
@@ -7,6 +7,9 @@ import (
 	"strconv"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/prometheus/common/model"
 	"github.com/prometheus/prometheus/pkg/labels"
 )
@@ -59,6 +62,24 @@ func TestQueryRequest(t *testing.T) {
 	}
 }
 
+func TestLabelNamesRequest(t *testing.T) {
+	const (
+		mint, maxt = 0, 10
+	)
+
+	matchers := []*labels.Matcher{labels.MustNewMatcher(labels.MatchEqual, "foo", "bar")}
+
+	req, err := ToLabelNamesRequest(mint, maxt, matchers)
+	require.NoError(t, err)
+
+	actualMinT, actualMaxT, actualMatchers, err := FromLabelNamesRequest(req)
+	require.NoError(t, err)
+
+	assert.Equal(t, int64(mint), actualMinT)
+	assert.Equal(t, int64(maxt), actualMaxT)
+	assert.Equal(t, matchers, actualMatchers)
+}
+
 func buildTestMatrix(numSeries int, samplesPerSeries int, offset int) model.Matrix {
 	m := make(model.Matrix, 0, numSeries)
 	for i := 0; i < numSeries; i++ {