fix: minor issues found in development and enhancements (#2542)

* temp commit * chore: add setpInterval * chore: update prepare func type
2025-08-14 04:26:02 +08:00 · 2023-04-07 09:46:21 +05:30 · 2023-04-07 09:46:21 +05:30 · 4db109cbad
commit 4db109cbad
parent d09290528f
7 changed files with 115 additions and 48 deletions
--- a/pkg/query-service/app/clickhouseReader/reader.go
+++ b/pkg/query-service/app/clickhouseReader/reader.go
@ -3714,7 +3714,8 @@ func (r *ClickHouseReader) GetMetricAggregateAttributes(ctx context.Context, req
 		key := v3.AttributeKey{
 			Key:      metricName,
 			DataType: v3.AttributeKeyDataTypeFloat64,
-			Type:     v3.AttributeKeyTypeTag,
+			Type:     v3.AttributeKeyTypeUnspecified,
+			IsColumn: true,
 		}
 		response.AttributeKeys = append(response.AttributeKeys, key)
 	}
@ -3750,6 +3751,7 @@ func (r *ClickHouseReader) GetMetricAttributeKeys(ctx context.Context, req *v3.F
 			Key:      attributeKey,
 			DataType: v3.AttributeKeyDataTypeString, // https://github.com/OpenObservability/OpenMetrics/blob/main/proto/openmetrics_data_model.proto#L64-L72.
 			Type:     v3.AttributeKeyTypeTag,
+			IsColumn: false,
 		}
 		response.AttributeKeys = append(response.AttributeKeys, key)
 	}
--- a/pkg/query-service/app/http_handler.go
+++ b/pkg/query-service/app/http_handler.go
@ -106,10 +106,10 @@ func NewAPIHandler(opts APIHandlerOpts) (*APIHandler, error) {

 	builderOpts := queryBuilderOptions{
 		BuildMetricQuery: metricsv3.PrepareMetricQuery,
-		BuildTraceQuery: func(start, end, step int64, queryType v3.QueryType, panelType v3.PanelType, bq *v3.BuilderQuery) (string, error) {
+		BuildTraceQuery: func(start, end int64, queryType v3.QueryType, panelType v3.PanelType, bq *v3.BuilderQuery) (string, error) {
 			return "", errors.New("not implemented")
 		},
-		BuildLogQuery: func(start, end, step int64, queryType v3.QueryType, panelType v3.PanelType, bq *v3.BuilderQuery) (string, error) {
+		BuildLogQuery: func(start, end int64, queryType v3.QueryType, panelType v3.PanelType, bq *v3.BuilderQuery) (string, error) {
 			return "", errors.New("not implemented")
 		},
 	}
--- a/pkg/query-service/app/metrics/v3/query_builder.go
+++ b/pkg/query-service/app/metrics/v3/query_builder.go
@ -10,7 +10,7 @@ import (
 )

 var aggregateOperatorToPercentile = map[v3.AggregateOperator]float64{
-	v3.AggregateOperatorP05:         0.5,
+	v3.AggregateOperatorP05:         0.05,
 	v3.AggregateOperatorP10:         0.10,
 	v3.AggregateOperatorP20:         0.20,
 	v3.AggregateOperatorP25:         0.25,
@ -49,9 +49,10 @@ func buildMetricsTimeSeriesFilterQuery(fs *v3.FilterSet, groupTags []v3.Attribut
 	if fs != nil && len(fs.Items) != 0 {
 		for _, item := range fs.Items {
 			toFormat := item.Value
-			op := strings.ToLower(strings.TrimSpace(item.Operator))
+			op := v3.FilterOperator(strings.ToLower(strings.TrimSpace(string(item.Operator))))
 			// if the received value is an array for like/match op, just take the first value
-			if op == "like" || op == "match" || op == "nlike" || op == "nmatch" {
+			// or should we throw an error?
+			if op == v3.FilterOperatorLike || op == v3.FilterOperatorRegex || op == v3.FilterOperatorNotLike || op == v3.FilterOperatorNotRegex {
 				x, ok := item.Value.([]interface{})
 				if ok {
 					if len(x) == 0 {
@ -62,37 +63,37 @@ func buildMetricsTimeSeriesFilterQuery(fs *v3.FilterSet, groupTags []v3.Attribut
 			}
 			fmtVal := utils.ClickHouseFormattedValue(toFormat)
 			switch op {
-			case "eq":
+			case v3.FilterOperatorEqual:
 				conditions = append(conditions, fmt.Sprintf("JSONExtractString(labels, '%s') = %s", item.Key.Key, fmtVal))
-			case "neq":
+			case v3.FilterOperatorNotEqual:
 				conditions = append(conditions, fmt.Sprintf("JSONExtractString(labels, '%s') != %s", item.Key.Key, fmtVal))
-			case "in":
+			case v3.FilterOperatorIn:
 				conditions = append(conditions, fmt.Sprintf("JSONExtractString(labels, '%s') IN %s", item.Key.Key, fmtVal))
-			case "nin":
+			case v3.FilterOperatorNotIn:
 				conditions = append(conditions, fmt.Sprintf("JSONExtractString(labels, '%s') NOT IN %s", item.Key.Key, fmtVal))
-			case "like":
+			case v3.FilterOperatorLike:
 				conditions = append(conditions, fmt.Sprintf("like(JSONExtractString(labels, '%s'), %s)", item.Key.Key, fmtVal))
-			case "nlike":
+			case v3.FilterOperatorNotLike:
 				conditions = append(conditions, fmt.Sprintf("notLike(JSONExtractString(labels, '%s'), %s)", item.Key.Key, fmtVal))
-			case "match":
+			case v3.FilterOperatorRegex:
 				conditions = append(conditions, fmt.Sprintf("match(JSONExtractString(labels, '%s'), %s)", item.Key.Key, fmtVal))
-			case "nmatch":
+			case v3.FilterOperatorNotRegex:
 				conditions = append(conditions, fmt.Sprintf("not match(JSONExtractString(labels, '%s'), %s)", item.Key.Key, fmtVal))
-			case "gt":
+			case v3.FilterOperatorGreaterThan:
 				conditions = append(conditions, fmt.Sprintf("JSONExtractString(labels, '%s') > %s", item.Key.Key, fmtVal))
-			case "gte":
+			case v3.FilterOperatorGreaterThanOrEq:
 				conditions = append(conditions, fmt.Sprintf("JSONExtractString(labels, '%s') >= %s", item.Key.Key, fmtVal))
-			case "lt":
+			case v3.FilterOperatorLessThan:
 				conditions = append(conditions, fmt.Sprintf("JSONExtractString(labels, '%s') < %s", item.Key.Key, fmtVal))
-			case "lte":
+			case v3.FilterOperatorLessThanOrEq:
 				conditions = append(conditions, fmt.Sprintf("JSONExtractString(labels, '%s') <= %s", item.Key.Key, fmtVal))
-			case "contains":
+			case v3.FilterOperatorContains:
 				conditions = append(conditions, fmt.Sprintf("like(JSONExtractString(labels, '%s'), %s)", item.Key.Key, fmtVal))
-			case "ncontains":
+			case v3.FilterOperatorNotContains:
 				conditions = append(conditions, fmt.Sprintf("notLike(JSONExtractString(labels, '%s'), %s)", item.Key.Key, fmtVal))
-			case "exists":
+			case v3.FilterOperatorExists:
 				conditions = append(conditions, fmt.Sprintf("has(JSONExtractKeys(labels), %s)", item.Key.Key))
-			case "nexists":
+			case v3.FilterOperatorNotExists:
 				conditions = append(conditions, fmt.Sprintf("not has(JSONExtractKeys(labels), %s)", item.Key.Key))
 			default:
 				return "", fmt.Errorf("unsupported operation")
@ -117,7 +118,36 @@ func buildMetricsTimeSeriesFilterQuery(fs *v3.FilterSet, groupTags []v3.Attribut

 func buildMetricQuery(start, end, step int64, mq *v3.BuilderQuery, tableName string) (string, error) {

-	filterSubQuery, err := buildMetricsTimeSeriesFilterQuery(mq.Filters, mq.GroupBy, mq.AggregateAttribute.Key, mq.AggregateOperator)
+	metricQueryGroupBy := mq.GroupBy
+
+	// if the aggregate operator is a histogram quantile, and user has not forgotten
+	// the le tag in the group by then add the le tag to the group by
+	if mq.AggregateOperator == v3.AggregateOperatorHistQuant50 ||
+		mq.AggregateOperator == v3.AggregateOperatorHistQuant75 ||
+		mq.AggregateOperator == v3.AggregateOperatorHistQuant90 ||
+		mq.AggregateOperator == v3.AggregateOperatorHistQuant95 ||
+		mq.AggregateOperator == v3.AggregateOperatorHistQuant99 {
+		found := false
+		for _, tag := range mq.GroupBy {
+			if tag.Key == "le" {
+				found = true
+				break
+			}
+		}
+		if !found {
+			metricQueryGroupBy = append(
+				metricQueryGroupBy,
+				v3.AttributeKey{
+					Key:      "le",
+					DataType: v3.AttributeKeyDataTypeString,
+					Type:     v3.AttributeKeyTypeTag,
+					IsColumn: false,
+				},
+			)
+		}
+	}
+
+	filterSubQuery, err := buildMetricsTimeSeriesFilterQuery(mq.Filters, metricQueryGroupBy, mq.AggregateAttribute.Key, mq.AggregateOperator)
 	if err != nil {
 		return "", err
 	}
@ -151,18 +181,22 @@ func buildMetricQuery(start, end, step int64, mq *v3.BuilderQuery, tableName str
 	groupTagsWithoutLe := groupSelect(tagsWithoutLe...)
 	orderWithoutLe := orderBy(mq.OrderBy, tagsWithoutLe)

-	groupBy := groupByAttributeKeyTags(mq.GroupBy...)
-	groupTags := groupSelectAttributeKeyTags(mq.GroupBy...)
-	orderBy := orderByAttributeKeyTags(mq.OrderBy, mq.GroupBy)
+	groupBy := groupByAttributeKeyTags(metricQueryGroupBy...)
+	groupTags := groupSelectAttributeKeyTags(metricQueryGroupBy...)
+	orderBy := orderByAttributeKeyTags(mq.OrderBy, metricQueryGroupBy)

 	if len(orderBy) != 0 {
 		orderBy += ","
 	}
+	if len(orderWithoutLe) != 0 {
+		orderWithoutLe += ","
+	}

 	switch mq.AggregateOperator {
 	case v3.AggregateOperatorRate:
 		// Calculate rate of change of metric for each unique time series
 		groupBy = "fingerprint, ts"
+		orderBy = "fingerprint, "
 		groupTags = "fingerprint,"
 		op := "max(value)" // max value should be the closest value for point in time
 		subQuery := fmt.Sprintf(
@ -351,8 +385,8 @@ func reduceQuery(query string, reduceTo v3.ReduceToOperator, aggregateOperator v
 	return query, nil
 }

-func PrepareMetricQuery(start, end, step int64, queryType v3.QueryType, panelType v3.PanelType, mq *v3.BuilderQuery) (string, error) {
-	query, err := buildMetricQuery(start, end, step, mq, constants.SIGNOZ_TIMESERIES_TABLENAME)
+func PrepareMetricQuery(start, end int64, queryType v3.QueryType, panelType v3.PanelType, mq *v3.BuilderQuery) (string, error) {
+	query, err := buildMetricQuery(start, end, mq.StepInterval, mq, constants.SIGNOZ_TIMESERIES_TABLENAME)
 	if err != nil {
 		return "", err
 	}
--- a/pkg/query-service/app/metrics/v3/query_builder_test.go
+++ b/pkg/query-service/app/metrics/v3/query_builder_test.go
@ -26,7 +26,7 @@ func TestBuildQuery(t *testing.T) {
 				PanelType: v3.PanelTypeGraph,
 			},
 		}
-		query, err := PrepareMetricQuery(q.Start, q.End, q.Step, q.CompositeQuery.QueryType, q.CompositeQuery.PanelType, q.CompositeQuery.BuilderQueries["A"])
+		query, err := PrepareMetricQuery(q.Start, q.End, q.CompositeQuery.QueryType, q.CompositeQuery.PanelType, q.CompositeQuery.BuilderQueries["A"])
 		require.NoError(t, err)
 		require.Contains(t, query, "WHERE metric_name = 'name'")
 	})
@ -44,8 +44,8 @@ func TestBuildQueryWithFilters(t *testing.T) {
 						QueryName:          "A",
 						AggregateAttribute: v3.AttributeKey{Key: "name"},
 						Filters: &v3.FilterSet{Operator: "AND", Items: []v3.FilterItem{
-							{Key: v3.AttributeKey{Key: "a"}, Value: "b", Operator: "neq"},
-							{Key: v3.AttributeKey{Key: "code"}, Value: "ERROR_*", Operator: "nmatch"},
+							{Key: v3.AttributeKey{Key: "a"}, Value: "b", Operator: v3.FilterOperatorNotEqual},
+							{Key: v3.AttributeKey{Key: "code"}, Value: "ERROR_*", Operator: v3.FilterOperatorNotRegex},
 						}},
 						AggregateOperator: v3.AggregateOperatorRateMax,
 						Expression:        "A",
@ -53,7 +53,7 @@ func TestBuildQueryWithFilters(t *testing.T) {
 				},
 			},
 		}
-		query, err := PrepareMetricQuery(q.Start, q.End, q.Step, q.CompositeQuery.QueryType, q.CompositeQuery.PanelType, q.CompositeQuery.BuilderQueries["A"])
+		query, err := PrepareMetricQuery(q.Start, q.End, q.CompositeQuery.QueryType, q.CompositeQuery.PanelType, q.CompositeQuery.BuilderQueries["A"])
 		require.NoError(t, err)

 		require.Contains(t, query, "WHERE metric_name = 'name' AND JSONExtractString(labels, 'a') != 'b'")
@ -74,7 +74,7 @@ func TestBuildQueryWithMultipleQueries(t *testing.T) {
 						QueryName:          "A",
 						AggregateAttribute: v3.AttributeKey{Key: "name"},
 						Filters: &v3.FilterSet{Operator: "AND", Items: []v3.FilterItem{
-							{Key: v3.AttributeKey{Key: "in"}, Value: []interface{}{"a", "b", "c"}, Operator: "in"},
+							{Key: v3.AttributeKey{Key: "in"}, Value: []interface{}{"a", "b", "c"}, Operator: v3.FilterOperatorIn},
 						}},
 						AggregateOperator: v3.AggregateOperatorRateAvg,
 						Expression:        "A",
@ -89,7 +89,7 @@ func TestBuildQueryWithMultipleQueries(t *testing.T) {
 			},
 		}

-		query, err := PrepareMetricQuery(q.Start, q.End, q.Step, q.CompositeQuery.QueryType, q.CompositeQuery.PanelType, q.CompositeQuery.BuilderQueries["A"])
+		query, err := PrepareMetricQuery(q.Start, q.End, q.CompositeQuery.QueryType, q.CompositeQuery.PanelType, q.CompositeQuery.BuilderQueries["A"])
 		require.NoError(t, err)

 		require.Contains(t, query, "WHERE metric_name = 'name' AND JSONExtractString(labels, 'in') IN ['a','b','c']")
--- a/pkg/query-service/app/query_builder.go
+++ b/pkg/query-service/app/query_builder.go
@ -35,9 +35,9 @@ var SupportedFunctions = []string{

 var evalFuncs = map[string]govaluate.ExpressionFunction{}

-type prepareTracesQueryFunc func(start, end, step int64, queryType v3.QueryType, panelType v3.PanelType, bq *v3.BuilderQuery) (string, error)
-type prepareLogsQueryFunc func(start, end, step int64, queryType v3.QueryType, panelType v3.PanelType, bq *v3.BuilderQuery) (string, error)
-type prepareMetricQueryFunc func(start, end, step int64, queryType v3.QueryType, panelType v3.PanelType, bq *v3.BuilderQuery) (string, error)
+type prepareTracesQueryFunc func(start, end int64, queryType v3.QueryType, panelType v3.PanelType, bq *v3.BuilderQuery) (string, error)
+type prepareLogsQueryFunc func(start, end int64, queryType v3.QueryType, panelType v3.PanelType, bq *v3.BuilderQuery) (string, error)
+type prepareMetricQueryFunc func(start, end int64, queryType v3.QueryType, panelType v3.PanelType, bq *v3.BuilderQuery) (string, error)

 type queryBuilder struct {
 	options queryBuilderOptions
@ -139,19 +139,19 @@ func (qb *queryBuilder) prepareQueries(params *v3.QueryRangeParamsV3) (map[strin
 			if query.Expression == queryName {
 				switch query.DataSource {
 				case v3.DataSourceTraces:
-					queryString, err := qb.options.BuildTraceQuery(params.Start, params.End, params.Step, compositeQuery.QueryType, compositeQuery.PanelType, query)
+					queryString, err := qb.options.BuildTraceQuery(params.Start, params.End, compositeQuery.QueryType, compositeQuery.PanelType, query)
 					if err != nil {
 						return nil, err
 					}
 					queries[queryName] = queryString
 				case v3.DataSourceLogs:
-					queryString, err := qb.options.BuildLogQuery(params.Start, params.End, params.Step, compositeQuery.QueryType, compositeQuery.PanelType, query)
+					queryString, err := qb.options.BuildLogQuery(params.Start, params.End, compositeQuery.QueryType, compositeQuery.PanelType, query)
 					if err != nil {
 						return nil, err
 					}
 					queries[queryName] = queryString
 				case v3.DataSourceMetrics:
-					queryString, err := qb.options.BuildMetricQuery(params.Start, params.End, params.Step, compositeQuery.QueryType, compositeQuery.PanelType, query)
+					queryString, err := qb.options.BuildMetricQuery(params.Start, params.End, compositeQuery.QueryType, compositeQuery.PanelType, query)
 					if err != nil {
 						return nil, err
 					}
--- a/pkg/query-service/app/query_builder_test.go
+++ b/pkg/query-service/app/query_builder_test.go
@ -22,7 +22,7 @@ func TestBuildQueryWithMultipleQueriesAndFormula(t *testing.T) {
 						DataSource:         v3.DataSourceMetrics,
 						AggregateAttribute: v3.AttributeKey{Key: "name"},
 						Filters: &v3.FilterSet{Operator: "AND", Items: []v3.FilterItem{
-							{Key: v3.AttributeKey{Key: "in"}, Value: []interface{}{"a", "b", "c"}, Operator: "in"},
+							{Key: v3.AttributeKey{Key: "in"}, Value: []interface{}{"a", "b", "c"}, Operator: v3.FilterOperatorIn},
 						}},
 						AggregateOperator: v3.AggregateOperatorRateMax,
 						Expression:        "A",
@ -68,7 +68,7 @@ func TestBuildQueryWithIncorrectQueryRef(t *testing.T) {
 						DataSource:         v3.DataSourceMetrics,
 						AggregateAttribute: v3.AttributeKey{Key: "name"},
 						Filters: &v3.FilterSet{Operator: "AND", Items: []v3.FilterItem{
-							{Key: v3.AttributeKey{Key: "in"}, Value: []interface{}{"a", "b", "c"}, Operator: "in"},
+							{Key: v3.AttributeKey{Key: "in"}, Value: []interface{}{"a", "b", "c"}, Operator: v3.FilterOperatorIn},
 						}},
 						AggregateOperator: v3.AggregateOperatorRateMax,
 						Expression:        "A",
@ -105,7 +105,7 @@ func TestBuildQueryWithThreeOrMoreQueriesRefAndFormula(t *testing.T) {
 						DataSource:         v3.DataSourceMetrics,
 						AggregateAttribute: v3.AttributeKey{Key: "name"},
 						Filters: &v3.FilterSet{Operator: "AND", Items: []v3.FilterItem{
-							{Key: v3.AttributeKey{Key: "in"}, Value: []interface{}{"a", "b", "c"}, Operator: "in"},
+							{Key: v3.AttributeKey{Key: "in"}, Value: []interface{}{"a", "b", "c"}, Operator: v3.FilterOperatorIn},
 						}},
 						AggregateOperator: v3.AggregateOperatorRateMax,
 						Expression:        "A",
--- a/pkg/query-service/model/v3/v3.go
+++ b/pkg/query-service/model/v3/v3.go
@ -249,6 +249,7 @@ type FilterAttributeKeyResponse struct {
 type AttributeKeyType string

 const (
+	AttributeKeyTypeUnspecified AttributeKeyType = ""
 	AttributeKeyTypeTag         AttributeKeyType = "tag"
 	AttributeKeyTypeResource    AttributeKeyType = "resource"
 )
@ -387,6 +388,7 @@ func (c *CompositeQuery) Validate() error {

 type BuilderQuery struct {
 	QueryName          string            `json:"queryName"`
+	StepInterval       int64             `json:"stepInterval"`
 	DataSource         DataSource        `json:"dataSource"`
 	AggregateOperator  AggregateOperator `json:"aggregateOperator"`
 	AggregateAttribute AttributeKey      `json:"aggregateAttribute,omitempty"`
@ -436,6 +438,12 @@ func (b *BuilderQuery) Validate() error {
 				return fmt.Errorf("group by is invalid %w", err)
 			}
 		}
+
+		if b.DataSource == DataSourceMetrics && len(b.GroupBy) > 0 {
+			if b.AggregateOperator == AggregateOperatorNoOp || b.AggregateOperator == AggregateOperatorRate {
+				return fmt.Errorf("group by requires aggregate operator other than noop or rate")
+			}
+		}
 	}

 	if b.SelectColumns != nil {
@ -472,10 +480,33 @@ func (f *FilterSet) Validate() error {
 	return nil
 }

+type FilterOperator string
+
+const (
+	FilterOperatorEqual           FilterOperator = "="
+	FilterOperatorNotEqual        FilterOperator = "!="
+	FilterOperatorGreaterThan     FilterOperator = ">"
+	FilterOperatorGreaterThanOrEq FilterOperator = ">="
+	FilterOperatorLessThan        FilterOperator = "<"
+	FilterOperatorLessThanOrEq    FilterOperator = "<="
+	FilterOperatorIn              FilterOperator = "in"
+	FilterOperatorNotIn           FilterOperator = "nin"
+	FilterOperatorContains        FilterOperator = "contains"
+	FilterOperatorNotContains     FilterOperator = "ncontains"
+	FilterOperatorRegex           FilterOperator = "regex"
+	FilterOperatorNotRegex        FilterOperator = "nregex"
+	// (I)LIKE is faster than REGEX and supports index
+	FilterOperatorLike    FilterOperator = "like"
+	FilterOperatorNotLike FilterOperator = "nlike"
+
+	FilterOperatorExists    FilterOperator = "exists"
+	FilterOperatorNotExists FilterOperator = "nexists"
+)
+
 type FilterItem struct {
 	Key      AttributeKey   `json:"key"`
 	Value    interface{}    `json:"value"`
-	Operator string       `json:"op"`
+	Operator FilterOperator `json:"op"`
 }

 type OrderBy struct {