From f365d35cf8277e825489861df69694bfd0aab288 Mon Sep 17 00:00:00 2001 From: Matthew Jacobson Date: Wed, 10 Jan 2024 14:40:00 -0500 Subject: [PATCH] Alerting: Show warning when query optimized (#78751) * Alerting: Show warning when query optimized * Use frame.AppendNotices * Improve warning to include why and a prompt for action --- pkg/services/ngalert/api/api_testing.go | 22 +++++- pkg/services/ngalert/api/api_testing_test.go | 73 ++++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/pkg/services/ngalert/api/api_testing.go b/pkg/services/ngalert/api/api_testing.go index 637150df2b1..8e7f76ce7b6 100644 --- a/pkg/services/ngalert/api/api_testing.go +++ b/pkg/services/ngalert/api/api_testing.go @@ -8,9 +8,11 @@ import ( "time" "github.com/benbjohnson/clock" - "github.com/grafana/alerting/models" amv2 "github.com/prometheus/alertmanager/api/v2/models" + "github.com/grafana/alerting/models" + "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/grafana/grafana/pkg/api/response" @@ -163,7 +165,7 @@ func (srv TestingApiSrv) RouteEvalQueries(c *contextmodel.ReqContext, cmd apimod cond.Condition = cond.Data[len(cond.Data)-1].RefID } - _, err := store.OptimizeAlertQueries(cond.Data) + optimizations, err := store.OptimizeAlertQueries(cond.Data) if err != nil { return ErrResp(http.StatusInternalServerError, err, "Failed to optimize query") } @@ -185,9 +187,25 @@ func (srv TestingApiSrv) RouteEvalQueries(c *contextmodel.ReqContext, cmd apimod return ErrResp(http.StatusInternalServerError, err, "Failed to evaluate queries and expressions") } + addOptimizedQueryWarnings(evalResults, optimizations) return response.JSONStreaming(http.StatusOK, evalResults) } +// addOptimizedQueryWarnings adds warnings to the query results for any queries that were optimized. +func addOptimizedQueryWarnings(evalResults *backend.QueryDataResponse, optimizations []store.Optimization) { + for _, opt := range optimizations { + if res, ok := evalResults.Responses[opt.RefID]; ok { + if len(res.Frames) > 0 { + res.Frames[0].AppendNotices(data.Notice{ + Severity: data.NoticeSeverityWarning, + Text: "Query optimized from Range to Instant type; all uses exclusively require the last datapoint. " + + "Consider modifying your query to Instant type to ensure accuracy.", // Currently this is the only optimization we do. + }) + } + } + } +} + func (srv TestingApiSrv) BacktestAlertRule(c *contextmodel.ReqContext, cmd apimodels.BacktestConfig) response.Response { if !srv.featureManager.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingBacktesting) { return ErrResp(http.StatusNotFound, nil, "Backgtesting API is not enabled") diff --git a/pkg/services/ngalert/api/api_testing_test.go b/pkg/services/ngalert/api/api_testing_test.go index e1efb86b6d1..5f81985acfe 100644 --- a/pkg/services/ngalert/api/api_testing_test.go +++ b/pkg/services/ngalert/api/api_testing_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -266,6 +267,78 @@ func TestRouteEvalQueries(t *testing.T) { evaluator.AssertCalled(t, "EvaluateRaw", mock.Anything, currentTime) }) }) + + t.Run("when query is optimizable", func(t *testing.T) { + rc := &contextmodel.ReqContext{ + Context: &web.Context{ + Req: &http.Request{}, + }, + SignedInUser: &user.SignedInUser{ + OrgID: 1, + }, + } + t.Run("should return warning notice on optimized queries", func(t *testing.T) { + queries := []models.AlertQuery{ + models.CreatePrometheusQuery("A", "1", 1000, 43200, false, "some-ds"), + models.CreatePrometheusQuery("B", "1", 1000, 43200, false, "some-ds"), + models.CreatePrometheusQuery("C", "1", 1000, 43200, false, "some-ds"), // Not optimizable. + models.CreateReduceExpression("D", "A", "last"), + models.CreateReduceExpression("E", "B", "last"), + } + + currentTime := time.Now() + + ac := acMock.New().WithPermissions([]ac.Permission{ + {Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(queries[0].DatasourceUID)}, + }) + + ds := &fakes.FakeCacheService{DataSources: []*datasources.DataSource{ + {UID: queries[0].DatasourceUID}, + }} + + evaluator := &eval_mocks.ConditionEvaluatorMock{} + createEmptyFrameResponse := func(refId string) backend.DataResponse { + frame := data.NewFrame("") + frame.RefID = refId + frame.SetMeta(&data.FrameMeta{}) + return backend.DataResponse{ + Frames: []*data.Frame{frame}, + Error: nil, + } + } + result := &backend.QueryDataResponse{ + Responses: map[string]backend.DataResponse{ + "A": createEmptyFrameResponse("A"), + "B": createEmptyFrameResponse("B"), + "C": createEmptyFrameResponse("C"), + }, + } + evaluator.EXPECT().EvaluateRaw(mock.Anything, mock.Anything).Return(result, nil) + + srv := createTestingApiSrv(t, ds, ac, eval_mocks.NewEvaluatorFactory(evaluator)) + + response := srv.RouteEvalQueries(rc, definitions.EvalQueriesPayload{ + Data: ApiAlertQueriesFromAlertQueries(queries), + Now: currentTime, + }) + + require.Equal(t, http.StatusOK, response.Status()) + + evaluator.AssertCalled(t, "EvaluateRaw", mock.Anything, currentTime) + + require.Equal(t, []data.Notice{{ + Severity: data.NoticeSeverityWarning, + Text: "Query optimized from Range to Instant type; all uses exclusively require the last datapoint. Consider modifying your query to Instant type to ensure accuracy.", + }}, result.Responses["A"].Frames[0].Meta.Notices) + + require.Equal(t, []data.Notice{{ + Severity: data.NoticeSeverityWarning, + Text: "Query optimized from Range to Instant type; all uses exclusively require the last datapoint. Consider modifying your query to Instant type to ensure accuracy.", + }}, result.Responses["B"].Frames[0].Meta.Notices) + + require.Equal(t, 0, len(result.Responses["C"].Frames[0].Meta.Notices)) + }) + }) } func createTestingApiSrv(t *testing.T, ds *fakes.FakeCacheService, ac *acMock.Mock, evaluator eval.EvaluatorFactory) *TestingApiSrv {