diff --git a/CHANGELOG.md b/CHANGELOG.md index 080b12bac..40414bf42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Improve the accuracy of duration calculations in cron jobs monitoring ([#2471](https://github.com/getsentry/sentry-ruby/pull/2471)) - Use attempt_threshold to skip reporting on first N attempts ([#2503](https://github.com/getsentry/sentry-ruby/pull/2503)) - Support `code.namespace` for Ruby 3.4+ stacktraces ([#2506](https://github.com/getsentry/sentry-ruby/pull/2506)) +- Support report_after_job_retries for activejob ([#2500](https://github.com/getsentry/sentry-ruby/pull/2500)) ### Bug fixes diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 953af632b..0280d5c54 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -20,7 +20,7 @@ def already_supported_by_sentry_integration? class SentryReporter OP_NAME = "queue.active_job" SPAN_ORIGIN = "auto.queue.active_job" - + NOTIFICATION_NAME = "retry_stopped.active_job" class << self def record(job, &block) Sentry.with_scope do |scope| @@ -46,19 +46,42 @@ def record(job, &block) rescue Exception => e # rubocop:disable Lint/RescueException finish_sentry_transaction(transaction, 500) - Sentry::Rails.capture_exception( - e, - extra: sentry_context(job), - tags: { - job_id: job.job_id, - provider_job_id: job.provider_job_id - } - ) + unless Sentry.configuration.rails.active_job_report_after_job_retries + capture_exception(job, e) + end + raise end end end + def capture_exception(job, e) + Sentry::Rails.capture_exception( + e, + extra: sentry_context(job), + tags: { + job_id: job.job_id, + provider_job_id: job.provider_job_id + } + ) + end + + def register_retry_stopped_subscriber + ActiveSupport::Notifications.subscribe(NOTIFICATION_NAME) do |*args| + retry_stopped_handler(*args) + end + end + + def retry_stopped_handler(*args) + event = ActiveSupport::Notifications::Event.new(*args) + job = event.payload[:job] + error = event.payload[:error] + + return if !Sentry.initialized? || job.already_supported_by_sentry_integration? + return unless Sentry.configuration.rails.active_job_report_after_job_retries + capture_exception(job, error) + end + def finish_sentry_transaction(transaction, status) return unless transaction diff --git a/sentry-rails/lib/sentry/rails/configuration.rb b/sentry-rails/lib/sentry/rails/configuration.rb index a28a07f69..1b2fa93db 100644 --- a/sentry-rails/lib/sentry/rails/configuration.rb +++ b/sentry-rails/lib/sentry/rails/configuration.rb @@ -156,6 +156,10 @@ class Configuration # @return [Hash>] attr_accessor :active_support_logger_subscription_items + # Set this option to true if you want Sentry to only capture the last job + # retry if it fails. + attr_accessor :active_job_report_after_job_retries + def initialize @register_error_subscriber = false @report_rescued_exceptions = true @@ -172,6 +176,7 @@ def initialize @enable_db_query_source = true @db_query_source_threshold_ms = 100 @active_support_logger_subscription_items = Sentry::Rails::ACTIVE_SUPPORT_LOGGER_SUBSCRIPTION_ITEMS_DEFAULT.dup + @active_job_report_after_job_retries = false end end end diff --git a/sentry-rails/lib/sentry/rails/railtie.rb b/sentry-rails/lib/sentry/rails/railtie.rb index 0a0cdf213..4c8e5423e 100644 --- a/sentry-rails/lib/sentry/rails/railtie.rb +++ b/sentry-rails/lib/sentry/rails/railtie.rb @@ -51,6 +51,8 @@ class Railtie < ::Rails::Railtie activate_tracing register_error_subscriber(app) if ::Rails.version.to_f >= 7.0 && Sentry.configuration.rails.register_error_subscriber + + register_retry_stopped_subscriber if defined?(ActiveJob) end runner do @@ -137,5 +139,9 @@ def register_error_subscriber(app) require "sentry/rails/error_subscriber" app.executor.error_reporter.subscribe(Sentry::Rails::ErrorSubscriber.new) end + + def register_retry_stopped_subscriber + Sentry::Rails::ActiveJobExtensions::SentryReporter.register_retry_stopped_subscriber + end end end diff --git a/sentry-rails/spec/sentry/rails/activejob_spec.rb b/sentry-rails/spec/sentry/rails/activejob_spec.rb index ffcb4d44c..38a5ce236 100644 --- a/sentry-rails/spec/sentry/rails/activejob_spec.rb +++ b/sentry-rails/spec/sentry/rails/activejob_spec.rb @@ -67,6 +67,11 @@ class FailedJobWithCron < FailedJob sentry_monitor_check_ins slug: "failed_job", monitor_config: Sentry::Cron::MonitorConfig.from_crontab("5 * * * *") end +class FailedJobWithRetryOn < FailedJob + if respond_to? :retry_on + retry_on StandardError, attempts: 3, wait: 0 + end +end RSpec.describe "without Sentry initialized", type: :job do it "runs job" do @@ -318,7 +323,6 @@ def perform(event, hint) it "does not trigger sentry and re-raises" do expect { FailedJob.perform_now }.to raise_error(FailedJob::TestError) - expect(transport.events.size).to eq(0) end end @@ -386,4 +390,42 @@ def perform(event, hint) end end end + + describe "active_job_report_after_job_retries", skip: Rails.version.to_f < 5.1 do + before do + allow(Sentry::Rails::ActiveJobExtensions::SentryReporter) + .to receive(:capture_exception) + .and_call_original + end + context "when active_job_report_after_job_retries is false" do + it "reports 3 exceptions" do + assert_performed_jobs 3 do + FailedJobWithRetryOn.perform_later rescue nil + end + + expect(Sentry::Rails::ActiveJobExtensions::SentryReporter) + .to have_received(:capture_exception) + .exactly(3).times + end + end + context "when active_job_report_after_job_retries is true" do + before do + Sentry.configuration.rails.active_job_report_after_job_retries = true + end + + after do + Sentry.configuration.rails.active_job_report_after_job_retries = false + end + + it "reports 1 exception" do + assert_performed_jobs 3 do + FailedJobWithRetryOn.perform_later rescue nil + end + + expect(Sentry::Rails::ActiveJobExtensions::SentryReporter) + .to have_received(:capture_exception) + .exactly(1).times + end + end + end end diff --git a/sentry-rails/spec/sentry/rails/configuration_spec.rb b/sentry-rails/spec/sentry/rails/configuration_spec.rb index 37c5a41d6..b7961f894 100644 --- a/sentry-rails/spec/sentry/rails/configuration_spec.rb +++ b/sentry-rails/spec/sentry/rails/configuration_spec.rb @@ -59,4 +59,10 @@ class MySubscriber; end expect(subject.active_support_logger_subscription_items["foo"]).to include(:bar) end end + + describe "#active_job_report_after_job_retries" do + it "has correct default value" do + expect(subject.active_job_report_after_job_retries).to eq(false) + end + end end