Skip to content

Support anchored and smoothed modifiers#696

Open
sylr wants to merge 9 commits intothanos-io:mainfrom
sylr:feat/anchored-smoothed-modifiers
Open

Support anchored and smoothed modifiers#696
sylr wants to merge 9 commits intothanos-io:mainfrom
sylr:feat/anchored-smoothed-modifiers

Conversation

@sylr
Copy link
Copy Markdown

@sylr sylr commented Mar 20, 2026

Summary

Implements Prometheus proposal 0052 — the anchored and smoothed modifiers for range vector selectors.

Closes #630

  • anchored: Pins boundary values to real observed samples, giving exact integer results for increase() with no extrapolation artifacts
  • smoothed: Interpolates boundary values between nearest samples, giving robust rate estimation that handles scrape jitter
  • Function whitelists enforced: anchored supports rate, increase, delta, resets, changes; smoothed supports rate, increase, delta
  • Gated behind EnableExtendedRangeSelectors engine option (sets parser.EnableExtendedRangeSelectors)
  • Counter-reset handling during smoothed interpolation matches upstream Prometheus v0.310.0 semantics
  • hints.Range preserved as original selector range (not widened), matching Prometheus behavior for backend pushdown

Changes across engine layers

Layer File Change
Engine engine/engine.go EnableExtendedRangeSelectors option, parser flag
Logical plan logicalplan/plan.go Extended storage time ranges
Execution execution/execution.go Function whitelist enforcement, query window extension
Execution execution/parse/functions.go Whitelist maps
Ring buffer ringbuffer/functions.go extendedRangeRate with boundary pick/interpolate and counter-reset correction
Ring buffer ringbuffer/generic.go NewAnchored/NewSmoothed constructors
Storage storage/prometheus/matrix_selector.go Thread anchored/smoothed through operator
Storage storage/prometheus/scanners.go Pass modifier flags

Test plan

  • 18 test cases comparing thanos engine output against upstream Prometheus engine (rate, increase, delta, resets, changes with anchored/smoothed, counter resets, multiple series, quadratic data)
  • Function whitelist enforcement tests (unsupported functions rejected, supported functions accepted)
  • SelectHints regression test asserting hints.Range equals original selector range for standard, anchored, and smoothed queries
  • All existing tests pass (pre-existing fuzz test flake excluded)

🤖 Generated with Claude Code

@sylr sylr force-pushed the feat/anchored-smoothed-modifiers branch from 5a81567 to 5082016 Compare March 20, 2026 08:18
@fpetkovski fpetkovski marked this pull request as ready for review March 20, 2026 08:41
@fpetkovski fpetkovski marked this pull request as draft March 20, 2026 08:41
@sylr sylr force-pushed the feat/anchored-smoothed-modifiers branch from 5082016 to f497bd3 Compare March 20, 2026 09:29
@sylr sylr marked this pull request as ready for review March 20, 2026 09:30
sylr and others added 4 commits March 20, 2026 10:31
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>
@sylr sylr force-pushed the feat/anchored-smoothed-modifiers branch from f497bd3 to 5e88f32 Compare March 20, 2026 09:31
sylr and others added 2 commits March 20, 2026 10:51
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>
"github.com/prometheus/prometheus/promql/promqltest"
promstorage "github.com/prometheus/prometheus/storage"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update engine_test.go to exclude the extended range test cases?

"testdata/extended_vectors.test", // experimental anchored/smoothed modifiers unsupported

That should cover the same test cases from upstream and validate this implementation

@sylr sylr force-pushed the feat/anchored-smoothed-modifiers branch from ed93cda to 857e3f2 Compare March 24, 2026 08:49
sylr and others added 2 commits March 24, 2026 10:42
… 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>
@sylr sylr force-pushed the feat/anchored-smoothed-modifiers branch from 857e3f2 to 203abc8 Compare March 24, 2026 12:45
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants