Open
Conversation
5a81567 to 5082016 Compare 5082016 to f497bd3 Compare Implement Prometheus proposal 0052 (extended range selectors) for the thanos promql-engine. The anchored modifier pins boundary values to real samples, while the smoothed modifier interpolates boundary values, eliminating extrapolation artifacts in rate/increase/delta calculations. Closes thanos-io#630 Changes across the engine layers: - engine: add EnableExtendedRangeSelectors option, set parser flag - logicalplan: extend storage time ranges for anchored/smoothed selectors - execution: enforce function whitelists (anchored: rate, increase, delta, resets, changes; smoothed: rate, increase, delta), extend hint ranges - ringbuffer: add Anchored/Smoothed fields to FunctionArgs, implement extendedRangeRate with pickOrInterpolateLeft/Right boundary logic and counter-reset correction - storage/prometheus: thread anchored/smoothed through matrixSelector, use ext-lookback buffers, extend maxt for smoothed post-range samples Constraint: parser.EnableExtendedRangeSelectors is a global variable in prometheus/parser — can only be set to true (additive) to avoid races Rejected: Streaming ring buffers for anchored/smoothed | they lack ext-lookback and boundary interpolation support Confidence: high Scope-risk: narrow Not-tested: native histograms with anchored/smoothed (rejected upstream too) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
…thed Address review feedback on the anchored/smoothed implementation: 1. Counter-reset at boundaries: interpolateAt now handles counter resets during smoothed interpolation (left edge zeros y1, right edge adds y1 to y2), matching upstream Prometheus' interpolate function. correctForCounterResets now checks the right boundary value against the last interior sample, preventing impossible negative increase values when a reset occurs at the range boundary. 2. hints.Range semantics: the widened query window (for ext-lookback) is now only applied to hints.Start/End. hints.Range stays equal to the original selector range so backends using it for resolution selection or pushdown see the true range, matching Prometheus behavior. 3. Added regression test for smoothed increase/rate with counter reset at the range boundary. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
Assert that hints.Range passed to NewMatrixSelector always equals the original selector range, not the widened query window. Uses a spy scanner that captures hints and verifies the contract for standard, anchored, and smoothed selectors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
Remove the leftEdge parameter from interpolateAt. Counter resets during interpolation now always set y1=0 for both edges, matching the upstream simplification in prometheus/prometheus v0.310.0 (promql/functions.go). The old v0.308.0 behavior was asymmetric: left edge zeroed y1, right edge added y1 to y2. The new unified approach is cleaner and correctly models counters as starting from 0 post-reset regardless of which boundary is being interpolated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
f497bd3 to 5e88f32 Compare When step intervals don't align with sample intervals, the sample needed as the ext lookback boundary may be trapped in lastSample from the previous step and not pushed back to the buffer (since its timestamp is <= mint). For anchored/smoothed selectors, push lastSample back into the buffer before Reset so the ext lookback retention logic can decide whether to keep it. This is scoped to anchored/smoothed only to avoid changing existing x-function behavior. Also adds a fuzz test (FuzzAnchoredSmoothedModifiers) that generates random data and step parameters, testing all anchored/smoothed function combinations against the upstream Prometheus engine. The fuzzer found this bug within seconds. Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix a potential slice panic in correctForCounterResets when all interior samples are excluded by the boundary narrowing (corrFirst > corrLast with corrLast = -1). Clamp corrLast to corrFirst-1 to produce a valid empty slice while still allowing the right-boundary counter-reset check to fire. Also remove a duplicate mint computation in selectPoints (dead code), and document the parser.EnableExtendedRangeSelectors process-global limitation in engine.go. Found by architect review. Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
yeya24 reviewed Mar 24, 2026
| "github.com/prometheus/prometheus/promql/promqltest" | ||
| promstorage "github.com/prometheus/prometheus/storage" | ||
| ) | ||
| |
Contributor
There was a problem hiding this comment.
Can we update engine_test.go to exclude the extended range test cases?
promql-engine/engine/engine_test.go
Line 94 in bd201f9
That should cover the same test cases from upstream and validate this implementation
ed93cda to 857e3f2 Compare … fix anchored/smoothed gaps - Replace "proposal 0052" references with actual GitHub link to prometheus/proposals/0052-extended-range-selectors-semantics.md - Remove extended_vectors.test from acceptance test skip list and enable EnableExtendedRangeSelectors in TestPromqlAcceptance - Fix error message format to match Prometheus ("can only be used with: X, Y - not with Z") and defer validation errors to eval time via deferredError operator for expect-fail-msg compatibility - Implement smoothed instant vector interpolation (selectPointSmoothed) matching Prometheus smoothSeries behaviour - Fix anchored changes/resets on sparse data by retaining an additional lookback sample in the ring buffer and adding pickFirstSampleIndex to start comparisons from the correct boundary sample Constraint: error messages must match Prometheus exactly for expect-fail-msg tests Constraint: smoothed instant vectors need forward-looking samples for interpolation Rejected: extending mint in matrix selector for anchored | would over-widen sample window for all functions Confidence: high Scope-risk: moderate Not-tested: smoothed instant vectors with native histograms (returns no data, not an error) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr> … flag The Prometheus acceptance test framework toggles the process-global EnableExtendedRangeSelectors flag during its run. Extended range tests used t.Parallel(), causing them to overlap and see the flag in the wrong state — producing data races and spurious "expected error, got nothing" failures. - Remove t.Parallel() from all extended range tests so they cannot overlap with the acceptance test's flag toggling - Fix whitelist test to check errors at Exec() time (deferred error pattern) instead of at NewInstantQuery() time - Remove redundant flag manipulation in TestPromqlAcceptance cleanup Constraint: parser.EnableExtendedRangeSelectors is a process-global Rejected: sync.Once in engine | flag is legitimately toggled by acceptance tests Confidence: high Scope-risk: narrow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
857e3f2 to 203abc8 Compare Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements Prometheus proposal 0052 — the
anchoredandsmoothedmodifiers for range vector selectors.Closes #630
anchored: Pins boundary values to real observed samples, giving exact integer results forincrease()with no extrapolation artifactssmoothed: Interpolates boundary values between nearest samples, giving robust rate estimation that handles scrape jitterrate,increase,delta,resets,changes; smoothed supportsrate,increase,deltaEnableExtendedRangeSelectorsengine option (setsparser.EnableExtendedRangeSelectors)hints.Rangepreserved as original selector range (not widened), matching Prometheus behavior for backend pushdownChanges across engine layers
engine/engine.goEnableExtendedRangeSelectorsoption, parser flaglogicalplan/plan.goexecution/execution.goexecution/parse/functions.goringbuffer/functions.goextendedRangeRatewith boundary pick/interpolate and counter-reset correctionringbuffer/generic.goNewAnchored/NewSmoothedconstructorsstorage/prometheus/matrix_selector.gostorage/prometheus/scanners.goTest plan
hints.Rangeequals original selector range for standard, anchored, and smoothed queries🤖 Generated with Claude Code