Skip to content

Commit

Permalink
Alerting: Add database migration for recording rule fields (#87012)
Browse files Browse the repository at this point in the history
* Create recording rule fields in model

* Add migration

* Write to database, support in version table

* extend fingerprint

* Force fields to be empty on validate

* Another storage spot, tests for fingerprint

* Explicitly set defaults in provisioning API

* Tests for main API validation

* Add diff tests even though fields are unpopulated for now

* Use struct tag approach instead of FromDB/ToDB hooks as it better handles nulls when deserializing

* test for deser

* Backout RecordTo for now since it's not decided in the doc

* back out of migration too

* Drop datasourceref for now

* address linter complaints

* Try a single outer struct with all fields embedded
  • Loading branch information
alexweav committed May 9, 2024
1 parent 9bd44ae commit 36ef611
Show file tree
Hide file tree
Showing 11 changed files with 82 additions and 1 deletion.
3 changes: 3 additions & 0 deletions pkg/services/ngalert/api/api_ruler_validation.go
Expand Up @@ -108,6 +108,9 @@ func validateRuleNode(
RuleGroup: groupName,
NoDataState: noDataState,
ExecErrState: errorState,
// Recording Rule fields will be implemented in the future.
// For now, no rules can be recording rules. So, we force these to be empty.
Record: nil,
}

if ruleNode.GrafanaManagedAlert.NotificationSettings != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/services/ngalert/api/api_ruler_validation_test.go
Expand Up @@ -339,6 +339,7 @@ func TestValidateRuleNode_NoUID(t *testing.T) {
require.Equal(t, time.Duration(*api.ApiRuleNode.For), alert.For)
require.Equal(t, api.ApiRuleNode.Annotations, alert.Annotations)
require.Equal(t, api.ApiRuleNode.Labels, alert.Labels)
require.Nil(t, alert.Record)
},
},
{
Expand Down
3 changes: 3 additions & 0 deletions pkg/services/ngalert/api/compat.go
Expand Up @@ -31,6 +31,9 @@ func AlertRuleFromProvisionedAlertRule(a definitions.ProvisionedAlertRule) (mode
Labels: a.Labels,
IsPaused: a.IsPaused,
NotificationSettings: NotificationSettingsFromAlertRuleNotificationSettings(a.NotificationSettings),
// Recording Rule fields will be implemented in the future.
// For now, no rules can be recording rules. So, we force these to be empty.
Record: nil,
}, nil
}

Expand Down
34 changes: 33 additions & 1 deletion pkg/services/ngalert/models/alert_rule.go
Expand Up @@ -5,14 +5,17 @@ import (
"encoding/json"
"errors"
"fmt"
"hash/fnv"
"sort"
"strconv"
"strings"
"time"
"unsafe"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
alertingModels "github.com/grafana/alerting/models"
"github.com/grafana/grafana-plugin-sdk-go/data"

"github.com/grafana/grafana/pkg/services/quota"
"github.com/grafana/grafana/pkg/setting"
Expand Down Expand Up @@ -239,7 +242,8 @@ type AlertRule struct {
DashboardUID *string `xorm:"dashboard_uid"`
PanelID *int64 `xorm:"panel_id"`
RuleGroup string
RuleGroupIndex int `xorm:"rule_group_idx"`
RuleGroupIndex int `xorm:"rule_group_idx"`
Record *Record `xorm:"text null 'record'"`
NoDataState NoDataState
ExecErrState ExecutionErrorState
// ideally this field should have been apimodels.ApiDuration
Expand Down Expand Up @@ -514,6 +518,10 @@ func (alertRule *AlertRule) ValidateAlertRule(cfg setting.UnifiedAlertingSetting
}
}

if alertRule.Record != nil {
return fmt.Errorf("%w: storing recording rules is not yet allowed", ErrAlertRuleFailedValidation)
}

if len(alertRule.NotificationSettings) > 0 {
if len(alertRule.NotificationSettings) != 1 {
return fmt.Errorf("%w: only one notification settings entry is allowed", ErrAlertRuleFailedValidation)
Expand Down Expand Up @@ -561,6 +569,7 @@ type AlertRuleVersion struct {
Condition string
Data []AlertQuery
IntervalSeconds int64
Record *Record `xorm:"text null 'record'"`
NoDataState NoDataState
ExecErrState ExecutionErrorState
// ideally this field should have been apimodels.ApiDuration
Expand Down Expand Up @@ -757,3 +766,26 @@ func GroupByAlertRuleGroupKey(rules []*AlertRule) map[AlertRuleGroupKey]RulesGro
}
return result
}

// Record contains mapping information for Recording Rules.
type Record struct {
// Metric indicates a metric name to send results to.
Metric string
// From contains a query RefID, indicating which expression node is the output of the recording rule.
From string
}

func (r *Record) Fingerprint() data.Fingerprint {
h := fnv.New64()

writeString := func(s string) {
// save on extra slice allocation when string is converted to bytes.
_, _ = h.Write(unsafe.Slice(unsafe.StringData(s), len(s))) //nolint:gosec
// ignore errors returned by Write method because fnv never returns them.
_, _ = h.Write([]byte{255}) // use an invalid utf-8 sequence as separator
}

writeString(r.Metric)
writeString(r.From)
return data.Fingerprint(h.Sum64())
}
7 changes: 7 additions & 0 deletions pkg/services/ngalert/models/alert_rule_test.go
Expand Up @@ -505,6 +505,13 @@ func TestDiff(t *testing.T) {
assert.Equal(t, rule2.RuleGroupIndex, diff[0].Right.Interface())
difCnt++
}
if rule1.Record != rule2.Record {
diff := diffs.GetDiffsForField("Record")
assert.Len(t, diff, 1)
assert.Equal(t, rule1.Record, diff[0].Left.String())
assert.Equal(t, rule2.Record, diff[0].Right.String())
difCnt++
}

require.Lenf(t, diffs, difCnt, "Got some unexpected diffs. Either add to ignore or add assert to it")

Expand Down
5 changes: 5 additions & 0 deletions pkg/services/ngalert/schedule/registry.go
Expand Up @@ -311,5 +311,10 @@ func (r ruleWithFolder) Fingerprint() fingerprint {
writeInt(int64(rule.RuleGroupIndex))
writeString(string(rule.NoDataState))
writeString(string(rule.ExecErrState))
if rule.Record != nil {
binary.LittleEndian.PutUint64(tmp, uint64(rule.Record.Fingerprint()))
writeBytes(tmp)
}

return fingerprint(sum.Sum64())
}
2 changes: 2 additions & 0 deletions pkg/services/ngalert/schedule/registry_test.go
Expand Up @@ -192,6 +192,7 @@ func TestRuleWithFolderFingerprint(t *testing.T) {
RuleGroupIndex: 1,
NoDataState: "test-nodata",
ExecErrState: "test-err",
Record: &models.Record{Metric: "my_metric", From: "A"},
For: 12,
Annotations: map[string]string{
"key-annotation": "value-annotation",
Expand Down Expand Up @@ -230,6 +231,7 @@ func TestRuleWithFolderFingerprint(t *testing.T) {
RuleGroupIndex: 22,
NoDataState: "test-nodata2",
ExecErrState: "test-err2",
Record: &models.Record{Metric: "my_metric2", From: "B"},
For: 1141,
Annotations: map[string]string{
"key-annotation2": "value-annotation",
Expand Down
2 changes: 2 additions & 0 deletions pkg/services/ngalert/store/alert_rule.go
Expand Up @@ -161,6 +161,7 @@ func (st DBstore) InsertAlertRules(ctx context.Context, rules []ngmodels.AlertRu
For: r.For,
Annotations: r.Annotations,
Labels: r.Labels,
Record: r.Record,
NotificationSettings: r.NotificationSettings,
})
}
Expand Down Expand Up @@ -238,6 +239,7 @@ func (st DBstore) UpdateAlertRules(ctx context.Context, rules []ngmodels.UpdateR
IntervalSeconds: r.New.IntervalSeconds,
NoDataState: r.New.NoDataState,
ExecErrState: r.New.ExecErrState,
Record: r.New.Record,
For: r.New.For,
Annotations: r.New.Annotations,
Labels: r.New.Labels,
Expand Down
6 changes: 6 additions & 0 deletions pkg/services/ngalert/store/alert_rule_test.go
Expand Up @@ -643,6 +643,12 @@ func TestIntegrationInsertAlertRules(t *testing.T) {
require.Truef(t, found, "Rule with key %#v was not found in database", keyWithID)
}

t.Run("inserted alerting rules should have nil recording rule fields on model", func(t *testing.T) {
for _, rule := range dbRules {
require.Nil(t, rule.Record)
}
})

t.Run("fail to insert rules with same ID", func(t *testing.T) {
_, err = store.InsertAlertRules(context.Background(), []models.AlertRule{deref[0]})
require.ErrorIs(t, err, models.ErrAlertRuleConflictBase)
Expand Down
2 changes: 2 additions & 0 deletions pkg/services/sqlstore/migrations/migrations.go
Expand Up @@ -121,6 +121,8 @@ func (oss *OSSMigrations) AddMigration(mg *Migrator) {
accesscontrol.AddAlertingScopeRemovalMigration(mg)

accesscontrol.AddManagedFolderAlertingSilencesActionsMigrator(mg)

ualert.AddRecordingRuleColumns(mg)
}

func addStarMigrations(mg *Migrator) {
Expand Down
18 changes: 18 additions & 0 deletions pkg/services/sqlstore/migrations/ualert/recording_rules_mig.go
@@ -0,0 +1,18 @@
package ualert

import "github.com/grafana/grafana/pkg/services/sqlstore/migrator"

// AddRecordingRuleColumns adds columns to alert_rule to represent recording rules.
func AddRecordingRuleColumns(mg *migrator.Migrator) {
mg.AddMigration("add record column to alert_rule table", migrator.NewAddColumnMigration(migrator.Table{Name: "alert_rule"}, &migrator.Column{
Name: "record",
Type: migrator.DB_Text, // Text, to allow for future growth, as this contains a JSON-ified struct.
Nullable: true,
}))

mg.AddMigration("add record column to alert_rule_version table", migrator.NewAddColumnMigration(migrator.Table{Name: "alert_rule_version"}, &migrator.Column{
Name: "record",
Type: migrator.DB_Text,
Nullable: true,
}))
}

0 comments on commit 36ef611

Please sign in to comment.