From 31da3ac0e29dad55daa2061e8a35177ea5f8355b Mon Sep 17 00:00:00 2001 From: Raj Date: Wed, 14 Aug 2024 14:47:10 +0530 Subject: [PATCH 01/12] chore: put together helper for fetching values for multiple attributes --- .../app/clickhouseReader/reader.go | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/pkg/query-service/app/clickhouseReader/reader.go b/pkg/query-service/app/clickhouseReader/reader.go index 5d8f7ece82..0a357ebc8c 100644 --- a/pkg/query-service/app/clickhouseReader/reader.go +++ b/pkg/query-service/app/clickhouseReader/reader.go @@ -4522,6 +4522,96 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( return &suggestions, nil } +func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attributes []v3.AttributeKey, limit uint64) ( + []v3.FilterAttributeValueResponse, *model.ApiError, +) { + query := fmt.Sprintf( + `select tagKey, stringTagValue, int64TagValue, float64TagValue + from ( + select + tagKey, + stringTagValue, + int64TagValue, + float64TagValue, + row_number() over (partition by tagKey order by timestamp desc) as rank + from %s.%s + where tagKey in $1 + ) + where rank <= %d + `, + r.logsDB, r.logsTagAttributeTable, limit, + ) + // query = fmt.Sprintf("select distinct %s from %s.%s where tagKey=$1 and tagType=$2 limit $3", + // filterValueColumn, r.logsDB, r.logsTagAttributeTable) + attribNames := []string{} + for _, attrib := range attributes { + attribNames = append(attribNames, attrib.Key) + } + rows, err := r.db.Query(ctx, query, attribNames) + if err != nil { + zap.L().Error("couldn't query attrib values for suggestions", zap.Error(err)) + return nil, model.InternalError(fmt.Errorf( + "couldn't query attrib values for suggestions: %w", err, + )) + } + defer rows.Close() + + result := make([]v3.FilterAttributeValueResponse, len(attributes)) + + // Helper for getting hold of the result to populate for each scanned row + resultForAttrib := func(key string, dataType v3.AttributeKeyDataType) *v3.FilterAttributeValueResponse { + resultIdx := slices.IndexFunc(attributes, func(attrib v3.AttributeKey) bool { + return attrib.Key == key && attrib.DataType == dataType + }) + + if resultIdx < 0 { + return nil + } else { + return &result[resultIdx] + } + } + + var tagKey string + var stringValue string + var float64Value sql.NullFloat64 + var int64Value sql.NullInt64 + for rows.Next() { + err := rows.Scan( + &tagKey, &stringValue, &int64Value, &float64Value, + ) + if err != nil { + return nil, model.InternalError(fmt.Errorf( + "couldn't scan attrib value rows: %w", err, + )) + } + + if len(stringValue) > 0 { + result := resultForAttrib(tagKey, v3.AttributeKeyDataTypeString) + if result != nil { + result.StringAttributeValues = append(result.StringAttributeValues, stringValue) + } + } else if int64Value.Valid { + result := resultForAttrib(tagKey, v3.AttributeKeyDataTypeInt64) + if result != nil { + result.NumberAttributeValues = append(result.NumberAttributeValues, int64Value.Int64) + } + } else if float64Value.Valid { + result := resultForAttrib(tagKey, v3.AttributeKeyDataTypeFloat64) + if result != nil { + result.NumberAttributeValues = append(result.NumberAttributeValues, float64Value.Float64) + } + } + } + + if err := rows.Err(); err != nil { + return nil, model.InternalError(fmt.Errorf( + "couldn't scan attrib value rows: %w", err, + )) + } + + return result, nil +} + func readRow(vars []interface{}, columnNames []string, countOfNumberCols int) ([]string, map[string]string, []map[string]string, *v3.Point) { // Each row will have a value and a timestamp, and an optional list of label values // example: {Timestamp: ..., Value: ...} From dc4464fc1378af9b78bcc25342cc2bb2fb329158 Mon Sep 17 00:00:00 2001 From: Raj Date: Wed, 14 Aug 2024 15:17:07 +0530 Subject: [PATCH 02/12] chore: poc: use helper for filter suggestions --- .../app/clickhouseReader/reader.go | 110 ++++++++++++------ .../integration/filter_suggestions_test.go | 1 + 2 files changed, 76 insertions(+), 35 deletions(-) diff --git a/pkg/query-service/app/clickhouseReader/reader.go b/pkg/query-service/app/clickhouseReader/reader.go index 0a357ebc8c..84b192952f 100644 --- a/pkg/query-service/app/clickhouseReader/reader.go +++ b/pkg/query-service/app/clickhouseReader/reader.go @@ -4464,45 +4464,85 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( // Example queries for multiple top attributes using a batch version of // GetLogAttributeValues is expected to come in a follow up change if len(suggestions.AttributeKeys) > 0 { - topAttrib := suggestions.AttributeKeys[0] - - resp, err := r.GetLogAttributeValues(ctx, &v3.FilterAttributeValueRequest{ - DataSource: v3.DataSourceLogs, - FilterAttributeKey: topAttrib.Key, - FilterAttributeKeyDataType: topAttrib.DataType, - TagType: v3.TagType(topAttrib.Type), - Limit: 1, - }) + // suggest 2 examples for top 2 attribs + topAttribs := suggestions.AttributeKeys[:1] + if len(suggestions.AttributeKeys) > 1 { + topAttribs = suggestions.AttributeKeys[:2] + } + + topAttribValues, err := r.getValuesForLogAttributes( + ctx, topAttribs, 2, + ) + + fmt.Printf("\n\nDEBUG: err: %v; %v\n\n", err, topAttribValues) if err != nil { // Do not fail the entire request if only example query generation fails zap.L().Error("could not find attribute values for creating example query", zap.Error(err)) } else { - addExampleQuerySuggestion := func(value any) { - exampleQuery := newExampleQuery() - - exampleQuery.Items = append(exampleQuery.Items, v3.FilterItem{ - Key: topAttrib, - Operator: "=", - Value: value, - }) - - suggestions.ExampleQueries = append( - suggestions.ExampleQueries, exampleQuery, - ) - } - if len(resp.StringAttributeValues) > 0 { - addExampleQuerySuggestion(resp.StringAttributeValues[0]) - } else if len(resp.NumberAttributeValues) > 0 { - addExampleQuerySuggestion(resp.NumberAttributeValues[0]) - } else if len(resp.BoolAttributeValues) > 0 { - addExampleQuerySuggestion(resp.BoolAttributeValues[0]) + for valueIdx := 0; valueIdx < 2; valueIdx++ { + for attrIdx, topAttr := range topAttribs { + attrValues := topAttribValues[attrIdx] + + if valueIdx < len(attrValues) { + exampleQuery := newExampleQuery() + exampleQuery.Items = append(exampleQuery.Items, v3.FilterItem{ + Key: topAttr, + Operator: "=", + Value: attrValues[valueIdx], + }) + + suggestions.ExampleQueries = append( + suggestions.ExampleQueries, exampleQuery, + ) + } + } } } } + // if len(suggestions.AttributeKeys) > 0 { + // topAttrib := suggestions.AttributeKeys[0] + + // resp, err := r.GetLogAttributeValues(ctx, &v3.FilterAttributeValueRequest{ + // DataSource: v3.DataSourceLogs, + // FilterAttributeKey: topAttrib.Key, + // FilterAttributeKeyDataType: topAttrib.DataType, + // TagType: v3.TagType(topAttrib.Type), + // Limit: 1, + // }) + + // if err != nil { + // // Do not fail the entire request if only example query generation fails + // zap.L().Error("could not find attribute values for creating example query", zap.Error(err)) + + // } else { + // addExampleQuerySuggestion := func(value any) { + // exampleQuery := newExampleQuery() + + // exampleQuery.Items = append(exampleQuery.Items, v3.FilterItem{ + // Key: topAttrib, + // Operator: "=", + // Value: value, + // }) + + // suggestions.ExampleQueries = append( + // suggestions.ExampleQueries, exampleQuery, + // ) + // } + + // if len(resp.StringAttributeValues) > 0 { + // addExampleQuerySuggestion(resp.StringAttributeValues[0]) + // } else if len(resp.NumberAttributeValues) > 0 { + // addExampleQuerySuggestion(resp.NumberAttributeValues[0]) + // } else if len(resp.BoolAttributeValues) > 0 { + // addExampleQuerySuggestion(resp.BoolAttributeValues[0]) + // } + // } + // } + // Suggest static example queries for standard log attributes if needed. if len(suggestions.ExampleQueries) < req.Limit { exampleQuery := newExampleQuery() @@ -4523,7 +4563,7 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( } func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attributes []v3.AttributeKey, limit uint64) ( - []v3.FilterAttributeValueResponse, *model.ApiError, + [][]any, *model.ApiError, ) { query := fmt.Sprintf( `select tagKey, stringTagValue, int64TagValue, float64TagValue @@ -4556,10 +4596,10 @@ func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attrib } defer rows.Close() - result := make([]v3.FilterAttributeValueResponse, len(attributes)) + result := make([][]any, len(attributes)) // Helper for getting hold of the result to populate for each scanned row - resultForAttrib := func(key string, dataType v3.AttributeKeyDataType) *v3.FilterAttributeValueResponse { + resultForAttrib := func(key string, dataType v3.AttributeKeyDataType) []any { resultIdx := slices.IndexFunc(attributes, func(attrib v3.AttributeKey) bool { return attrib.Key == key && attrib.DataType == dataType }) @@ -4567,7 +4607,7 @@ func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attrib if resultIdx < 0 { return nil } else { - return &result[resultIdx] + return result[resultIdx] } } @@ -4588,17 +4628,17 @@ func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attrib if len(stringValue) > 0 { result := resultForAttrib(tagKey, v3.AttributeKeyDataTypeString) if result != nil { - result.StringAttributeValues = append(result.StringAttributeValues, stringValue) + result = append(result, stringValue) } } else if int64Value.Valid { result := resultForAttrib(tagKey, v3.AttributeKeyDataTypeInt64) if result != nil { - result.NumberAttributeValues = append(result.NumberAttributeValues, int64Value.Int64) + result = append(result, int64Value.Int64) } } else if float64Value.Valid { result := resultForAttrib(tagKey, v3.AttributeKeyDataTypeFloat64) if result != nil { - result.NumberAttributeValues = append(result.NumberAttributeValues, float64Value.Float64) + result = append(result, float64Value.Float64) } } } diff --git a/pkg/query-service/tests/integration/filter_suggestions_test.go b/pkg/query-service/tests/integration/filter_suggestions_test.go index 8c379b1c10..5f7fcea541 100644 --- a/pkg/query-service/tests/integration/filter_suggestions_test.go +++ b/pkg/query-service/tests/integration/filter_suggestions_test.go @@ -71,6 +71,7 @@ func TestLogsFilterSuggestionsWithoutExistingFilter(t *testing.T) { )) require.Greater(len(suggestionsResp.ExampleQueries), 0) + fmt.Println(suggestionsResp) require.True(slices.ContainsFunc( suggestionsResp.ExampleQueries, func(q v3.FilterSet) bool { return slices.ContainsFunc(q.Items, func(i v3.FilterItem) bool { From ff726379ed3af24ddf775ec835d23d45e55bb1c0 Mon Sep 17 00:00:00 2001 From: Raj Date: Thu, 15 Aug 2024 19:35:34 +0530 Subject: [PATCH 03/12] chore: add a working impl for getting attrib values for multiple attributes --- .../app/clickhouseReader/reader.go | 81 ++++++++++--------- pkg/query-service/app/parser.go | 12 +++ pkg/query-service/constants/constants.go | 1 + pkg/query-service/model/v3/v3.go | 3 +- 4 files changed, 59 insertions(+), 38 deletions(-) diff --git a/pkg/query-service/app/clickhouseReader/reader.go b/pkg/query-service/app/clickhouseReader/reader.go index 84b192952f..5f132865da 100644 --- a/pkg/query-service/app/clickhouseReader/reader.go +++ b/pkg/query-service/app/clickhouseReader/reader.go @@ -4474,29 +4474,29 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( ctx, topAttribs, 2, ) - fmt.Printf("\n\nDEBUG: err: %v; %v\n\n", err, topAttribValues) - if err != nil { // Do not fail the entire request if only example query generation fails zap.L().Error("could not find attribute values for creating example query", zap.Error(err)) } else { + // TODO(Raj): Clean this up for valueIdx := 0; valueIdx < 2; valueIdx++ { for attrIdx, topAttr := range topAttribs { - attrValues := topAttribValues[attrIdx] - - if valueIdx < len(attrValues) { - exampleQuery := newExampleQuery() - exampleQuery.Items = append(exampleQuery.Items, v3.FilterItem{ - Key: topAttr, - Operator: "=", - Value: attrValues[valueIdx], - }) - - suggestions.ExampleQueries = append( - suggestions.ExampleQueries, exampleQuery, - ) + if len(suggestions.ExampleQueries) < req.ExamplesLimit { + attrValues := topAttribValues[attrIdx] + if valueIdx < len(attrValues) { + exampleQuery := newExampleQuery() + exampleQuery.Items = append(exampleQuery.Items, v3.FilterItem{ + Key: topAttr, + Operator: "=", + Value: attrValues[valueIdx], + }) + + suggestions.ExampleQueries = append( + suggestions.ExampleQueries, exampleQuery, + ) + } } } } @@ -4544,7 +4544,7 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( // } // Suggest static example queries for standard log attributes if needed. - if len(suggestions.ExampleQueries) < req.Limit { + if len(suggestions.ExampleQueries) < req.ExamplesLimit { exampleQuery := newExampleQuery() exampleQuery.Items = append(exampleQuery.Items, v3.FilterItem{ Key: v3.AttributeKey{ @@ -4566,18 +4566,28 @@ func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attrib [][]any, *model.ApiError, ) { query := fmt.Sprintf( - `select tagKey, stringTagValue, int64TagValue, float64TagValue - from ( + ` + select tagKey, stringTagValue, int64TagValue, float64TagValue + from ( + select + tagKey, + stringTagValue, + int64TagValue, + float64TagValue, + row_number() over (partition by tagKey order by ts desc) as rank + from ( select tagKey, stringTagValue, int64TagValue, float64TagValue, - row_number() over (partition by tagKey order by timestamp desc) as rank + max(timestamp) as ts from %s.%s where tagKey in $1 + group by (tagKey, stringTagValue, int64TagValue, float64TagValue) ) - where rank <= %d + ) + where rank <= %d `, r.logsDB, r.logsTagAttributeTable, limit, ) @@ -4587,6 +4597,7 @@ func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attrib for _, attrib := range attributes { attribNames = append(attribNames, attrib.Key) } + rows, err := r.db.Query(ctx, query, attribNames) if err != nil { zap.L().Error("couldn't query attrib values for suggestions", zap.Error(err)) @@ -4599,16 +4610,11 @@ func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attrib result := make([][]any, len(attributes)) // Helper for getting hold of the result to populate for each scanned row - resultForAttrib := func(key string, dataType v3.AttributeKeyDataType) []any { - resultIdx := slices.IndexFunc(attributes, func(attrib v3.AttributeKey) bool { + + resultIdxForAttrib := func(key string, dataType v3.AttributeKeyDataType) int { + return slices.IndexFunc(attributes, func(attrib v3.AttributeKey) bool { return attrib.Key == key && attrib.DataType == dataType }) - - if resultIdx < 0 { - return nil - } else { - return result[resultIdx] - } } var tagKey string @@ -4626,19 +4632,20 @@ func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attrib } if len(stringValue) > 0 { - result := resultForAttrib(tagKey, v3.AttributeKeyDataTypeString) - if result != nil { - result = append(result, stringValue) + + attrResultIdx := resultIdxForAttrib(tagKey, v3.AttributeKeyDataTypeString) + if attrResultIdx >= 0 { + result[attrResultIdx] = append(result[attrResultIdx], stringValue) } } else if int64Value.Valid { - result := resultForAttrib(tagKey, v3.AttributeKeyDataTypeInt64) - if result != nil { - result = append(result, int64Value.Int64) + attrResultIdx := resultIdxForAttrib(tagKey, v3.AttributeKeyDataTypeInt64) + if attrResultIdx >= 0 { + result[attrResultIdx] = append(result[attrResultIdx], int64Value.Int64) } } else if float64Value.Valid { - result := resultForAttrib(tagKey, v3.AttributeKeyDataTypeFloat64) - if result != nil { - result = append(result, float64Value.Float64) + attrResultIdx := resultIdxForAttrib(tagKey, v3.AttributeKeyDataTypeFloat64) + if attrResultIdx >= 0 { + result[attrResultIdx] = append(result[attrResultIdx], float64Value.Float64) } } } diff --git a/pkg/query-service/app/parser.go b/pkg/query-service/app/parser.go index 9bdde09be6..701bc31a8d 100644 --- a/pkg/query-service/app/parser.go +++ b/pkg/query-service/app/parser.go @@ -857,6 +857,17 @@ func parseQBFilterSuggestionsRequest(r *http.Request) ( } } + examplesLimit := baseconstants.DefaultFilterSuggestionsExamplesLimit + examplesLimitStr := r.URL.Query().Get("exampleLimit") + if len(examplesLimitStr) > 0 { + examplesLimit, err := strconv.Atoi(examplesLimitStr) + if err != nil || examplesLimit < 1 { + return nil, model.BadRequest(fmt.Errorf( + "invalid examples limit: %s", limitStr, + )) + } + } + var existingFilter *v3.FilterSet existingFilterB64 := r.URL.Query().Get("existingFilter") if len(existingFilterB64) > 0 { @@ -878,6 +889,7 @@ func parseQBFilterSuggestionsRequest(r *http.Request) ( DataSource: dataSource, Limit: limit, SearchText: searchText, + ExamplesLimit: examplesLimit, ExistingFilter: existingFilter, }, nil } diff --git a/pkg/query-service/constants/constants.go b/pkg/query-service/constants/constants.go index 4b5134c6ee..814f63797b 100644 --- a/pkg/query-service/constants/constants.go +++ b/pkg/query-service/constants/constants.go @@ -418,3 +418,4 @@ var TracesListViewDefaultSelectedColumns = []v3.AttributeKey{ } const DefaultFilterSuggestionsLimit = 100 +const DefaultFilterSuggestionsExamplesLimit = 2 diff --git a/pkg/query-service/model/v3/v3.go b/pkg/query-service/model/v3/v3.go index b0e786a6d6..6d199d1a4b 100644 --- a/pkg/query-service/model/v3/v3.go +++ b/pkg/query-service/model/v3/v3.go @@ -256,7 +256,8 @@ type QBFilterSuggestionsRequest struct { DataSource DataSource `json:"dataSource"` SearchText string `json:"searchText"` Limit int `json:"limit"` - ExistingFilter *FilterSet `json:"existing_filter"` + ExamplesLimit int `json:"examplesLimit"` + ExistingFilter *FilterSet `json:"existingFilter"` } type QBFilterSuggestionsResponse struct { From 128882674bab1830a738347a09f273850673f264 Mon Sep 17 00:00:00 2001 From: Raj Date: Thu, 15 Aug 2024 19:57:39 +0530 Subject: [PATCH 04/12] chore: start updating integration test to account for new approach for getting log attrib values --- .../app/clickhouseReader/reader.go | 40 ------------------- .../integration/filter_suggestions_test.go | 31 +++++++++----- 2 files changed, 21 insertions(+), 50 deletions(-) diff --git a/pkg/query-service/app/clickhouseReader/reader.go b/pkg/query-service/app/clickhouseReader/reader.go index 5f132865da..90af898e54 100644 --- a/pkg/query-service/app/clickhouseReader/reader.go +++ b/pkg/query-service/app/clickhouseReader/reader.go @@ -4503,46 +4503,6 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( } } - // if len(suggestions.AttributeKeys) > 0 { - // topAttrib := suggestions.AttributeKeys[0] - - // resp, err := r.GetLogAttributeValues(ctx, &v3.FilterAttributeValueRequest{ - // DataSource: v3.DataSourceLogs, - // FilterAttributeKey: topAttrib.Key, - // FilterAttributeKeyDataType: topAttrib.DataType, - // TagType: v3.TagType(topAttrib.Type), - // Limit: 1, - // }) - - // if err != nil { - // // Do not fail the entire request if only example query generation fails - // zap.L().Error("could not find attribute values for creating example query", zap.Error(err)) - - // } else { - // addExampleQuerySuggestion := func(value any) { - // exampleQuery := newExampleQuery() - - // exampleQuery.Items = append(exampleQuery.Items, v3.FilterItem{ - // Key: topAttrib, - // Operator: "=", - // Value: value, - // }) - - // suggestions.ExampleQueries = append( - // suggestions.ExampleQueries, exampleQuery, - // ) - // } - - // if len(resp.StringAttributeValues) > 0 { - // addExampleQuerySuggestion(resp.StringAttributeValues[0]) - // } else if len(resp.NumberAttributeValues) > 0 { - // addExampleQuerySuggestion(resp.NumberAttributeValues[0]) - // } else if len(resp.BoolAttributeValues) > 0 { - // addExampleQuerySuggestion(resp.BoolAttributeValues[0]) - // } - // } - // } - // Suggest static example queries for standard log attributes if needed. if len(suggestions.ExampleQueries) < req.ExamplesLimit { exampleQuery := newExampleQuery() diff --git a/pkg/query-service/tests/integration/filter_suggestions_test.go b/pkg/query-service/tests/integration/filter_suggestions_test.go index 5f7fcea541..03f64dcd6f 100644 --- a/pkg/query-service/tests/integration/filter_suggestions_test.go +++ b/pkg/query-service/tests/integration/filter_suggestions_test.go @@ -59,7 +59,9 @@ func TestLogsFilterSuggestionsWithoutExistingFilter(t *testing.T) { testAttribValue := "test-container" tb.mockAttribKeysQueryResponse([]v3.AttributeKey{testAttrib}) - tb.mockAttribValuesQueryResponse(testAttrib, []string{testAttribValue}) + tb.mockAttribValuesQueryResponse( + []v3.AttributeKey{testAttrib}, [][]string{[]string{testAttribValue}}, + ) suggestionsQueryParams := map[string]string{} suggestionsResp := tb.GetQBFilterSuggestionsForLogs(suggestionsQueryParams) @@ -114,7 +116,9 @@ func TestLogsFilterSuggestionsWithExistingFilter(t *testing.T) { } tb.mockAttribKeysQueryResponse([]v3.AttributeKey{testAttrib, testFilterAttrib}) - tb.mockAttribValuesQueryResponse(testAttrib, []string{testAttribValue}) + tb.mockAttribValuesQueryResponse( + []v3.AttributeKey{testAttrib}, [][]string{[]string{testAttribValue}}, + ) testFilterJson, err := json.Marshal(testFilter) require.Nil(err, "couldn't serialize existing filter to JSON") @@ -170,22 +174,29 @@ func (tb *FilterSuggestionsTestBed) mockAttribKeysQueryResponse( // Mocks response for CH queries made by reader.GetLogAttributeValues func (tb *FilterSuggestionsTestBed) mockAttribValuesQueryResponse( - expectedAttrib v3.AttributeKey, - stringValuesToReturn []string, + expectedAttribs []v3.AttributeKey, + expectedStringValues [][]string, ) { cols := []mockhouse.ColumnType{} + cols = append(cols, mockhouse.ColumnType{Type: "String", Name: "tagKey"}) cols = append(cols, mockhouse.ColumnType{Type: "String", Name: "stringTagValue"}) + cols = append(cols, mockhouse.ColumnType{Type: "Nullable(Int64)", Name: "int64TagValue"}) + cols = append(cols, mockhouse.ColumnType{Type: "Nullable(Float64)", Name: "float64TagValue"}) + expectedAttribKeys := []string{} values := [][]any{} - for _, v := range stringValuesToReturn { - rowValues := []any{} - rowValues = append(rowValues, v) - values = append(values, rowValues) + for idx, attrib := range expectedAttribs { + expectedAttribKeys = append(expectedAttribKeys, attrib.Key) + for _, val := range expectedStringValues[idx] { + rowValues := []any{} + rowValues = append(rowValues, attrib.Key, val, nil, nil) + values = append(values, rowValues) + } } tb.mockClickhouse.ExpectQuery( - "select distinct.*stringTagValue.*from.*signoz_logs.distributed_tag_attributes.*", - ).WithArgs(string(expectedAttrib.Key), v3.TagType(expectedAttrib.Type), 1).WillReturnRows(mockhouse.NewRows(cols, values)) + "select.*tagKey.*stringTagValue.*int64TagValue.*float64TagValue.*distributed_tag_attributes.*tagKey.*IN.*", + ).WithArgs(expectedAttribKeys).WillReturnRows(mockhouse.NewRows(cols, values)) } type FilterSuggestionsTestBed struct { From c084766a9fadb75ab724a89a260d94e31baa0670 Mon Sep 17 00:00:00 2001 From: Raj Date: Mon, 2 Sep 2024 19:52:08 +0530 Subject: [PATCH 05/12] chore: use a global zap logger in filter suggestion tests --- .../tests/integration/filter_suggestions_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/query-service/tests/integration/filter_suggestions_test.go b/pkg/query-service/tests/integration/filter_suggestions_test.go index 03f64dcd6f..db49525763 100644 --- a/pkg/query-service/tests/integration/filter_suggestions_test.go +++ b/pkg/query-service/tests/integration/filter_suggestions_test.go @@ -19,6 +19,7 @@ import ( "go.signoz.io/signoz/pkg/query-service/model" v3 "go.signoz.io/signoz/pkg/query-service/model/v3" "go.signoz.io/signoz/pkg/query-service/utils" + "go.uber.org/zap" ) // If no data has been received yet, filter suggestions should contain @@ -60,7 +61,7 @@ func TestLogsFilterSuggestionsWithoutExistingFilter(t *testing.T) { tb.mockAttribKeysQueryResponse([]v3.AttributeKey{testAttrib}) tb.mockAttribValuesQueryResponse( - []v3.AttributeKey{testAttrib}, [][]string{[]string{testAttribValue}}, + []v3.AttributeKey{testAttrib}, [][]string{{testAttribValue}}, ) suggestionsQueryParams := map[string]string{} suggestionsResp := tb.GetQBFilterSuggestionsForLogs(suggestionsQueryParams) @@ -73,7 +74,7 @@ func TestLogsFilterSuggestionsWithoutExistingFilter(t *testing.T) { )) require.Greater(len(suggestionsResp.ExampleQueries), 0) - fmt.Println(suggestionsResp) + require.True(slices.ContainsFunc( suggestionsResp.ExampleQueries, func(q v3.FilterSet) bool { return slices.ContainsFunc(q.Items, func(i v3.FilterItem) bool { @@ -256,6 +257,13 @@ func NewFilterSuggestionsTestBed(t *testing.T) *FilterSuggestionsTestBed { t.Fatalf("could not create a test user: %v", apiErr) } + logger := zap.NewExample() + originalLogger := zap.L() + zap.ReplaceGlobals(logger) + t.Cleanup(func() { + zap.ReplaceGlobals(originalLogger) + }) + return &FilterSuggestionsTestBed{ t: t, testUser: user, From c8ab8737d7667ea9bbcb12b15ed1718e45b70d92 Mon Sep 17 00:00:00 2001 From: Raj Date: Mon, 2 Sep 2024 19:52:40 +0530 Subject: [PATCH 06/12] chore: fix attrib values clickhouse query expectation --- pkg/query-service/tests/integration/filter_suggestions_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/query-service/tests/integration/filter_suggestions_test.go b/pkg/query-service/tests/integration/filter_suggestions_test.go index db49525763..8dc199d2c5 100644 --- a/pkg/query-service/tests/integration/filter_suggestions_test.go +++ b/pkg/query-service/tests/integration/filter_suggestions_test.go @@ -196,7 +196,7 @@ func (tb *FilterSuggestionsTestBed) mockAttribValuesQueryResponse( } tb.mockClickhouse.ExpectQuery( - "select.*tagKey.*stringTagValue.*int64TagValue.*float64TagValue.*distributed_tag_attributes.*tagKey.*IN.*", + "select.*tagKey.*stringTagValue.*int64TagValue.*float64TagValue.*distributed_tag_attributes.*tagKey.*in.*", ).WithArgs(expectedAttribKeys).WillReturnRows(mockhouse.NewRows(cols, values)) } From 18930d84dce34d7368f7d94e40327c4de4e4e3b5 Mon Sep 17 00:00:00 2001 From: Raj Date: Mon, 2 Sep 2024 19:53:34 +0530 Subject: [PATCH 07/12] chore: only query values for actual attributes when generating example queries --- .../app/clickhouseReader/reader.go | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/pkg/query-service/app/clickhouseReader/reader.go b/pkg/query-service/app/clickhouseReader/reader.go index 90af898e54..88ac1c3eab 100644 --- a/pkg/query-service/app/clickhouseReader/reader.go +++ b/pkg/query-service/app/clickhouseReader/reader.go @@ -4465,19 +4465,32 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( // GetLogAttributeValues is expected to come in a follow up change if len(suggestions.AttributeKeys) > 0 { // suggest 2 examples for top 2 attribs - topAttribs := suggestions.AttributeKeys[:1] - if len(suggestions.AttributeKeys) > 1 { - topAttribs = suggestions.AttributeKeys[:2] + topAttribs := []v3.AttributeKey{} + for _, attrib := range suggestions.AttributeKeys { + if len(topAttribs) >= req.ExamplesLimit { + break + } + + // only suggest examples for actual attributes + _, isTopLevelField := constants.StaticFieldsLogsV3[attrib.Key] + isNumOrStringType := slices.Contains([]v3.AttributeKeyDataType{ + v3.AttributeKeyDataTypeInt64, v3.AttributeKeyDataTypeFloat64, v3.AttributeKeyDataTypeString, + }, attrib.DataType) + + if !isTopLevelField && isNumOrStringType { + topAttribs = append(topAttribs, attrib) + } } + numValuesPerAttrib := math.Ceil(float64(req.ExamplesLimit) / float64(len(topAttribs))) + topAttribValues, err := r.getValuesForLogAttributes( - ctx, topAttribs, 2, + ctx, topAttribs, uint64(numValuesPerAttrib), ) if err != nil { // Do not fail the entire request if only example query generation fails zap.L().Error("could not find attribute values for creating example query", zap.Error(err)) - } else { // TODO(Raj): Clean this up @@ -4551,8 +4564,7 @@ func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attrib `, r.logsDB, r.logsTagAttributeTable, limit, ) - // query = fmt.Sprintf("select distinct %s from %s.%s where tagKey=$1 and tagType=$2 limit $3", - // filterValueColumn, r.logsDB, r.logsTagAttributeTable) + attribNames := []string{} for _, attrib := range attributes { attribNames = append(attribNames, attrib.Key) From a3c271106132dca31b5c21ef4697e8c4342cdbf6 Mon Sep 17 00:00:00 2001 From: Raj Date: Tue, 3 Sep 2024 10:59:34 +0530 Subject: [PATCH 08/12] chore: update clickhouse-go-mock --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index c7cafd89cc..9d61916d42 100644 --- a/go.mod +++ b/go.mod @@ -47,7 +47,7 @@ require ( github.com/sethvargo/go-password v0.2.0 github.com/smartystreets/goconvey v1.8.1 github.com/soheilhy/cmux v0.1.5 - github.com/srikanthccv/ClickHouse-go-mock v0.8.0 + github.com/srikanthccv/ClickHouse-go-mock v0.9.0 github.com/stretchr/testify v1.9.0 go.opentelemetry.io/collector/component v0.103.0 go.opentelemetry.io/collector/confmap v0.103.0 diff --git a/go.sum b/go.sum index a98137d6db..a442200b0e 100644 --- a/go.sum +++ b/go.sum @@ -716,6 +716,8 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/srikanthccv/ClickHouse-go-mock v0.8.0 h1:DeeM8XLbTFl6sjYPPwazPEXx7kmRV8TgPFVkt1SqT0Y= github.com/srikanthccv/ClickHouse-go-mock v0.8.0/go.mod h1:pgJm+apjvi7FHxEdgw1Bt4MRbUYpVxyhKQ/59Wkig24= +github.com/srikanthccv/ClickHouse-go-mock v0.9.0 h1:XKr1Tb7GL1HlifKH874QGR3R6l0e6takXasROUiZawU= +github.com/srikanthccv/ClickHouse-go-mock v0.9.0/go.mod h1:pgJm+apjvi7FHxEdgw1Bt4MRbUYpVxyhKQ/59Wkig24= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= From d4b0cf2cc8d34e08161d218612583efd071ea7e3 Mon Sep 17 00:00:00 2001 From: Raj Date: Tue, 3 Sep 2024 19:53:42 +0530 Subject: [PATCH 09/12] chore: cleanup: separate params for attributesLimit and examplesLimit for filter suggestions --- .../app/clickhouseReader/reader.go | 10 ++-- pkg/query-service/app/parser.go | 51 +++++++++++-------- pkg/query-service/constants/constants.go | 2 +- pkg/query-service/model/v3/v3.go | 10 ++-- .../integration/filter_suggestions_test.go | 2 +- 5 files changed, 41 insertions(+), 34 deletions(-) diff --git a/pkg/query-service/app/clickhouseReader/reader.go b/pkg/query-service/app/clickhouseReader/reader.go index 88ac1c3eab..2ff4e56a00 100644 --- a/pkg/query-service/app/clickhouseReader/reader.go +++ b/pkg/query-service/app/clickhouseReader/reader.go @@ -4414,7 +4414,7 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( ctx, &v3.FilterAttributeKeyRequest{ SearchText: req.SearchText, DataSource: v3.DataSourceLogs, - Limit: req.Limit, + Limit: int(req.AttributesLimit), }) if err != nil { return nil, model.InternalError(fmt.Errorf("couldn't get attribute keys: %w", err)) @@ -4467,7 +4467,7 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( // suggest 2 examples for top 2 attribs topAttribs := []v3.AttributeKey{} for _, attrib := range suggestions.AttributeKeys { - if len(topAttribs) >= req.ExamplesLimit { + if len(topAttribs) >= int(req.ExamplesLimit) { break } @@ -4494,9 +4494,9 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( } else { // TODO(Raj): Clean this up - for valueIdx := 0; valueIdx < 2; valueIdx++ { + for valueIdx := 0; valueIdx < int(numValuesPerAttrib); valueIdx++ { for attrIdx, topAttr := range topAttribs { - if len(suggestions.ExampleQueries) < req.ExamplesLimit { + if len(suggestions.ExampleQueries) < int(req.ExamplesLimit) { attrValues := topAttribValues[attrIdx] if valueIdx < len(attrValues) { exampleQuery := newExampleQuery() @@ -4517,7 +4517,7 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( } // Suggest static example queries for standard log attributes if needed. - if len(suggestions.ExampleQueries) < req.ExamplesLimit { + if len(suggestions.ExampleQueries) < int(req.ExamplesLimit) { exampleQuery := newExampleQuery() exampleQuery.Items = append(exampleQuery.Items, v3.FilterItem{ Key: v3.AttributeKey{ diff --git a/pkg/query-service/app/parser.go b/pkg/query-service/app/parser.go index 701bc31a8d..a8b08e5ad7 100644 --- a/pkg/query-service/app/parser.go +++ b/pkg/query-service/app/parser.go @@ -846,26 +846,33 @@ func parseQBFilterSuggestionsRequest(r *http.Request) ( return nil, model.BadRequest(err) } - limit := baseconstants.DefaultFilterSuggestionsLimit - limitStr := r.URL.Query().Get("limit") - if len(limitStr) > 0 { - limit, err := strconv.Atoi(limitStr) - if err != nil || limit < 1 { - return nil, model.BadRequest(fmt.Errorf( - "invalid limit: %s", limitStr, - )) + parsePositiveIntQP := func(queryParam string, defaultValue uint64) (uint64, *model.ApiError) { + value := defaultValue + qpValue := r.URL.Query().Get(queryParam) + if len(qpValue) > 0 { + value, err := strconv.Atoi(qpValue) + if err != nil || value < 1 { + return 0, model.BadRequest(fmt.Errorf( + "invalid %s: %s", queryParam, qpValue, + )) + } } + return value, nil + } - examplesLimit := baseconstants.DefaultFilterSuggestionsExamplesLimit - examplesLimitStr := r.URL.Query().Get("exampleLimit") - if len(examplesLimitStr) > 0 { - examplesLimit, err := strconv.Atoi(examplesLimitStr) - if err != nil || examplesLimit < 1 { - return nil, model.BadRequest(fmt.Errorf( - "invalid examples limit: %s", limitStr, - )) - } + attributesLimit, err := parsePositiveIntQP( + "attributesLimit", baseconstants.DefaultFilterSuggestionsAttributesLimit, + ) + if err != nil { + return nil, err + } + + examplesLimit, err := parsePositiveIntQP( + "examplesLimit", baseconstants.DefaultFilterSuggestionsExamplesLimit, + ) + if err != nil { + return nil, err } var existingFilter *v3.FilterSet @@ -886,11 +893,11 @@ func parseQBFilterSuggestionsRequest(r *http.Request) ( searchText := r.URL.Query().Get("searchText") return &v3.QBFilterSuggestionsRequest{ - DataSource: dataSource, - Limit: limit, - SearchText: searchText, - ExamplesLimit: examplesLimit, - ExistingFilter: existingFilter, + DataSource: dataSource, + AttributesLimit: attributesLimit, + SearchText: searchText, + ExamplesLimit: examplesLimit, + ExistingFilter: existingFilter, }, nil } diff --git a/pkg/query-service/constants/constants.go b/pkg/query-service/constants/constants.go index 814f63797b..34ff35ef92 100644 --- a/pkg/query-service/constants/constants.go +++ b/pkg/query-service/constants/constants.go @@ -417,5 +417,5 @@ var TracesListViewDefaultSelectedColumns = []v3.AttributeKey{ }, } -const DefaultFilterSuggestionsLimit = 100 +const DefaultFilterSuggestionsAttributesLimit = 100 const DefaultFilterSuggestionsExamplesLimit = 2 diff --git a/pkg/query-service/model/v3/v3.go b/pkg/query-service/model/v3/v3.go index 6d199d1a4b..0f04375198 100644 --- a/pkg/query-service/model/v3/v3.go +++ b/pkg/query-service/model/v3/v3.go @@ -253,11 +253,11 @@ type FilterAttributeKeyRequest struct { } type QBFilterSuggestionsRequest struct { - DataSource DataSource `json:"dataSource"` - SearchText string `json:"searchText"` - Limit int `json:"limit"` - ExamplesLimit int `json:"examplesLimit"` - ExistingFilter *FilterSet `json:"existingFilter"` + DataSource DataSource `json:"dataSource"` + SearchText string `json:"searchText"` + ExistingFilter *FilterSet `json:"existingFilter"` + AttributesLimit uint64 `json:"attributesLimit"` + ExamplesLimit uint64 `json:"examplesLimit"` } type QBFilterSuggestionsResponse struct { diff --git a/pkg/query-service/tests/integration/filter_suggestions_test.go b/pkg/query-service/tests/integration/filter_suggestions_test.go index 8dc199d2c5..05a9a122aa 100644 --- a/pkg/query-service/tests/integration/filter_suggestions_test.go +++ b/pkg/query-service/tests/integration/filter_suggestions_test.go @@ -158,7 +158,7 @@ func (tb *FilterSuggestionsTestBed) mockAttribKeysQueryResponse( tb.mockClickhouse.ExpectQuery( "select.*from.*signoz_logs.distributed_tag_attributes.*", ).WithArgs( - constants.DefaultFilterSuggestionsLimit, + constants.DefaultFilterSuggestionsAttributesLimit, ).WillReturnRows( mockhouse.NewRows(cols, values), ) From 0805e983bdbdafa3e4a53948e336a58ea81678d9 Mon Sep 17 00:00:00 2001 From: Raj Date: Tue, 3 Sep 2024 20:06:21 +0530 Subject: [PATCH 10/12] chore: some test cleanup --- .../integration/filter_suggestions_test.go | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/pkg/query-service/tests/integration/filter_suggestions_test.go b/pkg/query-service/tests/integration/filter_suggestions_test.go index 05a9a122aa..83951f3c12 100644 --- a/pkg/query-service/tests/integration/filter_suggestions_test.go +++ b/pkg/query-service/tests/integration/filter_suggestions_test.go @@ -118,7 +118,7 @@ func TestLogsFilterSuggestionsWithExistingFilter(t *testing.T) { tb.mockAttribKeysQueryResponse([]v3.AttributeKey{testAttrib, testFilterAttrib}) tb.mockAttribValuesQueryResponse( - []v3.AttributeKey{testAttrib}, [][]string{[]string{testAttribValue}}, + []v3.AttributeKey{testAttrib}, [][]string{{testAttribValue}}, ) testFilterJson, err := json.Marshal(testFilter) @@ -176,28 +176,29 @@ func (tb *FilterSuggestionsTestBed) mockAttribKeysQueryResponse( // Mocks response for CH queries made by reader.GetLogAttributeValues func (tb *FilterSuggestionsTestBed) mockAttribValuesQueryResponse( expectedAttribs []v3.AttributeKey, - expectedStringValues [][]string, + stringValuesToReturn [][]string, ) { - cols := []mockhouse.ColumnType{} - cols = append(cols, mockhouse.ColumnType{Type: "String", Name: "tagKey"}) - cols = append(cols, mockhouse.ColumnType{Type: "String", Name: "stringTagValue"}) - cols = append(cols, mockhouse.ColumnType{Type: "Nullable(Int64)", Name: "int64TagValue"}) - cols = append(cols, mockhouse.ColumnType{Type: "Nullable(Float64)", Name: "float64TagValue"}) + resultCols := []mockhouse.ColumnType{ + {Type: "String", Name: "tagKey"}, + {Type: "String", Name: "stringTagValue"}, + {Type: "Nullable(Int64)", Name: "int64TagValue"}, + {Type: "Nullable(Float64)", Name: "float64TagValue"}, + } - expectedAttribKeys := []string{} - values := [][]any{} + expectedAttribKeysInQuery := []string{} + mockResultRows := [][]any{} for idx, attrib := range expectedAttribs { - expectedAttribKeys = append(expectedAttribKeys, attrib.Key) - for _, val := range expectedStringValues[idx] { - rowValues := []any{} - rowValues = append(rowValues, attrib.Key, val, nil, nil) - values = append(values, rowValues) + expectedAttribKeysInQuery = append(expectedAttribKeysInQuery, attrib.Key) + for _, stringTagValue := range stringValuesToReturn[idx] { + mockResultRows = append(mockResultRows, []any{ + attrib.Key, stringTagValue, nil, nil, + }) } } tb.mockClickhouse.ExpectQuery( "select.*tagKey.*stringTagValue.*int64TagValue.*float64TagValue.*distributed_tag_attributes.*tagKey.*in.*", - ).WithArgs(expectedAttribKeys).WillReturnRows(mockhouse.NewRows(cols, values)) + ).WithArgs(expectedAttribKeysInQuery).WillReturnRows(mockhouse.NewRows(resultCols, mockResultRows)) } type FilterSuggestionsTestBed struct { From 550870adb9661b09c91c89380457413b5c7b7669 Mon Sep 17 00:00:00 2001 From: Raj Date: Tue, 3 Sep 2024 20:38:08 +0530 Subject: [PATCH 11/12] chore: some more cleanup --- .../app/clickhouseReader/reader.go | 68 +++++++++---------- pkg/query-service/app/parser.go | 5 +- 2 files changed, 35 insertions(+), 38 deletions(-) diff --git a/pkg/query-service/app/clickhouseReader/reader.go b/pkg/query-service/app/clickhouseReader/reader.go index 2ff4e56a00..1c2fb706a9 100644 --- a/pkg/query-service/app/clickhouseReader/reader.go +++ b/pkg/query-service/app/clickhouseReader/reader.go @@ -4458,58 +4458,56 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( } } - // Suggest example query for top suggested attribute using existing - // autocomplete logic for recommending attrib values - // - // Example queries for multiple top attributes using a batch version of - // GetLogAttributeValues is expected to come in a follow up change + // Suggest example queries for top suggested attributes if len(suggestions.AttributeKeys) > 0 { - // suggest 2 examples for top 2 attribs - topAttribs := []v3.AttributeKey{} + exampleAttribs := []v3.AttributeKey{} for _, attrib := range suggestions.AttributeKeys { - if len(topAttribs) >= int(req.ExamplesLimit) { - break - } + // only suggest examples for actual log attributes or resource attributes + isAttributeOrResource := slices.Contains([]v3.AttributeKeyType{ + v3.AttributeKeyTypeResource, v3.AttributeKeyTypeTag, + }, attrib.Type) - // only suggest examples for actual attributes - _, isTopLevelField := constants.StaticFieldsLogsV3[attrib.Key] isNumOrStringType := slices.Contains([]v3.AttributeKeyDataType{ v3.AttributeKeyDataTypeInt64, v3.AttributeKeyDataTypeFloat64, v3.AttributeKeyDataTypeString, }, attrib.DataType) - if !isTopLevelField && isNumOrStringType { - topAttribs = append(topAttribs, attrib) + if isAttributeOrResource && isNumOrStringType { + exampleAttribs = append(exampleAttribs, attrib) + } + + if len(exampleAttribs) >= int(req.ExamplesLimit) { + break } } - numValuesPerAttrib := math.Ceil(float64(req.ExamplesLimit) / float64(len(topAttribs))) + valuesToQueryPerAttrib := math.Ceil(float64(req.ExamplesLimit) / float64(len(exampleAttribs))) - topAttribValues, err := r.getValuesForLogAttributes( - ctx, topAttribs, uint64(numValuesPerAttrib), + exampleAttribValues, err := r.getValuesForLogAttributes( + ctx, exampleAttribs, uint64(valuesToQueryPerAttrib), ) - if err != nil { // Do not fail the entire request if only example query generation fails zap.L().Error("could not find attribute values for creating example query", zap.Error(err)) } else { - // TODO(Raj): Clean this up - for valueIdx := 0; valueIdx < int(numValuesPerAttrib); valueIdx++ { - for attrIdx, topAttr := range topAttribs { - if len(suggestions.ExampleQueries) < int(req.ExamplesLimit) { - attrValues := topAttribValues[attrIdx] - if valueIdx < len(attrValues) { - exampleQuery := newExampleQuery() - exampleQuery.Items = append(exampleQuery.Items, v3.FilterItem{ - Key: topAttr, - Operator: "=", - Value: attrValues[valueIdx], - }) - - suggestions.ExampleQueries = append( - suggestions.ExampleQueries, exampleQuery, - ) - } + // add example queries for as many attributes as possible. + // suggest 1st value for 1st attrib, followed by 1st value for second attrib and so on + // and if there is still room, suggest 2nd value for 1st attrib, 2nd value for 2nd attrib and so on + for valueIdx := 0; valueIdx < int(valuesToQueryPerAttrib); valueIdx++ { + for attrIdx, attr := range exampleAttribs { + needMoreExamples := len(suggestions.ExampleQueries) < int(req.ExamplesLimit) + + if needMoreExamples && valueIdx < len(exampleAttribValues[attrIdx]) { + exampleQuery := newExampleQuery() + exampleQuery.Items = append(exampleQuery.Items, v3.FilterItem{ + Key: attr, + Operator: "=", + Value: exampleAttribValues[attrIdx][valueIdx], + }) + + suggestions.ExampleQueries = append( + suggestions.ExampleQueries, exampleQuery, + ) } } } diff --git a/pkg/query-service/app/parser.go b/pkg/query-service/app/parser.go index a8b08e5ad7..6e63d12679 100644 --- a/pkg/query-service/app/parser.go +++ b/pkg/query-service/app/parser.go @@ -858,7 +858,6 @@ func parseQBFilterSuggestionsRequest(r *http.Request) ( } } return value, nil - } attributesLimit, err := parsePositiveIntQP( @@ -894,10 +893,10 @@ func parseQBFilterSuggestionsRequest(r *http.Request) ( return &v3.QBFilterSuggestionsRequest{ DataSource: dataSource, - AttributesLimit: attributesLimit, SearchText: searchText, - ExamplesLimit: examplesLimit, ExistingFilter: existingFilter, + AttributesLimit: attributesLimit, + ExamplesLimit: examplesLimit, }, nil } From ae1fb55b934293c614a33e761b32fe60fc5b14ef Mon Sep 17 00:00:00 2001 From: Raj Date: Wed, 4 Sep 2024 10:46:54 +0530 Subject: [PATCH 12/12] chore: some more cleanup --- .../app/clickhouseReader/reader.go | 67 ++++++++++--------- pkg/query-service/app/parser.go | 17 +++-- pkg/query-service/constants/constants.go | 4 +- .../integration/filter_suggestions_test.go | 3 +- 4 files changed, 53 insertions(+), 38 deletions(-) diff --git a/pkg/query-service/app/clickhouseReader/reader.go b/pkg/query-service/app/clickhouseReader/reader.go index 1c2fb706a9..bac50ca157 100644 --- a/pkg/query-service/app/clickhouseReader/reader.go +++ b/pkg/query-service/app/clickhouseReader/reader.go @@ -4458,32 +4458,29 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( } } - // Suggest example queries for top suggested attributes - if len(suggestions.AttributeKeys) > 0 { - exampleAttribs := []v3.AttributeKey{} - for _, attrib := range suggestions.AttributeKeys { - // only suggest examples for actual log attributes or resource attributes - isAttributeOrResource := slices.Contains([]v3.AttributeKeyType{ - v3.AttributeKeyTypeResource, v3.AttributeKeyTypeTag, - }, attrib.Type) - - isNumOrStringType := slices.Contains([]v3.AttributeKeyDataType{ - v3.AttributeKeyDataTypeInt64, v3.AttributeKeyDataTypeFloat64, v3.AttributeKeyDataTypeString, - }, attrib.DataType) - - if isAttributeOrResource && isNumOrStringType { - exampleAttribs = append(exampleAttribs, attrib) - } + // Suggest example queries for top suggested log attributes and resource attributes + exampleAttribs := []v3.AttributeKey{} + for _, attrib := range suggestions.AttributeKeys { + isAttributeOrResource := slices.Contains([]v3.AttributeKeyType{ + v3.AttributeKeyTypeResource, v3.AttributeKeyTypeTag, + }, attrib.Type) - if len(exampleAttribs) >= int(req.ExamplesLimit) { - break - } + isNumOrStringType := slices.Contains([]v3.AttributeKeyDataType{ + v3.AttributeKeyDataTypeInt64, v3.AttributeKeyDataTypeFloat64, v3.AttributeKeyDataTypeString, + }, attrib.DataType) + + if isAttributeOrResource && isNumOrStringType { + exampleAttribs = append(exampleAttribs, attrib) } - valuesToQueryPerAttrib := math.Ceil(float64(req.ExamplesLimit) / float64(len(exampleAttribs))) + if len(exampleAttribs) >= int(req.ExamplesLimit) { + break + } + } + if len(exampleAttribs) > 0 { exampleAttribValues, err := r.getValuesForLogAttributes( - ctx, exampleAttribs, uint64(valuesToQueryPerAttrib), + ctx, exampleAttribs, req.ExamplesLimit, ) if err != nil { // Do not fail the entire request if only example query generation fails @@ -4493,7 +4490,7 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( // add example queries for as many attributes as possible. // suggest 1st value for 1st attrib, followed by 1st value for second attrib and so on // and if there is still room, suggest 2nd value for 1st attrib, 2nd value for 2nd attrib and so on - for valueIdx := 0; valueIdx < int(valuesToQueryPerAttrib); valueIdx++ { + for valueIdx := 0; valueIdx < int(req.ExamplesLimit); valueIdx++ { for attrIdx, attr := range exampleAttribs { needMoreExamples := len(suggestions.ExampleQueries) < int(req.ExamplesLimit) @@ -4533,9 +4530,13 @@ func (r *ClickHouseReader) GetQBFilterSuggestionsForLogs( return &suggestions, nil } -func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attributes []v3.AttributeKey, limit uint64) ( - [][]any, *model.ApiError, -) { +// Get up to `limit` values seen for each attribute in `attributes` +// Returns a slice of slices where the ith slice has values for ith entry in `attributes` +func (r *ClickHouseReader) getValuesForLogAttributes( + ctx context.Context, attributes []v3.AttributeKey, limit uint64, +) ([][]any, *model.ApiError) { + // query top `limit` distinct values seen for `tagKey`s of interest + // ordered by timestamp when the value was seen query := fmt.Sprintf( ` select tagKey, stringTagValue, int64TagValue, float64TagValue @@ -4579,19 +4580,20 @@ func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attrib result := make([][]any, len(attributes)) - // Helper for getting hold of the result to populate for each scanned row - + // Helper for getting hold of the result slice to append to for each scanned row resultIdxForAttrib := func(key string, dataType v3.AttributeKeyDataType) int { return slices.IndexFunc(attributes, func(attrib v3.AttributeKey) bool { return attrib.Key == key && attrib.DataType == dataType }) } - var tagKey string - var stringValue string - var float64Value sql.NullFloat64 - var int64Value sql.NullInt64 + // Scan rows and append to result for rows.Next() { + var tagKey string + var stringValue string + var float64Value sql.NullFloat64 + var int64Value sql.NullInt64 + err := rows.Scan( &tagKey, &stringValue, &int64Value, &float64Value, ) @@ -4602,16 +4604,17 @@ func (r *ClickHouseReader) getValuesForLogAttributes(ctx context.Context, attrib } if len(stringValue) > 0 { - attrResultIdx := resultIdxForAttrib(tagKey, v3.AttributeKeyDataTypeString) if attrResultIdx >= 0 { result[attrResultIdx] = append(result[attrResultIdx], stringValue) } + } else if int64Value.Valid { attrResultIdx := resultIdxForAttrib(tagKey, v3.AttributeKeyDataTypeInt64) if attrResultIdx >= 0 { result[attrResultIdx] = append(result[attrResultIdx], int64Value.Int64) } + } else if float64Value.Valid { attrResultIdx := resultIdxForAttrib(tagKey, v3.AttributeKeyDataTypeFloat64) if attrResultIdx >= 0 { diff --git a/pkg/query-service/app/parser.go b/pkg/query-service/app/parser.go index 6e63d12679..bbf97c7adf 100644 --- a/pkg/query-service/app/parser.go +++ b/pkg/query-service/app/parser.go @@ -846,29 +846,38 @@ func parseQBFilterSuggestionsRequest(r *http.Request) ( return nil, model.BadRequest(err) } - parsePositiveIntQP := func(queryParam string, defaultValue uint64) (uint64, *model.ApiError) { + parsePositiveIntQP := func( + queryParam string, defaultValue uint64, maxValue uint64, + ) (uint64, *model.ApiError) { value := defaultValue + qpValue := r.URL.Query().Get(queryParam) if len(qpValue) > 0 { value, err := strconv.Atoi(qpValue) - if err != nil || value < 1 { + + if err != nil || value < 1 || value > int(maxValue) { return 0, model.BadRequest(fmt.Errorf( "invalid %s: %s", queryParam, qpValue, )) } } + return value, nil } attributesLimit, err := parsePositiveIntQP( - "attributesLimit", baseconstants.DefaultFilterSuggestionsAttributesLimit, + "attributesLimit", + baseconstants.DefaultFilterSuggestionsAttributesLimit, + baseconstants.MaxFilterSuggestionsAttributesLimit, ) if err != nil { return nil, err } examplesLimit, err := parsePositiveIntQP( - "examplesLimit", baseconstants.DefaultFilterSuggestionsExamplesLimit, + "examplesLimit", + baseconstants.DefaultFilterSuggestionsExamplesLimit, + baseconstants.MaxFilterSuggestionsExamplesLimit, ) if err != nil { return nil, err diff --git a/pkg/query-service/constants/constants.go b/pkg/query-service/constants/constants.go index 34ff35ef92..8c8a038f2f 100644 --- a/pkg/query-service/constants/constants.go +++ b/pkg/query-service/constants/constants.go @@ -417,5 +417,7 @@ var TracesListViewDefaultSelectedColumns = []v3.AttributeKey{ }, } -const DefaultFilterSuggestionsAttributesLimit = 100 +const DefaultFilterSuggestionsAttributesLimit = 50 +const MaxFilterSuggestionsAttributesLimit = 100 const DefaultFilterSuggestionsExamplesLimit = 2 +const MaxFilterSuggestionsExamplesLimit = 10 diff --git a/pkg/query-service/tests/integration/filter_suggestions_test.go b/pkg/query-service/tests/integration/filter_suggestions_test.go index 83951f3c12..6859a6ac2f 100644 --- a/pkg/query-service/tests/integration/filter_suggestions_test.go +++ b/pkg/query-service/tests/integration/filter_suggestions_test.go @@ -118,7 +118,8 @@ func TestLogsFilterSuggestionsWithExistingFilter(t *testing.T) { tb.mockAttribKeysQueryResponse([]v3.AttributeKey{testAttrib, testFilterAttrib}) tb.mockAttribValuesQueryResponse( - []v3.AttributeKey{testAttrib}, [][]string{{testAttribValue}}, + []v3.AttributeKey{testAttrib, testFilterAttrib}, + [][]string{{testAttribValue}, {testFilterAttribValue}}, ) testFilterJson, err := json.Marshal(testFilter)