Merge 6834572f73ce05012b7a398a5102b7e589192ab7 into cb096a8ea894af06aff1c4846857800a1cea9b8d

promql: fix rate calculation with a counter reset after the 1st sample

.github/workflows/ci.yml

@@ -233,6 +233,8 @@ jobs:
           docker_hub_password: ${{ secrets.docker_hub_password }}
           quay_io_login: ${{ secrets.quay_io_login }}
           quay_io_password: ${{ secrets.quay_io_password }}
+          ghcr_io_login: ${{ secrets.ghcr_io_login }}
+          ghcr_io_password: ${{ secrets.ghcr_io_password }}
           github_token: ${{ secrets.PROMBOT_GITHUB_TOKEN }}
   publish_ui_release:
     name: Publish UI on npm Registry

promql/engine.go

@@ -2357,6 +2357,11 @@ func (ev *evaluator) matrixIterSlice(
 		}
 	}
 
+	if mint == maxt {
+		// Empty range: return the empty slices.
+		return floats, histograms
+	}
+
 	soughtValueType := it.Seek(maxt)
 	if soughtValueType == chunkenc.ValNone {
 		if it.Err() != nil {

promql/engine_test.go

@@ -1900,6 +1900,15 @@ func TestSubquerySelector(t *testing.T) {
 					},
 					Start: time.Unix(35, 0),
 				},
+				{
+					Query: "metric[0:10s]",
+					Result: promql.Result{
+						nil,
+						promql.Matrix{},
+						nil,
+					},
+					Start: time.Unix(10, 0),
+				},
 			},
 		},
 		{
@@ -3199,6 +3208,7 @@ func TestInstantQueryWithRangeVectorSelector(t *testing.T) {
 		load 1m
 			some_metric{env="1"} 0+1x4
 			some_metric{env="2"} 0+2x4
+			some_metric{env="3"} {{count:0}}+{{count:1}}x4
 			some_metric_with_stale_marker 0 1 stale 3
 	`)
 	t.Cleanup(func() { require.NoError(t, storage.Close()) })
@@ -3226,6 +3236,13 @@ func TestInstantQueryWithRangeVectorSelector(t *testing.T) {
 						{T: timestamp.FromTime(baseT.Add(2 * time.Minute)), F: 4},
 					},
 				},
+				{
+					Metric: labels.FromStrings("__name__", "some_metric", "env", "3"),
+					Histograms: []promql.HPoint{
+						{T: timestamp.FromTime(baseT.Add(time.Minute)), H: &histogram.FloatHistogram{Count: 1, CounterResetHint: histogram.NotCounterReset}},
+						{T: timestamp.FromTime(baseT.Add(2 * time.Minute)), H: &histogram.FloatHistogram{Count: 2, CounterResetHint: histogram.NotCounterReset}},
+					},
+				},
 			},
 		},
 		"matches no series": {
@@ -3251,6 +3268,11 @@ func TestInstantQueryWithRangeVectorSelector(t *testing.T) {
 				},
 			},
 		},
+		"matches series but range is 0": {
+			expr:     "some_metric[0]",
+			ts:       baseT.Add(2 * time.Minute),
+			expected: promql.Matrix{},
+		},
 	}
 
 	for name, testCase := range testCases {

promql/functions.go

@@ -187,35 +187,48 @@ func extrapolatedRate(vals []parser.Value, args parser.Expressions, enh *EvalNod
 // not a histogram, and a warning wrapped in an annotation in that case.
 // Otherwise, it returns the calculated histogram and an empty annotation.
 func histogramRate(points []HPoint, isCounter bool, metricName string, pos posrange.PositionRange) (*histogram.FloatHistogram, annotations.Annotations) {
-	prev := points[0].H
-	usingCustomBuckets := prev.UsesCustomBuckets()
-	last := points[len(points)-1].H
+	var (
+		prev               = points[0].H
+		usingCustomBuckets = prev.UsesCustomBuckets()
+		last               = points[len(points)-1].H
+		annos              annotations.Annotations
+	)
+
 	if last == nil {
-		return nil, annotations.New().Add(annotations.NewMixedFloatsHistogramsWarning(metricName, pos))
+		return nil, annos.Add(annotations.NewMixedFloatsHistogramsWarning(metricName, pos))
 	}
 
-	minSchema := prev.Schema
-	if last.Schema < minSchema {
-		minSchema = last.Schema
+	// We check for gauge type histograms in the loop below, but the loop
+	// below does not run on the first and last point, so check the first
+	// and last point now.
+	if isCounter && (prev.CounterResetHint == histogram.GaugeType || last.CounterResetHint == histogram.GaugeType) {
+		annos.Add(annotations.NewNativeHistogramNotCounterWarning(metricName, pos))
+	}
+
+	// Null out the 1st sample if there is a counter reset between the 1st
+	// and 2nd. In this case, we want to ignore any incompatibility in the
+	// bucket layout of the 1st sample because we do not need to look at it.
+	if isCounter && len(points) > 1 {
+		second := points[1].H
+		if second != nil && second.DetectReset(prev) {
+			prev = &histogram.FloatHistogram{}
+			prev.Schema = second.Schema
+			prev.CustomValues = second.CustomValues
+			usingCustomBuckets = second.UsesCustomBuckets()
+		}
 	}
 
 	if last.UsesCustomBuckets() != usingCustomBuckets {
-		return nil, annotations.New().Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, pos))
-	}
-
-	var annos annotations.Annotations
-
-	// We check for gauge type histograms in the loop below, but the loop below does not run on the first and last point,
-	// so check the first and last point now.
-	if isCounter && (prev.CounterResetHint == histogram.GaugeType || last.CounterResetHint == histogram.GaugeType) {
-		annos.Add(annotations.NewNativeHistogramNotCounterWarning(metricName, pos))
+		return nil, annos.Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, pos))
 	}
 
 	// First iteration to find out two things:
 	// - What's the smallest relevant schema?
 	// - Are all data points histograms?
-	//   TODO(beorn7): Find a way to check that earlier, e.g. by handing in a
-	//   []FloatPoint and a []HistogramPoint separately.
+	minSchema := prev.Schema
+	if last.Schema < minSchema {
+		minSchema = last.Schema
+	}
 	for _, currPoint := range points[1 : len(points)-1] {
 		curr := currPoint.H
 		if curr == nil {
@@ -1612,7 +1625,7 @@ func (ev *evaluator) evalLabelReplace(ctx context.Context, args parser.Expressio
 	if err != nil {
 		panic(fmt.Errorf("invalid regular expression in label_replace(): %s", regexStr))
 	}
-	if !model.LabelNameRE.MatchString(dst) {
+	if !model.LabelName(dst).IsValid() {
 		panic(fmt.Errorf("invalid destination label name in label_replace(): %s", dst))
 	}
 
@@ -1689,6 +1702,9 @@ func (ev *evaluator) evalLabelJoin(ctx context.Context, args parser.Expressions)
 			matrix[i].DropName = el.DropName
 		}
 	}
+	if matrix.ContainsSameLabelset() {
+		ev.errorf("vector cannot contain metrics with the same labelset")
+	}
 
 	return matrix, ws
 }

promql/promqltest/testdata/functions.test

@@ -466,7 +466,7 @@ eval instant at 0m label_replace(testmetric, "dst", "", "dst", ".*")
 eval_fail instant at 0m label_replace(testmetric, "dst", "value-$1", "src", "(.*")
 
 # label_replace fails when the destination label name is not a valid Prometheus label name.
-eval_fail instant at 0m label_replace(testmetric, "invalid-label-name", "", "src", "(.*)")
+eval_fail instant at 0m label_replace(testmetric, "\xff", "", "src", "(.*)")
 
 # label_replace fails when there would be duplicated identical output label sets.
 eval_fail instant at 0m label_replace(testmetric, "src", "", "", "")
@@ -499,6 +499,8 @@ eval instant at 20s timestamp(metric)
 load 5m
   testmetric{src="a",src1="b",src2="c",dst="original-destination-value"} 0
   testmetric{src="d",src1="e",src2="f",dst="original-destination-value"} 1
+  dup{label="a", this="a"} 1.0
+  dup{label="b", this="a"} 1.0
 
 # label_join joins all src values in order.
 eval instant at 0m label_join(testmetric, "dst", "-", "src", "src1", "src2")
@@ -530,6 +532,9 @@ eval instant at 0m label_join(testmetric1, "dst", ", ", "src", "src1", "src2")
   testmetric1{src="foo",src1="bar",src2="foobar",dst="foo, bar, foobar"} 0
   testmetric1{src="fizz",src1="buzz",src2="fizzbuzz",dst="fizz, buzz, fizzbuzz"} 1
 
+eval_fail instant at 0m label_join(dup, "label", "", "this")
+  expected_fail_message vector cannot contain metrics with the same labelset
+
 clear
 
 # Tests for vector.

promql/promqltest/testdata/native_histograms.test

@@ -1013,7 +1013,7 @@ eval instant at 5m sum(custom_buckets_histogram)
 
 clear
 
-# Test 'this native histogram metric is not a gauge' warning for rate
+# Test 'this native histogram metric is not a counter' warning for rate
 load 30s
     some_metric {{schema:0 sum:1 count:1 buckets:[1] counter_reset_hint:gauge}} {{schema:0 sum:2 count:2 buckets:[2] counter_reset_hint:gauge}} {{schema:0 sum:3 count:3 buckets:[3] counter_reset_hint:gauge}}
 
@@ -1022,7 +1022,7 @@ eval_warn instant at 30s rate(some_metric[1m])
     {} {{count:0.03333333333333333 sum:0.03333333333333333 buckets:[0.03333333333333333]}}
 
 # Test the case where we have more than two points for rate
-eval_warn instant at 1m rate(some_metric[1m])
+eval_warn instant at 1m rate(some_metric[1m30s])
     {} {{count:0.03333333333333333 sum:0.03333333333333333 buckets:[0.03333333333333333]}}
 
 clear
@@ -1032,20 +1032,20 @@ load 30s
     some_metric {{schema:0 sum:1 count:1 buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:0 sum:5 count:4 buckets:[1 2 1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}}
 
 # Start and end with exponential, with custom in the middle.
-eval_warn instant at 1m rate(some_metric[1m])
+eval_warn instant at 1m rate(some_metric[1m30s])
     # Should produce no results.
 
 # Start and end with custom, with exponential in the middle.
-eval_warn instant at 1m30s rate(some_metric[1m])
+eval_warn instant at 1m30s rate(some_metric[1m30s])
     # Should produce no results.
 
-# Start with custom, end with exponential.
-eval_warn instant at 1m rate(some_metric[1m])
-    # Should produce no results.
+# Start with custom, end with exponential. Return the exponential histogram divided by 30.
+eval instant at 1m rate(some_metric[1m])
+    {} {{schema:0 sum:0.16666666666666666 count:0.13333333333333333 buckets:[0.03333333333333333 0.06666666666666667 0.03333333333333333]}}
 
-# Start with exponential, end with custom.
-eval_warn instant at 30s rate(some_metric[1m])
-    # Should produce no results.
+# Start with exponential, end with custom. Return the custom buckets histogram divided by 30.
+eval instant at 30s rate(some_metric[1m])
+    {} {{schema:-53 sum:0.03333333333333333 count:0.03333333333333333 custom_values:[5 10] buckets:[0.03333333333333333]}}
 
 clear
 
@@ -1179,7 +1179,10 @@ eval_info range from 0 to 6m step 6m metric2 > metric2
 clear
 
 load 6m
-  nhcb_metric {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}}
+  nhcb_metric {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}}
+
+# If evaluating at 12m, the first two NHCBs have the same custom values
+# while the 3rd one has different ones.
 
 eval_warn instant at 12m sum_over_time(nhcb_metric[13m])
 
@@ -1206,6 +1209,38 @@ eval_warn instant at 12m rate(nhcb_metric[13m])
 eval instant at 12m resets(nhcb_metric[13m])
 {} 1
 
+# Now doing the same again, but at 18m, where the first NHCB has
+# different custom_values compared to the other two. This now
+# works with no warning for increase() and rate(). No change
+# otherwise.
+
+eval_warn instant at 18m sum_over_time(nhcb_metric[13m])
+
+eval_warn instant at 18m avg_over_time(nhcb_metric[13m])
+
+eval instant at 18m last_over_time(nhcb_metric[13m])
+nhcb_metric{} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}}
+
+eval instant at 18m count_over_time(nhcb_metric[13m])
+{} 3
+
+eval instant at 18m present_over_time(nhcb_metric[13m])
+{} 1
+
+eval instant at 18m changes(nhcb_metric[13m])
+{} 1
+
+eval_warn instant at 18m delta(nhcb_metric[13m])
+
+eval instant at 18m increase(nhcb_metric[13m])
+{} {{schema:-53 count:1.0833333333333333 sum:1.0833333333333333 custom_values:[5 10] buckets:[1.0833333333333333]}}
+
+eval instant at 18m rate(nhcb_metric[13m])
+{} {{schema:-53 count:0.0013888888888888887 sum:0.0013888888888888887 custom_values:[5 10] buckets:[0.0013888888888888887]}}
+
+eval instant at 18m resets(nhcb_metric[13m])
+{} 1
+
 clear
 
 load 1m