From ec288eb222bda8cef3622dfd95fbb3878cfdc0cd Mon Sep 17 00:00:00 2001 From: grandwizard28 Date: Thu, 16 Apr 2026 17:45:37 +0530 Subject: [PATCH] refactor(ruler): use ProviderFactory pattern and register in factory.Registry Replace the rulerCallback with rulerProviderFactories following the standard ProviderFactory pattern (like auditorProviderFactories). The ruler is now created via factory.NewProviderFromNamedMap and registered in factory.Registry for lifecycle management. Start/Stop are no longer called manually in server.go. - Ruler interface embeds factory.Service (Start/Stop return error) - signozruler.NewFactory accepts all deps including EE task funcs - provider uses named field (not embedding) with explicit delegation - cmd/community passes nil task funcs, cmd/enterprise passes EE funcs - Remove NewRulerProviderFactories (replaced by callback from cmd/) - Remove manual Start/Stop from both OSS and EE server.go --- cmd/community/server.go | 4 +- cmd/enterprise/server.go | 4 +- ee/query-service/app/server.go | 6 -- pkg/query-service/app/server.go | 6 -- pkg/ruler/ruler.go | 8 +-- pkg/ruler/signozruler/provider.go | 110 ++++++++++++++++++++---------- pkg/signoz/provider.go | 9 +-- pkg/signoz/provider_test.go | 6 -- pkg/signoz/signoz.go | 7 +- 9 files changed, 84 insertions(+), 76 deletions(-) diff --git a/cmd/community/server.go b/cmd/community/server.go index a904001494..7eb0fef3b2 100644 --- a/cmd/community/server.go +++ b/cmd/community/server.go @@ -115,8 +115,8 @@ func runServer(ctx context.Context, config signoz.Config, logger *slog.Logger) e func(_ sqlstore.SQLStore, _ global.Global, _ zeus.Zeus, _ gateway.Gateway, _ licensing.Licensing, _ serviceaccount.Module, _ cloudintegration.Config) (cloudintegration.Module, error) { return implcloudintegration.NewModule(), nil }, - func(config ruler.Config, c cache.Cache, am alertmanager.Alertmanager, ss sqlstore.SQLStore, ts telemetrystore.TelemetryStore, ms telemetrytypes.MetadataStore, p prometheus.Prometheus, og organization.Getter, rsh rulestatehistory.Module, q querier.Querier, ps factory.ProviderSettings, qp queryparser.QueryParser) (ruler.Ruler, error) { - return signozruler.New(config, c, am, ss, ts, ms, p, og, rsh, q, ps, qp, nil, nil) + func(c cache.Cache, am alertmanager.Alertmanager, ss sqlstore.SQLStore, ts telemetrystore.TelemetryStore, ms telemetrytypes.MetadataStore, p prometheus.Prometheus, og organization.Getter, rsh rulestatehistory.Module, q querier.Querier, qp queryparser.QueryParser) factory.NamedMap[factory.ProviderFactory[ruler.Ruler, ruler.Config]] { + return factory.MustNewNamedMap(signozruler.NewFactory(c, am, ss, ts, ms, p, og, rsh, q, qp, nil, nil)) }, ) if err != nil { diff --git a/cmd/enterprise/server.go b/cmd/enterprise/server.go index 0ce3cf3fa4..c05fa4bfb0 100644 --- a/cmd/enterprise/server.go +++ b/cmd/enterprise/server.go @@ -175,8 +175,8 @@ func runServer(ctx context.Context, config signoz.Config, logger *slog.Logger) e return implcloudintegration.NewModule(pkgcloudintegration.NewStore(sqlStore), global, zeus, gateway, licensing, serviceAccount, cloudProvidersMap, config) }, - func(config ruler.Config, c cache.Cache, am alertmanager.Alertmanager, ss sqlstore.SQLStore, ts telemetrystore.TelemetryStore, ms telemetrytypes.MetadataStore, p prometheus.Prometheus, og organization.Getter, rsh rulestatehistory.Module, q querier.Querier, ps factory.ProviderSettings, qp queryparser.QueryParser) (ruler.Ruler, error) { - return signozruler.New(config, c, am, ss, ts, ms, p, og, rsh, q, ps, qp, eerules.PrepareTaskFunc, eerules.TestNotification) + func(c cache.Cache, am alertmanager.Alertmanager, ss sqlstore.SQLStore, ts telemetrystore.TelemetryStore, ms telemetrytypes.MetadataStore, p prometheus.Prometheus, og organization.Getter, rsh rulestatehistory.Module, q querier.Querier, qp queryparser.QueryParser) factory.NamedMap[factory.ProviderFactory[ruler.Ruler, ruler.Config]] { + return factory.MustNewNamedMap(signozruler.NewFactory(c, am, ss, ts, ms, p, og, rsh, q, qp, eerules.PrepareTaskFunc, eerules.TestNotification)) }, ) if err != nil { diff --git a/ee/query-service/app/server.go b/ee/query-service/app/server.go index 0525774722..499ccb27fc 100644 --- a/ee/query-service/app/server.go +++ b/ee/query-service/app/server.go @@ -255,8 +255,6 @@ func (s *Server) initListeners() error { // Start listening on http and private http port concurrently func (s *Server) Start(ctx context.Context) error { - s.signoz.Ruler.Start(ctx) - err := s.initListeners() if err != nil { return err @@ -300,10 +298,6 @@ func (s *Server) Stop(ctx context.Context) error { s.opampServer.Stop() - if s.signoz.Ruler != nil { - s.signoz.Ruler.Stop(ctx) - } - // stop usage manager s.usageManager.Stop(ctx) diff --git a/pkg/query-service/app/server.go b/pkg/query-service/app/server.go index 1501b50579..383c0093a6 100644 --- a/pkg/query-service/app/server.go +++ b/pkg/query-service/app/server.go @@ -238,8 +238,6 @@ func (s *Server) initListeners() error { // Start listening on http and private http port concurrently func (s *Server) Start(ctx context.Context) error { - s.signoz.Ruler.Start(ctx) - err := s.initListeners() if err != nil { return err @@ -283,10 +281,6 @@ func (s *Server) Stop(ctx context.Context) error { s.opampServer.Stop() - if s.signoz.Ruler != nil { - s.signoz.Ruler.Stop(ctx) - } - return nil } diff --git a/pkg/ruler/ruler.go b/pkg/ruler/ruler.go index cc1bf9e970..8fa612c6e3 100644 --- a/pkg/ruler/ruler.go +++ b/pkg/ruler/ruler.go @@ -3,12 +3,14 @@ package ruler import ( "context" + "github.com/SigNoz/signoz/pkg/factory" "github.com/SigNoz/signoz/pkg/statsreporter" "github.com/SigNoz/signoz/pkg/types/ruletypes" "github.com/SigNoz/signoz/pkg/valuer" ) type Ruler interface { + factory.Service statsreporter.StatsCollector // ListRuleStates returns all rules with their current evaluation state. @@ -34,10 +36,4 @@ type Ruler interface { // MaintenanceStore returns the store for planned maintenance / downtime schedules. MaintenanceStore() ruletypes.MaintenanceStore - - // Start begins rule evaluation. Blocks until Stop is called. - Start(ctx context.Context) - - // Stop halts rule evaluation. - Stop(ctx context.Context) } diff --git a/pkg/ruler/signozruler/provider.go b/pkg/ruler/signozruler/provider.go index cdad05e625..949e30e6fa 100644 --- a/pkg/ruler/signozruler/provider.go +++ b/pkg/ruler/signozruler/provider.go @@ -22,18 +22,11 @@ import ( ) type provider struct { - *rules.Manager + manager *rules.Manager ruleStore ruletypes.RuleStore } -func NewFactory(sqlstore sqlstore.SQLStore, queryParser queryparser.QueryParser) factory.ProviderFactory[ruler.Ruler, ruler.Config] { - return factory.NewProviderFactory(factory.MustNewName("signoz"), func(ctx context.Context, settings factory.ProviderSettings, config ruler.Config) (ruler.Ruler, error) { - return &provider{ruleStore: sqlrulestore.NewRuleStore(sqlstore, queryParser, settings)}, nil - }) -} - -func New( - config ruler.Config, +func NewFactory( cache cache.Cache, alertmanager alertmanager.Alertmanager, sqlstore sqlstore.SQLStore, @@ -43,40 +36,51 @@ func New( orgGetter organization.Getter, ruleStateHistoryModule rulestatehistory.Module, querier querier.Querier, - providerSettings factory.ProviderSettings, queryParser queryparser.QueryParser, prepareTaskFunc func(rules.PrepareTaskOptions) (rules.Task, error), prepareTestRuleFunc func(rules.PrepareTestRuleOptions) (int, error), -) (ruler.Ruler, error) { - ruleStore := sqlrulestore.NewRuleStore(sqlstore, queryParser, providerSettings) - maintenanceStore := sqlrulestore.NewMaintenanceStore(sqlstore) +) factory.ProviderFactory[ruler.Ruler, ruler.Config] { + return factory.NewProviderFactory(factory.MustNewName("signoz"), func(ctx context.Context, providerSettings factory.ProviderSettings, config ruler.Config) (ruler.Ruler, error) { + ruleStore := sqlrulestore.NewRuleStore(sqlstore, queryParser, providerSettings) + maintenanceStore := sqlrulestore.NewMaintenanceStore(sqlstore) - managerOpts := &rules.ManagerOptions{ - TelemetryStore: telemetryStore, - MetadataStore: metadataStore, - Prometheus: prometheus, - Context: context.Background(), - Querier: querier, - Logger: providerSettings.Logger, - Cache: cache, - EvalDelay: valuer.MustParseTextDuration(config.EvalDelay.String()), - PrepareTaskFunc: prepareTaskFunc, - PrepareTestRuleFunc: prepareTestRuleFunc, - Alertmanager: alertmanager, - OrgGetter: orgGetter, - RuleStore: ruleStore, - MaintenanceStore: maintenanceStore, - SQLStore: sqlstore, - QueryParser: queryParser, - RuleStateHistoryModule: ruleStateHistoryModule, - } + managerOpts := &rules.ManagerOptions{ + TelemetryStore: telemetryStore, + MetadataStore: metadataStore, + Prometheus: prometheus, + Context: context.Background(), + Querier: querier, + Logger: providerSettings.Logger, + Cache: cache, + EvalDelay: valuer.MustParseTextDuration(config.EvalDelay.String()), + PrepareTaskFunc: prepareTaskFunc, + PrepareTestRuleFunc: prepareTestRuleFunc, + Alertmanager: alertmanager, + OrgGetter: orgGetter, + RuleStore: ruleStore, + MaintenanceStore: maintenanceStore, + SQLStore: sqlstore, + QueryParser: queryParser, + RuleStateHistoryModule: ruleStateHistoryModule, + } - manager, err := rules.NewManager(managerOpts) - if err != nil { - return nil, err - } + manager, err := rules.NewManager(managerOpts) + if err != nil { + return nil, err + } - return &provider{Manager: manager, ruleStore: ruleStore}, nil + return &provider{manager: manager, ruleStore: ruleStore}, nil + }) +} + +func (provider *provider) Start(ctx context.Context) error { + provider.manager.Start(ctx) + return nil +} + +func (provider *provider) Stop(ctx context.Context) error { + provider.manager.Stop(ctx) + return nil } func (provider *provider) Collect(ctx context.Context, orgID valuer.UUID) (map[string]any, error) { @@ -87,3 +91,35 @@ func (provider *provider) Collect(ctx context.Context, orgID valuer.UUID) (map[s return ruletypes.NewStatsFromRules(rules), nil } + +func (provider *provider) ListRuleStates(ctx context.Context) (*ruletypes.GettableRules, error) { + return provider.manager.ListRuleStates(ctx) +} + +func (provider *provider) GetRule(ctx context.Context, id valuer.UUID) (*ruletypes.GettableRule, error) { + return provider.manager.GetRule(ctx, id) +} + +func (provider *provider) CreateRule(ctx context.Context, ruleStr string) (*ruletypes.GettableRule, error) { + return provider.manager.CreateRule(ctx, ruleStr) +} + +func (provider *provider) EditRule(ctx context.Context, ruleStr string, id valuer.UUID) error { + return provider.manager.EditRule(ctx, ruleStr, id) +} + +func (provider *provider) DeleteRule(ctx context.Context, idStr string) error { + return provider.manager.DeleteRule(ctx, idStr) +} + +func (provider *provider) PatchRule(ctx context.Context, ruleStr string, id valuer.UUID) (*ruletypes.GettableRule, error) { + return provider.manager.PatchRule(ctx, ruleStr, id) +} + +func (provider *provider) TestNotification(ctx context.Context, orgID valuer.UUID, ruleStr string) (int, error) { + return provider.manager.TestNotification(ctx, orgID, ruleStr) +} + +func (provider *provider) MaintenanceStore() ruletypes.MaintenanceStore { + return provider.manager.MaintenanceStore() +} diff --git a/pkg/signoz/provider.go b/pkg/signoz/provider.go index e4d4dfb3c1..06dec1946d 100644 --- a/pkg/signoz/provider.go +++ b/pkg/signoz/provider.go @@ -44,9 +44,6 @@ import ( "github.com/SigNoz/signoz/pkg/prometheus/clickhouseprometheus" "github.com/SigNoz/signoz/pkg/querier" "github.com/SigNoz/signoz/pkg/querier/signozquerier" - "github.com/SigNoz/signoz/pkg/queryparser" - "github.com/SigNoz/signoz/pkg/ruler" - "github.com/SigNoz/signoz/pkg/ruler/signozruler" "github.com/SigNoz/signoz/pkg/sharder" "github.com/SigNoz/signoz/pkg/sharder/noopsharder" "github.com/SigNoz/signoz/pkg/sharder/singlesharder" @@ -229,11 +226,7 @@ func NewAlertmanagerProviderFactories(sqlstore sqlstore.SQLStore, orgGetter orga ) } -func NewRulerProviderFactories(sqlstore sqlstore.SQLStore, queryParser queryparser.QueryParser) factory.NamedMap[factory.ProviderFactory[ruler.Ruler, ruler.Config]] { - return factory.MustNewNamedMap( - signozruler.NewFactory(sqlstore, queryParser), - ) -} + func NewEmailingProviderFactories() factory.NamedMap[factory.ProviderFactory[emailing.Emailing, emailing.Config]] { return factory.MustNewNamedMap( diff --git a/pkg/signoz/provider_test.go b/pkg/signoz/provider_test.go index 9001d76e04..73ef84f0ed 100644 --- a/pkg/signoz/provider_test.go +++ b/pkg/signoz/provider_test.go @@ -11,7 +11,6 @@ import ( "github.com/SigNoz/signoz/pkg/instrumentation/instrumentationtest" "github.com/SigNoz/signoz/pkg/modules/organization/implorganization" "github.com/SigNoz/signoz/pkg/modules/user/impluser" - "github.com/SigNoz/signoz/pkg/queryparser" "github.com/SigNoz/signoz/pkg/sqlschema" "github.com/SigNoz/signoz/pkg/sqlschema/sqlschematest" "github.com/SigNoz/signoz/pkg/sqlstore" @@ -64,11 +63,6 @@ func TestNewProviderFactories(t *testing.T) { NewAlertmanagerProviderFactories(sqlstoretest.New(sqlstore.Config{Provider: "sqlite"}, sqlmock.QueryMatcherEqual), orgGetter, notificationManager) }) - assert.NotPanics(t, func() { - queryParser := queryparser.New(instrumentationtest.New().ToProviderSettings()) - NewRulerProviderFactories(sqlstoretest.New(sqlstore.Config{Provider: "sqlite"}, sqlmock.QueryMatcherEqual), queryParser) - }) - assert.NotPanics(t, func() { NewEmailingProviderFactories() }) diff --git a/pkg/signoz/signoz.go b/pkg/signoz/signoz.go index 5c2555c08c..8440e3182f 100644 --- a/pkg/signoz/signoz.go +++ b/pkg/signoz/signoz.go @@ -106,7 +106,7 @@ func New( auditorProviderFactories func(licensing.Licensing) factory.NamedMap[factory.ProviderFactory[auditor.Auditor, auditor.Config]], querierHandlerCallback func(factory.ProviderSettings, querier.Querier, analytics.Analytics) querier.Handler, cloudIntegrationCallback func(sqlstore.SQLStore, global.Global, zeus.Zeus, gateway.Gateway, licensing.Licensing, serviceaccount.Module, cloudintegration.Config) (cloudintegration.Module, error), - rulerCallback func(ruler.Config, cache.Cache, alertmanager.Alertmanager, sqlstore.SQLStore, telemetrystore.TelemetryStore, telemetrytypes.MetadataStore, prometheus.Prometheus, organization.Getter, rulestatehistory.Module, querier.Querier, factory.ProviderSettings, queryparser.QueryParser) (ruler.Ruler, error), + rulerProviderFactories func(cache.Cache, alertmanager.Alertmanager, sqlstore.SQLStore, telemetrystore.TelemetryStore, telemetrytypes.MetadataStore, prometheus.Prometheus, organization.Getter, rulestatehistory.Module, querier.Querier, queryparser.QueryParser) factory.NamedMap[factory.ProviderFactory[ruler.Ruler, ruler.Config]], ) (*SigNoz, error) { // Initialize instrumentation instrumentation, err := instrumentation.New(ctx, config.Instrumentation, version.Info, "signoz") @@ -433,8 +433,8 @@ func New( // Initialize all modules modules := NewModules(sqlstore, tokenizer, emailing, providerSettings, orgGetter, alertmanager, analytics, querier, telemetrystore, telemetryMetadataStore, authNs, authz, cache, queryParser, config, dashboard, userGetter, userRoleStore, serviceAccount, cloudIntegrationModule) - // Initialize ruler via callback (allows EE to set PrepareTaskFunc/PrepareTestRuleFunc) - rulerInstance, err := rulerCallback(config.Ruler, cache, alertmanager, sqlstore, telemetrystore, telemetryMetadataStore, prometheus, orgGetter, modules.RuleStateHistory, querier, providerSettings, queryParser) + // Initialize ruler from the variant-specific provider factories + rulerInstance, err := factory.NewProviderFromNamedMap(ctx, providerSettings, config.Ruler, rulerProviderFactories(cache, alertmanager, sqlstore, telemetrystore, telemetryMetadataStore, prometheus, orgGetter, modules.RuleStateHistory, querier, queryParser), "signoz") if err != nil { return nil, err } @@ -491,6 +491,7 @@ func New( factory.NewNamedService(factory.MustNewName("authz"), authz), factory.NewNamedService(factory.MustNewName("user"), userService, factory.MustNewName("authz")), factory.NewNamedService(factory.MustNewName("auditor"), auditor), + factory.NewNamedService(factory.MustNewName("ruler"), rulerInstance), ) if err != nil { return nil, err