Add status endpoint to seperate webserver (#4441)

* Add status endpoint to seperate webserver

The /internal/v4/status endpoint gives more fine-granular insights in the health state of the CC.
When at leats one Puma worker is idling it returns OK. If all workers are occupied, it returns BUSY.
If all workers are busy and since 60s no requests have been processed, it returns UNHEALTHY.
This endpoint will be used to prevent restarts even if CC is still healthy and working off peak-loads.

* Refactor runner and move seperate webserver into own class

Author: svkrieger

lib/cloud_controller/metrics_webserver.rb

@@ -0,0 +1,98 @@
+require 'rack'
+require 'prometheus/middleware/exporter'
+
+module VCAP
+  module CloudController
+    class MetricsWebserver
+      attr_reader :app
+
+      def initialize
+        @app = build_app
+      end
+
+      def start(config)
+        server = Puma::Server.new(@app)
+
+        if config.get(:nginx, :metrics_socket).nil? || config.get(:nginx, :metrics_socket).empty?
+          server.add_tcp_listener('127.0.0.1', 9395)
+        else
+          server.add_unix_listener(config.get(:nginx, :metrics_socket))
+        end
+
+        server.run
+      end
+
+      private
+
+      def build_app
+        status_proc = method(:status)
+        Rack::Builder.new do
+          use Prometheus::Middleware::Exporter, path: '/internal/v4/metrics'
+
+          map '/internal/v4/status' do
+            run ->(_env) { status_proc.call }
+          end
+
+          map '/' do
+            run lambda { |_env|
+              # Return 404 for any other request
+              ['404', { 'Content-Type' => 'text/plain' }, ['Not Found']]
+            }
+          end
+        end
+      end
+
+      def status
+        stats = Puma.stats_hash
+        worker_statuses = stats[:worker_status]
+
+        all_busy = all_workers_busy?(worker_statuses)
+        current_requests_count_sum = worker_requests_count_sum(worker_statuses)
+
+        track_request_count_increase(current_requests_count_sum)
+
+        unhealthy = determine_unhealthy_state(all_busy)
+
+        build_status_response(all_busy, unhealthy)
+      rescue StandardError => e
+        [500, { 'Content-Type' => 'text/plain' }, ["Readiness check error: #{e}"]]
+      end
+
+      def track_request_count_increase(current_requests_count_sum)
+        now = Time.now
+        prev = @previous_requests_count_sum
+
+        @last_requests_count_increase_time = now if prev.nil? || current_requests_count_sum > prev
+        @previous_requests_count_sum = current_requests_count_sum
+      end
+
+      def determine_unhealthy_state(all_busy)
+        return false unless all_busy && @last_requests_count_increase_time
+
+        (Time.now - @last_requests_count_increase_time) > 60
+      end
+
+      def build_status_response(all_busy, unhealthy)
+        if all_busy && unhealthy
+          [503, { 'Content-Type' => 'text/plain' }, ['UNHEALTHY']]
+        elsif all_busy
+          [429, { 'Content-Type' => 'text/plain' }, ['BUSY']]
+        else
+          [200, { 'Content-Type' => 'text/plain' }, ['OK']]
+        end
+      end
+
+      def all_workers_busy?(worker_statuses)
+        worker_statuses.all? do |worker|
+          worker[:last_status][:busy_threads] == worker[:last_status][:running]
+        end
+      end
+
+      def worker_requests_count_sum(worker_statuses)
+        worker_statuses.sum do |worker|
+          worker[:last_status][:requests_count] || 0
+        end
+      end
+    end
+  end
+end

lib/cloud_controller/runner.rb

@@ -13,6 +13,7 @@
 require 'cloud_controller/secrets_fetcher'
 require 'cloud_controller/runners/thin_runner'
 require 'cloud_controller/runners/puma_runner'
+require 'cloud_controller/metrics_webserver'
 require 'prometheus/client/data_stores/direct_file_store'
 require 'prometheus/middleware/exporter'
 
@@ -133,33 +134,8 @@ def setup_metrics
 
       Prometheus::Client.config.data_store = Prometheus::Client::DataStores::DirectFileStore.new(dir: prometheus_dir)
 
-      setup_metrics_webserver
-    end
-
-    # The webserver runs in the main process and serves only the metrics endpoint.
-    # This makes it possible to retrieve metrics even if all Puma workers of the main app are busy.
-    def setup_metrics_webserver
-      metrics_app = Rack::Builder.new do
-        use Prometheus::Middleware::Exporter, path: '/internal/v4/metrics'
-
-        map '/' do
-          run lambda { |_env|
-            # Return 404 for any other request
-            ['404', { 'Content-Type' => 'text/plain' }, ['Not Found']]
-          }
-        end
-      end
-
       Thread.new do
-        server = Puma::Server.new(metrics_app)
-
-        if config.get(:nginx, :metrics_socket).nil? || config.get(:nginx, :metrics_socket).empty?
-          server.add_tcp_listener('127.0.0.1', 9395)
-        else
-          server.add_unix_listener(@config.get(:nginx, :metrics_socket))
-        end
-
-        server.run
+        VCAP::CloudController::MetricsWebserver.new.start(@config)
       end
     end
 

spec/request/status_spec.rb

@@ -0,0 +1,69 @@
+require 'spec_helper'
+
+module VCAP::CloudController
+  RSpec.describe 'Status Endpoint' do
+    include Rack::Test::Methods
+
+    let(:metrics_webserver) { MetricsWebserver.new }
+
+    delegate :app, to: :metrics_webserver
+
+    describe 'GET /internal/v4/status' do
+      context 'when all workers are busy and unhealthy' do
+        before do
+          allow(Puma).to receive(:stats_hash).and_return(
+            worker_status: [
+              { last_status: { busy_threads: 2, running: 2, requests_count: 5 } },
+              { last_status: { busy_threads: 1, running: 1, requests_count: 3 } }
+            ]
+          )
+          allow(metrics_webserver).to receive(:determine_unhealthy_state).and_return(true)
+        end
+
+        it 'returns 503 UNHEALTHY' do
+          get '/internal/v4/status'
+
+          expect(last_response.status).to eq(503)
+          expect(last_response.body).to eq('UNHEALTHY')
+        end
+      end
+
+      context 'when all workers are busy but not unhealthy' do
+        before do
+          allow(Puma).to receive(:stats_hash).and_return(
+            worker_status: [
+              { last_status: { busy_threads: 2, running: 2, requests_count: 5 } },
+              { last_status: { busy_threads: 1, running: 1, requests_count: 3 } }
+            ]
+          )
+          allow(metrics_webserver).to receive(:determine_unhealthy_state).and_return(false)
+        end
+
+        it 'returns 429 BUSY' do
+          get '/internal/v4/status'
+
+          expect(last_response.status).to eq(429)
+          expect(last_response.body).to eq('BUSY')
+        end
+      end
+
+      context 'when not all workers are busy' do
+        before do
+          allow(Puma).to receive(:stats_hash).and_return(
+            worker_status: [
+              { last_status: { busy_threads: 1, running: 2, requests_count: 5 } },
+              { last_status: { busy_threads: 0, running: 1, requests_count: 3 } }
+            ]
+          )
+        end
+
+        it 'returns 200 OK' do
+          get '/internal/v4/status'
+
+          expect(last_response.status).to eq(200)
+          expect(last_response.body).to eq('OK')
+        end
+      end
+    end
+  end
+end

spec/unit/lib/cloud_controller/metrics_webserver_spec.rb

@@ -0,0 +1,41 @@
+require 'spec_helper'
+
+module VCAP::CloudController
+  RSpec.describe MetricsWebserver do
+    let(:metrics_webserver) { described_class.new }
+    let(:config) { double('config', get: nil) }
+
+    describe '#start' do
+      it 'configures and starts a Puma server' do
+        allow(Puma::Server).to receive(:new).and_call_original
+        expect_any_instance_of(Puma::Server).to receive(:run)
+
+        metrics_webserver.start(config)
+      end
+
+      context 'when no socket is specified' do
+        before do
+          allow(config).to receive(:get).with(:nginx, :metrics_socket).and_return(nil)
+        end
+
+        it 'uses a TCP listener' do
+          expect_any_instance_of(Puma::Server).to receive(:add_tcp_listener).with('127.0.0.1', 9395)
+
+          metrics_webserver.start(config)
+        end
+      end
+
+      context 'when a socket is specified' do
+        before do
+          allow(config).to receive(:get).with(:nginx, :metrics_socket).and_return('/tmp/metrics.sock')
+        end
+
+        it 'uses a Unix socket listener' do
+          expect_any_instance_of(Puma::Server).to receive(:add_unix_listener).with('/tmp/metrics.sock')
+
+          metrics_webserver.start(config)
+        end
+      end
+    end
+  end
+end

spec/unit/lib/cloud_controller/runner_spec.rb

@@ -290,36 +290,21 @@ module VCAP::CloudController
         context 'when the webserver is puma' do
           let(:test_config_overrides) { super().merge(webserver: 'puma') }
 
-          it 'sets up a separate webserver for metrics' do
-            subject
-
-            expect(Puma::Server).to have_received(:new).once
-          end
+          it 'starts the MetricsWebserver' do
+            expect(MetricsWebserver).to receive(:new).and_call_original
+            expect_any_instance_of(MetricsWebserver).to receive(:start)
 
-          it 'listens on the specified metrics port' do
             subject
-
-            expect(puma_server_double).to have_received(:add_tcp_listener).with('127.0.0.1', 9395)
-          end
-
-          context 'metrics_socket is configured' do
-            let(:test_config_overrides) { super().merge(nginx: { metrics_socket: '/tmp/metrics_socket.sock' }) }
-
-            it 'listens on the specified metrics socket' do
-              subject
-
-              expect(puma_server_double).to have_received(:add_unix_listener).with('/tmp/metrics_socket.sock')
-            end
           end
         end
 
         context 'when the webserver is not puma' do
           let(:test_config_overrides) { super().merge(webserver: 'thin') }
 
-          it 'does not set up the webserver' do
-            subject
+          it 'does not start the MetricsWebserver' do
+            expect(MetricsWebserver).not_to receive(:new)
 
-            expect(Puma::Server).not_to have_received(:new)
+            subject
           end
         end
       end