diff --git a/sentry-ruby/lib/sentry/cron/monitor_check_ins.rb b/sentry-ruby/lib/sentry/cron/monitor_check_ins.rb index 5f147c926..b7eee3f5f 100644 --- a/sentry-ruby/lib/sentry/cron/monitor_check_ins.rb +++ b/sentry-ruby/lib/sentry/cron/monitor_check_ins.rb @@ -13,32 +13,42 @@ def perform(*args, **opts) monitor_config: monitor_config) start = Sentry.utc_now.to_i - # need to do this on ruby <= 2.6 sadly - ret = method(:perform).super_method.arity == 0 ? super() : super - duration = Sentry.utc_now.to_i - start - Sentry.capture_check_in(slug, - :ok, - check_in_id: check_in_id, - duration: duration, - monitor_config: monitor_config) + begin + # need to do this on ruby <= 2.6 sadly + ret = method(:perform).super_method.arity == 0 ? super() : super + duration = Sentry.utc_now.to_i - start - ret - rescue Exception - duration = Sentry.utc_now.to_i - start + Sentry.capture_check_in(slug, + :ok, + check_in_id: check_in_id, + duration: duration, + monitor_config: monitor_config) - Sentry.capture_check_in(slug, - :error, - check_in_id: check_in_id, - duration: duration, - monitor_config: monitor_config) + ret + rescue Exception + duration = Sentry.utc_now.to_i - start - raise + Sentry.capture_check_in(slug, + :error, + check_in_id: check_in_id, + duration: duration, + monitor_config: monitor_config) + + raise + end end end module ClassMethods def sentry_monitor_check_ins(slug: nil, monitor_config: nil) + if monitor_config && Sentry.configuration + cron_config = Sentry.configuration.cron + monitor_config.checkin_margin ||= cron_config.default_checkin_margin + monitor_config.max_runtime ||= cron_config.default_max_runtime + monitor_config.timezone ||= cron_config.default_timezone + end + @sentry_monitor_slug = slug @sentry_monitor_config = monitor_config @@ -57,8 +67,6 @@ def sentry_monitor_config end end - extend ClassMethods - def self.included(base) base.extend(ClassMethods) end diff --git a/sentry-ruby/lib/sentry/cron/monitor_config.rb b/sentry-ruby/lib/sentry/cron/monitor_config.rb index 98cbf2aff..68b8c709c 100644 --- a/sentry-ruby/lib/sentry/cron/monitor_config.rb +++ b/sentry-ruby/lib/sentry/cron/monitor_config.rb @@ -25,9 +25,9 @@ class MonitorConfig def initialize(schedule, checkin_margin: nil, max_runtime: nil, timezone: nil) @schedule = schedule - @checkin_margin = checkin_margin || Sentry.configuration&.cron&.default_checkin_margin - @max_runtime = max_runtime || Sentry.configuration&.cron&.default_max_runtime - @timezone = timezone || Sentry.configuration&.cron&.default_timezone + @checkin_margin = checkin_margin + @max_runtime = max_runtime + @timezone = timezone end def self.from_crontab(crontab, **options) diff --git a/sentry-ruby/spec/sentry/cron/monitor_check_ins_spec.rb b/sentry-ruby/spec/sentry/cron/monitor_check_ins_spec.rb index 3f5580094..4cb08874a 100644 --- a/sentry-ruby/spec/sentry/cron/monitor_check_ins_spec.rb +++ b/sentry-ruby/spec/sentry/cron/monitor_check_ins_spec.rb @@ -33,8 +33,9 @@ def perform(a, b = 42, c: 99) it_behaves_like 'original_job' it 'does not call capture_check_in' do - expect(Sentry).not_to receive(:capture_check_in) job.perform(1) + + expect(sentry_events.count).to eq(0) end end @@ -105,20 +106,21 @@ def perform(a, b = 42, c: 99) expect(Job.ancestors.first).to eq(described_class::Patch) end - it 'calls capture_check_in twice' do - expect(Sentry).to receive(:capture_check_in).with( - 'job', - :in_progress, - hash_including(monitor_config: nil) - ).ordered.and_call_original + it 'records 2 check-in events' do + job.perform(1) + + expect(sentry_events.count).to eq(2) + in_progress_event = sentry_events.first - expect(Sentry).to receive(:capture_check_in).with( - 'job', - :ok, - hash_including(:check_in_id, monitor_config: nil, duration: 0) - ).ordered.and_call_original + expect(in_progress_event.monitor_slug).to eq('job') + expect(in_progress_event.status).to eq(:in_progress) + expect(in_progress_event.monitor_config).to be_nil - job.perform(1) + ok_event = sentry_events.last + + expect(ok_event.monitor_slug).to eq('job') + expect(ok_event.status).to eq(:ok) + expect(ok_event.monitor_config).to be_nil end end @@ -143,25 +145,21 @@ def perform; work end expect(Job.ancestors.first).to eq(described_class::Patch) end - it 'does the work' do - expect(job).to receive(:work).and_call_original - expect(job.perform).to eq(42) - end + it 'records 2 check-in events' do + expect(job.perform(1)).to eq(42) - it 'calls capture_check_in twice' do - expect(Sentry).to receive(:capture_check_in).with( - 'job', - :in_progress, - hash_including(monitor_config: nil) - ).ordered.and_call_original + expect(sentry_events.count).to eq(2) + in_progress_event = sentry_events.first - expect(Sentry).to receive(:capture_check_in).with( - 'job', - :ok, - hash_including(:check_in_id, monitor_config: nil, duration: 0) - ).ordered.and_call_original + expect(in_progress_event.monitor_slug).to eq('job') + expect(in_progress_event.status).to eq(:in_progress) + expect(in_progress_event.monitor_config).to be_nil - job.perform(1) + ok_event = sentry_events.last + + expect(ok_event.monitor_slug).to eq('job') + expect(ok_event.status).to eq(:ok) + expect(ok_event.monitor_config).to be_nil end end @@ -174,10 +172,7 @@ def perform; work end sentry_monitor_check_ins - def work(a, b, c); a + b + c end - - def perform(a, b = 42, c: 99) - work(a, b, c) + def perform end end @@ -190,17 +185,17 @@ def perform(a, b = 42, c: 99) end end - context 'patched with custom options' do - let(:config) { Sentry::Cron::MonitorConfig::from_interval(1, :minute) } + context 'patched with monitor config' do + let(:monitor_config) { Sentry::Cron::MonitorConfig::from_interval(1, :minute) } before do mod = described_class - conf = config + config = monitor_config job_class = Class.new do include mod - sentry_monitor_check_ins slug: 'custom_slug', monitor_config: conf + sentry_monitor_check_ins slug: 'custom_slug', monitor_config: config def work(a, b, c); a + b + c end @@ -222,37 +217,104 @@ def perform(a, b = 42, c: 99) it 'has correct custom options' do expect(Job.sentry_monitor_slug).to eq('custom_slug') - expect(Job.sentry_monitor_config).to eq(config) + expect(Job.sentry_monitor_config).to eq(monitor_config) + end + + it 'records 2 check-in events' do + job.perform(1) + + expect(sentry_events.count).to eq(2) + in_progress_event = sentry_events.first + + expect(in_progress_event.monitor_slug).to eq('custom_slug') + expect(in_progress_event.status).to eq(:in_progress) + expect(in_progress_event.monitor_config).to eq(monitor_config) + expect(in_progress_event.monitor_config.checkin_margin).to eq(nil) + expect(in_progress_event.monitor_config.max_runtime).to eq(nil) + expect(in_progress_event.monitor_config.timezone).to eq(nil) + + ok_event = sentry_events.last + + expect(ok_event.monitor_slug).to eq('custom_slug') + expect(ok_event.status).to eq(:ok) + expect(ok_event.monitor_config).to eq(monitor_config) + end + end + + context 'with custom monitor config object and cron configs' do + let(:monitor_config) { Sentry::Cron::MonitorConfig::from_interval(1, :minute) } + + before do + perform_basic_setup do |config| + config.cron.default_checkin_margin = 10 + config.cron.default_max_runtime = 20 + config.cron.default_timezone = 'Europe/Vienna' + end + + mod = described_class + config = monitor_config + + job_class = Class.new do + include mod + + sentry_monitor_check_ins slug: 'custom_slug', monitor_config: config + + def work(a, b, c); a + b + c end + + def perform(a, b = 42, c: 99) + work(a, b, c) + end + end + + stub_const('Job', job_class) end - it 'calls capture_check_in twice' do - expect(Sentry).to receive(:capture_check_in).with( - 'custom_slug', - :in_progress, - hash_including(monitor_config: config) - ).ordered.and_call_original + let(:job) { Job.new } + + it_behaves_like 'original_job' + + it 'prepends the patch' do + expect(Job.ancestors.first).to eq(described_class::Patch) + end - expect(Sentry).to receive(:capture_check_in).with( - 'custom_slug', - :ok, - hash_including(:check_in_id, monitor_config: config, duration: 0) - ).ordered.and_call_original + it 'has correct custom options' do + expect(Job.sentry_monitor_slug).to eq('custom_slug') + expect(Job.sentry_monitor_config).to eq(monitor_config) + end + it 'records 2 check-in events' do job.perform(1) + + expect(sentry_events.count).to eq(2) + in_progress_event = sentry_events.first + + expect(in_progress_event.monitor_slug).to eq('custom_slug') + expect(in_progress_event.status).to eq(:in_progress) + expect(in_progress_event.monitor_config.checkin_margin).to eq(10) + expect(in_progress_event.monitor_config.max_runtime).to eq(20) + expect(in_progress_event.monitor_config.timezone).to eq('Europe/Vienna') + + ok_event = sentry_events.last + + expect(ok_event.monitor_slug).to eq('custom_slug') + expect(ok_event.status).to eq(:ok) + expect(ok_event.monitor_config.checkin_margin).to eq(10) + expect(ok_event.monitor_config.max_runtime).to eq(20) + expect(ok_event.monitor_config.timezone).to eq('Europe/Vienna') end end context 'patched with custom options with exception' do - let(:config) { Sentry::Cron::MonitorConfig::from_crontab('5 * * * *') } + let(:monitor_config) { Sentry::Cron::MonitorConfig::from_crontab('5 * * * *') } before do mod = described_class - conf = config + config = monitor_config job_class = Class.new do include mod - sentry_monitor_check_ins slug: 'custom_slug', monitor_config: conf + sentry_monitor_check_ins slug: 'custom_slug', monitor_config: config def work(a, b, c); 1 / 0 @@ -284,23 +346,24 @@ def perform(a, b = 42, c: 99) it 'has correct custom options' do expect(Job.sentry_monitor_slug).to eq('custom_slug') - expect(Job.sentry_monitor_config).to eq(config) + expect(Job.sentry_monitor_config).to eq(monitor_config) end it 'calls capture_check_in twice with error status and re-raises exception' do - expect(Sentry).to receive(:capture_check_in).with( - 'custom_slug', - :in_progress, - hash_including(monitor_config: config) - ).ordered.and_call_original - - expect(Sentry).to receive(:capture_check_in).with( - 'custom_slug', - :error, - hash_including(:check_in_id, monitor_config: config, duration: 0) - ).ordered.and_call_original - expect { job.perform(1) }.to raise_error(ZeroDivisionError) + + expect(sentry_events.count).to eq(2) + in_progress_event = sentry_events.first + + expect(in_progress_event.monitor_slug).to eq('custom_slug') + expect(in_progress_event.status).to eq(:in_progress) + expect(in_progress_event.monitor_config).to eq(monitor_config) + + error_event = sentry_events.last + + expect(error_event.monitor_slug).to eq('custom_slug') + expect(error_event.status).to eq(:error) + expect(error_event.monitor_config).to eq(monitor_config) end end end diff --git a/sentry-ruby/spec/sentry/cron/monitor_config_spec.rb b/sentry-ruby/spec/sentry/cron/monitor_config_spec.rb index 7a6d91cf6..68bbbe95a 100644 --- a/sentry-ruby/spec/sentry/cron/monitor_config_spec.rb +++ b/sentry-ruby/spec/sentry/cron/monitor_config_spec.rb @@ -1,13 +1,7 @@ require 'spec_helper' RSpec.describe Sentry::Cron::MonitorConfig do - before do - perform_basic_setup do |config| - config.cron.default_checkin_margin = 1 - config.cron.default_max_runtime = 30 - config.cron.default_timezone = 'America/New_York' - end - end + before { perform_basic_setup } describe '.from_crontab' do it 'has correct attributes' do @@ -24,16 +18,6 @@ expect(subject.max_runtime).to eq(20) expect(subject.timezone).to eq('Europe/Vienna') end - - it 'fills in correct defaults from cron configuration' do - subject = described_class.from_crontab('5 * * * *') - - expect(subject.schedule).to be_a(Sentry::Cron::MonitorSchedule::Crontab) - expect(subject.schedule.value).to eq('5 * * * *') - expect(subject.checkin_margin).to eq(1) - expect(subject.max_runtime).to eq(30) - expect(subject.timezone).to eq('America/New_York') - end end describe '.from_interval' do @@ -57,17 +41,6 @@ expect(subject.max_runtime).to eq(20) expect(subject.timezone).to eq('Europe/Vienna') end - - it 'fills in correct defaults from cron configuration' do - subject = described_class.from_interval(5, :minute) - - expect(subject.schedule).to be_a(Sentry::Cron::MonitorSchedule::Interval) - expect(subject.schedule.value).to eq(5) - expect(subject.schedule.unit).to eq(:minute) - expect(subject.checkin_margin).to eq(1) - expect(subject.max_runtime).to eq(30) - expect(subject.timezone).to eq('America/New_York') - end end describe '#to_hash' do