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.
This commit is contained in:
parent
91903d3a9e
commit
22e2917b18
3 changed files with 122 additions and 2 deletions
|
@ -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|
|
||||
|
|
42
lib/prometheus/pid_provider.rb
Normal file
42
lib/prometheus/pid_provider.rb
Normal file
|
@ -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
|
79
spec/lib/prometheus/pid_provider_spec.rb
Normal file
79
spec/lib/prometheus/pid_provider_spec.rb
Normal file
|
@ -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
|
Loading…
Reference in a new issue