Merge branch 'web-metrics-exporter-ee' into 'master'

Allow additional method of gathering web metrics

See merge request gitlab-org/gitlab!17943

changelogs/unreleased/web-metrics-exporter-ee.yml

+---
+title: Add web_exporter to expose Prometheus metrics
+merge_request: 17943
+author:
+type: added

config/gitlab.yml.example

@@ -1017,7 +1017,14 @@ production: &base
     sidekiq_exporter:
     #  enabled: true
     #  address: localhost
-    #  port: 3807
+    #  port: 8082
+
+    # Web exporter is webserver built in to Unicorn/Puma to expose Prometheus metrics
+    # It runs alongside the `/metrics` endpoints to ease the publish of metrics
+    web_exporter:
+    #  enabled: true
+    #  address: localhost
+    #  port: 8083
 
   ## Prometheus settings
   # Do not modify these settings here. They should be modified in /etc/gitlab/gitlab.rb
@@ -1061,6 +1068,21 @@ production: &base
 development:
   <<: *base
 
+  # We want to run web/sidekiq exporters for devs
+  # to catch errors from using them.
+  #
+  # We use random port to not block ability to run
+  # multiple instances of the service
+  monitoring:
+    sidekiq_exporter:
+      enabled: true
+      address: 127.0.0.1
+      port: 0
+    web_exporter:
+      enabled: true
+      address: 127.0.0.1
+      port: 0
+
 test:
   <<: *base
   gravatar:

config/initializers/1_settings.rb

@@ -663,7 +663,11 @@ Settings.monitoring['ruby_sampler_interval'] ||= 60
 Settings.monitoring['sidekiq_exporter'] ||= Settingslogic.new({})
 Settings.monitoring.sidekiq_exporter['enabled'] ||= false
 Settings.monitoring.sidekiq_exporter['address'] ||= 'localhost'
-Settings.monitoring.sidekiq_exporter['port'] ||= 3807
+Settings.monitoring.sidekiq_exporter['port'] ||= 8082
+Settings.monitoring['web_exporter'] ||= Settingslogic.new({})
+Settings.monitoring.web_exporter['enabled'] ||= false
+Settings.monitoring.web_exporter['address'] ||= 'localhost'
+Settings.monitoring.web_exporter['port'] ||= 8083
 
 #
 # Testing settings

config/initializers/7_prometheus_metrics.rb

@@ -35,7 +35,7 @@ Sidekiq.configure_server do |config|
     # webserver metrics are cleaned up in config.ru: `warmup` block
     Prometheus::CleanupMultiprocDirService.new.execute
 
-    Gitlab::Metrics::SidekiqMetricsExporter.instance.start
+    Gitlab::Metrics::Exporter::SidekiqExporter.instance.start
   end
 end
 
@@ -58,3 +58,24 @@ if !Rails.env.test? && Gitlab::Metrics.prometheus_metrics_enabled?
     Gitlab::Metrics::RequestsRackMiddleware.initialize_http_request_duration_seconds
   end
 end
+
+if defined?(::Unicorn) || defined?(::Puma)
+  Gitlab::Cluster::LifecycleEvents.on_master_start do
+    Gitlab::Metrics::Exporter::WebExporter.instance.start
+  end
+
+  Gitlab::Cluster::LifecycleEvents.on_before_master_restart do
+    # We need to ensure that before we re-exec server
+    # we do stop the exporter
+    Gitlab::Metrics::Exporter::WebExporter.instance.stop
+  end
+
+  Gitlab::Cluster::LifecycleEvents.on_worker_start do
+    # The `#close_on_exec=` takes effect only on `execve`
+    # but this does not happen for Ruby fork
+    #
+    # This does stop server, as it is running on master.
+    # However, ensures that we close the TCPSocket.
+    Gitlab::Metrics::Exporter::WebExporter.instance.stop
+  end
+end

config/puma.example.development.rb

@@ -45,7 +45,7 @@ require_relative "/home/git/gitlab/lib/gitlab/cluster/lifecycle_events"
 
 on_restart do
   # Signal application hooks that we're about to restart
-  Gitlab::Cluster::LifecycleEvents.do_master_restart
+  Gitlab::Cluster::LifecycleEvents.do_before_master_restart
 end
 
 before_fork do

config/puma.rb.example

@@ -40,7 +40,7 @@ require_relative "/home/git/gitlab/lib/gitlab/cluster/puma_worker_killer_initial
 
 on_restart do
   # Signal application hooks that we're about to restart
-  Gitlab::Cluster::LifecycleEvents.do_master_restart
+  Gitlab::Cluster::LifecycleEvents.do_before_master_restart
 end
 
 before_fork do

config/unicorn.rb.example

@@ -85,7 +85,7 @@ require_relative "/home/git/gitlab/lib/gitlab/cluster/lifecycle_events"
 
 before_exec do |server|
   # Signal application hooks that we're about to restart
-  Gitlab::Cluster::LifecycleEvents.do_master_restart
+  Gitlab::Cluster::LifecycleEvents.do_before_master_restart
 end
 
 run_once = true

config/unicorn.rb.example.development

@@ -18,7 +18,7 @@ require_relative "/home/git/gitlab/lib/gitlab/cluster/lifecycle_events"
 
 before_exec do |server|
   # Signal application hooks that we're about to restart
-  Gitlab::Cluster::LifecycleEvents.do_master_restart
+  Gitlab::Cluster::LifecycleEvents.do_before_master_restart
 end
 
 run_once = true

doc/administration/logs.md

@@ -327,14 +327,20 @@ is populated whenever `gitlab-ctl reconfigure` is run manually or as part of an
 Reconfigure logs files are named according to the UNIX timestamp of when the reconfigure
 was initiated, such as `1509705644.log`
 
-## `sidekiq_exporter.log`
+## `sidekiq_exporter.log` and `web_exporter.log`
 
 If Prometheus metrics and the Sidekiq Exporter are both enabled, Sidekiq will
-start a Web server and listen to the defined port (default: 3807). Access logs
+start a Web server and listen to the defined port (default: 8082). Access logs
 will be generated in `/var/log/gitlab/gitlab-rails/sidekiq_exporter.log` for
 Omnibus GitLab packages or in `/home/git/gitlab/log/sidekiq_exporter.log` for
 installations from source.
 
+If Prometheus metrics and the Web Exporter are both enabled, Unicorn/Puma will
+start a Web server and listen to the defined port (default: 8083). Access logs
+will be generated in `/var/log/gitlab/gitlab-rails/web_exporter.log` for
+Omnibus GitLab packages or in `/home/git/gitlab/log/web_exporter.log` for
+installations from source.
+
 [repocheck]: repository_checks.md
 [Rack Attack]: ../security/rack_attack.md
 [Rate Limit]: ../user/admin_area/settings/rate_limits_on_raw_endpoints.md

lib/gitlab/cluster/lifecycle_events.rb

@@ -40,7 +40,7 @@ module Gitlab
           (@before_fork_hooks ||= []) << block
         end
 
-        def on_master_restart(&block)
+        def on_before_master_restart(&block)
           return unless in_clustered_environment?
 
           # Defer block execution
@@ -70,12 +70,15 @@ module Gitlab
           end
         end
 
-        def do_master_restart
+        def do_before_master_restart
           @master_restart_hooks && @master_restart_hooks.each do |block|
             block.call
           end
         end
 
+        # DEPRECATED
+        alias_method :do_master_restart, :do_before_master_restart
+
         # Puma doesn't use singletons (which is good) but
         # this means we need to pass through whether the
         # puma server is running in single mode or cluster mode

lib/gitlab/metrics/exporter/base_exporter.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Metrics
+    module Exporter
+      class BaseExporter < Daemon
+        attr_reader :server
+
+        def enabled?
+          settings.enabled
+        end
+
+        def settings
+          raise NotImplementedError
+        end
+
+        def log_filename
+          raise NotImplementedError
+        end
+
+        private
+
+        def start_working
+          logger = WEBrick::Log.new(log_filename)
+          logger.time_format = "[%Y-%m-%dT%H:%M:%S.%L%z]"
+
+          access_log = [
+            [logger, WEBrick::AccessLog::COMBINED_LOG_FORMAT]
+          ]
+
+          @server = ::WEBrick::HTTPServer.new(
+            Port: settings.port, BindAddress: settings.address,
+            Logger: logger, AccessLog: access_log)
+          server.mount "/", Rack::Handler::WEBrick, rack_app
+          server.start
+        end
+
+        def stop_working
+          if server # rubocop:disable Cop/LineBreakAroundConditionalBlock
+            server.shutdown
+            server.listeners.each(&:close)
+          end
+          @server = nil
+        end
+
+        def rack_app
+          Rack::Builder.app do
+            use Rack::Deflater
+            use ::Prometheus::Client::Rack::Exporter if ::Gitlab::Metrics.metrics_folder_present?
+            run -> (env) { [404, {}, ['']] }
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/metrics/exporter/sidekiq_exporter.rb

+# frozen_string_literal: true
+
+require 'webrick'
+require 'prometheus/client/rack/exporter'
+
+module Gitlab
+  module Metrics
+    module Exporter
+      class SidekiqExporter < BaseExporter
+        def settings
+          Settings.monitoring.sidekiq_exporter
+        end
+
+        def log_filename
+          File.join(Rails.root, 'log', 'sidekiq_exporter.log')
+        end
+      end
+    end
+  end
+end

lib/gitlab/metrics/exporter/web_exporter.rb

+# frozen_string_literal: true
+
+require 'webrick'
+require 'prometheus/client/rack/exporter'
+
+module Gitlab
+  module Metrics
+    module Exporter
+      class WebExporter < BaseExporter
+        def settings
+          Settings.monitoring.web_exporter
+        end
+
+        def log_filename
+          File.join(Rails.root, 'log', 'web_exporter.log')
+        end
+      end
+    end
+  end
+end

lib/gitlab/metrics/sidekiq_metrics_exporter.rb

-# frozen_string_literal: true
-
-require 'webrick'
-require 'prometheus/client/rack/exporter'
-
-module Gitlab
-  module Metrics
-    class SidekiqMetricsExporter < Daemon
-      LOG_FILENAME = File.join(Rails.root, 'log', 'sidekiq_exporter.log')
-
-      def enabled?
-        ::Gitlab::Metrics.metrics_folder_present? && settings.enabled
-      end
-
-      def settings
-        Settings.monitoring.sidekiq_exporter
-      end
-
-      private
-
-      attr_reader :server
-
-      def start_working
-        logger = WEBrick::Log.new(LOG_FILENAME)
-        access_log = [
-          [logger, WEBrick::AccessLog::COMBINED_LOG_FORMAT]
-        ]
-
-        @server = ::WEBrick::HTTPServer.new(Port: settings.port, BindAddress: settings.address,
-                                            Logger: logger, AccessLog: access_log)
-        server.mount "/", Rack::Handler::WEBrick, rack_app
-        server.start
-      end
-
-      def stop_working
-        server.shutdown if server
-        @server = nil
-      end
-
-      def rack_app
-        Rack::Builder.app do
-          use Rack::Deflater
-          use ::Prometheus::Client::Rack::Exporter
-          run -> (env) { [404, {}, ['']] }
-        end
-      end
-    end
-  end
-end

spec/lib/gitlab/metrics/sidekiq_metrics_exporter_spec.rb → spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb

@@ -2,20 +2,29 @@
 
 require 'spec_helper'
 
-describe Gitlab::Metrics::SidekiqMetricsExporter do
+describe Gitlab::Metrics::Exporter::BaseExporter do
   let(:exporter) { described_class.new }
   let(:server) { double('server') }
+  let(:socket) { double('socket') }
+  let(:log_filename) { File.join(Rails.root, 'log', 'sidekiq_exporter.log') }
+  let(:settings) { double('settings') }
 
   before do
     allow(::WEBrick::HTTPServer).to receive(:new).and_return(server)
     allow(server).to receive(:mount)
     allow(server).to receive(:start)
     allow(server).to receive(:shutdown)
+    allow(server).to receive(:listeners) { [socket] }
+    allow(socket).to receive(:close)
+    allow_any_instance_of(described_class).to receive(:log_filename).and_return(log_filename)
+    allow_any_instance_of(described_class).to receive(:settings).and_return(settings)
   end
 
   describe 'when exporter is enabled' do
     before do
-      allow(Settings.monitoring.sidekiq_exporter).to receive(:enabled).and_return(true)
+      allow(settings).to receive(:enabled).and_return(true)
+      allow(settings).to receive(:port).and_return(3707)
+      allow(settings).to receive(:address).and_return('localhost')
     end
 
     describe 'when exporter is stopped' do
@@ -31,8 +40,8 @@ describe Gitlab::Metrics::SidekiqMetricsExporter do
           let(:address) { 'sidekiq_exporter_address' }
 
           before do
-            allow(Settings.monitoring.sidekiq_exporter).to receive(:port).and_return(port)
-            allow(Settings.monitoring.sidekiq_exporter).to receive(:address).and_return(address)
+            allow(settings).to receive(:port).and_return(port)
+            allow(settings).to receive(:address).and_return(address)
           end
 
           it 'starts server with port and address from settings' do
@@ -74,6 +83,7 @@ describe Gitlab::Metrics::SidekiqMetricsExporter do
         it 'shutdowns server' do
           expect { exporter.stop }.to change { exporter.thread? }.from(true).to(false)
 
+          expect(socket).to have_received(:close)
           expect(server).to have_received(:shutdown)
         end
       end
@@ -82,7 +92,7 @@ describe Gitlab::Metrics::SidekiqMetricsExporter do
 
   describe 'when exporter is disabled' do
     before do
-      allow(Settings.monitoring.sidekiq_exporter).to receive(:enabled).and_return(false)
+      allow(settings).to receive(:enabled).and_return(false)
     end
 
     describe '#start' do