fix label filter validation

hasura · Sep 26, 2024 · f4039e5 · f4039e5
1 parent 8987f7e
commit f4039e5
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 8 deletions.
diff --git a/configuration/update.go b/configuration/update.go
@@ -141,7 +141,7 @@ func (uc *updateCommand) getAllLabelsOfMetric(ctx context.Context, name string,
 	if metric.Type == v1.MetricTypeHistogram || metric.Type == v1.MetricTypeGaugeHistogram {
 		metricName = fmt.Sprintf("%s_count", metricName)
 	}
-	labels, warnings, err := uc.Client.Series(ctx, []string{metricName}, uc.Config.Generator.Metrics.StartAt, time.Now(), 1)
+	labels, warnings, err := uc.Client.Series(ctx, []string{metricName}, uc.Config.Generator.Metrics.StartAt, time.Now(), 10)
 	if err != nil {
 		return nil, err
 	}
@@ -159,12 +159,14 @@ func (uc *updateCommand) getAllLabelsOfMetric(ctx context.Context, name string,
 			excludedLabels = append(excludedLabels, el.Labels...)
 		}
 	}
-	for key := range labels[0] {
-		if !key.IsValid() || slices.Contains(excludedLabels, string(key)) {
-			continue
-		}
+	for _, ls := range labels {
+		for key := range ls {
+			if !key.IsValid() || slices.Contains(excludedLabels, string(key)) {
+				continue
+			}
 
-		results[string(key)] = metadata.LabelInfo{}
+			results[string(key)] = metadata.LabelInfo{}
+		}
 	}
 	return results, nil
 }

diff --git a/connector/internal/collection_test.go b/connector/internal/collection_test.go
@@ -15,6 +15,7 @@ var testCases = []struct {
 	Request     schema.QueryRequest
 	Predicate   CollectionRequest
 	QueryString string
+	IsEmpty     bool
 }{
 	{
 		Name: "nested_expressions",
@@ -104,6 +105,7 @@ var testCases = []struct {
 			},
 		},
 		QueryString: "",
+		IsEmpty:     true,
 	},
 	{
 		Name: "label_expressions_equal",
@@ -310,6 +312,64 @@ var testCases = []struct {
 		},
 		QueryString: `go_gc_duration_seconds{job=~"ndc-prometheus|node|prometheus"}`,
 	},
+	{
+		Name: "label_expressions_eq_neq",
+		Request: schema.QueryRequest{
+			Collection: "go_gc_duration_seconds",
+			Arguments: schema.QueryRequestArguments{
+				"timeout": schema.NewArgumentLiteral("5m").Encode(),
+			},
+			Query: schema.Query{
+				Predicate: schema.NewExpressionAnd(
+					schema.NewExpressionBinaryComparisonOperator(*schema.NewComparisonTargetColumn("job", nil, nil), "_eq", schema.NewComparisonValueScalar(`ndc-prometheus`)),
+					schema.NewExpressionBinaryComparisonOperator(*schema.NewComparisonTargetColumn("job", nil, nil), "_neq", schema.NewComparisonValueScalar(`ndc-prometheus`)),
+				).Encode(),
+			},
+		},
+		Predicate: CollectionRequest{
+			LabelExpressions: map[string]*LabelExpression{
+				"job": {
+					Name: "job",
+					Expressions: []schema.ExpressionBinaryComparisonOperator{
+						*schema.NewExpressionBinaryComparisonOperator(*schema.NewComparisonTargetColumn("job", nil, nil), "_eq", schema.NewComparisonValueScalar(`ndc-prometheus`)),
+						*schema.NewExpressionBinaryComparisonOperator(*schema.NewComparisonTargetColumn("job", nil, nil), "_neq", schema.NewComparisonValueScalar(`ndc-prometheus`)),
+					},
+				},
+			},
+		},
+		QueryString: ``,
+		IsEmpty:     true,
+	},
+	{
+		Name: "label_expressions_eq_in_neq",
+		Request: schema.QueryRequest{
+			Collection: "go_gc_duration_seconds",
+			Arguments: schema.QueryRequestArguments{
+				"timeout": schema.NewArgumentLiteral("5m").Encode(),
+			},
+			Query: schema.Query{
+				Predicate: schema.NewExpressionAnd(
+					schema.NewExpressionBinaryComparisonOperator(*schema.NewComparisonTargetColumn("job", nil, nil), "_eq", schema.NewComparisonValueScalar(`ndc-prometheus`)),
+					schema.NewExpressionBinaryComparisonOperator(*schema.NewComparisonTargetColumn("job", nil, nil), "_in", schema.NewComparisonValueScalar([]string{`ndc-prometheus`, "node"})),
+					schema.NewExpressionBinaryComparisonOperator(*schema.NewComparisonTargetColumn("job", nil, nil), "_neq", schema.NewComparisonValueScalar(`ndc-prometheus`)),
+				).Encode(),
+			},
+		},
+		Predicate: CollectionRequest{
+			LabelExpressions: map[string]*LabelExpression{
+				"job": {
+					Name: "job",
+					Expressions: []schema.ExpressionBinaryComparisonOperator{
+						*schema.NewExpressionBinaryComparisonOperator(*schema.NewComparisonTargetColumn("job", nil, nil), "_eq", schema.NewComparisonValueScalar(`ndc-prometheus`)),
+						*schema.NewExpressionBinaryComparisonOperator(*schema.NewComparisonTargetColumn("job", nil, nil), "_in", schema.NewComparisonValueScalar([]string{`ndc-prometheus`, "node"})),
+						*schema.NewExpressionBinaryComparisonOperator(*schema.NewComparisonTargetColumn("job", nil, nil), "_neq", schema.NewComparisonValueScalar(`ndc-prometheus`)),
+					},
+				},
+			},
+		},
+		QueryString: ``,
+		IsEmpty:     true,
+	},
 }
 
 func TestCollectionQueryExplain(t *testing.T) {
@@ -324,10 +384,11 @@ func TestCollectionQueryExplain(t *testing.T) {
 				Arguments: arguments,
 			}
 
-			request, queryString, _, err := executor.Explain(context.TODO())
+			request, queryString, ok, err := executor.Explain(context.TODO())
 			assert.NilError(t, err)
 			assert.DeepEqual(t, tc.Predicate, *request)
 			assert.Equal(t, tc.QueryString, queryString)
+			assert.Equal(t, !tc.IsEmpty, ok)
 		})
 	}
 }
diff --git a/connector/internal/expression_label.go b/connector/internal/expression_label.go
@@ -62,9 +62,12 @@ func (le *LabelExpressionBuilder) Evaluate(variables map[string]any) (string, bo
 		isIncludeRegex = isIncludeRegex || inc.IsRegex
 	}
 	if len(includes) == 0 && len(le.excludes) == 0 {
-		return "", true, nil
+		// all equal and not-equal labels are matched together,
+		// so the result is always empty
+		return "", false, nil
 	}
 
+	// if the label equals A or B but not C => equals A or B
 	if len(includes) > 0 {
 		operator := "="
 		if len(includes) > 1 || isIncludeRegex {
@@ -73,6 +76,7 @@ func (le *LabelExpressionBuilder) Evaluate(variables map[string]any) (string, bo
 		return fmt.Sprintf(`%s%s"%s"`, le.Name, operator, strings.Join(includes, "|")), true, nil
 	}
 
+	// exclude only
 	var isExcludeRegex bool
 	excludes := make([]string, 0, len(le.excludes))
 	for ev := range le.excludes {