Fix avg_over_time for nan and float64 overflows (#7346)

* Fix avg_over_time with Inf and NaN values

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>

promql/engine.go

@@ -2005,7 +2005,25 @@ func (ev *evaluator) aggregation(op parser.ItemType, grouping []string, without
 
 		case parser.AVG:
 			group.groupCount++
-			group.mean += (s.V - group.mean) / float64(group.groupCount)
+			if math.IsInf(group.mean, 0) {
+				if math.IsInf(s.V, 0) && (group.mean > 0) == (s.V > 0) {
+					// The `mean` and `s.V` values are `Inf` of the same sign.  They
+					// can't be subtracted, but the value of `mean` is correct
+					// already.
+					break
+				}
+				if !math.IsInf(s.V, 0) && !math.IsNaN(s.V) {
+					// At this stage, the mean is an infinite. If the added
+					// value is neither an Inf or a Nan, we can keep that mean
+					// value.
+					// This is required because our calculation below removes
+					// the mean value, which would look like Inf += x - Inf and
+					// end up as a NaN.
+					break
+				}
+			}
+			// Divide each side of the `-` by `group.groupCount` to avoid float64 overflows.
+			group.mean += s.V/float64(group.groupCount) - group.mean/float64(group.groupCount)
 
 		case parser.GROUP:
 			// Do nothing. Required to avoid the panic in `default:` below.

promql/functions.go

@@ -354,7 +354,24 @@ func funcAvgOverTime(vals []parser.Value, args parser.Expressions, enh *EvalNode
 		var mean, count float64
 		for _, v := range values {
 			count++
-			mean += (v.V - mean) / count
+			if math.IsInf(mean, 0) {
+				if math.IsInf(v.V, 0) && (mean > 0) == (v.V > 0) {
+					// The `mean` and `v.V` values are `Inf` of the same sign.  They
+					// can't be subtracted, but the value of `mean` is correct
+					// already.
+					continue
+				}
+				if !math.IsInf(v.V, 0) && !math.IsNaN(v.V) {
+					// At this stage, the mean is an infinite. If the added
+					// value is neither an Inf or a Nan, we can keep that mean
+					// value.
+					// This is required because our calculation below removes
+					// the mean value, which would look like Inf += x - Inf and
+					// end up as a NaN.
+					continue
+				}
+			}
+			mean += v.V/count - mean/count
 		}
 		return mean
 	})

promql/testdata/aggregators.test

@@ -320,3 +320,80 @@ eval instant at 1m group without(point)(data)
 
 eval instant at 1m group(foo)
 	{} 1
+
+# Tests for avg.
+clear
+
+load 10s
+	data{test="ten",point="a"} 8
+	data{test="ten",point="b"} 10
+	data{test="ten",point="c"} 12
+	data{test="inf",point="a"} 0
+	data{test="inf",point="b"} Inf
+	data{test="inf",point="d"} Inf
+	data{test="inf",point="c"} 0
+	data{test="-inf",point="a"} -Inf
+	data{test="-inf",point="b"} -Inf
+	data{test="-inf",point="c"} 0
+	data{test="inf2",point="a"} Inf
+	data{test="inf2",point="b"} 0
+	data{test="inf2",point="c"} Inf
+	data{test="-inf2",point="a"} -Inf
+	data{test="-inf2",point="b"} 0
+	data{test="-inf2",point="c"} -Inf
+	data{test="inf3",point="b"} Inf
+	data{test="inf3",point="d"} Inf
+	data{test="inf3",point="c"} Inf
+	data{test="inf3",point="d"} -Inf
+	data{test="-inf3",point="b"} -Inf
+	data{test="-inf3",point="d"} -Inf
+	data{test="-inf3",point="c"} -Inf
+	data{test="-inf3",point="c"} Inf
+	data{test="nan",point="a"} -Inf
+	data{test="nan",point="b"} 0
+	data{test="nan",point="c"} Inf
+	data{test="big",point="a"} 9.988465674311579e+307
+	data{test="big",point="b"} 9.988465674311579e+307
+	data{test="big",point="c"} 9.988465674311579e+307
+	data{test="big",point="d"} 9.988465674311579e+307
+	data{test="-big",point="a"} -9.988465674311579e+307
+	data{test="-big",point="b"} -9.988465674311579e+307
+	data{test="-big",point="c"} -9.988465674311579e+307
+	data{test="-big",point="d"} -9.988465674311579e+307
+	data{test="bigzero",point="a"} -9.988465674311579e+307
+	data{test="bigzero",point="b"} -9.988465674311579e+307
+	data{test="bigzero",point="c"} 9.988465674311579e+307
+	data{test="bigzero",point="d"} 9.988465674311579e+307
+
+eval instant at 1m avg(data{test="ten"})
+	{} 10
+
+eval instant at 1m avg(data{test="inf"})
+	{} Inf
+
+eval instant at 1m avg(data{test="inf2"})
+	{} Inf
+
+eval instant at 1m avg(data{test="inf3"})
+	{} NaN
+
+eval instant at 1m avg(data{test="-inf"})
+	{} -Inf
+
+eval instant at 1m avg(data{test="-inf2"})
+	{} -Inf
+
+eval instant at 1m avg(data{test="-inf3"})
+	{} NaN
+
+eval instant at 1m avg(data{test="nan"})
+	{} NaN
+
+eval instant at 1m avg(data{test="big"})
+	{} 9.988465674311579e+307
+
+eval instant at 1m avg(data{test="-big"})
+	{} -9.988465674311579e+307
+
+eval instant at 1m avg(data{test="bigzero"})
+	{} 0

promql/testdata/functions.test

@@ -395,10 +395,107 @@ eval instant at 8000s holt_winters(http_requests[1m], 0.01, 0.1)
 clear
 load 10s
   metric 1 2 3 4 5
+  metric2 1 2 3 4 Inf
+  metric3 1 2 3 4 -Inf
+  metric4 1 2 3 Inf -Inf
+  metric5 Inf 0 Inf
+  metric5b Inf 0 Inf
+  metric5c Inf Inf Inf -Inf
+  metric6 1 2 3 -Inf -Inf
+  metric6b -Inf 0 -Inf
+  metric6c -Inf -Inf -Inf Inf
+  metric7 1 2 -Inf -Inf Inf
+  metric8 9.988465674311579e+307 9.988465674311579e+307
+  metric9 -9.988465674311579e+307 -9.988465674311579e+307 -9.988465674311579e+307
+  metric10 -9.988465674311579e+307 9.988465674311579e+307
 
 eval instant at 1m avg_over_time(metric[1m])
   {} 3
 
+eval instant at 1m sum_over_time(metric[1m])/count_over_time(metric[1m])
+  {} 3
+
+eval instant at 1m avg_over_time(metric2[1m])
+  {} Inf
+
+eval instant at 1m sum_over_time(metric2[1m])/count_over_time(metric2[1m])
+  {} Inf
+
+eval instant at 1m avg_over_time(metric3[1m])
+  {} -Inf
+
+eval instant at 1m sum_over_time(metric3[1m])/count_over_time(metric3[1m])
+  {} -Inf
+
+eval instant at 1m avg_over_time(metric4[1m])
+  {} NaN
+
+eval instant at 1m sum_over_time(metric4[1m])/count_over_time(metric4[1m])
+  {} NaN
+
+eval instant at 1m avg_over_time(metric5[1m])
+  {} Inf
+
+eval instant at 1m sum_over_time(metric5[1m])/count_over_time(metric5[1m])
+  {} Inf
+
+eval instant at 1m avg_over_time(metric5b[1m])
+  {} Inf
+
+eval instant at 1m sum_over_time(metric5b[1m])/count_over_time(metric5b[1m])
+  {} Inf
+
+eval instant at 1m avg_over_time(metric5c[1m])
+  {} NaN
+
+eval instant at 1m sum_over_time(metric5c[1m])/count_over_time(metric5c[1m])
+  {} NaN
+
+eval instant at 1m avg_over_time(metric6[1m])
+  {} -Inf
+
+eval instant at 1m sum_over_time(metric6[1m])/count_over_time(metric6[1m])
+  {} -Inf
+
+eval instant at 1m avg_over_time(metric6b[1m])
+  {} -Inf
+
+eval instant at 1m sum_over_time(metric6b[1m])/count_over_time(metric6b[1m])
+  {} -Inf
+
+eval instant at 1m avg_over_time(metric6c[1m])
+  {} NaN
+
+eval instant at 1m sum_over_time(metric6c[1m])/count_over_time(metric6c[1m])
+  {} NaN
+
+
+eval instant at 1m avg_over_time(metric7[1m])
+  {} NaN
+
+eval instant at 1m sum_over_time(metric7[1m])/count_over_time(metric7[1m])
+  {} NaN
+
+eval instant at 1m avg_over_time(metric8[1m])
+  {} 9.988465674311579e+307
+
+# This overflows float64.
+eval instant at 1m sum_over_time(metric8[1m])/count_over_time(metric8[1m])
+  {} Inf
+
+eval instant at 1m avg_over_time(metric9[1m])
+  {} -9.988465674311579e+307
+
+# This overflows float64.
+eval instant at 1m sum_over_time(metric9[1m])/count_over_time(metric9[1m])
+  {} -Inf
+
+eval instant at 1m avg_over_time(metric10[1m])
+  {} 0
+
+eval instant at 1m sum_over_time(metric10[1m])/count_over_time(metric10[1m])
+  {} 0
+
 # Tests for stddev_over_time and stdvar_over_time.
 clear
 load 10s