diff options
author | Aleksei Lipniagov <alipniagov@gitlab.com> | 2019-07-18 13:54:11 +0000 |
---|---|---|
committer | Kamil TrzciĆski <ayufan@ayufan.eu> | 2019-07-18 13:54:11 +0000 |
commit | 22e2917b18f9e0544d807a047117b06311f7083b (patch) | |
tree | 7fe8ef692fc9e77797dc8870c954ae3a6a5b951e | |
parent | 91903d3a9eb3f72dd0684c983fb200ae14a8eb33 (diff) | |
download | gitlab-ce-22e2917b18f9e0544d807a047117b06311f7083b.tar.gz |
Fix pid providing for Prometheus
Use relative worker identifier for metrics (instead of Process.pid) and
identify when Unicorn/Puma/Sidekiq is used.
Previously, it was assumed that all metrics are gathered from Unicorn
due to hardcoded implementation which was incorrect.
-rw-r--r-- | config/initializers/7_prometheus_metrics.rb | 3 | ||||
-rw-r--r-- | lib/prometheus/pid_provider.rb | 42 | ||||
-rw-r--r-- | spec/lib/prometheus/pid_provider_spec.rb | 79 |
3 files changed, 122 insertions, 2 deletions
diff --git a/config/initializers/7_prometheus_metrics.rb b/config/initializers/7_prometheus_metrics.rb index 53c3eac3c74..3f2691dde95 100644 --- a/config/initializers/7_prometheus_metrics.rb +++ b/config/initializers/7_prometheus_metrics.rb @@ -1,5 +1,4 @@ require 'prometheus/client' -require 'prometheus/client/support/unicorn' # Keep separate directories for separate processes def prometheus_default_multiproc_dir @@ -23,7 +22,7 @@ Prometheus::Client.configure do |config| config.multiprocess_files_dir = ENV['prometheus_multiproc_dir'] || prometheus_default_multiproc_dir - config.pid_provider = Prometheus::Client::Support::Unicorn.method(:worker_pid_provider) + config.pid_provider = Prometheus::PidProvider.method(:worker_id) end Gitlab::Application.configure do |config| diff --git a/lib/prometheus/pid_provider.rb b/lib/prometheus/pid_provider.rb new file mode 100644 index 00000000000..c92522c73c5 --- /dev/null +++ b/lib/prometheus/pid_provider.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'prometheus/client/support/unicorn' + +module Prometheus + module PidProvider + extend self + + def worker_id + if Sidekiq.server? + 'sidekiq' + elsif defined?(Unicorn::Worker) + "unicorn_#{unicorn_worker_id}" + elsif defined?(::Puma) + "puma_#{puma_worker_id}" + else + "process_#{Process.pid}" + 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' + end + + # See the comment for #unicorn_worker_id + def puma_worker_id + match = process_name.match(/cluster worker ([0-9]+):/) + match ? match[1] : 'master' + end + + def process_name + $0 + end + end +end diff --git a/spec/lib/prometheus/pid_provider_spec.rb b/spec/lib/prometheus/pid_provider_spec.rb new file mode 100644 index 00000000000..e7d500612b1 --- /dev/null +++ b/spec/lib/prometheus/pid_provider_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +describe Prometheus::PidProvider do + describe '.worker_id' do + subject { described_class.worker_id } + + let(:sidekiq_module) { Module.new } + + before do + allow(sidekiq_module).to receive(:server?).and_return(false) + stub_const('Sidekiq', sidekiq_module) + end + + context 'when running in Sidekiq server mode' do + before do + expect(Sidekiq).to receive(:server?).and_return(true) + end + + it { is_expected.to eq 'sidekiq' } + end + + context 'when running in Unicorn mode' do + before do + stub_const('Unicorn::Worker', Class.new) + hide_const('Puma') + 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) + end + + it { is_expected.to eq 'unicorn_1' } + 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) + end + + it { is_expected.to eq 'unicorn_master' } + end + end + + context 'when running in Puma mode' do + before do + stub_const('Puma', Module.new) + hide_const('Unicorn::Worker') + 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 + + 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 + + it { is_expected.to eq 'puma_master' } + end + end + + context 'when running in unknown mode' do + before do + hide_const('Puma') + hide_const('Unicorn::Worker') + end + + it { is_expected.to eq "process_#{Process.pid}" } + end + end +end |