Split metrics from health controller into metrics controller

app/controllers/health_controller.rb

@@ -22,37 +22,8 @@ class HealthController < ActionController::Base
     render_check_results(results)
   end
 
-  def metrics
-    response = health_metrics_text + "\n"
-
-    if Gitlab::Metrics.prometheus_metrics_enabled?
-      response += Prometheus::Client::Formats::Text.marshal_multiprocess(ENV['prometheus_multiproc_dir'])
-    end
-
-    render text: response, content_type: 'text/plain; version=0.0.4'
-  end
-
   private
 
-  def health_metrics_text
-    results = CHECKS.flat_map(&:metrics)
-
-    types = results.map(&:name)
-              .uniq
-              .map { |metric_name| "# TYPE #{metric_name} gauge" }
-    metrics = results.map(&method(:metric_to_prom_line))
-    types.concat(metrics).join("\n")
-  end
-
-  def metric_to_prom_line(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
-
   def render_check_results(results)
     flattened = results.flat_map do |name, result|
       if result.is_a?(Gitlab::HealthChecks::Result)

app/controllers/metrics_controller.rb

@@ -0,0 +1,41 @@
+require 'prometheus/client/formats/text'
+
+class MetricsController < ActionController::Base
+  protect_from_forgery with: :exception
+
+  CHECKS = [
+    Gitlab::HealthChecks::DbCheck,
+    Gitlab::HealthChecks::RedisCheck,
+    Gitlab::HealthChecks::FsShardsCheck,
+  ].freeze
+
+  def metrics
+    render_404 unless Gitlab::Metrics.prometheus_metrics_enabled?
+
+    metrics_text = Prometheus::Client::Formats::Text.marshal_multiprocess(Settings.gitlab['prometheus_multiproc_dir'])
+    response = health_metrics_text + "\n" + metrics_text
+
+    render text: response, content_type: 'text/plain; version=0.0.4'
+  end
+
+  private
+
+  def health_metrics_text
+    results = CHECKS.flat_map(&:metrics)
+
+    types = results.map(&:name)
+              .uniq
+              .map { |metric_name| "# TYPE #{metric_name} gauge" }
+    metrics = results.map(&method(:metric_to_prom_line))
+    types.concat(metrics).join("\n")
+  end
+
+  def metric_to_prom_line(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

config/gitlab.yml.example

@@ -102,6 +102,11 @@ production: &base
     # The default is 'shared/cache/archive/' relative to the root of the Rails app.
     # repository_downloads_path: shared/cache/archive/
 
+    ## Prometheus Client Data directory
+    # To be used efficiently in multiprocess Ruby setup like Unicorn, Prometheus client needs to share metrics with other instances.
+    # The default is 'tmp/prometheus_data_dir' relative to Rails.root
+    # prometheus_multiproc_dir: tmp/prometheus_data_dir
+
   ## Reply by email
   # Allow users to comment on issues and merge requests by replying to notification emails.
   # For documentation on how to set this up, see http://doc.gitlab.com/ce/administration/reply_by_email.html

config/initializers/1_settings.rb

@@ -242,6 +242,7 @@ Settings.gitlab['import_sources'] ||= %w[github bitbucket gitlab google_code fog
 Settings.gitlab['trusted_proxies'] ||= []
 Settings.gitlab['no_todos_messages'] ||= YAML.load_file(Rails.root.join('config', 'no_todos_messages.yml'))
 Settings.gitlab['usage_ping_enabled'] = true if Settings.gitlab['usage_ping_enabled'].nil?
+Settings.gitlab['prometheus_multiproc_dir'] ||= ENV['prometheus_multiproc_dir'] || 'tmp/prometheus_data_dir'
 
 #
 # CI

lib/gitlab/metrics.rb

@@ -78,28 +78,6 @@ module Gitlab
     def self.submit_metrics(metrics)
       prepared = prepare_metrics(metrics)
 
-      if prometheus_metrics_enabled?
-        metrics.map do |metric|
-          known = [:series, :tags,:values, :timestamp]
-          value = metric&.[](:values)&.[](:value)
-          handled=  [:rails_gc_statistics]
-          if handled.include? metric[:series].to_sym
-            next
-          end
-
-          if metric.keys.any? {|k| !known.include?(k)} || value.nil?
-            print metric
-            print "\n"
-
-            {:series=>"rails_gc_statistics", :tags=>{}, :values=>{:count=>0, :heap_allocated_pages=>4245, :heap_sorted_length=>4426, :heap_allocatable_pages=>0, :heap_available_slots=>1730264, :heap_live_slots=>1729935, :heap_free_slots=>329, :heap_final_slots=>0, :heap_marked_slots=>1184216, :heap_swept_slots=>361843, :heap_eden_pages=>4245, :heap_tomb_pages=>0, :total_allocated_pages=>4245, :total_freed_pages=>0, :total_allocated_objects=>15670757, :total_freed_objects=>13940822, :malloc_increase_bytes=>4842256, :malloc_increase_bytes_limit=>29129457, :minor_gc_count=>0, :major_gc_count=>0, :remembered_wb_unprotected_objects=>39905, :remembered_wb_unprotected_objects_limit=>74474, :old_objects=>1078731, :old_objects_limit=>1975860, :oldmalloc_increase_bytes=>4842640, :oldmalloc_increase_bytes_limit=>31509677, :total_time=>0.0}, :timestamp=>1494356175592659968}
-
-            next
-          end
-          metric_value = gauge(metric[:series].to_sym, metric[:series])
-          metric_value.set(metric[:tags], value)
-        end
-      end
-
       pool&.with do |connection|
         prepared.each_slice(settings[:packet_size]) do |slice|
           begin

spec/controllers/health_controller_spec.rb

@@ -54,43 +54,4 @@ describe HealthController do
       end
     end
   end
-
-  describe '#metrics' do
-    context 'authorization token provided' do
-      before do
-        request.headers['TOKEN'] = token
-      end
-
-      it 'returns DB ping metrics' do
-        get :metrics
-        expect(response.body).to match(/^db_ping_timeout 0$/)
-        expect(response.body).to match(/^db_ping_success 1$/)
-        expect(response.body).to match(/^db_ping_latency [0-9\.]+$/)
-      end
-
-      it 'returns Redis ping metrics' do
-        get :metrics
-        expect(response.body).to match(/^redis_ping_timeout 0$/)
-        expect(response.body).to match(/^redis_ping_success 1$/)
-        expect(response.body).to match(/^redis_ping_latency [0-9\.]+$/)
-      end
-
-      it 'returns file system check metrics' do
-        get :metrics
-        expect(response.body).to match(/^filesystem_access_latency{shard="default"} [0-9\.]+$/)
-        expect(response.body).to match(/^filesystem_accessible{shard="default"} 1$/)
-        expect(response.body).to match(/^filesystem_write_latency{shard="default"} [0-9\.]+$/)
-        expect(response.body).to match(/^filesystem_writable{shard="default"} 1$/)
-        expect(response.body).to match(/^filesystem_read_latency{shard="default"} [0-9\.]+$/)
-        expect(response.body).to match(/^filesystem_readable{shard="default"} 1$/)
-      end
-    end
-
-    context 'without authorization token' do
-      it 'returns proper response' do
-        get :metrics
-        expect(response.status).to eq(404)
-      end
-    end
-  end
 end

spec/controllers/metrics_controller_spec.rb

@@ -0,0 +1,51 @@
+require 'spec_helper'
+
+describe MetricsController do
+  include StubENV
+
+  let(:token) { current_application_settings.health_check_access_token }
+  let(:json_response) { JSON.parse(response.body) }
+
+  before do
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+  end
+
+  describe '#metrics' do
+    context 'authorization token provided' do
+      before do
+        request.headers['TOKEN'] = token
+      end
+
+      it 'returns DB ping metrics' do
+        get :metrics
+        expect(response.body).to match(/^db_ping_timeout 0$/)
+        expect(response.body).to match(/^db_ping_success 1$/)
+        expect(response.body).to match(/^db_ping_latency [0-9\.]+$/)
+      end
+
+      it 'returns Redis ping metrics' do
+        get :metrics
+        expect(response.body).to match(/^redis_ping_timeout 0$/)
+        expect(response.body).to match(/^redis_ping_success 1$/)
+        expect(response.body).to match(/^redis_ping_latency [0-9\.]+$/)
+      end
+
+      it 'returns file system check metrics' do
+        get :metrics
+        expect(response.body).to match(/^filesystem_access_latency{shard="default"} [0-9\.]+$/)
+        expect(response.body).to match(/^filesystem_accessible{shard="default"} 1$/)
+        expect(response.body).to match(/^filesystem_write_latency{shard="default"} [0-9\.]+$/)
+        expect(response.body).to match(/^filesystem_writable{shard="default"} 1$/)
+        expect(response.body).to match(/^filesystem_read_latency{shard="default"} [0-9\.]+$/)
+        expect(response.body).to match(/^filesystem_readable{shard="default"} 1$/)
+      end
+    end
+
+    context 'without authorization token' do
+      it 'returns proper response' do
+        get :metrics
+        expect(response.status).to eq(404)
+      end
+    end
+  end
+end