fix(crons): Skip scheduled jobs with unsupported schedules (#3165)

Author: wedamija

sentry-rails/lib/sentry/rails/solid_queue_recurring.rb

@@ -52,7 +52,9 @@ def monitor_for(task, job)
           return unless slug
 
           config = Sentry::Cron::FugitSchedule.from_cron(cron_for(task))
-          { "slug" => slug, "crontab" => config&.schedule&.value, "timezone" => config&.timezone }.compact
+          return unless config
+
+          { "slug" => slug, "crontab" => config.schedule.value, "timezone" => config.timezone }.compact
         rescue StandardError => e
           Sentry.sdk_logger.error(Sentry::LOGGER_PROGNAME) { "Not monitoring Solid Queue recurring task #{task.key}: #{e.class}: #{e.message}" }
           nil
@@ -62,18 +64,17 @@ def perform(job)
           data = job.instance_variable_get(:@_sentry_recurring_task)
           return yield unless data && Sentry.initialized?
 
-          slug = data["slug"]
-          if data["crontab"]
-            cron_config = Sentry.configuration.cron
-            monitor_config = Sentry::Cron::MonitorConfig.from_crontab(
-              data["crontab"],
-              timezone: data["timezone"],
-              checkin_margin: cron_config.default_checkin_margin,
-              max_runtime: cron_config.default_max_runtime
-            )
-          end
+          return yield unless data["crontab"]
+
+          cron_config = Sentry.configuration.cron
+          monitor_config = Sentry::Cron::MonitorConfig.from_crontab(
+            data["crontab"],
+            timezone: data["timezone"],
+            checkin_margin: cron_config.default_checkin_margin,
+            max_runtime: cron_config.default_max_runtime
+          )
 
-          Sentry::Cron::MonitorCheckIns.with_check_in(slug, monitor_config: monitor_config) { yield }
+          Sentry::Cron::MonitorCheckIns.with_check_in(data["slug"], monitor_config: monitor_config) { yield }
         end
 
         def patch

sentry-rails/spec/active_job/solid_queue_recurring_spec.rb

@@ -104,11 +104,10 @@ def perform
       expect(check_ins.first.monitor_config.to_h[:schedule]).to eq(type: :crontab, value: "*/15 * * * *")
     end
 
-    it "sends check-ins without a config for schedules Crons cannot express" do
+    it "does not monitor schedules Crons cannot express" do
       run_task("every_ten_seconds", class: "ReportJob", schedule: "every 10 seconds")
 
-      expect(check_ins.map(&:status)).to eq([:in_progress, :ok])
-      expect(check_ins.map(&:monitor_config)).to all(be_nil)
+      expect(check_ins).to be_empty
     end
 
     it "applies SolidQueue.time_zone to schedules without a zone" do

sentry-ruby/lib/sentry/rufus_scheduler.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 module Sentry
-  # Sends Crons check-ins for rufus-scheduler `cron`, `every` and `interval` jobs.
+  # Sends Crons check-ins for rufus-scheduler `cron` and `every` jobs.
   # Enable with `config.enabled_patches << :rufus_scheduler`.
   module RufusScheduler
     module Job
@@ -46,7 +46,7 @@ def sentry_monitor
     end
 
     class << self
-      # Returns [slug, monitor_config], or nil for jobs that aren't monitored (like `at`/`in`).
+      # Returns [slug, monitor_config], or nil for jobs that aren't monitored (like `at`/`in`, or unsupported schedules).
       def monitor_for(job)
         return unless job.is_a?(::Rufus::Scheduler::RepeatJob)
         return if job.opts[:sentry_monitor] == false
@@ -61,7 +61,13 @@ def monitor_for(job)
           return
         end
 
-        [slug, monitor_config_for(job)]
+        monitor_config = monitor_config_for(job)
+        unless monitor_config
+          Sentry.sdk_logger.debug(Sentry::LOGGER_PROGNAME) { "Not monitoring rufus-scheduler job #{slug}: its schedule can't be expressed as a Crons schedule." }
+          return
+        end
+
+        [slug, monitor_config]
       end
 
       private
@@ -98,8 +104,7 @@ def monitor_config_for(job)
         when ::Rufus::Scheduler::EveryJob
           Sentry::Cron::FugitSchedule.from_seconds(job.frequency)
         end
-        # IntervalJob waits `interval` after each run ends, so it has no fixed
-        # schedule to monitor against.
+        # IntervalJob waits `interval` after each run ends, so no fixed schedule matches it.
       end
     end
   end

sentry-ruby/spec/sentry/rufus_scheduler_spec.rb

@@ -59,18 +59,26 @@ def monitor_config(type, schedule, **opts)
     expect(sentry_events.map(&:status)).to eq([:in_progress, :error])
   end
 
-  it "sends no config for cron lines Crons can't express" do
-    expect(monitor_config(:cron, "*/10 * * * * *", name: "job")).to be_nil
-    expect(sentry_events.count).to eq(2)
+  it "does not monitor cron lines Crons can't express" do
+    expect(schedule(:cron, "*/10 * * * * *", name: "job").call(true)).to eq(42)
+
+    expect(sentry_events).to be_empty
   end
 
   it "sends an interval config for every jobs" do
     expect(monitor_config(:every, "2h", name: "job")[:schedule]).to eq(type: :interval, value: 2, unit: :hour)
   end
 
-  it "sends check-ins without config for interval jobs" do
-    expect(monitor_config(:interval, "5m", name: "job")).to be_nil
-    expect(sentry_events.count).to eq(2)
+  it "does not monitor every jobs Crons can't express" do
+    schedule(:every, "90s", name: "job").call(true)
+
+    expect(sentry_events).to be_empty
+  end
+
+  it "does not monitor interval jobs" do
+    expect(schedule(:interval, "5m", name: "job").call(true)).to eq(42)
+
+    expect(sentry_events).to be_empty
   end
 
   it "does not monitor one-off jobs" do