Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
86e6e55
Upgrade prometheus to b1ed4a0a663d0c62526312311c7529471abbc565
colega Jul 16, 2021
8b2a36b
Add matchers to all LabelNames implementations
colega Jul 14, 2021
5329ae5
Pass matchers from distributor to ingester's block tsdb
colega Jul 14, 2021
8793512
Ingester.LabelNames(w/matchers) for chunks too
colega Jul 14, 2021
9e32615
Use the old distributor query path when cfg is disabled
colega Jul 14, 2021
99aa870
Sort label names in old implementations
colega Jul 15, 2021
a31b302
BlockStoreQueryable querying both paths
colega Jul 15, 2021
2342f8d
Fix pkg/querier/querier_test.go
colega Jul 16, 2021
c74a9b1
Use the provided ctx
colega Jul 16, 2021
d476d30
Make doc
colega Jul 16, 2021
f241e5e
Test distributorQueryable.LabelNames
colega Jul 16, 2021
bd8492b
Test Distributor.LabelNames
colega Jul 16, 2021
893228a
Test Ingester.LabelNames with matchers
colega Jul 16, 2021
5a7ea80
Add matchers to querier test
colega Jul 16, 2021
f9f5518
Add CHANGELOG.md entry
colega Jul 16, 2021
ce8a310
Move changes to mimir/unreleased changelog section
colega Jul 22, 2021
4025689
Add another label to testcase to improve test
colega Jul 22, 2021
01e894d
Update docs, add `enabled` suffix to the config.
colega Jul 22, 2021
6b451eb
Remove fixme comment and feature flag from blocks_store_queryable
colega Jul 22, 2021
79ab2db
Update span name
colega Jul 22, 2021
28aa6a8
Merge branch 'main' into label-names-api-with-matchers
pracucci Jul 22, 2021
0e3681d
Remove feature flag from blocks_store_queryable
colega Jul 22, 2021
2809837
Fixup changelog entry
colega Jul 22, 2021
70d23fd
Remove fingerprint collision testcase
colega Jul 22, 2021
20597a8
Handle matchers in tenantfederation/mergeQuerier.LabelNames
colega Jul 23, 2021
8f51e15
Opinionated refactor of filterValuesByMatchers
colega Jul 23, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* [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
* [CHANGE] 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` flag once the ingesters are updated to this version. In the future this is expected to become the default behavior.
* [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

* [CHANGE] Update Go version to 1.16.6. #4362
Expand Down
7 changes: 7 additions & 0 deletions docs/blocks-storage/querier.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,13 @@ querier:
# CLI flag: -querier.at-modifier-enabled
[at_modifier_enabled: <boolean> | default = false]

# Use LabelNames() call to ingesters when matchers are provided. Can be
# enabled once this code is deployed on all ingesters, so they correctly read
# that request param. When disabled, the MetricsForLabelMatchers() method is
# used to retrieve label names when matchers are provided.
# CLI flag: -querier.query-label-names-with-matchers
[query_label_names_with_matchers: <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
Expand Down
7 changes: 7 additions & 0 deletions docs/configuration/config-file-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,13 @@ The `querier_config` configures the Cortex querier.
# CLI flag: -querier.at-modifier-enabled
[at_modifier_enabled: <boolean> | default = false]

# Use LabelNames() call to ingesters when matchers are provided. Can be enabled
# once this code is deployed on all ingesters, so they correctly read that
# request param. When disabled, the MetricsForLabelMatchers() method is used to
# retrieve label names when matchers are provided.
# CLI flag: -querier.query-label-names-with-matchers
[query_label_names_with_matchers: <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
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
9 changes: 5 additions & 4 deletions pkg/distributor/distributor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
137 changes: 137 additions & 0 deletions pkg/distributor/distributor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1808,6 +1808,115 @@ 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"}}, 1, 110000},
{labels.Labels{{Name: labels.MetricName, Value: "test_2"}}, 2, 200000},
// The two following series have the same FastFingerprint=e002a3a451262627
{labels.Labels{{Name: labels.MetricName, Value: "fast_fingerprint_collision"}, {Name: "app", Value: "l"}, {Name: "uniq0", Value: "0"}, {Name: "uniq1", Value: "1"}}, 1, 300000},
{labels.Labels{{Name: labels.MetricName, Value: "fast_fingerprint_collision"}, {Name: "app", Value: "m"}, {Name: "uniq0", Value: "1"}, {Name: "uniq1", Value: "1"}}, 1, 300000},
}

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, "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 return all matching metrics even if their FastFingerprint collide": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case comes from TestDistributor_MetricsForLabelMatchers but actually doesn't test that a fingerprint collision doesn't hide away label names (cause the two series have the same label names). Not sure it's worth spending time to write a better test case for the fingerprint collision, we could probably just get rid of the fingerprint collision test case of LabelNames().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the testcase until I find two labels with different names but same fingerprint

matchers: []*labels.Matcher{
mustNewMatcher(labels.MatchEqual, model.MetricNameLabel, "fast_fingerprint_collision"),
},
expectedResult: []string{labels.MetricName, "app", "uniq0", "uniq1"},
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, "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, "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

Expand Down Expand Up @@ -2346,6 +2455,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()
Expand Down
29 changes: 29 additions & 0 deletions pkg/ingester/client/compat.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 21 additions & 0 deletions pkg/ingester/client/compat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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++ {
Expand Down
Loading