diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fa1f05d4..2be995b72 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,15 +4,56 @@ - Record client reports for profiles [#2107](https://github.com/getsentry/sentry-ruby/pull/2107) - Adopt Rails 7.1's new BroadcastLogger [#2120](https://github.com/getsentry/sentry-ruby/pull/2120) -- Add `Sentry.capture_check_in` API for Cron Monitoring [#2117](https://github.com/getsentry/sentry-ruby/pull/2117) +- Add [Cron Monitoring](https://docs.sentry.io/product/crons/) support + - Add `Sentry.capture_check_in` API for Cron Monitoring [#2117](https://github.com/getsentry/sentry-ruby/pull/2117) - You can now track progress of long running scheduled jobs. + You can now track progress of long running scheduled jobs. - ```rb - check_in_id = Sentry.capture_check_in('job_name', :in_progress) - # do job stuff - Sentry.capture_check_in('job_name', :ok, check_in_id: check_in_id) - ``` + ```rb + check_in_id = Sentry.capture_check_in('job_name', :in_progress) + # do job stuff + Sentry.capture_check_in('job_name', :ok, check_in_id: check_in_id) + ``` + - Add `Sentry::Cron::MonitorCheckIns` module for automatic monitoring of jobs [#2130](https://github.com/getsentry/sentry-ruby/pull/2130) + + Standard job frameworks such as `ActiveJob` and `Sidekiq` can now use this module to automatically capture check ins. + + ```rb + class ExampleJob < ApplicationJob + include Sentry::Cron::MonitorCheckIns + + sentry_monitor_check_ins + + def perform(*args) + # do stuff + end + end + ``` + + ```rb + class SidekiqJob + include Sidekiq::Job + include Sentry::Cron::MonitorCheckIns + + sentry_monitor_check_ins + + def perform(*args) + # do stuff + end + end + ``` + + You can pass in optional attributes to `sentry_monitor_check_ins` as follows. + ```rb + # slug defaults to the job class name + sentry_monitor_check_ins slug: 'custom_slug' + + # define the monitor config with an interval + sentry_monitor_check_ins monitor_config: Sentry::Cron::MonitorConfig.from_interval(1, :minute) + + # define the monitor config with a crontab + sentry_monitor_check_ins monitor_config: Sentry::Cron::MonitorConfig.from_crontab('5 * * * *') + ``` ### Bug Fixes diff --git a/sentry-rails/spec/sentry/rails/activejob_spec.rb b/sentry-rails/spec/sentry/rails/activejob_spec.rb index a2fe3dd39..f7a89c5e6 100644 --- a/sentry-rails/spec/sentry/rails/activejob_spec.rb +++ b/sentry-rails/spec/sentry/rails/activejob_spec.rb @@ -55,6 +55,17 @@ def rescue_callback(error) end end +class NormalJobWithCron < NormalJob + include Sentry::Cron::MonitorCheckIns + sentry_monitor_check_ins +end + +class FailedJobWithCron < FailedJob + include Sentry::Cron::MonitorCheckIns + sentry_monitor_check_ins slug: "failed_job", monitor_config: Sentry::Cron::MonitorConfig.from_crontab("5 * * * *") +end + + RSpec.describe "without Sentry initialized" do it "runs job" do expect { FailedJob.perform_now }.to raise_error(FailedJob::TestError) @@ -311,4 +322,68 @@ def perform(event, hint) end end end + + context "with cron monitoring mixin" do + context "normal job" do + it "returns #perform method's return value" do + expect(NormalJobWithCron.perform_now).to eq("foo") + end + + it "captures two check ins" do + NormalJobWithCron.perform_now + + expect(transport.events.size).to eq(2) + + first = transport.events[0] + check_in_id = first.check_in_id + expect(first).to be_a(Sentry::CheckInEvent) + expect(first.to_hash).to include( + type: 'check_in', + check_in_id: check_in_id, + monitor_slug: "NormalJobWithCron", + status: :in_progress + ) + + second = transport.events[1] + expect(second).to be_a(Sentry::CheckInEvent) + expect(second.to_hash).to include( + :duration, + type: 'check_in', + check_in_id: check_in_id, + monitor_slug: "NormalJobWithCron", + status: :ok + ) + end + end + + context "failed job" do + it "captures two check ins" do + expect { FailedJobWithCron.perform_now }.to raise_error(FailedJob::TestError) + + expect(transport.events.size).to eq(3) + + first = transport.events[0] + check_in_id = first.check_in_id + expect(first).to be_a(Sentry::CheckInEvent) + expect(first.to_hash).to include( + type: 'check_in', + check_in_id: check_in_id, + monitor_slug: "failed_job", + status: :in_progress, + monitor_config: { schedule: { type: :crontab, value: "5 * * * *" } } + ) + + second = transport.events[1] + expect(second).to be_a(Sentry::CheckInEvent) + expect(second.to_hash).to include( + :duration, + type: 'check_in', + check_in_id: check_in_id, + monitor_slug: "failed_job", + status: :error, + monitor_config: { schedule: { type: :crontab, value: "5 * * * *" } } + ) + end + end + end end diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index 3572ae8ac..59edf50d7 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -21,6 +21,7 @@ require "sentry/hub" require "sentry/background_worker" require "sentry/session_flusher" +require "sentry/cron/monitor_check_ins" [ "sentry/rake", diff --git a/sentry-ruby/lib/sentry/cron/monitor_check_ins.rb b/sentry-ruby/lib/sentry/cron/monitor_check_ins.rb new file mode 100644 index 000000000..b6323439e --- /dev/null +++ b/sentry-ruby/lib/sentry/cron/monitor_check_ins.rb @@ -0,0 +1,61 @@ +module Sentry + module Cron + module MonitorCheckIns + module Patch + def perform(*args) + slug = self.class.sentry_monitor_slug || self.class.name + monitor_config = self.class.sentry_monitor_config + + check_in_id = Sentry.capture_check_in(slug, + :in_progress, + monitor_config: monitor_config) + + start = Sentry.utc_now.to_i + ret = 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) + + ret + rescue Exception + duration = Sentry.utc_now.to_i - start + + Sentry.capture_check_in(slug, + :error, + check_in_id: check_in_id, + duration: duration, + monitor_config: monitor_config) + + raise + end + end + + module ClassMethods + def sentry_monitor_check_ins(slug: nil, monitor_config: nil) + @sentry_monitor_slug = slug + @sentry_monitor_config = monitor_config + + prepend Patch + end + + def sentry_monitor_slug + @sentry_monitor_slug + end + + def sentry_monitor_config + @sentry_monitor_config + end + end + + extend ClassMethods + + def self.included(base) + base.extend(ClassMethods) + end + end + end +end diff --git a/sentry-ruby/spec/sentry/cron/monitor_check_ins_spec.rb b/sentry-ruby/spec/sentry/cron/monitor_check_ins_spec.rb new file mode 100644 index 000000000..d03e9b6cc --- /dev/null +++ b/sentry-ruby/spec/sentry/cron/monitor_check_ins_spec.rb @@ -0,0 +1,234 @@ +require 'spec_helper' + +RSpec.describe Sentry::Cron::MonitorCheckIns do + before { perform_basic_setup } + + context 'without including mixin' do + before do + job_class = Class.new do + def work(a, b, c); end + + def perform(a, b = 42, c: 99) + work(a, b, c) + end + end + + stub_const('Job', job_class) + end + + let(:job) { Job.new } + + it 'does the work' do + expect(job).to receive(:work).with(1, 42, 99) + job.perform(1) + end + + it 'does not call capture_check_in' do + expect(Sentry).not_to receive(:capture_check_in) + job.perform(1) + end + end + + context 'including mixin' do + context 'without patching' do + before do + mod = described_class + + job_class = Class.new do + include mod + + def work(a, b, c); end + + def perform(a, b = 42, c: 99) + work(a, b, c) + end + end + + stub_const('Job', job_class) + end + + let(:job) { Job.new } + + it 'does the work' do + expect(job).to receive(:work).with(1, 42, 99) + job.perform(1) + end + + it 'does not prepend the patch' do + expect(Job.ancestors.first).not_to eq(described_class::Patch) + end + + it 'does not call capture_check_in' do + expect(Sentry).not_to receive(:capture_check_in) + job.perform(1) + end + + it 'class has extended methods' do + expect(Job.methods).to include( + :sentry_monitor_check_ins, + :sentry_monitor_slug, + :sentry_monitor_config + ) + end + end + + context 'patched with default options' do + before do + mod = described_class + + job_class = Class.new do + include mod + + sentry_monitor_check_ins + + def work(a, b, c); end + + def perform(a, b = 42, c: 99) + work(a, b, c) + end + end + + stub_const('Job', job_class) + end + + let(:job) { Job.new } + + it 'does the work' do + expect(job).to receive(:work).with(1, 42, 99) + job.perform(1) + end + + it 'prepends the patch' do + 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 + + expect(Sentry).to receive(:capture_check_in).with( + 'Job', + :ok, + hash_including(:check_in_id, monitor_config: nil, duration: 0) + ).ordered.and_call_original + + job.perform(1) + end + end + + context 'patched with custom options' do + let(:config) { Sentry::Cron::MonitorConfig::from_interval(1, :minute) } + + before do + mod = described_class + conf = config + + job_class = Class.new do + include mod + + sentry_monitor_check_ins slug: 'custom_slug', monitor_config: conf + + def work(a, b, c); end + + def perform(a, b = 42, c: 99) + work(a, b, c) + end + end + + stub_const('Job', job_class) + end + + let(:job) { Job.new } + + it 'does the work' do + expect(job).to receive(:work).with(1, 42, 99) + job.perform(1) + end + + it 'prepends the patch' do + expect(Job.ancestors.first).to eq(described_class::Patch) + end + + it 'has correct custom options' do + expect(Job.sentry_monitor_slug).to eq('custom_slug') + expect(Job.sentry_monitor_config).to eq(config) + 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 + + 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 + + job.perform(1) + end + end + + context 'patched with custom options with exception' do + let(:config) { Sentry::Cron::MonitorConfig::from_crontab('5 * * * *') } + + before do + mod = described_class + conf = config + + job_class = Class.new do + include mod + + sentry_monitor_check_ins slug: 'custom_slug', monitor_config: conf + + def work(a, b, c); + 1 / 0 + end + + def perform(a, b = 42, c: 99) + work(a, b, c) + end + end + + stub_const('Job', job_class) + end + + let(:job) { Job.new } + + it 'does the work' do + expect(job).to receive(:work).with(1, 42, 99) + job.perform(1) + end + + it 'prepends the patch' do + expect(Job.ancestors.first).to eq(described_class::Patch) + end + + it 'has correct custom options' do + expect(Job.sentry_monitor_slug).to eq('custom_slug') + expect(Job.sentry_monitor_config).to eq(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) + end + end + end +end diff --git a/sentry-sidekiq/spec/sentry/sidekiq_spec.rb b/sentry-sidekiq/spec/sentry/sidekiq_spec.rb index 114225189..f033da5d6 100644 --- a/sentry-sidekiq/spec/sentry/sidekiq_spec.rb +++ b/sentry-sidekiq/spec/sentry/sidekiq_spec.rb @@ -255,5 +255,59 @@ def retry_last_failed_job end end end -end + context "cron monitoring" do + it "records check ins" do + execute_worker(processor, HappyWorkerWithCron) + + expect(transport.events.size).to eq(2) + + first = transport.events[0] + check_in_id = first.check_in_id + expect(first).to be_a(Sentry::CheckInEvent) + expect(first.to_hash).to include( + type: 'check_in', + check_in_id: check_in_id, + monitor_slug: "HappyWorkerWithCron", + status: :in_progress + ) + + second = transport.events[1] + expect(second).to be_a(Sentry::CheckInEvent) + expect(second.to_hash).to include( + :duration, + type: 'check_in', + check_in_id: check_in_id, + monitor_slug: "HappyWorkerWithCron", + status: :ok + ) + end + + it "records check ins with error" do + execute_worker(processor, SadWorkerWithCron) + expect(transport.events.size).to eq(3) + + first = transport.events[0] + check_in_id = first.check_in_id + expect(first).to be_a(Sentry::CheckInEvent) + expect(first.to_hash).to include( + type: 'check_in', + check_in_id: check_in_id, + monitor_slug: "failed_job", + status: :in_progress, + monitor_config: { schedule: { type: :crontab, value: "5 * * * *" } } + ) + + second = transport.events[1] + expect(second).to be_a(Sentry::CheckInEvent) + expect(second.to_hash).to include( + :duration, + type: 'check_in', + check_in_id: check_in_id, + monitor_slug: "failed_job", + status: :error, + monitor_config: { schedule: { type: :crontab, value: "5 * * * *" } } + ) + end + end +end diff --git a/sentry-sidekiq/spec/spec_helper.rb b/sentry-sidekiq/spec/spec_helper.rb index 7b4ea9071..b4932fb79 100644 --- a/sentry-sidekiq/spec/spec_helper.rb +++ b/sentry-sidekiq/spec/spec_helper.rb @@ -129,6 +129,16 @@ def perform end end +class HappyWorkerWithCron < HappyWorker + include Sentry::Cron::MonitorCheckIns + sentry_monitor_check_ins +end + +class SadWorkerWithCron < SadWorker + include Sentry::Cron::MonitorCheckIns + sentry_monitor_check_ins slug: "failed_job", monitor_config: Sentry::Cron::MonitorConfig.from_crontab("5 * * * *") +end + class VerySadWorker include Sidekiq::Worker