Alerting: Add message to alert_rule_version table (Part 1). (#114194)
This adds a `message` column to the `alert_rule_version` table. This follows the pattern established for dashboards as closely as possible. A new type is introduced internally for passing the new `message` field around in a type-safe manner, but doing the same for the API types becomes very messy. In that case, a new field is added with omitempty. Note this PR is only: - The column addition - The "read" path; API for listing versions Subsequent PRs will add code to actually set the message when updating rules.
This commit is contained in:
@@ -385,14 +385,24 @@ func (srv RulerSrv) RouteGetRuleVersionsByUID(c *contextmodel.ReqContext, ruleUI
|
||||
}
|
||||
sort.Slice(rules, func(i, j int) bool { return rules[i].ID > rules[j].ID })
|
||||
result := make(apimodels.GettableRuleVersions, 0, len(rules))
|
||||
userUIDmapping := srv.getUserUIDmapping(ctx, rules)
|
||||
userUIDmapping := srv.getUserUIDmapping(ctx, alertRuleVersionsToAlertRules(rules))
|
||||
for _, rule := range rules {
|
||||
// do not provide provenance status because we do not have historical changes for it
|
||||
result = append(result, toGettableExtendedRuleNode(*rule, map[string]ngmodels.Provenance{}, userUIDmapping))
|
||||
ruleNode := toGettableExtendedRuleNode(rule.AlertRule, map[string]ngmodels.Provenance{}, userUIDmapping)
|
||||
ruleNode.GrafanaManagedAlert.Message = rule.Message
|
||||
result = append(result, ruleNode)
|
||||
}
|
||||
return response.JSON(http.StatusOK, result)
|
||||
}
|
||||
|
||||
func alertRuleVersionsToAlertRules(vs []*ngmodels.AlertRuleVersion) []*ngmodels.AlertRule {
|
||||
result := make([]*ngmodels.AlertRule, len(vs))
|
||||
for i := range vs {
|
||||
result[i] = &vs[i].AlertRule
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
func (srv RulerSrv) RoutePostNameRulesConfig(c *contextmodel.ReqContext, ruleGroupConfig apimodels.PostableRuleGroupConfig, namespaceUID string) response.Response {
|
||||
var deletePermanently bool
|
||||
if c.QueryBool("deletePermanently") {
|
||||
|
||||
@@ -560,11 +560,17 @@ func TestRouteGetRuleVersionsByUID(t *testing.T) {
|
||||
ruleStore.Folders[orgID] = append(ruleStore.Folders[orgID], f)
|
||||
|
||||
rule := gen.GenerateRef()
|
||||
history := gen.With(gen.WithUID(rule.UID)).GenerateManyRef(3)
|
||||
historyRules := gen.With(gen.WithUID(rule.UID)).GenerateManyRef(3)
|
||||
history := make([]*models.AlertRuleVersion, len(historyRules))
|
||||
// simulate order of the history
|
||||
rule.ID = 100
|
||||
for i, alertRule := range history {
|
||||
for i, alertRule := range historyRules {
|
||||
alertRule.ID = rule.ID - int64(i) - 1
|
||||
|
||||
history[i] = &models.AlertRuleVersion{
|
||||
AlertRule: *alertRule,
|
||||
Message: fmt.Sprintf("revision %d", i),
|
||||
}
|
||||
}
|
||||
|
||||
ruleStore.PutRule(context.Background(), rule)
|
||||
@@ -584,11 +590,22 @@ func TestRouteGetRuleVersionsByUID(t *testing.T) {
|
||||
require.Len(t, result, len(history)+1) // history + current version
|
||||
|
||||
t.Run("should be in correct order", func(t *testing.T) {
|
||||
expectedHistory := append([]*models.AlertRule{rule}, history...)
|
||||
expectedHistory := append([]*models.AlertRuleVersion{{AlertRule: *rule}}, history...)
|
||||
for i, rul := range expectedHistory {
|
||||
assert.Equal(t, rul.UID, result[i].GrafanaManagedAlert.UID)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("should have correct messages", func(t *testing.T) {
|
||||
expectedMessages := make([]string, 0, len(history)+1)
|
||||
expectedMessages = append(expectedMessages, "")
|
||||
for i := range history {
|
||||
expectedMessages = append(expectedMessages, history[i].Message)
|
||||
}
|
||||
for i := range expectedMessages {
|
||||
assert.Equal(t, expectedMessages[i], result[i].GrafanaManagedAlert.Message)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
t.Run("NotFound when rule does not exist", func(t *testing.T) {
|
||||
@@ -599,10 +616,17 @@ func TestRouteGetRuleVersionsByUID(t *testing.T) {
|
||||
UID: "test",
|
||||
}
|
||||
guid := uuid.NewString()
|
||||
history := gen.With(gen.WithGUID(guid), gen.WithKey(ruleKey)).GenerateManyRef(3)
|
||||
historyRules := gen.With(gen.WithGUID(guid), gen.WithKey(ruleKey)).GenerateManyRef(3)
|
||||
history := make([]*models.AlertRuleVersion, len(historyRules))
|
||||
for i, alertRule := range historyRules {
|
||||
history[i] = &models.AlertRuleVersion{
|
||||
AlertRule: *alertRule,
|
||||
Message: fmt.Sprintf("revision %d", i),
|
||||
}
|
||||
}
|
||||
ruleStore.History[guid] = append(ruleStore.History[guid], history...) // even if history is full of records
|
||||
|
||||
perms := createPermissionsForRules(history, orgID)
|
||||
perms := createPermissionsForRules(historyRules, orgID)
|
||||
req := createRequestContextWithPerms(orgID, perms, nil)
|
||||
response := createService(ruleStore, nil).RouteGetRuleVersionsByUID(req, ruleKey.UID)
|
||||
|
||||
@@ -643,10 +667,17 @@ func TestRouteGetRuleVersionsByUID(t *testing.T) {
|
||||
guid := uuid.NewString()
|
||||
rule := gen.With(gen.WithGUID(guid), gen.WithKey(ruleKey), gen.WithNamespaceUID(anotherFolder.UID)).GenerateRef()
|
||||
ruleStore.PutRule(context.Background(), rule)
|
||||
history := gen.With(gen.WithGUID(guid), gen.WithKey(ruleKey)).GenerateManyRef(3)
|
||||
historyRules := gen.With(gen.WithGUID(guid), gen.WithKey(ruleKey)).GenerateManyRef(3)
|
||||
history := make([]*models.AlertRuleVersion, len(historyRules))
|
||||
for i, alertRule := range historyRules {
|
||||
history[i] = &models.AlertRuleVersion{
|
||||
AlertRule: *alertRule,
|
||||
Message: fmt.Sprintf("revision %d", i),
|
||||
}
|
||||
}
|
||||
ruleStore.History[guid] = history
|
||||
|
||||
perms := createPermissionsForRules(history, orgID) // grant permissions to all records in history but not the rule itself
|
||||
perms := createPermissionsForRules(historyRules, orgID) // grant permissions to all records in history but not the rule itself
|
||||
req := createRequestContextWithPerms(orgID, perms, nil)
|
||||
response := createService(ruleStore, nil).RouteGetRuleVersionsByUID(req, ruleKey.UID)
|
||||
|
||||
|
||||
@@ -35,6 +35,6 @@ type RuleStore interface {
|
||||
|
||||
// IncreaseVersionForAllRulesInNamespaces Increases version for all rules that have specified namespace uids
|
||||
IncreaseVersionForAllRulesInNamespaces(ctx context.Context, orgID int64, namespaceUIDs []string) ([]ngmodels.AlertRuleKeyWithVersion, error)
|
||||
GetAlertRuleVersions(ctx context.Context, orgID int64, guid string) ([]*ngmodels.AlertRule, error)
|
||||
GetAlertRuleVersions(ctx context.Context, orgID int64, guid string) ([]*ngmodels.AlertRuleVersion, error)
|
||||
accesscontrol.RuleUIDToNamespaceStore
|
||||
}
|
||||
|
||||
@@ -617,6 +617,9 @@ type GettableGrafanaRule struct {
|
||||
Metadata *AlertRuleMetadata `json:"metadata,omitempty" yaml:"metadata,omitempty"`
|
||||
GUID string `json:"guid" yaml:"guid"`
|
||||
MissingSeriesEvalsToResolve *int64 `json:"missing_series_evals_to_resolve,omitempty" yaml:"missing_series_evals_to_resolve,omitempty"`
|
||||
|
||||
// Field is only populated when listing alert rule versions.
|
||||
Message string `yaml:"message,omitempty" json:"message,omitempty"`
|
||||
}
|
||||
|
||||
// UserInfo represents user-related information, including a unique identifier and a name.
|
||||
|
||||
@@ -369,6 +369,13 @@ type AlertRule struct {
|
||||
MissingSeriesEvalsToResolve *int64
|
||||
}
|
||||
|
||||
type AlertRuleVersion struct {
|
||||
AlertRule
|
||||
|
||||
// Message is only stored in the alert_rule_version table.
|
||||
Message string
|
||||
}
|
||||
|
||||
type AlertRuleMetadata struct {
|
||||
EditorSettings EditorSettings `json:"editor_settings"`
|
||||
PrometheusStyleRule *PrometheusStyleRule `json:"prometheus_style_rule,omitempty"`
|
||||
|
||||
@@ -192,8 +192,8 @@ func (st DBstore) GetAlertRuleByUID(ctx context.Context, query *ngmodels.GetAler
|
||||
return result, err
|
||||
}
|
||||
|
||||
func (st DBstore) GetAlertRuleVersions(ctx context.Context, orgID int64, guid string) ([]*ngmodels.AlertRule, error) {
|
||||
alertRules := make([]*ngmodels.AlertRule, 0)
|
||||
func (st DBstore) GetAlertRuleVersions(ctx context.Context, orgID int64, guid string) ([]*ngmodels.AlertRuleVersion, error) {
|
||||
alertRules := make([]*ngmodels.AlertRuleVersion, 0)
|
||||
err := st.SQLStore.WithDbSession(ctx, func(sess *db.Session) error {
|
||||
rows, err := sess.Table(new(alertRuleVersion)).Where("rule_org_id = ? AND rule_guid = ?", orgID, guid).Asc("id").Rows(new(alertRuleVersion))
|
||||
if err != nil {
|
||||
@@ -213,7 +213,7 @@ func (st DBstore) GetAlertRuleVersions(ctx context.Context, orgID int64, guid st
|
||||
if previousVersion != nil && previousVersion.EqualSpec(*rule) {
|
||||
continue
|
||||
}
|
||||
converted, err := alertRuleToModelsAlertRule(alertRuleVersionToAlertRule(*rule), st.Logger)
|
||||
converted, err := alertRuleVersionToModelsAlertRuleVersion(*rule, st.Logger)
|
||||
if err != nil {
|
||||
st.Logger.Error("Invalid rule found in DB store, cannot convert, ignoring it", "func", "GetAlertRuleVersions", "error", err, "version_id", rule.ID)
|
||||
continue
|
||||
@@ -226,7 +226,7 @@ func (st DBstore) GetAlertRuleVersions(ctx context.Context, orgID int64, guid st
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
slices.SortFunc(alertRules, func(a, b *ngmodels.AlertRule) int {
|
||||
slices.SortFunc(alertRules, func(a, b *ngmodels.AlertRuleVersion) int {
|
||||
if a.ID > b.ID {
|
||||
return -1
|
||||
}
|
||||
@@ -257,6 +257,7 @@ func (st DBstore) ListDeletedRules(ctx context.Context, orgID int64) ([]*ngmodel
|
||||
st.Logger.Error("Invalid rule version found in DB store, ignoring it", "func", "GetAlertRuleVersions", "error", err)
|
||||
continue
|
||||
}
|
||||
// Note: Message is not returned as a message cannot be set when deleting rules.
|
||||
converted, err := alertRuleToModelsAlertRule(alertRuleVersionToAlertRule(*rule), st.Logger)
|
||||
if err != nil {
|
||||
st.Logger.Error("Invalid rule found in DB store, cannot convert, ignoring it", "func", "GetAlertRuleVersions", "error", err, "version_id", rule.ID)
|
||||
|
||||
@@ -1682,7 +1682,7 @@ func TestIntegrationGetRuleVersions(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, versions, 2)
|
||||
assert.IsDecreasing(t, versions[0].ID, versions[1].ID)
|
||||
diff := versions[1].Diff(versions[0], AlertRuleFieldsToIgnoreInDiff[:]...)
|
||||
diff := versions[1].Diff(&versions[0].AlertRule, AlertRuleFieldsToIgnoreInDiff[:]...)
|
||||
assert.ElementsMatch(t, []string{"Title", "RuleGroupIndex"}, diff.Paths())
|
||||
})
|
||||
|
||||
@@ -1712,7 +1712,7 @@ func TestIntegrationGetRuleVersions(t *testing.T) {
|
||||
versions, err := store.GetAlertRuleVersions(context.Background(), ruleV3.OrgID, ruleV3.GUID)
|
||||
require.NoError(t, err)
|
||||
assert.Len(t, versions, 3)
|
||||
diff := versions[0].Diff(versions[1], AlertRuleFieldsToIgnoreInDiff[:]...)
|
||||
diff := versions[0].Diff(&versions[1].AlertRule, AlertRuleFieldsToIgnoreInDiff[:]...)
|
||||
assert.ElementsMatch(t, []string{"RuleGroup", "NamespaceUID"}, diff.Paths())
|
||||
})
|
||||
}
|
||||
|
||||
@@ -210,6 +210,7 @@ func alertRuleToAlertRuleVersion(rule alertRule) alertRuleVersion {
|
||||
Version: rule.Version,
|
||||
Created: rule.Updated, // assuming the Updated time as the creation time
|
||||
CreatedBy: rule.UpdatedBy,
|
||||
Message: "", // Message is set by caller when creating versions
|
||||
Title: rule.Title,
|
||||
Condition: rule.Condition,
|
||||
Data: rule.Data,
|
||||
@@ -261,3 +262,15 @@ func alertRuleVersionToAlertRule(version alertRuleVersion) alertRule {
|
||||
MissingSeriesEvalsToResolve: version.MissingSeriesEvalsToResolve,
|
||||
}
|
||||
}
|
||||
|
||||
func alertRuleVersionToModelsAlertRuleVersion(version alertRuleVersion, l log.Logger) (models.AlertRuleVersion, error) {
|
||||
result, err := alertRuleToModelsAlertRule(alertRuleVersionToAlertRule(version), l)
|
||||
if err != nil {
|
||||
return models.AlertRuleVersion{}, err
|
||||
}
|
||||
|
||||
return models.AlertRuleVersion{
|
||||
AlertRule: result,
|
||||
Message: version.Message,
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -69,10 +69,11 @@ type alertRuleVersion struct {
|
||||
NotificationSettings string `xorm:"notification_settings"`
|
||||
Metadata string `xorm:"metadata"`
|
||||
MissingSeriesEvalsToResolve *int64 `xorm:"missing_series_evals_to_resolve"`
|
||||
Message string
|
||||
}
|
||||
|
||||
// EqualSpec compares two alertRuleVersion objects for equality based on their specifications and returns true if they match.
|
||||
// The comparison is very basic and can produce false-negative. Fields excluded: ID, ParentVersion, RestoredFrom, Version, Created, RuleGroupIndex and CreatedBy
|
||||
// The comparison is very basic and can produce false-negative. Fields excluded: ID, ParentVersion, RestoredFrom, Version, Created, RuleGroupIndex, CreatedBy and Message
|
||||
func (a alertRuleVersion) EqualSpec(b alertRuleVersion) bool {
|
||||
return a.RuleOrgID == b.RuleOrgID &&
|
||||
a.RuleGUID == b.RuleGUID &&
|
||||
|
||||
@@ -23,7 +23,7 @@ type RuleStore struct {
|
||||
mtx sync.Mutex
|
||||
// OrgID -> RuleGroup -> Namespace -> Rules
|
||||
Rules map[int64][]*models.AlertRule
|
||||
History map[string][]*models.AlertRule
|
||||
History map[string][]*models.AlertRuleVersion
|
||||
Deleted map[int64][]*models.AlertRule
|
||||
Hook func(cmd any) error // use Hook if you need to intercept some query and return an error
|
||||
RecordedOps []any
|
||||
@@ -43,7 +43,7 @@ func NewRuleStore(t *testing.T) *RuleStore {
|
||||
return nil
|
||||
},
|
||||
Folders: map[int64][]*folder.Folder{},
|
||||
History: map[string][]*models.AlertRule{},
|
||||
History: map[string][]*models.AlertRuleVersion{},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -55,7 +55,10 @@ mainloop:
|
||||
for _, r := range rules {
|
||||
rgs := f.Rules[r.OrgID]
|
||||
cp := models.CopyRule(r)
|
||||
f.History[r.GUID] = append(f.History[r.GUID], cp)
|
||||
f.History[r.GUID] = append(f.History[r.GUID], &models.AlertRuleVersion{
|
||||
AlertRule: *cp,
|
||||
Message: "",
|
||||
})
|
||||
for idx, rulePtr := range rgs {
|
||||
if rulePtr.UID == r.UID {
|
||||
rgs[idx] = r
|
||||
@@ -87,6 +90,18 @@ mainloop:
|
||||
}
|
||||
}
|
||||
|
||||
// AppendHistory appends to rules to the version history with the given change message.
|
||||
func (f *RuleStore) AppendHistory(guid string, rules []*models.AlertRule, message string) {
|
||||
versions := make([]*models.AlertRuleVersion, len(rules))
|
||||
for i := range rules {
|
||||
versions[i] = &models.AlertRuleVersion{
|
||||
AlertRule: *rules[i],
|
||||
Message: message,
|
||||
}
|
||||
}
|
||||
f.History[guid] = append(f.History[guid], versions...)
|
||||
}
|
||||
|
||||
// GetRecordedCommands filters recorded commands using predicate function. Returns the subset of the recorded commands that meet the predicate
|
||||
func (f *RuleStore) GetRecordedCommands(predicate func(cmd any) (any, bool)) []any {
|
||||
f.mtx.Lock()
|
||||
@@ -563,7 +578,7 @@ func (f *RuleStore) GetNamespacesByRuleUID(ctx context.Context, orgID int64, uid
|
||||
return namespacesMap, nil
|
||||
}
|
||||
|
||||
func (f *RuleStore) GetAlertRuleVersions(_ context.Context, orgID int64, guid string) ([]*models.AlertRule, error) {
|
||||
func (f *RuleStore) GetAlertRuleVersions(_ context.Context, orgID int64, guid string) ([]*models.AlertRuleVersion, error) {
|
||||
f.mtx.Lock()
|
||||
defer f.mtx.Unlock()
|
||||
|
||||
|
||||
@@ -24,6 +24,9 @@ func AddTablesMigrations(mg *migrator.Migrator) {
|
||||
mg.AddMigration("add last_applied column to alert_configuration_history", migrator.NewAddColumnMigration(migrator.Table{Name: "alert_configuration_history"}, &migrator.Column{
|
||||
Name: "last_applied", Type: migrator.DB_Int, Nullable: false, Default: "0",
|
||||
}))
|
||||
mg.AddMigration("add message column to alert_rule_version", migrator.NewAddColumnMigration(migrator.Table{Name: "alert_rule_version"}, &migrator.Column{
|
||||
Name: "message", Type: migrator.DB_Text, Nullable: true,
|
||||
}))
|
||||
// End of migration log, add new migrations above this line.
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user