From 69915e6c9736f38a0952c5ce8ad9a2d180783a43 Mon Sep 17 00:00:00 2001 From: Toan Nguyen Date: Thu, 26 Sep 2024 16:18:53 +0700 Subject: [PATCH] fix: no generated label when introspecting histogram metrics (#12) --- configuration/update.go | 23 ++++++---- connector/internal/collection_test.go | 63 +++++++++++++++++++++++++- connector/internal/expression_label.go | 6 ++- tests/configuration/configuration.yaml | 9 +++- 4 files changed, 89 insertions(+), 12 deletions(-) diff --git a/configuration/update.go b/configuration/update.go index b6d13f1..4f676b2 100644 --- a/configuration/update.go +++ b/configuration/update.go @@ -15,6 +15,7 @@ import ( "github.com/hasura/ndc-prometheus/connector/client" "github.com/hasura/ndc-prometheus/connector/metadata" "github.com/hasura/ndc-prometheus/connector/types" + v1 "github.com/prometheus/client_golang/api/prometheus/v1" "github.com/prometheus/common/model" "go.opentelemetry.io/otel" "gopkg.in/yaml.v3" @@ -121,7 +122,7 @@ func (uc *updateCommand) updateMetricsMetadata(ctx context.Context) error { continue } slog.Info(key, slog.String("type", string(info[0].Type))) - labels, err := uc.getAllLabelsOfMetric(ctx, key) + labels, err := uc.getAllLabelsOfMetric(ctx, key, info[0]) if err != nil { return fmt.Errorf("error when fetching labels for metric `%s`: %s", key, err) } @@ -135,8 +136,12 @@ func (uc *updateCommand) updateMetricsMetadata(ctx context.Context) error { return nil } -func (uc *updateCommand) getAllLabelsOfMetric(ctx context.Context, name string) (map[string]metadata.LabelInfo, error) { - labels, warnings, err := uc.Client.Series(ctx, []string{name}, uc.Config.Generator.Metrics.StartAt, time.Now(), 1) +func (uc *updateCommand) getAllLabelsOfMetric(ctx context.Context, name string, metric v1.Metadata) (map[string]metadata.LabelInfo, error) { + metricName := name + 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(), 10) if err != nil { return nil, err } @@ -154,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 index 7e6bb7e..a66b76a 100644 --- 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 index dd9ad8f..d68181b 100644 --- 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 { diff --git a/tests/configuration/configuration.yaml b/tests/configuration/configuration.yaml index 73ac831..1f93cd1 100644 --- a/tests/configuration/configuration.yaml +++ b/tests/configuration/configuration.yaml @@ -1,4 +1,4 @@ -# yaml-language-server: $schema=../../jsonschema/configuration.json +# yaml-language-server: $schema=https://raw.githubusercontent.com/hasura/ndc-prometheus/main/jsonschema/configuration.json connection_settings: url: env: CONNECTION_URL @@ -37,7 +37,11 @@ metadata: ndc_prometheus_query_total_time: type: histogram description: Total time taken to plan and execute a query, in seconds - labels: {} + labels: + collection: {} + instance: {} + job: {} + otel_scope_name: {} net_conntrack_dialer_conn_attempted_total: type: counter description: Total number of connections attempted by the given dialer a given name. @@ -70,6 +74,7 @@ metadata: instance: {} job: {} otel_scope_name: {} + otel_scope_version: {} process_cpu_seconds_total: type: counter description: Total user and system CPU time spent in seconds.