diff --git a/Gemfile b/Gemfile index 8ec2c94a2e0..0c18007d447 100644 --- a/Gemfile +++ b/Gemfile @@ -270,8 +270,7 @@ group :metrics do gem 'influxdb', '~> 0.2', require: false # Prometheus - gem 'mmap2', '~> 2.2.6' - gem 'prometheus-client-mmap', '~>0.7.0.beta3' + gem 'prometheus-client-mmap', '~>0.7.0.beta5' end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index bb574ae1834..12520570dfe 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -561,8 +561,8 @@ GEM premailer-rails (1.9.2) actionmailer (>= 3, < 6) premailer (~> 1.7, >= 1.7.9) - prometheus-client-mmap (0.7.0.beta3) - quantile (~> 0.2.0) + prometheus-client-mmap (0.7.0.beta5) + mmap2 (~> 2.2.6) pry (0.10.4) coderay (~> 1.1.0) method_source (~> 0.8.1) @@ -573,7 +573,6 @@ GEM pry-rails (0.3.5) pry (>= 0.9.10) pyu-ruby-sasl (0.0.3.3) - quantile (0.2.0) rack (1.6.5) rack-accept (0.4.5) rack (>= 0.4) @@ -972,7 +971,6 @@ DEPENDENCIES mail_room (~> 0.9.1) method_source (~> 0.8) minitest (~> 5.7.0) - mmap2 (~> 2.2.6) mousetrap-rails (~> 1.4.6) mysql2 (~> 0.3.16) net-ssh (~> 3.0.1) @@ -1000,7 +998,7 @@ DEPENDENCIES pg (~> 0.18.2) poltergeist (~> 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-rails (~> 0.3.4) rack-attack (~> 4.4.1) diff --git a/app/controllers/metrics_controller.rb b/app/controllers/metrics_controller.rb index 9bcd6f96b34..0e9a19c0b6f 100644 --- a/app/controllers/metrics_controller.rb +++ b/app/controllers/metrics_controller.rb @@ -5,10 +5,8 @@ class MetricsController < ActionController::Base before_action :validate_prometheus_metrics - def metrics - response = "#{metrics_service.health_metrics_text}\n#{metrics_service.prometheus_metrics_text}" - - render text: response, content_type: 'text/plain; version=0.0.4' + def index + render text: metrics_service.metrics_text, content_type: 'text/plain; verssion=0.0.4' end private diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index fc9de30f256..ab52b676e01 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -48,7 +48,7 @@ class SessionsController < Devise::SessionsController private 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 # Handle an "initial setup" state, where there's only one user, it's an admin, diff --git a/app/services/metrics_service.rb b/app/services/metrics_service.rb index 350c3639e92..0faa86a228b 100644 --- a/app/services/metrics_service.rb +++ b/app/services/metrics_service.rb @@ -12,18 +12,23 @@ class MetricsService end 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" } - metrics = results.map(&method(:metric_to_prom_line)) + formatter.marshal(metrics) + end - types.concat(metrics).join("\n") + def metrics_text + "#{health_metrics_text}\n#{prometheus_metrics_text}" end private + def formatter + @formatter ||= PrometheusText.new + end + def multiprocess_metrics_path - Rails.root.join(ENV['prometheus_multiproc_dir']) + @multiprocess_metrics_path ||= Rails.root.join(ENV['prometheus_multiproc_dir']) end def metric_to_prom_line(metric) diff --git a/config.ru b/config.ru index 2614c9aaf74..89aba462f19 100644 --- a/config.ru +++ b/config.ru @@ -16,7 +16,7 @@ if defined?(Unicorn) end # 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__) diff --git a/lib/gitlab/health_checks/prometheus_text.rb b/lib/gitlab/health_checks/prometheus_text.rb new file mode 100644 index 00000000000..a01e6b2be1f --- /dev/null +++ b/lib/gitlab/health_checks/prometheus_text.rb @@ -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 diff --git a/lib/gitlab/metrics.rb b/lib/gitlab/metrics.rb index a41cbd214a1..34f6b32f7da 100644 --- a/lib/gitlab/metrics.rb +++ b/lib/gitlab/metrics.rb @@ -73,7 +73,7 @@ module Gitlab if prometheus_metrics_enabled? registry.get(name) else - DummyMetric.new + NullMetric.new end end diff --git a/lib/gitlab/metrics/dummy_metric.rb b/lib/gitlab/metrics/null_metric.rb similarity index 54% rename from lib/gitlab/metrics/dummy_metric.rb rename to lib/gitlab/metrics/null_metric.rb index d27bb83854a..1501cd38676 100644 --- a/lib/gitlab/metrics/dummy_metric.rb +++ b/lib/gitlab/metrics/null_metric.rb @@ -1,7 +1,12 @@ module Gitlab module 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) raise NotImplementedError end @@ -9,21 +14,6 @@ module Gitlab def values(*args) raise NotImplementedError end - - # counter - def increment(*args) - # noop - end - - # gauge - def set(*args) - # noop - end - - # histogram / summary - def observe(*args) - # noop - end end end end diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index cd31f750ffd..7baf7d1bef9 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -6,10 +6,10 @@ describe MetricsController do let(:token) { current_application_settings.health_check_access_token } let(:json_response) { JSON.parse(response.body) } - around do |examples| + around do |example| Dir.mktmpdir do |tmp_dir| @metrics_multiproc_dir = tmp_dir - examples.run + example.run end end diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb index 863868e1576..87c9f4ebda4 100644 --- a/spec/lib/gitlab/metrics_spec.rb +++ b/spec/lib/gitlab/metrics_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::Metrics do + include StubENV + describe '.settings' do it 'returns a Hash' do expect(described_class.settings).to be_an_instance_of(Hash) @@ -247,45 +249,53 @@ describe Gitlab::Metrics do it_behaves_like 'prometheus metrics API' - describe '#dummy_metric' do + describe '#null_metric' do 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 describe '#counter' do 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 describe '#summary' do 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 describe '#gauge' do 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 describe '#histogram' do 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 context 'prometheus metrics enabled' do + around do |example| + Dir.mktmpdir do |tmp_dir| + @metrics_multiproc_dir = tmp_dir + example.run + end + end + before do + stub_const('Prometheus::Client::Multiprocdir', @metrics_multiproc_dir) allow(described_class).to receive(:prometheus_metrics_enabled?).and_return(true) end it_behaves_like 'prometheus metrics API' - describe '#dummy_metric' do + describe '#null_metric' do subject { described_class.provide_metric(:test) } it { is_expected.to be_nil } @@ -294,25 +304,25 @@ describe Gitlab::Metrics do describe '#counter' do 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 describe '#summary' do 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 describe '#gauge' do 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 describe '#histogram' do 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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 994c7dcbb46..f800c5bcb07 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,6 +3,7 @@ SimpleCovEnv.start! ENV["RAILS_ENV"] ||= 'test' ENV["IN_MEMORY_APPLICATION_SETTINGS"] = 'true' +# ENV['prometheus_multiproc_dir'] = 'tmp/prometheus_multiproc_dir_test' require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' diff --git a/tmp/prometheus_data_dir/.gitkeep b/tmp/prometheus_multiproc_dir/.gitkeep similarity index 100% rename from tmp/prometheus_data_dir/.gitkeep rename to tmp/prometheus_multiproc_dir/.gitkeep