From a910188675c013d9371e078cb71f82985147aa4e Mon Sep 17 00:00:00 2001 From: Tito Lins Date: Thu, 12 Sep 2024 10:24:12 +0200 Subject: [PATCH] Replace prom MustRegister with Register (#92725) --- pkg/services/ngalert/metrics/alertmanager.go | 17 +++++++--- .../ngalert/metrics/alertmanager_test.go | 34 +++++++++++++++++++ .../ngalert/notifier/alertmanager_test.go | 6 ++-- .../ngalert/notifier/multiorg_alertmanager.go | 6 ++-- 4 files changed, 54 insertions(+), 9 deletions(-) create mode 100644 pkg/services/ngalert/metrics/alertmanager_test.go diff --git a/pkg/services/ngalert/metrics/alertmanager.go b/pkg/services/ngalert/metrics/alertmanager.go index 597694c9785..dca5c187e32 100644 --- a/pkg/services/ngalert/metrics/alertmanager.go +++ b/pkg/services/ngalert/metrics/alertmanager.go @@ -3,6 +3,8 @@ package metrics import ( "fmt" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/prometheus/alertmanager/api/metrics" "github.com/prometheus/client_golang/prometheus" ) @@ -14,12 +16,12 @@ type Alertmanager struct { } // NewAlertmanagerMetrics creates a set of metrics for the Alertmanager of each organization. -func NewAlertmanagerMetrics(r prometheus.Registerer) *Alertmanager { +func NewAlertmanagerMetrics(r prometheus.Registerer, l log.Logger) *Alertmanager { other := prometheus.WrapRegistererWithPrefix(fmt.Sprintf("%s_%s_", Namespace, Subsystem), r) return &Alertmanager{ Registerer: r, Alerts: metrics.NewAlerts(other), - AlertmanagerConfigMetrics: NewAlertmanagerConfigMetrics(r), + AlertmanagerConfigMetrics: NewAlertmanagerConfigMetrics(r, l), } } @@ -32,7 +34,7 @@ type AlertmanagerConfigMetrics struct { ObjectMatchers prometheus.Gauge } -func NewAlertmanagerConfigMetrics(r prometheus.Registerer) *AlertmanagerConfigMetrics { +func NewAlertmanagerConfigMetrics(r prometheus.Registerer, l log.Logger) *AlertmanagerConfigMetrics { m := &AlertmanagerConfigMetrics{ ConfigHash: prometheus.NewGaugeVec(prometheus.GaugeOpts{ Name: "alertmanager_config_hash", @@ -60,7 +62,14 @@ func NewAlertmanagerConfigMetrics(r prometheus.Registerer) *AlertmanagerConfigMe }), } if r != nil { - r.MustRegister(m.ConfigHash, m.ConfigSizeBytes, m.Matchers, m.MatchRE, m.Match, m.ObjectMatchers) + for _, c := range []prometheus.Collector{m.ConfigHash, m.ConfigSizeBytes, m.Matchers, m.MatchRE, m.Match, m.ObjectMatchers} { + // simpler than handling prometheus.AlreadyRegisteredError and replacing collectors + r.Unregister(c) + + if err := r.Register(c); err != nil { + l.Error("Error registering prometheus collector for new alertmanager", "error", err) + } + } } return m } diff --git a/pkg/services/ngalert/metrics/alertmanager_test.go b/pkg/services/ngalert/metrics/alertmanager_test.go new file mode 100644 index 00000000000..b5edf0af12a --- /dev/null +++ b/pkg/services/ngalert/metrics/alertmanager_test.go @@ -0,0 +1,34 @@ +package metrics + +import ( + "testing" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/require" +) + +func Test_NewAlertmanagerConfigMetrics(t *testing.T) { + t.Run("Successfully registers collectors despite previous call", func(t *testing.T) { + r := prometheus.NewRegistry() + l := log.NewNopLogger() + + require.NotPanics(t, func() { + for i := 0; i < 3; i++ { + m := NewAlertmanagerConfigMetrics(r, l) + m.ConfigHash.WithLabelValues("test").Set(1) + m.Matchers.Set(1) + m.MatchRE.Set(1) + m.Match.Set(1) + m.ObjectMatchers.Set(1) + + mf, err := r.Gather() + require.NoError(t, err) + + for j := 0; j < len(mf); j++ { + require.Equal(t, float64(1), mf[j].GetMetric()[0].GetGauge().GetValue()) + } + } + }) + }) +} diff --git a/pkg/services/ngalert/notifier/alertmanager_test.go b/pkg/services/ngalert/notifier/alertmanager_test.go index caabe38bdb6..4f372d0fdda 100644 --- a/pkg/services/ngalert/notifier/alertmanager_test.go +++ b/pkg/services/ngalert/notifier/alertmanager_test.go @@ -31,7 +31,9 @@ func setupAMTest(t *testing.T) *alertmanager { AppURL: "http://localhost:9093", } - m := metrics.NewAlertmanagerMetrics(prometheus.NewRegistry()) + l := log.New("alertmanager-test") + + m := metrics.NewAlertmanagerMetrics(prometheus.NewRegistry(), l) sqlStore := db.InitTestDB(t) s := &store.DBstore{ Cfg: setting.UnifiedAlertingSettings{ @@ -39,7 +41,7 @@ func setupAMTest(t *testing.T) *alertmanager { DefaultRuleEvaluationInterval: time.Minute, }, SQLStore: sqlStore, - Logger: log.New("alertmanager-test"), + Logger: l, DashboardService: dashboards.NewFakeDashboardService(t), } diff --git a/pkg/services/ngalert/notifier/multiorg_alertmanager.go b/pkg/services/ngalert/notifier/multiorg_alertmanager.go index 8e0f89dceb8..464c1036296 100644 --- a/pkg/services/ngalert/notifier/multiorg_alertmanager.go +++ b/pkg/services/ngalert/notifier/multiorg_alertmanager.go @@ -153,7 +153,7 @@ func NewMultiOrgAlertmanager( // Set up the default per tenant Alertmanager factory. moa.factory = func(ctx context.Context, orgID int64) (Alertmanager, error) { - m := metrics.NewAlertmanagerMetrics(moa.metrics.GetOrCreateOrgRegistry(orgID)) + m := metrics.NewAlertmanagerMetrics(moa.metrics.GetOrCreateOrgRegistry(orgID), l) stateStore := NewFileStore(orgID, kvStore) return NewAlertmanager(ctx, orgID, moa.settings, moa.configStore, stateStore, moa.peer, moa.decryptFn, moa.ns, m, featureManager.IsEnabled(ctx, featuremgmt.FlagAlertingSimplifiedRouting)) } @@ -211,7 +211,6 @@ func (moa *MultiOrgAlertmanager) setupClustering(cfg *setting.Cfg) error { true, cfg.UnifiedAlerting.HALabel, ) - if err != nil { return fmt.Errorf("unable to initialize gossip mesh: %w", err) } @@ -358,7 +357,8 @@ func (moa *MultiOrgAlertmanager) SyncAlertmanagersForOrgs(ctx context.Context, o // active organizations. The original intention for this was the cleanup deleted orgs, that have had their states // saved to the kvstore after deletion on instance shutdown. func (moa *MultiOrgAlertmanager) cleanupOrphanLocalOrgState(ctx context.Context, - activeOrganizations map[int64]struct{}) { + activeOrganizations map[int64]struct{}, +) { storedFiles := []string{NotificationLogFilename, SilencesFilename} for _, fileName := range storedFiles { keys, err := moa.kvStore.Keys(ctx, kvstore.AllOrganizations, KVNamespace, fileName)