DEV: Add DB backed problem checks to support perform_every config (#25834)

As part of problem checks refactoring, we're moving some data to be DB backed. In this PR it's the tracking of problem check execution. When was it last run, when was the last problem, when should it run next, how many consecutive checks had problems, etc.

This allows us to implement the perform_every feature in scheduled problem checks for checks that don't need to be run every 10 minutes.

app/jobs/regular/problem_check.rb

@@ -18,7 +18,12 @@ module Jobs
       problems = check.call
       raise RetrySignal if problems.present? && retry_count < check.max_retries
 
+      if problems.present?
         problems.each { |problem| AdminDashboardData.add_found_scheduled_check_problem(problem) }
+        ProblemCheckTracker[identifier].problem!(next_run_at: check.perform_every.from_now)
+      else
+        ProblemCheckTracker[identifier].no_problem!(next_run_at: check.perform_every.from_now)
+      end
     rescue RetrySignal
       Jobs.enqueue_in(
         check.retry_after,

app/jobs/scheduled/problem_checks.rb

@@ -14,7 +14,12 @@ module Jobs
       # not be re-added by the relevant checker, and will be cleared.
       AdminDashboardData.clear_found_scheduled_check_problems
 
-      ::ProblemCheck.scheduled.each do |check|
+      scheduled_checks =
+        ProblemCheckTracker.all.filter_map do |tracker|
+          tracker.check if tracker.check.scheduled? && tracker.ready_to_run?
+        end
+
+      scheduled_checks.each do |check|
         Jobs.enqueue(:problem_check, check_identifier: check.identifier.to_s)
       end
     end

app/models/problem_check_tracker.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+class ProblemCheckTracker < ActiveRecord::Base
+  validates :identifier, presence: true, uniqueness: true
+  validates :blips, presence: true, numericality: { greater_than_or_equal_to: 0 }
+
+  def self.[](identifier)
+    find_or_create_by(identifier:)
+  end
+
+  def ready_to_run?
+    next_run_at.blank? || next_run_at.past?
+  end
+
+  def problem!(next_run_at: nil)
+    now = Time.current
+
+    update!(blips: blips + 1, last_run_at: now, last_problem_at: now, next_run_at:)
+  end
+
+  def no_problem!(next_run_at: nil)
+    now = Time.current
+
+    update!(blips: 0, last_run_at: now, last_success_at: now, next_run_at:)
+  end
+
+  def check
+    ProblemCheck[identifier]
+  end
+end
+
+# == Schema Information
+#
+# Table name: problem_check_trackers
+#
+#  id              :bigint           not null, primary key
+#  identifier      :string           not null
+#  blips           :integer          default(0), not null
+#  last_run_at     :datetime
+#  next_run_at     :datetime
+#  last_success_at :datetime
+#  last_problem_at :datetime
+#
+# Indexes
+#
+#  index_problem_check_trackers_on_identifier  (identifier) UNIQUE
+#

db/migrate/20240223052820_create_problem_check_trackers.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class CreateProblemCheckTrackers < ActiveRecord::Migration[7.0]
+  def change
+    create_table :problem_check_trackers do |t|
+      t.string :identifier, null: false, index: { unique: true }
+
+      t.integer :blips, null: false, default: 0
+
+      t.datetime :last_run_at
+      t.datetime :next_run_at
+      t.datetime :last_success_at
+      t.datetime :last_problem_at
+    end
+  end
+end

spec/fabricators/problem_check_tracker_fabricator.rb

@@ -0,0 +1,3 @@
+# frozen_string_literal: true
+
+Fabricator(:problem_check_tracker) {}

spec/jobs/problem_check_spec.rb

@@ -11,6 +11,7 @@ RSpec.describe Jobs::ProblemCheck do
     before do
       ::ProblemCheck::TestCheck =
         Class.new(::ProblemCheck) do
+          self.perform_every = 30.minutes
           self.max_retries = 0
 
           def call
@@ -39,6 +40,7 @@ RSpec.describe Jobs::ProblemCheck do
     before do
       ::ProblemCheck::TestCheck =
         Class.new(::ProblemCheck) do
+          self.perform_every = 30.minutes
           self.max_retries = 0
 
           def call
@@ -71,6 +73,7 @@ RSpec.describe Jobs::ProblemCheck do
     before do
       ::ProblemCheck::TestCheck =
         Class.new(::ProblemCheck) do
+          self.perform_every = 30.minutes
           self.max_retries = 2
 
           def call
@@ -79,6 +82,12 @@ RSpec.describe Jobs::ProblemCheck do
         end
     end
 
+    it "does not yet update the problem check tracker" do
+      expect {
+        described_class.new.execute(check_identifier: :test_check, retry_count: 1)
+      }.not_to change { ProblemCheckTracker.where("blips > ?", 0).count }
+    end
+
     it "schedules a retry" do
       expect_enqueued_with(
         job: :problem_check,
@@ -94,6 +103,7 @@ RSpec.describe Jobs::ProblemCheck do
     before do
       ::ProblemCheck::TestCheck =
         Class.new(::ProblemCheck) do
+          self.perform_every = 30.minutes
           self.max_retries = 1
 
           def call
@@ -102,9 +112,15 @@ RSpec.describe Jobs::ProblemCheck do
         end
     end
 
+    it "updates the problem check tracker" do
+      expect {
+        described_class.new.execute(check_identifier: :test_check, retry_count: 1)
+      }.to change { ProblemCheckTracker.where("blips > ?", 0).count }.by(1)
+    end
+
     it "does not schedule a retry" do
       expect_not_enqueued_with(job: :problem_check) do
-        described_class.new.execute(check_identifier: :test_identifier, retry_count: 1)
+        described_class.new.execute(check_identifier: :test_check, retry_count: 1)
       end
     end
   end

spec/jobs/problem_checks_spec.rb

@@ -20,13 +20,37 @@ RSpec.describe Jobs::ProblemChecks do
     ProblemCheck.send(:remove_const, "NonScheduledCheck")
   end
 
+  context "when the tracker determines the check is ready to run" do
+    before do
+      ProblemCheckTracker.create!(identifier: "scheduled_check", next_run_at: 5.minutes.ago)
+    end
+
     it "schedules the individual scheduled checks" do
       expect_enqueued_with(job: :problem_check, args: { check_identifier: "scheduled_check" }) do
         described_class.new.execute([])
       end
     end
+  end
 
-  it "does not schedule non-scheduled checks" do
+  context "when the tracker determines the check shouldn't run yet" do
+    before do
+      ProblemCheckTracker.create!(identifier: "scheduled_check", next_run_at: 5.minutes.from_now)
+    end
+
+    it "does not schedule any check" do
+      expect_not_enqueued_with(
+        job: :problem_check,
+        args: {
+          check_identifier: "scheduled_check",
+        },
+      ) { described_class.new.execute([]) }
+    end
+  end
+
+  context "when dealing with a non-scheduled check" do
+    before { ProblemCheckTracker.create!(identifier: "non_scheduled_check", next_run_at: nil) }
+
+    it "does not schedule any check" do
       expect_not_enqueued_with(
         job: :problem_check,
         args: {
@@ -34,6 +58,7 @@ RSpec.describe Jobs::ProblemChecks do
         },
       ) { described_class.new.execute([]) }
     end
+  end
 
   it "does not schedule non-scheduled checks" do
     expect_not_enqueued_with(

spec/models/problem_check_tracker_spec.rb

@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+RSpec.describe ProblemCheckTracker do
+  describe "validations" do
+    let(:record) { described_class.new(identifier: "twitter_login") }
+
+    it { expect(record).to validate_presence_of(:identifier) }
+    it { expect(record).to validate_uniqueness_of(:identifier) }
+
+    it { expect(record).to validate_numericality_of(:blips).is_greater_than_or_equal_to(0) }
+  end
+
+  describe ".[]" do
+    before { Fabricate(:problem_check_tracker, identifier: "twitter_login") }
+
+    context "when the problem check tracker already exists" do
+      it { expect(described_class[:twitter_login]).not_to be_new_record }
+    end
+
+    context "when the problem check tracker doesn't exist yet" do
+      it { expect(described_class[:facebook_login]).to be_previously_new_record }
+    end
+  end
+
+  describe "#ready_to_run?" do
+    let(:problem_tracker) { described_class.new(next_run_at:) }
+
+    context "when the next run timestamp is not set" do
+      let(:next_run_at) { nil }
+
+      it { expect(problem_tracker).to be_ready_to_run }
+    end
+
+    context "when the next run timestamp is in the past" do
+      let(:next_run_at) { 5.minutes.ago }
+
+      it { expect(problem_tracker).to be_ready_to_run }
+    end
+
+    context "when the next run timestamp is in the future" do
+      let(:next_run_at) { 5.minutes.from_now }
+
+      it { expect(problem_tracker).not_to be_ready_to_run }
+    end
+  end
+
+  describe "#problem!" do
+    let(:problem_tracker) do
+      Fabricate(:problem_check_tracker, identifier: "twitter_login", **original_attributes)
+    end
+
+    let(:original_attributes) do
+      {
+        blips: 0,
+        last_problem_at: 1.week.ago,
+        last_success_at: 24.hours.ago,
+        last_run_at: 24.hours.ago,
+        next_run_at: nil,
+      }
+    end
+
+    let(:updated_attributes) { { blips: 1 } }
+
+    it do
+      freeze_time
+
+      expect { problem_tracker.problem!(next_run_at: 24.hours.from_now) }.to change {
+        problem_tracker.attributes
+      }.to(hash_including(updated_attributes))
+    end
+  end
+
+  describe "#no_problem!" do
+    let(:problem_tracker) do
+      Fabricate(:problem_check_tracker, identifier: "twitter_login", **original_attributes)
+    end
+
+    let(:original_attributes) do
+      {
+        blips: 0,
+        last_problem_at: 1.week.ago,
+        last_success_at: Time.current,
+        last_run_at: 24.hours.ago,
+        next_run_at: nil,
+      }
+    end
+
+    let(:updated_attributes) { { blips: 0 } }
+
+    it do
+      freeze_time
+
+      expect { problem_tracker.no_problem!(next_run_at: 24.hours.from_now) }.to change {
+        problem_tracker.attributes
+      }.to(hash_including(updated_attributes))
+    end
+  end
+end