diff --git a/metrics_server/metrics_server.rb b/metrics_server/metrics_server.rb
index a324b034b5e5e7264ded1c0bfeac4a8c2a2f6b60..269d03f6d0c1a3c2e0a241aeac9d46b827e31e4a 100644
--- a/metrics_server/metrics_server.rb
+++ b/metrics_server/metrics_server.rb
@@ -38,8 +38,7 @@ def start_for_sidekiq(**options)
def spawn(target, metrics_dir:, **options)
return spawn_ruby_server(target, metrics_dir: metrics_dir, **options) unless new_metrics_server?
- name = settings_key(target)
- settings = ::Settings.monitoring[name]
+ settings = settings_value(target)
path = options[:path]&.then { |p| Pathname.new(p) } || Pathname.new('')
cmd = path.join('gitlab-metrics-exporter').to_path
env = {
@@ -51,7 +50,7 @@ def spawn(target, metrics_dir:, **options)
}
if settings['log_enabled']
- env['GME_LOG_FILE'] = File.join(Rails.root, 'log', "#{name}.log")
+ env['GME_LOG_FILE'] = File.join(Rails.root, 'log', "#{name(target)}.log")
env['GME_LOG_LEVEL'] = 'info'
else
env['GME_LOG_LEVEL'] = 'quiet'
@@ -106,8 +105,27 @@ def fork(target, metrics_dir:, wipe_metrics_dir: false, reset_signals: [])
pid
end
+ def name(target)
+ case target
+ when 'puma' then 'web_exporter'
+ when 'sidekiq' then 'sidekiq_exporter'
+ else ensure_valid_target!(target)
+ end
+ end
+
private
+ # We need to use `.` (dot) notation to access the updates we did in `config/initializers/1_settings.rb`
+ # For that reason, avoid using `[]` ("optional/dynamic settings notation") to resolve it dynamically.
+ # Refer to https://gitlab.com/gitlab-org/gitlab/-/issues/386865
+ def settings_value(target)
+ case target
+ when 'puma' then ::Settings.monitoring.web_exporter
+ when 'sidekiq' then ::Settings.monitoring.sidekiq_exporter
+ else ensure_valid_target!(target)
+ end
+ end
+
def new_metrics_server?
Gitlab::Utils.to_boolean(ENV['GITLAB_GOLANG_METRICS_SERVER'])
end
@@ -115,14 +133,6 @@ def new_metrics_server?
def ensure_valid_target!(target)
raise "Target must be one of [puma,sidekiq]" unless %w(puma sidekiq).include?(target)
end
-
- def settings_key(target)
- case target
- when 'puma' then 'web_exporter'
- when 'sidekiq' then 'sidekiq_exporter'
- else ensure_valid_target!(target)
- end
- end
end
def initialize(target, metrics_dir, wipe_metrics_dir)
@@ -152,7 +162,7 @@ def start
when 'puma'
Gitlab::Metrics::Exporter::WebExporter.instance(**default_opts)
when 'sidekiq'
- settings = Settings.new(Settings.monitoring[name])
+ settings = Settings.new(Settings.monitoring.sidekiq_exporter)
Gitlab::Metrics::Exporter::SidekiqExporter.instance(settings, **default_opts)
end
@@ -160,9 +170,6 @@ def start
end
def name
- case @target
- when 'puma' then 'web_exporter'
- when 'sidekiq' then 'sidekiq_exporter'
- end
+ self.class.name(@target)
end
end
diff --git a/spec/commands/metrics_server/metrics_server_spec.rb b/spec/commands/metrics_server/metrics_server_spec.rb
index f93be1d9f8854dddfd828c46e6b7384946f42770..310e31da04501c7a7d4c6966393407082e89cb2a 100644
--- a/spec/commands/metrics_server/metrics_server_spec.rb
+++ b/spec/commands/metrics_server/metrics_server_spec.rb
@@ -70,7 +70,8 @@
before do
if use_golang_server
stub_env('GITLAB_GOLANG_METRICS_SERVER', '1')
- allow(Settings).to receive(:monitoring).and_return(config.dig('test', 'monitoring'))
+ allow(Settings).to receive(:monitoring).and_return(
+ Settingslogic.new(config.dig('test', 'monitoring')))
else
config_file.write(YAML.dump(config))
config_file.close
diff --git a/spec/metrics_server/metrics_server_spec.rb b/spec/metrics_server/metrics_server_spec.rb
index 76ea9cd66ba42662eebd6389df6e368ce7094b5b..efa716754f171d343393c6346c16849266047127 100644
--- a/spec/metrics_server/metrics_server_spec.rb
+++ b/spec/metrics_server/metrics_server_spec.rb
@@ -99,20 +99,22 @@
context 'for Golang server' do
let(:log_enabled) { false }
let(:settings) do
- {
- 'web_exporter' => {
- 'enabled' => true,
- 'address' => 'localhost',
- 'port' => '8083',
- 'log_enabled' => log_enabled
- },
- 'sidekiq_exporter' => {
- 'enabled' => true,
- 'address' => 'localhost',
- 'port' => '8082',
- 'log_enabled' => log_enabled
+ Settingslogic.new(
+ {
+ 'web_exporter' => {
+ 'enabled' => true,
+ 'address' => 'localhost',
+ 'port' => '8083',
+ 'log_enabled' => log_enabled
+ },
+ 'sidekiq_exporter' => {
+ 'enabled' => true,
+ 'address' => 'localhost',
+ 'port' => '8082',
+ 'log_enabled' => log_enabled
+ }
}
- }
+ )
end
let(:expected_port) { target == 'puma' ? '8083' : '8082' }
@@ -175,11 +177,13 @@
context 'when TLS settings are present' do
before do
- %w(web_exporter sidekiq_exporter).each do |key|
- settings[key]['tls_enabled'] = true
- settings[key]['tls_cert_path'] = '/path/to/cert.pem'
- settings[key]['tls_key_path'] = '/path/to/key.pem'
- end
+ settings.web_exporter['tls_enabled'] = true
+ settings.web_exporter['tls_cert_path'] = '/path/to/cert.pem'
+ settings.web_exporter['tls_key_path'] = '/path/to/key.pem'
+
+ settings.sidekiq_exporter['tls_enabled'] = true
+ settings.sidekiq_exporter['tls_cert_path'] = '/path/to/cert.pem'
+ settings.sidekiq_exporter['tls_key_path'] = '/path/to/key.pem'
end
it 'sets the correct environment variables' do
@@ -300,12 +304,12 @@
end
context 'for sidekiq' do
- let(:settings) { { "sidekiq_exporter" => { "enabled" => true } } }
+ let(:settings) { Settingslogic.new({ "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
+ settings.sidekiq_exporter, gc_requests: true, synchronous: true
).and_return(exporter_double)
end
@@ -358,4 +362,28 @@
end
end
end
+
+ describe '.name' do
+ subject { described_class.name(target) }
+
+ context 'for puma' do
+ let(:target) { 'puma' }
+
+ it { is_expected.to eq 'web_exporter' }
+ end
+
+ context 'for sidekiq' do
+ let(:target) { 'sidekiq' }
+
+ it { is_expected.to eq 'sidekiq_exporter' }
+ end
+
+ context 'for invalid target' do
+ let(:target) { 'invalid' }
+
+ it 'raises error' do
+ expect { subject }.to raise_error(RuntimeError, "Target must be one of [puma,sidekiq]")
+ end
+ end
+ end
end