PromQL: Fix bug with inconsistent results for queries with OR expression and EnableDelayedNameRemoval (#17161)

Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Signed-off-by: zenador <zenador@users.noreply.github.com>
Co-authored-by: Björn Rabenstein <github@rabenste.in>

docs/feature_flags.md

@@ -150,6 +150,8 @@ These queries are rare to occur and easy to fix. (In the above example,
 removing `by (__name__)` doesn't change anything without the feature flag and
 fixes the possible problem with the feature flag.)
 
+It is possible to craft a query that aggregates by `__name__` and puts samples with and without delayed name removal into the same group. In that case, the name is removed from the affected group. Note that this case hardly occurs in queries that fulfill a practical purpose.
+
 ## Auto Reload Config
 
 `--enable-feature=auto-reload-config`

promql/engine.go

@@ -3185,6 +3185,7 @@ type groupedAggregation struct {
 	incrementalMean        bool // True after reverting to incremental calculation of the mean value.
 	counterResetSeen       bool // Counter reset hint CounterReset seen. Currently only used for histogram samples.
 	notCounterResetSeen    bool // Counter reset hint NotCounterReset seen. Currently only used for histogram samples.
+	dropName               bool // True if any sample in this group has DropName set.
 }
 
 // aggregation evaluates sum, avg, count, stdvar, stddev or quantile at one timestep on inputMatrix.
@@ -3213,6 +3214,7 @@ func (ev *evaluator) aggregation(e *parser.AggregateExpr, q float64, inputMatrix
 				floatMean:              f,
 				incompatibleHistograms: false,
 				groupCount:             1,
+				dropName:               inputMatrix[si].DropName,
 			}
 			switch op {
 			case parser.AVG, parser.SUM:
@@ -3269,6 +3271,10 @@ func (ev *evaluator) aggregation(e *parser.AggregateExpr, q float64, inputMatrix
 			continue
 		}
 
+		if inputMatrix[si].DropName {
+			group.dropName = true
+		}
+
 		switch op {
 		case parser.SUM:
 			if h != nil {
@@ -3507,7 +3513,7 @@ func (ev *evaluator) aggregation(e *parser.AggregateExpr, q float64, inputMatrix
 
 		ss := &outputMatrix[ri]
 		addToSeries(ss, enh.Ts, aggr.floatValue, aggr.histogramValue, numSteps)
-		ss.DropName = inputMatrix[ri].DropName
+		ss.DropName = aggr.dropName
 	}
 
 	return annos

promql/promqltest/testdata/name_label_dropping.test

@@ -91,3 +91,38 @@ eval instant at 10m topk(10, sum by (__name__, env) (metric_total{env="1"}))
 
 eval instant at 10m topk(10, sum by (__name__, env) (rate(metric_total{env="1"}[10m])))
 	{env="1"} 0.2
+
+clear
+
+# More testing for __name__ label drop with different input series.
+load 1m
+  metric_total{env="1"} 0+1x10
+  metric_total{env="2"} 0+3x10
+
+# Metric name is preserved as there is no function that drops it.
+eval instant at 10m sum by (__name__) (metric_total{env="1"})
+  metric_total 10
+
+# Metric name is dropped at the end because of rate and because there is no label function to preserve it.
+eval instant at 10m sum by (__name__) (rate(metric_total{env="2"}[5m]))
+  {} 0.05
+
+# Metric name is preserved with label_replace even though it would have been dropped with rate.
+eval instant at 10m label_replace(sum by (__name__) (rate(metric_total{env="2"}[5m])), "__name__", "$1", "__name__", "(.+)")
+  metric_total 0.05
+
+# Combining the above cases in an OR expression, we drop the name if any of the series drops it.
+eval instant at 10m sum by (__name__) (metric_total{env="1"} or rate(metric_total{env="2"}[5m]))
+  {} 10.05
+
+# Changing the order of the OR expression should not change the result.
+eval instant at 10m sum by (__name__) (rate(metric_total{env="2"}[5m]) or metric_total{env="1"})
+  {} 10.05
+
+# With non-matching first selector, we use the second to determine if __name__ is dropped.
+eval instant at 10m sum by (__name__) (metric_total{env="3"} or rate(metric_total{env="2"}[5m]))
+  {} 0.05
+
+# Same as above, but with reversed order.
+eval instant at 10m sum by (__name__) (rate(metric_total{env="3"}[5m]) or metric_total{env="1"})
+  metric_total 10