From e67358d203864018ecbbe8c74c1cb3af3be4b2b4 Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Wed, 4 Sep 2024 15:39:05 +1000 Subject: [PATCH 1/4] histogram: include counter reset hint in test expression output Signed-off-by: Charles Korn --- model/histogram/float_histogram.go | 11 +++++++++++ promql/parser/parse_test.go | 16 ++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index 2a37ea66d4..1777afdbf1 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -230,6 +230,17 @@ func (h *FloatHistogram) TestExpression() string { res = append(res, fmt.Sprintf("custom_values:%g", m.CustomValues)) } + switch m.CounterResetHint { + case UnknownCounterReset: + // Unknown is the default, don't add anything. + case CounterReset: + res = append(res, fmt.Sprintf("counter_reset_hint:reset")) + case NotCounterReset: + res = append(res, fmt.Sprintf("counter_reset_hint:not_reset")) + case GaugeType: + res = append(res, fmt.Sprintf("counter_reset_hint:gauge")) + } + addBuckets := func(kind, bucketsKey, offsetKey string, buckets []float64, spans []Span) []string { if len(spans) > 1 { panic(fmt.Sprintf("histogram with multiple %s spans not supported", kind)) diff --git a/promql/parser/parse_test.go b/promql/parser/parse_test.go index 37748323ce..40e6809183 100644 --- a/promql/parser/parse_test.go +++ b/promql/parser/parse_test.go @@ -4385,6 +4385,22 @@ func TestHistogramTestExpression(t *testing.T) { }, expected: `{{offset:-3 buckets:[5.1 0 0 0 0 10 7] n_offset:-1 n_buckets:[4.1 5 0 0 7 8 9]}}`, }, + { + name: "known counter reset hint", + input: histogram.FloatHistogram{ + Schema: 1, + Sum: -0.3, + Count: 3.1, + ZeroCount: 7.1, + ZeroThreshold: 0.05, + PositiveBuckets: []float64{5.1, 10, 7}, + PositiveSpans: []histogram.Span{{Offset: -3, Length: 3}}, + NegativeBuckets: []float64{4.1, 5}, + NegativeSpans: []histogram.Span{{Offset: -5, Length: 2}}, + CounterResetHint: histogram.CounterReset, + }, + expected: `{{schema:1 count:3.1 sum:-0.3 z_bucket:7.1 z_bucket_w:0.05 counter_reset_hint:reset offset:-3 buckets:[5.1 10 7] n_offset:-5 n_buckets:[4.1 5]}}`, + }, } { t.Run(test.name, func(t *testing.T) { expression := test.input.TestExpression() From 90dc1b45dbb448f6ce0ff9349dcd06e76db4f525 Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Wed, 4 Sep 2024 15:47:07 +1000 Subject: [PATCH 2/4] promqltest: use test expression format for histograms in assertion failure messages Signed-off-by: Charles Korn --- promql/promqltest/test.go | 10 ++++++++-- promql/promqltest/test_test.go | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/promql/promqltest/test.go b/promql/promqltest/test.go index 065e52e33f..bab8388622 100644 --- a/promql/promqltest/test.go +++ b/promql/promqltest/test.go @@ -779,7 +779,7 @@ func (ev *evalCmd) compareResult(result parser.Value) error { } if !compareNativeHistogram(expected.H.Compact(0), actual.H.Compact(0)) { - return fmt.Errorf("expected histogram value at index %v (t=%v) for %s to be %v, but got %v (result has %s)", i, actual.T, ev.metrics[hash], expected.H, actual.H, formatSeriesResult(s)) + return fmt.Errorf("expected histogram value at index %v (t=%v) for %s to be %v, but got %v (result has %s)", i, actual.T, ev.metrics[hash], expected.H.TestExpression(), actual.H.TestExpression(), formatSeriesResult(s)) } } } @@ -995,7 +995,13 @@ func formatSeriesResult(s promql.Series) string { histogramPlural = "" } - return fmt.Sprintf("%v float point%s %v and %v histogram point%s %v", len(s.Floats), floatPlural, s.Floats, len(s.Histograms), histogramPlural, s.Histograms) + histograms := make([]string, 0, len(s.Histograms)) + + for _, p := range s.Histograms { + histograms = append(histograms, fmt.Sprintf("%v @[%v]", p.H.TestExpression(), p.T)) + } + + return fmt.Sprintf("%v float point%s %v and %v histogram point%s %v", len(s.Floats), floatPlural, s.Floats, len(s.Histograms), histogramPlural, histograms) } // HistogramTestExpression returns TestExpression() for the given histogram or "" if the histogram is nil. diff --git a/promql/promqltest/test_test.go b/promql/promqltest/test_test.go index 49b43eb126..bd965b00b5 100644 --- a/promql/promqltest/test_test.go +++ b/promql/promqltest/test_test.go @@ -381,7 +381,7 @@ load 5m eval range from 0 to 10m step 5m testmetric testmetric {{schema:-1 sum:4 count:1 buckets:[1] offset:1}} {{schema:-1 sum:7 count:1 buckets:[1] offset:1}} {{schema:-1 sum:8 count:1 buckets:[1] offset:1}} `, - expectedError: `error in eval testmetric (line 5): expected histogram value at index 1 (t=300000) for {__name__="testmetric"} to be {count:1, sum:7, (1,4]:1}, but got {count:1, sum:5, (1,4]:1} (result has 0 float points [] and 3 histogram points [{count:1, sum:4, (1,4]:1} @[0] {count:1, sum:5, (1,4]:1} @[300000] {count:1, sum:6, (1,4]:1} @[600000]])`, + expectedError: `error in eval testmetric (line 5): expected histogram value at index 1 (t=300000) for {__name__="testmetric"} to be {{schema:-1 count:1 sum:7 offset:1 buckets:[1]}}, but got {{schema:-1 count:1 sum:5 counter_reset_hint:not_reset offset:1 buckets:[1]}} (result has 0 float points [] and 3 histogram points [{{schema:-1 count:1 sum:4 offset:1 buckets:[1]}} @[0] {{schema:-1 count:1 sum:5 counter_reset_hint:not_reset offset:1 buckets:[1]}} @[300000] {{schema:-1 count:1 sum:6 counter_reset_hint:not_reset offset:1 buckets:[1]}} @[600000]])`, }, "range query with too many points for query time range": { input: testData + ` @@ -532,7 +532,7 @@ load 5m eval range from 0 to 5m step 5m testmetric testmetric 2 3 `, - expectedError: `error in eval testmetric (line 5): expected 2 float points and 0 histogram points for {__name__="testmetric"}, but got 0 float points [] and 2 histogram points [{count:0, sum:0} @[0] {count:0, sum:0} @[300000]]`, + expectedError: `error in eval testmetric (line 5): expected 2 float points and 0 histogram points for {__name__="testmetric"}, but got 0 float points [] and 2 histogram points [{{}} @[0] {{counter_reset_hint:not_reset}} @[300000]]`, }, "range query with expected mixed results": { input: ` @@ -552,7 +552,7 @@ load 5m eval range from 0 to 5m step 5m testmetric testmetric {{}} 3 `, - expectedError: `error in eval testmetric (line 5): expected float value at index 0 for {__name__="testmetric"} to have timestamp 300000, but it had timestamp 0 (result has 1 float point [3 @[0]] and 1 histogram point [{count:0, sum:0} @[300000]])`, + expectedError: `error in eval testmetric (line 5): expected float value at index 0 for {__name__="testmetric"} to have timestamp 300000, but it had timestamp 0 (result has 1 float point [3 @[0]] and 1 histogram point [{{}} @[300000]])`, }, "instant query with expected scalar result": { input: ` From 4da551578c691591d54bddf38fd8b1620c5faa73 Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Wed, 4 Sep 2024 16:33:18 +1000 Subject: [PATCH 3/4] Fix test broken by inclusion of `counter_reset_hint` Signed-off-by: Charles Korn --- cmd/promtool/testdata/unittest.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/promtool/testdata/unittest.yml b/cmd/promtool/testdata/unittest.yml index ff511729ba..d6224d785f 100644 --- a/cmd/promtool/testdata/unittest.yml +++ b/cmd/promtool/testdata/unittest.yml @@ -69,13 +69,13 @@ tests: eval_time: 2m exp_samples: - labels: "test_histogram_repeat" - histogram: "{{count:2 sum:3 buckets:[2]}}" + histogram: "{{count:2 sum:3 counter_reset_hint:not_reset buckets:[2]}}" - expr: test_histogram_increase eval_time: 2m exp_samples: - labels: "test_histogram_increase" - histogram: "{{count:4 sum:5.6 buckets:[4]}}" + histogram: "{{count:4 sum:5.6 counter_reset_hint:not_reset buckets:[4]}}" # Ensure a value is stale as soon as it is marked as such. - expr: test_stale From 6dbb4e1a94f75a057720bf0dd101897283465c8e Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Fri, 20 Sep 2024 11:49:54 +1000 Subject: [PATCH 4/4] Fix linting issues Signed-off-by: Charles Korn --- model/histogram/float_histogram.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index 1777afdbf1..300f3176e4 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -234,11 +234,11 @@ func (h *FloatHistogram) TestExpression() string { case UnknownCounterReset: // Unknown is the default, don't add anything. case CounterReset: - res = append(res, fmt.Sprintf("counter_reset_hint:reset")) + res = append(res, "counter_reset_hint:reset") case NotCounterReset: - res = append(res, fmt.Sprintf("counter_reset_hint:not_reset")) + res = append(res, "counter_reset_hint:not_reset") case GaugeType: - res = append(res, fmt.Sprintf("counter_reset_hint:gauge")) + res = append(res, "counter_reset_hint:gauge") } addBuckets := func(kind, bucketsKey, offsetKey string, buckets []float64, spans []Span) []string {