The funcResets and funcChanges functions now correctly return no result when all float samples are at or before the range start for anchored selectors, consistent with the behavior of rate/increase functions.
Signed-off-by: Julien Pivotto <291750+roidelapluie@users.noreply.github.com>
Fixes#17255.
The implementation happens mostly in the Add and Sub method, but the reconciliation works for all relevant operations. For example, you can now `rate` over a range wherein the custom bucket boundaries are changing.
Any custom bucket reconciliation is flagged with an info-level annotation.
---------
Signed-off-by: Linas Medziunas <linas.medziunas@gmail.com>
Signed-off-by: Linas Medžiūnas <linasm@users.noreply.github.com>
avg_over_time already correctly checked the counter reset hint fo all
histograms, but in sum_over_time, the 1st histogram was missed in the
loop. This commit exposes the bug in a test.
Signed-off-by: beorn7 <beorn@grafana.com>
CI / Report status of build Prometheus for all architectures (push) Has been cancelledDetails
CI / Publish main branch artifacts (push) Has been cancelledDetails
CI / Publish release artefacts (push) Has been cancelledDetails
CI / Publish UI on npm Registry (push) Has been cancelledDetails
Fixes#17308.
As explained adding the warn-annotation about conflicting counter
reset hints doesn't happen consistently. Furthermore, because of
incremental mean calculation being used so far (which includes
subtraction), avg calculation always created gauge histograms.
The fix is to make Sub behave like Add WRT counter reset handling, and
then set the result of a subtraction to gauge explicitly in actual
PromQL subtraction (rather than using Sub for something else, like
incremental mean calculation). Also, track the presence of a
CounterReset hint and a NotCounterReset hint separately for the
entirety of aggregated histograms and create the warn-annotation based
on that.
As a minor fix, this commit also consistently creates the warn
annotation in aggregation to be about "aggregation" rather than
"subtraction" or "addition", because the latter are just internal
operations within the aggregation, which is not of interest for the
user.
Signed-off-by: beorn7 <beorn@grafana.com>
* Add anchored and smoothed to vector selectors.
This adds "anchored" and "smoothed" keywords that can be used following a matrix selector.
"Anchored" selects the last point before the range (or the first one after the range) and adds it at the boundary of the matrix selector.
"Smoothed" applies linear interpolation at the edges using the points around the edges. In the absence of a point before or after the edge, the first or the last point is added to the edge, without interpolation.
*Exemple usage*
* `increase(caddy_http_requests_total[5m] anchored)` (equivalent of *caddy_http_requests_total - caddy_http_requests_total offset 5m* but takes counter reset into consideration)
* `rate(caddy_http_requests_total[step()] smoothed)`
Signed-off-by: Julien Pivotto <291750+roidelapluie@users.noreply.github.com>
* Update docs/feature_flags.md
Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com>
Signed-off-by: Julien <291750+roidelapluie@users.noreply.github.com>
* Smoothed/Anchored rate: Add more tests
Signed-off-by: Julien Pivotto <291750+roidelapluie@users.noreply.github.com>
* Anchored/Smoothed modifier: error out with histograms
Signed-off-by: Julien Pivotto <291750+roidelapluie@users.noreply.github.com>
---------
Signed-off-by: Julien Pivotto <291750+roidelapluie@users.noreply.github.com>
Signed-off-by: Julien <291750+roidelapluie@users.noreply.github.com>
Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com>
Signed-off-by: Andrew Hall <andrew.hall@grafana.com>
Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com>
Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
The current code stops the walk after we have found the first relevant
function. However, in expressions with multiple legs, we will then use
the `HistogramStatsIterator` at most once. This change should make
sure we explore all legs.
The added tests make sure we are not using `HistogramStatsIterator`
where we shouldn't (but the opposite can only be seen in a benchmark
or with a more explicit test).
Signed-off-by: beorn7 <beorn@grafana.com>
Prior to #17127, we needed to add another level in the AST to trigger
the usage of `HistogramStatsIterator`. This is fixed now.
Signed-off-by: beorn7 <beorn@grafana.com>
This is an attempt to make sure that we are not accidentally warning
about conflicting counter resets in rate calculation, see
https://github.com/prometheus/prometheus/pull/17051#issuecomment-3226503416 .
This is done by being more explicit about the warn expectation.
However, as long as
https://github.com/prometheus/prometheus/issues/15346 is not
addressed, we won't be able to trigger the annotation this way anyway.
However, we can play a trick, by wrapping a suitable expression in
`histogram_count` or `histogram_sum`, which will invoke the
`HistogramStatsIterator`, which in turn creates counter reset hints on
the fly. So this commit also adds tests with that, both for absence of
an annotation with `rate` and presence of an annotation with
`sum_over_time`.
Signed-off-by: beorn7 <beorn@grafana.com>
test tbs
Signed-off-by: beorn7 <beorn@grafana.com>
Add further tests for first_over_time (also covering existing
last_over_time, count_over_time, etc) to exercise vectors
containing a mix of float and histogram samples where the
histogram samples do not come last in the series.
This tripped over https://github.com/prometheus/prometheus/issues/17025
so it's structured a bit oddly to work around that bug in the
appender as used by promtest.
Signed-off-by: Craig Ringer <craig.ringer@enterprisedb.com>
Add a first_over_time function, and corresponding ts_of_first_over_time
function. Both are behind the experimental functions feature flag.
Signed-off-by: Craig Ringer <craig.ringer@enterprisedb.com>
See
https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize
for details.
This ran into a few issues (arguably bugs in the modernize tool),
which I will fix in the next commit, so that we have transparency what
was done automatically.
Beyond those hiccups, I believe all the changes applied are
legitimate. Even where there might be no tangible direct gain, I would
argue it's still better to use the "modern" way to avoid micro
discussions in tiny style PRs later.
Signed-off-by: beorn7 <beorn@grafana.com>
CI / Publish main branch artifacts (push) Blocked by required conditionsDetails
CI / Publish release artefacts (push) Blocked by required conditionsDetails
CI / Publish UI on npm Registry (push) Blocked by required conditionsDetails
Scorecards supply-chain security / Scorecards analysis (push) Waiting to runDetails
As `histogram_count` is playing tricks to improve performance, we
better make sure that the limitation of extrapolation below zero still
works as expected.
Signed-off-by: beorn7 <beorn@grafana.com>
CI / Report status of build Prometheus for all architectures (push) Has been cancelledDetails
CI / Publish main branch artifacts (push) Has been cancelledDetails
CI / Publish release artefacts (push) Has been cancelledDetails
CI / Publish UI on npm Registry (push) Has been cancelledDetails
This deals with the count field of native histograms in the same way
as with simple float counters. It then scale the whole histogram with
the same factor as it has scaled the count. This will still allow
individual buckets to get extrapolated below zero, but maybe that is
fine.
This implements approach (2) as described in
https://github.com/prometheus/prometheus/issues/15976#issuecomment-3032095158
Signed-off-by: beorn7 <beorn@grafana.com>
CI / Report status of build Prometheus for all architectures (push) Has been cancelledDetails
CI / Publish main branch artifacts (push) Has been cancelledDetails
CI / Publish release artefacts (push) Has been cancelledDetails
CI / Publish UI on npm Registry (push) Has been cancelledDetails
This shows how float counters cannot go below zero when extrapolationg
for rate/increase, and how histograms do not have that protection yet,
leading to an overestimation of the rate/increase.
This also demonstrates edge cases where the count extrapolation does
not need to be limited, but an individual bucket still goes below
zero.
Signed-off-by: beorn7 <beorn@grafana.com>
step() is a new keyword introduced to represent the query step width in duration expressions.
min(a,b) and max(a,b) return the min and max from two duration expressions.
Signed-off-by: Julien Pivotto <291750+roidelapluie@users.noreply.github.com>
This commit brings back direct mean calculation (for `avg` and
`avg_over_time`) but isn't an outright revert of #16569. It keeps the
improved incremental mean calculation and features generally a bit
cleaner code than before.
Also, this commit...
- ...updates the lengthy comment explaining the whole situation and
trade-offs.
- ...divides the running sum and the Kahan compensation term
separately (in direct mean calculation) to avoid the (unlikely)
possibility that sum and Kahan compensation together ovorflow
float64.
- ...uncomments the tests that should now work again on darwin/arm64.
- ...uncomments the test that should now reliably yield the
(inaccurate) value 0 on all hardware platforms. Also, the test
description has been updated accordingly.
- ...adds avg_over_time tests for zero and one sample in the range.
Signed-off-by: beorn7 <beorn@grafana.com>
The test in question actually worked fine even before #16569. The
finding reported in the comment has turned out to be caused by
something else.
Signed-off-by: beorn7 <beorn@grafana.com>
CI / Publish main branch artifacts (push) Blocked by required conditionsDetails
CI / Publish release artefacts (push) Blocked by required conditionsDetails
CI / Publish UI on npm Registry (push) Blocked by required conditionsDetails
Scorecards supply-chain security / Scorecards analysis (push) Waiting to runDetails
* fix(promql): histogram_quantile NaN observed in native histogram
Fixes: #16578
See the issue for detailed explanation.
When a histogram had only NaN observations and no normal observations,
we returned 0 from the quantile, which is completely wrong. If there were
normal observations but we went over them, we returned the upper bound of
the existing buckets, however that contradicts expectations on
histogram_fraction. Now we return NaN if the quantile is calculated to be
over all normal observations, falling into NaNs (in a virtual +Inf bucket).
We also return info level annotations if we see any NaN observations.
The annotation calls out if we returned NaN or even if we took the
virtual +Inf bucket into account.
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
* fix(promql): histogram_fraction NaN observed in native histogram
Fixes: #16580
According to the specification we should not take NaN observations
into account when calculating the fraction. This commit fixes that
and adds an info level annotation to let the user know about this.
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
This PR fixes a bug in ts_of_last_over_time where the float samples
where used when computing the last timestamp of the histogram samples.
Signed-off-by: Michael Hoffmann <mhoffmann@cloudflare.com>
Split unary operator handling in duration expressions into two specific
cases to fix precedence conflicts:
- Handle unary operators with number literals directly
- Handle unary operators with parenthesized expressions separately
This prevents unary minus from incorrectly binding to subsequent
operators in expressions like `foo offset -1^1`, ensuring it parses
as `(foo offset -1) ^ 1` rather than `foo offset (-1^1)`.
Fixes#16711
Signed-off-by: Julien Pivotto <291750+roidelapluie@users.noreply.github.com>
This commit adds the ts_of_(max,min,last)_over_time functions behind the experimental feature flag.
Signed-off-by: Michael Hoffmann <mhoffmann@cloudflare.com>
Andrew Hall
Julien Pivotto
geogrego
Linas Medžiūnas
zenador
beorn7
Linas Medziunas
Craig Ringer
Neeraj Gartia
Julius Hinze
Matthieu MOREL
Kapil Lamba
Michael Hoffmann
George Krajcsovits
🌲 Harry 🌊 John 🏔