Crons: add sidekiq-scheduler zero-config monitor check-ins (#2172)

* Crons: add sidekiq-scheduler zero config support.

- Adds support for `sidekiq-scheduler` instrumentation without any
  configuration from the user.
- Based on #2170.

* Crons: support intervals for sidekiq-scheduler jobs

- AppliesApply review feedback.
- Adds support for interval and every interval_types for
  sidekiq-scheduler-schedule
- Adds specs for the above.

* Crons: changelog for sidekiq-scheduler support.

* Crons: fix tests on Sidekiq 6

* Require sidekiq-scheduler to ensure it's there

* sidekiq-scheduler mock config without delegation

* Make version checks consistent

* Fix some stuff

* need int for interval
* constantize doesn't exist outside rails
* cleanup specs

---------

Co-authored-by: Neel Shah <neelshah.sa@gmail.com>

CHANGELOG.md

@@ -12,6 +12,13 @@
     ```rb
     config.enabled_patches += [:sidekiq_cron]
     ```
+  - Add support for [`sidekiq-scheduler`](https://github.com/sidekiq-scheduler/sidekiq-scheduler) [#2172](https://github.com/getsentry/sentry-ruby/pull/2172)
+
+    You can opt in to the `sidekiq-scheduler` patch and we will automatically monitor check-ins for all repeating jobs (i.e. `cron`, `every`, and `interval`) specified in the config.
+
+    ```rb
+    config.enabled_patches += [:sidekiq_scheduler]
+    ```
 
 ### Bug Fixes
 

sentry-sidekiq/Gemfile

@@ -23,7 +23,11 @@ sidekiq_version = "7.0" if sidekiq_version.nil?
 sidekiq_version = Gem::Version.new(sidekiq_version)
 
 gem "sidekiq", "~> #{sidekiq_version}"
-gem "sidekiq-cron" if sidekiq_version >= Gem::Version.new("6.0")
+
+if RUBY_VERSION.to_f >= 2.7 && sidekiq_version >= Gem::Version.new("6.0")
+  gem "sidekiq-cron"
+  gem "sidekiq-scheduler"
+end
 
 gem "rails", "> 5.0.0", "< 7.1.0"
 

sentry-sidekiq/lib/sentry-sidekiq.rb

@@ -42,3 +42,4 @@ class Railtie < ::Rails::Railtie
 
 # patches
 require "sentry/sidekiq/cron/job"
+require "sentry/sidekiq-scheduler/scheduler"

sentry-sidekiq/lib/sentry/sidekiq-scheduler/scheduler.rb

@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+# Try to require sidekiq-scheduler to make sure it's loaded before the integration.
+begin
+  require "sidekiq-scheduler"
+rescue LoadError
+  return
+end
+
+# If we've loaded sidekiq-scheduler, but the API changed,
+# and the Scheduler class is not there, fail gracefully.
+return unless defined?(::SidekiqScheduler::Scheduler)
+
+module Sentry
+  module SidekiqScheduler
+    module Scheduler
+      def new_job(name, interval_type, config, schedule, options)
+        # Schedule the job upstream first
+        # SidekiqScheduler does not validate schedules
+        # It will fail with an error if the schedule in the config is invalid.
+        # If this errors out, let it fall through.
+        rufus_job = super
+
+        klass = config.fetch("class")
+        return rufus_job unless klass
+
+        # Constantize the job class, and fail gracefully if it could not be found
+        klass_const =
+          begin
+            Object.const_get(klass)
+          rescue NameError
+            return rufus_job
+          end
+
+        # For cron, every, or interval jobs — grab their schedule.
+        # Rufus::Scheduler::EveryJob stores it's frequency in seconds,
+        # 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)
+          when "every", "interval"
+            Sentry::Cron::MonitorConfig.from_interval(rufus_job.frequency.to_i / 60, :minute)
+        end
+
+        # If we couldn't build a monitor config, it's either an error, or
+        # it's a one-time job (interval_type is in, or at), in which case
+        # we should not make a monitof for it automaticaly.
+        return rufus_job if monitor_config.nil?
+
+        # only patch if not explicitly included in job by user
+        unless klass_const.send(:ancestors).include?(Sentry::Cron::MonitorCheckIns)
+          klass_const.send(:include, Sentry::Cron::MonitorCheckIns)
+          klass_const.send(:sentry_monitor_check_ins,
+                           slug: name,
+                           monitor_config: monitor_config)
+
+          ::Sidekiq.logger.info "Injected Sentry Crons monitor checkins into #{klass}"
+        end
+
+        rufus_job
+      end
+    end
+  end
+end
+
+Sentry.register_patch(:sidekiq_scheduler, Sentry::SidekiqScheduler::Scheduler, ::SidekiqScheduler::Scheduler)

sentry-sidekiq/spec/fixtures/schedule.yml → sentry-sidekiq/spec/fixtures/sidekiq-cron-schedule.yml

@@ -1,6 +1,6 @@
 happy:
   cron: "* * * * *"
-  class: "HappyWorkerDup"
+  class: "HappyWorkerForCron"
 
 manual:
   cron: "* * * * *"

sentry-sidekiq/spec/fixtures/sidekiq-scheduler-schedule.yml

@@ -0,0 +1,14 @@
+:schedule:
+  happy:
+    cron: "* * * * *"
+    class: "HappyWorkerForScheduler"
+  manual:
+    cron: "* * * * *"
+    class: "SadWorkerWithCron"
+  regularly_happy:
+    every: "10 minutes"
+    class: "EveryHappyWorker"
+  reports:
+    in: "2 hours"
+    class: "ReportingWorker"
+

sentry-sidekiq/spec/sentry/sidekiq-scheduler/scheduler_spec.rb

@@ -0,0 +1,56 @@
+require 'spec_helper'
+
+return unless defined?(SidekiqScheduler::Scheduler)
+
+RSpec.describe Sentry::SidekiqScheduler::Scheduler do
+  before do
+    perform_basic_setup { |c| c.enabled_patches += [:sidekiq_scheduler] }
+  end
+
+  before do
+    schedule_file = 'spec/fixtures/sidekiq-scheduler-schedule.yml'
+    config_options = { scheduler: YAML.load_file(schedule_file) }
+
+    # Sidekiq::Scheduler merges it's config with Sidekiq.
+    # To grab a config for it to start, we need to pass sidekiq configuration
+    # (defaults should be fine though).
+    scheduler_config = SidekiqScheduler::Config.new(sidekiq_config: sidekiq_config(config_options))
+
+    # Making and starting a Manager instance will load the jobs
+    schedule_manager = SidekiqScheduler::Manager.new(scheduler_config)
+    schedule_manager.start
+  end
+
+  it 'patches class' do
+    expect(SidekiqScheduler::Scheduler.ancestors).to include(described_class)
+  end
+
+  it 'patches HappyWorkerForScheduler' do
+    expect(HappyWorkerForScheduler.ancestors).to include(Sentry::Cron::MonitorCheckIns)
+    expect(HappyWorkerForScheduler.sentry_monitor_slug).to eq('happy')
+    expect(HappyWorkerForScheduler.sentry_monitor_config).to be_a(Sentry::Cron::MonitorConfig)
+    expect(HappyWorkerForScheduler.sentry_monitor_config.schedule).to be_a(Sentry::Cron::MonitorSchedule::Crontab)
+    expect(HappyWorkerForScheduler.sentry_monitor_config.schedule.value).to eq('* * * * *')
+  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')
+    expect(SadWorkerWithCron.sentry_monitor_config).to be_a(Sentry::Cron::MonitorConfig)
+    expect(SadWorkerWithCron.sentry_monitor_config.schedule).to be_a(Sentry::Cron::MonitorSchedule::Crontab)
+    expect(SadWorkerWithCron.sentry_monitor_config.schedule.value).to eq('5 * * * *')
+  end
+
+  it "sets correct monitor config based on `every` schedule" do
+    expect(EveryHappyWorker.ancestors).to include(Sentry::Cron::MonitorCheckIns)
+    expect(EveryHappyWorker.sentry_monitor_slug).to eq('regularly_happy')
+    expect(EveryHappyWorker.sentry_monitor_config).to be_a(Sentry::Cron::MonitorConfig)
+    expect(EveryHappyWorker.sentry_monitor_config.schedule).to be_a(Sentry::Cron::MonitorSchedule::Interval)
+    expect(EveryHappyWorker.sentry_monitor_config.schedule.to_hash).to eq({value: 10, type: :interval, unit: :minute})
+  end
+
+  it "does not add monitors for a one-off job" do
+    expect(ReportingWorker.ancestors).not_to include(Sentry::Cron::MonitorCheckIns)
+  end
+end
+

sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb

@@ -8,7 +8,7 @@
   end
 
   before do
-    schedule_file = 'spec/fixtures/schedule.yml'
+    schedule_file = 'spec/fixtures/sidekiq-cron-schedule.yml'
     schedule = Sidekiq::Cron::Support.load_yaml(ERB.new(IO.read(schedule_file)).result)
     # sidekiq-cron 2.0+ accepts second argument to `load_from_hash!` with options,
     # such as {source: 'schedule'}, but sidekiq-cron 1.9.1 (last version to support Ruby 2.6) does not.
@@ -40,12 +40,12 @@
     end.not_to raise_error
   end
 
-  it 'patches HappyWorker' do
-    expect(HappyWorkerDup.ancestors).to include(Sentry::Cron::MonitorCheckIns)
-    expect(HappyWorkerDup.sentry_monitor_slug).to eq('happy')
-    expect(HappyWorkerDup.sentry_monitor_config).to be_a(Sentry::Cron::MonitorConfig)
-    expect(HappyWorkerDup.sentry_monitor_config.schedule).to be_a(Sentry::Cron::MonitorSchedule::Crontab)
-    expect(HappyWorkerDup.sentry_monitor_config.schedule.value).to eq('* * * * *')
+  it 'patches HappyWorkerForCron' do
+    expect(HappyWorkerForCron.ancestors).to include(Sentry::Cron::MonitorCheckIns)
+    expect(HappyWorkerForCron.sentry_monitor_slug).to eq('happy')
+    expect(HappyWorkerForCron.sentry_monitor_config).to be_a(Sentry::Cron::MonitorConfig)
+    expect(HappyWorkerForCron.sentry_monitor_config.schedule).to be_a(Sentry::Cron::MonitorSchedule::Crontab)
+    expect(HappyWorkerForCron.sentry_monitor_config.schedule.value).to eq('* * * * *')
   end
 
   it 'does not override SadWorkerWithCron manually set values' do

sentry-sidekiq/spec/spec_helper.rb

@@ -5,11 +5,16 @@
 # this enables sidekiq's server mode
 require "sidekiq/cli"
 
+MIN_SIDEKIQ_6 = Gem::Version.new(Sidekiq::VERSION) >= Gem::Version.new("6.0")
 WITH_SIDEKIQ_7 = Gem::Version.new(Sidekiq::VERSION) >= Gem::Version.new("7.0")
-WITH_SIDEKIQ_6 = Gem::Version.new(Sidekiq::VERSION) >= Gem::Version.new("6.0") && !WITH_SIDEKIQ_7
+WITH_SIDEKIQ_6 = MIN_SIDEKIQ_6 && !WITH_SIDEKIQ_7
 
 require "sidekiq/embedded" if WITH_SIDEKIQ_7
-require 'sidekiq-cron' if RUBY_VERSION.to_f >= 2.7 && WITH_SIDEKIQ_6 || WITH_SIDEKIQ_7
+
+if RUBY_VERSION.to_f >= 2.7 && MIN_SIDEKIQ_6
+  require 'sidekiq-cron'
+  require 'sidekiq-scheduler'
+end
 
 require "sentry-ruby"
 
@@ -128,7 +133,9 @@ def perform
   end
 end
 
-class HappyWorkerDup < HappyWorker; end
+class HappyWorkerForCron < HappyWorker; end
+class HappyWorkerForScheduler < HappyWorker; end
+class EveryHappyWorker < HappyWorker; end
 
 class HappyWorkerWithCron < HappyWorker
   include Sentry::Cron::MonitorCheckIns
@@ -184,6 +191,28 @@ def new_processor
   manager.workers.first
 end
 
+class SidekiqConfigMock
+  include ::Sidekiq
+  attr_accessor :options
+
+  def initialize(options = {})
+    @options = DEFAULTS.merge(options)
+  end
+
+  def fetch(key, default = nil)
+    options.fetch(key, default)
+  end
+
+  def [](key)
+    options[key]
+  end
+end
+
+# Sidekiq 7 has a Config class, but for Sidekiq 6, we'll mock it.
+def sidekiq_config(opts)
+  WITH_SIDEKIQ_7 ? ::Sidekiq::Config.new(opts) : SidekiqConfigMock.new(opts)
+end
+
 def execute_worker(processor, klass, **options)
   klass_options = klass.sidekiq_options_hash || {}