app/vmselect/promql: do not take into account the previous point before time window in square brackets for min_over_time, max_over_time, rollup_first and rollup_last functions

This makes the behaviour for these functions similar to Prometheus when processing broken time series with irregular data points
like `gitlab_runner_jobs`. See https://gitlab.com/gitlab-org/gitlab-exporter/issues/50 for details.
This commit is contained in:
Aliaksandr Valialkin 2020-01-11 00:24:31 +02:00
parent 1c445bf7eb
commit 16fb128bbc
3 changed files with 45 additions and 52 deletions

View File

@ -4519,7 +4519,7 @@ func TestExecSuccess(t *testing.T) {
}} }}
r2 := netstorage.Result{ r2 := netstorage.Result{
MetricName: metricNameExpected, MetricName: metricNameExpected,
Values: []float64{0.9, 0.9, 1, 0.9, 1, 0.9}, Values: []float64{0.8, 0.9, 1, 0.9, 1, 0.9},
Timestamps: timestampsExpected, Timestamps: timestampsExpected,
} }
r2.MetricName.Tags = []storage.Tag{{ r2.MetricName.Tags = []storage.Tag{{
@ -4543,7 +4543,7 @@ func TestExecSuccess(t *testing.T) {
q := `avg(aggr_over_time(("min_over_time", "max_over_time"), time()[:10s]))` q := `avg(aggr_over_time(("min_over_time", "max_over_time"), time()[:10s]))`
r := netstorage.Result{ r := netstorage.Result{
MetricName: metricNameExpected, MetricName: metricNameExpected,
Values: []float64{900, 1100, 1300, 1500, 1700, 1900}, Values: []float64{905, 1105, 1305, 1505, 1705, 1905},
Timestamps: timestampsExpected, Timestamps: timestampsExpected,
} }
resultExpected := []netstorage.Result{r} resultExpected := []netstorage.Result{r}
@ -4554,7 +4554,7 @@ func TestExecSuccess(t *testing.T) {
q := `sort(avg(aggr_over_time(("min_over_time", "max_over_time"), time()[:10s])) by (rollup))` q := `sort(avg(aggr_over_time(("min_over_time", "max_over_time"), time()[:10s])) by (rollup))`
r1 := netstorage.Result{ r1 := netstorage.Result{
MetricName: metricNameExpected, MetricName: metricNameExpected,
Values: []float64{800, 1000, 1200, 1400, 1600, 1800}, Values: []float64{810, 1010, 1210, 1410, 1610, 1810},
Timestamps: timestampsExpected, Timestamps: timestampsExpected,
} }
r1.MetricName.Tags = []storage.Tag{{ r1.MetricName.Tags = []storage.Tag{{
@ -4587,25 +4587,25 @@ func TestExecSuccess(t *testing.T) {
}} }}
r2 := netstorage.Result{ r2 := netstorage.Result{
MetricName: metricNameExpected, MetricName: metricNameExpected,
Values: []float64{0.32, 0.82, 0.13, 0.28, 0.86, 0.57}, Values: []float64{0.85, 0.15, 0.43, 0.76, 0.47, 0.21},
Timestamps: timestampsExpected, Timestamps: timestampsExpected,
} }
r2.MetricName.Tags = []storage.Tag{{ r2.MetricName.Tags = []storage.Tag{{
Key: []byte("rollup"), Key: []byte("rollup"),
Value: []byte("close"), Value: []byte("open"),
}} }}
r3 := netstorage.Result{ r3 := netstorage.Result{
MetricName: metricNameExpected, MetricName: metricNameExpected,
Values: []float64{0.9, 0.32, 0.82, 0.13, 0.28, 0.86}, Values: []float64{0.32, 0.82, 0.13, 0.28, 0.86, 0.57},
Timestamps: timestampsExpected, Timestamps: timestampsExpected,
} }
r3.MetricName.Tags = []storage.Tag{{ r3.MetricName.Tags = []storage.Tag{{
Key: []byte("rollup"), Key: []byte("rollup"),
Value: []byte("open"), Value: []byte("close"),
}} }}
r4 := netstorage.Result{ r4 := netstorage.Result{
MetricName: metricNameExpected, MetricName: metricNameExpected,
Values: []float64{0.9, 0.94, 0.97, 0.93, 0.98, 0.92}, Values: []float64{0.85, 0.94, 0.97, 0.93, 0.98, 0.92},
Timestamps: timestampsExpected, Timestamps: timestampsExpected,
} }
r4.MetricName.Tags = []storage.Tag{{ r4.MetricName.Tags = []storage.Tag{{
@ -4653,7 +4653,7 @@ func TestExecSuccess(t *testing.T) {
q := `sort(rollup(time()[:50s]))` q := `sort(rollup(time()[:50s]))`
r1 := netstorage.Result{ r1 := netstorage.Result{
MetricName: metricNameExpected, MetricName: metricNameExpected,
Values: []float64{800, 1000, 1200, 1400, 1600, 1800}, Values: []float64{850, 1050, 1250, 1450, 1650, 1850},
Timestamps: timestampsExpected, Timestamps: timestampsExpected,
} }
r1.MetricName.Tags = []storage.Tag{{ r1.MetricName.Tags = []storage.Tag{{

View File

@ -114,6 +114,7 @@ var rollupFuncsMayAdjustWindow = map[string]bool{
"irate": true, "irate": true,
"rate": true, "rate": true,
"lifetime": true, "lifetime": true,
"lag": true,
"scrape_interval": true, "scrape_interval": true,
} }
@ -884,14 +885,14 @@ func rollupAvg(rfa *rollupFuncArg) float64 {
func rollupMin(rfa *rollupFuncArg) float64 { func rollupMin(rfa *rollupFuncArg) float64 {
// There is no need in handling NaNs here, since they must be cleaned up // There is no need in handling NaNs here, since they must be cleaned up
// before calling rollup funcs. // before calling rollup funcs.
minValue := rfa.prevValue
values := rfa.values values := rfa.values
if math.IsNaN(minValue) { if len(values) == 0 {
if len(values) == 0 { // Do not take into account rfa.prevValue, since it may lead
return nan // to inconsistent results comparing to Prometheus on broken time series
} // with irregular data points.
minValue = values[0] return nan
} }
minValue := values[0]
for _, v := range values { for _, v := range values {
if v < minValue { if v < minValue {
minValue = v minValue = v
@ -903,14 +904,14 @@ func rollupMin(rfa *rollupFuncArg) float64 {
func rollupMax(rfa *rollupFuncArg) float64 { func rollupMax(rfa *rollupFuncArg) float64 {
// There is no need in handling NaNs here, since they must be cleaned up // There is no need in handling NaNs here, since they must be cleaned up
// before calling rollup funcs. // before calling rollup funcs.
maxValue := rfa.prevValue
values := rfa.values values := rfa.values
if math.IsNaN(maxValue) { if len(values) == 0 {
if len(values) == 0 { // Do not take into account rfa.prevValue, since it may lead
return nan // to inconsistent results comparing to Prometheus on broken time series
} // with irregular data points.
maxValue = values[0] return nan
} }
maxValue := values[0]
for _, v := range values { for _, v := range values {
if v > maxValue { if v > maxValue {
maxValue = v maxValue = v
@ -922,17 +923,13 @@ func rollupMax(rfa *rollupFuncArg) float64 {
func rollupTmin(rfa *rollupFuncArg) float64 { func rollupTmin(rfa *rollupFuncArg) float64 {
// There is no need in handling NaNs here, since they must be cleaned up // There is no need in handling NaNs here, since they must be cleaned up
// before calling rollup funcs. // before calling rollup funcs.
minValue := rfa.prevValue
minTimestamp := rfa.prevTimestamp
values := rfa.values values := rfa.values
timestamps := rfa.timestamps timestamps := rfa.timestamps
if math.IsNaN(minValue) { if len(values) == 0 {
if len(values) == 0 { return nan
return nan
}
minValue = values[0]
minTimestamp = timestamps[0]
} }
minValue := values[0]
minTimestamp := timestamps[0]
for i, v := range values { for i, v := range values {
if v < minValue { if v < minValue {
minValue = v minValue = v
@ -945,17 +942,13 @@ func rollupTmin(rfa *rollupFuncArg) float64 {
func rollupTmax(rfa *rollupFuncArg) float64 { func rollupTmax(rfa *rollupFuncArg) float64 {
// There is no need in handling NaNs here, since they must be cleaned up // There is no need in handling NaNs here, since they must be cleaned up
// before calling rollup funcs. // before calling rollup funcs.
maxValue := rfa.prevValue
maxTimestamp := rfa.prevTimestamp
values := rfa.values values := rfa.values
timestamps := rfa.timestamps timestamps := rfa.timestamps
if math.IsNaN(maxValue) { if len(values) == 0 {
if len(values) == 0 { return nan
return nan
}
maxValue = values[0]
maxTimestamp = timestamps[0]
} }
maxValue := values[0]
maxTimestamp := timestamps[0]
for i, v := range values { for i, v := range values {
if v > maxValue { if v > maxValue {
maxValue = v maxValue = v
@ -1335,16 +1328,13 @@ func rollupResets(rfa *rollupFuncArg) float64 {
} }
func rollupFirst(rfa *rollupFuncArg) float64 { func rollupFirst(rfa *rollupFuncArg) float64 {
// See https://prometheus.io/docs/prometheus/latest/querying/basics/#staleness
v := rfa.prevValue
if !math.IsNaN(v) {
return v
}
// There is no need in handling NaNs here, since they must be cleaned up // There is no need in handling NaNs here, since they must be cleaned up
// before calling rollup funcs. // before calling rollup funcs.
values := rfa.values values := rfa.values
if len(values) == 0 { if len(values) == 0 {
// Do not take into account rfa.prevValue, since it may lead
// to inconsistent results comparing to Prometheus on broken time series
// with irregular data points.
return nan return nan
} }
return values[0] return values[0]
@ -1357,7 +1347,10 @@ func rollupLast(rfa *rollupFuncArg) float64 {
// before calling rollup funcs. // before calling rollup funcs.
values := rfa.values values := rfa.values
if len(values) == 0 { if len(values) == 0 {
return rfa.prevValue // Do not take into account rfa.prevValue, since it may lead
// to inconsistent results comparing to Prometheus on broken time series
// with irregular data points.
return nan
} }
return values[len(values)-1] return values[len(values)-1]
} }

View File

@ -459,7 +459,7 @@ func TestRollupNoWindowPartialPoints(t *testing.T) {
} }
rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step)
values := rc.Do(nil, testValues, testTimestamps) values := rc.Do(nil, testValues, testTimestamps)
valuesExpected := []float64{nan, 123, 123, 123, 34, 34} valuesExpected := []float64{nan, 123, nan, 34, nan, 44}
timestampsExpected := []int64{0, 5, 10, 15, 20, 25} timestampsExpected := []int64{0, 5, 10, 15, 20, 25}
testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected)
}) })
@ -473,7 +473,7 @@ func TestRollupNoWindowPartialPoints(t *testing.T) {
} }
rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step)
values := rc.Do(nil, testValues, testTimestamps) values := rc.Do(nil, testValues, testTimestamps)
valuesExpected := []float64{12, 44, 34, nan} valuesExpected := []float64{44, 32, 34, nan}
timestampsExpected := []int64{100, 120, 140, 160} timestampsExpected := []int64{100, 120, 140, 160}
testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected)
}) })
@ -487,7 +487,7 @@ func TestRollupNoWindowPartialPoints(t *testing.T) {
} }
rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step)
values := rc.Do(nil, testValues, testTimestamps) values := rc.Do(nil, testValues, testTimestamps)
valuesExpected := []float64{nan, nan, 123, 54, 44} valuesExpected := []float64{nan, nan, 123, 34, 32}
timestampsExpected := []int64{-50, 0, 50, 100, 150} timestampsExpected := []int64{-50, 0, 50, 100, 150}
testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected)
}) })
@ -549,7 +549,7 @@ func TestRollupFuncsLookbackDelta(t *testing.T) {
} }
rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step)
values := rc.Do(nil, testValues, testTimestamps) values := rc.Do(nil, testValues, testTimestamps)
valuesExpected := []float64{99, 12, 44, nan, 32, 34, nan} valuesExpected := []float64{99, nan, 44, nan, 32, 34, nan}
timestampsExpected := []int64{80, 90, 100, 110, 120, 130, 140} timestampsExpected := []int64{80, 90, 100, 110, 120, 130, 140}
testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected)
}) })
@ -563,7 +563,7 @@ func TestRollupFuncsLookbackDelta(t *testing.T) {
} }
rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step)
values := rc.Do(nil, testValues, testTimestamps) values := rc.Do(nil, testValues, testTimestamps)
valuesExpected := []float64{99, 12, 44, 44, 32, 34, nan} valuesExpected := []float64{99, nan, 44, nan, 32, 34, nan}
timestampsExpected := []int64{80, 90, 100, 110, 120, 130, 140} timestampsExpected := []int64{80, 90, 100, 110, 120, 130, 140}
testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected)
}) })
@ -577,7 +577,7 @@ func TestRollupFuncsLookbackDelta(t *testing.T) {
} }
rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step)
values := rc.Do(nil, testValues, testTimestamps) values := rc.Do(nil, testValues, testTimestamps)
valuesExpected := []float64{34, 12, 12, 44, 44, 34, nan} valuesExpected := []float64{99, nan, 44, nan, 32, 34, nan}
timestampsExpected := []int64{80, 90, 100, 110, 120, 130, 140} timestampsExpected := []int64{80, 90, 100, 110, 120, 130, 140}
testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected)
}) })
@ -594,7 +594,7 @@ func TestRollupFuncsNoWindow(t *testing.T) {
} }
rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step)
values := rc.Do(nil, testValues, testTimestamps) values := rc.Do(nil, testValues, testTimestamps)
valuesExpected := []float64{nan, 123, 21, 12, 34} valuesExpected := []float64{nan, 123, 54, 44, 34}
timestampsExpected := []int64{0, 40, 80, 120, 160} timestampsExpected := []int64{0, 40, 80, 120, 160}
testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected)
}) })
@ -622,7 +622,7 @@ func TestRollupFuncsNoWindow(t *testing.T) {
} }
rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step)
values := rc.Do(nil, testValues, testTimestamps) values := rc.Do(nil, testValues, testTimestamps)
valuesExpected := []float64{nan, 21, 12, 12, 34} valuesExpected := []float64{nan, 21, 12, 32, 34}
timestampsExpected := []int64{0, 40, 80, 120, 160} timestampsExpected := []int64{0, 40, 80, 120, 160}
testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected)
}) })