Do not emit partition_remove_hook and CDR compaction hook metrics whe…

[dmitry-sinina]

…n hooks are disabled

[Copilot]

Pull request overview

Updates Prometheus hook collectors so they don’t export permanently-zero metric series when the corresponding optional shell hooks are not configured, preventing false “hook has not run” alerts for disabled features.

Changes:

• Add HookConfig.configured? helper to detect whether a hook command is configured in YetiConfig.
• Gate “seed zeros at process start” behavior in PartitionRemoveHookCollector and CdrCompactionHookCollector behind hook configuration.
• Extend collectors’ specs and add new HookConfig specs to cover configured/unconfigured behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
spec/lib/prometheus/partition_remove_hook_collector_spec.rb	Adds coverage ensuring no metrics are exported when the hook is unconfigured (but still records reported data).
spec/lib/prometheus/hook_config_spec.rb	New unit tests for hook configuration detection, including error-path behavior.
spec/lib/prometheus/cdr_compaction_hook_collector_spec.rb	Mirrors partition-remove tests for the CDR compaction hook collector.
lib/prometheus/partition_remove_hook_collector.rb	Adds seed_zeros: parameter and skips zero seeding when the hook is not configured.
lib/prometheus/hook_config.rb	Introduces centralized hook configuration detection used by collectors.
lib/prometheus/cdr_compaction_hook_collector.rb	Adds seed_zeros: parameter and skips zero seeding when the hook is not configured.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (4)

lib/prometheus/partition_remove_hook_collector.rb:5

• collector_labels.rb already requires ../prometheus_config, so this additional require is redundant here and can be removed to avoid duplicated load paths and keep dependencies centralized.

require_relative './collector_labels'
require_relative '../prometheus_config'

lib/prometheus/cdr_compaction_hook_collector.rb:5

• collector_labels.rb already requires ../prometheus_config, so this additional require is redundant here and can be removed to avoid duplicated load paths and keep dependencies centralized.

require_relative './collector_labels'
require_relative '../prometheus_config'

spec/lib/prometheus_config_spec.rb:11

• This example currently relies on the real config/yeti_web.yml default (hooks commented out) to make the unconfigured case false. That makes the spec brittle if defaults change or the test config differs. Stub the config key explicitly so the example remains self-contained.

    it 'is false when the hook is not configured' do
      expect(subject).to be(false)
    end

lib/yeti_config_loader.rb:30

• YetiConfigLoader.call checks File.exist?, which also returns true for directories. If a directory is accidentally passed, the subsequent Config.load_and_set_settings will fail with a less clear error. Use File.file? to ensure the path is an actual file before proceeding.

    # Config.load_and_set_settings does not object to a path that does not exist: it defines an
    # empty YetiConfig and returns, so the absence has to be caught here. Checked before
    # Config.setup so a failure leaves no half-applied global configuration behind.
    raise Error, "config file not found: #{path}" unless File.exist?(path)

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.