From 169bf2ce73954796f55b729d1a7fff5eb1260659 Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Wed, 1 Oct 2025 21:21:33 +0200 Subject: [PATCH] Alerting: Add feature toggle to use the old simplified routing hash generation (#111900) * Revert "Alerting: Generate simplified routing routes with old fingerprint function (#111893)" This reverts commit 0da9d4989652218d73ea9aa176ed300dba9aa884. * Add alertingUseNewSimplifiedRoutingHashAlgorithm flag * Alerting: Add feature toggle to use the old simplified routing hash generation --- .../src/types/featureToggles.gen.ts | 10 +- pkg/services/featuremgmt/registry.go | 17 +- pkg/services/featuremgmt/toggles_gen.csv | 2 +- pkg/services/featuremgmt/toggles_gen.go | 7 +- pkg/services/featuremgmt/toggles_gen.json | 53 +++- .../ngalert/api/api_alertmanager_test.go | 27 +- pkg/services/ngalert/api/api_testing.go | 2 +- pkg/services/ngalert/models/notifications.go | 51 +--- .../ngalert/models/notifications_test.go | 53 +--- .../ngalert/notifier/autogen_alertmanager.go | 50 ++-- .../notifier/autogen_alertmanager_test.go | 268 ++++-------------- pkg/services/ngalert/schedule/alert_rule.go | 2 +- .../ngalert/schedule/alert_rule_test.go | 2 +- pkg/services/ngalert/schedule/registry.go | 2 +- pkg/services/ngalert/state/state.go | 5 +- pkg/services/ngalert/state/state_test.go | 6 +- 16 files changed, 187 insertions(+), 370 deletions(-) diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index ac2354d8d6d..a47d3db8f8e 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -885,6 +885,11 @@ export interface FeatureToggles { */ alertingJiraIntegration?: boolean; /** + * + * @default true + */ + alertingUseNewSimplifiedRoutingHashAlgorithm?: boolean; + /** * Use the scopes navigation endpoint instead of the dashboardbindings endpoint */ useScopesNavigationEndpoint?: boolean; @@ -1009,11 +1014,6 @@ export interface FeatureToggles { */ alertRuleUseFiredAtForStartsAt?: boolean; /** - * Generate simplified routing with old hashes - * @default false - */ - alertingGenerateSimplifiedRoutingWithOldHashes?: boolean; - /** * Enables the alerting bulk actions in the UI * @default true */ diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 6ee6382e1d1..54c66da34bc 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1520,6 +1520,16 @@ var ( FrontendOnly: true, HideFromDocs: true, }, + { + Name: "alertingUseNewSimplifiedRoutingHashAlgorithm", + Description: "", + Stage: FeatureStagePublicPreview, + Owner: grafanaAlertingSquad, + HideFromAdminPage: true, + HideFromDocs: true, + RequiresRestart: true, + Expression: "true", + }, { Name: "useScopesNavigationEndpoint", Description: "Use the scopes navigation endpoint instead of the dashboardbindings endpoint", @@ -1739,13 +1749,6 @@ var ( Owner: grafanaAlertingSquad, Expression: "false", }, - { - Name: "alertingGenerateSimplifiedRoutingWithOldHashes", - Description: "Generate simplified routing with old hashes", - Stage: FeatureStageExperimental, - Owner: grafanaAlertingSquad, - Expression: "false", - }, { Name: "alertingBulkActionsInUI", Description: "Enables the alerting bulk actions in the UI", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 846bd800a53..b453c49da28 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -198,6 +198,7 @@ fetchRulesUsingPost,experimental,@grafana/alerting-squad,false,false,false newLogsPanel,experimental,@grafana/observability-logs,false,false,true grafanaconThemes,GA,@grafana/grafana-frontend-platform,false,true,false alertingJiraIntegration,experimental,@grafana/alerting-squad,false,false,true +alertingUseNewSimplifiedRoutingHashAlgorithm,preview,@grafana/alerting-squad,false,true,false useScopesNavigationEndpoint,experimental,@grafana/grafana-frontend-platform,false,false,true scopeSearchAllLevels,experimental,@grafana/grafana-frontend-platform,false,false,false alertingRuleVersionHistoryRestore,GA,@grafana/alerting-squad,false,false,true @@ -226,7 +227,6 @@ tempoAlerting,experimental,@grafana/observability-traces-and-profiling,false,fal pluginsAutoUpdate,experimental,@grafana/plugins-platform-backend,false,false,false alertingListViewV2PreviewToggle,privatePreview,@grafana/alerting-squad,false,false,true alertRuleUseFiredAtForStartsAt,experimental,@grafana/alerting-squad,false,false,false -alertingGenerateSimplifiedRoutingWithOldHashes,experimental,@grafana/alerting-squad,false,false,false alertingBulkActionsInUI,GA,@grafana/alerting-squad,false,false,true kubernetesAuthzApis,experimental,@grafana/identity-access-team,false,false,false kubernetesAuthZHandlerRedirect,experimental,@grafana/identity-access-team,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 710f0404365..ba8b131f74f 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -803,6 +803,9 @@ const ( // Enables the new Jira integration for contact points in cloud alert managers. FlagAlertingJiraIntegration = "alertingJiraIntegration" + // FlagAlertingUseNewSimplifiedRoutingHashAlgorithm + FlagAlertingUseNewSimplifiedRoutingHashAlgorithm = "alertingUseNewSimplifiedRoutingHashAlgorithm" + // FlagUseScopesNavigationEndpoint // Use the scopes navigation endpoint instead of the dashboardbindings endpoint FlagUseScopesNavigationEndpoint = "useScopesNavigationEndpoint" @@ -915,10 +918,6 @@ const ( // Use FiredAt for StartsAt when sending alerts to Alertmaanger FlagAlertRuleUseFiredAtForStartsAt = "alertRuleUseFiredAtForStartsAt" - // FlagAlertingGenerateSimplifiedRoutingWithOldHashes - // Generate simplified routing with old hashes - FlagAlertingGenerateSimplifiedRoutingWithOldHashes = "alertingGenerateSimplifiedRoutingWithOldHashes" - // FlagAlertingBulkActionsInUI // Enables the alerting bulk actions in the UI FlagAlertingBulkActionsInUI = "alertingBulkActionsInUI" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 2e027cce650..f2995c233b0 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -307,19 +307,6 @@ "hideFromDocs": true } }, - { - "metadata": { - "name": "alertingGenerateSimplifiedRoutingWithOldHashes", - "resourceVersion": "1759334385382", - "creationTimestamp": "2025-10-01T15:59:45Z" - }, - "spec": { - "description": "Generate simplified routing with old hashes", - "stage": "experimental", - "codeowner": "@grafana/alerting-squad", - "expression": "false" - } - }, { "metadata": { "name": "alertingImportAlertmanagerAPI", @@ -623,6 +610,46 @@ "expression": "true" } }, + { + "metadata": { + "name": "alertingUseNewSimplifiedRoutingHashAlgorithm", + "resourceVersion": "1759339813575", + "creationTimestamp": "2025-10-01T17:28:42Z", + "deletionTimestamp": "2025-10-01T17:29:29Z", + "annotations": { + "grafana.app/updatedTimestamp": "2025-10-01 17:30:13.575464 +0000 UTC" + } + }, + "spec": { + "description": "", + "stage": "preview", + "codeowner": "@grafana/alerting-squad", + "requiresRestart": true, + "hideFromAdminPage": true, + "hideFromDocs": true, + "expression": "true" + } + }, + { + "metadata": { + "name": "alertingUseOldSimplifiedRoutingHashAlgorithm", + "resourceVersion": "1759339782639", + "creationTimestamp": "2025-10-01T17:29:29Z", + "deletionTimestamp": "2025-10-01T17:30:13Z", + "annotations": { + "grafana.app/updatedTimestamp": "2025-10-01 17:29:42.63941 +0000 UTC" + } + }, + "spec": { + "description": "", + "stage": "deprecated", + "codeowner": "@grafana/alerting-squad", + "requiresRestart": true, + "hideFromAdminPage": true, + "hideFromDocs": true, + "expression": "false" + } + }, { "metadata": { "name": "alertmanagerRemotePrimary", diff --git a/pkg/services/ngalert/api/api_alertmanager_test.go b/pkg/services/ngalert/api/api_alertmanager_test.go index bbe1df17155..433c4d173f6 100644 --- a/pkg/services/ngalert/api/api_alertmanager_test.go +++ b/pkg/services/ngalert/api/api_alertmanager_test.go @@ -291,7 +291,8 @@ func TestAlertmanagerAutogenConfig(t *testing.T) { 1: {AlertmanagerConfiguration: validConfig, OrgID: 1}, 2: {AlertmanagerConfiguration: validConfigWithoutAutogen, OrgID: 2}, } - sut.mam = createMultiOrgAlertmanager(t, configs) + ft := featuremgmt.WithFeatures(featuremgmt.FlagAlertingUseNewSimplifiedRoutingHashAlgorithm) + sut.mam = createMultiOrgAlertmanager(t, configs, withAMFeatureToggles(ft)) return sut, configs } @@ -577,9 +578,29 @@ func createSut(t *testing.T) AlertmanagerSrv { } } -func createMultiOrgAlertmanager(t *testing.T, configs map[int64]*ngmodels.AlertConfiguration) *notifier.MultiOrgAlertmanager { +type createMultiOrgAMOptions struct { + featureToggles featuremgmt.FeatureToggles +} + +type createMultiOrgAMOptionsFunc func(*createMultiOrgAMOptions) + +func withAMFeatureToggles(toggles featuremgmt.FeatureToggles) createMultiOrgAMOptionsFunc { + return func(opts *createMultiOrgAMOptions) { + opts.featureToggles = toggles + } +} + +func createMultiOrgAlertmanager(t *testing.T, configs map[int64]*ngmodels.AlertConfiguration, opts ...createMultiOrgAMOptionsFunc) *notifier.MultiOrgAlertmanager { t.Helper() + options := createMultiOrgAMOptions{ + featureToggles: featuremgmt.WithFeatures(), + } + + for _, opt := range opts { + opt(&options) + } + configStore := notifier.NewFakeConfigStore(t, configs) orgStore := notifier.NewFakeOrgStore(t, []int64{1, 2, 3}) provStore := ngfakes.NewFakeProvisioningStore() @@ -610,7 +631,7 @@ func createMultiOrgAlertmanager(t *testing.T, configs map[int64]*ngmodels.AlertC ngfakes.NewFakeReceiverPermissionsService(), log.New("testlogger"), secretsService, - featuremgmt.WithManager(), + options.featureToggles, nil, ) require.NoError(t, err) diff --git a/pkg/services/ngalert/api/api_testing.go b/pkg/services/ngalert/api/api_testing.go index e26f2a89f9a..ceda51a7a22 100644 --- a/pkg/services/ngalert/api/api_testing.go +++ b/pkg/services/ngalert/api/api_testing.go @@ -113,7 +113,7 @@ func (srv TestingApiSrv) RouteTestGrafanaRuleConfig(c *contextmodel.ReqContext, now, rule, results, - state.GetRuleExtraLabels(log.New("testing"), rule, folder.Fullpath, includeFolder), + state.GetRuleExtraLabels(log.New("testing"), rule, folder.Fullpath, includeFolder, srv.featureManager), nil, ) diff --git a/pkg/services/ngalert/models/notifications.go b/pkg/services/ngalert/models/notifications.go index f3793f9a881..609ee724085 100644 --- a/pkg/services/ngalert/models/notifications.go +++ b/pkg/services/ngalert/models/notifications.go @@ -8,6 +8,7 @@ import ( "unsafe" "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/prometheus/common/model" ) @@ -102,12 +103,12 @@ func (s *NotificationSettings) Validate() error { // - AutogeneratedRouteLabel: "true" // - AutogeneratedRouteReceiverNameLabel: Receiver // - AutogeneratedRouteSettingsHashLabel: Fingerprint (if the NotificationSettings are not all default) -func (s *NotificationSettings) ToLabels() data.Labels { +func (s *NotificationSettings) ToLabels(features featuremgmt.FeatureToggles) data.Labels { result := make(data.Labels, 3) result[AutogeneratedRouteLabel] = "true" result[AutogeneratedRouteReceiverNameLabel] = s.Receiver if !s.IsAllDefault() { - result[AutogeneratedRouteSettingsHashLabel] = s.Fingerprint().String() + result[AutogeneratedRouteSettingsHashLabel] = s.Fingerprint(features).String() } return result } @@ -160,7 +161,7 @@ func NewDefaultNotificationSettings(receiver string) NotificationSettings { // Fingerprint calculates a hash value to uniquely identify a NotificationSettings by its attributes. // The hash is calculated by concatenating the strings and durations of the NotificationSettings attributes // and using an invalid UTF-8 sequence as a separator. -func (s *NotificationSettings) Fingerprint() data.Fingerprint { +func (s *NotificationSettings) Fingerprint(features featuremgmt.FeatureToggles) data.Fingerprint { h := fnv.New64() tmp := make([]byte, 8) @@ -192,49 +193,13 @@ func (s *NotificationSettings) Fingerprint() data.Fingerprint { } // Add a separator between the time intervals to avoid collisions // when all settings are the same including interval names except for the interval type (mute vs active). - _, _ = h.Write([]byte{255}) + // Use new algorithm by default, unless feature flag is explicitly disabled + if features == nil || (features != nil && features.IsEnabledGlobally(featuremgmt.FlagAlertingUseNewSimplifiedRoutingHashAlgorithm)) { + _, _ = h.Write([]byte{255}) + } for _, interval := range s.ActiveTimeIntervals { writeString(interval) } return data.Fingerprint(h.Sum64()) } - -// FingerprintOld calculates a hash value using the old algorithm (before the separator fix in the Fingerpring function). -// This is used temporarily during migration to support existing alerts with old hash labels. -// TODO: Remove this method once the migration is complete. -func (s *NotificationSettings) FingerprintOld() data.Fingerprint { - h := fnv.New64() - tmp := make([]byte, 8) - - 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 - } - writeDuration := func(d *model.Duration) { - if d == nil { - _, _ = h.Write([]byte{255}) - } else { - binary.LittleEndian.PutUint64(tmp, uint64(*d)) - _, _ = h.Write(tmp) - _, _ = h.Write([]byte{255}) - } - } - - writeString(s.Receiver) - for _, gb := range s.NormalizedGroupBy() { - writeString(gb) - } - writeDuration(s.GroupWait) - writeDuration(s.GroupInterval) - writeDuration(s.RepeatInterval) - for _, interval := range s.MuteTimeIntervals { - writeString(interval) - } - for _, interval := range s.ActiveTimeIntervals { - writeString(interval) - } - return data.Fingerprint(h.Sum64()) -} diff --git a/pkg/services/ngalert/models/notifications_test.go b/pkg/services/ngalert/models/notifications_test.go index b27e4e0e817..1bacbe403e7 100644 --- a/pkg/services/ngalert/models/notifications_test.go +++ b/pkg/services/ngalert/models/notifications_test.go @@ -195,61 +195,12 @@ func TestNotificationSettingsLabels(t *testing.T) { for _, tt := range testCases { t.Run(tt.name, func(t *testing.T) { - labels := tt.notificationSettings.ToLabels() + labels := tt.notificationSettings.ToLabels(nil) require.Equal(t, tt.labels, labels) }) } } -func TestNotificationSettings_OldFingerprint(t *testing.T) { - testCases := []struct { - name string - notificationSettings NotificationSettings - expectedOldFingerprint string - }{ - { - name: "default notification settings with hardcoded default group by", - notificationSettings: NotificationSettings{ - Receiver: "receiver name", - GroupBy: DefaultNotificationSettingsGroupBy, - }, - expectedOldFingerprint: "6027cdeaff62ba3f", - }, - { - name: "custom notification settings", - notificationSettings: NotificationSettings{ - Receiver: "receiver name", - GroupBy: []string{"label1", "label2"}, - GroupWait: util.Pointer(model.Duration(1 * time.Minute)), - GroupInterval: util.Pointer(model.Duration(2 * time.Minute)), - RepeatInterval: util.Pointer(model.Duration(3 * time.Minute)), - MuteTimeIntervals: []string{"maintenance1", "maintenance2"}, - }, - expectedOldFingerprint: "47164c92f2986a35", - }, - { - name: "custom notification settings with active time interval", - notificationSettings: NotificationSettings{ - Receiver: "receiver name", - GroupBy: []string{"label1", "label2"}, - GroupWait: util.Pointer(model.Duration(1 * time.Minute)), - GroupInterval: util.Pointer(model.Duration(2 * time.Minute)), - RepeatInterval: util.Pointer(model.Duration(3 * time.Minute)), - MuteTimeIntervals: []string{"maintenance1", "maintenance2"}, - ActiveTimeIntervals: []string{"active1", "active2"}, - }, - expectedOldFingerprint: "a173df6210e43af0", - }, - } - - for _, tt := range testCases { - t.Run(tt.name, func(t *testing.T) { - oldFp := tt.notificationSettings.FingerprintOld() - require.Equal(t, tt.expectedOldFingerprint, oldFp.String()) - }) - } -} - func TestNotificationSettings_TimeIntervals(t *testing.T) { // Create notification settings with default settings and usign the same // time interval, but in one case as a mute time interval and in another case @@ -268,7 +219,7 @@ func TestNotificationSettings_TimeIntervals(t *testing.T) { ActiveTimeIntervals: []string{timeInterval}, } - require.NotEqual(t, activeSettings.Fingerprint(), muteSettings.Fingerprint()) + require.NotEqual(t, activeSettings.Fingerprint(nil), muteSettings.Fingerprint(nil)) } func TestNormalizedGroupBy(t *testing.T) { diff --git a/pkg/services/ngalert/notifier/autogen_alertmanager.go b/pkg/services/ngalert/notifier/autogen_alertmanager.go index 4ab8b35b5f2..7523e368ad5 100644 --- a/pkg/services/ngalert/notifier/autogen_alertmanager.go +++ b/pkg/services/ngalert/notifier/autogen_alertmanager.go @@ -51,7 +51,7 @@ func newAutogeneratedRoute[R receiver](ctx context.Context, logger log.Logger, s // contact point even if no rules are using it. This will prevent race conditions between AM sync and rule sync. for _, receiver := range cfg.GetReceivers() { setting := models.NewDefaultNotificationSettings(receiver.GetName()) - fp := setting.Fingerprint() + fp := setting.Fingerprint(features) notificationSettings[fp] = setting } @@ -66,7 +66,7 @@ func newAutogeneratedRoute[R receiver](ctx context.Context, logger log.Logger, s } return autogeneratedRoute{}, fmt.Errorf("invalid notification settings for rule %s: %w", ruleKey.UID, err) } - fp := setting.Fingerprint() + fp := setting.Fingerprint(features) // Keep only unique settings. if _, ok := notificationSettings[fp]; ok { continue @@ -77,7 +77,7 @@ func newAutogeneratedRoute[R receiver](ctx context.Context, logger log.Logger, s if len(notificationSettings) == 0 { return autogeneratedRoute{}, nil } - newAutogenRoute, err := generateRouteFromSettings(cfg.GetRoute().Receiver, notificationSettings, features) + newAutogenRoute, err := generateRouteFromSettings(cfg.GetRoute().Receiver, notificationSettings) if err != nil { return autogeneratedRoute{}, fmt.Errorf("failed to create autogenerated route: %w", err) } @@ -92,7 +92,7 @@ type autogeneratedRoute struct { // 1. with matcher by label models.AutogeneratedRouteLabel equals 'true'. // 2. with matcher by receiver name. // 3. with matcher by unique combination of optional settings. It is created only if there are optional settings. -func generateRouteFromSettings(defaultReceiver string, settings map[data.Fingerprint]models.NotificationSettings, features featuremgmt.FeatureToggles) (autogeneratedRoute, error) { +func generateRouteFromSettings(defaultReceiver string, settings map[data.Fingerprint]models.NotificationSettings) (autogeneratedRoute, error) { keys := maps.Keys(settings) // sort keys to make sure that the hash we calculate using it is stable slices.Sort(keys) @@ -136,34 +136,26 @@ func generateRouteFromSettings(defaultReceiver string, settings map[data.Fingerp if s.IsAllDefault() { continue } - + settingMatcher, err := labels.NewMatcher(labels.MatchEqual, models.AutogeneratedRouteSettingsHashLabel, fingerprint.String()) + if err != nil { + return autogeneratedRoute{}, err + } normalized := s.NormalizedGroupBy() groupByAll, groupBy := toGroupBy(normalized...) + receiverRoute.Routes = append(receiverRoute.Routes, &definitions.Route{ + Receiver: s.Receiver, + ObjectMatchers: definitions.ObjectMatchers{settingMatcher}, + Continue: false, // Only a single setting-specific route should match. - fingerprints := []data.Fingerprint{fingerprint} - if features.IsEnabledGlobally(featuremgmt.FlagAlertingGenerateSimplifiedRoutingWithOldHashes) { - fingerprints = append([]data.Fingerprint{s.FingerprintOld()}, fingerprints...) - } - - for _, fp := range fingerprints { - matcher, err := labels.NewMatcher(labels.MatchEqual, models.AutogeneratedRouteSettingsHashLabel, fp.String()) - if err != nil { - return autogeneratedRoute{}, err - } - receiverRoute.Routes = append(receiverRoute.Routes, &definitions.Route{ - Receiver: s.Receiver, - ObjectMatchers: definitions.ObjectMatchers{matcher}, - Continue: false, // Only a single setting-specific route should match. - GroupByStr: normalized, - GroupBy: groupBy, - GroupByAll: groupByAll, - MuteTimeIntervals: s.MuteTimeIntervals, - ActiveTimeIntervals: s.ActiveTimeIntervals, - GroupWait: s.GroupWait, - GroupInterval: s.GroupInterval, - RepeatInterval: s.RepeatInterval, - }) - } + GroupByStr: normalized, + GroupBy: groupBy, + GroupByAll: groupByAll, + MuteTimeIntervals: s.MuteTimeIntervals, + ActiveTimeIntervals: s.ActiveTimeIntervals, + GroupWait: s.GroupWait, + GroupInterval: s.GroupInterval, + RepeatInterval: s.RepeatInterval, + }) } return autogeneratedRoute{ diff --git a/pkg/services/ngalert/notifier/autogen_alertmanager_test.go b/pkg/services/ngalert/notifier/autogen_alertmanager_test.go index 6fec524ff1a..bf376502caa 100644 --- a/pkg/services/ngalert/notifier/autogen_alertmanager_test.go +++ b/pkg/services/ngalert/notifier/autogen_alertmanager_test.go @@ -13,15 +13,12 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/log/logtest" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/util" ) func TestAddAutogenConfig(t *testing.T) { - withOldHashFeature := featuremgmt.WithFeatures(featuremgmt.FlagAlertingGenerateSimplifiedRoutingWithOldHashes) - rootRoute := func() *definitions.Route { return &definitions.Route{ Receiver: "default", @@ -72,7 +69,6 @@ func TestAddAutogenConfig(t *testing.T) { existingConfig *definitions.PostableApiAlertingConfig storeSettings []models.NotificationSettings skipInvalid bool - featureToggles featuremgmt.FeatureToggles expRoute *definitions.Route expErrorContains string }{ @@ -80,14 +76,12 @@ func TestAddAutogenConfig(t *testing.T) { name: "no settings or receivers, no change", existingConfig: configGen(nil, nil), storeSettings: []models.NotificationSettings{}, - featureToggles: withOldHashFeature, expRoute: rootRoute(), }, { name: "no settings but some receivers, add default routes for receivers", existingConfig: configGen([]string{"receiver1", "receiver2", "receiver3"}, nil), storeSettings: []models.NotificationSettings{}, - featureToggles: withOldHashFeature, expRoute: withChildRoutes(rootRoute(), &definitions.Route{ Receiver: "default", ObjectMatchers: matcher(models.AutogeneratedRouteLabel, "true"), @@ -115,7 +109,6 @@ func TestAddAutogenConfig(t *testing.T) { { name: "settings with custom options, add option-specific routes", existingConfig: configGen([]string{"receiver1", "receiver2", "receiver3", "receiver4", "receiver5"}, []string{"maintenance", "active"}), - featureToggles: withOldHashFeature, storeSettings: []models.NotificationSettings{ models.CopyNotificationSettings(models.NewDefaultNotificationSettings("receiver1"), models.NSMuts.WithGroupInterval(util.Pointer(1*time.Minute))), models.CopyNotificationSettings(models.NewDefaultNotificationSettings("receiver2"), models.NSMuts.WithGroupWait(util.Pointer(2*time.Minute))), @@ -137,117 +130,50 @@ func TestAddAutogenConfig(t *testing.T) { Receiver: "default", ObjectMatchers: matcher(models.AutogeneratedRouteLabel, "true"), Routes: []*definitions.Route{ - withChildRoutes(basicContactRoute("receiver1"), - // Old hash for complex settings - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "f134b8faf7db083c"), - GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel, "custom"}, - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - GroupWait: util.Pointer(model.Duration(2 * time.Minute)), - RepeatInterval: util.Pointer(model.Duration(3 * time.Minute)), - MuteTimeIntervals: []string{"maintenance"}, - ActiveTimeIntervals: []string{"active"}, - }, - // New hash for complex settings - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "02466789dc88da23"), - GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel, "custom"}, - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - GroupWait: util.Pointer(model.Duration(2 * time.Minute)), - RepeatInterval: util.Pointer(model.Duration(3 * time.Minute)), - MuteTimeIntervals: []string{"maintenance"}, - ActiveTimeIntervals: []string{"active"}, - }, - // Old hash for simple GroupInterval - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "dde34b8127e68f31"), - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - }, - // New hash for simple GroupInterval - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "efc87d76ccc550bc"), - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - }, - ), - withChildRoutes(basicContactRoute("receiver2"), - // Old hash - &definitions.Route{ - Receiver: "receiver2", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "27e1d1717c9ef621"), - GroupWait: util.Pointer(model.Duration(2 * time.Minute)), - }, - // New hash - &definitions.Route{ - Receiver: "receiver2", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "63ad04d6c21c3aec"), - GroupWait: util.Pointer(model.Duration(2 * time.Minute)), - }, - ), - withChildRoutes(basicContactRoute("receiver5"), - // Old hash for active intervals - &definitions.Route{ - Receiver: "receiver5", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "cd6cd2089632453c"), - ActiveTimeIntervals: []string{"active"}, - }, - // New hash for active intervals - &definitions.Route{ - Receiver: "receiver5", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "8cd5f9adeac58123"), - ActiveTimeIntervals: []string{"active"}, - }, - // Old hash for mute intervals - &definitions.Route{ - Receiver: "receiver5", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "030d6474aec0b553"), - MuteTimeIntervals: []string{"maintenance"}, - }, - // New hash for mute intervals - &definitions.Route{ - Receiver: "receiver5", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "f0770544f1741cf6"), - MuteTimeIntervals: []string{"maintenance"}, - }, - ), - withChildRoutes(basicContactRoute("receiver4"), - // Old hash - &definitions.Route{ - Receiver: "receiver4", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "b3a2fa5e615dcc7e"), - GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel, "custom"}, - }, - // New hash - &definitions.Route{ - Receiver: "receiver4", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "9bbbec5f72627ae5"), - GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel, "custom"}, - }, - ), - withChildRoutes(basicContactRoute("receiver3"), - // Old hash - &definitions.Route{ - Receiver: "receiver3", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "9e282ef0193d830a"), - RepeatInterval: util.Pointer(model.Duration(3 * time.Minute)), - }, - // New hash - &definitions.Route{ - Receiver: "receiver3", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "fbcacbfae385a901"), - RepeatInterval: util.Pointer(model.Duration(3 * time.Minute)), - }, - ), + withChildRoutes(basicContactRoute("receiver1"), &definitions.Route{ + Receiver: "receiver1", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "02466789dc88da23"), + GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel, "custom"}, + GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), + GroupWait: util.Pointer(model.Duration(2 * time.Minute)), + RepeatInterval: util.Pointer(model.Duration(3 * time.Minute)), + MuteTimeIntervals: []string{"maintenance"}, + ActiveTimeIntervals: []string{"active"}, + }, &definitions.Route{ + Receiver: "receiver1", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "efc87d76ccc550bc"), + GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), + }), + withChildRoutes(basicContactRoute("receiver2"), &definitions.Route{ + Receiver: "receiver2", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "63ad04d6c21c3aec"), + GroupWait: util.Pointer(model.Duration(2 * time.Minute)), + }), + withChildRoutes(basicContactRoute("receiver5"), &definitions.Route{ + Receiver: "receiver5", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "8cd5f9adeac58123"), + ActiveTimeIntervals: []string{"active"}, + }, &definitions.Route{ + Receiver: "receiver5", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "f0770544f1741cf6"), + MuteTimeIntervals: []string{"maintenance"}, + }), + withChildRoutes(basicContactRoute("receiver4"), &definitions.Route{ + Receiver: "receiver4", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "9bbbec5f72627ae5"), + GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel, "custom"}, + }), + withChildRoutes(basicContactRoute("receiver3"), &definitions.Route{ + Receiver: "receiver3", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "fbcacbfae385a901"), + RepeatInterval: util.Pointer(model.Duration(3 * time.Minute)), + }), }, }), }, { name: "settings with custom options and nil groupBy, groupBy should inherit from parent", existingConfig: configGen([]string{"receiver1"}, nil), - featureToggles: withOldHashFeature, storeSettings: []models.NotificationSettings{ models.CopyNotificationSettings(models.NewDefaultNotificationSettings("receiver1"), models.NSMuts.WithGroupInterval(util.Pointer(1*time.Minute)), models.NSMuts.WithGroupBy()), }, @@ -255,29 +181,18 @@ func TestAddAutogenConfig(t *testing.T) { Receiver: "default", ObjectMatchers: matcher(models.AutogeneratedRouteLabel, "true"), Routes: []*definitions.Route{ - withChildRoutes(basicContactRoute("receiver1"), - // Old hash - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "dde34b8127e68f31"), - GroupByStr: nil, - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - }, - // New hash - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "efc87d76ccc550bc"), - GroupByStr: nil, - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - }, - ), + withChildRoutes(basicContactRoute("receiver1"), &definitions.Route{ + Receiver: "receiver1", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "efc87d76ccc550bc"), + GroupByStr: nil, + GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), + }), }, }), }, { name: "settings with nil groupBy should have different fingerprint than default groupBy", existingConfig: configGen([]string{"receiver1"}, nil), - featureToggles: withOldHashFeature, storeSettings: []models.NotificationSettings{ models.CopyNotificationSettings(models.NewDefaultNotificationSettings("receiver1"), models.NSMuts.WithGroupInterval(util.Pointer(1*time.Minute)), models.NSMuts.WithGroupBy()), models.CopyNotificationSettings(models.NewDefaultNotificationSettings("receiver1"), models.NSMuts.WithGroupInterval(util.Pointer(1*time.Minute)), models.NSMuts.WithGroupBy(models.DefaultNotificationSettingsGroupBy...)), @@ -286,36 +201,17 @@ func TestAddAutogenConfig(t *testing.T) { Receiver: "default", ObjectMatchers: matcher(models.AutogeneratedRouteLabel, "true"), Routes: []*definitions.Route{ - withChildRoutes(basicContactRoute("receiver1"), - // Old hash for explicit default groupBy - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "e1f3a275a8918385"), - GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel}, - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - }, - // New hash for explicit default groupBy - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "828092ed6f427a00"), - GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel}, - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - }, - // Old hash for nil groupBy - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "dde34b8127e68f31"), - GroupByStr: nil, - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - }, - // New hash for nil groupBy - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "efc87d76ccc550bc"), - GroupByStr: nil, - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - }, - ), + withChildRoutes(basicContactRoute("receiver1"), &definitions.Route{ + Receiver: "receiver1", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "828092ed6f427a00"), // Different hash. + GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel}, + GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), + }, &definitions.Route{ + Receiver: "receiver1", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "efc87d76ccc550bc"), + GroupByStr: nil, + GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), + }), }, }), }, @@ -327,27 +223,16 @@ func TestAddAutogenConfig(t *testing.T) { models.CopyNotificationSettings(models.NewDefaultNotificationSettings("receiver1"), models.NSMuts.WithGroupInterval(util.Pointer(1*time.Minute)), models.NSMuts.WithGroupBy(model.AlertNameLabel)), models.CopyNotificationSettings(models.NewDefaultNotificationSettings("receiver1"), models.NSMuts.WithGroupInterval(util.Pointer(1*time.Minute)), models.NSMuts.WithGroupBy(models.DefaultNotificationSettingsGroupBy...)), }, - featureToggles: withOldHashFeature, expRoute: withChildRoutes(rootRoute(), &definitions.Route{ Receiver: "default", ObjectMatchers: matcher(models.AutogeneratedRouteLabel, "true"), Routes: []*definitions.Route{ - withChildRoutes(basicContactRoute("receiver1"), - // Old hash - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "e1f3a275a8918385"), - GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel}, - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - }, - // New hash - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "828092ed6f427a00"), - GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel}, - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - }, - ), + withChildRoutes(basicContactRoute("receiver1"), &definitions.Route{ + Receiver: "receiver1", + ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "828092ed6f427a00"), + GroupByStr: []string{models.FolderTitleLabel, model.AlertNameLabel}, + GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), + }), }, }), }, @@ -359,8 +244,7 @@ func TestAddAutogenConfig(t *testing.T) { models.CopyNotificationSettings(models.NewDefaultNotificationSettings("receiver1"), models.NSMuts.WithMuteTimeIntervals("maintenance")), // Doesn't exist. models.CopyNotificationSettings(models.NewDefaultNotificationSettings("receiver2"), models.NSMuts.WithGroupWait(util.Pointer(-2*time.Minute))), // Negative. }, - featureToggles: withOldHashFeature, - skipInvalid: true, + skipInvalid: true, expRoute: withChildRoutes(rootRoute(), &definitions.Route{ Receiver: "default", ObjectMatchers: matcher(models.AutogeneratedRouteLabel, "true"), @@ -392,27 +276,6 @@ func TestAddAutogenConfig(t *testing.T) { skipInvalid: false, expErrorContains: "group wait", }, - { - name: "without feature toggle, only new hash route is generated", - existingConfig: configGen([]string{"receiver1"}, nil), - storeSettings: []models.NotificationSettings{ - models.CopyNotificationSettings(models.NewDefaultNotificationSettings("receiver1"), models.NSMuts.WithGroupInterval(util.Pointer(1*time.Minute))), - }, - expRoute: withChildRoutes(rootRoute(), &definitions.Route{ - Receiver: "default", - ObjectMatchers: matcher(models.AutogeneratedRouteLabel, "true"), - Routes: []*definitions.Route{ - withChildRoutes(basicContactRoute("receiver1"), - // Only new hash route - &definitions.Route{ - Receiver: "receiver1", - ObjectMatchers: matcher(models.AutogeneratedRouteSettingsHashLabel, "efc87d76ccc550bc"), - GroupInterval: util.Pointer(model.Duration(1 * time.Minute)), - }, - ), - }, - }), - }, } for _, tt := range testCases { @@ -427,12 +290,7 @@ func TestAddAutogenConfig(t *testing.T) { store.notificationSettings[orgId][models.AlertRuleKey{OrgID: orgId, UID: util.GenerateShortUID()}] = []models.NotificationSettings{setting} } - features := tt.featureToggles - if features == nil { - features = featuremgmt.WithFeatures() - } - - err := AddAutogenConfig(context.Background(), &logtest.Fake{}, store, orgId, tt.existingConfig, tt.skipInvalid, features) + err := AddAutogenConfig(context.Background(), &logtest.Fake{}, store, orgId, tt.existingConfig, tt.skipInvalid, nil) if tt.expErrorContains != "" { require.Error(t, err) require.ErrorContains(t, err, tt.expErrorContains) diff --git a/pkg/services/ngalert/schedule/alert_rule.go b/pkg/services/ngalert/schedule/alert_rule.go index 523dcf0d86c..3991df89e77 100644 --- a/pkg/services/ngalert/schedule/alert_rule.go +++ b/pkg/services/ngalert/schedule/alert_rule.go @@ -471,7 +471,7 @@ func (a *alertRule) evaluate(ctx context.Context, e *Evaluation, span trace.Span e.scheduledAt, e.rule, results, - state.GetRuleExtraLabels(logger, e.rule, e.folderTitle, !a.disableGrafanaFolder), + state.GetRuleExtraLabels(logger, e.rule, e.folderTitle, !a.disableGrafanaFolder, a.featureToggles), func(ctx context.Context, statesToSend state.StateTransitions) { start := a.clock.Now() alerts := a.send(ctx, logger, statesToSend) diff --git a/pkg/services/ngalert/schedule/alert_rule_test.go b/pkg/services/ngalert/schedule/alert_rule_test.go index 5e301c5aa08..1ae84649fbd 100644 --- a/pkg/services/ngalert/schedule/alert_rule_test.go +++ b/pkg/services/ngalert/schedule/alert_rule_test.go @@ -1317,7 +1317,7 @@ func stateForRule(rule *models.AlertRule, ts time.Time, evalState eval.State) *s for k, v := range rule.Labels { s.Labels[k] = v } - for k, v := range state.GetRuleExtraLabels(&logtest.Fake{}, rule, "", true) { + for k, v := range state.GetRuleExtraLabels(&logtest.Fake{}, rule, "", true, nil) { if _, ok := s.Labels[k]; !ok { s.Labels[k] = v } diff --git a/pkg/services/ngalert/schedule/registry.go b/pkg/services/ngalert/schedule/registry.go index 240c62bdc40..ede71c94d75 100644 --- a/pkg/services/ngalert/schedule/registry.go +++ b/pkg/services/ngalert/schedule/registry.go @@ -304,7 +304,7 @@ func (r ruleWithFolder) Fingerprint() fingerprint { } for _, setting := range rule.NotificationSettings { - binary.LittleEndian.PutUint64(tmp, uint64(setting.Fingerprint())) + binary.LittleEndian.PutUint64(tmp, uint64(setting.Fingerprint(nil))) writeBytes(tmp) } diff --git a/pkg/services/ngalert/state/state.go b/pkg/services/ngalert/state/state.go index db807953b13..9df04681353 100644 --- a/pkg/services/ngalert/state/state.go +++ b/pkg/services/ngalert/state/state.go @@ -18,6 +18,7 @@ import ( "github.com/grafana/grafana/pkg/apimachinery/errutil" "github.com/grafana/grafana/pkg/expr" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ngalert/eval" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/screenshot" @@ -753,7 +754,7 @@ func ParseFormattedState(stateStr string) (eval.State, string, error) { } // GetRuleExtraLabels returns a map of built-in labels that should be added to an alert before it is sent to the Alertmanager or its state is cached. -func GetRuleExtraLabels(l log.Logger, rule *models.AlertRule, folderTitle string, includeFolder bool) map[string]string { +func GetRuleExtraLabels(l log.Logger, rule *models.AlertRule, folderTitle string, includeFolder bool, features featuremgmt.FeatureToggles) map[string]string { extraLabels := make(map[string]string, 4) extraLabels[alertingModels.NamespaceUIDLabel] = rule.NamespaceUID @@ -771,7 +772,7 @@ func GetRuleExtraLabels(l log.Logger, rule *models.AlertRule, folderTitle string ignored, _ := json.Marshal(rule.NotificationSettings[1:]) l.Error("Detected multiple notification settings, which is not supported. Only the first will be applied", "ignored_settings", string(ignored)) } - return mergeLabels(extraLabels, rule.NotificationSettings[0].ToLabels()) + return mergeLabels(extraLabels, rule.NotificationSettings[0].ToLabels(features)) } return extraLabels } diff --git a/pkg/services/ngalert/state/state_test.go b/pkg/services/ngalert/state/state_test.go index 8daf82f9a66..acffa24e2cc 100644 --- a/pkg/services/ngalert/state/state_test.go +++ b/pkg/services/ngalert/state/state_test.go @@ -779,7 +779,7 @@ func TestGetRuleExtraLabels(t *testing.T) { models.RuleUIDLabel: rule.UID, ngmodels.AutogeneratedRouteLabel: "true", ngmodels.AutogeneratedRouteReceiverNameLabel: ns.Receiver, - ngmodels.AutogeneratedRouteSettingsHashLabel: ns.Fingerprint().String(), + ngmodels.AutogeneratedRouteSettingsHashLabel: ns.Fingerprint(nil).String(), }, }, "ignore_multiple_notifications": { @@ -794,14 +794,14 @@ func TestGetRuleExtraLabels(t *testing.T) { models.RuleUIDLabel: rule.UID, ngmodels.AutogeneratedRouteLabel: "true", ngmodels.AutogeneratedRouteReceiverNameLabel: ns.Receiver, - ngmodels.AutogeneratedRouteSettingsHashLabel: ns.Fingerprint().String(), + ngmodels.AutogeneratedRouteSettingsHashLabel: ns.Fingerprint(nil).String(), }, }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { - result := GetRuleExtraLabels(logger, tc.rule, folderTitle, tc.includeFolder) + result := GetRuleExtraLabels(logger, tc.rule, folderTitle, tc.includeFolder, nil) require.Equal(t, tc.expected, result) }) }