diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cdb069d..91a7f804 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# unreleased + +* BREAKING: Implement RFC 141 - remove unsuitable healthchecks and return a 500 on healthcheck failure ([#193](https://github.com/alphagov/govuk_app_config/pull/193)) + # 2.10.0 * Allow LUX domain on img-src policy ([#191](https://github.com/alphagov/govuk_app_config/pull/191)) diff --git a/lib/govuk_app_config/govuk_healthcheck.rb b/lib/govuk_app_config/govuk_healthcheck.rb index 71e29edd..138b3919 100644 --- a/lib/govuk_app_config/govuk_healthcheck.rb +++ b/lib/govuk_app_config/govuk_healthcheck.rb @@ -4,19 +4,16 @@ require "govuk_app_config/govuk_healthcheck/rails_cache" require "govuk_app_config/govuk_healthcheck/redis" require "govuk_app_config/govuk_healthcheck/sidekiq_redis" -require "govuk_app_config/govuk_healthcheck/threshold_check" -require "govuk_app_config/govuk_healthcheck/sidekiq_queue_check" -require "govuk_app_config/govuk_healthcheck/sidekiq_queue_latency_check" -require "govuk_app_config/govuk_healthcheck/sidekiq_retry_size_check" require "json" module GovukHealthcheck def self.rack_response(*checks) proc do + checkup = healthcheck(checks) [ - 200, + checkup[:status] == :ok ? 200 : 500, { "Content-Type" => "application/json" }, - [JSON.dump(healthcheck(checks))], + [JSON.dump(checkup)], ] end end diff --git a/lib/govuk_app_config/govuk_healthcheck/sidekiq_queue_check.rb b/lib/govuk_app_config/govuk_healthcheck/sidekiq_queue_check.rb deleted file mode 100644 index 97a0e270..00000000 --- a/lib/govuk_app_config/govuk_healthcheck/sidekiq_queue_check.rb +++ /dev/null @@ -1,62 +0,0 @@ -module GovukHealthcheck - class SidekiqQueueCheck - def status - queues.each do |name, value| - if value >= critical_threshold(queue: name) - return :critical - elsif value >= warning_threshold(queue: name) - return :warning - end - end - - :ok - end - - def message - messages = queues.map do |name, value| - critical = critical_threshold(queue: name) - warning = warning_threshold(queue: name) - - if value >= critical - "#{name} (#{value}) is above the critical threshold (#{critical})" - elsif value >= warning - "#{name} (#{value}) is above the warning threshold (#{warning})" - end - end - - messages = messages.compact - - if messages.empty? - "all queues are below the critical and warning thresholds" - else - messages.join("\n") - end - end - - def details - { - queues: queues.each_with_object({}) do |(name, value), hash| - hash[name] = { - value: value, - thresholds: { - critical: critical_threshold(queue: name), - warning: warning_threshold(queue: name), - }.reject { |_, val| val.to_f.infinite? || val.to_f.nan? }, - } - end, - } - end - - def queues - raise "This method must be overriden to be a hash of queue names and data." - end - - def critical_threshold(queue:) # rubocop:disable Lint/UnusedMethodArgument - raise "This method must be overriden to be the critical threshold." - end - - def warning_threshold(queue:) # rubocop:disable Lint/UnusedMethodArgument - raise "This method must be overriden to be the warning threshold." - end - end -end diff --git a/lib/govuk_app_config/govuk_healthcheck/sidekiq_queue_latency_check.rb b/lib/govuk_app_config/govuk_healthcheck/sidekiq_queue_latency_check.rb deleted file mode 100644 index 5d7459a4..00000000 --- a/lib/govuk_app_config/govuk_healthcheck/sidekiq_queue_latency_check.rb +++ /dev/null @@ -1,13 +0,0 @@ -module GovukHealthcheck - class SidekiqQueueLatencyCheck < SidekiqQueueCheck - def name - :sidekiq_queue_latency - end - - def queues - @queues ||= Sidekiq::Stats.new.queues.keys.each_with_object({}) do |name, hash| - hash[name] = Sidekiq::Queue.new(name).latency - end - end - end -end diff --git a/lib/govuk_app_config/govuk_healthcheck/sidekiq_retry_size_check.rb b/lib/govuk_app_config/govuk_healthcheck/sidekiq_retry_size_check.rb deleted file mode 100644 index f6e915e2..00000000 --- a/lib/govuk_app_config/govuk_healthcheck/sidekiq_retry_size_check.rb +++ /dev/null @@ -1,11 +0,0 @@ -module GovukHealthcheck - class SidekiqRetrySizeCheck < ThresholdCheck - def name - :sidekiq_retry_size - end - - def value - Sidekiq::Stats.new.retry_size - end - end -end diff --git a/lib/govuk_app_config/govuk_healthcheck/threshold_check.rb b/lib/govuk_app_config/govuk_healthcheck/threshold_check.rb deleted file mode 100644 index 58f0f15f..00000000 --- a/lib/govuk_app_config/govuk_healthcheck/threshold_check.rb +++ /dev/null @@ -1,50 +0,0 @@ -module GovukHealthcheck - class ThresholdCheck - def status - if value >= critical_threshold - :critical - elsif value >= warning_threshold - :warning - else - :ok - end - end - - def message - if value >= critical_threshold - "#{value} is above the critical threshold (#{critical_threshold})" - elsif value >= warning_threshold - "#{value} is above the warning threshold (#{warning_threshold})" - else - "#{value} is below the critical and warning thresholds" - end - end - - def details - { - value: value, - total: total, - thresholds: { - critical: critical_threshold, - warning: warning_threshold, - }, - } - end - - def value - raise "This method must be overridden to be the check value." - end - - def total - nil # This method can be overriden to provide the total for the check. - end - - def critical_threshold - raise "This method must be overriden to be the critical threshold." - end - - def warning_threshold - raise "This method must be overriden to be the warning threshold." - end - end -end diff --git a/spec/lib/govuk_healthcheck/sidekiq_queue_check_spec.rb b/spec/lib/govuk_healthcheck/sidekiq_queue_check_spec.rb deleted file mode 100644 index b20623c9..00000000 --- a/spec/lib/govuk_healthcheck/sidekiq_queue_check_spec.rb +++ /dev/null @@ -1,88 +0,0 @@ -require "spec_helper" -require "govuk_app_config/govuk_healthcheck" -require_relative "shared_interface" - -RSpec.describe GovukHealthcheck::SidekiqQueueCheck do - context "an ok check" do - subject { TestQueueCheck.new({ queue: 0 }, 10, 20) } - - it_behaves_like "a healthcheck" - - its(:status) { is_expected.to eq(:ok) } - its(:message) { is_expected.to match(/below the critical and warning thresholds/) } - its(:details) do - is_expected.to match( - queues: { - queue: hash_including(value: 0, thresholds: { warning: 10, critical: 20 }), - }, - ) - end - end - - context "a warning check" do - subject { TestQueueCheck.new({ queue: 11 }, 10, 20) } - - it_behaves_like "a healthcheck" - - its(:status) { is_expected.to eq(:warning) } - its(:message) { is_expected.to match(/above the warning threshold/) } - its(:details) do - is_expected.to match( - queues: { - queue: hash_including(value: 11, thresholds: { warning: 10, critical: 20 }), - }, - ) - end - end - - context "a critical check" do - subject { TestQueueCheck.new({ queue: 21 }, 10, 20) } - - it_behaves_like "a healthcheck" - - its(:status) { is_expected.to eq(:critical) } - its(:message) { is_expected.to match(/above the critical threshold/) } - its(:details) do - is_expected.to match( - queues: { - queue: hash_including(value: 21, thresholds: { warning: 10, critical: 20 }), - }, - ) - end - end - - context "NaNs and infinities" do - subject { TestQueueCheck.new({ queue: 0 }, Float::INFINITY, Float::NAN) } - - it_behaves_like "a healthcheck" - - its(:status) { is_expected.to eq(:ok) } - its(:message) { is_expected.to match(/below the critical and warning thresholds/) } - its(:details) do - is_expected.to match( - queues: { - queue: hash_including(value: 0, thresholds: {}), - }, - ) - end - end - - class TestQueueCheck < GovukHealthcheck::SidekiqQueueCheck - def initialize(queues, warning_threshold, critical_threshold, name = :test) - @queues = queues - @warning_threshold = warning_threshold - @critical_threshold = critical_threshold - @name = name - end - - attr_reader :queues, :name - - def warning_threshold(queue:) # rubocop:disable Lint/UnusedMethodArgument - @warning_threshold - end - - def critical_threshold(queue:) # rubocop:disable Lint/UnusedMethodArgument - @critical_threshold - end - end -end diff --git a/spec/lib/govuk_healthcheck/sidekiq_queue_latency_check_spec.rb b/spec/lib/govuk_healthcheck/sidekiq_queue_latency_check_spec.rb deleted file mode 100644 index 78c729fd..00000000 --- a/spec/lib/govuk_healthcheck/sidekiq_queue_latency_check_spec.rb +++ /dev/null @@ -1,33 +0,0 @@ -require "spec_helper" -require "govuk_app_config/govuk_healthcheck" -require_relative "shared_interface" - -RSpec.describe GovukHealthcheck::SidekiqQueueLatencyCheck do - subject { TestQueueLatencyCheck.new } - - let(:sidekiq_stats) { double(queues: { test: 10 }) } - let(:sidekiq_queue) { double(latency: 5) } - - let(:sidekiq_stats_class) { double } - let(:sidekiq_queue_class) { double } - - before do - allow(sidekiq_stats_class).to receive(:new).and_return(sidekiq_stats) - allow(sidekiq_queue_class).to receive(:new).with(:test).and_return(sidekiq_queue) - - stub_const("Sidekiq::Stats", sidekiq_stats_class) - stub_const("Sidekiq::Queue", sidekiq_queue_class) - end - - it_behaves_like "a healthcheck" - - class TestQueueLatencyCheck < GovukHealthcheck::SidekiqQueueLatencyCheck - def warning_threshold(queue:) # rubocop:disable Lint/UnusedMethodArgument - 10 - end - - def critical_threshold(queue:) # rubocop:disable Lint/UnusedMethodArgument - 20 - end - end -end diff --git a/spec/lib/govuk_healthcheck/sidekiq_retry_size_check_spec.rb b/spec/lib/govuk_healthcheck/sidekiq_retry_size_check_spec.rb deleted file mode 100644 index f17ff680..00000000 --- a/spec/lib/govuk_healthcheck/sidekiq_retry_size_check_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -require "spec_helper" -require "govuk_app_config/govuk_healthcheck" -require_relative "shared_interface" - -RSpec.describe GovukHealthcheck::SidekiqRetrySizeCheck do - subject { TestRetrySizeCheck.new } - - let(:sidekiq_stats) { double(retry_size: 10) } - let(:sidekiq_stats_class) { double } - - before do - allow(sidekiq_stats_class).to receive(:new).and_return(sidekiq_stats) - stub_const("Sidekiq::Stats", sidekiq_stats_class) - end - - it_behaves_like "a healthcheck" - - class TestRetrySizeCheck < GovukHealthcheck::SidekiqRetrySizeCheck - def warning_threshold - 10 - end - - def critical_threshold - 20 - end - end -end diff --git a/spec/lib/govuk_healthcheck/threshold_check_spec.rb b/spec/lib/govuk_healthcheck/threshold_check_spec.rb deleted file mode 100644 index a4ba7bbc..00000000 --- a/spec/lib/govuk_healthcheck/threshold_check_spec.rb +++ /dev/null @@ -1,67 +0,0 @@ -require "spec_helper" -require "govuk_app_config/govuk_healthcheck" -require_relative "shared_interface" - -RSpec.describe GovukHealthcheck::ThresholdCheck do - context "an ok check" do - subject { TestThresholdCheck.new(0, 10, 20) } - - it_behaves_like "a healthcheck" - - its(:status) { is_expected.to eq(:ok) } - its(:message) { is_expected.to match(/below the critical and warning thresholds/) } - its(:details) do - is_expected.to match( - hash_including(value: 0, thresholds: { warning: 10, critical: 20 }), - ) - end - end - - context "a warning check" do - subject { TestThresholdCheck.new(11, 10, 20) } - - it_behaves_like "a healthcheck" - - its(:status) { is_expected.to eq(:warning) } - its(:message) { is_expected.to match(/above the warning threshold/) } - its(:details) do - is_expected.to match( - hash_including(value: 11, thresholds: { warning: 10, critical: 20 }), - ) - end - end - - context "a critical check" do - subject { TestThresholdCheck.new(21, 10, 20) } - - it_behaves_like "a healthcheck" - - its(:status) { is_expected.to eq(:critical) } - its(:message) { is_expected.to match(/above the critical threshold/) } - its(:details) do - is_expected.to match( - hash_including(value: 21, thresholds: { warning: 10, critical: 20 }), - ) - end - end - - context "with a total" do - subject { TestThresholdCheck.new(0, 10, 20, 40) } - - it_behaves_like "a healthcheck" - - its(:details) { is_expected.to match(hash_including(total: 40)) } - end - - class TestThresholdCheck < GovukHealthcheck::ThresholdCheck - def initialize(value, warning_threshold, critical_threshold, total = nil, name = :test) - @value = value - @warning_threshold = warning_threshold - @critical_threshold = critical_threshold - @total = total - @name = name - end - - attr_reader :value, :warning_threshold, :critical_threshold, :total, :name - end -end diff --git a/spec/lib/govuk_healthcheck_spec.rb b/spec/lib/govuk_healthcheck_spec.rb index eb87f12c..1c1b79b8 100644 --- a/spec/lib/govuk_healthcheck_spec.rb +++ b/spec/lib/govuk_healthcheck_spec.rb @@ -3,10 +3,27 @@ RSpec.describe GovukHealthcheck do describe "#rack_response" do - let(:response) { described_class.rack_response.call } + let(:active_record) { double(:active_record, connected?: true, connection: true) } + before { stub_const("ActiveRecord::Base", active_record) } + + let(:response) { described_class.rack_response(GovukHealthcheck::ActiveRecord).call } it "sets the content type" do expect(response[1].fetch("Content-Type")).to eq("application/json") end + + it "returns a 200 status code" do + expect(response[0]).to eq(200) + end + + context "a check fails" do + before do + allow(active_record).to receive(:connection) { raise } + end + + it "returns a 500 status code" do + expect(response[0]).to eq(500) + end + end end end