Skip to content

fix(rules): propagate initiate() errors through Manager.Start and ruler provider - #12567

Open
rossigee wants to merge 1 commit into
SigNoz:mainfrom
rossigee:fix/ruler-init-error-propagation
Open

fix(rules): propagate initiate() errors through Manager.Start and ruler provider#12567
rossigee wants to merge 1 commit into
SigNoz:mainfrom
rossigee:fix/ruler-init-error-propagation

Conversation

@rossigee

Copy link
Copy Markdown

Summary

pkg/query-service/rules/manager.go Manager.Start swallowed initiate() errors and proceeded to run() anyway, leaving the ruler in a broken state with no signal in the registry logs. pkg/ruler/signozruler/provider.go compounded the problem by calling provider.manager.Start(ctx) without inspecting the return value.

  • Manager.Start now returns error and surfaces the initiate failure instead of logging-and-continuing
  • signozruler.provider.Start propagates that error so the factory registry marks the ruler service as StateFailed and exits, consistent with every other Start error path

Closes #12565

Changes

  • pkg/query-service/rules/manager.goStart(ctx) now returns error; initiate failure short-circuits before run
  • pkg/ruler/signozruler/provider.go — capture and return manager.Start error from provider.Start

Verification

  • go build ./... — clean
  • golangci-lint run ./pkg/query-service/rules/... ./pkg/ruler/... ./pkg/factory/... ./pkg/alertmanager/... — 0 issues
  • go test -race ./pkg/query-service/rules/... ./pkg/ruler/... ./pkg/factory/... — passing

Risk

The Manager.Start return signature changes from void to error. Tree-wide audit (grep -rnE "\\.Manager\\.Start\\b|manager\\.Start\\b" pkg/ ee/ cmd/) confirms pkg/ruler/signozruler/provider.go:81 is the only caller — already updated in this commit. No external consumers of *rules.Manager exist outside pkg/ruler/.

Follow-up (out of scope)

pkg/query-service/rules/manager_test.go exercises NewManager and rule evaluation but does not exercise Start failure. A regression test that stubs dependencies to fail initiate and asserts Start(ctx) != nil would lock in this behaviour.

Manager.Start() previously logged initiate() failures and continued to
run() anyway, leaving the ruler in a broken state. Ruler provider.Start
also ignored any returned error.

- Manager.Start now returns the initiate() error instead of swallowing it
- Ruler provider propagates the error through Start() so the registry
  can mark the service as failed and exit cleanly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rules: Manager.Start swallows initiate() errors and proceeds to run() in a broken state

1 participant