From 037fd9cc8cac8a7a17a647cf743d1511a54884d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Farkas?= Date: Mon, 22 Nov 2021 14:28:51 +0100 Subject: [PATCH] Loki: alerting: adjust step-calculation to be the same as in frontend (#42033) * loki: alerting: adjust step-calculation to be the same as in frontend * loki: simplified test * lint fix --- pkg/tsdb/loki/loki.go | 2 +- pkg/tsdb/loki/loki_test.go | 52 +--------------------------------- pkg/tsdb/loki/step.go | 31 +++++++++++++++++++++ pkg/tsdb/loki/step_test.go | 57 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 52 deletions(-) create mode 100644 pkg/tsdb/loki/step.go create mode 100644 pkg/tsdb/loki/step_test.go diff --git a/pkg/tsdb/loki/loki.go b/pkg/tsdb/loki/loki.go index e8491dc6820..1737a040d3f 100644 --- a/pkg/tsdb/loki/loki.go +++ b/pkg/tsdb/loki/loki.go @@ -197,7 +197,7 @@ func parseQuery(dsInfo *datasourceInfo, queryContext *backend.QueryDataRequest) resolution = model.Resolution } - step := time.Duration(int64(query.Interval) * resolution) + step := calculateStep(query.Interval, query.TimeRange.To.Sub(query.TimeRange.From), resolution) qs = append(qs, &lokiQuery{ Expr: model.Expr, diff --git a/pkg/tsdb/loki/loki_test.go b/pkg/tsdb/loki/loki_test.go index 1619614be5e..ad492adebcb 100644 --- a/pkg/tsdb/loki/loki_test.go +++ b/pkg/tsdb/loki/loki_test.go @@ -1,7 +1,6 @@ package loki import ( - "fmt" "testing" "time" @@ -41,7 +40,7 @@ func TestLoki(t *testing.T) { require.Equal(t, `http_request_total{app="backend", device="mobile"}`, formatLegend(metric, query)) }) - t.Run("parsing query model with step", func(t *testing.T) { + t.Run("parsing query model", func(t *testing.T) { queryContext := &backend.QueryDataRequest{ Queries: []backend.DataQuery{ { @@ -65,55 +64,6 @@ func TestLoki(t *testing.T) { require.NoError(t, err) require.Equal(t, time.Second*30, models[0].Step) }) - - t.Run("parsing query model without step parameter", func(t *testing.T) { - queryContext1 := &backend.QueryDataRequest{ - Queries: []backend.DataQuery{ - { - JSON: []byte(` - { - "expr": "go_goroutines", - "format": "time_series", - "refId": "A" - }`, - ), - TimeRange: backend.TimeRange{ - From: time.Now().Add(-48 * time.Hour), - To: time.Now(), - }, - Interval: time.Minute * 2, - }, - }, - } - dsInfo := &datasourceInfo{} - models, err := parseQuery(dsInfo, queryContext1) - require.NoError(t, err) - require.Equal(t, time.Minute*2, models[0].Step) - - queryContext2 := &backend.QueryDataRequest{ - Queries: []backend.DataQuery{ - { - JSON: []byte(` - { - "expr": "go_goroutines", - "format": "time_series", - "refId": "A" - }`, - ), - TimeRange: backend.TimeRange{ - From: time.Now().Add(-48 * time.Hour), - To: time.Now(), - }, - Interval: time.Second * 2, - }, - }, - } - - models, err = parseQuery(dsInfo, queryContext2) - require.NoError(t, err) - fmt.Println(models) - require.Equal(t, time.Second*2, models[0].Step) - }) } func TestParseResponse(t *testing.T) { diff --git a/pkg/tsdb/loki/step.go b/pkg/tsdb/loki/step.go new file mode 100644 index 00000000000..78f5d651d79 --- /dev/null +++ b/pkg/tsdb/loki/step.go @@ -0,0 +1,31 @@ +package loki + +import ( + "math" + "time" +) + +// round the duration to the nearest millisecond larger-or-equal-to the duration +func ceilMs(duration time.Duration) time.Duration { + floatMs := float64(duration.Nanoseconds()) / 1000.0 / 1000.0 + ceilMs := math.Ceil(floatMs) + return time.Duration(ceilMs) * time.Millisecond +} + +func durationMax(d1 time.Duration, d2 time.Duration) time.Duration { + if d1.Nanoseconds() >= d2.Nanoseconds() { + return d1 + } else { + return d2 + } +} + +func calculateStep(baseInterval time.Duration, timeRange time.Duration, resolution int64) time.Duration { + step := time.Duration(baseInterval.Nanoseconds() * resolution) + + safeStep := timeRange / 11000 + + chosenStep := durationMax(step, safeStep) + + return ceilMs(chosenStep) +} diff --git a/pkg/tsdb/loki/step_test.go b/pkg/tsdb/loki/step_test.go new file mode 100644 index 00000000000..ad85d513879 --- /dev/null +++ b/pkg/tsdb/loki/step_test.go @@ -0,0 +1,57 @@ +package loki + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestLokiStep(t *testing.T) { + t.Run("base case", func(t *testing.T) { + require.Equal(t, time.Second*14, calculateStep(time.Second*7, time.Second, 2)) + }) + + t.Run("step should be at least 1 millisecond", func(t *testing.T) { + require.Equal(t, time.Millisecond*1, calculateStep(time.Microsecond*500, time.Second, 1)) + }) + + t.Run("safeInterval should happen", func(t *testing.T) { + // safeInterval + require.Equal(t, time.Second*3, calculateStep(time.Second*2, time.Second*33000, 1)) + }) + + t.Run("step should math.Ceil in milliseconds", func(t *testing.T) { + require.Equal(t, time.Millisecond*2, calculateStep(time.Microsecond*1234, time.Second*1, 1)) + }) + + t.Run("step should math.Ceil in milliseconds, even if safeInterval happens", func(t *testing.T) { + require.Equal(t, time.Millisecond*3001, calculateStep(time.Second*2, time.Second*33001, 1)) + }) + + t.Run("resolution should happen", func(t *testing.T) { + require.Equal(t, time.Second*5, calculateStep(time.Second*1, time.Second*100, 5)) + }) + + t.Run("safeInterval check should happen after resolution is used", func(t *testing.T) { + require.Equal(t, time.Second*4, calculateStep(time.Second*2, time.Second*33000, 2)) + }) + + t.Run("survive interval=0", func(t *testing.T) { + // interval=0. this should never happen, but we make sure we return something sane + // (in this case safeInterval will take care of the problem) + require.Equal(t, time.Second*2, calculateStep(time.Second*0, time.Second*22000, 1)) + }) + + t.Run("survive resolution=0", func(t *testing.T) { + // resolution=0. this should never happen, but we make sure we return something sane + // (in this case safeInterval will take care of the problem) + require.Equal(t, time.Second*2, calculateStep(time.Second*1, time.Second*22000, 0)) + }) + + t.Run("survive interval=0 and resolution=0", func(t *testing.T) { + // resolution=0 and interval=0. this should never happen, but we make sure we return something sane + // (in this case safeInterval will take care of the problem) + require.Equal(t, time.Second*2, calculateStep(time.Second*0, time.Second*22000, 0)) + }) +}