fix: resolve gaps identified in the query builder (#2680)

This commit is contained in:
Srikanth Chekuri 2023-05-12 16:25:22 +05:30 committed by GitHub
parent e8f2176566
commit df0502726d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 184 additions and 24 deletions

View File

@ -61,6 +61,10 @@ func buildMetricsTimeSeriesFilterQuery(fs *v3.FilterSet, groupTags []v3.Attribut
toFormat = x[0] toFormat = x[0]
} }
} }
if op == v3.FilterOperatorContains || op == v3.FilterOperatorNotContains {
toFormat = fmt.Sprintf("%%%s%%", toFormat)
}
fmtVal := utils.ClickHouseFormattedValue(toFormat) fmtVal := utils.ClickHouseFormattedValue(toFormat)
switch op { switch op {
case v3.FilterOperatorEqual: case v3.FilterOperatorEqual:
@ -92,9 +96,9 @@ func buildMetricsTimeSeriesFilterQuery(fs *v3.FilterSet, groupTags []v3.Attribut
case v3.FilterOperatorNotContains: case v3.FilterOperatorNotContains:
conditions = append(conditions, fmt.Sprintf("notLike(JSONExtractString(labels, '%s'), %s)", item.Key.Key, fmtVal)) conditions = append(conditions, fmt.Sprintf("notLike(JSONExtractString(labels, '%s'), %s)", item.Key.Key, fmtVal))
case v3.FilterOperatorExists: case v3.FilterOperatorExists:
conditions = append(conditions, fmt.Sprintf("has(JSONExtractKeys(labels), %s)", item.Key.Key)) conditions = append(conditions, fmt.Sprintf("has(JSONExtractKeys(labels), '%s')", item.Key.Key))
case v3.FilterOperatorNotExists: case v3.FilterOperatorNotExists:
conditions = append(conditions, fmt.Sprintf("not has(JSONExtractKeys(labels), %s)", item.Key.Key)) conditions = append(conditions, fmt.Sprintf("not has(JSONExtractKeys(labels), '%s')", item.Key.Key))
default: default:
return "", fmt.Errorf("unsupported operation") return "", fmt.Errorf("unsupported operation")
} }
@ -202,7 +206,7 @@ func buildMetricQuery(start, end, step int64, mq *v3.BuilderQuery, tableName str
subQuery := fmt.Sprintf( subQuery := fmt.Sprintf(
queryTmpl, "any(labels) as labels, "+groupTags, step, op, filterSubQuery, groupBy, orderBy, queryTmpl, "any(labels) as labels, "+groupTags, step, op, filterSubQuery, groupBy, orderBy,
) // labels will be same so any should be fine ) // labels will be same so any should be fine
query := `SELECT %s ts, ` + rateWithoutNegative + ` as value FROM(%s)` query := `SELECT %s ts, ` + rateWithoutNegative + ` as value FROM(%s) WHERE isNaN(value) = 0`
query = fmt.Sprintf(query, "labels as fullLabels,", subQuery) query = fmt.Sprintf(query, "labels as fullLabels,", subQuery)
return query, nil return query, nil
@ -214,7 +218,7 @@ func buildMetricQuery(start, end, step int64, mq *v3.BuilderQuery, tableName str
subQuery := fmt.Sprintf( subQuery := fmt.Sprintf(
queryTmpl, rateGroupTags, step, op, filterSubQuery, rateGroupBy, rateOrderBy, queryTmpl, rateGroupTags, step, op, filterSubQuery, rateGroupBy, rateOrderBy,
) // labels will be same so any should be fine ) // labels will be same so any should be fine
query := `SELECT %s ts, ` + rateWithoutNegative + `as value FROM(%s)` query := `SELECT %s ts, ` + rateWithoutNegative + `as value FROM(%s) WHERE isNaN(value) = 0`
query = fmt.Sprintf(query, groupTags, subQuery) query = fmt.Sprintf(query, groupTags, subQuery)
query = fmt.Sprintf(`SELECT %s ts, sum(value) as value FROM (%s) GROUP BY %s ORDER BY %s ts`, groupTags, query, groupBy, orderBy) query = fmt.Sprintf(`SELECT %s ts, sum(value) as value FROM (%s) GROUP BY %s ORDER BY %s ts`, groupTags, query, groupBy, orderBy)
return query, nil return query, nil
@ -225,7 +229,7 @@ func buildMetricQuery(start, end, step int64, mq *v3.BuilderQuery, tableName str
v3.AggregateOperatorRateMin: v3.AggregateOperatorRateMin:
op := fmt.Sprintf("%s(value)", aggregateOperatorToSQLFunc[mq.AggregateOperator]) op := fmt.Sprintf("%s(value)", aggregateOperatorToSQLFunc[mq.AggregateOperator])
subQuery := fmt.Sprintf(queryTmpl, groupTags, step, op, filterSubQuery, groupBy, orderBy) subQuery := fmt.Sprintf(queryTmpl, groupTags, step, op, filterSubQuery, groupBy, orderBy)
query := `SELECT %s ts, ` + rateWithoutNegative + `as value FROM(%s)` query := `SELECT %s ts, ` + rateWithoutNegative + `as value FROM(%s) WHERE isNaN(value) = 0`
query = fmt.Sprintf(query, groupTags, subQuery) query = fmt.Sprintf(query, groupTags, subQuery)
return query, nil return query, nil
case case
@ -249,9 +253,9 @@ func buildMetricQuery(start, end, step int64, mq *v3.BuilderQuery, tableName str
subQuery := fmt.Sprintf( subQuery := fmt.Sprintf(
queryTmpl, rateGroupTags, step, op, filterSubQuery, rateGroupBy, rateOrderBy, queryTmpl, rateGroupTags, step, op, filterSubQuery, rateGroupBy, rateOrderBy,
) // labels will be same so any should be fine ) // labels will be same so any should be fine
query := `SELECT %s ts, ` + rateWithoutNegative + ` as value FROM(%s)` query := `SELECT %s ts, ` + rateWithoutNegative + ` as value FROM(%s) WHERE isNaN(value) = 0`
query = fmt.Sprintf(query, groupTags, subQuery) query = fmt.Sprintf(query, groupTags, subQuery)
query = fmt.Sprintf(`SELECT %s ts, sum(value) as value FROM (%s) GROUP BY %s ORDER BY %s ts`, groupTags, query, groupBy, orderBy) query = fmt.Sprintf(`SELECT %s ts, sum(value) as value FROM (%s) GROUP BY %s HAVING isNaN(value) = 0 ORDER BY %s ts`, groupTags, query, groupBy, orderBy)
value := aggregateOperatorToPercentile[mq.AggregateOperator] value := aggregateOperatorToPercentile[mq.AggregateOperator]
query = fmt.Sprintf(`SELECT %s ts, histogramQuantile(arrayMap(x -> toFloat64(x), groupArray(le)), groupArray(value), %.3f) as value FROM (%s) GROUP BY %s ORDER BY %s ts`, groupTagsWithoutLe, value, query, groupByWithoutLe, orderWithoutLe) query = fmt.Sprintf(`SELECT %s ts, histogramQuantile(arrayMap(x -> toFloat64(x), groupArray(le)), groupArray(value), %.3f) as value FROM (%s) GROUP BY %s ORDER BY %s ts`, groupTagsWithoutLe, value, query, groupByWithoutLe, orderWithoutLe)
@ -350,7 +354,7 @@ func orderByAttributeKeyTags(items []v3.OrderBy, tags []v3.AttributeKey) string
func having(items []v3.Having) string { func having(items []v3.Having) string {
var having []string var having []string
for _, item := range items { for _, item := range items {
having = append(having, fmt.Sprintf("%s %s %v", item.ColumnName, item.Operator, utils.ClickHouseFormattedValue(item.Value))) having = append(having, fmt.Sprintf("%s %s %v", "value", item.Operator, utils.ClickHouseFormattedValue(item.Value)))
} }
return strings.Join(having, " AND ") return strings.Join(having, " AND ")
} }
@ -390,6 +394,10 @@ func PrepareMetricQuery(start, end int64, queryType v3.QueryType, panelType v3.P
if err != nil { if err != nil {
return "", err return "", err
} }
if having(mq.Having) != "" {
query = fmt.Sprintf("SELECT * FROM (%s) HAVING %s", query, having(mq.Having))
}
if panelType == v3.PanelTypeValue { if panelType == v3.PanelTypeValue {
query, err = reduceQuery(query, mq.ReduceTo, mq.AggregateOperator) query, err = reduceQuery(query, mq.ReduceTo, mq.AggregateOperator)
} }

View File

@ -1,6 +1,7 @@
package v3 package v3
import ( import (
"fmt"
"testing" "testing"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@ -96,3 +97,140 @@ func TestBuildQueryWithMultipleQueries(t *testing.T) {
require.Contains(t, query, rateWithoutNegative) require.Contains(t, query, rateWithoutNegative)
}) })
} }
func TestBuildQueryOperators(t *testing.T) {
testCases := []struct {
operator v3.FilterOperator
filterSet v3.FilterSet
expectedWhereClause string
}{
{
operator: v3.FilterOperatorEqual,
filterSet: v3.FilterSet{
Operator: "AND",
Items: []v3.FilterItem{
{Key: v3.AttributeKey{Key: "service_name"}, Value: "route", Operator: v3.FilterOperatorEqual},
},
},
expectedWhereClause: "JSONExtractString(labels, 'service_name') = 'route'",
},
{
operator: v3.FilterOperatorNotEqual,
filterSet: v3.FilterSet{
Operator: "AND",
Items: []v3.FilterItem{
{Key: v3.AttributeKey{Key: "service_name"}, Value: "route", Operator: v3.FilterOperatorNotEqual},
},
},
expectedWhereClause: "JSONExtractString(labels, 'service_name') != 'route'",
},
{
operator: v3.FilterOperatorRegex,
filterSet: v3.FilterSet{
Operator: "AND",
Items: []v3.FilterItem{
{Key: v3.AttributeKey{Key: "service_name"}, Value: "out", Operator: v3.FilterOperatorRegex},
},
},
expectedWhereClause: "match(JSONExtractString(labels, 'service_name'), 'out')",
},
{
operator: v3.FilterOperatorNotRegex,
filterSet: v3.FilterSet{
Operator: "AND",
Items: []v3.FilterItem{
{Key: v3.AttributeKey{Key: "service_name"}, Value: "out", Operator: v3.FilterOperatorNotRegex},
},
},
expectedWhereClause: "not match(JSONExtractString(labels, 'service_name'), 'out')",
},
{
operator: v3.FilterOperatorIn,
filterSet: v3.FilterSet{
Operator: "AND",
Items: []v3.FilterItem{
{Key: v3.AttributeKey{Key: "service_name"}, Value: []interface{}{"route", "driver"}, Operator: v3.FilterOperatorIn},
},
},
expectedWhereClause: "JSONExtractString(labels, 'service_name') IN ['route','driver']",
},
{
operator: v3.FilterOperatorNotIn,
filterSet: v3.FilterSet{
Operator: "AND",
Items: []v3.FilterItem{
{Key: v3.AttributeKey{Key: "service_name"}, Value: []interface{}{"route", "driver"}, Operator: v3.FilterOperatorNotIn},
},
},
expectedWhereClause: "JSONExtractString(labels, 'service_name') NOT IN ['route','driver']",
},
{
operator: v3.FilterOperatorExists,
filterSet: v3.FilterSet{
Operator: "AND",
Items: []v3.FilterItem{
{Key: v3.AttributeKey{Key: "horn"}, Operator: v3.FilterOperatorExists},
},
},
expectedWhereClause: "has(JSONExtractKeys(labels), 'horn')",
},
{
operator: v3.FilterOperatorNotExists,
filterSet: v3.FilterSet{
Operator: "AND",
Items: []v3.FilterItem{
{Key: v3.AttributeKey{Key: "horn"}, Operator: v3.FilterOperatorNotExists},
},
},
expectedWhereClause: "not has(JSONExtractKeys(labels), 'horn')",
},
{
operator: v3.FilterOperatorContains,
filterSet: v3.FilterSet{
Operator: "AND",
Items: []v3.FilterItem{
{Key: v3.AttributeKey{Key: "service_name"}, Value: "out", Operator: v3.FilterOperatorContains},
},
},
expectedWhereClause: "like(JSONExtractString(labels, 'service_name'), '%out%')",
},
{
operator: v3.FilterOperatorNotContains,
filterSet: v3.FilterSet{
Operator: "AND",
Items: []v3.FilterItem{
{Key: v3.AttributeKey{Key: "serice_name"}, Value: "out", Operator: v3.FilterOperatorNotContains},
},
},
expectedWhereClause: "notLike(JSONExtractString(labels, 'serice_name'), '%out%')",
},
{
operator: v3.FilterOperatorLike,
filterSet: v3.FilterSet{
Operator: "AND",
Items: []v3.FilterItem{
{Key: v3.AttributeKey{Key: "service_name"}, Value: "dri", Operator: v3.FilterOperatorLike},
},
},
expectedWhereClause: "like(JSONExtractString(labels, 'service_name'), 'dri')",
},
{
operator: v3.FilterOperatorNotLike,
filterSet: v3.FilterSet{
Operator: "AND",
Items: []v3.FilterItem{
{Key: v3.AttributeKey{Key: "serice_name"}, Value: "dri", Operator: v3.FilterOperatorNotLike},
},
},
expectedWhereClause: "notLike(JSONExtractString(labels, 'serice_name'), 'dri')",
},
}
for i, tc := range testCases {
t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) {
whereClause, err := buildMetricsTimeSeriesFilterQuery(&tc.filterSet, []v3.AttributeKey{}, "signoz_calls_total", "sum")
require.NoError(t, err)
require.Contains(t, whereClause, tc.expectedWhereClause)
})
}
}

View File

@ -836,7 +836,6 @@ func parseFilterAttributeKeyRequest(r *http.Request) (*v3.FilterAttributeKeyRequ
dataSource := v3.DataSource(r.URL.Query().Get("dataSource")) dataSource := v3.DataSource(r.URL.Query().Get("dataSource"))
aggregateOperator := v3.AggregateOperator(r.URL.Query().Get("aggregateOperator")) aggregateOperator := v3.AggregateOperator(r.URL.Query().Get("aggregateOperator"))
aggregateAttribute := r.URL.Query().Get("aggregateAttribute") aggregateAttribute := r.URL.Query().Get("aggregateAttribute")
tagType := v3.TagType(r.URL.Query().Get("tagType"))
limit, err := strconv.Atoi(r.URL.Query().Get("limit")) limit, err := strconv.Atoi(r.URL.Query().Get("limit"))
if err != nil { if err != nil {
@ -851,15 +850,10 @@ func parseFilterAttributeKeyRequest(r *http.Request) (*v3.FilterAttributeKeyRequ
return nil, err return nil, err
} }
if err := tagType.Validate(); err != nil && tagType != v3.TagType("") {
return nil, err
}
req = v3.FilterAttributeKeyRequest{ req = v3.FilterAttributeKeyRequest{
DataSource: dataSource, DataSource: dataSource,
AggregateOperator: aggregateOperator, AggregateOperator: aggregateOperator,
AggregateAttribute: aggregateAttribute, AggregateAttribute: aggregateAttribute,
TagType: tagType,
Limit: limit, Limit: limit,
SearchText: r.URL.Query().Get("searchText"), SearchText: r.URL.Query().Get("searchText"),
} }

View File

@ -518,7 +518,7 @@ func TestParseQueryRangeParamsCompositeQuery(t *testing.T) {
BuilderQueries: map[string]*v3.BuilderQuery{ BuilderQueries: map[string]*v3.BuilderQuery{
"A": { "A": {
QueryName: "A", QueryName: "A",
DataSource: "traces", DataSource: "metrics",
AggregateOperator: "sum", AggregateOperator: "sum",
AggregateAttribute: v3.AttributeKey{}, AggregateAttribute: v3.AttributeKey{},
Expression: "A", Expression: "A",

View File

@ -183,5 +183,12 @@ func (qb *QueryBuilder) PrepareQueries(params *v3.QueryRangeParamsV3, args ...in
} }
} }
} }
// filter out disabled queries
for queryName := range queries {
if compositeQuery.BuilderQueries[queryName].Disabled {
delete(queries, queryName)
}
}
return queries, nil return queries, nil
} }

View File

@ -28,6 +28,7 @@ func TestBuildQueryWithMultipleQueriesAndFormula(t *testing.T) {
Expression: "A", Expression: "A",
}, },
"B": { "B": {
QueryName: "B",
AggregateAttribute: v3.AttributeKey{Key: "name2"}, AggregateAttribute: v3.AttributeKey{Key: "name2"},
DataSource: v3.DataSourceMetrics, DataSource: v3.DataSourceMetrics,
AggregateOperator: v3.AggregateOperatorRateAvg, AggregateOperator: v3.AggregateOperatorRateAvg,
@ -112,6 +113,7 @@ func TestBuildQueryWithThreeOrMoreQueriesRefAndFormula(t *testing.T) {
Disabled: true, Disabled: true,
}, },
"B": { "B": {
QueryName: "B",
AggregateAttribute: v3.AttributeKey{Key: "name2"}, AggregateAttribute: v3.AttributeKey{Key: "name2"},
DataSource: v3.DataSourceMetrics, DataSource: v3.DataSourceMetrics,
@ -120,6 +122,7 @@ func TestBuildQueryWithThreeOrMoreQueriesRefAndFormula(t *testing.T) {
Disabled: true, Disabled: true,
}, },
"C": { "C": {
QueryName: "C",
AggregateAttribute: v3.AttributeKey{Key: "name3"}, AggregateAttribute: v3.AttributeKey{Key: "name3"},
DataSource: v3.DataSourceMetrics, DataSource: v3.DataSourceMetrics,
@ -175,6 +178,12 @@ func TestBuildQueryWithThreeOrMoreQueriesRefAndFormula(t *testing.T) {
require.Contains(t, queries["F5"], "SELECT A.ts as ts, ((A.value - B.value) / B.value) * 100") require.Contains(t, queries["F5"], "SELECT A.ts as ts, ((A.value - B.value) / B.value) * 100")
require.Equal(t, 1, strings.Count(queries["F5"], " ON ")) require.Equal(t, 1, strings.Count(queries["F5"], " ON "))
for _, query := range q.CompositeQuery.BuilderQueries {
if query.Disabled {
require.NotContains(t, queries, query.QueryName)
}
}
// res := PrepareBuilderMetricQueries(q, "table") // res := PrepareBuilderMetricQueries(q, "table")
// So(res.Err, ShouldBeNil) // So(res.Err, ShouldBeNil)
// queries := res.Queries // queries := res.Queries

View File

@ -101,14 +101,19 @@ func (a AggregateOperator) Validate() error {
// RequireAttribute returns true if the aggregate operator requires an attribute // RequireAttribute returns true if the aggregate operator requires an attribute
// to be specified. // to be specified.
func (a AggregateOperator) RequireAttribute() bool { func (a AggregateOperator) RequireAttribute(dataSource DataSource) bool {
switch a { switch dataSource {
case AggregateOperatorNoOp, case DataSourceMetrics:
AggregateOperatorCount, switch a {
AggregateOperatorCountDistinct: case AggregateOperatorNoOp,
return false AggregateOperatorCount,
AggregateOperatorCountDistinct:
return false
default:
return true
}
default: default:
return true return false
} }
} }
@ -202,7 +207,6 @@ type FilterAttributeKeyRequest struct {
DataSource DataSource `json:"dataSource"` DataSource DataSource `json:"dataSource"`
AggregateOperator AggregateOperator `json:"aggregateOperator"` AggregateOperator AggregateOperator `json:"aggregateOperator"`
AggregateAttribute string `json:"aggregateAttribute"` AggregateAttribute string `json:"aggregateAttribute"`
TagType TagType `json:"tagType"`
SearchText string `json:"searchText"` SearchText string `json:"searchText"`
Limit int `json:"limit"` Limit int `json:"limit"`
} }
@ -424,7 +428,7 @@ func (b *BuilderQuery) Validate() error {
if err := b.AggregateOperator.Validate(); err != nil { if err := b.AggregateOperator.Validate(); err != nil {
return fmt.Errorf("aggregate operator is invalid: %w", err) return fmt.Errorf("aggregate operator is invalid: %w", err)
} }
if b.AggregateAttribute == (AttributeKey{}) && b.AggregateOperator.RequireAttribute() { if b.AggregateAttribute == (AttributeKey{}) && b.AggregateOperator.RequireAttribute(b.DataSource) {
return fmt.Errorf("aggregate attribute is required") return fmt.Errorf("aggregate attribute is required")
} }
} }