Commit Graph

126 Commits (a574335d6bd09f301c94a909200fce0ae940db8b)

Author SHA1 Message Date
Neeraj Gartia 7fd3b13bd2
[TEST] PromQL: Adds tests with histograms for simple functions (#15488)
adds tests with histograms for simple functions

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>

---------

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
2024-12-04 13:11:05 +01:00
Björn Rabenstein 6a6630d2a7
Merge pull request #15527 from NeerajGartia21/promql/funcResets
[BUGFIX] PromQL: Fix `resets` function for histograms
2024-12-03 22:43:07 +01:00
Neeraj Gartia 4aeed2c4b1 fix resets function for histograms
Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
2024-12-03 19:01:05 +05:30
George Krajcsovits aceabbc81c
Merge pull request #14638 from roidelapluie/evalsubtest
promqltest: run each eval command in a subtest
2024-12-03 12:41:37 +01:00
Björn Rabenstein dfbfe8b5e8
Merge pull request #15479 from NeerajGartia21/promql/datewrapper
[BUGFIX] PromQL: Ignore histograms in all time related functions
2024-11-29 00:26:52 +01:00
Neeraj Gartia 0de340c1b4 ignore histogram in dateWrapper
Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
2024-11-28 01:57:04 +05:30
Neeraj Gartia 36e0897f0f
[BUGFIX] PromQL: Fix behaviour of `changes()` for mix of histograms and floats (#15469)
PromQL: Fix behaviour of changes() for mix of histograms and floats

---------

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
2024-11-27 21:05:08 +01:00
beorn7 a44c449155 promql: Add NaN tests for avg and avg_over_time
Signed-off-by: beorn7 <beorn@grafana.com>
2024-11-27 13:08:34 +01:00
Neeraj Gartia 38bb6ece25
[BUGFIX] PromQL: Fix behaviour of some aggregations with histograms (#15432)
promql: fix some aggregations for histograms

This PR fixes the behaviour of `topk`,`bottomk`, `limitk` and `limit_ratio` with histograms. The fixed behaviour are as follows:
- For `topk` and `bottomk` histograms are ignored and add info annotations added.
- For `limitk` and `limit_ratio` histograms are included in the results(if applicable).

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
2024-11-26 19:12:36 +01:00
György Krajcsovits a48d05912d nhcb: optimize, do not recalculate suffixes multiple times
Reduce string manipulation by just cutting off the histogram suffixes from
the series name label once.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2024-11-25 15:37:38 +01:00
Björn Rabenstein 125a90899c
promqltest: Complete the tests for info annotations (#15429)
promqltest: Complete the tests for info annotations

So far, we did not test for the _absence_ of an info annotation
(because many tests triggered info annotations, which we haven't taken
into account so far).

The test for info annotations was also missed for range queries.

This completes the tests for info annotations (and refactors the many
`if` statements into a somewhat more compact `switch` statement).

It fixes most tests to not emit an info annotation anymore. Or it
changes the `eval` to `eval_info` where we actually want to test for
the info annotation.

It also fixes a few spelling errors in comments.

---------

Signed-off-by: beorn7 <beorn@grafana.com>
Signed-off-by: Björn Rabenstein <github@rabenste.in>
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
2024-11-21 14:20:38 +01:00
Neeraj Gartia a6fb16fcb4
PromQL: Convert more native histogram tests to promql-test framework (#15419)
This converts `TestNativeHistogram_SubOperator` to the promql testing framework. It also removes `TestNativeHistogram_Sum_Count_Add_AvgOperator`, which got converted earlier.

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
2024-11-20 11:41:36 +01:00
Björn Rabenstein 4ef1170868
Merge pull request #15422 from NeerajGartia21/promql-corrections
[BUGFIX] PromQL: Fix `count_values` for histograms
2024-11-20 11:27:32 +01:00
George Krajcsovits 5cd9855999
tests(promql/testdata): add regression test for and-on (#15425)
* tests(promql/testdata): add regression test for and-on

I'd like to use queries of the form "x and on() (vector(y)==1)" to be
able to include and exclude series for dashboards. This helps migration
to native histograms in dashboards by using a dashboard variable to
set "y" to either -1 or 1 to exclude or include the result.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>

---------

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2024-11-20 10:29:18 +01:00
Neeraj Gartia 048222867a fix count_values for histograms
Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
2024-11-20 02:07:31 +05:30
Charles Korn 62e6e55c07
promql: fix issues with comparison binary operations with `bool` modifier and native histograms (#15413)
* Fix issue where comparison operations with `bool` modifier and native histograms return histograms rather than 0 or 1

* Don't emit anything for comparisons between floats and histograms when `bool` modifier is used

* Don't emit anything for comparisons between floats and histograms when `bool` modifier is used between a vector and a scalar

---------

Signed-off-by: Charles Korn <charles.korn@grafana.com>
2024-11-19 09:13:34 +01:00
Neeraj Gartia 789c9b1a5e
[BUGFIX] PromQL: Corrects the behaviour of some operator and aggregators with Native Histograms (#15245)
PromQL: Correct the behaviour of some operator and aggregators with Native Histograms

---------

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
2024-11-12 15:37:05 +01:00
Joshua Hesketh ed2668bbda
Merge branch 'main' into jhesketh/clamp
Signed-off-by: Joshua Hesketh <josh@nitrotech.org>
2024-11-12 10:20:58 +11:00
Bartlomiej Plotka 76432aaf4e
Merge pull request #15220 from prometheus/nhcb-scrape-optimize
perf(nhcb): scrape optimize
2024-11-08 19:02:48 +01:00
Matthieu MOREL af1a19fc78 enable errorf rule from perfsprint linter
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
2024-11-06 16:50:36 +01:00
Bryan Boreham 6979b237b9 Merge branch 'release-2.55' into merge-2.55-into-main2 2024-11-04 12:58:38 +00:00
Ben Ye b7aca45de7 fix round function ignoring enableDelayedNameRemoval feature flag
Signed-off-by: Ben Ye <benye@amazon.com>
2024-10-31 00:30:22 -07:00
György Krajcsovits eafe72a0d0 perf(nhcb): optimize away most allocations in convertnhcb
In general aim for the happy case when the exposer lists the buckets
in ascending order.

Use Compact(2) to compact the result of nhcb convert.

This is more in line with how client_golang optimizes spans vs
buckets.
aef8aedb4b/prometheus/histogram.go (L1485)

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
2024-10-28 08:34:54 +01:00
Joshua Hesketh 14ef1ce8ab Round function should ignore native histograms
As per the documentation, native histograms are skipped. This is in line with other simpleFunc's.

Signed-off-by: Joshua Hesketh <josh@nitrotech.org>
2024-10-17 15:39:48 +11:00
Joshua Hesketh 5a4e4f6936
Fix stddev/stdvar when aggregating histograms, NaNs, and infinities (#14941)
promql: Fix stddev/stdvar when aggregating histograms, NaNs, and Infs

Native histograms are ignored when calculating stddev or stdvar.

However, for the first series of each group, a `groupedAggregation` is
always created. If the first series that was encountered is a histogram
then it acts as the equivalent of a 0 point.

This change creates the first `groupedAggregation` with the `seen` field set to `false` if the point is a
histogram, thus ignoring it like the rest of the aggregation function does. A new `groupedAggregation`
will then be created once an actual float value is encountered.

This commit also sets the `floatValue` field of the `groupedAggregation` to `NaN`, if the first
float value of a group is `NaN` or `±Inf`, so that the outcome is consistently `NaN` once those
values are in the mix.

(The added tests fail without this change).

Signed-off-by: Joshua Hesketh <josh@nitrotech.org>
Signed-off-by: beorn7 <beorn@grafana.com>

---------

Signed-off-by: Joshua Hesketh <josh@nitrotech.org>
Signed-off-by: beorn7 <beorn@grafana.com>
Co-authored-by: beorn7 <beorn@grafana.com>
2024-10-16 15:00:46 +02:00
Joshua Hesketh 31d19381f6 Clamp functions should ignore native histograms
As per the documentation, native histograms are skipped. This is in line
with other `simpleFunc`'s.

Signed-off-by: Joshua Hesketh <josh@nitrotech.org>
2024-10-16 15:10:54 +11:00
Neeraj Gartia d4b1f9eb33
Corrects the behaviour of binary opperators between histogram and float (#14726)
promql: corrects binary operators functioning for mixed sample with histogram and float

For invalid pairings of sample types, an annotation is added now.

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>

---------

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
2024-10-15 14:44:36 +02:00
George Krajcsovits 522149a2ae
model: move classic NHCB conversion into its own file (#15156)
* model: move classic to NHCB conversion into its own file

In preparation for #14978.

Author: Jeanette Tan <jeanette.tan@grafana.com>  2024-07-03 11:56:48
Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Co-authored-by: Jeanette Tan <jeanette.tan@grafana.com>
Co-authored-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>

* Better naming from review comment

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>

* Add doc strings.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>

---------

Signed-off-by: Jeanette Tan <jeanette.tan@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Co-authored-by: Jeanette Tan <jeanette.tan@grafana.com>
2024-10-14 14:23:11 +02:00
Fiona Liao 8650d25804
Add additional basic nhcb unit tests (#15086)
* Add additional basic nhcb unit tests
* Update promql/promqltest/testdata/histograms.test

Signed-off-by: Fiona Liao <fiona.liao@grafana.com>
Signed-off-by: Fiona Liao <fiona.y.liao@gmail.com>
Co-authored-by: George Krajcsovits <krajorama@users.noreply.github.com>
2024-10-08 14:34:32 +02:00
Neeraj Gartia 8aef821e10 eval_info command
Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
2024-09-27 01:10:00 +05:30
Björn Rabenstein 5b9148e552
Merge pull request #14820 from charleskorn/promqltest-native-histogram-format
promqltest: use test expression format for histograms in assertion failure messages and include reset hint in the test expression
2024-09-20 16:47:08 +02:00
Björn Rabenstein df9916ef66
Merge pull request #14677 from prometheus/beorn7/histogram
promql(native histograms): Introduce exponential interpolation
2024-09-19 18:08:59 +02:00
Jan Fajerski 96e5a94d29 promql: rename holt_winters to double_exponential_smoothing
Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
2024-09-19 15:29:01 +02:00
Björn Rabenstein 1639450172 Merge pull request #14821 from charleskorn/nh-negative-multiplication-division
promql: correctly handle unary negation of native histograms and add tests for multiplication and division of native histograms by negative scalars
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
2024-09-19 14:07:37 +01:00
beorn7 6fcd225aee promql(native histograms): Introduce exponential interpolation
The linear interpolation (assuming that observations are uniformly
distributed within a bucket) is a solid and simple assumption in lack
of any other information. However, the exponential bucketing used by
standard schemas of native histograms has been chosen to cover the
whole range of observations in a way that bucket populations are
spread out over buckets in a reasonably way for typical distributions
encountered in real-world scenarios.

This is the origin of the idea implemented here: If we divide a given
bucket into two (or more) smaller exponential buckets, we "most
naturally" expect that the samples in the original buckets will split
among those smaller buckets in a more or less uniform fashion. With
this assumption, we end up with an "exponential interpolation", which
therefore appears to be a better match for histograms with exponential
bucketing.

This commit leaves the linear interpolation in place for NHCB, but
changes the interpolation for exponential native histograms to
exponential. This affects `histogram_quantile` and
`histogram_fraction` (because the latter is more or less the inverse
of the former).

The zero bucket has to be treated specially because the assumption
above would lead to an "interpolation to zero" (the bucket density
approaches infinity around zero, and with the postulated uniform usage
of buckets, we would end up with an estimate of zero for all quantiles
ending up in the zero bucket). We simply fall back to linear
interpolation within the zero bucket.

At the same time, this commit makes the call to stick with the
assumption that the zero bucket only contains positive observations
for native histograms without negative buckets (and vice versa). (This
is an assumption relevant for interpolation. It is a mostly academic
point, as the zero bucket is supposed to be very small anyway.
However, in cases where it _is_ relevantly broad, the assumption helps
a lot in practice.)

This commit also updates and completes the documentation to match both
details about interpolation.

As a more high level note: The approach here attempts to strike a
balance between a more simplistic approach without any assumption, and
a more involved approach with more sophisticated assumptions. I will
shortly describe both for reference:

The "zero assumption" approach would be to not interpolate at all, but
_always_ return the harmonic mean of the bucket boundaries of the
bucket the quantile ends up in. This has the advantage of minimizing
the maximum possible relative error of the quantile estimation.
(Depending on the exact definition of the relative error of an
estimation, there is also an argument to return the arithmetic mean of
the bucket boundaries.) While limiting the maximum possible relative
error is a good property, this approach would throw away the
information if a quantile is closer to the upper or lower end of the
population within a bucket. This can be valuable trending information
in a dashboard. With any kind of interpolation, the maximum possible
error of a quantile estimation increases to the full width of a bucket
(i.e. it more than doubles for the harmonic mean approach, and
precisely doubles for the arithmetic mean approach). However, in
return the _expectation value_ of the error decreases. The increase of
the theoretical maximum only has practical relevance for pathologic
distributions. For example, if there are thousand observations within
a bucket, they could _all_ be at the upper bound of the bucket. If the
quantile calculation picks the 1st observation in the bucket as the
relevant one, an interpolation will yield a value close to the lower
bucket boundary, while the true quantile value is close to the upper
boundary.

The "fancy interpolation" approach would be one that analyses the
_actual_ distribution of samples in the histogram. A lot of statistics
could be applied based on the information we have available in the
histogram. This would include the population of neighboring (or even
all) buckets in the histogram. In general, the resolution of a native
histogram should be quite high, and therefore, those "fancy"
approaches would increase the computational cost quite a bit with very
little practical benefits (i.e. just tiny corrections of the estimated
quantile value). The results are also much harder to reason with.

Signed-off-by: beorn7 <beorn@grafana.com>
2024-09-19 14:19:10 +02:00
Jan Fajerski 91608c002f Merge branch 'main' into release-3.0-beta.0
Conflicts:
	scrape/scrape_test.go
          Pick both changes.
2024-09-10 20:51:20 +02:00
Björn Rabenstein 8114f9a281
Merge pull request #14821 from charleskorn/nh-negative-multiplication-division
promql: correctly handle unary negation of native histograms and add tests for multiplication and division of native histograms by negative scalars
2024-09-10 15:23:00 +02:00
Charles Korn e8c7482137
Return negative counts when multiplied or divided by a negative value
Signed-off-by: Charles Korn <charles.korn@grafana.com>
2024-09-09 14:37:59 +10:00
Julien Pivotto ec73d8dd29 promqltest: run each eval command in a subtest
Simpler approach to #14576

Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>
Signed-off-by: Julien <roidelapluie@o11y.eu>
2024-09-06 13:05:46 +02:00
Jan Fajerski fe4289b502 Merge branch 'main' into HEAD 2024-09-04 18:50:00 +02:00
Arve Knudsen 6ef1eb4e68
Merge pull request #14064 from aknuds1/arve/close-engine
Release PromQL engine resources on close, from tests
2024-09-04 12:07:35 +02:00
Charles Korn 113de6301c
Add failing test cases for unary negation and multiplication and division with negative scalars
Signed-off-by: Charles Korn <charles.korn@grafana.com>
2024-09-04 16:20:28 +10:00
Charles Korn 9b451abec7
Make positive and negative bucket counts different in existing test cases
Signed-off-by: Charles Korn <charles.korn@grafana.com>
2024-09-04 16:08:05 +10:00
Charles Korn 90dc1b45db
promqltest: use test expression format for histograms in assertion failure messages
Signed-off-by: Charles Korn <charles.korn@grafana.com>
2024-09-04 15:47:07 +10:00
Arve Knudsen 70bb219d33
Merge pull request #14680 from charleskorn/promqltest-scalar
promqltest: improve error messages when unexpected type is returned, and fix inverted expected and actual values in error message
2024-09-03 10:38:04 +02:00
Arve Knudsen 5dfbcc390e Merge remote-tracking branch 'prometheus/main' into arve/close-engine
Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
2024-09-02 16:26:59 +02:00
Jan Fajerski 956245b25b promqltest: adjust eval times and range selector
In order to fix new tests for changes added in
https://github.com/prometheus/prometheus/pull/13904.

Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
2024-09-02 11:27:39 +02:00
Jan Fajerski 00315ce15e Merge branch 'main' into 3.0-main-sync-24-08-30
using -Xours

Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
2024-09-02 11:27:18 +02:00
Neeraj Gartia 8c7bf39d96
Moves TestNativeHistogram_MulDivOperator to promql testing framework (#14688)
PromQL: add test for mul and div operator

Also, remove the converted test from the engine_test.go file.

This also includes an extension of the test framework to allow NaN/Inf in histogram buckets.

---------

Signed-off-by: Neeraj Gartia <neerajgartia211002@gmail.com>
2024-08-29 16:42:35 +02:00
Jorge Creixell e9e3d64b7c
PromQL engine: Delay deletion of __name__ label to the end of the query evaluation (#14477)
PromQL engine: Delay deletion of __name__ label to the end of the query evaluation

  - This change allows optionally preserving the `__name__` label via the `label_replace` and `label_join` functions, and helps prevent the dreaded "vector cannot contain metrics with the same labelset" error.
  - The implementation extends the `Series` and `Sample` structs with a boolean flag indicating whether the `__name__` label should be deleted at the end of the query evaluation.
  - The `label_replace` and `label_join` functions can still access the value of the `__name__` label, even if it has been previously marked for deletion. If  `__name__` is used as target label, it won't be dropped at the end of the query evaluation.
  - Fixes https://github.com/prometheus/prometheus/issues/11397
  - See https://github.com/jcreixell/prometheus/pull/2 for previous discussion, including the decision to create this PR and benchmark it before considering other alternatives (like refactoring `labels.Labels`).
  - See https://github.com/jcreixell/prometheus/pull/1 for an alternative implementation using a special label instead of boolean flags.
  - Note: a feature flag  `promql-delayed-name-removal` has been added as it changes the behavior of some "weird" queries (see https://github.com/prometheus/prometheus/issues/11397#issuecomment-1451998792)

Example (this always fails, as `__name__` is being dropped by `count_over_time`):

```
count_over_time({__name__!=""}[1m])

=> Error executing query: vector cannot contain metrics with the same labelset
```

Before:

```
label_replace(count_over_time({__name__!=""}[1m]), "__name__", "count_$1", "__name__", "(.+)")

=> Error executing query: vector cannot contain metrics with the same labelset
```

After:

```
label_replace(count_over_time({__name__!=""}[1m]), "__name__", "count_$1", "__name__", "(.+)")

=>
count_go_gc_cycles_automatic_gc_cycles_total{instance="localhost:9090", job="prometheus"} 1
count_go_gc_cycles_forced_gc_cycles_total{instance="localhost:9090", job="prometheus"} 1
...
```

Signed-off-by: Jorge Creixell <jcreixell@gmail.com>

---------

Signed-off-by: Jorge Creixell <jcreixell@gmail.com>
Signed-off-by: Björn Rabenstein <github@rabenste.in>
2024-08-29 15:50:39 +02:00