DEV-46410 Do not send a notification on error or no data state

pkg/services/ngalert/api/api_ruler_validation.go

@@ -54,18 +54,20 @@ func validateRuleNode(
 		}
 	}
 
-	errorState := ngmodels.AlertingErrState
-
-	if ruleNode.GrafanaManagedAlert.ExecErrState == "" && canPatch {
-		errorState = ""
-	}
-
-	if ruleNode.GrafanaManagedAlert.ExecErrState != "" {
-		errorState, err = ngmodels.ErrStateFromString(string(ruleNode.GrafanaManagedAlert.ExecErrState))
-		if err != nil {
-			return nil, err
-		}
-	}
+	// LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
+	errorState := ngmodels.OkErrState
+
+	//if ruleNode.GrafanaManagedAlert.ExecErrState == "" && canPatch {
+	//	errorState = ""
+	//}
+	//
+	//if ruleNode.GrafanaManagedAlert.ExecErrState != "" {
+	//	if err != nil {
+	//		return nil, err
+	//	}
+	//	errorState, err = ngmodels.ErrStateFromString(string(ruleNode.GrafanaManagedAlert.ExecErrState))
+	//}
+	// LOGZ.IO GRAFANA CHANGE :: End
 
 	if len(ruleNode.GrafanaManagedAlert.Data) == 0 {
 		if canPatch {

pkg/services/ngalert/migration/migration_test.go

@@ -1235,7 +1235,7 @@ func TestDashAlertQueryMigration(t *testing.T) {
 						return a.ID < b.ID
 					}),
 					cmpopts.IgnoreUnexported(ngModels.AlertRule{}, ngModels.AlertQuery{}),
-					cmpopts.IgnoreFields(ngModels.AlertRule{}, "Updated", "UID", "ID"),
+					cmpopts.IgnoreFields(ngModels.AlertRule{}, "Updated", "UID", "ID", "ExecErrState"), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 				}
 				if tt.expectedFolder != nil {
 					cOpt = append(cOpt, cmpopts.IgnoreFields(ngModels.AlertRule{}, "NamespaceUID"))

pkg/services/ngalert/migration/permissions_test.go

@@ -744,7 +744,7 @@ func TestDashAlertPermissionMigration(t *testing.T) {
 							return a.Permission < b.Permission
 						}),
 						cmpopts.IgnoreUnexported(ngModels.AlertRule{}, ngModels.AlertQuery{}),
-						cmpopts.IgnoreFields(ngModels.AlertRule{}, "ID", "Updated", "UID"),
+						cmpopts.IgnoreFields(ngModels.AlertRule{}, "ID", "Updated", "UID", "ExecErrState"), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 						cmpopts.IgnoreFields(dashboards.Dashboard{}, "ID", "Created", "Updated", "Data", "Slug"),
 					}
 					if !cmp.Equal(tt.expected, actual, cOpt...) {

pkg/services/ngalert/migration/service_test.go

@@ -1617,7 +1617,7 @@ func compareRules(t *testing.T, x *xorm.Engine, orgId int64, expectedRules []*mo
 			return a.Title < b.Title
 		}),
 		cmpopts.IgnoreUnexported(models.AlertRule{}, models.AlertQuery{}),
-		cmpopts.IgnoreFields(models.AlertRule{}, "Updated", "UID", "ID", "Version"),
+		cmpopts.IgnoreFields(models.AlertRule{}, "Updated", "UID", "ID", "Version", "ExecErrState"), // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 	}
 	if !cmp.Equal(expectedRules, rules, cOpt...) {
 		t.Errorf("Unexpected Rule: %v", cmp.Diff(expectedRules, rules, cOpt...))

pkg/services/ngalert/schedule/schedule_unit_test.go

@@ -752,13 +752,20 @@ func TestSchedule_ruleRoutine(t *testing.T) {
 			require.NoError(t, err)
 		})
 
-		t.Run("it should send special alert DatasourceError", func(t *testing.T) {
-			sender.AssertNumberOfCalls(t, "Send", 1)
-			args, ok := sender.Calls()[0].Arguments[2].(definitions.PostableAlerts)
-			require.Truef(t, ok, fmt.Sprintf("expected argument of function was supposed to be 'definitions.PostableAlerts' but got %T", sender.Calls()[0].Arguments[2]))
-			assert.Len(t, args.PostableAlerts, 1)
-			assert.Equal(t, state.ErrorAlertName, args.PostableAlerts[0].Labels[prometheusModel.AlertNameLabel])
+		// LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Do not send a notification on error or no data state
+		//t.Run("it should send special alert DatasourceError", func(t *testing.T) {
+		//	sender.AssertNumberOfCalls(t, "Send", 1)
+		//	args, ok := sender.Calls()[0].Arguments[2].(definitions.PostableAlerts)
+		//	require.Truef(t, ok, fmt.Sprintf("expected argument of function was supposed to be 'definitions.PostableAlerts' but got %T", sender.Calls()[0].Arguments[2]))
+		//	assert.Len(t, args.PostableAlerts, 1)
+		//	assert.Equal(t, state.ErrorAlertName, args.PostableAlerts[0].Labels[prometheusModel.AlertNameLabel])
+		//})
+		t.Run("it should not send special alert DatasourceError", func(t *testing.T) {
+			sender.AssertNotCalled(t, "Send", mock.Anything, mock.Anything)
+
+			require.NotEmpty(t, sch.stateManager.GetStatesForRuleUID(rule.OrgID, rule.UID))
 		})
+		// LOGZ.IO GRAFANA CHANGE :: End
 	})
 
 	t.Run("when there are alerts that should be firing", func(t *testing.T) {

pkg/services/ngalert/state/state.go

@@ -408,6 +408,10 @@ func (a *State) NeedsSending(resendDelay time.Duration) bool {
 	case eval.Normal:
 		// We should send a notification if the state is Normal because it was resolved
 		return a.Resolved
+	// LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Do not send a notification on error or no data state
+	case eval.NoData, eval.Error:
+		return false
+	// LOGZ.IO GRAFANA CHANGE :: end
 	default:
 		// We should send, and re-send notifications, each time LastSentAt is <= LastEvaluationTime + resendDelay
 		nextSent := a.LastSentAt.Add(resendDelay)

pkg/services/ngalert/state/state_test.go

@@ -426,8 +426,10 @@ func TestNeedsSending(t *testing.T) {
 			},
 		},
 		{
-			name:        "state: no-data, needs to be re-sent",
-			expected:    true,
+			// LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Do not send a notification on error or no data state
+			name:     "state: no-data, should not be re-sent with evaluation time ",
+			expected: false,
+			// LOGZ.IO GRAFANA CHANGE :: End
 			resendDelay: 1 * time.Minute,
 			testState: &State{
 				State:              eval.NoData,
@@ -446,8 +448,10 @@ func TestNeedsSending(t *testing.T) {
 			},
 		},
 		{
-			name:        "state: error, needs to be re-sent",
-			expected:    true,
+			// LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Do not send a notification on error or no data state
+			name:     "state: error, should not be re-sent with evaluation time",
+			expected: false,
+			// LOGZ.IO GRAFANA CHANGE :: End
 			resendDelay: 1 * time.Minute,
 			testState: &State{
 				State:              eval.Error,

pkg/services/ngalert/store/alert_rule.go

@@ -137,6 +137,7 @@ func (st DBstore) InsertAlertRules(ctx context.Context, rules []ngmodels.AlertRu
 				r.UID = uid
 			}
 			r.Version = 1
+			r.ExecErrState = ngmodels.OkErrState // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 			if err := st.validateAlertRule(r); err != nil {
 				return err
 			}
@@ -203,6 +204,7 @@ func (st DBstore) UpdateAlertRules(ctx context.Context, rules []ngmodels.UpdateR
 			var parentVersion int64
 			r.New.ID = r.Existing.ID
 			r.New.Version = r.Existing.Version // xorm will take care of increasing it (see https://xorm.io/docs/chapter-06/1.lock/)
+			r.New.ExecErrState = ngmodels.OkErrState // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 			if err := st.validateAlertRule(r.New); err != nil {
 				return err
 			}

pkg/tests/api/alerting/api_alertmanager_test.go

@@ -731,6 +731,7 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) {
 		re = regexp.MustCompile(`"updated":"(\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z)"`)
 		b = re.ReplaceAll(b, []byte(`"updated":"2021-05-19T19:47:55Z"`))
 
+		// LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK
 		expectedGetRulesResponseBody := fmt.Sprintf(`{
 			"default": [
 				{
@@ -776,7 +777,7 @@ func TestIntegrationDeleteFolderWithRules(t *testing.T) {
 								"namespace_uid": %q,
 								"rule_group": "arulegroup",
 								"no_data_state": "NoData",
-								"exec_err_state": "Alerting"
+								"exec_err_state": "OK"
 							}
 						}
 					]
@@ -1207,6 +1208,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
 		// copy result to a variable with a wider scope
 		// to be used by the next test
 		ruleUID = generatedUIDs[0]
+		// LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK
 		expectedGetNamespaceResponseBody = `
 		{
 		   "default":[
@@ -1253,7 +1255,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
 						  "namespace_uid":"nsuid",
 						  "rule_group":"arulegroup",
 						  "no_data_state":"NoData",
-						  "exec_err_state":"Alerting"
+						  "exec_err_state":"OK"
 					   }
 					},
 					{
@@ -1289,7 +1291,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
 						  "namespace_uid":"nsuid",
 						  "rule_group":"arulegroup",
 						  "no_data_state":"Alerting",
-						  "exec_err_state":"Alerting"
+						  "exec_err_state":"OK"
 					   }
 					}
 				 ]
@@ -1549,6 +1551,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
 		assert.True(t, ok)
 		assert.Equal(t, 1, len(returnedUIDs))
 		assert.Equal(t, ruleUID, returnedUIDs[0])
+		// LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK
 		assert.JSONEq(t, `
 		{
 		   "default":[
@@ -1597,7 +1600,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
 		                  "namespace_uid":"nsuid",
 		                  "rule_group":"arulegroup",
 		                  "no_data_state":"Alerting",
-		                  "exec_err_state":"Alerting"
+		                  "exec_err_state":"OK"
 		               }
 		            }
 		         ]
@@ -1666,6 +1669,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
 		assert.True(t, ok)
 		assert.Equal(t, 1, len(returnedUIDs))
 		assert.Equal(t, ruleUID, returnedUIDs[0])
+		// LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK
 		assert.JSONEq(t, `
 			{
 			   "default":[
@@ -1706,7 +1710,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
 					  "namespace_uid":"nsuid",
 					  "rule_group":"arulegroup",
 					  "no_data_state":"Alerting",
-					  "exec_err_state":"Alerting"
+					  "exec_err_state":"OK"
 				       }
 				    }
 				 ]
@@ -1754,6 +1758,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
 		assert.True(t, ok)
 		assert.Equal(t, 1, len(returnedUIDs))
 		assert.Equal(t, ruleUID, returnedUIDs[0])
+		// LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK
 		assert.JSONEq(t, `
 			{
 			   "default":[
@@ -1794,7 +1799,7 @@ func TestIntegrationAlertRuleCRUD(t *testing.T) {
 					  "namespace_uid":"nsuid",
 					  "rule_group":"arulegroup",
 					  "no_data_state":"Alerting",
-					  "exec_err_state":"Alerting"
+					  "exec_err_state":"OK"
 				       }
 				    }
 				 ]
@@ -2209,6 +2214,7 @@ func TestIntegrationQuota(t *testing.T) {
 		assert.True(t, ok)
 		assert.Equal(t, 1, len(returnedUIDs))
 		assert.Equal(t, ruleUID, returnedUIDs[0])
+		// LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK
 		assert.JSONEq(t, `
 				{
 				   "default":[
@@ -2249,7 +2255,7 @@ func TestIntegrationQuota(t *testing.T) {
 						  "namespace_uid":"nsuid",
 						  "rule_group":"arulegroup",
 						  "no_data_state":"NoData",
-						  "exec_err_state":"Alerting"
+						  "exec_err_state":"OK"
 					       }
 					    }
 					 ]

pkg/tests/api/alerting/api_ruler_test.go

@@ -123,6 +123,7 @@ func TestIntegrationAlertRulePermissions(t *testing.T) {
 				"GrafanaManagedAlert.Data.Model",
 				"GrafanaManagedAlert.NamespaceUID",
 				"GrafanaManagedAlert.NamespaceID",
+				"GrafanaManagedAlert.ExecErrState", // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 			}
 
 			// compare expected and actual and ignore the dynamic fields
@@ -139,9 +140,11 @@ func TestIntegrationAlertRulePermissions(t *testing.T) {
 
 			for _, rule := range allRules["folder1"][0].Rules {
 				assert.Equal(t, "folder1", rule.GrafanaManagedAlert.NamespaceUID)
+				require.Equal(t, rule.GrafanaManagedAlert.ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 			}
 			for _, rule := range allRules["folder2"][0].Rules {
 				assert.Equal(t, "folder2", rule.GrafanaManagedAlert.NamespaceUID)
+				require.Equal(t, rule.GrafanaManagedAlert.ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 			}
 		})
 
@@ -173,6 +176,7 @@ func TestIntegrationAlertRulePermissions(t *testing.T) {
 			pathsToIgnore := []string{
 				"Groups.Rules.UID",
 				"Groups.Folder",
+				"Groups.Rules.ExecErrState", // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 			}
 
 			// compare expected and actual and ignore the dynamic fields
@@ -189,6 +193,8 @@ func TestIntegrationAlertRulePermissions(t *testing.T) {
 
 			require.Equal(t, "folder1", allExport.Groups[0].Folder)
 			require.Equal(t, "folder2", allExport.Groups[1].Folder)
+			require.Equal(t, allExport.Groups[0].Rules[0].ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
+			require.Equal(t, allExport.Groups[1].Rules[0].ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 		})
 
 		t.Run("Export from one folder", func(t *testing.T) {
@@ -424,6 +430,7 @@ func TestIntegrationAlertRuleNestedPermissions(t *testing.T) {
 				"GrafanaManagedAlert.Data.Model",
 				"GrafanaManagedAlert.NamespaceUID",
 				"GrafanaManagedAlert.NamespaceID",
+				"GrafanaManagedAlert.ExecErrState", // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 			}
 
 			// compare expected and actual and ignore the dynamic fields
@@ -440,14 +447,17 @@ func TestIntegrationAlertRuleNestedPermissions(t *testing.T) {
 
 			for _, rule := range allRules["folder1"][0].Rules {
 				assert.Equal(t, "folder1", rule.GrafanaManagedAlert.NamespaceUID)
+				require.Equal(t, rule.GrafanaManagedAlert.ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 			}
 
 			for _, rule := range allRules["folder2"][0].Rules {
 				assert.Equal(t, "folder2", rule.GrafanaManagedAlert.NamespaceUID)
+				require.Equal(t, rule.GrafanaManagedAlert.ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 			}
 
 			for _, rule := range allRules["folder1/subfolder"][0].Rules {
 				assert.Equal(t, "subfolder", rule.GrafanaManagedAlert.NamespaceUID)
+				require.Equal(t, rule.GrafanaManagedAlert.ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 			}
 		})
 
@@ -493,6 +503,7 @@ func TestIntegrationAlertRuleNestedPermissions(t *testing.T) {
 			pathsToIgnore := []string{
 				"Groups.Rules.UID",
 				"Groups.Folder",
+				"Groups.Rules.ExecErrState", // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 			}
 
 			// compare expected and actual and ignore the dynamic fields
@@ -510,6 +521,9 @@ func TestIntegrationAlertRuleNestedPermissions(t *testing.T) {
 			require.Equal(t, "folder1", allExport.Groups[0].Folder)
 			require.Equal(t, "folder2", allExport.Groups[1].Folder)
 			require.Equal(t, "subfolder", allExport.Groups[2].Folder)
+			require.Equal(t, allExport.Groups[0].Rules[0].ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
+			require.Equal(t, allExport.Groups[1].Rules[0].ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
+			require.Equal(t, allExport.Groups[2].Rules[0].ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 		})
 
 		t.Run("Export from one folder", func(t *testing.T) {
@@ -769,6 +783,7 @@ func TestAlertRulePostExport(t *testing.T) {
 			"Groups.Rules.UID",
 			"Groups.Folder",
 			"Data.Model", // Model is not amended with default values
+			"Groups.Rules.ExecErrState", // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 		}
 
 		// compare expected and actual and ignore the dynamic fields
@@ -784,6 +799,7 @@ func TestAlertRulePostExport(t *testing.T) {
 		require.Empty(t, diff)
 
 		require.Equal(t, actual.Groups[0].Folder, "folder1")
+		require.Equal(t, actual.Groups[0].Rules[0].ExecErrState, apimodels.OkErrState) // LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK and enforce OK value
 	})
 
 	t.Run("should return 403 when no access to folder", func(t *testing.T) {
@@ -981,6 +997,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) {
 		require.Len(t, resp.Created, len(rules.Rules))
 	}
 
+	// LOGZ.IO GRAFANA CHANGE :: DEV-46410 - Change default ExecErrState to OK
 	expectedAllJSON := fmt.Sprintf(`
 {
 	"default": [{
@@ -1021,7 +1038,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) {
 				"namespace_uid": "nsuid",
 				"rule_group": "anotherrulegroup",
 				"no_data_state": "NoData",
-				"exec_err_state": "Alerting"
+				"exec_err_state": "OK"
 			}
 		}, {
 			"expr": "",
@@ -1054,7 +1071,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) {
 				"namespace_uid": "nsuid",
 				"rule_group": "anotherrulegroup",
 				"no_data_state": "Alerting",
-				"exec_err_state": "Alerting"
+				"exec_err_state": "OK"
 			}
 		}]
 	}]
@@ -1099,7 +1116,7 @@ func TestIntegrationRulerRulesFilterByDashboard(t *testing.T) {
 				"namespace_uid": "nsuid",
 				"rule_group": "anotherrulegroup",
 				"no_data_state": "NoData",
-				"exec_err_state": "Alerting"
+				"exec_err_state": "OK"
 			}
 		}]
 	}]