From ae8f7666e597493ab404f8524c1216a924338291 Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Mon, 29 May 2017 23:23:19 +0200
Subject: [PATCH] Add prometheus text formatter  + rename controler method to
 #index from #metrics  + remove assertion from nullMetric

---
 app/services/metrics_service.rb               |  4 +-
 config/routes.rb                              |  2 +-
 ...heus_text.rb => prometheus_text_format.rb} |  2 +-
 lib/gitlab/metrics/null_metric.rb             |  9 ----
 spec/controllers/metrics_controller_spec.rb   | 12 ++---
 .../prometheus_text_format_spec.rb            | 44 +++++++++++++++++++
 6 files changed, 54 insertions(+), 19 deletions(-)
 rename lib/gitlab/health_checks/{prometheus_text.rb => prometheus_text_format.rb} (96%)
 create mode 100644 spec/lib/gitlab/health_checks/prometheus_text_format_spec.rb

diff --git a/app/services/metrics_service.rb b/app/services/metrics_service.rb
index 0faa86a228..025598cdc7 100644
--- a/app/services/metrics_service.rb
+++ b/app/services/metrics_service.rb
@@ -24,11 +24,11 @@ class MetricsService
   private
 
   def formatter
-    @formatter ||= PrometheusText.new
+    @formatter ||= Gitlab::HealthChecks::PrometheusTextFormat.new
   end
 
   def multiprocess_metrics_path
-    @multiprocess_metrics_path ||= Rails.root.join(ENV['prometheus_multiproc_dir'])
+    @multiprocess_metrics_path ||= Rails.root.join(ENV['prometheus_multiproc_dir']).freeze
   end
 
   def metric_to_prom_line(metric)
diff --git a/config/routes.rb b/config/routes.rb
index 4051c33d5d..d909be38b4 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -41,7 +41,7 @@ Rails.application.routes.draw do
   scope path: '-' do
     get 'liveness' => 'health#liveness'
     get 'readiness' => 'health#readiness'
-    get 'metrics' => 'metrics#metrics'
+    resources :metrics, only: [:index]
   end
 
   # Koding route
diff --git a/lib/gitlab/health_checks/prometheus_text.rb b/lib/gitlab/health_checks/prometheus_text_format.rb
similarity index 96%
rename from lib/gitlab/health_checks/prometheus_text.rb
rename to lib/gitlab/health_checks/prometheus_text_format.rb
index a01e6b2be1..5fc6f19c37 100644
--- a/lib/gitlab/health_checks/prometheus_text.rb
+++ b/lib/gitlab/health_checks/prometheus_text_format.rb
@@ -1,5 +1,5 @@
 module Gitlab::HealthChecks
-  class PrometheusText
+  class PrometheusTextFormat
     def marshal(metrics)
       metrics_with_type_declarations(metrics).join("\n")
     end
diff --git a/lib/gitlab/metrics/null_metric.rb b/lib/gitlab/metrics/null_metric.rb
index 1501cd3867..3b5a290719 100644
--- a/lib/gitlab/metrics/null_metric.rb
+++ b/lib/gitlab/metrics/null_metric.rb
@@ -5,15 +5,6 @@ module Gitlab
       def method_missing(name, *args, &block)
         nil
       end
-
-      # these methods shouldn't be called when metrics are disabled
-      def get(*args)
-        raise NotImplementedError
-      end
-
-      def values(*args)
-        raise NotImplementedError
-      end
     end
   end
 end
diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb
index 7baf7d1bef..b26ebc1377 100644
--- a/spec/controllers/metrics_controller_spec.rb
+++ b/spec/controllers/metrics_controller_spec.rb
@@ -19,14 +19,14 @@ describe MetricsController do
     allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(true)
   end
 
-  describe '#metrics' do
+  describe '#index' do
     context 'authorization token provided' do
       before do
         request.headers['TOKEN'] = token
       end
 
       it 'returns DB ping metrics' do
-        get :metrics
+        get :index
 
         expect(response.body).to match(/^db_ping_timeout 0$/)
         expect(response.body).to match(/^db_ping_success 1$/)
@@ -34,7 +34,7 @@ describe MetricsController do
       end
 
       it 'returns Redis ping metrics' do
-        get :metrics
+        get :index
 
         expect(response.body).to match(/^redis_ping_timeout 0$/)
         expect(response.body).to match(/^redis_ping_success 1$/)
@@ -42,7 +42,7 @@ describe MetricsController do
       end
 
       it 'returns file system check metrics' do
-        get :metrics
+        get :index
 
         expect(response.body).to match(/^filesystem_access_latency{shard="default"} [0-9\.]+$/)
         expect(response.body).to match(/^filesystem_accessible{shard="default"} 1$/)
@@ -58,7 +58,7 @@ describe MetricsController do
         end
 
         it 'returns proper response' do
-          get :metrics
+          get :index
 
           expect(response.status).to eq(404)
         end
@@ -67,7 +67,7 @@ describe MetricsController do
 
     context 'without authorization token' do
       it 'returns proper response' do
-        get :metrics
+        get :index
 
         expect(response.status).to eq(404)
       end
diff --git a/spec/lib/gitlab/health_checks/prometheus_text_format_spec.rb b/spec/lib/gitlab/health_checks/prometheus_text_format_spec.rb
new file mode 100644
index 0000000000..a9feab8ff7
--- /dev/null
+++ b/spec/lib/gitlab/health_checks/prometheus_text_format_spec.rb
@@ -0,0 +1,44 @@
+describe Gitlab::HealthChecks::PrometheusTextFormat do
+  let(:metric_class) { Gitlab::HealthChecks::Metric }
+  subject { described_class.new }
+
+  describe '#marshal' do
+    let(:sample_metrics) do
+      [
+        metric_class.new('metric1', 1),
+        metric_class.new('metric2', 2)
+      ]
+    end
+
+    it 'marshal to text with non repeating type definition' do
+      expected = <<-EXPECTED
+# TYPE metric1 gauge
+metric1 1
+# TYPE metric2 gauge
+metric2 2
+EXPECTED
+      expect(subject.marshal(sample_metrics)).to eq(expected.chomp)
+    end
+
+    context 'metrics where name repeats' do
+      let(:sample_metrics) do
+        [
+          metric_class.new('metric1', 1),
+          metric_class.new('metric1', 2),
+          metric_class.new('metric2', 3)
+        ]
+      end
+
+      it 'marshal to text with non repeating type definition' do
+        expected = <<-EXPECTED
+# TYPE metric1 gauge
+metric1 1
+metric1 2
+# TYPE metric2 gauge
+metric2 3
+        EXPECTED
+        expect(subject.marshal(sample_metrics)).to eq(expected.chomp)
+      end
+    end
+  end
+end
-- 
2.30.9