Uh oh!
There was an error while loading. Please reload this page.
Add yrate/yincrease/ydelta (2.55) - #22
Conversation
| eval instant at 26s yrate(http_requests[50s]) | ||
| {path="/foo"} 0.04 | ||
| {path="/bar"} 0.12 |
There was a problem hiding this comment.
Why is this returning 0.12? I thought this would still be 0.1? I guess I don't know what the example samples timestamps are indexed at. Is the first value have a timestamp of 0s or 5s? For this case, the results are that 26s must have an increase of 6 / 50s => 0.12. However, why wouldn't the eval instant at 25s not include 6?
// http_requests{path="/bar"} V: 1 2 3 4 5 6 7 8 9 10 11
T: 0 5 10 15 20 25 30 35 // Index start at 0 Seconds
T: 5 10 15 20 25 30 35 ... // Index start at 5 seconds
There was a problem hiding this comment.
(via Cursor — posting as Colin)
Good question — the sample grid is the key.
Timestamps:load 5s in promqltest puts the first point at t=0, then every 5s (docs). So for /bar:
| T (s) | 0 | 5 | 10 | 15 | 20 | 25 | 30 |
|---|---|---|---|---|---|---|---|
| value | 1 | 2 | 3 | 4 | 5 | 6 | 7 |
yrate range is [rangeStart, rangeEnd) on this PR — inclusive start, exclusive end (rangeEndMsec is the eval instant).
For eval instant at 26s yrate(http_requests[50s]):
rangeStart= −24s,rangeEnd= 26s- Samples included: those with
rangeStart ≤ T < rangeEnd→ 0…25s - Last included point: T=25s, value=6
- Counter baseline: 0 (yrate “unwritten origin”)
yincrease= 6 − 0 = 6 →yrate= 6 / 50s = 0.12
For eval instant at 25s (same 50s window):
rangeEnd= 25s- Sample at T=25s fails
T < 25s→ excluded - Last included point: T=20s, value=5
yincrease= 5 − 0 = 5 →yrate= 0.10
So the 26s vs 25s jump is exactly “did the T=25s sample fall inside the half-open window?” — not xrate-style extrapolation. That’s why xrate stays at 0.1 at both 25s and 26s here (extended-range extrapolation + different boundary handling), while yrate steps when a grid point crosses the exclusive end.
/foo stays at 0.04 across these evals because its counter is flat (1→1→1→2…) so adding or dropping the last boundary sample doesn’t change the increase over this window.
There was a problem hiding this comment.
@ttstarck I noticed that this sample grid simply increases by 1 so it won't do a good job of finding off-by-1 errors, and it doesn't highlight the "unwritten origin" behavior either, since that's also just one. I've asked Cursor to find a better sequence. I suggested this one that increases by one more each time, and never repeats the origin increase:
7 8 10 13 17 22 28
There was a problem hiding this comment.
And it turns out it needed 11 points in total, with implies a maximum internal increase of 10, so it's going to start with 11:
11 12 14 17 21 26 ...
| // yIncrease is a utility function for yincrease/yrate/ydelta. | ||
| // It calculates the increase of the range (allowing for counter resets if isCounter is true), | ||
| // taking into account the sample at the end of the previous range (just before rangeStartMsec). | ||
| // It returns the result across the range [rangeStartMsec, rangeEndMsec). |
There was a problem hiding this comment.
I see we're doing an inclusive range at rangestart but not inclusive at rangeEnd. Is this what we always had 🤔 I guess it doesn't matter that much because the rangeEnd is the instant eval timestamp which won't likely fall on the exact timestamp of a point.
There was a problem hiding this comment.
(from Colin)
I understand we flipped to (rangeStartMsec, rangeEngdMsec] so this comment is likely out of date. I'll have Cursor confirm.
There was a problem hiding this comment.
(via Cursor — posting as Colin)
Yes — on this PR (#22) the y-family range is [rangeStartMsec, rangeEndMsec) (inclusive left, exclusive right). That matches what yIncrease and rangeFromSelectors implement here:
- loop condition:
points[i].T < rangeEndMsec - in-range check:
points[i].T >= rangeStartMsec
So a sample whose timestamp equalsrangeEnd (the eval instant) is not counted — which is exactly what drives the 25s vs 26s /bar difference in the other thread.
Historical note: this is the semantics we shipped on the original 2.53/2.55 yrate port. Colin’s right that it’s out of date relative to later stack PRs: #20 (align-yrate-to-3x-range-boundary) flips to (rangeStartMsec, rangeEndMsec] to match Prometheus 3.x range-selector boundary rules (prometheus#13213). If you’re reviewing the full stack, read the comment on #20+; if you’re reviewing only #22, the [start, end) comment is accurate for this layer.
And agreed it usually doesn’t matter when eval times fall off the scrape grid — these tests use 24s / 25s / 26s evals deliberately so boundary behavior is visible.
f9a1dbd to
9483ce1Compare5d25fc0 to
2e35102Compare
ttstarck
left a comment
There was a problem hiding this comment.
This looks good! Thanks for the responses to my questions
Ported from invoca-2.39.2-extensions and adapted to the 2.53 FPoint API. yIncrease computes the per-range increase using a simple linear algorithm that assumes one extended-range sample is kept before the range start (mirrors the mechanism xrate already uses via ExtRange: true). Handles counter-reset skew when isCounter is true. rangeFromSelectors consolidates the boilerplate for unpacking the matrix-selector arguments shared by yincrease/yrate/ydelta. Both are unexported and not yet wired up; the follow-up commit adds the funcY* wrappers and parser entries. Made-with: Cursor
Adds three new range-vector functions that mirror the shape of the existing xrate/xincrease/xdelta family: - ydelta(m[r]) for gauges: last-in-range minus last-before-range - yincrease(m[r]) for counters: yIncrease with isCounter=true - yrate(m[r]) same as yincrease, divided by range seconds The funcY* wrappers delegate to the yIncrease helper added in the previous commit. Parser entries use ExtRange: true to opt into the extended-range window populated by matrixIterSlice. Smoke-verified with a small loaded-storage test: yrate/yincrease/ ydelta match expected values derived from the yIncrease algorithm (last_in_range - last_before_range + counter_reset_skew), without requiring any engine changes beyond what xrate already relies on. No REPLACE_RATE_FUNCS wiring yet; that lands in a follow-up commit. Made-with: Cursor
Extends the existing REPLACE_RATE_FUNCS init-time hook to also recognise "x"/"X" and "2"/"y"/"Y" values (matching the invoca-2.39.2 extension). The "x" and "y" cases repoint rate/increase/delta at the xrate or yrate implementations respectively, while keeping the x*/y* names available under their original spellings. Also introduces repointParserFunctions and repointFunction as small helpers so the dispatch-table rewrites read cleanly. Made-with: Cursor
Ports the full set of yrate test expectations from invoca-2.39.2-extensions, split across three existing sections of promql/promqltest/testdata/functions.test: - Rate-vs-xrate showcase block: yrate() evals added in parallel with each rate/xrate variant, so readers can see yrate's pre-origin- treated-as-zero behaviour next to the other two semantics. - "Tests for xincrease/xrate" block: replaced the 0+10x10 / 0+10x5 0+10x4 load with the 1000+10x10 / 2000+10x5 5+10x4 load from 2.39.2. Absolute offsets make the yrate family's per-range values visibly diverge from xrate (e.g. yincrease[50s] returns 1090 for /foo while xincrease[50s] returns 90). Re-adds increase() coverage for the same data as a standard-Prometheus reference. - Counter-reset block: switched from 0 1 2 3 2 3 4 to the 2.39.2 1000 1001 1003 1006 4 9 16 load, added yincrease coverage at the 30m / 10m / 5m ranges and at evaluation times that straddle the counter-reset boundary. - New ydelta block: exercises ydelta on two mirrored gauge series (one increasing, one decreasing) at 30m / 25m / 5m ranges with reference delta()/xdelta() comparisons. Eval times are shifted one second (49s instead of 50s) or one minute (29m instead of 30m) off the sample cadence so that sample timestamps land strictly inside the half-open [rangeStart, rangeEnd) interval. When samples align exactly on a range boundary, matrixIterSlice treats the sample as the retained pre-range sample, which hides the pre-range counter value from yIncrease; the shifted eval times mirror the +321ms offset the legacy promql.NewTest framework applied for the same reason. All ported 2.39.2 expected values reproduce exactly under the new promqltest framework with this offset. Made-with: Cursor
The yrate/yincrease/ydelta family is designed so that two adjacent windows over a series partition a wider window without double-counting any sample. Restated as an invariant: for any three timestamps T_0 < T_1 < T_2 with r_1 = T_1 - T_0 and r_2 = T_2 - T_1, yincrease(m[r_1]) @ T_1 + yincrease(m[r_2]) @ T_2 == yincrease(m[r_1 + r_2]) @ T_2 This property is what makes yincrease safe to aggregate across adjacent query ranges and what distinguishes it from stock rate/increase. Until now it was only covered implicitly through the 1000+/2000+ numeric regression block, which made silent semantic regressions plausible (e.g. a boundary flip that preserves individual queries' values but breaks additivity). Add three direct scenarios that assert LHS and RHS separately so any drift is visible side-by-side: 1. uniform counter, no resets 2. counter reset inside the earlier window 3. counter reset inside the later window All three pick T_0, T_1, T_2 off-cadence so no sample lands on a range boundary. That makes the expected values stable across both boundary conventions (left-inclusive on the add-yrate line, right-inclusive after align-yrate-to-3x-range-boundary) and lets the block cherry-pick cleanly up the yrate branch stack. Made-with: Cursor
Match the upstream-facing proposal vocabulary by retiring "linear" /
"linearity" from yrate-related internal artifacts in favor of:
* "additive over adjacent ranges" -- the precise mathematical anchor
(finite additivity over a partition of disjoint intervals)
* "composable" -- the user-facing benefit derived from additivity
(already the term PROM-52 names as a goal for `anchored`)
Two atomic edits, no behavior change:
* promql/functions.go: yIncrease docstring now says "additive over
adjacent periods, and therefore composable across any partitioning
of a wider range into contiguous sub-ranges". The formula stays
the same.
* promql/promqltest/testdata/functions.test: rename the test-data
block header from "Linearity invariant" to "Additivity invariant",
expand the introduction to mention composability, and rename the
three test series from linearity_{uniform,reset_early,reset_late}
to additivity_{uniform,reset_early,reset_late}.
Strict "linearity" overclaimed (the property at issue is finite
additivity over a partition of adjacent intervals, not full linearity
over a vector space) and the two-term split keeps the proof anchor
distinct from the user-facing benefit.
Unrelated upstream references to "linear" (predict_linear,
linearRegression, "linear search" complexity comments) are left
untouched.
Co-authored-by: Cursor <cursoragent@cursor.com>Replace the linear 1..11 /bar ladder with an 11-point series whose deltas grow +1..+10, starting at 11 (max step + 1) so yrate unwritten-origin offset cannot be confused with any in-sequence increment. Refresh expected values in the rate-vs-xrate showcase block. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace linear additivity and ydelta fixtures with counters whose per-step delta grows by 1, starting at max_delta+1 so origin-offset bugs cannot hide behind a repeated +1 or +10 step. Refresh ydelta and yincrease expectations. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
bf2d555 to
8dba4ffCompare
Summary
Forward-port of #17 onto
invoca-2.55.1/fix-xrate-extended-range.All four commits cherry-picked cleanly from
invoca-2.53.1/add-yrate;promql/functions.go,promql/parser/functions.go, andpromql/promqltest/testdata/functions.testall auto-merged. No code changes from the 2.53.1 version — the 2.53→2.55 upstream delta is entirely in other subsystems (OTLP, remote write, UI) and does not touch the range-extension path.yIncrease+rangeFromSelectorshelpers.yrate/yincrease/ydeltaPromQL functions.REPLACE_RATE_FUNCSenv switch for yrate; run once at init — supportsx/X(point rate→xrate, keep x* names available) and2/y/Y(point rate→yrate, keep y* names available).[rangeStart, rangeEnd)interval.Range-boundary reminder: mainline 2.55 still uses the 2.x closed-closed
[start, end]range selector semantics, so this port requires no expected-value changes for its[start, end)policy. Prometheus 3.0 flips to left-open(start, end](upstream issue prometheus#13213), which will require a re-examination ofyIncreaseat that cutover.Test plan
go test -count=1 ./promql/...passes on Go 1.26.2 (with and withoutREPLACE_RATE_FUNCS=y)invoca-2.53.1/add-yrateRelated
invoca-2.55.1/fix-xrate-extended-range→invoca-2.55.1-base.Once #21 lands on
invoca-2.55.1-base, this PR's base can be retargeted toinvoca-2.55.1-basedirectly.