Merge branch '350546-puma-metrics-server-support' into 'master'

Allow `metrics_server` to target Puma

See merge request gitlab-org/gitlab!78527

lib/gitlab/daemon.rb

@@ -20,6 +20,8 @@ module Gitlab
       !thread.nil?
     end
 
+    # Possible options:
+    # - synchronous [Boolean] if true, turns `start` into a blocking call
     def initialize(**options)
       @synchronous = options[:synchronous]
       @mutex = Mutex.new

lib/gitlab/metrics/exporter/base_exporter.rb

@@ -9,6 +9,10 @@ module Gitlab
       class BaseExporter < Daemon
         attr_reader :server
 
+        # @param settings [Hash] SettingsLogic hash containing the `*_exporter` config
+        # @param log_enabled [Boolean] whether to log HTTP requests
+        # @param log_file [String] path to where the server log should be located
+        # @param gc_requests [Boolean] whether to run a major GC after each scraper request
         def initialize(settings, log_enabled:, log_file:, gc_requests: false, **options)
           super(**options)
 

metrics_server/dependencies.rb

@@ -23,6 +23,7 @@ require_relative '../lib/gitlab/metrics/system'
 require_relative '../lib/gitlab/metrics/samplers/base_sampler'
 require_relative '../lib/gitlab/metrics/samplers/ruby_sampler'
 require_relative '../lib/gitlab/metrics/exporter/base_exporter'
+require_relative '../lib/gitlab/metrics/exporter/web_exporter'
 require_relative '../lib/gitlab/metrics/exporter/sidekiq_exporter'
 require_relative '../lib/gitlab/metrics/exporter/metrics_middleware'
 require_relative '../lib/gitlab/metrics/exporter/health_checks_middleware'

metrics_server/metrics_server.rb

@@ -7,7 +7,7 @@ require_relative 'dependencies'
 class MetricsServer # rubocop:disable Gitlab/NamespacedClass
   class << self
     def spawn(target, metrics_dir:, wipe_metrics_dir: false, trapped_signals: [])
-      raise "The only valid target is 'sidekiq' currently" unless target == 'sidekiq'
+      raise "Target must be one of [puma,sidekiq]" unless %w(puma sidekiq).include?(target)
 
       pid = Process.fork
 
@@ -52,11 +52,18 @@ class MetricsServer # rubocop:disable Gitlab/NamespacedClass
     # Warming up ensures that these files exist prior to the exporter starting up.
     Gitlab::Metrics::Samplers::RubySampler.initialize_instance(prefix: name, warmup: true).start
 
+    default_opts = { gc_requests: true, synchronous: true }
+    exporter =
+      case @target
+      when 'puma'
+        Gitlab::Metrics::Exporter::WebExporter.instance(**default_opts)
+      else
         exporter_class = "Gitlab::Metrics::Exporter::#{@target.camelize}Exporter".constantize
         settings = Settings.new(Settings.monitoring[name])
-    server = exporter_class.instance(settings, gc_requests: true, synchronous: true)
+        exporter_class.instance(settings, **default_opts)
+      end
 
-    server.start
+    exporter.start
   end
 
   def name

spec/commands/metrics_server/metrics_server_spec.rb

@@ -12,6 +12,11 @@ RSpec.describe 'bin/metrics-server', :aggregate_failures do
     {
       'test' => {
         'monitoring' => {
+          'web_exporter' => {
+            'address' => 'localhost',
+            'enabled' => true,
+            'port' => 3807
+          },
           'sidekiq_exporter' => {
             'address' => 'localhost',
             'enabled' => true,
@@ -22,7 +27,8 @@ RSpec.describe 'bin/metrics-server', :aggregate_failures do
     }
   end
 
-  context 'with a running server' do
+  %w(puma sidekiq).each do |target|
+    context "with a running server targeting #{target}" do
       let(:metrics_dir) { Dir.mktmpdir }
 
       before do
@@ -34,7 +40,7 @@ RSpec.describe 'bin/metrics-server', :aggregate_failures do
 
         env = {
           'GITLAB_CONFIG' => config_file.path,
-        'METRICS_SERVER_TARGET' => 'sidekiq',
+          'METRICS_SERVER_TARGET' => target,
           'WIPE_METRICS_DIR' => '1',
           'prometheus_multiproc_dir' => metrics_dir
         }
@@ -74,4 +80,5 @@ RSpec.describe 'bin/metrics-server', :aggregate_failures do
         end.not_to raise_error
       end
     end
+  end
 end

spec/metrics_server/metrics_server_spec.rb

@@ -15,9 +15,16 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath
   # we need to reset it after testing.
   let!(:old_multiprocess_files_dir) { prometheus_config.multiprocess_files_dir }
 
+  let(:ruby_sampler_double) { double(Gitlab::Metrics::Samplers::RubySampler) }
+
   before do
     # We do not want this to have knock-on effects on the test process.
     allow(Gitlab::ProcessManagement).to receive(:modify_signals)
+
+    # This being a singleton, we stub it out because only one instance is allowed
+    # to exist per process.
+    allow(Gitlab::Metrics::Samplers::RubySampler).to receive(:initialize_instance).and_return(ruby_sampler_double)
+    allow(ruby_sampler_double).to receive(:start)
   end
 
   after do
@@ -27,13 +34,15 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath
     FileUtils.rm_rf(metrics_dir, secure: true)
   end
 
+  %w(puma sidekiq).each do |target|
+    context "when targeting #{target}" do
       describe '.spawn' do
         context 'when in parent process' do
           it 'forks into a new process and detaches it' do
             expect(Process).to receive(:fork).and_return(99)
             expect(Process).to receive(:detach).with(99)
 
-        described_class.spawn('sidekiq', metrics_dir: metrics_dir)
+            described_class.spawn(target, metrics_dir: metrics_dir)
           end
         end
 
@@ -49,13 +58,25 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath
               expect(server).to receive(:start)
             end
 
-        described_class.spawn('sidekiq', metrics_dir: metrics_dir)
+            described_class.spawn(target, metrics_dir: metrics_dir)
           end
 
           it 'resets signal handlers from parent process' do
             expect(Gitlab::ProcessManagement).to receive(:modify_signals).with(%i[A B], 'DEFAULT')
 
-        described_class.spawn('sidekiq', metrics_dir: metrics_dir, trapped_signals: %i[A B])
+            described_class.spawn(target, metrics_dir: metrics_dir, trapped_signals: %i[A B])
+          end
+        end
+      end
+    end
+  end
+
+  context 'when targeting invalid target' do
+    describe '.spawn' do
+      it 'raises an error' do
+        expect { described_class.spawn('unsupported', metrics_dir: metrics_dir) }.to(
+          raise_error('Target must be one of [puma,sidekiq]')
+        )
       end
     end
   end
@@ -64,7 +85,6 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath
     let(:exporter_class) { Class.new(Gitlab::Metrics::Exporter::BaseExporter) }
     let(:exporter_double) { double('fake_exporter', start: true) }
     let(:settings) { { "fake_exporter" => { "enabled" => true } } }
-    let(:ruby_sampler_double) { double(Gitlab::Metrics::Samplers::RubySampler) }
 
     subject(:metrics_server) { described_class.new('fake', metrics_dir, true)}
 
@@ -74,9 +94,6 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath
         settings['fake_exporter'], gc_requests: true, synchronous: true
       ).and_return(exporter_double)
       expect(Settings).to receive(:monitoring).and_return(settings)
-
-      allow(Gitlab::Metrics::Samplers::RubySampler).to receive(:initialize_instance).and_return(ruby_sampler_double)
-      allow(ruby_sampler_double).to receive(:start)
     end
 
     it 'configures ::Prometheus::Client' do