From 94ad489328623fd9a4df33e498138b48d5363c0c Mon Sep 17 00:00:00 2001 From: suntala Date: Sun, 11 Aug 2024 21:07:54 +0200 Subject: [PATCH 1/4] Fall back to comparing by label set when sorting by label Co-authored-by: Aleks Fazlieva Signed-off-by: suntala --- promql/functions.go | 10 +++++----- promql/promqltest/testdata/functions.test | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/promql/functions.go b/promql/functions.go index 018023bf0..353d4155a 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -406,10 +406,10 @@ func funcSortDesc(vals []parser.Value, args parser.Expressions, enh *EvalNodeHel // === sort_by_label(vector parser.ValueTypeVector, label parser.ValueTypeString...) (Vector, Annotations) === func funcSortByLabel(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper) (Vector, annotations.Annotations) { - // In case the labels are the same, NaN should sort to the bottom, so take - // ascending sort with NaN first and reverse it. - var anno annotations.Annotations - vals[0], anno = funcSort(vals, args, enh) + slices.SortFunc(vals[0].(Vector), func(a, b Sample) int { + return labels.Compare(a.Metric, b.Metric) + }) + labels := stringSliceFromArgs(args[1:]) slices.SortFunc(vals[0].(Vector), func(a, b Sample) int { // Iterate over each given label @@ -431,7 +431,7 @@ func funcSortByLabel(vals []parser.Value, args parser.Expressions, enh *EvalNode return 0 }) - return vals[0].(Vector), anno + return vals[0].(Vector), nil } // === sort_by_label_desc(vector parser.ValueTypeVector, label parser.ValueTypeString...) (Vector, Annotations) === diff --git a/promql/promqltest/testdata/functions.test b/promql/promqltest/testdata/functions.test index b89d44fce..177afca35 100644 --- a/promql/promqltest/testdata/functions.test +++ b/promql/promqltest/testdata/functions.test @@ -523,16 +523,16 @@ load 5m node_uname_info{job="node_exporter", instance="4m1000", release="1.111.3"} 0+10x10 eval_ordered instant at 50m sort_by_label(http_requests, "instance") - http_requests{group="production", instance="0", job="api-server"} 100 http_requests{group="canary", instance="0", job="api-server"} 300 - http_requests{group="production", instance="0", job="app-server"} 500 http_requests{group="canary", instance="0", job="app-server"} 700 - http_requests{group="production", instance="1", job="api-server"} 200 + http_requests{group="production", instance="0", job="api-server"} 100 + http_requests{group="production", instance="0", job="app-server"} 500 http_requests{group="canary", instance="1", job="api-server"} 400 - http_requests{group="production", instance="1", job="app-server"} 600 http_requests{group="canary", instance="1", job="app-server"} 800 - http_requests{group="production", instance="2", job="api-server"} 100 + http_requests{group="production", instance="1", job="api-server"} 200 + http_requests{group="production", instance="1", job="app-server"} 600 http_requests{group="canary", instance="2", job="api-server"} NaN + http_requests{group="production", instance="2", job="api-server"} 100 eval_ordered instant at 50m sort_by_label(http_requests, "instance", "group") http_requests{group="canary", instance="0", job="api-server"} 300 From fd2f44af7fa48c5b57df51883b6f69c2e73a4809 Mon Sep 17 00:00:00 2001 From: suntala Date: Sun, 11 Aug 2024 21:24:09 +0200 Subject: [PATCH 2/4] Fall back to comparing by label set when sorting by label desc Co-authored-by: Aleks Fazlieva Signed-off-by: suntala --- promql/functions.go | 10 +++++----- promql/promqltest/testdata/functions.test | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/promql/functions.go b/promql/functions.go index 353d4155a..e9bfe45f4 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -436,10 +436,10 @@ func funcSortByLabel(vals []parser.Value, args parser.Expressions, enh *EvalNode // === sort_by_label_desc(vector parser.ValueTypeVector, label parser.ValueTypeString...) (Vector, Annotations) === func funcSortByLabelDesc(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper) (Vector, annotations.Annotations) { - // In case the labels are the same, NaN should sort to the bottom, so take - // ascending sort with NaN first and reverse it. - var anno annotations.Annotations - vals[0], anno = funcSortDesc(vals, args, enh) + slices.SortFunc(vals[0].(Vector), func(a, b Sample) int { + return labels.Compare(b.Metric, a.Metric) + }) + labels := stringSliceFromArgs(args[1:]) slices.SortFunc(vals[0].(Vector), func(a, b Sample) int { // Iterate over each given label @@ -461,7 +461,7 @@ func funcSortByLabelDesc(vals []parser.Value, args parser.Expressions, enh *Eval return 0 }) - return vals[0].(Vector), anno + return vals[0].(Vector), nil } // === clamp(Vector parser.ValueTypeVector, min, max Scalar) (Vector, Annotations) === diff --git a/promql/promqltest/testdata/functions.test b/promql/promqltest/testdata/functions.test index 177afca35..6e2b3630b 100644 --- a/promql/promqltest/testdata/functions.test +++ b/promql/promqltest/testdata/functions.test @@ -585,14 +585,14 @@ eval_ordered instant at 50m sort_by_label(http_requests, "job", "instance", "gro eval_ordered instant at 50m sort_by_label_desc(http_requests, "instance") http_requests{group="production", instance="2", job="api-server"} 100 http_requests{group="canary", instance="2", job="api-server"} NaN - http_requests{group="canary", instance="1", job="app-server"} 800 http_requests{group="production", instance="1", job="app-server"} 600 - http_requests{group="canary", instance="1", job="api-server"} 400 http_requests{group="production", instance="1", job="api-server"} 200 - http_requests{group="canary", instance="0", job="app-server"} 700 + http_requests{group="canary", instance="1", job="app-server"} 800 + http_requests{group="canary", instance="1", job="api-server"} 400 http_requests{group="production", instance="0", job="app-server"} 500 - http_requests{group="canary", instance="0", job="api-server"} 300 http_requests{group="production", instance="0", job="api-server"} 100 + http_requests{group="canary", instance="0", job="app-server"} 700 + http_requests{group="canary", instance="0", job="api-server"} 300 eval_ordered instant at 50m sort_by_label_desc(http_requests, "instance", "group") http_requests{group="production", instance="2", job="api-server"} 100 From 532904a1d6c34ec2e7064bc0ceb959a6b63acaf7 Mon Sep 17 00:00:00 2001 From: suntala Date: Sun, 11 Aug 2024 21:34:39 +0200 Subject: [PATCH 3/4] Document changes to sort by label Co-authored-by: Aleks Fazlieva Signed-off-by: suntala --- docs/querying/functions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/querying/functions.md b/docs/querying/functions.md index ee81328b5..951b90a68 100644 --- a/docs/querying/functions.md +++ b/docs/querying/functions.md @@ -619,7 +619,7 @@ Like `sort`, `sort_desc` only affects the results of instant queries, as range q **This function has to be enabled via the [feature flag](../feature_flags.md) `--enable-feature=promql-experimental-functions`.** -`sort_by_label(v instant-vector, label string, ...)` returns vector elements sorted by their label values and sample value in case of label values being equal, in ascending order. +`sort_by_label(v instant-vector, label string, ...)` returns vector elements sorted by the values of the given labels in ascending order. In case these label values are equal, elements are sorted by their full label sets. Please note that the sort by label functions only affect the results of instant queries, as range query results always have a fixed output ordering. From 8e82ac8d2bbee98c1f835fe715d2ea2ad4aa362a Mon Sep 17 00:00:00 2001 From: suntala Date: Tue, 20 Aug 2024 20:40:55 +0200 Subject: [PATCH 4/4] Add comments to the sort by label functions Signed-off-by: suntala --- promql/functions.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/promql/functions.go b/promql/functions.go index e9bfe45f4..189d67caf 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -406,17 +406,22 @@ func funcSortDesc(vals []parser.Value, args parser.Expressions, enh *EvalNodeHel // === sort_by_label(vector parser.ValueTypeVector, label parser.ValueTypeString...) (Vector, Annotations) === func funcSortByLabel(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper) (Vector, annotations.Annotations) { + // First, sort by the full label set. This ensures a consistent ordering in case sorting by the + // labels provided as arguments is not conclusive. slices.SortFunc(vals[0].(Vector), func(a, b Sample) int { return labels.Compare(a.Metric, b.Metric) }) labels := stringSliceFromArgs(args[1:]) + // Next, sort by the labels provided as arguments. slices.SortFunc(vals[0].(Vector), func(a, b Sample) int { - // Iterate over each given label + // Iterate over each given label. for _, label := range labels { lv1 := a.Metric.Get(label) lv2 := b.Metric.Get(label) + // If we encounter multiple samples with the same label values, the sorting which was + // performed in the first step will act as a "tie breaker". if lv1 == lv2 { continue } @@ -436,17 +441,22 @@ func funcSortByLabel(vals []parser.Value, args parser.Expressions, enh *EvalNode // === sort_by_label_desc(vector parser.ValueTypeVector, label parser.ValueTypeString...) (Vector, Annotations) === func funcSortByLabelDesc(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper) (Vector, annotations.Annotations) { + // First, sort by the full label set. This ensures a consistent ordering in case sorting by the + // labels provided as arguments is not conclusive. slices.SortFunc(vals[0].(Vector), func(a, b Sample) int { return labels.Compare(b.Metric, a.Metric) }) labels := stringSliceFromArgs(args[1:]) + // Next, sort by the labels provided as arguments. slices.SortFunc(vals[0].(Vector), func(a, b Sample) int { - // Iterate over each given label + // Iterate over each given label. for _, label := range labels { lv1 := a.Metric.Get(label) lv2 := b.Metric.Get(label) + // If we encounter multiple samples with the same label values, the sorting which was + // performed in the first step will act as a "tie breaker". if lv1 == lv2 { continue }