Skip to content

Commit

Permalink
Store: Fix LabelNames and LabelValues when using non-equal matchers (#…
Browse files Browse the repository at this point in the history
…7661)

* fix non-equal matchers in bucket FilterExtLabelsMatchers

Signed-off-by: Walther Lee <[email protected]>

* add acceptance tests

Signed-off-by: Walther Lee <[email protected]>

---------

Signed-off-by: Walther Lee <[email protected]>
  • Loading branch information
wallee94 authored Aug 24, 2024
1 parent 6737c8d commit d966613
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 2 deletions.
28 changes: 28 additions & 0 deletions pkg/store/acceptance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,34 @@ func testStoreAPIsAcceptance(t *testing.T, startStore startStoreFn) {
},
},
},
{
desc: "label values and names with non-equal matchers on external labels",
appendFn: func(app storage.Appender) {
_, err := app.Append(0, labels.FromStrings("__name__", "up", "foo", "bar"), 0, 0)
testutil.Ok(t, err)
testutil.Ok(t, app.Commit())
},
labelNameCalls: []labelNameCallCase{
{
start: timestamp.FromTime(minTime),
end: timestamp.FromTime(maxTime),
expectedNames: []string{"__name__", "foo", "region"},
matchers: []storepb.LabelMatcher{{Type: storepb.LabelMatcher_RE, Name: "region", Value: ".*"}},
},
},
labelValuesCalls: []labelValuesCallCase{
{
start: timestamp.FromTime(minTime),
end: timestamp.FromTime(maxTime),
label: "region",
matchers: []storepb.LabelMatcher{
{Type: storepb.LabelMatcher_EQ, Name: "__name__", Value: "up"},
{Type: storepb.LabelMatcher_RE, Name: "region", Value: ".*"},
},
expectedValues: []string{"eu-west"},
},
},
},
} {
t.Run(tc.desc, func(t *testing.T) {
appendFn := tc.appendFn
Expand Down
2 changes: 1 addition & 1 deletion pkg/store/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -1960,7 +1960,7 @@ func (b *bucketBlock) FilterExtLabelsMatchers(matchers []*labels.Matcher) ([]*la
// If value is empty string the matcher is a valid one since it's not part of external labels.
if v == "" {
result = append(result, m)
} else if v != "" && v != m.Value {
} else if v != "" && !m.Matches(v) {
// If matcher is external label but value is different we don't want to look in block anyway.
return []*labels.Matcher{}, false
}
Expand Down
14 changes: 13 additions & 1 deletion pkg/store/bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func TestBucketFilterExtLabelsMatchers(t *testing.T) {
{Type: labels.MatchNotEqual, Name: "a", Value: "a"},
}
_, ok := b.FilterExtLabelsMatchers(ms)
testutil.Equals(t, ok, false)
testutil.Equals(t, ok, true)

ms = []*labels.Matcher{
{Type: labels.MatchNotEqual, Name: "a", Value: "a"},
Expand All @@ -248,6 +248,18 @@ func TestBucketFilterExtLabelsMatchers(t *testing.T) {
res, _ = b.FilterExtLabelsMatchers(ms)
testutil.Equals(t, len(res), 1)
testutil.Equals(t, res, ms)

// validate that it can filter out ext labels that match non-equal matchers
ext, err := labels.NewMatcher(labels.MatchRegexp, "a", ".*")
if err != nil {
t.Error(err)
}
ms = []*labels.Matcher{
{Type: labels.MatchNotEqual, Name: "a2", Value: "a"},
}
res, _ = b.FilterExtLabelsMatchers(append(ms, ext))
testutil.Equals(t, len(res), 1)
testutil.Equals(t, res, ms)
}

func TestBucketBlock_matchLabels(t *testing.T) {
Expand Down

0 comments on commit d966613

Please sign in to comment.