Fix the wrong utils.ncHistogramLeRate implementation (grafana#1296)

Signed-off-by: Yuri Nikolic <[email protected]>
observIQ · Aug 3, 2024 · 661b885 · 661b885
1 parent 4965a27
commit 661b885
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 deletions.
diff --git a/mixin-utils/test/test_native-classic-histogram.libsonnet b/mixin-utils/test/test_native-classic-histogram.libsonnet
@@ -153,7 +153,7 @@ test.new(std.thisFile)
   test=test.expect.eq(
     actual=utils.ncHistogramLeRate('request_duration_seconds', 'cluster="cluster1", job="job1"', '0.1'),
     expected={
-      classic: 'rate(request_duration_seconds_bucket{cluster="cluster1", job="job1", le="0.1"}[$__rate_interval])',
+      classic: 'sum (\n  rate(request_duration_seconds_bucket{cluster="cluster1", job="job1", le="0.1"}[$__rate_interval])\n)\n',
       native: 'histogram_fraction(0, 0.1, sum (\n  rate(request_duration_seconds{cluster="cluster1", job="job1"}[$__rate_interval]))\n)\n*\nhistogram_count(sum (\n  rate(request_duration_seconds{cluster="cluster1", job="job1"}[$__rate_interval]))\n)\n',
     },
   )
@@ -163,7 +163,7 @@ test.new(std.thisFile)
   test=test.expect.eq(
     actual=utils.ncHistogramLeRate('request_duration_seconds', 'cluster="cluster1", job="job1"', '10'),
     expected={
-      classic: 'rate(request_duration_seconds_bucket{cluster="cluster1", job="job1", le=~"10|10\\\\.0"}[$__rate_interval])',
+      classic: 'sum (\n  rate(request_duration_seconds_bucket{cluster="cluster1", job="job1", le=~"10|10\\\\.0"}[$__rate_interval])\n)\n',
       native: 'histogram_fraction(0, 10.0, sum (\n  rate(request_duration_seconds{cluster="cluster1", job="job1"}[$__rate_interval]))\n)\n*\nhistogram_count(sum (\n  rate(request_duration_seconds{cluster="cluster1", job="job1"}[$__rate_interval]))\n)\n',
     },
   )
@@ -173,7 +173,7 @@ test.new(std.thisFile)
   test=test.expect.eq(
     actual=utils.ncHistogramLeRate('request_duration_seconds', 'cluster="cluster1", job="job1"', '+Inf'),
     expected={
-      classic: 'rate(request_duration_seconds_bucket{cluster="cluster1", job="job1", le="+Inf"}[$__rate_interval])',
+      classic: 'sum (\n  rate(request_duration_seconds_bucket{cluster="cluster1", job="job1", le="+Inf"}[$__rate_interval])\n)\n',
       native: 'histogram_fraction(0, +Inf, sum (\n  rate(request_duration_seconds{cluster="cluster1", job="job1"}[$__rate_interval]))\n)\n*\nhistogram_count(sum (\n  rate(request_duration_seconds{cluster="cluster1", job="job1"}[$__rate_interval]))\n)\n',
     },
   )
@@ -183,7 +183,7 @@ test.new(std.thisFile)
   test=test.expect.eq(
     actual=utils.ncHistogramLeRate('request_duration_seconds', 'cluster="cluster1", job="job1"', '-Inf'),
     expected={
-      classic: 'rate(request_duration_seconds_bucket{cluster="cluster1", job="job1", le="-Inf"}[$__rate_interval])',
+      classic: 'sum (\n  rate(request_duration_seconds_bucket{cluster="cluster1", job="job1", le="-Inf"}[$__rate_interval])\n)\n',
       native: 'histogram_fraction(0, -Inf, sum (\n  rate(request_duration_seconds{cluster="cluster1", job="job1"}[$__rate_interval]))\n)\n*\nhistogram_count(sum (\n  rate(request_duration_seconds{cluster="cluster1", job="job1"}[$__rate_interval]))\n)\n',
     },
   )
@@ -193,7 +193,7 @@ test.new(std.thisFile)
   test=test.expect.eq(
     actual=utils.ncHistogramLeRate('request_duration_seconds', 'cluster="cluster1", job="job1"', '0.1', '5m'),
     expected={
-      classic: 'rate(request_duration_seconds_bucket{cluster="cluster1", job="job1", le="0.1"}[5m])',
+      classic: 'sum (\n  rate(request_duration_seconds_bucket{cluster="cluster1", job="job1", le="0.1"}[5m])\n)\n',
       native: 'histogram_fraction(0, 0.1, sum (\n  rate(request_duration_seconds{cluster="cluster1", job="job1"}[5m]))\n)\n*\nhistogram_count(sum (\n  rate(request_duration_seconds{cluster="cluster1", job="job1"}[5m]))\n)\n',
     },
   )
@@ -203,7 +203,7 @@ test.new(std.thisFile)
   test=test.expect.eq(
     actual=utils.ncHistogramLeRate('request_duration_seconds', 'cluster="cluster1", job="job1"', '0.1', sum_by=['cluster', 'namespace']),
     expected={
-      classic: 'rate(request_duration_seconds_bucket{cluster="cluster1", job="job1", le="0.1"}[$__rate_interval])',
+      classic: 'sum by (cluster, namespace) (\n  rate(request_duration_seconds_bucket{cluster="cluster1", job="job1", le="0.1"}[$__rate_interval])\n)\n',
       native: 'histogram_fraction(0, 0.1, sum by (cluster, namespace) (\n  rate(request_duration_seconds{cluster="cluster1", job="job1"}[$__rate_interval]))\n)\n*\nhistogram_count(sum by (cluster, namespace) (\n  rate(request_duration_seconds{cluster="cluster1", job="job1"}[$__rate_interval]))\n)\n',
     },
   )

diff --git a/mixin-utils/utils.libsonnet b/mixin-utils/utils.libsonnet
@@ -164,13 +164,18 @@ local g = import 'grafana-builder/grafana.libsonnet';
         selector: selector,
         sumBy: sumBy,
       },
-      classic: 'rate(%(metric)s_bucket{%(selector)s, %(le)s}[%(rateInterval)s])' % {
+      classic: |||
+        sum%(sumBy)s(
+          rate(%(metric)s_bucket{%(selector)s, %(le)s}[%(rateInterval)s])
+        )
+      ||| % {
         // le is treated as string, thus it needs to account for Prometheus text format not having '.0', but OpenMetrics having it.
         // Also the resulting string in yaml is stored directly, so the \\ needs to be escaped to \\\\.
         le: if isWholeNumber(le) then 'le=~"%(le)s|%(le)s\\\\.0"' % { le: le } else 'le="%s"' % le,
         metric: metric,
         rateInterval: rate_interval,
         selector: selector,
+        sumBy: sumBy,
       },
     },