Add anchored and smoothed to vector selectors.#16457
Add anchored and smoothed to vector selectors.#16457roidelapluie merged 4 commits intoprometheus:mainfrom
Conversation
|
Hi @roidelapluie, I have a question about From what I understand:
Does the Could you clarify how |
|
Design doc coming but shortly: anchored changes the underlying vector so that "increase" does not extrapolate. Therefore you have the result without extrapolation. |
|
Proposal: prometheus/proposals#52 |
9d019a1 to
53921a1
Compare
926638f to
47fdeb9
Compare
|
Hey any updates on that? we need that for increase() |
0077020 to
fee0fc4
Compare
|
bench.out.txt TL;DR Single series (a_one)
100 series (a_hundred)
Sparse series
|
beorn7
left a comment
There was a problem hiding this comment.
Thanks for doing this epic work.
Some of my comments might be a result of my limited understanding of the PromQL engine (but then at least they would point to the need of a code comment).
Note: The syntax changes need to be mirrored in the frontend code to enable auto-completion and avoid squiggly lines underneath smoothed and anchored. This could be done in a separate PR, but experience tells that users are very confused if we delay it too much.
|
@roidelapluie If things are getting complicated to explain, I'm happy to jump on a call. |
3d0d394 to
a46ba27
Compare
krajorama
left a comment
There was a problem hiding this comment.
this is looking very good, I've left some comments and plan to come back to this for another round
beorn7
left a comment
There was a problem hiding this comment.
Not much substantial left from my side. (But @krajorama's comments are also still relevant, of course.)
9da15e1 to
b9e3962
Compare
|
@beorn7 @krajorama I have addressed the reviews. |
|
Thanks. Will have a look ASAP. In different news: @charleskorn you should know that this is happening (because it will be needed in MQE, too, but you might also see something we don't). |
beorn7
left a comment
There was a problem hiding this comment.
No more comments from my side. I leave it to @krajorama and @charleskorn to check off once their comments have been addressed. Then we can merge. 🎉
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>
Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com> Signed-off-by: Julien <291750+roidelapluie@users.noreply.github.com>
Signed-off-by: Julien Pivotto <291750+roidelapluie@users.noreply.github.com>
Signed-off-by: Julien Pivotto <291750+roidelapluie@users.noreply.github.com>
krajorama
left a comment
There was a problem hiding this comment.
LGTM, there might be see some issues when real samples meet the code, but as far as I see the feature is isolated with the feature flag.
|
We still need tracking issues for the frontend code update and for native histogram support. |
…#13398) #### What this PR does This PR adds support for the `anchored` and `smoothed` range selector modifiers into the Mimir query engine. ie `eval instant at 60s metric[1m] anchored` or `eval instant at 60s metric[1m] smoothed` This is the equivalent implementation to [prometheus/pull/16457](prometheus/prometheus#16457 ) and the original proposal for this extension can be [found here](prometheus/proposals#52). A new CLI argument has been added to enable these extensions; ```-query-frontend.enabled-promql-extended-range-selectors=smoothed,anchored``` The `experimental_functions.go` middleware has been extended (and renamed) to also manage the tenant access to use these extensions. As part of adding support for this, the error messages when a function/aggregate/modifier is not enabled has been updated to correctly identify if the denied keyword is a function, aggregate or extended range modifier. #### Which issue(s) this PR fixes or relates to Fixes #3260 #### Checklist - [ x] Tests updated. - [ ] Documentation added. - [x ] `CHANGELOG.md` updated - the order of entries should be `[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If changelog entry is not needed, please add the `changelog-not-needed` label to the PR. - [x ] [`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md) updated with experimental features. Notes to reviewers; * there is a prometheus PR which will need to be merged and vendored into mimir - prometheus/prometheus#17479. This is not a blocker, but once merged will enable full use of upstream test files. The other note is to call attention the oddity in the use of `smoothed` ranges in rate/increase functions. Prometheus calculates the range boundary values differently when the range is used in one of these functions. In the prometheus implementation, the entire matrix is re-walked to do these different calculations. See https://github.com/prometheus/prometheus/blob/main/promql/functions.go#L128-L173 To avoid this, you will see that I pre-calculate these alternate values on the range boundary and pass these along with the step data. These are interpolated calculations and it would seem more efficient to pre-calculate these and not use them rather then to have to later re-walk the data. I also did not attempt to determine if the parent of the range vector was a rate/increase function and use this to decide which boundary point calculation to use. The reason for this was in case this range vector result was ever cached / re-used then this implementation may be confusing. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds MQE support for experimental `smoothed` and `anchored` range selector modifiers with per-tenant enablement, updated planning/execution, and docs/flags. > > - **MQE/Execution**: > - Implement extended range selector modifiers `smoothed` and `anchored` in range and instant selectors, including extended-boundary handling and smoothed basis points for rate/increase. > - Update rate/increase/delta logic to honor modifiers; disallow with native histograms; validate modifier-function compatibility. > - Enable parser extended-range selectors in frontend/querier; clone `Anchored`/`Smoothed` in AST. > - **Planning/Protocol**: > - Propagate modifier flags through planning (core proto), raise minimum plan version to `V4`. > - Add planner errors for unsupported functions with these modifiers. > - **Middleware/Gating**: > - Replace experimental functions middleware with unified experimental features middleware to gate functions, aggregations, and extended range modifiers; improve error messages per feature type. > - **Config/CLI/Docs**: > - Add `-query-frontend.enabled-promql-extended-range-selectors` (per-tenant) with defaults, help text, config descriptor, and operations defaults. > - Update docs (`about-versioning`, configuration parameters) and CHANGELOG entries. > - **Tests/Benchmarks**: > - Add extensive unit/plan tests, testdata, and benchmark cases for new modifiers. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 49b1131. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Taylor C <41653732+tacole02@users.noreply.github.com> Co-authored-by: Charles Korn <charles.korn@grafana.com> Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com> Co-authored-by: Nick Pillitteri <56quarters@users.noreply.github.com>
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.
Design doc: prometheus/proposals#53
Exemple usage
increase(caddy_http_requests_total[5m] anchored)(equivalent ofcaddy_http_requests_total - caddy_http_requests_total offset 5mbut takes counter reset into consideration)rate(caddy_http_requests_total[step()] smoothed)Other limitations
Does not work with Subqueries and Native Histograms.