From ed5c7d11b19c9507206ada5c6e12eef477370fa9 Mon Sep 17 00:00:00 2001
From: Pawel Chojnacki <pawel@chojnacki.ws>
Date: Thu, 15 Jun 2017 23:41:47 +0200
Subject: Do not enable prometheus metrics when data folder is not present.

+ Set defaults correctly only for when not in production or staging
+ set ENV['prometheus_multiproc_dir'] in config/boot.rb instead of config.ru

Test prometheus metrics unmemoized
---
 .../admin/application_settings/_form.html.haml     |  4 +++
 config/boot.rb                                     |  4 ++-
 lib/gitlab/metrics/prometheus.rb                   | 13 +++++++++-
 spec/lib/gitlab/metrics_spec.rb                    | 30 ++++++++++++++++++++++
 4 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index 95dffdafabe..b21d5665970 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -325,6 +325,10 @@
           = f.label :prometheus_metrics_enabled do
             = f.check_box :prometheus_metrics_enabled
             Enable Prometheus Metrics
+            - unless Gitlab::Metrics.metrics_folder_present?
+              .help-block
+                %strong.cred WARNING:
+                Environment variable `prometheus_multiproc_dir` does not exist or is not pointing to a valid directory.
 
   %fieldset
     %legend Background Jobs
diff --git a/config/boot.rb b/config/boot.rb
index db5ab918021..16de55d7a86 100644
--- a/config/boot.rb
+++ b/config/boot.rb
@@ -6,7 +6,9 @@ ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __FILE__)
 require 'bundler/setup' if File.exist?(ENV['BUNDLE_GEMFILE'])
 
 # set default directory for multiproces metrics gathering
-ENV['prometheus_multiproc_dir'] ||= 'tmp/prometheus_multiproc_dir'
+if ENV['RAILS_ENV'] == 'development' || ENV['RAILS_ENV'] == 'test'
+  ENV['prometheus_multiproc_dir'] ||= 'tmp/prometheus_multiproc_dir'
+end
 
 # Default Bootsnap configuration from https://github.com/Shopify/bootsnap#usage
 require 'bootsnap'
diff --git a/lib/gitlab/metrics/prometheus.rb b/lib/gitlab/metrics/prometheus.rb
index 60686509332..25421c0a92f 100644
--- a/lib/gitlab/metrics/prometheus.rb
+++ b/lib/gitlab/metrics/prometheus.rb
@@ -5,8 +5,13 @@ module Gitlab
     module Prometheus
       include Gitlab::CurrentSettings
 
+      def metrics_folder_present?
+        ENV.has_key?('prometheus_multiproc_dir') && ::Dir.exist?(ENV['prometheus_multiproc_dir']) &&
+          ::File.writable?(ENV['prometheus_multiproc_dir'])
+      end
+
       def prometheus_metrics_enabled?
-        @prometheus_metrics_enabled ||= current_application_settings[:prometheus_metrics_enabled] || false
+        @prometheus_metrics_enabled ||= prometheus_metrics_enabled_unmemoized
       end
 
       def registry
@@ -36,6 +41,12 @@ module Gitlab
           NullMetric.new
         end
       end
+
+      private
+
+      def prometheus_metrics_enabled_unmemoized
+        metrics_folder_present? && current_application_settings[:prometheus_metrics_enabled] || false
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb
index 5a87b906609..58a84cd3fe1 100644
--- a/spec/lib/gitlab/metrics_spec.rb
+++ b/spec/lib/gitlab/metrics_spec.rb
@@ -15,6 +15,36 @@ describe Gitlab::Metrics do
     end
   end
 
+  describe '.prometheus_metrics_enabled_unmemoized' do
+    subject { described_class.send(:prometheus_metrics_enabled_unmemoized) }
+
+    context 'prometheus metrics enabled in config' do
+      before do
+        allow(described_class).to receive(:current_application_settings).and_return(prometheus_metrics_enabled: true)
+      end
+
+      context 'when metrics folder is present' do
+        before do
+          allow(described_class).to receive(:metrics_folder_present?).and_return(true)
+        end
+
+        it 'metrics are enabled' do
+          expect(subject).to eq(true)
+        end
+      end
+
+      context 'when metrics folder is missing' do
+        before do
+          allow(described_class).to receive(:metrics_folder_present?).and_return(false)
+        end
+
+        it 'metrics are disabled' do
+          expect(subject).to eq(false)
+        end
+      end
+    end
+  end
+
   describe '.prometheus_metrics_enabled?' do
     it 'returns a boolean' do
       expect(described_class.prometheus_metrics_enabled?).to be_in([true, false])
-- 
cgit v1.2.1