diff --git a/lib/gitlab/daemon.rb b/lib/gitlab/daemon.rb
index 058fe1c8139263b4558053723bfbdef6e437b763..5f579f90629324101009a0095a35f60df4ecf238 100644
--- a/lib/gitlab/daemon.rb
+++ b/lib/gitlab/daemon.rb
@@ -20,6 +20,8 @@ def thread?
!thread.nil?
end
+ # Possible options:
+ # - synchronous [Boolean] if true, turns `start` into a blocking call
def initialize(**options)
@synchronous = options[:synchronous]
@mutex = Mutex.new
diff --git a/lib/gitlab/metrics/exporter/base_exporter.rb b/lib/gitlab/metrics/exporter/base_exporter.rb
index 4874ef62e4154b71024c0255ddde5a867d70b29d..2aea8d655fa1ff3f4b8960217842c146e2461e15 100644
--- a/lib/gitlab/metrics/exporter/base_exporter.rb
+++ b/lib/gitlab/metrics/exporter/base_exporter.rb
@@ -9,6 +9,10 @@ module Exporter
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)
diff --git a/metrics_server/dependencies.rb b/metrics_server/dependencies.rb
index 5615cef42cebfd65cf3e1e363a771294f932d9ab..02cec1173e0ca1a2660183b3df55b7d0e85e11f8 100644
--- a/metrics_server/dependencies.rb
+++ b/metrics_server/dependencies.rb
@@ -23,6 +23,7 @@
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'
diff --git a/metrics_server/metrics_server.rb b/metrics_server/metrics_server.rb
index 68d39eaf77c2deba7846389b8a54ae7177b87ea3..f1bcd6f611eedc239f1953766379398640978ef3 100644
--- a/metrics_server/metrics_server.rb
+++ b/metrics_server/metrics_server.rb
@@ -7,7 +7,7 @@
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 @@ def start
# Warming up ensures that these files exist prior to the exporter starting up.
Gitlab::Metrics::Samplers::RubySampler.initialize_instance(prefix: name, warmup: true).start
- 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)
+ 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])
+ exporter_class.instance(settings, **default_opts)
+ end
- server.start
+ exporter.start
end
def name
diff --git a/spec/commands/metrics_server/metrics_server_spec.rb b/spec/commands/metrics_server/metrics_server_spec.rb
index 0178017ee371a22abc7851ba050d196f6a03e063..3b92c3c38fab6170040666b8a8db14083780d1b7 100644
--- a/spec/commands/metrics_server/metrics_server_spec.rb
+++ b/spec/commands/metrics_server/metrics_server_spec.rb
@@ -12,6 +12,11 @@
{
'test' => {
'monitoring' => {
+ 'web_exporter' => {
+ 'address' => 'localhost',
+ 'enabled' => true,
+ 'port' => 3807
+ },
'sidekiq_exporter' => {
'address' => 'localhost',
'enabled' => true,
@@ -22,56 +27,58 @@
}
end
- context 'with a running server' do
- let(:metrics_dir) { Dir.mktmpdir }
+ %w(puma sidekiq).each do |target|
+ context "with a running server targeting #{target}" do
+ let(:metrics_dir) { Dir.mktmpdir }
- before do
- # We need to send a request to localhost
- WebMock.allow_net_connect!
+ before do
+ # We need to send a request to localhost
+ WebMock.allow_net_connect!
- config_file.write(YAML.dump(config))
- config_file.close
+ config_file.write(YAML.dump(config))
+ config_file.close
- env = {
- 'GITLAB_CONFIG' => config_file.path,
- 'METRICS_SERVER_TARGET' => 'sidekiq',
- 'WIPE_METRICS_DIR' => '1',
- 'prometheus_multiproc_dir' => metrics_dir
- }
- @pid = Process.spawn(env, 'bin/metrics-server', pgroup: true)
- end
+ env = {
+ 'GITLAB_CONFIG' => config_file.path,
+ 'METRICS_SERVER_TARGET' => target,
+ 'WIPE_METRICS_DIR' => '1',
+ 'prometheus_multiproc_dir' => metrics_dir
+ }
+ @pid = Process.spawn(env, 'bin/metrics-server', pgroup: true)
+ end
- after do
- webmock_enable!
+ after do
+ webmock_enable!
- if @pid
- pgrp = Process.getpgid(@pid)
+ if @pid
+ pgrp = Process.getpgid(@pid)
- Timeout.timeout(10) do
- Process.kill('TERM', -pgrp)
- Process.waitpid(@pid)
- end
+ Timeout.timeout(10) do
+ Process.kill('TERM', -pgrp)
+ Process.waitpid(@pid)
+ end
- expect(Gitlab::ProcessManagement.process_alive?(@pid)).to be(false)
+ expect(Gitlab::ProcessManagement.process_alive?(@pid)).to be(false)
+ end
+ rescue Errno::ESRCH => _
+ # 'No such process' means the process died before
+ ensure
+ config_file.unlink
+ FileUtils.rm_rf(metrics_dir, secure: true)
end
- rescue Errno::ESRCH => _
- # 'No such process' means the process died before
- ensure
- config_file.unlink
- FileUtils.rm_rf(metrics_dir, secure: true)
- end
- it 'serves /metrics endpoint' do
- expect do
- Timeout.timeout(10) do
- http_ok = false
- until http_ok
- sleep 1
- response = Gitlab::HTTP.try_get("http://localhost:3807/metrics", allow_local_requests: true)
- http_ok = response&.success?
+ it 'serves /metrics endpoint' do
+ expect do
+ Timeout.timeout(10) do
+ http_ok = false
+ until http_ok
+ sleep 1
+ response = Gitlab::HTTP.try_get("http://localhost:3807/metrics", allow_local_requests: true)
+ http_ok = response&.success?
+ end
end
- end
- end.not_to raise_error
+ end.not_to raise_error
+ end
end
end
end
diff --git a/spec/metrics_server/metrics_server_spec.rb b/spec/metrics_server/metrics_server_spec.rb
index fc18df9b5cda65dcbfc51066b09d305549357d30..c90fd162814f49f4e2a2fb35f3a9839e6137decd 100644
--- a/spec/metrics_server/metrics_server_spec.rb
+++ b/spec/metrics_server/metrics_server_spec.rb
@@ -15,9 +15,16 @@
# 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,35 +34,49 @@
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 '.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)
- end
- end
+ described_class.spawn(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)
+ end
- it 'starts the metrics server with the given arguments' do
- expect_next_instance_of(MetricsServer) do |server|
- expect(server).to receive(:start)
- end
+ 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.spawn('sidekiq', metrics_dir: metrics_dir)
- end
+ 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')
+ 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 @@
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 @@
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