Fix code style and unit tests

Author: svkrieger

lib/cloud_controller/db.rb

@@ -71,7 +71,7 @@ def self.add_connection_expiration_extension(db, opts)
     end
 
     def self.add_connection_metrics_extension(db)
-      # only add the metrics for api processes. Otherwise e.g. rake db:migrate would also initialize metric updaters, which need additional config
+      # only add the metrics for api and cc-worker processes. Otherwise e.g. rake db:migrate would also initialize metric updaters, which need additional config
       return if Object.const_defined?(:RakeConfig) && RakeConfig.context != :worker
 
       db.extension(:connection_metrics)

lib/cloud_controller/metrics/prometheus_updater.rb

@@ -81,10 +81,6 @@ def initialize(registry: Prometheus::Client.registry, cc_worker: false)
       PUMA_METRICS.each { |metric| register(metric) } if VCAP::CloudController::Config.config&.get(:webserver) == 'puma'
     end
 
-    def registry
-      @registry
-    end
-
     def update_gauge_metric(metric, value, labels: {})
       @registry.get(metric).set(value, labels:)
     end

lib/delayed_job/delayed_worker.rb

@@ -22,7 +22,7 @@ def initialize(options)
 
   def start_working
     config = RakeConfig.config
-    setup_metrics_endpoint(config) if @publish_metrics
+    setup_metrics(config) if @publish_metrics
     BackgroundJobEnvironment.new(config).setup_environment(readiness_port)
 
     logger = Steno.logger('cc-worker')
@@ -99,36 +99,39 @@ def is_first_generic_worker_on_machine?
     RakeConfig.context != :api && ENV['INDEX']&.to_i == 1
   end
 
-  def setup_metrics_endpoint(config)
+  def setup_metrics(config)
     prometheus_dir = File.join(config.get(:directories, :tmpdir), 'prometheus')
     Prometheus::Client.config.data_store = Prometheus::Client::DataStores::DirectFileStore.new(dir: prometheus_dir)
 
-    if is_first_generic_worker_on_machine?
-      FileUtils.mkdir_p(prometheus_dir)
+    setup_webserver(config, prometheus_dir) if is_first_generic_worker_on_machine?
 
-      # Resetting metrics on startup
-      Dir["#{prometheus_dir}/*.bin"].each do |file_path|
-        File.unlink(file_path)
-      end
+    # initialize metric with 0 for discoverability, because it likely won't get updated on healthy systems
+    CloudController::DependencyLocator.instance.cc_worker_prometheus_updater.update_gauge_metric(:cc_db_connection_pool_timeouts_total, 0, labels: { process_type: 'cc-worker' })
+  end
 
-      metrics_app = Rack::Builder.new do
-        use Prometheus::Middleware::Exporter, path: '/metrics'
+  def setup_webserver(config, prometheus_dir)
+    FileUtils.mkdir_p(prometheus_dir)
 
-        map '/' do
-          run lambda { |env|
-            # Return 404 for any other request
-            ['404', { 'Content-Type' => 'text/plain' }, ['Not Found']]
-          }
-        end
-      end
+    # Resetting metrics on startup
+    Dir["#{prometheus_dir}/*.bin"].each do |file_path|
+      File.unlink(file_path)
+    end
+
+    metrics_app = Rack::Builder.new do
+      use Prometheus::Middleware::Exporter, path: '/metrics'
 
-      Thread.new do
-        server = Puma::Server.new(metrics_app)
-        server.add_tcp_listener '0.0.0.0', config.get(:prometheus_port) || 9394
-        server.run
+      map '/' do
+        run lambda { |_env|
+          # Return 404 for any other request
+          ['404', { 'Content-Type' => 'text/plain' }, ['Not Found']]
+        }
       end
     end
 
-    CloudController::DependencyLocator.instance.cc_worker_prometheus_updater.update_gauge_metric(:cc_db_connection_pool_timeouts_total, 0, labels: { process_type: 'cc-worker' })
+    Thread.new do
+      server = Puma::Server.new(metrics_app)
+      server.add_tcp_listener '0.0.0.0', config.get(:prometheus_port) || 9394
+      server.run
+    end
   end
 end

lib/sequel/extensions/connection_metrics.rb

@@ -24,9 +24,9 @@ def self.extended(pool)
       pool.instance_exec do
         sync do
           @prometheus_updater = if process_type == 'cc-worker'
-            CloudController::DependencyLocator.instance.cc_worker_prometheus_updater
+                                  CloudController::DependencyLocator.instance.cc_worker_prometheus_updater
                                 else
-            CloudController::DependencyLocator.instance.prometheus_updater
+                                  CloudController::DependencyLocator.instance.prometheus_updater
                                 end
           @connection_info = {}
         end

spec/unit/lib/cloud_controller/metrics/prometheus_updater_spec.rb

@@ -3,7 +3,7 @@
 
 module VCAP::CloudController::Metrics
   RSpec.describe PrometheusUpdater do
-    let(:updater) { PrometheusUpdater.new(prom_client) }
+    let(:updater) { PrometheusUpdater.new(registry: prom_client) }
     let(:tmpdir) { Dir.mktmpdir }
     let(:prom_client) do
       Prometheus::Client.config.data_store = Prometheus::Client::DataStores::DirectFileStore.new(dir: tmpdir)

spec/unit/lib/delayed_job/delayed_worker_spec.rb

@@ -64,6 +64,26 @@
       worker_instance = CloudController::DelayedWorker.new(options)
       expect(worker_instance.instance_variable_get(:@queue_options)).not_to include(:grace_period_seconds)
     end
+
+    describe 'publish metrics' do
+      context 'when not set' do
+        it 'does not publish metrics' do
+          worker_instance = CloudController::DelayedWorker.new(options)
+          expect(worker_instance.instance_variable_get(:@publish_metrics)).to be(false)
+        end
+      end
+
+      context 'when set to true' do
+        before do
+          options[:publish_metrics] = true
+        end
+
+        it 'publishes metrics' do
+          worker_instance = CloudController::DelayedWorker.new(options)
+          expect(worker_instance.instance_variable_get(:@publish_metrics)).to be(true)
+        end
+      end
+    end
   end
 
   describe '#start_working' do
@@ -120,6 +140,47 @@
         cc_delayed_worker.start_working
       end
     end
+
+    describe 'publish metrics' do
+      before do
+        allow(Prometheus::Client::DataStores::DirectFileStore).to receive(:new)
+      end
+
+      context 'when set to false' do
+        before do
+          options[:publish_metrics] = false
+        end
+
+        it 'does not publish metrics' do
+          cc_delayed_worker.start_working
+          expect(Prometheus::Client::DataStores::DirectFileStore).not_to have_received(:new)
+        end
+      end
+
+      context 'when set to true' do
+        before do
+          options[:publish_metrics] = true
+        end
+
+        it 'publishes metrics' do
+          cc_delayed_worker.start_working
+          expect(Prometheus::Client::DataStores::DirectFileStore).to have_received(:new)
+        end
+
+        context 'when first worker on machine' do
+          before do
+            allow(cc_delayed_worker).to receive(:is_first_generic_worker_on_machine?).and_return(true)
+            allow(cc_delayed_worker).to receive(:readiness_port)
+            allow(cc_delayed_worker).to receive(:setup_webserver)
+          end
+
+          it 'sets up a webserver' do
+            cc_delayed_worker.start_working
+            expect(cc_delayed_worker).to have_received(:setup_webserver)
+          end
+        end
+      end
+    end
   end
 
   describe '#clear_locks!' do