Skip to content

Commit

Permalink
Crons: add sidekiq-scheduler zero config support.
Browse files Browse the repository at this point in the history
- Adds support for `sidekiq-scheduler` instrumentation without any
  configuration from the user.
- Based on getsentry#2170.
  • Loading branch information
natikgadzhi committed Nov 20, 2023
1 parent c7c8e5b commit 0794804
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 0 deletions.
1 change: 1 addition & 0 deletions sentry-sidekiq/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ sidekiq_version = Gem::Version.new(sidekiq_version)

gem "sidekiq", "~> #{sidekiq_version}"
gem "sidekiq-cron" if sidekiq_version >= Gem::Version.new("6.0")
gem "sidekiq-scheduler" if sidekiq_version >= Gem::Version.new("6.0")

gem "rails", "> 5.0.0", "< 7.1.0"

Expand Down
1 change: 1 addition & 0 deletions sentry-sidekiq/lib/sentry-sidekiq.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,4 @@ class Railtie < ::Rails::Railtie

# patches
require "sentry/sidekiq/cron/job"
require "sentry/sidekiq/sidekiq-scheduler/scheduler"
41 changes: 41 additions & 0 deletions sentry-sidekiq/lib/sentry/sidekiq/sidekiq-scheduler/scheduler.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# frozen_string_literal: true

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(name, interval_type, config, schedule, options)

# Constantize the job class, and fail gracefully if it could not be found
klass_const =
begin
config.fetch("class").constantize
rescue NameError
return rufus_job
end

monitor_config = Sentry::Cron::MonitorConfig.from_crontab(schedule)

# 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 #{config.fetch("class")}"
end

return rufus_job
end
end
end
end

Sentry.register_patch(:sidekiq_scheduler, Sentry::SidekiqScheduler::Scheduler, ::SidekiqScheduler::Scheduler)
9 changes: 9 additions & 0 deletions sentry-sidekiq/spec/fixtures/sidekiq-scheduler-schedule.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
enabled: true
dynamic: true
:schedule:
happy:
cron: "* * * * *"
class: "HappyWorkerDup"
manual:
cron: "* * * * *"
class: "SadWorkerWithCron"
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
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'
sidekiq_config = ::Sidekiq::Config.new({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)

# 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 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('* * * * *')
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
end

4 changes: 4 additions & 0 deletions sentry-sidekiq/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
require "sidekiq/embedded" if WITH_SIDEKIQ_7
require 'sidekiq-cron' if RUBY_VERSION.to_f >= 2.7 && WITH_SIDEKIQ_6 || WITH_SIDEKIQ_7

# TODO: Make this requirement optional?
# TODO: Verify sidekiq-schedule works on Ruby 2.7
require 'sidekiq-scheduler' if RUBY_VERSION.to_f >= 2.7 && WITH_SIDEKIQ_6 || WITH_SIDEKIQ_7

require "sentry-ruby"

require 'simplecov'
Expand Down

0 comments on commit 0794804

Please sign in to comment.