diff options
| -rw-r--r-- | changelogs/unreleased/fix-unicorn-sampler-workers-count.yml | 5 | ||||
| -rw-r--r-- | lib/gitlab/metrics/samplers/unicorn_sampler.rb | 11 | ||||
| -rw-r--r-- | spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb | 33 |
3 files changed, 38 insertions, 11 deletions
diff --git a/changelogs/unreleased/fix-unicorn-sampler-workers-count.yml b/changelogs/unreleased/fix-unicorn-sampler-workers-count.yml new file mode 100644 index 00000000000..9a263b7f460 --- /dev/null +++ b/changelogs/unreleased/fix-unicorn-sampler-workers-count.yml @@ -0,0 +1,5 @@ +--- +title: Make unicorn_workers to return meaningful results +merge_request: 30506 +author: +type: fixed diff --git a/lib/gitlab/metrics/samplers/unicorn_sampler.rb b/lib/gitlab/metrics/samplers/unicorn_sampler.rb index 9af7e0afed4..355f938704e 100644 --- a/lib/gitlab/metrics/samplers/unicorn_sampler.rb +++ b/lib/gitlab/metrics/samplers/unicorn_sampler.rb @@ -54,7 +54,16 @@ module Gitlab end def unicorn_workers_count - `pgrep -f '[u]nicorn_rails worker.+ #{Rails.root.to_s}'`.split.count + http_servers.sum(&:worker_processes) # rubocop: disable CodeReuse/ActiveRecord + end + + # Traversal of ObjectSpace is expensive, on fully loaded application + # it takes around 80ms. The instances of HttpServers are not a subject + # to change so we can cache the list of servers. + def http_servers + return [] unless defined?(::Unicorn::HttpServer) + + @http_servers ||= ObjectSpace.each_object(::Unicorn::HttpServer).to_a end end end diff --git a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb index 090e456644f..4b697b2ba0f 100644 --- a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do subject { described_class.new(1.second) } describe '#sample' do - let(:unicorn) { double('unicorn') } + let(:unicorn) { Module.new } let(:raindrops) { double('raindrops') } let(:stats) { double('stats') } @@ -78,19 +78,32 @@ describe Gitlab::Metrics::Samplers::UnicornSampler do end end - context 'additional metrics' do - let(:unicorn_workers) { 2 } - + context 'unicorn workers' do before do - allow(unicorn).to receive(:listener_names).and_return([""]) - allow(::Gitlab::Metrics::System).to receive(:cpu_time).and_return(3.14) - allow(subject).to receive(:unicorn_workers_count).and_return(unicorn_workers) + allow(unicorn).to receive(:listener_names).and_return([]) end - it "sets additional metrics" do - expect(subject.metrics[:unicorn_workers]).to receive(:set).with({}, unicorn_workers) + context 'without http server' do + it "does set unicorn_workers to 0" do + expect(subject.metrics[:unicorn_workers]).to receive(:set).with({}, 0) - subject.sample + subject.sample + end + end + + context 'with http server' do + let(:http_server_class) { Struct.new(:worker_processes) } + let!(:http_server) { http_server_class.new(5) } + + before do + stub_const('Unicorn::HttpServer', http_server_class) + end + + it "sets additional metrics" do + expect(subject.metrics[:unicorn_workers]).to receive(:set).with({}, 5) + + subject.sample + end end end end |