Alerting: Remove feature toggle alertingNoNormalState (#99905)

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                                                                                               |

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;

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",

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

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"

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",

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,
@@ -539,7 +538,6 @@ func initInstanceStore(sqlStore db.DB, logger log.Logger, featureToggles feature
 	simpleInstanceStore := store.InstanceDBStore{
 		SQLStore: sqlStore,
 		Logger:   logger,
-		FeatureToggles: featureToggles,
 	}
 
 	if featureToggles.IsEnabledGlobally(featuremgmt.FlagAlertingSaveStateCompressed) {

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

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 {

pkg/services/ngalert/state/persister_async.go

@@ -13,13 +13,11 @@ 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
@@ -31,7 +29,6 @@ func NewAsyncStatePersister(log log.Logger, ticker *clock.Ticker, cfg ManagerCfg
 		log:       log,
 		store:     cfg.InstanceStore,
 		ticker:    ticker,
-		doNotSaveNormalState: cfg.DoNotSaveNormalState,
 		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

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())

pkg/services/ngalert/state/persister_sync_rule.go

@@ -13,14 +13,12 @@ import (
 type SyncRuleStatePersister struct {
 	log   log.Logger
 	store InstanceStore
-	doNotSaveNormalState bool
 }
 
 func NewSyncRuleStatePersisiter(log log.Logger, cfg ManagerCfg) StatePersister {
 	return &SyncRuleStatePersister{
 		log:   log,
 		store: cfg.InstanceStore,
-		doNotSaveNormalState: cfg.DoNotSaveNormalState,
 	}
 }
 
@@ -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)

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

pkg/services/ngalert/store/instance_database.go

@@ -10,7 +10,6 @@ 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"
 )
@@ -18,7 +17,6 @@ import (
 type InstanceDBStore struct {
 	SQLStore db.DB
 	Logger   log.Logger
-	FeatureToggles featuremgmt.FeatureToggles
 }
 
 // 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
 		}

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()

pkg/services/ngalert/store/proto_instance_database.go

@@ -68,17 +68,12 @@ 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 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)
 			}
 		}
-		}
 
 		return nil
 	})