From 85cf4f4e2ef6cfc69c912e567cbf03b2efda647c Mon Sep 17 00:00:00 2001 From: Nityananda Gohain Date: Wed, 18 Dec 2024 21:07:31 +0700 Subject: [PATCH] fix: use placehold in limit and use proper exists (#6667) --- pkg/query-service/app/querier/helper.go | 2 +- pkg/query-service/app/querier/v2/helper.go | 2 +- pkg/query-service/app/traces/v4/enrich.go | 4 ++-- .../app/traces/v4/query_builder.go | 19 ++++++++++++++++--- .../app/traces/v4/query_builder_test.go | 8 +++++--- 5 files changed, 25 insertions(+), 10 deletions(-) diff --git a/pkg/query-service/app/querier/helper.go b/pkg/query-service/app/querier/helper.go index 1b2acbab8b..a82b3f815b 100644 --- a/pkg/query-service/app/querier/helper.go +++ b/pkg/query-service/app/querier/helper.go @@ -190,7 +190,7 @@ func (q *querier) runBuilderQuery( ch <- channelResult{Err: err, Name: queryName, Query: limitQuery, Series: nil} return } - query = fmt.Sprintf(placeholderQuery, limitQuery) + query = strings.Replace(placeholderQuery, "#LIMIT_PLACEHOLDER", limitQuery, 1) } else { query, err = tracesQueryBuilder( start, diff --git a/pkg/query-service/app/querier/v2/helper.go b/pkg/query-service/app/querier/v2/helper.go index b62fffe106..4846aa3f3b 100644 --- a/pkg/query-service/app/querier/v2/helper.go +++ b/pkg/query-service/app/querier/v2/helper.go @@ -190,7 +190,7 @@ func (q *querier) runBuilderQuery( ch <- channelResult{Err: err, Name: queryName, Query: limitQuery, Series: nil} return } - query = fmt.Sprintf(placeholderQuery, limitQuery) + query = strings.Replace(placeholderQuery, "#LIMIT_PLACEHOLDER", limitQuery, 1) } else { query, err = tracesQueryBuilder( start, diff --git a/pkg/query-service/app/traces/v4/enrich.go b/pkg/query-service/app/traces/v4/enrich.go index 848e489e86..c0974ea0b0 100644 --- a/pkg/query-service/app/traces/v4/enrich.go +++ b/pkg/query-service/app/traces/v4/enrich.go @@ -34,8 +34,8 @@ func enrichKeyWithMetadata(key v3.AttributeKey, keys map[string]v3.AttributeKey) return v } - for _, key := range utils.GenerateEnrichmentKeys(key) { - if val, ok := keys[key]; ok { + for _, tkey := range utils.GenerateEnrichmentKeys(key) { + if val, ok := keys[tkey]; ok { return val } } diff --git a/pkg/query-service/app/traces/v4/query_builder.go b/pkg/query-service/app/traces/v4/query_builder.go index d650e1cdaa..80d39b9016 100644 --- a/pkg/query-service/app/traces/v4/query_builder.go +++ b/pkg/query-service/app/traces/v4/query_builder.go @@ -74,6 +74,19 @@ func getSelectLabels(groupBy []v3.AttributeKey) string { return strings.Join(labels, ",") } +// TODO(nitya): use the _exists columns as well in the future similar to logs +func existsSubQueryForFixedColumn(key v3.AttributeKey, op v3.FilterOperator) (string, error) { + if key.DataType == v3.AttributeKeyDataTypeString { + if op == v3.FilterOperatorExists { + return fmt.Sprintf("%s %s ''", getColumnName(key), tracesOperatorMappingV3[v3.FilterOperatorNotEqual]), nil + } else { + return fmt.Sprintf("%s %s ''", getColumnName(key), tracesOperatorMappingV3[v3.FilterOperatorEqual]), nil + } + } else { + return "", fmt.Errorf("unsupported operation, exists and not exists can only be applied on custom attributes or string type columns") + } +} + func buildTracesFilterQuery(fs *v3.FilterSet) (string, error) { var conditions []string @@ -110,7 +123,7 @@ func buildTracesFilterQuery(fs *v3.FilterSet) (string, error) { conditions = append(conditions, fmt.Sprintf(operator, columnName, fmtVal)) case v3.FilterOperatorExists, v3.FilterOperatorNotExists: if item.Key.IsColumn { - subQuery, err := tracesV3.ExistsSubQueryForFixedColumn(item.Key, item.Operator) + subQuery, err := existsSubQueryForFixedColumn(item.Key, item.Operator) if err != nil { return "", err } @@ -312,7 +325,7 @@ func buildTracesQuery(start, end, step int64, mq *v3.BuilderQuery, panelType v3. } if options.GraphLimitQtype == constants.SecondQueryGraphLimit { - filterSubQuery = filterSubQuery + " AND " + fmt.Sprintf("(%s) GLOBAL IN (", tracesV3.GetSelectKeys(mq.AggregateOperator, mq.GroupBy)) + "%s)" + filterSubQuery = filterSubQuery + " AND " + fmt.Sprintf("(%s) GLOBAL IN (", tracesV3.GetSelectKeys(mq.AggregateOperator, mq.GroupBy)) + "#LIMIT_PLACEHOLDER)" } switch mq.AggregateOperator { @@ -350,7 +363,7 @@ func buildTracesQuery(start, end, step int64, mq *v3.BuilderQuery, panelType v3. case v3.AggregateOperatorCount: if mq.AggregateAttribute.Key != "" { if mq.AggregateAttribute.IsColumn { - subQuery, err := tracesV3.ExistsSubQueryForFixedColumn(mq.AggregateAttribute, v3.FilterOperatorExists) + subQuery, err := existsSubQueryForFixedColumn(mq.AggregateAttribute, v3.FilterOperatorExists) if err == nil { filterSubQuery = fmt.Sprintf("%s AND %s", filterSubQuery, subQuery) } diff --git a/pkg/query-service/app/traces/v4/query_builder_test.go b/pkg/query-service/app/traces/v4/query_builder_test.go index 9db2e815f9..a9b055b67f 100644 --- a/pkg/query-service/app/traces/v4/query_builder_test.go +++ b/pkg/query-service/app/traces/v4/query_builder_test.go @@ -265,9 +265,11 @@ func Test_buildTracesFilterQuery(t *testing.T) { {Key: v3.AttributeKey{Key: "isDone", DataType: v3.AttributeKeyDataTypeBool, Type: v3.AttributeKeyTypeTag}, Operator: v3.FilterOperatorNotExists}, {Key: v3.AttributeKey{Key: "host1", DataType: v3.AttributeKeyDataTypeString, Type: v3.AttributeKeyTypeTag}, Operator: v3.FilterOperatorNotExists}, {Key: v3.AttributeKey{Key: "path", DataType: v3.AttributeKeyDataTypeString, Type: v3.AttributeKeyTypeTag, IsColumn: true}, Operator: v3.FilterOperatorNotExists}, + {Key: v3.AttributeKey{Key: "http_url", DataType: v3.AttributeKeyDataTypeString, Type: v3.AttributeKeyTypeTag, IsColumn: true}, Operator: v3.FilterOperatorNotExists}, + {Key: v3.AttributeKey{Key: "http.route", DataType: v3.AttributeKeyDataTypeString, Type: v3.AttributeKeyTypeTag, IsColumn: true}, Operator: v3.FilterOperatorNotExists}, }}, }, - want: "mapContains(attributes_string, 'host') AND mapContains(attributes_number, 'duration') AND NOT mapContains(attributes_bool, 'isDone') AND NOT mapContains(attributes_string, 'host1') AND path = ''", + want: "mapContains(attributes_string, 'host') AND mapContains(attributes_number, 'duration') AND NOT mapContains(attributes_bool, 'isDone') AND NOT mapContains(attributes_string, 'host1') AND `attribute_string_path` = '' AND http_url = '' AND `attribute_string_http$$route` = ''", }, } for _, tt := range tests { @@ -683,7 +685,7 @@ func TestPrepareTracesQuery(t *testing.T) { }, }, want: "SELECT attributes_string['function'] as `function`, toFloat64(count(distinct(name))) as value from signoz_traces.distributed_signoz_index_v3 where " + - "(timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') AND (ts_bucket_start >= 1680064560 AND ts_bucket_start <= 1680066458) AND mapContains(attributes_string, 'function') AND (`function`) GLOBAL IN (%s) group by `function` order by value DESC", + "(timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') AND (ts_bucket_start >= 1680064560 AND ts_bucket_start <= 1680066458) AND mapContains(attributes_string, 'function') AND (`function`) GLOBAL IN (#LIMIT_PLACEHOLDER) group by `function` order by value DESC", }, { name: "test with limit with resources- first", @@ -766,7 +768,7 @@ func TestPrepareTracesQuery(t *testing.T) { want: "SELECT `attribute_string_function` as `function`, serviceName as `serviceName`, toFloat64(count(distinct(name))) as value from signoz_traces.distributed_signoz_index_v3 " + "where (timestamp >= '1680066360726210000' AND timestamp <= '1680066458000000000') AND (ts_bucket_start >= 1680064560 AND ts_bucket_start <= 1680066458) AND attributes_number['line'] = 100 " + "AND (resource_fingerprint GLOBAL IN (SELECT fingerprint FROM signoz_traces.distributed_traces_v3_resource WHERE (seen_at_ts_bucket_start >= 1680064560) AND (seen_at_ts_bucket_start <= 1680066458) " + - "AND simpleJSONExtractString(labels, 'hostname') = 'server1' AND labels like '%hostname%server1%')) AND (`function`,`serviceName`) GLOBAL IN (%s) group by `function`,`serviceName` order by value DESC", + "AND simpleJSONExtractString(labels, 'hostname') = 'server1' AND labels like '%hostname%server1%')) AND (`function`,`serviceName`) GLOBAL IN (#LIMIT_PLACEHOLDER) group by `function`,`serviceName` order by value DESC", }, }