Support crons with timezone for sidekiq-scheduler (#2209)

* Support crons with timezone for sidekiq-scheduler * Update CHANGELOG.md Co-authored-by: Stefan Jandl <[email protected]> --------- Co-authored-by: Stefan Jandl <[email protected]>
getsentry · Dec 27, 2023 · 2f9ffaf · 2f9ffaf
1 parent d02cf27
commit 2f9ffaf
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,9 @@
  If your system serves heavy load, please let us know how this feature works for you!
 
 - Implement proper flushing logic on ``close`` for Client Reports and Sessions [#2206](https://github.com/getsentry/sentry-ruby/pull/2206)
+- Support cron with timezone for `sidekiq-scheduler` patch [#2209](https://github.com/getsentry/sentry-ruby/pull/2209)
+ - Fixes [#2187](https://github.com/getsentry/sentry-ruby/issues/2187)
+
 ## 5.15.2
 
 ### Bug Fixes

diff --git a/sentry-sidekiq/lib/sentry/sidekiq-scheduler/scheduler.rb b/sentry-sidekiq/lib/sentry/sidekiq-scheduler/scheduler.rb
@@ -37,7 +37,18 @@ def new_job(name, interval_type, config, schedule, options)
  # so we convert it to minutes before passing in to the monitor.
  monitor_config = case interval_type
  when "cron"
- Sentry::Cron::MonitorConfig.from_crontab(schedule)
+ parsed_cron = ::Fugit.parse_cron(schedule)
+ timezone = parsed_cron.timezone
+
+ # fugit supports having the timezone part of the cron string,
+ # so we need to pull that with some hacky stuff
+ if timezone
+ parsed_cron.instance_variable_set(:@timezone, nil)
+ cron_without_timezone = parsed_cron.to_cron_s
+ Sentry::Cron::MonitorConfig.from_crontab(cron_without_timezone, timezone: timezone.name)
+ else
+ Sentry::Cron::MonitorConfig.from_crontab(schedule)
+ end
  when "every", "interval"
  Sentry::Cron::MonitorConfig.from_interval(rufus_job.frequency.to_i / 60, :minute)
  end

diff --git a/sentry-sidekiq/spec/fixtures/sidekiq-scheduler-schedule.yml b/sentry-sidekiq/spec/fixtures/sidekiq-scheduler-schedule.yml
@@ -2,6 +2,9 @@
  happy:
  cron: "* * * * *"
  class: "HappyWorkerForScheduler"
+ happy_timezone:
+ cron: "* * * * * Europe/Vienna"
+ class: "HappyWorkerForSchedulerWithTimezone"
  manual:
  cron: "* * * * *"
  class: "SadWorkerWithCron"
@@ -13,4 +16,3 @@
  class: "ReportingWorker"
  VeryLongOuterModule::VeryVeryVeryVeryLongInnerModule::Job:
  cron: "* * * * *"
-
diff --git a/sentry-sidekiq/spec/sentry/sidekiq-scheduler/scheduler_spec.rb b/sentry-sidekiq/spec/sentry/sidekiq-scheduler/scheduler_spec.rb
@@ -33,6 +33,15 @@
  expect(HappyWorkerForScheduler.sentry_monitor_config.schedule.value).to eq('* * * * *')
  end
 
+ it 'patches HappyWorkerForSchedulerWithTimezoneWithTimezone' do
+ expect(HappyWorkerForSchedulerWithTimezone.ancestors).to include(Sentry::Cron::MonitorCheckIns)
+ expect(HappyWorkerForSchedulerWithTimezone.sentry_monitor_slug).to eq('happy_timezone')
+ expect(HappyWorkerForSchedulerWithTimezone.sentry_monitor_config).to be_a(Sentry::Cron::MonitorConfig)
+ expect(HappyWorkerForSchedulerWithTimezone.sentry_monitor_config.schedule).to be_a(Sentry::Cron::MonitorSchedule::Crontab)
+ expect(HappyWorkerForSchedulerWithTimezone.sentry_monitor_config.schedule.value).to eq('* * * * *')
+ expect(HappyWorkerForSchedulerWithTimezone.sentry_monitor_config.timezone).to eq('Europe/Vienna')
+ end
+
  it 'does not override SadWorkerWithCron manually set values' do
  expect(SadWorkerWithCron.ancestors).to include(Sentry::Cron::MonitorCheckIns)
  expect(SadWorkerWithCron.sentry_monitor_slug).to eq('failed_job')

diff --git a/sentry-sidekiq/spec/spec_helper.rb b/sentry-sidekiq/spec/spec_helper.rb
@@ -133,8 +133,10 @@ def perform
  end
 end
 
+class HappyWorkerForCron < HappyWorker; end
 class HappyWorkerForCron < HappyWorker; end
 class HappyWorkerForScheduler < HappyWorker; end
+class HappyWorkerForSchedulerWithTimezone < HappyWorker; end
 class EveryHappyWorker < HappyWorker; end
 
 class HappyWorkerWithCron < HappyWorker