From afa33f12b2cf50d2c7e438f498d27b0684797778 Mon Sep 17 00:00:00 2001 From: Matthew Jacobson Date: Wed, 10 Jan 2024 15:52:58 -0500 Subject: [PATCH] Alerting: Create alertingQueryOptimization feature flag for alert query optimization (#78932) * Alerting: Create feature flag for alert query optimization Adds a feature flag alertingQueryOptimization for an already existing functionality: alert query optimization. This feature flag will now be disabled by default. --- .../configure-grafana/feature-toggles/index.md | 1 + .../grafana-data/src/types/featureToggles.gen.ts | 1 + pkg/services/featuremgmt/registry.go | 8 ++++++++ pkg/services/featuremgmt/toggles_gen.csv | 1 + pkg/services/featuremgmt/toggles_gen.go | 4 ++++ pkg/services/ngalert/api/api_testing.go | 16 +++++++++++----- pkg/services/ngalert/api/api_testing_test.go | 12 +++++++----- pkg/services/ngalert/store/alert_rule.go | 11 +++++++---- 8 files changed, 40 insertions(+), 14 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 490e10da69d..c79f88f4240 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -58,6 +58,7 @@ Some features are enabled by default. You can disable these feature by setting t | `logRowsPopoverMenu` | Enable filtering menu displayed when text of a log line is selected | Yes | | `displayAnonymousStats` | Enables anonymous stats to be shown in the UI for Grafana | Yes | | `lokiQueryHints` | Enables query hints for Loki | Yes | +| `alertingQueryOptimization` | Optimizes eligible queries in order to reduce load on datasources | | ## Preview feature toggles diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 9e814711861..c4ed31183e2 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -173,4 +173,5 @@ export interface FeatureToggles { alertingPreviewUpgrade?: boolean; enablePluginsTracingByDefault?: boolean; cloudRBACRoles?: boolean; + alertingQueryOptimization?: boolean; } diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 2288fb2f510..2e5b976ad35 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1315,5 +1315,13 @@ var ( RequiresRestart: true, Created: time.Date(2024, time.January, 10, 12, 0, 0, 0, time.UTC), }, + { + Name: "alertingQueryOptimization", + Description: "Optimizes eligible queries in order to reduce load on datasources", + Stage: FeatureStageGeneralAvailability, + Owner: grafanaAlertingSquad, + AllowSelfServe: false, + Created: time.Date(2024, time.January, 10, 12, 0, 0, 0, time.UTC), + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index ce9386b947d..aab72c8ba50 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -154,3 +154,4 @@ lokiQueryHints,GA,@grafana/observability-logs,2023-12-18,false,false,false,true alertingPreviewUpgrade,experimental,@grafana/alerting-squad,2024-01-03,false,false,true,false enablePluginsTracingByDefault,experimental,@grafana/plugins-platform-backend,2024-01-09,false,false,true,false cloudRBACRoles,experimental,@grafana/identity-access-team,2024-01-10,false,false,true,false +alertingQueryOptimization,GA,@grafana/alerting-squad,2024-01-10,false,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 85bc17b8d38..42e47472d84 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -626,4 +626,8 @@ const ( // FlagCloudRBACRoles // Enabled grafana cloud specific RBAC roles FlagCloudRBACRoles = "cloudRBACRoles" + + // FlagAlertingQueryOptimization + // Optimizes eligible queries in order to reduce load on datasources + FlagAlertingQueryOptimization = "alertingQueryOptimization" ) diff --git a/pkg/services/ngalert/api/api_testing.go b/pkg/services/ngalert/api/api_testing.go index 8e7f76ce7b6..27b27e3f711 100644 --- a/pkg/services/ngalert/api/api_testing.go +++ b/pkg/services/ngalert/api/api_testing.go @@ -69,8 +69,10 @@ func (srv TestingApiSrv) RouteTestGrafanaRuleConfig(c *contextmodel.ReqContext, return response.ErrOrFallback(http.StatusInternalServerError, "failed to authorize access to rule group", err) } - if _, err := store.OptimizeAlertQueries(rule.Data); err != nil { - return ErrResp(http.StatusInternalServerError, err, "Failed to optimize query") + if srv.featureManager.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingQueryOptimization) { + if _, err := store.OptimizeAlertQueries(rule.Data); err != nil { + return ErrResp(http.StatusInternalServerError, err, "Failed to optimize query") + } } evaluator, err := srv.evaluator.Create(eval.NewContext(c.Req.Context(), c.SignedInUser), rule.GetEvalCondition()) @@ -165,9 +167,13 @@ func (srv TestingApiSrv) RouteEvalQueries(c *contextmodel.ReqContext, cmd apimod cond.Condition = cond.Data[len(cond.Data)-1].RefID } - optimizations, err := store.OptimizeAlertQueries(cond.Data) - if err != nil { - return ErrResp(http.StatusInternalServerError, err, "Failed to optimize query") + var optimizations []store.Optimization + if srv.featureManager.IsEnabled(c.Req.Context(), featuremgmt.FlagAlertingQueryOptimization) { + var err error + optimizations, err = store.OptimizeAlertQueries(cond.Data) + if err != nil { + return ErrResp(http.StatusInternalServerError, err, "Failed to optimize query") + } } evaluator, err := srv.evaluator.Create(eval.NewContext(c.Req.Context(), c.SignedInUser), cond) diff --git a/pkg/services/ngalert/api/api_testing_test.go b/pkg/services/ngalert/api/api_testing_test.go index 5f81985acfe..6caa8c0022b 100644 --- a/pkg/services/ngalert/api/api_testing_test.go +++ b/pkg/services/ngalert/api/api_testing_test.go @@ -17,6 +17,7 @@ import ( contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/datasources" fakes "github.com/grafana/grafana/pkg/services/datasources/fakes" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ngalert/accesscontrol" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/eval" @@ -146,7 +147,7 @@ func TestRouteTestGrafanaRuleConfig(t *testing.T) { {Action: datasources.ActionQuery, Scope: datasources.ScopeProvider.GetResourceScopeUID(data1.DatasourceUID)}, }) - srv := createTestingApiSrv(t, nil, ac, eval_mocks.NewEvaluatorFactory(&eval_mocks.ConditionEvaluatorMock{})) + srv := createTestingApiSrv(t, nil, ac, eval_mocks.NewEvaluatorFactory(&eval_mocks.ConditionEvaluatorMock{}), &featuremgmt.FeatureManager{}) rule := validRule() rule.GrafanaManagedAlert.Data = ApiAlertQueriesFromAlertQueries([]models.AlertQuery{data1, data2}) @@ -180,7 +181,7 @@ func TestRouteTestGrafanaRuleConfig(t *testing.T) { evalFactory := eval_mocks.NewEvaluatorFactory(evaluator) - srv := createTestingApiSrv(t, ds, ac, evalFactory) + srv := createTestingApiSrv(t, ds, ac, evalFactory, &featuremgmt.FeatureManager{}) rule := validRule() rule.GrafanaManagedAlert.Data = ApiAlertQueriesFromAlertQueries([]models.AlertQuery{data1, data2}) @@ -255,7 +256,7 @@ func TestRouteEvalQueries(t *testing.T) { } evaluator.EXPECT().EvaluateRaw(mock.Anything, mock.Anything).Return(result, nil) - srv := createTestingApiSrv(t, ds, ac, eval_mocks.NewEvaluatorFactory(evaluator)) + srv := createTestingApiSrv(t, ds, ac, eval_mocks.NewEvaluatorFactory(evaluator), &featuremgmt.FeatureManager{}) response := srv.RouteEvalQueries(rc, definitions.EvalQueriesPayload{ Data: ApiAlertQueriesFromAlertQueries([]models.AlertQuery{data1, data2}), @@ -315,7 +316,7 @@ func TestRouteEvalQueries(t *testing.T) { } evaluator.EXPECT().EvaluateRaw(mock.Anything, mock.Anything).Return(result, nil) - srv := createTestingApiSrv(t, ds, ac, eval_mocks.NewEvaluatorFactory(evaluator)) + srv := createTestingApiSrv(t, ds, ac, eval_mocks.NewEvaluatorFactory(evaluator), featuremgmt.WithManager(featuremgmt.FlagAlertingQueryOptimization)) response := srv.RouteEvalQueries(rc, definitions.EvalQueriesPayload{ Data: ApiAlertQueriesFromAlertQueries(queries), @@ -341,7 +342,7 @@ func TestRouteEvalQueries(t *testing.T) { }) } -func createTestingApiSrv(t *testing.T, ds *fakes.FakeCacheService, ac *acMock.Mock, evaluator eval.EvaluatorFactory) *TestingApiSrv { +func createTestingApiSrv(t *testing.T, ds *fakes.FakeCacheService, ac *acMock.Mock, evaluator eval.EvaluatorFactory, featureManager *featuremgmt.FeatureManager) *TestingApiSrv { if ac == nil { ac = acMock.New() } @@ -352,5 +353,6 @@ func createTestingApiSrv(t *testing.T, ds *fakes.FakeCacheService, ac *acMock.Mo evaluator: evaluator, cfg: config(t), tracer: tracing.InitializeTracerForTest(), + featureManager: featureManager, } } diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index 49161802f28..a94eca1602e 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/search/model" @@ -552,10 +553,12 @@ func (st DBstore) GetAlertRulesForScheduling(ctx context.Context, query *ngmodel st.Logger.Error("Invalid rule found in DB store, ignoring it", "func", "GetAlertRulesForScheduling", "error", err) continue } - if optimizations, err := OptimizeAlertQueries(rule.Data); err != nil { - st.Logger.Error("Could not migrate rule from range to instant query", "rule", rule.UID, "err", err) - } else if len(optimizations) > 0 { - st.Logger.Info("Migrated rule from range to instant query", "rule", rule.UID, "migrated_queries", len(optimizations)) + if st.FeatureToggles.IsEnabled(ctx, featuremgmt.FlagAlertingQueryOptimization) { + if optimizations, err := OptimizeAlertQueries(rule.Data); err != nil { + st.Logger.Error("Could not migrate rule from range to instant query", "rule", rule.UID, "err", err) + } else if len(optimizations) > 0 { + st.Logger.Info("Migrated rule from range to instant query", "rule", rule.UID, "migrated_queries", len(optimizations)) + } } rules = append(rules, rule) }