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 a9395c559ae..030c81df2ed 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -101,7 +101,6 @@ Most [generally available](https://grafana.com/docs/release-life-cycle/#general- | `autoMigrateStatPanel` | Migrate old stat panel to supported stat panel - broken out from autoMigrateOldPanels to enable granular tracking | | `disableAngular` | Dynamic flag to disable angular at runtime. The preferred method is to set `angular_support_enabled` to `false` in the [security] settings, which allows you to change the state at runtime. | | `grpcServer` | Run the GRPC server | -| `alertingNoNormalState` | Stop maintaining state of alerts that are not firing | | `renderAuthJWT` | Uses JWT-based auth for rendering instead of relying on remote cache | | `refactorVariablesTimeRange` | Refactor time range variables flow to reduce number of API calls made when query variables are chained | | `faroDatasourceSelector` | Enable the data source selector within the Frontend Apps section of the Frontend Observability | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index a5708006967..24f2244ac83 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -47,7 +47,6 @@ export interface FeatureToggles { nestedFolders?: boolean; alertingBacktesting?: boolean; editPanelCSVDragAndDrop?: boolean; - alertingNoNormalState?: boolean; logsContextDatasourceUi?: boolean; lokiShardSplitting?: boolean; lokiQuerySplitting?: boolean; diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index ffec9103ac7..7571ff6a37f 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -226,14 +226,6 @@ var ( Stage: FeatureStageExperimental, Owner: grafanaDatavizSquad, }, - { - Name: "alertingNoNormalState", - Description: "Stop maintaining state of alerts that are not firing", - Stage: FeatureStagePublicPreview, - RequiresRestart: false, - Owner: grafanaAlertingSquad, - HideFromAdminPage: true, - }, { Name: "logsContextDatasourceUi", Description: "Allow datasource to provide custom UI for context view", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 25f5855541b..250833a78a3 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -28,7 +28,6 @@ accessControlOnCall,GA,@grafana/identity-access-team,false,false,false nestedFolders,GA,@grafana/search-and-storage,false,false,false alertingBacktesting,experimental,@grafana/alerting-squad,false,false,false editPanelCSVDragAndDrop,experimental,@grafana/dataviz-squad,false,false,true -alertingNoNormalState,preview,@grafana/alerting-squad,false,false,false logsContextDatasourceUi,GA,@grafana/observability-logs,false,false,true lokiShardSplitting,experimental,@grafana/observability-logs,false,false,true lokiQuerySplitting,GA,@grafana/observability-logs,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index e75d59f0257..556275ed908 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -123,10 +123,6 @@ const ( // Enables drag and drop for CSV and Excel files FlagEditPanelCSVDragAndDrop = "editPanelCSVDragAndDrop" - // FlagAlertingNoNormalState - // Stop maintaining state of alerts that are not firing - FlagAlertingNoNormalState = "alertingNoNormalState" - // FlagLogsContextDatasourceUi // Allow datasource to provide custom UI for context view FlagLogsContextDatasourceUi = "logsContextDatasourceUi" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index d1a6a295a3d..573ae344f02 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -294,7 +294,8 @@ "metadata": { "name": "alertingNoNormalState", "resourceVersion": "1718727528075", - "creationTimestamp": "2023-01-13T23:29:29Z" + "creationTimestamp": "2023-01-13T23:29:29Z", + "deletionTimestamp": "2025-01-31T15:46:31Z" }, "spec": { "description": "Stop maintaining state of alerts that are not firing", diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index 841870f48d8..8688325d3ca 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -416,7 +416,6 @@ func (ng *AlertNG) init() error { Images: ng.ImageService, Clock: clk, Historian: history, - DoNotSaveNormalState: ng.FeatureToggles.IsEnabledGlobally(featuremgmt.FlagAlertingNoNormalState), ApplyNoDataAndErrorToAllStates: ng.FeatureToggles.IsEnabledGlobally(featuremgmt.FlagAlertingNoDataErrorExecution), MaxStateSaveConcurrency: ng.Cfg.UnifiedAlerting.MaxStateSaveConcurrency, StatePeriodicSaveBatchSize: ng.Cfg.UnifiedAlerting.StatePeriodicSaveBatchSize, @@ -537,9 +536,8 @@ func initInstanceStore(sqlStore db.DB, logger log.Logger, featureToggles feature FeatureToggles: featureToggles, } simpleInstanceStore := store.InstanceDBStore{ - SQLStore: sqlStore, - Logger: logger, - FeatureToggles: featureToggles, + SQLStore: sqlStore, + Logger: logger, } if featureToggles.IsEnabledGlobally(featuremgmt.FlagAlertingSaveStateCompressed) { diff --git a/pkg/services/ngalert/state/cache.go b/pkg/services/ngalert/state/cache.go index d08d9fde6d0..1abeff1c2a9 100644 --- a/pkg/services/ngalert/state/cache.go +++ b/pkg/services/ngalert/state/cache.go @@ -297,22 +297,19 @@ func (c *cache) get(orgID int64, alertRuleUID string, stateId data.Fingerprint) return nil } -func (c *cache) getAll(orgID int64, skipNormalState bool) []*State { +func (c *cache) getAll(orgID int64) []*State { var states []*State c.mtxStates.RLock() defer c.mtxStates.RUnlock() for _, v1 := range c.states[orgID] { for _, v2 := range v1.states { - if skipNormalState && IsNormalStateWithNoReason(v2) { - continue - } states = append(states, v2) } } return states } -func (c *cache) getStatesForRuleUID(orgID int64, alertRuleUID string, skipNormalState bool) []*State { +func (c *cache) getStatesForRuleUID(orgID int64, alertRuleUID string) []*State { c.mtxStates.RLock() defer c.mtxStates.RUnlock() orgRules, ok := c.states[orgID] @@ -325,9 +322,6 @@ func (c *cache) getStatesForRuleUID(orgID int64, alertRuleUID string, skipNormal } result := make([]*State, 0, len(rs.states)) for _, state := range rs.states { - if skipNormalState && IsNormalStateWithNoReason(state) { - continue - } result = append(result, state) } return result @@ -357,16 +351,13 @@ func (c *cache) removeByRuleUID(orgID int64, uid string) []*State { } // GetAlertInstances returns the whole content of the cache as a slice of AlertInstance. -func (c *cache) GetAlertInstances(skipNormalState bool) []ngModels.AlertInstance { +func (c *cache) GetAlertInstances() []ngModels.AlertInstance { var states []ngModels.AlertInstance c.mtxStates.RLock() defer c.mtxStates.RUnlock() for _, orgStates := range c.states { for _, v1 := range orgStates { for _, v2 := range v1.states { - if skipNormalState && IsNormalStateWithNoReason(v2) { - continue - } key, err := v2.GetAlertInstanceKey() if err != nil { continue diff --git a/pkg/services/ngalert/state/manager.go b/pkg/services/ngalert/state/manager.go index e037c673d4b..cc22a65f517 100644 --- a/pkg/services/ngalert/state/manager.go +++ b/pkg/services/ngalert/state/manager.go @@ -55,7 +55,6 @@ type Manager struct { historian Historian externalURL *url.URL - doNotSaveNormalState bool applyNoDataAndErrorToAllStates bool rulesPerRuleGroupLimit int64 @@ -69,8 +68,6 @@ type ManagerCfg struct { Images ImageCapturer Clock clock.Clock Historian Historian - // DoNotSaveNormalState controls whether eval.Normal state is persisted to the database and returned by get methods - DoNotSaveNormalState bool // MaxStateSaveConcurrency controls the number of goroutines (per rule) that can save alert state in parallel. MaxStateSaveConcurrency int // StatePeriodicSaveBatchSize controls the size of the alert instance batch that is saved periodically when the @@ -109,7 +106,6 @@ func NewManager(cfg ManagerCfg, statePersister StatePersister) *Manager { historian: cfg.Historian, clock: cfg.Clock, externalURL: cfg.ExternalURL, - doNotSaveNormalState: cfg.DoNotSaveNormalState, applyNoDataAndErrorToAllStates: cfg.ApplyNoDataAndErrorToAllStates, rulesPerRuleGroupLimit: cfg.RulesPerRuleGroupLimit, persister: statePersister, @@ -457,7 +453,7 @@ func (st *Manager) setNextStateForRule(ctx context.Context, alertRule *ngModels. } func (st *Manager) setNextStateForAll(alertRule *ngModels.AlertRule, result eval.Result, logger log.Logger, extraAnnotations data.Labels, takeImageFn takeImageFn) []StateTransition { - currentStates := st.cache.getStatesForRuleUID(alertRule.OrgID, alertRule.UID, false) + currentStates := st.cache.getStatesForRuleUID(alertRule.OrgID, alertRule.UID) transitions := make([]StateTransition, 0, len(currentStates)) updated := ruleStates{ states: make(map[data.Fingerprint]*State, len(currentStates)), @@ -582,11 +578,11 @@ func resultStateReason(result eval.Result, rule *ngModels.AlertRule) string { } func (st *Manager) GetAll(orgID int64) []*State { - allStates := st.cache.getAll(orgID, st.doNotSaveNormalState) + allStates := st.cache.getAll(orgID) return allStates } func (st *Manager) GetStatesForRuleUID(orgID int64, alertRuleUID string) []*State { - return st.cache.getStatesForRuleUID(orgID, alertRuleUID, st.doNotSaveNormalState) + return st.cache.getStatesForRuleUID(orgID, alertRuleUID) } func (st *Manager) GetStatusForRuleUID(orgID int64, alertRuleUID string) ngModels.RuleStatus { diff --git a/pkg/services/ngalert/state/persister_async.go b/pkg/services/ngalert/state/persister_async.go index dd741addf50..0afabfdbee5 100644 --- a/pkg/services/ngalert/state/persister_async.go +++ b/pkg/services/ngalert/state/persister_async.go @@ -13,27 +13,24 @@ import ( ) type AlertInstancesProvider interface { - GetAlertInstances(skipNormalState bool) []models.AlertInstance + GetAlertInstances() []models.AlertInstance } type AsyncStatePersister struct { - log log.Logger - // doNotSaveNormalState controls whether eval.Normal state is persisted to the database and returned by get methods. - doNotSaveNormalState bool - batchSize int - store InstanceStore - ticker *clock.Ticker - metrics *metrics.State + log log.Logger + batchSize int + store InstanceStore + ticker *clock.Ticker + metrics *metrics.State } func NewAsyncStatePersister(log log.Logger, ticker *clock.Ticker, cfg ManagerCfg) StatePersister { return &AsyncStatePersister{ - log: log, - store: cfg.InstanceStore, - ticker: ticker, - doNotSaveNormalState: cfg.DoNotSaveNormalState, - batchSize: cfg.StatePeriodicSaveBatchSize, - metrics: cfg.Metrics, + log: log, + store: cfg.InstanceStore, + ticker: ticker, + batchSize: cfg.StatePeriodicSaveBatchSize, + metrics: cfg.Metrics, } } @@ -59,7 +56,7 @@ func (a *AsyncStatePersister) Async(ctx context.Context, instancesProvider Alert func (a *AsyncStatePersister) fullSync(ctx context.Context, instancesProvider AlertInstancesProvider) error { startTime := time.Now() a.log.Debug("Full state sync start") - instances := instancesProvider.GetAlertInstances(a.doNotSaveNormalState) + instances := instancesProvider.GetAlertInstances() if err := a.store.FullSync(ctx, instances, a.batchSize); err != nil { a.log.Error("Full state sync failed", "duration", time.Since(startTime), "instances", len(instances)) return err diff --git a/pkg/services/ngalert/state/persister_sync.go b/pkg/services/ngalert/state/persister_sync.go index 5e0e653547b..39eddd2f794 100644 --- a/pkg/services/ngalert/state/persister_sync.go +++ b/pkg/services/ngalert/state/persister_sync.go @@ -15,8 +15,6 @@ import ( type SyncStatePersister struct { log log.Logger store InstanceStore - // doNotSaveNormalState controls whether eval.Normal state is persisted to the database and returned by get methods. - doNotSaveNormalState bool // maxStateSaveConcurrency controls the number of goroutines (per rule) that can save alert state in parallel. maxStateSaveConcurrency int } @@ -25,7 +23,6 @@ func NewSyncStatePersisiter(log log.Logger, cfg ManagerCfg) StatePersister { return &SyncStatePersister{ log: log, store: cfg.InstanceStore, - doNotSaveNormalState: cfg.DoNotSaveNormalState, maxStateSaveConcurrency: cfg.MaxStateSaveConcurrency, } } @@ -84,11 +81,6 @@ func (a *SyncStatePersister) saveAlertStates(ctx context.Context, states ...Stat return nil } - // Do not save normal state to database and remove transition to Normal state but keep mapped states - if a.doNotSaveNormalState && IsNormalStateWithNoReason(s.State) && !s.Changed() { - return nil - } - key, err := s.GetAlertInstanceKey() if err != nil { logger.Error("Failed to create a key for alert state to save it to database. The state will be ignored ", "cacheID", s.CacheID, "error", err, "labels", s.Labels.String()) diff --git a/pkg/services/ngalert/state/persister_sync_rule.go b/pkg/services/ngalert/state/persister_sync_rule.go index aa1d6096b33..846c23a239d 100644 --- a/pkg/services/ngalert/state/persister_sync_rule.go +++ b/pkg/services/ngalert/state/persister_sync_rule.go @@ -11,16 +11,14 @@ import ( ) type SyncRuleStatePersister struct { - log log.Logger - store InstanceStore - doNotSaveNormalState bool + log log.Logger + store InstanceStore } func NewSyncRuleStatePersisiter(log log.Logger, cfg ManagerCfg) StatePersister { return &SyncRuleStatePersister{ - log: log, - store: cfg.InstanceStore, - doNotSaveNormalState: cfg.DoNotSaveNormalState, + log: log, + store: cfg.InstanceStore, } } @@ -41,10 +39,6 @@ func (a *SyncRuleStatePersister) Sync(ctx context.Context, span trace.Span, rule continue } - if a.doNotSaveNormalState && IsNormalStateWithNoReason(s.State) && !s.Changed() { - continue - } - key, err := s.GetAlertInstanceKey() if err != nil { logger.Error("Failed to create a key for alert state to save it. The state will be ignored ", "cacheID", s.CacheID, "error", err, "labels", s.Labels.String(), "rule_uid", ruleKey.UID, "rule_group", ruleKey.RuleGroup) diff --git a/pkg/services/ngalert/state/state.go b/pkg/services/ngalert/state/state.go index 7a0cf4834ce..aee77903096 100644 --- a/pkg/services/ngalert/state/state.go +++ b/pkg/services/ngalert/state/state.go @@ -233,11 +233,6 @@ func (a *State) SetNextValues(result eval.Result) { a.Values = newValues } -// IsNormalStateWithNoReason returns true if the state is Normal and reason is empty -func IsNormalStateWithNoReason(s *State) bool { - return s.State == eval.Normal && s.StateReason == "" -} - // StateTransition describes the transition from one state to another. type StateTransition struct { *State diff --git a/pkg/services/ngalert/store/instance_database.go b/pkg/services/ngalert/store/instance_database.go index 4633920edb3..8d16ae9711d 100644 --- a/pkg/services/ngalert/store/instance_database.go +++ b/pkg/services/ngalert/store/instance_database.go @@ -10,15 +10,13 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/sqlstore" ) type InstanceDBStore struct { - SQLStore db.DB - Logger log.Logger - FeatureToggles featuremgmt.FeatureToggles + SQLStore db.DB + Logger log.Logger } // ListAlertInstances is a handler for retrieving alert instances within specific organisation @@ -44,9 +42,6 @@ func (st InstanceDBStore) ListAlertInstances(ctx context.Context, cmd *models.Li return errors.New("filtering by RuleGroup is not supported") } - if st.FeatureToggles.IsEnabled(ctx, featuremgmt.FlagAlertingNoNormalState) { - s.WriteString(fmt.Sprintf(" AND NOT (current_state = '%s' AND current_reason = '')", models.InstanceStateNormal)) - } if err := sess.SQL(s.String(), params...).Find(&alertInstances); err != nil { return err } diff --git a/pkg/services/ngalert/store/instance_database_test.go b/pkg/services/ngalert/store/instance_database_test.go index cb29fa394e7..4b8fbe0f906 100644 --- a/pkg/services/ngalert/store/instance_database_test.go +++ b/pkg/services/ngalert/store/instance_database_test.go @@ -96,69 +96,6 @@ func TestIntegration_CompressedAlertRuleStateOperations(t *testing.T) { } } -func TestIntegration_CompressedAlertRuleStateOperations_NoNormalState(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test") - } - - ctx := context.Background() - ng, dbstore := tests.SetupTestEnv( - t, - baseIntervalSeconds, - tests.WithFeatureToggles( - featuremgmt.WithFeatures( - featuremgmt.FlagAlertingSaveStateCompressed, - featuremgmt.FlagAlertingNoNormalState, - ), - ), - ) - - const mainOrgID int64 = 1 - - alertRule1 := tests.CreateTestAlertRule(t, ctx, dbstore, 60, mainOrgID) - orgID := alertRule1.OrgID - - tests := []struct { - name string - setupInstances func() []models.AlertInstance - listQuery *models.ListAlertInstancesQuery - validate func(t *testing.T, alerts []*models.AlertInstance) - }{ - { - name: "should ignore Normal state with no reason if feature flag is enabled", - setupInstances: func() []models.AlertInstance { - return []models.AlertInstance{ - createAlertInstance(orgID, util.GenerateShortUID(), util.GenerateShortUID(), "", models.InstanceStateNormal), - createAlertInstance(orgID, util.GenerateShortUID(), "errorHash", "error", models.InstanceStateNormal), - } - }, - listQuery: &models.ListAlertInstancesQuery{ - RuleOrgID: orgID, - }, - validate: func(t *testing.T, alerts []*models.AlertInstance) { - require.Len(t, alerts, 1) - containsHash(t, alerts, "errorHash") - for _, instance := range alerts { - if instance.CurrentState == models.InstanceStateNormal && instance.CurrentReason == "" { - require.Fail(t, "List operation expected to return all states except Normal but the result contains Normal states") - } - } - }, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - instances := tc.setupInstances() - err := ng.InstanceStore.SaveAlertInstancesForRule(ctx, alertRule1.GetKeyWithGroup(), instances) - require.NoError(t, err) - alerts, err := ng.InstanceStore.ListAlertInstances(ctx, tc.listQuery) - require.NoError(t, err) - tc.validate(t, alerts) - }) - } -} - // containsHash is a helper function to check if an instance with // a given labels hash exists in the list of alert instances. func containsHash(t *testing.T, instances []*models.AlertInstance, hash string) { @@ -318,57 +255,6 @@ func TestIntegrationAlertInstanceOperations(t *testing.T) { require.Len(t, alerts, 4) }) - t.Run("should ignore Normal state with no reason if feature flag is enabled", func(t *testing.T) { - ng, _ := tests.SetupTestEnv( - t, - baseIntervalSeconds, - tests.WithFeatureToggles( - featuremgmt.WithFeatures(featuremgmt.FlagAlertingNoNormalState), - ), - ) - - labels := models.InstanceLabels{"test": util.GenerateShortUID()} - instance1 := models.AlertInstance{ - AlertInstanceKey: models.AlertInstanceKey{ - RuleOrgID: orgID, - RuleUID: util.GenerateShortUID(), - LabelsHash: util.GenerateShortUID(), - }, - CurrentState: models.InstanceStateNormal, - CurrentReason: "", - Labels: labels, - } - instance2 := models.AlertInstance{ - AlertInstanceKey: models.AlertInstanceKey{ - RuleOrgID: orgID, - RuleUID: util.GenerateShortUID(), - LabelsHash: util.GenerateShortUID(), - }, - CurrentState: models.InstanceStateNormal, - CurrentReason: models.StateReasonError, - Labels: labels, - } - err := ng.InstanceStore.SaveAlertInstance(ctx, instance1) - require.NoError(t, err) - err = ng.InstanceStore.SaveAlertInstance(ctx, instance2) - require.NoError(t, err) - - listQuery := &models.ListAlertInstancesQuery{ - RuleOrgID: orgID, - } - - alerts, err := ng.InstanceStore.ListAlertInstances(ctx, listQuery) - require.NoError(t, err) - - containsHash(t, alerts, instance2.LabelsHash) - - for _, instance := range alerts { - if instance.CurrentState == models.InstanceStateNormal && instance.CurrentReason == "" { - require.Fail(t, "List operation expected to return all states except Normal but the result contains Normal states") - } - } - }) - t.Run("update instance with same org_id, uid and different state", func(t *testing.T) { labels := models.InstanceLabels{"test": "testValue"} _, hash, _ := labels.StringAndHash() diff --git a/pkg/services/ngalert/store/proto_instance_database.go b/pkg/services/ngalert/store/proto_instance_database.go index 4e8c8c60520..ae45d3b34d8 100644 --- a/pkg/services/ngalert/store/proto_instance_database.go +++ b/pkg/services/ngalert/store/proto_instance_database.go @@ -68,15 +68,10 @@ func (st ProtoInstanceDBStore) ListAlertInstances(ctx context.Context, cmd *mode // Convert proto instances to model instances for _, protoInstance := range instances { modelInstance := alertInstanceProtoToModel(row.RuleUID, row.OrgID, protoInstance) - if modelInstance != nil { - // If FlagAlertingNoNormalState is enabled, we should not return instances with normal state and no reason. - if st.FeatureToggles.IsEnabled(ctx, featuremgmt.FlagAlertingNoNormalState) { - if modelInstance.CurrentState == models.InstanceStateNormal && modelInstance.CurrentReason == "" { - continue - } - } - alertInstances = append(alertInstances, modelInstance) + if modelInstance == nil { + continue } + alertInstances = append(alertInstances, modelInstance) } }