Skip to content

Commit 5e88f32

Browse files
sylrclaude
andcommitted
refactor: simplify interpolateAt to match Prometheus v0.310.0
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>
1 parent a89bae5 commit 5e88f32

3 files changed

Lines changed: 40 additions & 48 deletions

File tree

engine/engine.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ func NewWithScanners(opts Opts, scanners engstorage.Scanners) *Engine {
187187
scanners: scanners,
188188
activeQueryTracker: queryTracker,
189189

190-
disableDuplicateLabelChecks: opts.DisableDuplicateLabelChecks,
190+
disableDuplicateLabelChecks: opts.DisableDuplicateLabelChecks,
191191
enableExtendedRangeSelectors: opts.EnableExtendedRangeSelectors,
192192

193193
logger: opts.Logger,
@@ -218,7 +218,7 @@ type Engine struct {
218218
scanners engstorage.Scanners
219219
activeQueryTracker promql.QueryTracker
220220

221-
disableDuplicateLabelChecks bool
221+
disableDuplicateLabelChecks bool
222222
enableExtendedRangeSelectors bool
223223

224224
logger *slog.Logger

engine/extended_range_test.go

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,18 @@ import (
1010
"testing"
1111
"time"
1212

13-
"github.com/efficientgo/core/testutil"
14-
"github.com/prometheus/prometheus/promql"
15-
"github.com/prometheus/prometheus/promql/parser"
16-
"github.com/prometheus/prometheus/promql/promqltest"
17-
promstorage "github.com/prometheus/prometheus/storage"
18-
1913
"github.com/thanos-io/promql-engine/engine"
2014
"github.com/thanos-io/promql-engine/execution/model"
2115
"github.com/thanos-io/promql-engine/logicalplan"
2216
"github.com/thanos-io/promql-engine/query"
2317
engstorage "github.com/thanos-io/promql-engine/storage"
2418
promscan "github.com/thanos-io/promql-engine/storage/prometheus"
19+
20+
"github.com/efficientgo/core/testutil"
21+
"github.com/prometheus/prometheus/promql"
22+
"github.com/prometheus/prometheus/promql/parser"
23+
"github.com/prometheus/prometheus/promql/promqltest"
24+
promstorage "github.com/prometheus/prometheus/storage"
2525
)
2626

2727
func TestAnchoredSmoothedModifiers(t *testing.T) {
@@ -47,119 +47,119 @@ func TestAnchoredSmoothedModifiers(t *testing.T) {
4747
{
4848
name: "anchored rate on linear counter",
4949
load: `load 10s
50-
http_total 0 10 20 30 40 50 60 70 80 90 100`,
50+
http_total 0 10 20 30 40 50 60 70 80 90 100`,
5151
query: `rate(http_total[30s] anchored)`,
5252
},
5353
{
5454
name: "anchored increase on linear counter",
5555
load: `load 10s
56-
http_total 0 10 20 30 40 50 60 70 80 90 100`,
56+
http_total 0 10 20 30 40 50 60 70 80 90 100`,
5757
query: `increase(http_total[30s] anchored)`,
5858
},
5959
{
6060
name: "anchored delta on gauge",
6161
load: `load 10s
62-
temperature 20 22 21 23 25 24 26 28 27 29 30`,
62+
temperature 20 22 21 23 25 24 26 28 27 29 30`,
6363
query: `delta(temperature[30s] anchored)`,
6464
},
6565
// Smoothed rate/increase/delta on a linear counter.
6666
{
6767
name: "smoothed rate on linear counter",
6868
load: `load 10s
69-
http_total 0 10 20 30 40 50 60 70 80 90 100`,
69+
http_total 0 10 20 30 40 50 60 70 80 90 100`,
7070
query: `rate(http_total[30s] smoothed)`,
7171
},
7272
{
7373
name: "smoothed increase on linear counter",
7474
load: `load 10s
75-
http_total 0 10 20 30 40 50 60 70 80 90 100`,
75+
http_total 0 10 20 30 40 50 60 70 80 90 100`,
7676
query: `increase(http_total[30s] smoothed)`,
7777
},
7878
{
7979
name: "smoothed delta on gauge",
8080
load: `load 10s
81-
temperature 20 22 21 23 25 24 26 28 27 29 30`,
81+
temperature 20 22 21 23 25 24 26 28 27 29 30`,
8282
query: `delta(temperature[30s] smoothed)`,
8383
},
8484
// Anchored with counter resets.
8585
{
8686
name: "anchored increase with counter reset",
8787
load: `load 10s
88-
resets_total 0 10 20 5 15 25 10 20 30 40 50`,
88+
resets_total 0 10 20 5 15 25 10 20 30 40 50`,
8989
query: `increase(resets_total[30s] anchored)`,
9090
},
9191
{
9292
name: "anchored rate with counter reset",
9393
load: `load 10s
94-
resets_total 0 10 20 5 15 25 10 20 30 40 50`,
94+
resets_total 0 10 20 5 15 25 10 20 30 40 50`,
9595
query: `rate(resets_total[30s] anchored)`,
9696
},
9797
// Smoothed with counter resets.
9898
{
9999
name: "smoothed increase with counter reset",
100100
load: `load 10s
101-
resets_total 0 10 20 5 15 25 10 20 30 40 50`,
101+
resets_total 0 10 20 5 15 25 10 20 30 40 50`,
102102
query: `increase(resets_total[30s] smoothed)`,
103103
},
104104
{
105105
name: "smoothed rate with counter reset",
106106
load: `load 10s
107-
resets_total 0 10 20 5 15 25 10 20 30 40 50`,
107+
resets_total 0 10 20 5 15 25 10 20 30 40 50`,
108108
query: `rate(resets_total[30s] smoothed)`,
109109
},
110110
// Anchored resets and changes (only supported for anchored).
111111
{
112112
name: "anchored resets",
113113
load: `load 10s
114-
resets_total 0 10 20 5 15 25 10 20 30 40 50`,
114+
resets_total 0 10 20 5 15 25 10 20 30 40 50`,
115115
query: `resets(resets_total[30s] anchored)`,
116116
},
117117
{
118118
name: "anchored changes",
119119
load: `load 10s
120-
metric 1 1 2 2 3 3 4 4 5 5 6`,
120+
metric 1 1 2 2 3 3 4 4 5 5 6`,
121121
query: `changes(metric[30s] anchored)`,
122122
},
123123
// Counter reset at range boundary (regression test: smoothed interpolation
124124
// must handle counter resets to avoid negative increase values).
125125
{
126126
name: "smoothed increase counter reset at boundary",
127127
load: `load 10s
128-
counter_boundary 0 4 5 1 6 11`,
128+
counter_boundary 0 4 5 1 6 11`,
129129
query: `increase(counter_boundary[10s] smoothed)`,
130130
},
131131
{
132132
name: "smoothed rate counter reset at boundary",
133133
load: `load 10s
134-
counter_boundary 0 4 5 1 6 11`,
134+
counter_boundary 0 4 5 1 6 11`,
135135
query: `rate(counter_boundary[10s] smoothed)`,
136136
},
137137
// Non-linear data.
138138
{
139139
name: "anchored rate on quadratic counter",
140140
load: `load 10s
141-
quadratic 0 1 4 9 16 25 36 49 64 81 100`,
141+
quadratic 0 1 4 9 16 25 36 49 64 81 100`,
142142
query: `rate(quadratic[30s] anchored)`,
143143
},
144144
{
145145
name: "smoothed rate on quadratic counter",
146146
load: `load 10s
147-
quadratic 0 1 4 9 16 25 36 49 64 81 100`,
147+
quadratic 0 1 4 9 16 25 36 49 64 81 100`,
148148
query: `rate(quadratic[30s] smoothed)`,
149149
},
150150
// Multiple series.
151151
{
152152
name: "anchored increase multiple series",
153153
load: `load 10s
154-
http_total{path="/foo"} 0 5 10 15 20 25 30 35 40 45 50
155-
http_total{path="/bar"} 0 10 20 30 40 50 60 70 80 90 100`,
154+
http_total{path="/foo"} 0 5 10 15 20 25 30 35 40 45 50
155+
http_total{path="/bar"} 0 10 20 30 40 50 60 70 80 90 100`,
156156
query: `increase(http_total[30s] anchored)`,
157157
},
158158
{
159159
name: "smoothed increase multiple series",
160160
load: `load 10s
161-
http_total{path="/foo"} 0 5 10 15 20 25 30 35 40 45 50
162-
http_total{path="/bar"} 0 10 20 30 40 50 60 70 80 90 100`,
161+
http_total{path="/foo"} 0 5 10 15 20 25 30 35 40 45 50
162+
http_total{path="/bar"} 0 10 20 30 40 50 60 70 80 90 100`,
163163
query: `increase(http_total[30s] smoothed)`,
164164
},
165165
}

ringbuffer/functions.go

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ import (
77
"math"
88
"sort"
99

10-
"github.com/efficientgo/core/errors"
11-
"github.com/prometheus/prometheus/model/histogram"
12-
1310
"github.com/thanos-io/promql-engine/compute"
1411
"github.com/thanos-io/promql-engine/execution/parse"
1512
"github.com/thanos-io/promql-engine/warnings"
13+
14+
"github.com/efficientgo/core/errors"
15+
"github.com/prometheus/prometheus/model/histogram"
1616
)
1717

1818
type SamplesBuffer GenericRingBuffer
@@ -672,12 +672,9 @@ func extendedRangeRate(samples []Sample, isCounter, isRate bool, stepTime, selec
672672
rangeStart := rangeEnd - selectRange
673673

674674
// Find firstSampleIndex: the last sample at or before rangeStart, clamped to 0.
675-
firstSampleIndex := sort.Search(lastSampleIndex, func(i int) bool {
675+
firstSampleIndex := max(sort.Search(lastSampleIndex, func(i int) bool {
676676
return samples[i].T > rangeStart
677-
}) - 1
678-
if firstSampleIndex < 0 {
679-
firstSampleIndex = 0
680-
}
677+
})-1, 0)
681678

682679
// For smoothed, extend lastSampleIndex to include the first sample at or after rangeEnd.
683680
if smoothed {
@@ -732,7 +729,7 @@ func extendedRangeRate(samples []Sample, isCounter, isRate bool, stepTime, selec
732729
// If no sample exists before rangeStart, the first sample value is used.
733730
func pickOrInterpolateLeft(samples []Sample, first int, rangeStart int64, smoothed, isCounter bool) float64 {
734731
if smoothed && samples[first].T < rangeStart && first+1 < len(samples) {
735-
return interpolateAt(samples[first], samples[first+1], rangeStart, isCounter, true)
732+
return interpolateAt(samples[first], samples[first+1], rangeStart, isCounter)
736733
}
737734
return samples[first].V.F
738735
}
@@ -744,26 +741,21 @@ func pickOrInterpolateLeft(samples []Sample, first int, rangeStart int64, smooth
744741
// If no sample exists after rangeEnd, the last sample value is used.
745742
func pickOrInterpolateRight(samples []Sample, last int, rangeEnd int64, smoothed, isCounter bool) float64 {
746743
if smoothed && last > 0 && samples[last].T > rangeEnd {
747-
return interpolateAt(samples[last-1], samples[last], rangeEnd, isCounter, false)
744+
return interpolateAt(samples[last-1], samples[last], rangeEnd, isCounter)
748745
}
749746
return samples[last].V.F
750747
}
751748

752749
// interpolateAt performs linear interpolation between two samples at the given timestamp.
753-
// If isCounter is true and there is a counter reset (right < left):
754-
// - on the left edge, it sets the left value to 0
755-
// - on the right edge, it adds the left value to the right value
750+
// If isCounter is true and there is a counter reset (y2 < y1), it models the
751+
// counter as starting from 0 post-reset by setting y1 to 0.
756752
//
757-
// This matches Prometheus' interpolate function in promql/functions.go.
758-
func interpolateAt(left, right Sample, timestamp int64, isCounter, leftEdge bool) float64 {
753+
// This matches Prometheus v0.310.0's interpolate function in promql/functions.go.
754+
func interpolateAt(left, right Sample, timestamp int64, isCounter bool) float64 {
759755
y1 := left.V.F
760756
y2 := right.V.F
761757
if isCounter && y2 < y1 {
762-
if leftEdge {
763-
y1 = 0
764-
} else {
765-
y2 += y1
766-
}
758+
y1 = 0
767759
}
768760
return y1 + (y2-y1)*float64(timestamp-left.T)/float64(right.T-left.T)
769761
}

0 commit comments

Comments
 (0)