From 4015c8ffbc9a8864db8563b4535d88858c7544f1 Mon Sep 17 00:00:00 2001
From: Raj <rkssisodiya@gmail.com>
Date: Wed, 25 Sep 2024 20:10:58 +0530
Subject: [PATCH] chore: some cleanup

---
 .../clickhouseReader/filter_suggestions.go    | 25 ++++-----
 .../integration/filter_suggestions_test.go    | 53 ++++++++++++-------
 2 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/pkg/query-service/app/clickhouseReader/filter_suggestions.go b/pkg/query-service/app/clickhouseReader/filter_suggestions.go
index c9948328ce..fe78fc5555 100644
--- a/pkg/query-service/app/clickhouseReader/filter_suggestions.go
+++ b/pkg/query-service/app/clickhouseReader/filter_suggestions.go
@@ -254,11 +254,12 @@ func (r *ClickHouseReader) getValuesForLogAttributes(
 var attribRanker = newRankingStrategy()
 
 func newRankingStrategy() attribRankingStrategy {
-	// Some attributes should be ranked independent of rest of the synonyms
-	resourceAttrsInDecreasingRank := []string{
+	// Some special resource attributes should get ranked above all others.
+	interestingResourceAttrsInDescRank := []string{
 		"service", "service.name", "env", "k8s.namespace.name",
 	}
-	// add synonyms for interesting attributes for rest of the ranking
+
+	// Synonyms of interesting attributes should come next
 	resourceHierarchy := logsv2.ResourceHierarchy()
 	for _, attr := range []string{
 		"service.name",
@@ -268,28 +269,28 @@ func newRankingStrategy() attribRankingStrategy {
 		"k8s.container.name",
 		"k8s.node.name",
 	} {
-		resourceAttrsInDecreasingRank = append(resourceAttrsInDecreasingRank, resourceHierarchy.Synonyms(attr)...)
+		interestingResourceAttrsInDescRank = append(
+			interestingResourceAttrsInDescRank, resourceHierarchy.Synonyms(attr)...,
+		)
 	}
 
-	resourceAttrsInIncreasingRank := resourceAttrsInDecreasingRank[:]
-	slices.Reverse(resourceAttrsInIncreasingRank)
+	interestingResourceAttrsInAscRank := interestingResourceAttrsInDescRank[:]
+	slices.Reverse(interestingResourceAttrsInAscRank)
 
 	return attribRankingStrategy{
-		resourceAttrsInIncreasingRank: resourceAttrsInIncreasingRank,
+		interestingResourceAttrsInAscRank: interestingResourceAttrsInAscRank,
 	}
 }
 
 type attribRankingStrategy struct {
-	// in decreasing order of ranking
-	resourceAttrsInIncreasingRank []string
+	interestingResourceAttrsInAscRank []string
 }
 
 // The higher the score, the higher the rank
 func (s *attribRankingStrategy) score(attrib v3.AttributeKey) int {
-
-	// Scoring criteria is expected to get more sophisticated in follow up changes
 	if attrib.Type == v3.AttributeKeyTypeResource {
-		return 3 + slices.Index(s.resourceAttrsInIncreasingRank, attrib.Key)
+		// 3 + (-1) if attrib.Key is not an interesting resource attribute
+		return 3 + slices.Index(s.interestingResourceAttrsInAscRank, attrib.Key)
 	}
 
 	if attrib.Type == v3.AttributeKeyTypeTag {
diff --git a/pkg/query-service/tests/integration/filter_suggestions_test.go b/pkg/query-service/tests/integration/filter_suggestions_test.go
index 330af2a79b..91bdcc5a40 100644
--- a/pkg/query-service/tests/integration/filter_suggestions_test.go
+++ b/pkg/query-service/tests/integration/filter_suggestions_test.go
@@ -141,44 +141,57 @@ func TestLogsFilterSuggestionsWithExistingFilter(t *testing.T) {
 func TestResourceAttribsRankedHigherInLogsFilterSuggestions(t *testing.T) {
 	require := require.New(t)
 
-	testAttribKeys := []v3.AttributeKey{}
-
-	tags := []string{"user_id", "user_email"}
-	for _, k := range tags {
-		testAttribKeys = append(testAttribKeys, v3.AttributeKey{
+	tagKeys := []v3.AttributeKey{}
+	for _, k := range []string{"user_id", "user_email"} {
+		tagKeys = append(tagKeys, v3.AttributeKey{
 			Key:      k,
 			Type:     v3.AttributeKeyTypeTag,
-			DataType: v3.AttributeKeyDataTypeArrayString,
+			DataType: v3.AttributeKeyDataTypeString,
 			IsColumn: false,
 		})
 	}
 
-	topResourceAttribs := []string{"service", "env"}
-	otherResourceAttribs := []string{"container_name", "container_id"}
+	specialResourceAttrKeys := []v3.AttributeKey{}
+	for _, k := range []string{"service", "env"} {
+		specialResourceAttrKeys = append(specialResourceAttrKeys, v3.AttributeKey{
+			Key:      k,
+			Type:     v3.AttributeKeyTypeResource,
+			DataType: v3.AttributeKeyDataTypeString,
+			IsColumn: false,
+		})
+	}
 
-	for _, k := range append(otherResourceAttribs, topResourceAttribs...) {
-		testAttribKeys = append(testAttribKeys, v3.AttributeKey{
+	otherResourceAttrKeys := []v3.AttributeKey{}
+	for _, k := range []string{"container_name", "container_id"} {
+		otherResourceAttrKeys = append(otherResourceAttrKeys, v3.AttributeKey{
 			Key:      k,
 			Type:     v3.AttributeKeyTypeResource,
-			DataType: v3.AttributeKeyDataTypeArrayString,
+			DataType: v3.AttributeKeyDataTypeString,
 			IsColumn: false,
 		})
 	}
 
 	tb := NewFilterSuggestionsTestBed(t)
-	tb.mockAttribKeysQueryResponse(testAttribKeys)
+
+	mockAttrKeysInDB := append(tagKeys, otherResourceAttrKeys...)
+	mockAttrKeysInDB = append(mockAttrKeysInDB, specialResourceAttrKeys...)
+
+	tb.mockAttribKeysQueryResponse(mockAttrKeysInDB)
+
+	expectedTopSuggestions := append(specialResourceAttrKeys, otherResourceAttrKeys...)
+	expectedTopSuggestions = append(expectedTopSuggestions, tagKeys...)
+
+	tb.mockAttribValuesQueryResponse(
+		expectedTopSuggestions[:2], [][]string{{"test"}, {"test"}},
+	)
 
 	suggestionsQueryParams := map[string]string{}
 	suggestionsResp := tb.GetQBFilterSuggestionsForLogs(suggestionsQueryParams)
-	suggestedKeys := []string{}
-	for _, k := range suggestionsResp.AttributeKeys {
-		suggestedKeys = append(suggestedKeys, k.Key)
-	}
-
-	expectedSuggestionKeys := append(topResourceAttribs, otherResourceAttribs...)
-	expectedSuggestionKeys = append(expectedSuggestionKeys, tags...)
 
-	require.Equal(expectedSuggestionKeys, suggestedKeys[:len(expectedSuggestionKeys)])
+	require.Equal(
+		expectedTopSuggestions,
+		suggestionsResp.AttributeKeys[:len(expectedTopSuggestions)],
+	)
 }
 
 // Mocks response for CH queries made by reader.GetLogAttributeKeys