From 974d9c0eeeb2c98d3e05ad21a2dd4d6efd395f5b Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Mon, 20 Dec 2021 13:13:16 +0200 Subject: [PATCH] app/vmselect/promql: follow-up after 177e345d8a7db0ee8520b9d6f74bf8ccdd08ad40 * Document changes_prometheus(), increase_prometheus() and delta_prometheus() functions. * Simplify their implementation * Mention these functions in docs/CHANGELOG.md --- app/vmselect/promql/exec_test.go | 38 +++++++++ app/vmselect/promql/rollup.go | 79 +++++-------------- app/vmselect/promql/rollup_test.go | 31 ++++++++ docs/CHANGELOG.md | 1 + docs/MetricsQL.md | 18 ++++- go.mod | 2 +- go.sum | 4 +- .../VictoriaMetrics/metricsql/rollup.go | 3 + vendor/modules.txt | 2 +- 9 files changed, 111 insertions(+), 67 deletions(-) diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go index b69cfbd87..2a5757fcf 100644 --- a/app/vmselect/promql/exec_test.go +++ b/app/vmselect/promql/exec_test.go @@ -6270,6 +6270,22 @@ func TestExecSuccess(t *testing.T) { resultExpected := []netstorage.Result{r} f(q, resultExpected) }) + t.Run(`increase_prometheus(time())`, func(t *testing.T) { + t.Parallel() + q := `increase_prometheus(time())` + f(q, nil) + }) + t.Run(`increase_prometheus(time()[201s])`, func(t *testing.T) { + t.Parallel() + q := `increase_prometheus(time()[201s])` + r := netstorage.Result{ + MetricName: metricNameExpected, + Values: []float64{200, 200, 200, 200, 200, 200}, + Timestamps: timestampsExpected, + } + resultExpected := []netstorage.Result{r} + f(q, resultExpected) + }) t.Run(`running_max(1)`, func(t *testing.T) { t.Parallel() q := `running_max(1)` @@ -6512,6 +6528,22 @@ func TestExecSuccess(t *testing.T) { resultExpected := []netstorage.Result{r} f(q, resultExpected) }) + t.Run(`delta_prometheus(time())`, func(t *testing.T) { + t.Parallel() + q := `delta_prometheus(time())` + f(q, nil) + }) + t.Run(`delta_prometheus(time()[201s])`, func(t *testing.T) { + t.Parallel() + q := `delta_prometheus(time()[201s])` + r := netstorage.Result{ + MetricName: metricNameExpected, + Values: []float64{200, 200, 200, 200, 200, 200}, + Timestamps: timestampsExpected, + } + resultExpected := []netstorage.Result{r} + f(q, resultExpected) + }) t.Run(`median_over_time("foo")`, func(t *testing.T) { t.Parallel() q := `median_over_time("foo")` @@ -7503,6 +7535,12 @@ func TestExecError(t *testing.T) { f(`bitmap_xor()`) f(`quantiles()`) f(`limit_offset()`) + f(`increase()`) + f(`increase_prometheus()`) + f(`changes()`) + f(`changes_prometheus()`) + f(`delta()`) + f(`delta_prometheus()`) // Invalid argument type f(`median_over_time({}, 2)`) diff --git a/app/vmselect/promql/rollup.go b/app/vmselect/promql/rollup.go index 8b44b30e6..0a7a15099 100644 --- a/app/vmselect/promql/rollup.go +++ b/app/vmselect/promql/rollup.go @@ -164,12 +164,13 @@ var rollupFuncsCanAdjustWindow = map[string]bool{ } var rollupFuncsRemoveCounterResets = map[string]bool{ - "increase": true, - "increase_pure": true, - "irate": true, - "rate": true, - "rollup_increase": true, - "rollup_rate": true, + "increase": true, + "increase_prometheus": true, + "increase_pure": true, + "irate": true, + "rate": true, + "rollup_increase": true, + "rollup_rate": true, } // These functions don't change physical meaning of input time series, @@ -1492,51 +1493,12 @@ func rollupDeltaPrometheus(rfa *rollupFuncArg) float64 { // There is no need in handling NaNs here, since they must be cleaned up // before calling rollup funcs. values := rfa.values - prevValue := rfa.prevValue - if math.IsNaN(prevValue) { - if len(values) == 0 { - return nan - } - if !math.IsNaN(rfa.realPrevValue) { - // Assume that the value didn't change during the current gap. - // This should fix high delta() and increase() values at the end of gaps. - // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/894 - return values[len(values)-1] - rfa.realPrevValue - } - // Assume that the previous non-existing value was 0 only in the following cases: - // - // - If the delta with the next value equals to 0. - // This is the case for slow-changing counter - see https://github.com/VictoriaMetrics/VictoriaMetrics/issues/962 - // - If the first value doesn't exceed too much the delta with the next value. - // - // This should prevent from improper increase() results for os-level counters - // such as cpu time or bytes sent over the network interface. - // These counters may start long ago before the first value appears in the db. - // - // This also should prevent from improper increase() results when a part of label values are changed - // without counter reset. - var d float64 - if len(values) > 1 { - d = values[1] - values[0] - } else if !math.IsNaN(rfa.realNextValue) { - d = rfa.realNextValue - values[0] - } - if d == 0 { - d = 10 - } - if math.Abs(values[0]) < 10*(math.Abs(d)+1) { - //prevValue = 0 - prevValue = values[0] - } else { - prevValue = values[0] - values = values[1:] - } + // Just return the difference between the last and the first sample like Prometheus does. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1962 + if len(values) < 2 { + return nan } - if len(values) == 0 { - // Assume that the value didn't change on the given interval. - return 0 - } - return values[len(values)-1] - prevValue + return values[len(values)-1] - values[0] } func rollupIdelta(rfa *rollupFuncArg) float64 { @@ -1702,17 +1664,14 @@ func rollupChangesPrometheus(rfa *rollupFuncArg) float64 { // There is no need in handling NaNs here, since they must be cleaned up // before calling rollup funcs. values := rfa.values - prevValue := rfa.prevValue - n := 0 - if math.IsNaN(prevValue) { - if len(values) == 0 { - return nan - } - prevValue = values[0] - values = values[1:] - //n++ + // Do not take into account rfa.prevValue like Prometheus does. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1962 + if len(values) < 1 { + return nan } - for _, v := range values { + prevValue := values[0] + n := 0 + for _, v := range values[1:] { if v != prevValue { n++ prevValue = v diff --git a/app/vmselect/promql/rollup_test.go b/app/vmselect/promql/rollup_test.go index e0348dfd7..ae0f2d9ab 100644 --- a/app/vmselect/promql/rollup_test.go +++ b/app/vmselect/promql/rollup_test.go @@ -490,11 +490,14 @@ func TestRollupNewRollupFuncSuccess(t *testing.T) { f("default_rollup", 34) f("changes", 11) + f("changes_prometheus", 10) f("delta", 34) + f("delta_prometheus", -89) f("deriv", -266.85860231406093) f("deriv_fast", -712) f("idelta", 0) f("increase", 398) + f("increase_prometheus", 275) f("irate", 0) f("rate", 2200) f("resets", 5) @@ -851,6 +854,20 @@ func TestRollupFuncsNoWindow(t *testing.T) { timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) + t.Run("delta_prometheus", func(t *testing.T) { + rc := rollupConfig{ + Func: rollupDeltaPrometheus, + Start: 0, + End: 160, + Step: 40, + Window: 0, + } + rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) + values := rc.Do(nil, testValues, testTimestamps) + valuesExpected := []float64{nan, -102, -42, -10, nan} + timestampsExpected := []int64{0, 40, 80, 120, 160} + testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) + }) t.Run("idelta", func(t *testing.T) { rc := rollupConfig{ Func: rollupIdelta, @@ -949,6 +966,20 @@ func TestRollupFuncsNoWindow(t *testing.T) { timestampsExpected := []int64{0, 40, 80, 120, 160} testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) }) + t.Run("changes_prometheus", func(t *testing.T) { + rc := rollupConfig{ + Func: rollupChangesPrometheus, + Start: 0, + End: 160, + Step: 40, + Window: 0, + } + rc.Timestamps = getTimestamps(rc.Start, rc.End, rc.Step) + values := rc.Do(nil, testValues, testTimestamps) + valuesExpected := []float64{nan, 3, 3, 2, 0} + timestampsExpected := []int64{0, 40, 80, 120, 160} + testRowsEqual(t, values, rc.Timestamps, valuesExpected, timestampsExpected) + }) t.Run("changes_small_window", func(t *testing.T) { rc := rollupConfig{ Func: rollupChanges, diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index d33e2bc80..4d0250942 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -17,6 +17,7 @@ sort: 15 * FEATURE: preserve the order of time series passed to [limit_offset](https://docs.victoriametrics.com/MetricsQL.html#limit_offset) function. This allows implementing series paging via `limit_offset(limit, offset, sort_by_label(...))`. See [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1920) and [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/951) issues. * FEATURE: automaticall convert `(value1|...|valueN)` into `{value1,...,valueN}` inside `__graphite__` pseudo-label. This allows using [Grafana multi-value template variables](https://grafana.com/docs/grafana/latest/variables/formatting-multi-value-variables/) inside `__graphite__` pseudo-label. For example, `{__graphite__=~"foo.($bar)"}` is expanded to `{__graphite__=~"foo.{x,y}"}` if both `x` and `y` are selected for `$bar` template variable. See [these docs](https://docs.victoriametrics.com/#selecting-graphite-metrics) for details. * FEATURE: add [timestamp_with_name](https://docs.victoriametrics.com/MetricsQL.html#timestamp_with_name) function. It works the same as [timestamp](https://docs.victoriametrics.com/MetricsQL.html#timestamp), but leaves the original time series names, so it can be used in queries, which match multiple time series names: `timestamp_with_name({foo="bar"}[1h])`. See [this comment](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/949#issuecomment-995222388) for more context. +* FEATURE: add [changes_prometheus](https://docs.victoriametrics.com/MetricsQL.html#changes_prometheus), [increase_prometheus](https://docs.victoriametrics.com/MetricsQL.html#increase_prometheus) and [delta_prometheus](https://docs.victoriametrics.com/MetricsQL.html#delta_prometheus) functions, which don't take into account the previous sample before the given lookbehind window specified in square brackets. These functions may be used when the Prometheus behaviour for `changes()`, `increase()` and `delta()` functions is needed to be preserved. VictoriaMetrics uses slightly different behaviour for `changes()`, `increase()` and `delta()` functions by default - see [this article](https://medium.com/@romanhavronenko/victoriametrics-promql-compliance-d4318203f51e) for details. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1962). * BUGFIX: fix `unaligned 64-bit atomic operation` panic on 32-bit architectures, which has been introduced in v1.70.0. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1944). * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): restore the ability to use `$labels.alertname` in labels templating. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1921). diff --git a/docs/MetricsQL.md b/docs/MetricsQL.md index dc3912cb6..ea4bf732f 100644 --- a/docs/MetricsQL.md +++ b/docs/MetricsQL.md @@ -92,7 +92,11 @@ See also [implicit query conversions](#implicit-query-conversions). #### changes -`changes(series_selector[d])` calculates the number of times the raw samples changed on the given lookbehind window `d` per each time series returned from the given [series_selector](https://prometheus.io/docs/prometheus/latest/querying/basics/#time-series-selectors). Metric names are stripped from the resulting rollups. This function is supported by PromQL. +`changes(series_selector[d])` calculates the number of times the raw samples changed on the given lookbehind window `d` per each time series returned from the given [series_selector](https://prometheus.io/docs/prometheus/latest/querying/basics/#time-series-selectors). Unlike `changes()` in Prometheus it takes into account the change from the last sample before the given lookbehind window `d`. See [this article](https://medium.com/@romanhavronenko/victoriametrics-promql-compliance-d4318203f51e) for details. Metric names are stripped from the resulting rollups. This function is supported by PromQL. See also [changes_prometheus](#changes_prometheus). + +#### changes_prometheus + +`changes_prometheus(series_selector[d])` calculates the number of times the raw samples changed on the given lookbehind window `d` per each time series returned from the given [series_selector](https://prometheus.io/docs/prometheus/latest/querying/basics/#time-series-selectors). It doesn't take into account the change from the last sample before the given lookbehind window `d` in the same way as Prometheus does. See [this article](https://medium.com/@romanhavronenko/victoriametrics-promql-compliance-d4318203f51e) for details. Metric names are stripped from the resulting rollups. This function is supported by PromQL. See also [changes](#changes). #### count_eq_over_time @@ -124,7 +128,11 @@ See also [implicit query conversions](#implicit-query-conversions). #### delta -`delta(series_selector[d])` calculates the difference between the first and the last point over the given lookbehind window `d` per each time series returned from the given [series_selector](https://prometheus.io/docs/prometheus/latest/querying/basics/#time-series-selectors). Metric names are stripped from the resulting rollups. This function is supported by PromQL. See also [increase](#increase). +`delta(series_selector[d])` calculates the difference between the last sample before the given lookbehind window `d` and the last sample at the given lookbehind window `d` per each time series returned from the given [series_selector](https://prometheus.io/docs/prometheus/latest/querying/basics/#time-series-selectors). The behaviour of `delta()` function in MetricsQL is slighly different to the behaviour of `delta()` function in Prometheus. See [this article](https://medium.com/@romanhavronenko/victoriametrics-promql-compliance-d4318203f51e) for details. Metric names are stripped from the resulting rollups. This function is supported by PromQL. See also [increase](#increase) and [delta_prometheus](#delta_prometheus). + +#### delta_prometheus + +`delta_prometheus(series_selector[d])` calculates the difference between the first and the last samples at the given lookbehind window `d` per each time series returned from the given [series_selector](https://prometheus.io/docs/prometheus/latest/querying/basics/#time-series-selectors). The behaviour of `delta_prometheus()` is close to the behaviour of `delta()` function in Prometheus. See [this article](https://medium.com/@romanhavronenko/victoriametrics-promql-compliance-d4318203f51e) for details. Metric names are stripped from the resulting rollups. See also [delta](#delta). #### deriv @@ -180,7 +188,11 @@ See also [implicit query conversions](#implicit-query-conversions). #### increase -`increase(series_selector[d])` calculates the increase over the given lookbehind window `d` per each time series returned from the given [series_selector](https://prometheus.io/docs/prometheus/latest/querying/basics/#time-series-selectors). It is expected that the `series_selector` returns time series of [counter type](https://prometheus.io/docs/concepts/metric_types/#counter). Metric names are stripped from the resulting rollups. This function is supported by PromQL. See also [increase_pure](#increase_pure) and [delta](#delta). +`increase(series_selector[d])` calculates the increase over the given lookbehind window `d` per each time series returned from the given [series_selector](https://prometheus.io/docs/prometheus/latest/querying/basics/#time-series-selectors). It is expected that the `series_selector` returns time series of [counter type](https://prometheus.io/docs/concepts/metric_types/#counter). Unlike Prometheus it takes into account the last sample before the given lookbehind window `d` when calculating the result. See [this article](https://medium.com/@romanhavronenko/victoriametrics-promql-compliance-d4318203f51e) for details. Metric names are stripped from the resulting rollups. This function is supported by PromQL. See also [increase_pure](#increase_pure), [increase_prometheus](#increase_prometheus) and [delta](#delta). + +#### increase_prometheus + +`increase_prometheus(series_selector[d])` calculates the increase over the given lookbehind window `d` per each time series returned from the given [series_selector](https://prometheus.io/docs/prometheus/latest/querying/basics/#time-series-selectors). It is expected that the `series_selector` returns time series of [counter type](https://prometheus.io/docs/concepts/metric_types/#counter). It doesn't take into account the last sample before the given lookbehind window `d` when calculating the result in the same way as Prometheus does. See [this article](https://medium.com/@romanhavronenko/victoriametrics-promql-compliance-d4318203f51e) for details. Metric names are stripped from the resulting rollups. This function is supported by PromQL. See also [increase_pure](#increase_pure) and [increase](#increase). #### increase_pure diff --git a/go.mod b/go.mod index ca8114507..3e11cd6ac 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( // like https://github.com/valyala/fasthttp/commit/996610f021ff45fdc98c2ce7884d5fa4e7f9199b github.com/VictoriaMetrics/fasthttp v1.1.0 github.com/VictoriaMetrics/metrics v1.18.1 - github.com/VictoriaMetrics/metricsql v0.33.0 + github.com/VictoriaMetrics/metricsql v0.34.0 github.com/VividCortex/ewma v1.2.0 // indirect github.com/aws/aws-sdk-go v1.42.23 github.com/census-instrumentation/opencensus-proto v0.3.0 // indirect diff --git a/go.sum b/go.sum index a4e67a49c..ac754a011 100644 --- a/go.sum +++ b/go.sum @@ -109,8 +109,8 @@ github.com/VictoriaMetrics/fasthttp v1.1.0/go.mod h1:/7DMcogqd+aaD3G3Hg5kFgoFwlR github.com/VictoriaMetrics/metrics v1.12.2/go.mod h1:Z1tSfPfngDn12bTfZSCqArT3OPY3u88J12hSoOhuiRE= github.com/VictoriaMetrics/metrics v1.18.1 h1:OZ0+kTTto8oPfHnVAnTOoyl0XlRhRkoQrD2n2cOuRw0= github.com/VictoriaMetrics/metrics v1.18.1/go.mod h1:ArjwVz7WpgpegX/JpB0zpNF2h2232kErkEnzH1sxMmA= -github.com/VictoriaMetrics/metricsql v0.33.0 h1:UBj7+Tf4dhD47tIxMYfAiy/4GXJN6xPYTweCZ+sRqw0= -github.com/VictoriaMetrics/metricsql v0.33.0/go.mod h1:ylO7YITho/Iw6P71oEaGyHbO94bGoGtzWfLGqFhMIg8= +github.com/VictoriaMetrics/metricsql v0.34.0 h1:zF9yzRyNCAxzgEBBnE4y/p0QYNpSQp2jGEBCVE2fUD0= +github.com/VictoriaMetrics/metricsql v0.34.0/go.mod h1:ylO7YITho/Iw6P71oEaGyHbO94bGoGtzWfLGqFhMIg8= github.com/VividCortex/ewma v1.1.1/go.mod h1:2Tkkvm3sRDVXaiyucHiACn4cqf7DpdyLvmxzcbUokwA= github.com/VividCortex/ewma v1.2.0 h1:f58SaIzcDXrSy3kWaHNvuJgJ3Nmz59Zji6XoJR/q1ow= github.com/VividCortex/ewma v1.2.0/go.mod h1:nz4BbCtbLyFDeC9SUHbtcT5644juEuWfUAUnGx7j5l4= diff --git a/vendor/github.com/VictoriaMetrics/metricsql/rollup.go b/vendor/github.com/VictoriaMetrics/metricsql/rollup.go index 4887cf2db..3ecc2b73d 100644 --- a/vendor/github.com/VictoriaMetrics/metricsql/rollup.go +++ b/vendor/github.com/VictoriaMetrics/metricsql/rollup.go @@ -10,6 +10,7 @@ var rollupFuncs = map[string]bool{ "ascent_over_time": true, "avg_over_time": true, "changes": true, + "changes_prometheus": true, "count_eq_over_time": true, "count_gt_over_time": true, "count_le_over_time": true, @@ -18,6 +19,7 @@ var rollupFuncs = map[string]bool{ "decreases_over_time": true, "default_rollup": true, "delta": true, + "delta_prometheus": true, "deriv": true, "deriv_fast": true, "descent_over_time": true, @@ -32,6 +34,7 @@ var rollupFuncs = map[string]bool{ "idelta": true, "ideriv": true, "increase": true, + "increase_prometheus": true, "increase_pure": true, "increases_over_time": true, "integrate": true, diff --git a/vendor/modules.txt b/vendor/modules.txt index fa426e45d..1d02916c4 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -21,7 +21,7 @@ github.com/VictoriaMetrics/fasthttp/stackless # github.com/VictoriaMetrics/metrics v1.18.1 ## explicit github.com/VictoriaMetrics/metrics -# github.com/VictoriaMetrics/metricsql v0.33.0 +# github.com/VictoriaMetrics/metricsql v0.34.0 ## explicit github.com/VictoriaMetrics/metricsql github.com/VictoriaMetrics/metricsql/binaryop