summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAleksei Lipniagov <alipniagov@gitlab.com>2019-07-26 15:02:21 +0000
committerKamil TrzciƄski <ayufan@ayufan.eu>2019-07-26 15:02:21 +0000
commit13676e021cddba7a801386a183dba696200312bf (patch)
treea4742226a642c1c2cad30ae0f80dd5530bdb32b5
parentbd1a5a9f422df267725bd5fbd254f3c1f16fd596 (diff)
downloadgitlab-ce-13676e021cddba7a801386a183dba696200312bf.tar.gz
Fix pid discovery for Unicorn in PidProvider
-rw-r--r--changelogs/unreleased/64972-fix-unicorn-workers-metric.yml5
-rw-r--r--lib/prometheus/pid_provider.rb35
-rw-r--r--spec/lib/prometheus/pid_provider_spec.rb66
3 files changed, 78 insertions, 28 deletions
diff --git a/changelogs/unreleased/64972-fix-unicorn-workers-metric.yml b/changelogs/unreleased/64972-fix-unicorn-workers-metric.yml
new file mode 100644
index 00000000000..f80111f0bd9
--- /dev/null
+++ b/changelogs/unreleased/64972-fix-unicorn-workers-metric.yml
@@ -0,0 +1,5 @@
+---
+title: Fix pid discovery for Unicorn processes in `PidProvider`
+merge_request: 31056
+author:
+type: fixed
diff --git a/lib/prometheus/pid_provider.rb b/lib/prometheus/pid_provider.rb
index c92522c73c5..e0f7e7e0a9e 100644
--- a/lib/prometheus/pid_provider.rb
+++ b/lib/prometheus/pid_provider.rb
@@ -1,7 +1,5 @@
# frozen_string_literal: true
-require 'prometheus/client/support/unicorn'
-
module Prometheus
module PidProvider
extend self
@@ -10,29 +8,38 @@ module Prometheus
if Sidekiq.server?
'sidekiq'
elsif defined?(Unicorn::Worker)
- "unicorn_#{unicorn_worker_id}"
+ unicorn_worker_id
elsif defined?(::Puma)
- "puma_#{puma_worker_id}"
+ puma_worker_id
else
- "process_#{Process.pid}"
+ unknown_process_id
end
end
private
- # This is not fully accurate as we don't really know if the nil returned
- # is actually means we're on master or not.
- # Follow up issue was created to address this problem and
- # to introduce more structrured approach to a current process discovery:
- # https://gitlab.com/gitlab-org/gitlab-ce/issues/64740
def unicorn_worker_id
- ::Prometheus::Client::Support::Unicorn.worker_id || 'master'
+ if matches = process_name.match(/unicorn.*worker\[([0-9]+)\]/)
+ "unicorn_#{matches[1]}"
+ elsif process_name =~ /unicorn/
+ "unicorn_master"
+ else
+ unknown_process_id
+ end
end
- # See the comment for #unicorn_worker_id
def puma_worker_id
- match = process_name.match(/cluster worker ([0-9]+):/)
- match ? match[1] : 'master'
+ if matches = process_name.match(/puma.*cluster worker ([0-9]+):/)
+ "puma_#{matches[1]}"
+ elsif process_name =~ /puma/
+ "puma_master"
+ else
+ unknown_process_id
+ end
+ end
+
+ def unknown_process_id
+ "process_#{Process.pid}"
end
def process_name
diff --git a/spec/lib/prometheus/pid_provider_spec.rb b/spec/lib/prometheus/pid_provider_spec.rb
index e7d500612b1..ba843b27254 100644
--- a/spec/lib/prometheus/pid_provider_spec.rb
+++ b/spec/lib/prometheus/pid_provider_spec.rb
@@ -25,22 +25,60 @@ describe Prometheus::PidProvider do
before do
stub_const('Unicorn::Worker', Class.new)
hide_const('Puma')
+
+ expect(described_class).to receive(:process_name)
+ .at_least(:once)
+ .and_return(process_name)
end
- context 'when `Prometheus::Client::Support::Unicorn` provides worker_id' do
- before do
- expect(::Prometheus::Client::Support::Unicorn).to receive(:worker_id).and_return(1)
+ context 'when unicorn master is specified in process name' do
+ context 'when running in Omnibus' do
+ context 'before the process was renamed' do
+ let(:process_name) { "/opt/gitlab/embedded/bin/unicorn"}
+
+ it { is_expected.to eq 'unicorn_master' }
+ end
+
+ context 'after the process was renamed' do
+ let(:process_name) { "unicorn master -D -E production -c /var/opt/gitlab/gitlab-rails/etc/unicorn.rb /opt/gitlab/embedded/service/gitlab-rails/config.ru" }
+
+ it { is_expected.to eq 'unicorn_master' }
+ end
end
- it { is_expected.to eq 'unicorn_1' }
+ context 'when in development env' do
+ context 'before the process was renamed' do
+ let(:process_name) { "path_to_bindir/bin/unicorn_rails"}
+
+ it { is_expected.to eq 'unicorn_master' }
+ end
+
+ context 'after the process was renamed' do
+ let(:process_name) { "unicorn_rails master -c /gitlab_dir/config/unicorn.rb -E development" }
+
+ it { is_expected.to eq 'unicorn_master' }
+ end
+ end
end
- context 'when no worker_id is provided from `Prometheus::Client::Support::Unicorn`' do
- before do
- expect(::Prometheus::Client::Support::Unicorn).to receive(:worker_id).and_return(nil)
+ context 'when unicorn worker id is specified in process name' do
+ context 'when running in Omnibus' do
+ let(:process_name) { "unicorn worker[1] -D -E production -c /var/opt/gitlab/gitlab-rails/etc/unicorn.rb /opt/gitlab/embedded/service/gitlab-rails/config.ru" }
+
+ it { is_expected.to eq 'unicorn_1' }
end
- it { is_expected.to eq 'unicorn_master' }
+ context 'when in development env' do
+ let(:process_name) { "unicorn_rails worker[1] -c gitlab_dir/config/unicorn.rb -E development" }
+
+ it { is_expected.to eq 'unicorn_1' }
+ end
+ end
+
+ context 'when no specified unicorn master or worker id in process name' do
+ let(:process_name) { "bin/unknown_process"}
+
+ it { is_expected.to eq "process_#{Process.pid}" }
end
end
@@ -48,20 +86,20 @@ describe Prometheus::PidProvider do
before do
stub_const('Puma', Module.new)
hide_const('Unicorn::Worker')
+
+ expect(described_class).to receive(:process_name)
+ .at_least(:once)
+ .and_return(process_name)
end
context 'when cluster worker id is specified in process name' do
- before do
- expect(described_class).to receive(:process_name).and_return('puma: cluster worker 1: 17483 [gitlab-puma-worker]')
- end
+ let(:process_name) { 'puma: cluster worker 1: 17483 [gitlab-puma-worker]' }
it { is_expected.to eq 'puma_1' }
end
context 'when no worker id is specified in process name' do
- before do
- expect(described_class).to receive(:process_name).and_return('bin/puma')
- end
+ let(:process_name) { 'bin/puma' }
it { is_expected.to eq 'puma_master' }
end