From b4e1e1871f004a00859f415575493a945386d509 Mon Sep 17 00:00:00 2001 From: Yuri Tseretyan Date: Wed, 11 Jan 2023 10:52:54 -0500 Subject: [PATCH] Alerting: Fix evaluation timeout (#61303) --- pkg/services/ngalert/eval/eval.go | 8 ++++-- pkg/services/ngalert/eval/eval_test.go | 36 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/pkg/services/ngalert/eval/eval.go b/pkg/services/ngalert/eval/eval.go index 3f2f982b0ea..2e2f905636e 100644 --- a/pkg/services/ngalert/eval/eval.go +++ b/pkg/services/ngalert/eval/eval.go @@ -41,9 +41,13 @@ type ConditionEvaluator interface { Evaluate(ctx context.Context, now time.Time) (Results, error) } +type expressionService interface { + ExecutePipeline(ctx context.Context, now time.Time, pipeline expr.DataPipeline) (*backend.QueryDataResponse, error) +} + type conditionEvaluator struct { pipeline expr.DataPipeline - expressionService *expr.Service + expressionService expressionService condition models.Condition evalTimeout time.Duration } @@ -62,7 +66,7 @@ func (r *conditionEvaluator) EvaluateRaw(ctx context.Context, now time.Time) (re }() execCtx := ctx - if r.evalTimeout <= 0 { + if r.evalTimeout >= 0 { timeoutCtx, cancel := context.WithTimeout(ctx, r.evalTimeout) defer cancel() execCtx = timeoutCtx diff --git a/pkg/services/ngalert/eval/eval_test.go b/pkg/services/ngalert/eval/eval_test.go index 188999b3c69..b53c4f3f51b 100644 --- a/pkg/services/ngalert/eval/eval_test.go +++ b/pkg/services/ngalert/eval/eval_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/stretchr/testify/require" ptr "github.com/xorcare/pointer" @@ -544,3 +545,38 @@ func TestValidate(t *testing.T) { }) } } + +func TestEvaluateRaw(t *testing.T) { + t.Run("should timeout if request takes too long", func(t *testing.T) { + unexpectedResponse := &backend.QueryDataResponse{} + + e := conditionEvaluator{ + pipeline: nil, + expressionService: &fakeExpressionService{ + hook: func(ctx context.Context, now time.Time, pipeline expr.DataPipeline) (*backend.QueryDataResponse, error) { + ts := time.Now() + for time.Since(ts) <= 10*time.Second { + if ctx.Err() != nil { + return nil, ctx.Err() + } + time.Sleep(10 * time.Millisecond) + } + return unexpectedResponse, nil + }, + }, + condition: models.Condition{}, + evalTimeout: 10 * time.Millisecond, + } + + _, err := e.EvaluateRaw(context.Background(), time.Now()) + require.ErrorIs(t, err, context.DeadlineExceeded) + }) +} + +type fakeExpressionService struct { + hook func(ctx context.Context, now time.Time, pipeline expr.DataPipeline) (*backend.QueryDataResponse, error) +} + +func (f fakeExpressionService) ExecutePipeline(ctx context.Context, now time.Time, pipeline expr.DataPipeline) (*backend.QueryDataResponse, error) { + return f.hook(ctx, now, pipeline) +}