Move Prometheus presentation logic to PrometheusText
+ Use NullMetrics to mock metrics when unused + Use method_missing in NullMetrics mocking + Update prometheus gem to version that correctly uses transitive dependencies + Ensure correct folders are used in Multiprocess prometheus client tests. + rename Sessions controller's metric
This commit is contained in:
parent
770f07cd5c
commit
c134a72cdb
3
Gemfile
3
Gemfile
|
@ -270,8 +270,7 @@ group :metrics do
|
||||||
gem 'influxdb', '~> 0.2', require: false
|
gem 'influxdb', '~> 0.2', require: false
|
||||||
|
|
||||||
# Prometheus
|
# Prometheus
|
||||||
gem 'mmap2', '~> 2.2.6'
|
gem 'prometheus-client-mmap', '~>0.7.0.beta5'
|
||||||
gem 'prometheus-client-mmap', '~>0.7.0.beta3'
|
|
||||||
end
|
end
|
||||||
|
|
||||||
group :development do
|
group :development do
|
||||||
|
|
|
@ -561,8 +561,8 @@ GEM
|
||||||
premailer-rails (1.9.2)
|
premailer-rails (1.9.2)
|
||||||
actionmailer (>= 3, < 6)
|
actionmailer (>= 3, < 6)
|
||||||
premailer (~> 1.7, >= 1.7.9)
|
premailer (~> 1.7, >= 1.7.9)
|
||||||
prometheus-client-mmap (0.7.0.beta3)
|
prometheus-client-mmap (0.7.0.beta5)
|
||||||
quantile (~> 0.2.0)
|
mmap2 (~> 2.2.6)
|
||||||
pry (0.10.4)
|
pry (0.10.4)
|
||||||
coderay (~> 1.1.0)
|
coderay (~> 1.1.0)
|
||||||
method_source (~> 0.8.1)
|
method_source (~> 0.8.1)
|
||||||
|
@ -573,7 +573,6 @@ GEM
|
||||||
pry-rails (0.3.5)
|
pry-rails (0.3.5)
|
||||||
pry (>= 0.9.10)
|
pry (>= 0.9.10)
|
||||||
pyu-ruby-sasl (0.0.3.3)
|
pyu-ruby-sasl (0.0.3.3)
|
||||||
quantile (0.2.0)
|
|
||||||
rack (1.6.5)
|
rack (1.6.5)
|
||||||
rack-accept (0.4.5)
|
rack-accept (0.4.5)
|
||||||
rack (>= 0.4)
|
rack (>= 0.4)
|
||||||
|
@ -972,7 +971,6 @@ DEPENDENCIES
|
||||||
mail_room (~> 0.9.1)
|
mail_room (~> 0.9.1)
|
||||||
method_source (~> 0.8)
|
method_source (~> 0.8)
|
||||||
minitest (~> 5.7.0)
|
minitest (~> 5.7.0)
|
||||||
mmap2 (~> 2.2.6)
|
|
||||||
mousetrap-rails (~> 1.4.6)
|
mousetrap-rails (~> 1.4.6)
|
||||||
mysql2 (~> 0.3.16)
|
mysql2 (~> 0.3.16)
|
||||||
net-ssh (~> 3.0.1)
|
net-ssh (~> 3.0.1)
|
||||||
|
@ -1000,7 +998,7 @@ DEPENDENCIES
|
||||||
pg (~> 0.18.2)
|
pg (~> 0.18.2)
|
||||||
poltergeist (~> 1.9.0)
|
poltergeist (~> 1.9.0)
|
||||||
premailer-rails (~> 1.9.0)
|
premailer-rails (~> 1.9.0)
|
||||||
prometheus-client-mmap (~> 0.7.0.beta3)
|
prometheus-client-mmap (~> 0.7.0.beta5)
|
||||||
pry-byebug (~> 3.4.1)
|
pry-byebug (~> 3.4.1)
|
||||||
pry-rails (~> 0.3.4)
|
pry-rails (~> 0.3.4)
|
||||||
rack-attack (~> 4.4.1)
|
rack-attack (~> 4.4.1)
|
||||||
|
|
|
@ -5,10 +5,8 @@ class MetricsController < ActionController::Base
|
||||||
|
|
||||||
before_action :validate_prometheus_metrics
|
before_action :validate_prometheus_metrics
|
||||||
|
|
||||||
def metrics
|
def index
|
||||||
response = "#{metrics_service.health_metrics_text}\n#{metrics_service.prometheus_metrics_text}"
|
render text: metrics_service.metrics_text, content_type: 'text/plain; verssion=0.0.4'
|
||||||
|
|
||||||
render text: response, content_type: 'text/plain; version=0.0.4'
|
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
|
@ -48,7 +48,7 @@ class SessionsController < Devise::SessionsController
|
||||||
private
|
private
|
||||||
|
|
||||||
def login_counter
|
def login_counter
|
||||||
@login_counter ||= Gitlab::Metrics.counter(:user_session_logins, 'User logins count')
|
@login_counter ||= Gitlab::Metrics.counter(:user_session_logins, 'User sign in count')
|
||||||
end
|
end
|
||||||
|
|
||||||
# Handle an "initial setup" state, where there's only one user, it's an admin,
|
# Handle an "initial setup" state, where there's only one user, it's an admin,
|
||||||
|
|
|
@ -12,18 +12,23 @@ class MetricsService
|
||||||
end
|
end
|
||||||
|
|
||||||
def health_metrics_text
|
def health_metrics_text
|
||||||
results = CHECKS.flat_map(&:metrics)
|
metrics = CHECKS.flat_map(&:metrics)
|
||||||
|
|
||||||
types = results.map(&:name).uniq.map { |metric_name| "# TYPE #{metric_name} gauge" }
|
formatter.marshal(metrics)
|
||||||
metrics = results.map(&method(:metric_to_prom_line))
|
end
|
||||||
|
|
||||||
types.concat(metrics).join("\n")
|
def metrics_text
|
||||||
|
"#{health_metrics_text}\n#{prometheus_metrics_text}"
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
|
def formatter
|
||||||
|
@formatter ||= PrometheusText.new
|
||||||
|
end
|
||||||
|
|
||||||
def multiprocess_metrics_path
|
def multiprocess_metrics_path
|
||||||
Rails.root.join(ENV['prometheus_multiproc_dir'])
|
@multiprocess_metrics_path ||= Rails.root.join(ENV['prometheus_multiproc_dir'])
|
||||||
end
|
end
|
||||||
|
|
||||||
def metric_to_prom_line(metric)
|
def metric_to_prom_line(metric)
|
||||||
|
|
|
@ -16,7 +16,7 @@ if defined?(Unicorn)
|
||||||
end
|
end
|
||||||
|
|
||||||
# set default directory for multiproces metrics gathering
|
# set default directory for multiproces metrics gathering
|
||||||
ENV['prometheus_multiproc_dir'] ||= 'tmp/prometheus_data_dir'
|
ENV['prometheus_multiproc_dir'] ||= 'tmp/prometheus_multiproc_dir'
|
||||||
|
|
||||||
require ::File.expand_path('../config/environment', __FILE__)
|
require ::File.expand_path('../config/environment', __FILE__)
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,38 @@
|
||||||
|
module Gitlab::HealthChecks
|
||||||
|
class PrometheusText
|
||||||
|
def marshal(metrics)
|
||||||
|
metrics_with_type_declarations(metrics).join("\n")
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def metrics_with_type_declarations(metrics)
|
||||||
|
type_declaration_added = {}
|
||||||
|
|
||||||
|
metrics.flat_map do |metric|
|
||||||
|
metric_lines = []
|
||||||
|
|
||||||
|
unless type_declaration_added.has_key?(metric.name)
|
||||||
|
type_declaration_added[metric.name] = true
|
||||||
|
metric_lines << metric_type_declaration(metric)
|
||||||
|
end
|
||||||
|
|
||||||
|
metric_lines << metric_text(metric)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def metric_type_declaration(metric)
|
||||||
|
"# TYPE #{metric.name} gauge"
|
||||||
|
end
|
||||||
|
|
||||||
|
def metric_text(metric)
|
||||||
|
labels = metric.labels&.map { |key, value| "#{key}=\"#{value}\"" }&.join(',') || ''
|
||||||
|
|
||||||
|
if labels.empty?
|
||||||
|
"#{metric.name} #{metric.value}"
|
||||||
|
else
|
||||||
|
"#{metric.name}{#{labels}} #{metric.value}"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -73,7 +73,7 @@ module Gitlab
|
||||||
if prometheus_metrics_enabled?
|
if prometheus_metrics_enabled?
|
||||||
registry.get(name)
|
registry.get(name)
|
||||||
else
|
else
|
||||||
DummyMetric.new
|
NullMetric.new
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -1,7 +1,12 @@
|
||||||
module Gitlab
|
module Gitlab
|
||||||
module Metrics
|
module Metrics
|
||||||
# Mocks ::Prometheus::Client::Metric and all derived metrics
|
# Mocks ::Prometheus::Client::Metric and all derived metrics
|
||||||
class DummyMetric
|
class NullMetric
|
||||||
|
def method_missing(name, *args, &block)
|
||||||
|
nil
|
||||||
|
end
|
||||||
|
|
||||||
|
# these methods shouldn't be called when metrics are disabled
|
||||||
def get(*args)
|
def get(*args)
|
||||||
raise NotImplementedError
|
raise NotImplementedError
|
||||||
end
|
end
|
||||||
|
@ -9,21 +14,6 @@ module Gitlab
|
||||||
def values(*args)
|
def values(*args)
|
||||||
raise NotImplementedError
|
raise NotImplementedError
|
||||||
end
|
end
|
||||||
|
|
||||||
# counter
|
|
||||||
def increment(*args)
|
|
||||||
# noop
|
|
||||||
end
|
|
||||||
|
|
||||||
# gauge
|
|
||||||
def set(*args)
|
|
||||||
# noop
|
|
||||||
end
|
|
||||||
|
|
||||||
# histogram / summary
|
|
||||||
def observe(*args)
|
|
||||||
# noop
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
|
@ -6,10 +6,10 @@ describe MetricsController do
|
||||||
let(:token) { current_application_settings.health_check_access_token }
|
let(:token) { current_application_settings.health_check_access_token }
|
||||||
let(:json_response) { JSON.parse(response.body) }
|
let(:json_response) { JSON.parse(response.body) }
|
||||||
|
|
||||||
around do |examples|
|
around do |example|
|
||||||
Dir.mktmpdir do |tmp_dir|
|
Dir.mktmpdir do |tmp_dir|
|
||||||
@metrics_multiproc_dir = tmp_dir
|
@metrics_multiproc_dir = tmp_dir
|
||||||
examples.run
|
example.run
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -1,6 +1,8 @@
|
||||||
require 'spec_helper'
|
require 'spec_helper'
|
||||||
|
|
||||||
describe Gitlab::Metrics do
|
describe Gitlab::Metrics do
|
||||||
|
include StubENV
|
||||||
|
|
||||||
describe '.settings' do
|
describe '.settings' do
|
||||||
it 'returns a Hash' do
|
it 'returns a Hash' do
|
||||||
expect(described_class.settings).to be_an_instance_of(Hash)
|
expect(described_class.settings).to be_an_instance_of(Hash)
|
||||||
|
@ -247,45 +249,53 @@ describe Gitlab::Metrics do
|
||||||
|
|
||||||
it_behaves_like 'prometheus metrics API'
|
it_behaves_like 'prometheus metrics API'
|
||||||
|
|
||||||
describe '#dummy_metric' do
|
describe '#null_metric' do
|
||||||
subject { described_class.provide_metric(:test) }
|
subject { described_class.provide_metric(:test) }
|
||||||
|
|
||||||
it { is_expected.to be_a(Gitlab::Metrics::DummyMetric) }
|
it { is_expected.to be_a(Gitlab::Metrics::NullMetric) }
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#counter' do
|
describe '#counter' do
|
||||||
subject { described_class.counter(:counter, 'doc') }
|
subject { described_class.counter(:counter, 'doc') }
|
||||||
|
|
||||||
it { is_expected.to be_a(Gitlab::Metrics::DummyMetric) }
|
it { is_expected.to be_a(Gitlab::Metrics::NullMetric) }
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#summary' do
|
describe '#summary' do
|
||||||
subject { described_class.summary(:summary, 'doc') }
|
subject { described_class.summary(:summary, 'doc') }
|
||||||
|
|
||||||
it { is_expected.to be_a(Gitlab::Metrics::DummyMetric) }
|
it { is_expected.to be_a(Gitlab::Metrics::NullMetric) }
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#gauge' do
|
describe '#gauge' do
|
||||||
subject { described_class.gauge(:gauge, 'doc') }
|
subject { described_class.gauge(:gauge, 'doc') }
|
||||||
|
|
||||||
it { is_expected.to be_a(Gitlab::Metrics::DummyMetric) }
|
it { is_expected.to be_a(Gitlab::Metrics::NullMetric) }
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#histogram' do
|
describe '#histogram' do
|
||||||
subject { described_class.histogram(:histogram, 'doc') }
|
subject { described_class.histogram(:histogram, 'doc') }
|
||||||
|
|
||||||
it { is_expected.to be_a(Gitlab::Metrics::DummyMetric) }
|
it { is_expected.to be_a(Gitlab::Metrics::NullMetric) }
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'prometheus metrics enabled' do
|
context 'prometheus metrics enabled' do
|
||||||
|
around do |example|
|
||||||
|
Dir.mktmpdir do |tmp_dir|
|
||||||
|
@metrics_multiproc_dir = tmp_dir
|
||||||
|
example.run
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
before do
|
before do
|
||||||
|
stub_const('Prometheus::Client::Multiprocdir', @metrics_multiproc_dir)
|
||||||
allow(described_class).to receive(:prometheus_metrics_enabled?).and_return(true)
|
allow(described_class).to receive(:prometheus_metrics_enabled?).and_return(true)
|
||||||
end
|
end
|
||||||
|
|
||||||
it_behaves_like 'prometheus metrics API'
|
it_behaves_like 'prometheus metrics API'
|
||||||
|
|
||||||
describe '#dummy_metric' do
|
describe '#null_metric' do
|
||||||
subject { described_class.provide_metric(:test) }
|
subject { described_class.provide_metric(:test) }
|
||||||
|
|
||||||
it { is_expected.to be_nil }
|
it { is_expected.to be_nil }
|
||||||
|
@ -294,25 +304,25 @@ describe Gitlab::Metrics do
|
||||||
describe '#counter' do
|
describe '#counter' do
|
||||||
subject { described_class.counter(:name, 'doc') }
|
subject { described_class.counter(:name, 'doc') }
|
||||||
|
|
||||||
it { is_expected.not_to be_a(Gitlab::Metrics::DummyMetric) }
|
it { is_expected.not_to be_a(Gitlab::Metrics::NullMetric) }
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#summary' do
|
describe '#summary' do
|
||||||
subject { described_class.summary(:name, 'doc') }
|
subject { described_class.summary(:name, 'doc') }
|
||||||
|
|
||||||
it { is_expected.not_to be_a(Gitlab::Metrics::DummyMetric) }
|
it { is_expected.not_to be_a(Gitlab::Metrics::NullMetric) }
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#gauge' do
|
describe '#gauge' do
|
||||||
subject { described_class.gauge(:name, 'doc') }
|
subject { described_class.gauge(:name, 'doc') }
|
||||||
|
|
||||||
it { is_expected.not_to be_a(Gitlab::Metrics::DummyMetric) }
|
it { is_expected.not_to be_a(Gitlab::Metrics::NullMetric) }
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#histogram' do
|
describe '#histogram' do
|
||||||
subject { described_class.histogram(:name, 'doc') }
|
subject { described_class.histogram(:name, 'doc') }
|
||||||
|
|
||||||
it { is_expected.not_to be_a(Gitlab::Metrics::DummyMetric) }
|
it { is_expected.not_to be_a(Gitlab::Metrics::NullMetric) }
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -3,6 +3,7 @@ SimpleCovEnv.start!
|
||||||
|
|
||||||
ENV["RAILS_ENV"] ||= 'test'
|
ENV["RAILS_ENV"] ||= 'test'
|
||||||
ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true'
|
ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true'
|
||||||
|
# ENV['prometheus_multiproc_dir'] = 'tmp/prometheus_multiproc_dir_test'
|
||||||
|
|
||||||
require File.expand_path("../../config/environment", __FILE__)
|
require File.expand_path("../../config/environment", __FILE__)
|
||||||
require 'rspec/rails'
|
require 'rspec/rails'
|
||||||
|
|
Loading…
Reference in New Issue