Merge pull request #16 from naver/fix-invalid_label_match

Fix(query): Fix the label matcher to avoid using `contains` when distinguishing each label for chunk
naver · Nov 14, 2024 · cf03850 · cf03850
2 parents 60e4a74 + 81022f3
commit cf03850
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 20 deletions.
diff --git a/pkg/lobster/model/labels.go b/pkg/lobster/model/labels.go
@@ -40,6 +40,16 @@ func (l Labels) Pairs() []string {
 	return kvs
 }
 
+func (l Labels) PairKeyMap() map[string]bool {
+	pairKeyMap := map[string]bool{}
+
+	for k, v := range l {
+		pairKeyMap[fmt.Sprintf("%s%s%s", k, LabelKeyValueDelimiter, v)] = true
+	}
+
+	return pairKeyMap
+}
+
 func (l Labels) String() string {
 	return strings.Join(l.Pairs(), LabelsDelimiter)
 }

diff --git a/pkg/lobster/querier/matcher.go b/pkg/lobster/querier/matcher.go
@@ -17,14 +17,12 @@
 package querier
 
 import (
-	"strings"
-
 	"github.com/naver/lobster/pkg/lobster/model"
 	"github.com/naver/lobster/pkg/lobster/query"
 )
 
-type keyFunc func(model.Chunk) string
-type seekFunc func(map[string]bool, string) bool
+type keyFunc func(model.Chunk) interface{}
+type seekFunc func(map[string]bool, interface{}) bool
 
 type chunkMatcher struct {
 	predicates []predicate
@@ -47,9 +45,11 @@ func labelMatchers(req query.Request) []matcher {
 	matchers := []matcher{}
 
 	for _, label := range req.Labels {
-		if len(label) > 0 {
-			matchers = append(matchers, newMatcher(label.Pairs(), seekByKeyValue, func(c model.Chunk) string { return c.Labels.String() }))
+		if len(label) == 0 {
+			continue
 		}
+
+		matchers = append(matchers, newMatcher(label.Pairs(), seekByKeyValuePairMap, func(c model.Chunk) interface{} { return c.Labels.PairKeyMap() }))
 	}
 
 	return matchers
@@ -59,23 +59,23 @@ func nameMatchers(req query.Request) []matcher {
 	matchers := []matcher{}
 
 	if len(req.Clusters) > 0 {
-		matchers = append(matchers, newMatcher(req.Clusters, seekByKey, func(c model.Chunk) string { return c.Cluster }))
+		matchers = append(matchers, newMatcher(req.Clusters, seekByKeyString, func(c model.Chunk) interface{} { return c.Cluster }))
 	}
 	if len(req.SetNames) > 0 {
-		matchers = append(matchers, newMatcher(req.SetNames, seekByKey, func(c model.Chunk) string { return c.SetName }))
+		matchers = append(matchers, newMatcher(req.SetNames, seekByKeyString, func(c model.Chunk) interface{} { return c.SetName }))
 	}
 	if len(req.Pods) > 0 {
-		matchers = append(matchers, newMatcher(req.Pods, seekByKey, func(c model.Chunk) string { return c.Pod }))
+		matchers = append(matchers, newMatcher(req.Pods, seekByKeyString, func(c model.Chunk) interface{} { return c.Pod }))
 	}
 	if len(req.Containers) > 0 {
-		matchers = append(matchers, newMatcher(req.Containers, seekByKey, func(c model.Chunk) string { return c.Container }))
+		matchers = append(matchers, newMatcher(req.Containers, seekByKeyString, func(c model.Chunk) interface{} { return c.Container }))
 	}
 	if len(req.Sources) > 0 {
 		sources := []string{}
 		for _, source := range req.Sources {
 			sources = append(sources, source.String())
 		}
-		matchers = append(matchers, newMatcher(sources, seekByKey, func(c model.Chunk) string { return c.Source.String() }))
+		matchers = append(matchers, newMatcher(sources, seekByKeyString, func(c model.Chunk) interface{} { return c.Source.String() }))
 	}
 
 	return matchers
@@ -104,6 +104,7 @@ func newMatcher(values []string, seekFunc seekFunc, keyFunc keyFunc) matcher {
 		if len(t) == 0 {
 			continue
 		}
+
 		requestedData[t] = true
 	}
 
@@ -114,16 +115,16 @@ func (m matcher) isMatched(chunk model.Chunk) bool {
 	return m.seekFunc(m.requestedData, m.keyFunc(chunk))
 }
 
-func seekByKey(requestedData map[string]bool, key string) bool {
-	_, ok := requestedData[key]
-	return ok
+func seekByKeyString(requestedData map[string]bool, key interface{}) bool {
+	return requestedData[key.(string)]
 }
 
-func seekByKeyValue(requestedData map[string]bool, keyValues string) bool {
+func seekByKeyValuePairMap(requestedData map[string]bool, keyValuesPairMap interface{}) bool {
 	matchedCnt := 0
+	converted := keyValuesPairMap.(map[string]bool)
 
-	for kv := range requestedData {
-		if strings.Contains(keyValues, kv) {
+	for keyValuePair := range requestedData {
+		if converted[keyValuePair] {
 			matchedCnt = matchedCnt + 1
 		}
 	}

diff --git a/pkg/lobster/querier/matcher_test.go b/pkg/lobster/querier/matcher_test.go
@@ -32,7 +32,7 @@ var (
 )
 
 func init() {
-	for i := 0; i < 10; i++ {
+	for i := 0; i < 20; i++ {
 		suffix := fmt.Sprintf("-%d", i)
 		chunk, _ := model.NewChunk(model.LogFile{
 			Labels:    map[string]string{"label": "label" + suffix},
@@ -186,9 +186,9 @@ func TestMatchMultipleLabels(t *testing.T) {
 		matchedLabelString := matched.Labels.String()
 
 		for _, labelString := range expectedLabelStrings {
-			if strings.Contains(matchedLabelString, labelString) {
+			if matchedLabelString == labelString {
 				hasProperLabel = true
-				break
+				break // escape this to check 'or condition'
 			}
 		}