From 3e4f24fbcdcdbf7a0ae2f25235b29ef08e1a69b5 Mon Sep 17 00:00:00 2001 From: Samuel Dufel Date: Tue, 18 Oct 2022 13:40:19 -0700 Subject: [PATCH 1/2] Fix sharding behavior for vector matches When analyzing a query composed of boolean expression with a non-"on" vector match, the current vertical sharding logic is incorrectly scoping to labels in the vector match and changing the semantics of the query. Example: ``` foo and without (lbl) bar ``` Signed-off-by: Samuel Dufel --- pkg/querysharding/analyzer.go | 3 +++ pkg/querysharding/analyzer_test.go | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/querysharding/analyzer.go b/pkg/querysharding/analyzer.go index f93a103fcf..8f99e3d594 100644 --- a/pkg/querysharding/analyzer.go +++ b/pkg/querysharding/analyzer.go @@ -99,6 +99,9 @@ func (a *QueryAnalyzer) Analyze(query string) (QueryAnalysis, error) { case *parser.BinaryExpr: if n.VectorMatching != nil { shardingLabels := without(n.VectorMatching.MatchingLabels, []string{"le"}) + if !n.VectorMatching.On && len(shardingLabels) > 0 { + shardingLabels = append(shardingLabels, "__name__") + } analysis = analysis.scopeToLabels(shardingLabels, n.VectorMatching.On) } case *parser.AggregateExpr: diff --git a/pkg/querysharding/analyzer_test.go b/pkg/querysharding/analyzer_test.go index efd9dd5d94..4364ed2b3f 100644 --- a/pkg/querysharding/analyzer_test.go +++ b/pkg/querysharding/analyzer_test.go @@ -142,12 +142,12 @@ sum by (container) ( { name: "binary expression with without vector matching and grouping", expression: `sum without (cluster, pod) (http_requests_total{code="400"}) / ignoring (pod) sum without (cluster, pod) (http_requests_total)`, - shardingLabels: []string{"pod", "cluster"}, + shardingLabels: []string{"pod", "cluster", "__name__"}, }, { name: "multiple binary expressions with without grouping", expression: `(http_requests_total{code="400"} + ignoring (pod) http_requests_total{code="500"}) / ignoring (cluster, pod) http_requests_total`, - shardingLabels: []string{"cluster", "pod"}, + shardingLabels: []string{"cluster", "pod", "__name__"}, }, { name: "multiple binary expressions with without vector matchers", @@ -155,7 +155,7 @@ sum by (container) ( (http_requests_total{code="400"} + ignoring (cluster, pod) http_requests_total{code="500"}) / ignoring (pod) http_requests_total`, - shardingLabels: []string{"cluster", "pod"}, + shardingLabels: []string{"cluster", "pod", "__name__"}, }, { name: "histogram quantile", From a8b39b575057528f9a69382d305d36e3291c2299 Mon Sep 17 00:00:00 2001 From: Samuel Dufel Date: Thu, 20 Oct 2022 08:01:26 -0700 Subject: [PATCH 2/2] Replace string literals with constants Signed-off-by: Samuel Dufel --- pkg/querysharding/analyzer.go | 3 ++- pkg/querysharding/analyzer_test.go | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/querysharding/analyzer.go b/pkg/querysharding/analyzer.go index 8f99e3d594..ccd1f4ccb9 100644 --- a/pkg/querysharding/analyzer.go +++ b/pkg/querysharding/analyzer.go @@ -7,6 +7,7 @@ import ( "fmt" lru "github.com/hashicorp/golang-lru" + "github.com/prometheus/common/model" "github.com/prometheus/prometheus/promql/parser" ) @@ -100,7 +101,7 @@ func (a *QueryAnalyzer) Analyze(query string) (QueryAnalysis, error) { if n.VectorMatching != nil { shardingLabels := without(n.VectorMatching.MatchingLabels, []string{"le"}) if !n.VectorMatching.On && len(shardingLabels) > 0 { - shardingLabels = append(shardingLabels, "__name__") + shardingLabels = append(shardingLabels, model.MetricNameLabel) } analysis = analysis.scopeToLabels(shardingLabels, n.VectorMatching.On) } diff --git a/pkg/querysharding/analyzer_test.go b/pkg/querysharding/analyzer_test.go index 4364ed2b3f..dc3a059b23 100644 --- a/pkg/querysharding/analyzer_test.go +++ b/pkg/querysharding/analyzer_test.go @@ -7,6 +7,7 @@ import ( "sort" "testing" + "github.com/prometheus/common/model" "github.com/stretchr/testify/require" ) @@ -142,12 +143,12 @@ sum by (container) ( { name: "binary expression with without vector matching and grouping", expression: `sum without (cluster, pod) (http_requests_total{code="400"}) / ignoring (pod) sum without (cluster, pod) (http_requests_total)`, - shardingLabels: []string{"pod", "cluster", "__name__"}, + shardingLabels: []string{"pod", "cluster", model.MetricNameLabel}, }, { name: "multiple binary expressions with without grouping", expression: `(http_requests_total{code="400"} + ignoring (pod) http_requests_total{code="500"}) / ignoring (cluster, pod) http_requests_total`, - shardingLabels: []string{"cluster", "pod", "__name__"}, + shardingLabels: []string{"cluster", "pod", model.MetricNameLabel}, }, { name: "multiple binary expressions with without vector matchers", @@ -155,7 +156,7 @@ sum by (container) ( (http_requests_total{code="400"} + ignoring (cluster, pod) http_requests_total{code="500"}) / ignoring (pod) http_requests_total`, - shardingLabels: []string{"cluster", "pod", "__name__"}, + shardingLabels: []string{"cluster", "pod", model.MetricNameLabel}, }, { name: "histogram quantile",