1 files changed, 142 insertions, 61 deletions

diff --git a/spec/metrics_server/metrics_server_spec.rb b/spec/metrics_server/metrics_server_spec.rb
index fc18df9b5cd..860a3299d85 100644
--- a/spec/metrics_server/metrics_server_spec.rb
+++ b/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,101 +34,175 @@ RSpec.describe MetricsServer do # rubocop:disable RSpec/FilePath
     FileUtils.rm_rf(metrics_dir, secure: true)
   end
 
-  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)
+  %w(puma sidekiq).each do |target|
+    context "when targeting #{target}" do
+      describe '.fork' 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)
-      end
-    end
+            described_class.fork(target, metrics_dir: metrics_dir)
+          end
+        end
+
+        context 'when in child process' do
+          before do
+            # This signals the process that it's "inside" the fork
+            expect(Process).to receive(:fork).and_return(nil)
+            expect(Process).not_to receive(:detach)
+          end
 
-    context 'when in child process' do
-      before do
-        # This signals the process that it's "inside" the fork
-        expect(Process).to receive(:fork).and_return(nil)
-        expect(Process).not_to receive(:detach)
+          it 'starts the metrics server with the given arguments' do
+            expect_next_instance_of(MetricsServer) do |server|
+              expect(server).to receive(:start)
+            end
+
+            described_class.fork(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.fork(target, metrics_dir: metrics_dir, reset_signals: %i[A B])
+          end
+        end
       end
 
-      it 'starts the metrics server with the given arguments' do
-        expect_next_instance_of(MetricsServer) do |server|
-          expect(server).to receive(:start)
+      describe '.spawn' do
+        let(:expected_env) do
+          {
+            'METRICS_SERVER_TARGET' => target,
+            'WIPE_METRICS_DIR' => '0'
+          }
+        end
+
+        it 'spawns a new server process and returns its PID' do
+          expect(Process).to receive(:spawn).with(
+            expected_env,
+            end_with('bin/metrics-server'),
+            hash_including(pgroup: true)
+          ).and_return(99)
+          expect(Process).to receive(:detach).with(99)
+
+          pid = described_class.spawn(target, metrics_dir: metrics_dir)
+
+          expect(pid).to eq(99)
         end
 
-        described_class.spawn('sidekiq', metrics_dir: metrics_dir)
+        context 'when path to gitlab.yml is passed' do
+          it 'sets the GITLAB_CONFIG environment variable' do
+            expect(Process).to receive(:spawn).with(
+              expected_env.merge('GITLAB_CONFIG' => 'path/to/config/gitlab.yml'),
+              end_with('bin/metrics-server'),
+              hash_including(pgroup: true)
+            ).and_return(99)
+
+            described_class.spawn(target, metrics_dir: metrics_dir, gitlab_config: 'path/to/config/gitlab.yml')
+          end
+        end
       end
+    end
+  end
 
-      it 'resets signal handlers from parent process' do
-        expect(Gitlab::ProcessManagement).to receive(:modify_signals).with(%i[A B], 'DEFAULT')
+  context 'when targeting invalid target' do
+    describe '.fork' do
+      it 'raises an error' do
+        expect { described_class.fork('unsupported', metrics_dir: metrics_dir) }.to(
+          raise_error('Target must be one of [puma,sidekiq]')
+        )
+      end
+    end
 
-        described_class.spawn('sidekiq', metrics_dir: metrics_dir, trapped_signals: %i[A B])
+    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
 
-  describe '#start' do
-    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) }
+  shared_examples 'a metrics exporter' do |target, expected_name|
+    describe '#start' do
+      let(:exporter_double) { double('exporter', start: true) }
+      let(:wipe_metrics_dir) { true }
 
-    subject(:metrics_server) { described_class.new('fake', metrics_dir, true)}
+      subject(:metrics_server) { described_class.new(target, metrics_dir, wipe_metrics_dir) }
 
-    before do
-      stub_const('Gitlab::Metrics::Exporter::FakeExporter', exporter_class)
-      expect(exporter_class).to receive(:instance).with(
-        settings['fake_exporter'], gc_requests: true, synchronous: true
-      ).and_return(exporter_double)
-      expect(Settings).to receive(:monitoring).and_return(settings)
+      it 'configures ::Prometheus::Client' do
+        metrics_server.start
 
-      allow(Gitlab::Metrics::Samplers::RubySampler).to receive(:initialize_instance).and_return(ruby_sampler_double)
-      allow(ruby_sampler_double).to receive(:start)
-    end
+        expect(prometheus_config.multiprocess_files_dir).to eq metrics_dir
+        expect(::Prometheus::Client.configuration.pid_provider.call).to eq expected_name
+      end
 
-    it 'configures ::Prometheus::Client' do
-      metrics_server.start
+      it 'ensures that metrics directory exists in correct mode (0700)' do
+        expect(FileUtils).to receive(:mkdir_p).with(metrics_dir, mode: 0700)
 
-      expect(prometheus_config.multiprocess_files_dir).to eq metrics_dir
-      expect(::Prometheus::Client.configuration.pid_provider.call).to eq 'fake_exporter'
-    end
+        metrics_server.start
+      end
 
-    it 'ensures that metrics directory exists in correct mode (0700)' do
-      expect(FileUtils).to receive(:mkdir_p).with(metrics_dir, mode: 0700)
+      context 'when wipe_metrics_dir is true' do
+        it 'removes any old metrics files' do
+          FileUtils.touch("#{metrics_dir}/remove_this.db")
 
-      metrics_server.start
-    end
+          expect { metrics_server.start }.to change { Dir.empty?(metrics_dir) }.from(false).to(true)
+        end
+      end
 
-    context 'when wipe_metrics_dir is true' do
-      subject(:metrics_server) { described_class.new('fake', metrics_dir, true)}
+      context 'when wipe_metrics_dir is false' do
+        let(:wipe_metrics_dir) { false }
 
-      it 'removes any old metrics files' do
-        FileUtils.touch("#{metrics_dir}/remove_this.db")
+        it 'does not remove any old metrics files' do
+          FileUtils.touch("#{metrics_dir}/remove_this.db")
 
-        expect { metrics_server.start }.to change { Dir.empty?(metrics_dir) }.from(false).to(true)
+          expect { metrics_server.start }.not_to change { Dir.empty?(metrics_dir) }.from(false)
+        end
       end
-    end
 
-    context 'when wipe_metrics_dir is false' do
-      subject(:metrics_server) { described_class.new('fake', metrics_dir, false)}
+      it 'starts a metrics server' do
+        expect(exporter_double).to receive(:start)
+
+        metrics_server.start
+      end
 
-      it 'does not remove any old metrics files' do
-        FileUtils.touch("#{metrics_dir}/remove_this.db")
+      it 'starts a RubySampler instance' do
+        expect(ruby_sampler_double).to receive(:start)
 
-        expect { metrics_server.start }.not_to change { Dir.empty?(metrics_dir) }.from(false)
+        subject.start
       end
     end
 
-    it 'starts a metrics server' do
-      expect(exporter_double).to receive(:start)
+    describe '#name' do
+      let(:exporter_double) { double('exporter', start: true) }
 
-      metrics_server.start
+      subject(:name) { described_class.new(target, metrics_dir, true).name }
+
+      it { is_expected.to eq(expected_name) }
     end
+  end
 
-    it 'starts a RubySampler instance' do
-      expect(ruby_sampler_double).to receive(:start)
+  context 'for puma' do
+    before do
+      allow(Gitlab::Metrics::Exporter::WebExporter).to receive(:instance).with(
+        gc_requests: true, synchronous: true
+      ).and_return(exporter_double)
+    end
 
-      subject.start
+    it_behaves_like 'a metrics exporter', 'puma', 'web_exporter'
+  end
+
+  context 'for sidekiq' do
+    let(:settings) { { "sidekiq_exporter" => { "enabled" => true } } }
+
+    before do
+      allow(::Settings).to receive(:monitoring).and_return(settings)
+      allow(Gitlab::Metrics::Exporter::SidekiqExporter).to receive(:instance).with(
+        settings['sidekiq_exporter'], gc_requests: true, synchronous: true
+      ).and_return(exporter_double)
     end
+
+    it_behaves_like 'a metrics exporter', 'sidekiq', 'sidekiq_exporter'
   end
 end