From 1938d619d8defa7bd05cece0ebaf2bfa85c40476 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 14 Nov 2023 15:35:03 +0100 Subject: [PATCH] Add `sidekiq-cron` patch for automatic monitoring of jobs listed in the schedule * optional patch under `sidekiq_cron` * patch the `Sidekiq::Cron::Job#save` method and auto inject the Sentry::MonitorCheckIns module and turn monitoring on part of #2134 --- CHANGELOG.md | 8 ++++ sentry-sidekiq/Gemfile | 1 + sentry-sidekiq/lib/sentry-sidekiq.rb | 3 ++ sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb | 34 ++++++++++++++ sentry-sidekiq/spec/fixtures/schedule.yml | 11 +++++ .../spec/sentry/sidekiq/cron/job_spec.rb | 46 +++++++++++++++++++ sentry-sidekiq/spec/spec_helper.rb | 5 +- 7 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb create mode 100644 sentry-sidekiq/spec/fixtures/schedule.yml create mode 100644 sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index c1bb112b9..74314ca37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ ### Features - Improve default slug generation for Crons [#2168](https://github.com/getsentry/sentry-ruby/pull/2168) +- Automatic Crons support for scheduling gems + - Add support for `sidekiq-cron` [#2170](https://github.com/getsentry/sentry-ruby/pull/2170) + + You can opt in to the `sidekiq-cron` patch and we will automatically monitor check-ins for all jobs listed in your `config/schedule.yml` file. + + ```rb + config.enabled_patches += [:sidekiq_cron] + ``` ### Bug Fixes diff --git a/sentry-sidekiq/Gemfile b/sentry-sidekiq/Gemfile index 776f21219..b4733357f 100644 --- a/sentry-sidekiq/Gemfile +++ b/sentry-sidekiq/Gemfile @@ -23,6 +23,7 @@ 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") gem "rails", "> 5.0.0", "< 7.1.0" diff --git a/sentry-sidekiq/lib/sentry-sidekiq.rb b/sentry-sidekiq/lib/sentry-sidekiq.rb index 2ff2816bd..b4e94b77c 100644 --- a/sentry-sidekiq/lib/sentry-sidekiq.rb +++ b/sentry-sidekiq/lib/sentry-sidekiq.rb @@ -39,3 +39,6 @@ class Railtie < ::Rails::Railtie chain.add Sentry::Sidekiq::SentryContextClientMiddleware end end + +# patches +require "sentry/sidekiq/cron/job" diff --git a/sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb b/sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb new file mode 100644 index 000000000..6e0abaeed --- /dev/null +++ b/sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +return unless defined?(::Sidekiq::Cron::Job) + +module Sentry + module Sidekiq + module Cron + module Job + def save + # validation failed, do nothing + return false unless super + + # fail gracefully if can't find class + klass_const = + begin + ::Sidekiq::Cron::Support.constantize(klass.to_s) + rescue NameError + return true + end + + # 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: Sentry::Cron::MonitorConfig.from_crontab(cron)) + end + end + end + end + end +end + +Sentry.register_patch(:sidekiq_cron, Sentry::Sidekiq::Cron::Job, ::Sidekiq::Cron::Job) diff --git a/sentry-sidekiq/spec/fixtures/schedule.yml b/sentry-sidekiq/spec/fixtures/schedule.yml new file mode 100644 index 000000000..bd3784d2b --- /dev/null +++ b/sentry-sidekiq/spec/fixtures/schedule.yml @@ -0,0 +1,11 @@ +happy: + cron: "* * * * *" + class: "HappyWorker" + +manual: + cron: "* * * * *" + class: "SadWorkerWithCron" + +invalid_cron: + cron: "not a crontab" + class: "ReportingWorker" diff --git a/sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb b/sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb new file mode 100644 index 000000000..45cbb5e4e --- /dev/null +++ b/sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +return unless defined?(Sidekiq::Cron::Job) + +RSpec.describe Sentry::Sidekiq::Cron::Job do + before do + perform_basic_setup { |c| c.enabled_patches += [:sidekiq_cron] } + end + + before do + schedule_file = 'spec/fixtures/schedule.yml' + schedule = Sidekiq::Cron::Support.load_yaml(ERB.new(IO.read(schedule_file)).result) + Sidekiq::Cron::Job.load_from_hash!(schedule, source: 'schedule') + end + + it 'patches class' do + expect(Sidekiq::Cron::Job.ancestors).to include(described_class) + end + + it 'patches HappyWorker' do + expect(HappyWorker.ancestors).to include(Sentry::Cron::MonitorCheckIns) + expect(HappyWorker.sentry_monitor_slug).to eq('happy') + expect(HappyWorker.sentry_monitor_config).to be_a(Sentry::Cron::MonitorConfig) + expect(HappyWorker.sentry_monitor_config.schedule).to be_a(Sentry::Cron::MonitorSchedule::Crontab) + expect(HappyWorker.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 'does not patch ReportingWorker because of invalid schedule' do + expect(ReportingWorker.ancestors).not_to include(Sentry::Cron::MonitorSchedule) + end + + it 'does not raise error on invalid class' do + expect do + Sidekiq::Cron::Job.create(name: 'invalid_class', cron: '* * * * *', class: 'UndefinedClass') + end.not_to raise_error + end + +end diff --git a/sentry-sidekiq/spec/spec_helper.rb b/sentry-sidekiq/spec/spec_helper.rb index b4932fb79..1ea5d919b 100644 --- a/sentry-sidekiq/spec/spec_helper.rb +++ b/sentry-sidekiq/spec/spec_helper.rb @@ -8,9 +8,8 @@ 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 -if WITH_SIDEKIQ_7 - require "sidekiq/embedded" -end +require "sidekiq/embedded" if WITH_SIDEKIQ_7 +require 'sidekiq-cron' if WITH_SIDEKIQ_6 || WITH_SIDEKIQ_7 require "sentry-ruby"