Repository: prometheus/prometheus (language: go)

Issue:
PromQL: add histogram_avg function for Native Histograms
### Proposal

**Feature Request**

Add `histogram_avg(v instant-vector)` function for Native Histograms for computing the average of observed values, which would essentially replace the current workaround suggested in the documentation (`histogram_sum(...) / histogram_count(...)`).

**Motivation**

1. _Query Language Completeness / Consistency_

PromQL offers `avg` functions on other dimensions. Not having it for Native Histogram observations feels like a gap there:
| Series    | Time | Histogram observations |
| -------- | ------- |------- |
| `sum`  | `sum_over_time` | `histogram_sum` |
| `count`  | `count_over_time` | `histogram_count` |
| `avg`  | `avg_over_time` | **N/A** |
| `stddev` | `stddev_over_time` | `histogram_stddev` | 
| `stdvar ` | `stdvar_over_time ` | `histogram_stdvar` | 

2. _Performance_

A single function would be more performant than executing two functions and a join.

In case this gets a green light, might be able to find some hands to help with the implementation.


Discussion / hints:
@beorn7 any thoughts on this?
Thank you very much. This PR fell through the cracks over the holidays. (Feel free to @-mention me on histogram related PRs. The histogram related code is sprinkled all over the codebase so I cannot be auto-assigned to histogram-related PRs.)

About the actual thing: I'm not sure we want this. The other functions you list are all necessary because there is no other way of getting those results. (And WRT `histogram_sum` and `histogram_count`, there is even the question if we want to make them obsolete by creating new syntax like `my_native_histogram.count`.)

We recently discussed guidelines when to add new PromQL functions at the dev summit, and the bottom line was that we want to avoid functions that are just syntactic sugar, unless they are saving a lot of syntax.

About the performance: We could always optimize the query behind the scenes (difficult with the current implementation of the engine, but it is something we want for other queries anyway), so I would say that performance gain is one of the weaker arguments for introducing a new PromQL function.

I think this needs a wider consensus. I'll put this on the agenda of the dev summit, but I would also like to invite our seniors to comment on it (including but not limited to @juliusv @roidelapluie @bboreham @bwplotka ).
Sure, I was expecting some discussion to be required for an introduction of a new function. 

Regarding
> The other functions you list are all necessary because there is no other way of getting those results.

I guess this does not apply to `avg` aggregation and `avg_over_time` function (which could also be expressed in terms of their `sum`/`count` counterparts).
> I guess this does not apply to avg aggregation and avg_over_time function (which could also be expressed in terms of their sum/count counterparts).

That's actually a good point. (The `avg` aggregator is so old that this thought didn't even cross my mind.)

To make this easier to understand for the interested reader:

`avg(up)` can be expressed as `sum(up) / count(up)`. Likewise, `avg_over_time(up[5m])` can be expressed as `sum_over_time(up[5m]) / count_over_time(up[5m])`.

From that, we can follow that
```PromQL
  histogram_sum(rate(request_duration_seconds[5m]))
/
  histogram_count(rate(request_duration_seconds[5m]))
```
should have an equivalent like
```PromQL
  histogram_avg(rate(request_duration_seconds[5m]))
```

I find this argument compelling enough that I think we should move forward with your PR. I'll review the code at my next convenience. If anyone has concerns, please voice them here, and then we can discuss them in a suitable form (here or during the dev summit). In any case, this is an experimental feature, so we can take it back, should it turn out to be a bad idea.

>  I think we should move forward with your PR. I'll review the code at my next convenience.

Sorry for my confusion. I wrote this while believing to comment on a PR, not an issue.

So the proper wording here should be: Yes, let's do it. If you plan to implement it yourself or collaborate with others to do so, please go ahead.
Thanks Björn. @faustuzas kindly please take this over when ready.

Fix patch:
diff --git a/docs/querying/functions.md b/docs/querying/functions.md
index 375104a0da2..c9e65fe6cca 100644
--- a/docs/querying/functions.md
+++ b/docs/querying/functions.md
@@ -175,6 +175,27 @@ Special cases are:
 `floor(v instant-vector)` rounds the sample values of all elements in `v` down
 to the nearest integer.
 
+## `histogram_avg()`
+
+_This function only acts on native histograms, which are an experimental
+feature. The behavior of this function may change in future versions of
+Prometheus, including its removal from PromQL._
+
+`histogram_avg(v instant-vector)` returns the arithmetic average of observed values stored in
+a native histogram. Samples that are not native histograms are ignored and do
+not show up in the returned vector.
+
+Use `histogram_avg` as demonstrated below to compute the average request duration
+over a 5-minute window from a native histogram:
+
+    histogram_avg(rate(http_request_duration_seconds[5m]))
+
+Which is equivalent to the following query:
+
+      histogram_sum(rate(http_request_duration_seconds[5m]))
+    /
+      histogram_count(rate(http_request_duration_seconds[5m]))
+
 ## `histogram_count()` and `histogram_sum()`
 
 _Both functions only act on native histograms, which are an experimental
@@ -193,13 +214,6 @@ Use `histogram_count` in the following way to calculate a rate of observations
 
     histogram_count(rate(http_request_duration_seconds[10m]))
 
-The additional use of `histogram_sum` enables the calculation of the average of
-observed values (in this case corresponding to “average request duration”):
-
-      histogram_sum(rate(http_request_duration_seconds[10m]))
-	/
-      histogram_count(rate(http_request_duration_seconds[10m]))
-
 ## `histogram_fraction()`
 
 _This function only acts on native histograms, which are an experimental
diff --git a/promql/functions.go b/promql/functions.go
index aef73e7f2b5..941f953588a 100644
--- a/promql/functions.go
+++ b/promql/functions.go
@@ -1081,6 +1081,23 @@ func funcHistogramSum(vals []parser.Value, args parser.Expressions, enh *EvalNod
 	return enh.Out, nil
 }
 
+// === histogram_avg(Vector parser.ValueTypeVector) (Vector, Annotations) ===
+func funcHistogramAvg(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper) (Vector, annotations.Annotations) {
+	inVec := vals[0].(Vector)
+
+	for _, sample := range inVec {
+		// Skip non-histogram samples.
+		if sample.H == nil {
+			continue
+		}
+		enh.Out = append(enh.Out, Sample{
+			Metric: sample.Metric.DropMetricName(),
+			F:      sample.H.Sum / sample.H.Count,
+		})
+	}
+	return enh.Out, nil
+}
+
 // === histogram_stddev(Vector parser.ValueTypeVector) (Vector, Annotations)  ===
 func funcHistogramStdDev(vals []parser.Value, args parser.Expressions, enh *EvalNodeHelper) (Vector, annotations.Annotations) {
 	inVec := vals[0].(Vector)
@@ -1532,6 +1549,7 @@ var FunctionCalls = map[string]FunctionCall{
 	"deriv":              funcDeriv,
 	"exp":                funcExp,
 	"floor":              funcFloor,
+	"histogram_avg":      funcHistogramAvg,
 	"histogram_count":    funcHistogramCount,
 	"histogram_fraction": funcHistogramFraction,
 	"histogram_quantile": funcHistogramQuantile,
diff --git a/promql/parser/functions.go b/promql/parser/functions.go
index 46d50d54761..99b41321fed 100644
--- a/promql/parser/functions.go
+++ b/promql/parser/functions.go
@@ -167,6 +167,11 @@ var Functions = map[string]*Function{
 		ArgTypes:   []ValueType{ValueTypeVector},
 		ReturnType: ValueTypeVector,
 	},
+	"histogram_avg": {
+		Name:       "histogram_avg",
+		ArgTypes:   []ValueType{ValueTypeVector},
+		ReturnType: ValueTypeVector,
+	},
 	"histogram_count": {
 		Name:       "histogram_count",
 		ArgTypes:   []ValueType{ValueTypeVector},
diff --git a/web/ui/module/codemirror-promql/src/complete/promql.terms.ts b/web/ui/module/codemirror-promql/src/complete/promql.terms.ts
index 963fc95f2f8..9d5d55f60b9 100644
--- a/web/ui/module/codemirror-promql/src/complete/promql.terms.ts
+++ b/web/ui/module/codemirror-promql/src/complete/promql.terms.ts
@@ -215,6 +215,12 @@ export const functionIdentifierTerms = [
     info: 'Round down values of input series to nearest integer',
     type: 'function',
   },
+  {
+    label: 'histogram_avg',
+    detail: 'function',
+    info: 'Return the average of observations from a native histogram (experimental feature)',
+    type: 'function',
+  },
   {
     label: 'histogram_count',
     detail: 'function',
diff --git a/web/ui/module/codemirror-promql/src/types/function.ts b/web/ui/module/codemirror-promql/src/types/function.ts
index 36947815866..2505edc2272 100644
--- a/web/ui/module/codemirror-promql/src/types/function.ts
+++ b/web/ui/module/codemirror-promql/src/types/function.ts
@@ -39,6 +39,7 @@ import {
   Deriv,
   Exp,
   Floor,
+  HistogramAvg,
   HistogramCount,
   HistogramFraction,
   HistogramQuantile,
@@ -269,6 +270,12 @@ const promqlFunctions: { [key: number]: PromQLFunction } = {
     variadic: 0,
     returnType: ValueType.vector,
   },
+  [HistogramAvg]: {
+    name: 'histogram_avg',
+    argTypes: [ValueType.vector],
+    variadic: 0,
+    returnType: ValueType.vector,
+  },
   [HistogramCount]: {
     name: 'histogram_count',
     argTypes: [ValueType.vector],
diff --git a/web/ui/module/lezer-promql/src/highlight.js b/web/ui/module/lezer-promql/src/highlight.js
index 1bf342d28f9..53321c75e36 100644
--- a/web/ui/module/lezer-promql/src/highlight.js
+++ b/web/ui/module/lezer-promql/src/highlight.js
@@ -20,7 +20,7 @@ export const promQLHighLight = styleTags({
     NumberLiteral: tags.number,
     Duration: tags.number,
     Identifier: tags.variableName,
-    'Abs Absent AbsentOverTime Acos Acosh Asin Asinh Atan Atanh AvgOverTime Ceil Changes Clamp ClampMax ClampMin Cos Cosh CountOverTime DaysInMonth DayOfMonth DayOfWeek DayOfYear Deg Delta Deriv Exp Floor HistogramCount HistogramFraction HistogramQuantile HistogramSum HoltWinters Hour Idelta Increase Irate LabelReplace LabelJoin LastOverTime Ln Log10 Log2 MaxOverTime MinOverTime Minute Month Pi PredictLinear PresentOverTime QuantileOverTime Rad Rate Resets Round Scalar Sgn Sin Sinh Sort SortDesc SortByLabel SortByLabelDesc Sqrt StddevOverTime StdvarOverTime SumOverTime Tan Tanh Time Timestamp Vector Year':
+    'Abs Absent AbsentOverTime Acos Acosh Asin Asinh Atan Atanh AvgOverTime Ceil Changes Clamp ClampMax ClampMin Cos Cosh CountOverTime DaysInMonth DayOfMonth DayOfWeek DayOfYear Deg Delta Deriv Exp Floor HistogramAvg HistogramCount HistogramFraction HistogramQuantile HistogramSum HoltWinters Hour Idelta Increase Irate LabelReplace LabelJoin LastOverTime Ln Log10 Log2 MaxOverTime MinOverTime Minute Month Pi PredictLinear PresentOverTime QuantileOverTime Rad Rate Resets Round Scalar Sgn Sin Sinh Sort SortDesc SortByLabel SortByLabelDesc Sqrt StddevOverTime StdvarOverTime SumOverTime Tan Tanh Time Timestamp Vector Year':
         tags.function(tags.variableName),
     'Avg Bottomk Count Count_values Group Max Min Quantile Stddev Stdvar Sum Topk': tags.operatorKeyword,
     'By Without Bool On Ignoring GroupLeft GroupRight Offset Start End': tags.modifier,
diff --git a/web/ui/module/lezer-promql/src/promql.grammar b/web/ui/module/lezer-promql/src/promql.grammar
index ab627c82966..49664831793 100644
--- a/web/ui/module/lezer-promql/src/promql.grammar
+++ b/web/ui/module/lezer-promql/src/promql.grammar
@@ -138,6 +138,7 @@ FunctionIdentifier {
   HistogramStdDev |
   HistogramStdVar |
   HistogramSum |
+  HistogramAvg |
   HoltWinters |
   Hour |
   Idelta |
@@ -364,6 +365,7 @@ NumberLiteral  {
   Deriv { condFn<"deriv"> }
   Exp { condFn<"exp"> }
   Floor { condFn<"floor"> }
+  HistogramAvg { condFn<"histogram_avg"> }
   HistogramCount { condFn<"histogram_count"> }
   HistogramFraction { condFn<"histogram_fraction"> }
   HistogramQuantile { condFn<"histogram_quantile"> }


Test patch:
diff --git a/promql/testdata/native_histograms.test b/promql/testdata/native_histograms.test
index b74886f73e7..a772fe88ec6 100644
--- a/promql/testdata/native_histograms.test
+++ b/promql/testdata/native_histograms.test
@@ -11,6 +11,9 @@ eval instant at 5m histogram_count(empty_histogram)
 eval instant at 5m histogram_sum(empty_histogram)
 	{} 0
 
+eval instant at 5m histogram_avg(empty_histogram)
+	{} NaN
+
 eval instant at 5m histogram_fraction(-Inf, +Inf, empty_histogram)
 	{} NaN
 
@@ -31,6 +34,10 @@ eval instant at 5m histogram_count(single_histogram)
 eval instant at 5m histogram_sum(single_histogram)
 	{} 5
 
+# histogram_avg calculates the average from sum and count properties.
+eval instant at 5m histogram_avg(single_histogram)
+	{} 1.25
+
 # We expect half of the values to fall in the range 1 < x <= 2.
 eval instant at 5m histogram_fraction(1, 2, single_histogram)
 	{} 0.5
@@ -55,6 +62,9 @@ eval instant at 5m histogram_count(multi_histogram)
 eval instant at 5m histogram_sum(multi_histogram)
 	{} 5
 
+eval instant at 5m histogram_avg(multi_histogram)
+	{} 1.25
+
 eval instant at 5m histogram_fraction(1, 2, multi_histogram)
 	{} 0.5
 
@@ -69,6 +79,9 @@ eval instant at 50m histogram_count(multi_histogram)
 eval instant at 50m histogram_sum(multi_histogram)
 	{} 5
 
+eval instant at 50m histogram_avg(multi_histogram)
+	{} 1.25
+
 eval instant at 50m histogram_fraction(1, 2, multi_histogram)
 	{} 0.5
 
@@ -89,6 +102,9 @@ eval instant at 5m histogram_count(incr_histogram)
 eval instant at 5m histogram_sum(incr_histogram)
 	{} 6
 
+eval instant at 5m histogram_avg(incr_histogram)
+	{} 1.2
+
 # We expect 3/5ths of the values to fall in the range 1 < x <= 2.
 eval instant at 5m histogram_fraction(1, 2, incr_histogram)
 	{} 0.6
@@ -106,6 +122,9 @@ eval instant at 50m histogram_count(incr_histogram)
 eval instant at 50m histogram_sum(incr_histogram)
 	{} 24
 
+eval instant at 50m histogram_avg(incr_histogram)
+    {} 1.7142857142857142
+
 # We expect 12/14ths of the values to fall in the range 1 < x <= 2.
 eval instant at 50m histogram_fraction(1, 2, incr_histogram)
 	{} 0.8571428571428571
@@ -140,6 +159,9 @@ eval instant at 5m histogram_count(low_res_histogram)
 eval instant at 5m histogram_sum(low_res_histogram)
 	{} 8
 
+eval instant at 5m histogram_avg(low_res_histogram)
+	{} 1.6
+
 # We expect all values to fall into the lower-resolution bucket with the range 1 < x <= 4.
 eval instant at 5m histogram_fraction(1, 4, low_res_histogram)
 	{} 1
@@ -157,6 +179,9 @@ eval instant at 5m histogram_count(single_zero_histogram)
 eval instant at 5m histogram_sum(single_zero_histogram)
 	{} 0.25
 
+eval instant at 5m histogram_avg(single_zero_histogram)
+	{} 0.25
+
 # When only the zero bucket is populated, or there are negative buckets, the distribution is assumed to be equally
 # distributed around zero; i.e. that there are an equal number of positive and negative observations. Therefore the
 # entire distribution must lie within the full range of the zero bucket, in this case: -0.5 < x <= +0.5.
@@ -179,6 +204,9 @@ eval instant at 5m histogram_count(negative_histogram)
 eval instant at 5m histogram_sum(negative_histogram)
 	{} -5
 
+eval instant at 5m histogram_avg(negative_histogram)
+	{} -1.25
+
 # We expect half of the values to fall in the range -2 < x <= -1.
 eval instant at 5m histogram_fraction(-2, -1, negative_histogram)
 	{} 0.5
@@ -199,6 +227,9 @@ eval instant at 10m histogram_count(two_samples_histogram)
 eval instant at 10m histogram_sum(two_samples_histogram)
 	{} -4
 
+eval instant at 10m histogram_avg(two_samples_histogram)
+	{} -1
+
 eval instant at 10m histogram_fraction(-2, -1, two_samples_histogram)
 	{} 0.5
 
@@ -217,6 +248,9 @@ eval instant at 5m histogram_count(balanced_histogram)
 eval instant at 5m histogram_sum(balanced_histogram)
 	{} 0
 
+eval instant at 5m histogram_avg(balanced_histogram)
+	{} 0
+
 eval instant at 5m histogram_fraction(0, 4, balanced_histogram)
 	{} 0.5
 
diff --git a/web/ui/module/codemirror-promql/src/parser/parser.test.ts b/web/ui/module/codemirror-promql/src/parser/parser.test.ts
index 78195a5c644..54b95553cbc 100644
--- a/web/ui/module/codemirror-promql/src/parser/parser.test.ts
+++ b/web/ui/module/codemirror-promql/src/parser/parser.test.ts
@@ -757,6 +757,18 @@ describe('promql operations', () => {
       expectedValueType: ValueType.vector,
       expectedDiag: [],
     },
+    {
+      expr:
+        'histogram_avg(                                           # Root of the query, final result, returns the average of observations.\n' +
+        '  sum by(method, path) (                                 # Argument to histogram_avg(), an aggregated histogram.\n' +
+        '    rate(                                                # Argument to sum(), the per-second increase of a histogram over 5m.\n' +
+        '      demo_api_request_duration_seconds{job="demo"}[5m]  # Argument to rate(), a vector of sparse histogram series over the last 5m.\n' +
+        '    )\n' +
+        '  )\n' +
+        ')',
+      expectedValueType: ValueType.vector,
+      expectedDiag: [],
+    },
     {
       expr:
         'histogram_stddev(                                           # Root of the query, final result, returns the standard deviation of observations.\n' +