From 52662e81053be6daa3abe3b6ae91213ccc531144 Mon Sep 17 00:00:00 2001 From: David N Perkins Date: Fri, 7 Jan 2022 15:31:37 -0500 Subject: [PATCH 1/2] Added metric name to histogram quartile binning and associated unit test Signed-off-by: David N Perkins --- promql/functions.go | 7 +++++-- promql/testdata/histograms.test | 9 +++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/promql/functions.go b/promql/functions.go index fbafc7864..3fad20d15 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -791,7 +791,7 @@ func funcPredictLinear(vals []parser.Value, args parser.Expressions, enh *EvalNo func funcHistogramQuantile(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper) Vector { q := vals[0].(Vector)[0].V inVec := vals[1].(Vector) - sigf := signatureFunc(false, enh.lblBuf, excludedLabels...) + sigf := signatureFunc(false, enh.lblBuf, labels.BucketLabel) if enh.signatureToMetricWithBuckets == nil { enh.signatureToMetricWithBuckets = map[string]*metricWithBuckets{} @@ -810,11 +810,14 @@ func funcHistogramQuantile(vals []parser.Value, args parser.Expressions, enh *Ev continue } l := sigf(el.Metric) + // Add the metric name (which is always removed) to the signature to prevent combining multiple histograms + // with the same label set + l = l + el.Metric.Get(model.MetricNameLabel) mb, ok := enh.signatureToMetricWithBuckets[l] if !ok { el.Metric = labels.NewBuilder(el.Metric). - Del(labels.BucketLabel, labels.MetricName). + Del(excludedLabels...). Labels() mb = &metricWithBuckets{el.Metric, nil} diff --git a/promql/testdata/histograms.test b/promql/testdata/histograms.test index 07e9a9225..508f1efac 100644 --- a/promql/testdata/histograms.test +++ b/promql/testdata/histograms.test @@ -219,3 +219,12 @@ load 5m eval instant at 50m histogram_quantile(0.2, rate(empty_bucket[5m])) {instance="ins1", job="job1"} NaN + +# Load a duplicate histogram with a different name to test failure scenario on multiple histograms with the same label set +# https://github.com/prometheus/prometheus/issues/9910 +load 5m + request_duration_seconds2_bucket{job="job1", instance="ins1", le="0.1"} 0+1x10 + request_duration_seconds2_bucket{job="job1", instance="ins1", le="0.2"} 0+3x10 + request_duration_seconds2_bucket{job="job1", instance="ins1", le="+Inf"} 0+4x10 + +eval_fail instant at 50m histogram_quantile(0.99, {__name__=~"request_duration.*"}) From 472456efdeabf007eda963ec53ff67cc6c75d690 Mon Sep 17 00:00:00 2001 From: David N Perkins Date: Fri, 7 Jan 2022 15:38:15 -0500 Subject: [PATCH 2/2] Added issue link to the comment Signed-off-by: David N Perkins --- promql/functions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/promql/functions.go b/promql/functions.go index 3fad20d15..20eafd0e6 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -811,7 +811,7 @@ func funcHistogramQuantile(vals []parser.Value, args parser.Expressions, enh *Ev } l := sigf(el.Metric) // Add the metric name (which is always removed) to the signature to prevent combining multiple histograms - // with the same label set + // with the same label set. See https://github.com/prometheus/prometheus/issues/9910 l = l + el.Metric.Get(model.MetricNameLabel) mb, ok := enh.signatureToMetricWithBuckets[l]