Merge branch '345802-sk-health-check-settings' into 'master'

Allow instantiating SidekiqExporter with settings

See merge request gitlab-org/gitlab!74875

config/gitlab.yml.example

@@ -1260,6 +1260,12 @@ production: &base
     #  address: localhost
     #  port: 8082
 
+    sidekiq_health_checks:
+    #  enabled: true
+    #  log_enabled: false
+    #  address: localhost
+    #  port: 8082
+
     # Web exporter is a dedicated Rack server running alongside Puma to expose Prometheus metrics
     # It runs alongside the `/metrics` endpoints to ease the publish of metrics
     web_exporter:

config/initializers/1_settings.rb

@@ -919,11 +919,23 @@ Settings.webpack.dev_server['https']   ||= false
 #
 Settings['monitoring'] ||= Settingslogic.new({})
 Settings.monitoring['ip_whitelist'] ||= ['127.0.0.1/8']
+
 Settings.monitoring['sidekiq_exporter'] ||= Settingslogic.new({})
 Settings.monitoring.sidekiq_exporter['enabled'] ||= false
 Settings.monitoring.sidekiq_exporter['log_enabled'] ||= false
 Settings.monitoring.sidekiq_exporter['address'] ||= 'localhost'
 Settings.monitoring.sidekiq_exporter['port'] ||= 8082
+
+# TODO: Once we split out health checks from SidekiqExporter, we
+# should not let this default to the same settings anymore; we only
+# do this for back-compat currently.
+# https://gitlab.com/gitlab-org/gitlab/-/issues/345804
+Settings.monitoring['sidekiq_health_checks'] ||= Settingslogic.new({})
+Settings.monitoring.sidekiq_health_checks['enabled'] ||= Settings.monitoring.sidekiq_exporter['enabled']
+Settings.monitoring.sidekiq_health_checks['log_enabled'] ||= Settings.monitoring.sidekiq_exporter['log_enabled']
+Settings.monitoring.sidekiq_health_checks['address'] ||= Settings.monitoring.sidekiq_exporter['address']
+Settings.monitoring.sidekiq_health_checks['port'] ||= Settings.monitoring.sidekiq_exporter['port']
+
 Settings.monitoring['web_exporter'] ||= Settingslogic.new({})
 Settings.monitoring.web_exporter['enabled'] ||= false
 Settings.monitoring.web_exporter['address'] ||= 'localhost'

config/initializers/7_prometheus_metrics.rb

@@ -31,11 +31,17 @@ end
 if Gitlab::Runtime.sidekiq? && (!ENV['SIDEKIQ_WORKER_ID'] || ENV['SIDEKIQ_WORKER_ID'] == '0')
   # The single worker outside of a sidekiq-cluster, or the first worker (sidekiq_0)
   # in a cluster of processes, is responsible for serving health checks.
+  #
+  # Do not clean the metrics directory here - the supervisor script should
+  # have already taken care of that.
   Sidekiq.configure_server do |config|
     config.on(:startup) do
-      # Do not clean the metrics directory here - the supervisor script should
-      # have already taken care of that
-      Gitlab::Metrics::Exporter::SidekiqExporter.instance.start
+      # In https://gitlab.com/gitlab-org/gitlab/-/issues/345804 we are looking to
+      # only serve health-checks from a worker process; for backwards compatibility
+      # we still go through the metrics exporter server, but start to configure it
+      # with the new settings keys.
+      exporter_settings = Settings.monitoring.sidekiq_health_checks
+      Gitlab::Metrics::Exporter::SidekiqExporter.instance(exporter_settings).start
     end
   end
 end

lib/gitlab/metrics/exporter/base_exporter.rb

@@ -11,12 +11,14 @@ module Gitlab
 
         attr_accessor :readiness_checks
 
-        def enabled?
-          settings.enabled
+        def initialize(settings, **options)
+          super(**options)
+
+          @settings = settings
         end
 
-        def settings
-          raise NotImplementedError
+        def enabled?
+          settings.enabled
         end
 
         def log_filename
@@ -25,6 +27,8 @@ module Gitlab
 
         private
 
+        attr_reader :settings
+
         def start_working
           logger = WEBrick::Log.new(log_filename)
           logger.time_format = "[%Y-%m-%dT%H:%M:%S.%L%z]"

lib/gitlab/metrics/exporter/sidekiq_exporter.rb

@@ -4,10 +4,6 @@ module Gitlab
   module Metrics
     module Exporter
       class SidekiqExporter < BaseExporter
-        def settings
-          Settings.monitoring.sidekiq_exporter
-        end
-
         def log_filename
           if settings['log_enabled']
             File.join(Rails.root, 'log', 'sidekiq_exporter.log')

lib/gitlab/metrics/exporter/web_exporter.rb

@@ -27,7 +27,7 @@ module Gitlab
 
         # This exporter is always run on master process
         def initialize
-          super
+          super(Settings.monitoring.web_exporter)
 
           # DEPRECATED:
           # these `readiness_checks` are deprecated
@@ -39,10 +39,6 @@ module Gitlab
           ]
         end
 
-        def settings
-          Gitlab.config.monitoring.web_exporter
-        end
-
         def log_filename
           File.join(Rails.root, 'log', 'web_exporter.log')
         end

spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb

@@ -3,9 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Metrics::Exporter::BaseExporter do
-  let(:exporter) { described_class.new }
-  let(:log_filename) { File.join(Rails.root, 'log', 'sidekiq_exporter.log') }
   let(:settings) { double('settings') }
+  let(:exporter) { described_class.new(settings) }
+  let(:log_filename) { File.join(Rails.root, 'log', 'sidekiq_exporter.log') }
 
   before do
     allow_any_instance_of(described_class).to receive(:log_filename).and_return(log_filename)

spec/lib/gitlab/metrics/exporter/sidekiq_exporter_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Metrics::Exporter::SidekiqExporter do
-  let(:exporter) { described_class.new }
+  let(:exporter) { described_class.new(Settings.monitoring.sidekiq_exporter) }
 
   after do
     exporter.stop