diff --git a/sentry-rails/lib/sentry/rails/solid_queue_recurring.rb b/sentry-rails/lib/sentry/rails/solid_queue_recurring.rb index bea96f505..70fd6c9df 100644 --- a/sentry-rails/lib/sentry/rails/solid_queue_recurring.rb +++ b/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 diff --git a/sentry-rails/spec/active_job/solid_queue_recurring_spec.rb b/sentry-rails/spec/active_job/solid_queue_recurring_spec.rb index 7fe119c20..30aedb04c 100644 --- a/sentry-rails/spec/active_job/solid_queue_recurring_spec.rb +++ b/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 diff --git a/sentry-ruby/lib/sentry/rufus_scheduler.rb b/sentry-ruby/lib/sentry/rufus_scheduler.rb index 11bb2019f..40424f10f 100644 --- a/sentry-ruby/lib/sentry/rufus_scheduler.rb +++ b/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 diff --git a/sentry-ruby/spec/sentry/rufus_scheduler_spec.rb b/sentry-ruby/spec/sentry/rufus_scheduler_spec.rb index 72afafe91..648bac26c 100644 --- a/sentry-ruby/spec/sentry/rufus_scheduler_spec.rb +++ b/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