From 2dc24dfdacebe7c4567fd6e8e91fa1b94632516f Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 08:26:52 -0800 Subject: [PATCH 01/20] add report_after_job_retries support for activejob --- sentry-rails/lib/sentry/rails/active_job.rb | 38 +++++++++++++++---- .../sentry/rails/active_job/configuration.rb | 23 +++++++++++ sentry-rails/lib/sentry/rails/railtie.rb | 6 +++ 3 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 sentry-rails/lib/sentry/rails/active_job/configuration.rb diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index fde7153eb..33fee71f0 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -46,19 +46,41 @@ 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.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("retry_stopped.active_job") do |*args| + retry_stopped_handler(*args) + end + end + + def retry_stopped_handler(*args) + return unless Sentry.configuration.active_job.report_after_job_retries + + event = ActiveSupport::Notifications::Event.new(*args) + job = event.payload[:job] + error = event.payload[:error] + capture_exception(job, error) + end + def finish_sentry_transaction(transaction, status) return unless transaction diff --git a/sentry-rails/lib/sentry/rails/active_job/configuration.rb b/sentry-rails/lib/sentry/rails/active_job/configuration.rb new file mode 100644 index 000000000..05362485a --- /dev/null +++ b/sentry-rails/lib/sentry/rails/active_job/configuration.rb @@ -0,0 +1,23 @@ +module Sentry + class Configuration + attr_reader :active_job + + add_post_initialization_callback do + @active_job = Sentry::Rails::ActiveJob::Configuration.new + end + end + + module Rails + module ActiveJob + class Configuration + # Set this option to true if you want Sentry to only capture the last job + # retry if it fails. + attr_accessor :report_after_job_retries + + def initialize + @report_after_job_retries = false + end + end + 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 From cf1b4eecdcb136ed376264459f4bc6ddd4b48e46 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 08:37:27 -0800 Subject: [PATCH 02/20] require configuration --- sentry-rails/lib/sentry/rails/active_job.rb | 1 + sentry-rails/lib/sentry/rails/active_job/configuration.rb | 2 ++ 2 files changed, 3 insertions(+) diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 33fee71f0..4464ff161 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "sentry/rails/active_job/configuration" module Sentry module Rails module ActiveJobExtensions diff --git a/sentry-rails/lib/sentry/rails/active_job/configuration.rb b/sentry-rails/lib/sentry/rails/active_job/configuration.rb index 05362485a..a1540cfac 100644 --- a/sentry-rails/lib/sentry/rails/active_job/configuration.rb +++ b/sentry-rails/lib/sentry/rails/active_job/configuration.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Sentry class Configuration attr_reader :active_job From 5508b64069ce494c50133d238be214868474382a Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 08:49:56 -0800 Subject: [PATCH 03/20] handle already_supported_by_sentry_integration --- sentry-rails/lib/sentry/rails/active_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 4464ff161..538edced7 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -74,8 +74,8 @@ def register_retry_stopped_subscriber end def retry_stopped_handler(*args) + return if already_supported_by_sentry_integration? return unless Sentry.configuration.active_job.report_after_job_retries - event = ActiveSupport::Notifications::Event.new(*args) job = event.payload[:job] error = event.payload[:error] From 0c6e90827e06b14e3658b3957406d00e19405991 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 08:53:11 -0800 Subject: [PATCH 04/20] copy other logic --- sentry-rails/lib/sentry/rails/active_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 538edced7..4effd4f17 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -74,7 +74,7 @@ def register_retry_stopped_subscriber end def retry_stopped_handler(*args) - return if already_supported_by_sentry_integration? + return if !Sentry.initialized? || already_supported_by_sentry_integration? return unless Sentry.configuration.active_job.report_after_job_retries event = ActiveSupport::Notifications::Event.new(*args) job = event.payload[:job] From 951864c48bdc63244e28a844f94c2ec9d5059a7b Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 08:54:22 -0800 Subject: [PATCH 05/20] changelog --- sentry-rails/CHANGELOG.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sentry-rails/CHANGELOG.md b/sentry-rails/CHANGELOG.md index 7e5238e6b..883fd72e3 100644 --- a/sentry-rails/CHANGELOG.md +++ b/sentry-rails/CHANGELOG.md @@ -2,11 +2,15 @@ Individual gem's changelog has been deprecated. Please check the [project changelog](https://github.com/getsentry/sentry-ruby/blob/master/CHANGELOG.md). +## Unreleased + +- Support report_after_job_retries for activejob ([#2500](https://github.com/getsentry/sentry-ruby/pull/2500)) + ## 4.4.0 ### Features -- Make tracing subscribers configurable [#1344](https://github.com/getsentry/sentry-ruby/pull/1344) +- Make tracing subscribers configurable [#1344](https://github.com/getsentry/sentry-ruby/pull/1344) ```ruby # current default: @@ -24,7 +28,7 @@ config.rails.tracing_subscribers = [MySubscriber] - Report exceptions from the interceptor middleware for exceptions app [#1379](https://github.com/getsentry/sentry-ruby/pull/1379) - Fixes [#1371](https://github.com/getsentry/sentry-ruby/issues/1371) -- Re-position CaptureExceptions middleware to reduce tracing noise [#1405](https://github.com/getsentry/sentry-ruby/pull/1405) +- Re-position CaptureExceptions middleware to reduce tracing noise [#1405](https://github.com/getsentry/sentry-ruby/pull/1405) ## 4.3.4 From 0c1fa77609ae699abec001710bda8fb6389cd4e3 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 08:58:11 -0800 Subject: [PATCH 06/20] changelog --- CHANGELOG.md | 1 + sentry-rails/CHANGELOG.md | 8 ++------ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c82bd7c6..8ee02c75c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,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/CHANGELOG.md b/sentry-rails/CHANGELOG.md index 883fd72e3..7e5238e6b 100644 --- a/sentry-rails/CHANGELOG.md +++ b/sentry-rails/CHANGELOG.md @@ -2,15 +2,11 @@ Individual gem's changelog has been deprecated. Please check the [project changelog](https://github.com/getsentry/sentry-ruby/blob/master/CHANGELOG.md). -## Unreleased - -- Support report_after_job_retries for activejob ([#2500](https://github.com/getsentry/sentry-ruby/pull/2500)) - ## 4.4.0 ### Features -- Make tracing subscribers configurable [#1344](https://github.com/getsentry/sentry-ruby/pull/1344) +- Make tracing subscribers configurable [#1344](https://github.com/getsentry/sentry-ruby/pull/1344) ```ruby # current default: @@ -28,7 +24,7 @@ config.rails.tracing_subscribers = [MySubscriber] - Report exceptions from the interceptor middleware for exceptions app [#1379](https://github.com/getsentry/sentry-ruby/pull/1379) - Fixes [#1371](https://github.com/getsentry/sentry-ruby/issues/1371) -- Re-position CaptureExceptions middleware to reduce tracing noise [#1405](https://github.com/getsentry/sentry-ruby/pull/1405) +- Re-position CaptureExceptions middleware to reduce tracing noise [#1405](https://github.com/getsentry/sentry-ruby/pull/1405) ## 4.3.4 From 4a51738f8f44162bb27310660a8721a6f8b40761 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 10:43:27 -0800 Subject: [PATCH 07/20] flatten configuration to rails.actibve_job_report_after_job_retries --- sentry-rails/lib/sentry/rails/active_job.rb | 5 ++-- .../sentry/rails/active_job/configuration.rb | 25 ------------------- .../lib/sentry/rails/configuration.rb | 4 +++ 3 files changed, 6 insertions(+), 28 deletions(-) delete mode 100644 sentry-rails/lib/sentry/rails/active_job/configuration.rb diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 4effd4f17..cfbce6256 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "sentry/rails/active_job/configuration" module Sentry module Rails module ActiveJobExtensions @@ -47,7 +46,7 @@ def record(job, &block) rescue Exception => e # rubocop:disable Lint/RescueException finish_sentry_transaction(transaction, 500) - unless Sentry.configuration.active_job.report_after_job_retries + unless Sentry.configuration.rails.active_job_report_after_job_retries capture_exception(job, e) end @@ -75,7 +74,7 @@ def register_retry_stopped_subscriber def retry_stopped_handler(*args) return if !Sentry.initialized? || already_supported_by_sentry_integration? - return unless Sentry.configuration.active_job.report_after_job_retries + return unless Sentry.configuration.rails.active_job_report_after_job_retries event = ActiveSupport::Notifications::Event.new(*args) job = event.payload[:job] error = event.payload[:error] diff --git a/sentry-rails/lib/sentry/rails/active_job/configuration.rb b/sentry-rails/lib/sentry/rails/active_job/configuration.rb deleted file mode 100644 index a1540cfac..000000000 --- a/sentry-rails/lib/sentry/rails/active_job/configuration.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -module Sentry - class Configuration - attr_reader :active_job - - add_post_initialization_callback do - @active_job = Sentry::Rails::ActiveJob::Configuration.new - end - end - - module Rails - module ActiveJob - class Configuration - # Set this option to true if you want Sentry to only capture the last job - # retry if it fails. - attr_accessor :report_after_job_retries - - def initialize - @report_after_job_retries = false - end - end - end - end -end diff --git a/sentry-rails/lib/sentry/rails/configuration.rb b/sentry-rails/lib/sentry/rails/configuration.rb index a28a07f69..8dfade4e6 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 From f3717a3aa4f85066e62d6b4949704e4f74cf30b1 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 10:46:07 -0800 Subject: [PATCH 08/20] add config default / spec --- sentry-rails/lib/sentry/rails/configuration.rb | 1 + sentry-rails/spec/sentry/rails/configuration_spec.rb | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/sentry-rails/lib/sentry/rails/configuration.rb b/sentry-rails/lib/sentry/rails/configuration.rb index 8dfade4e6..1b2fa93db 100644 --- a/sentry-rails/lib/sentry/rails/configuration.rb +++ b/sentry-rails/lib/sentry/rails/configuration.rb @@ -176,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/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 From 80e899ec3279a82519a4952b3c02677023ba767c Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 15:21:32 -0800 Subject: [PATCH 09/20] add tests --- sentry-rails/lib/sentry/rails/active_job.rb | 9 ++-- .../spec/sentry/rails/activejob_spec.rb | 42 ++++++++++++++++++- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index cfbce6256..7c8339b23 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| @@ -67,17 +67,18 @@ def capture_exception(job, e) end def register_retry_stopped_subscriber - ActiveSupport::Notifications.subscribe("retry_stopped.active_job") do |*args| + ActiveSupport::Notifications.subscribe(NOTIFICATION_NAME) do |*args| retry_stopped_handler(*args) end end def retry_stopped_handler(*args) - return if !Sentry.initialized? || already_supported_by_sentry_integration? - return unless Sentry.configuration.rails.active_job_report_after_job_retries 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 diff --git a/sentry-rails/spec/sentry/rails/activejob_spec.rb b/sentry-rails/spec/sentry/rails/activejob_spec.rb index 0758cc7d3..dd0f73948 100644 --- a/sentry-rails/spec/sentry/rails/activejob_spec.rb +++ b/sentry-rails/spec/sentry/rails/activejob_spec.rb @@ -67,6 +67,9 @@ class FailedJobWithCron < FailedJob sentry_monitor_check_ins slug: "failed_job", monitor_config: Sentry::Cron::MonitorConfig.from_crontab("5 * * * *") end +class FailedJobWithRetryOn < FailedJob + retry_on StandardError, attempts: 3, wait: 0 +end RSpec.describe "without Sentry initialized", type: :job do it "runs job" do @@ -361,7 +364,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 @@ -429,4 +431,42 @@ def perform(event, hint) end end end + + describe "active_job_report_after_job_retries" 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 From 02def8c8502d54b968914ecf1e4933be650ad246 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Fri, 10 Jan 2025 16:18:44 -0800 Subject: [PATCH 10/20] skip retry_on tests with rails < 5.1 --- sentry-rails/spec/sentry/rails/activejob_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-rails/spec/sentry/rails/activejob_spec.rb b/sentry-rails/spec/sentry/rails/activejob_spec.rb index dd0f73948..714331175 100644 --- a/sentry-rails/spec/sentry/rails/activejob_spec.rb +++ b/sentry-rails/spec/sentry/rails/activejob_spec.rb @@ -432,7 +432,7 @@ def perform(event, hint) end end - describe "active_job_report_after_job_retries" do + 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) From 125ee3f55149f499c13740a164a15e99beaeeb18 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Mon, 13 Jan 2025 08:49:22 -0800 Subject: [PATCH 11/20] move class declaration into rail > 5.1 block --- sentry-rails/spec/sentry/rails/activejob_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry-rails/spec/sentry/rails/activejob_spec.rb b/sentry-rails/spec/sentry/rails/activejob_spec.rb index 714331175..02a90dd95 100644 --- a/sentry-rails/spec/sentry/rails/activejob_spec.rb +++ b/sentry-rails/spec/sentry/rails/activejob_spec.rb @@ -67,10 +67,6 @@ class FailedJobWithCron < FailedJob sentry_monitor_check_ins slug: "failed_job", monitor_config: Sentry::Cron::MonitorConfig.from_crontab("5 * * * *") end -class FailedJobWithRetryOn < FailedJob - retry_on StandardError, attempts: 3, wait: 0 -end - RSpec.describe "without Sentry initialized", type: :job do it "runs job" do expect { FailedJob.perform_now }.to raise_error(FailedJob::TestError) @@ -433,6 +429,10 @@ def perform(event, hint) end describe "active_job_report_after_job_retries", skip: Rails.version.to_f < 5.1 do + class FailedJobWithRetryOn < FailedJob + retry_on StandardError, attempts: 3, wait: 0 + end + before do allow(Sentry::Rails::ActiveJobExtensions::SentryReporter) .to receive(:capture_exception) From 12f6a3684eab18f0f66326a36f1ffe72fa7a4520 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Mon, 13 Jan 2025 08:51:30 -0800 Subject: [PATCH 12/20] try a different way --- sentry-rails/spec/sentry/rails/activejob_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sentry-rails/spec/sentry/rails/activejob_spec.rb b/sentry-rails/spec/sentry/rails/activejob_spec.rb index 02a90dd95..cd95b7c9b 100644 --- a/sentry-rails/spec/sentry/rails/activejob_spec.rb +++ b/sentry-rails/spec/sentry/rails/activejob_spec.rb @@ -67,6 +67,12 @@ 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 expect { FailedJob.perform_now }.to raise_error(FailedJob::TestError) @@ -429,10 +435,6 @@ def perform(event, hint) end describe "active_job_report_after_job_retries", skip: Rails.version.to_f < 5.1 do - class FailedJobWithRetryOn < FailedJob - retry_on StandardError, attempts: 3, wait: 0 - end - before do allow(Sentry::Rails::ActiveJobExtensions::SentryReporter) .to receive(:capture_exception) From 943d418c8812823656fa78ab01534b453ab037ec Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Thu, 6 Mar 2025 12:24:38 +0000 Subject: [PATCH 13/20] Fix formatting --- sentry-rails/lib/sentry/rails/active_job.rb | 1 + sentry-rails/spec/sentry/rails/activejob_spec.rb | 14 ++++++++------ 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 7c8339b23..85aa94d5b 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -21,6 +21,7 @@ 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| diff --git a/sentry-rails/spec/sentry/rails/activejob_spec.rb b/sentry-rails/spec/sentry/rails/activejob_spec.rb index cd95b7c9b..a83e438a7 100644 --- a/sentry-rails/spec/sentry/rails/activejob_spec.rb +++ b/sentry-rails/spec/sentry/rails/activejob_spec.rb @@ -435,13 +435,11 @@ def perform(event, hint) 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 + allow(Sentry::Rails::ActiveJobExtensions::SentryReporter) + .to receive(:capture_exception).and_call_original + assert_performed_jobs 3 do FailedJobWithRetryOn.perform_later rescue nil end @@ -451,6 +449,7 @@ def perform(event, hint) .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 @@ -461,7 +460,10 @@ def perform(event, hint) end it "reports 1 exception" do - assert_performed_jobs 3 do + allow(Sentry::Rails::ActiveJobExtensions::SentryReporter) + .to receive(:capture_exception).and_call_original + + assert_performed_jobs 1 do FailedJobWithRetryOn.perform_later rescue nil end From 942bb6427be3a6b02807be1231493c53d4a85911 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Thu, 6 Mar 2025 12:59:12 +0000 Subject: [PATCH 14/20] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ee02c75c..e3c72f94e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ ### Features - Add correct breadcrumb levels for 4xx/5xx response codes ([#2549](https://github.com/getsentry/sentry-ruby/pull/2549)) +- [sentry-rails] New configuration option called `report_after_job_retries` for ActiveJob which makes reporting exceptions only happen when the last retry attempt failed ([#2500](https://github.com/getsentry/sentry-ruby/pull/2500)) ### Bug Fixes @@ -68,7 +69,6 @@ - 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 From ed1321e2a30503bca382e8c789da9fae2ae3ee44 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Thu, 6 Mar 2025 13:10:45 +0000 Subject: [PATCH 15/20] Properly detach retry_stopped.active_job subscriber Otherwise specs would be failing randomly --- sentry-rails/lib/sentry/rails/active_job.rb | 13 +++++++++++-- sentry-rails/spec/sentry/rails/activejob_spec.rb | 2 +- sentry-rails/spec/spec_helper.rb | 4 ++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 85aa94d5b..8fa1f8716 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -68,8 +68,17 @@ def capture_exception(job, e) end def register_retry_stopped_subscriber - ActiveSupport::Notifications.subscribe(NOTIFICATION_NAME) do |*args| - retry_stopped_handler(*args) + unless @retry_stopped_subscriber + @retry_stopped_subscriber = ActiveSupport::Notifications.subscribe(NOTIFICATION_NAME) do |*args| + retry_stopped_handler(*args) + end + end + end + + def detach_retry_stopped_subscriber + if @retry_stopped_subscriber + ActiveSupport::Notifications.unsubscribe(@retry_stopped_subscriber) + @retry_stopped_subscriber = nil end end diff --git a/sentry-rails/spec/sentry/rails/activejob_spec.rb b/sentry-rails/spec/sentry/rails/activejob_spec.rb index a83e438a7..b0b5764b0 100644 --- a/sentry-rails/spec/sentry/rails/activejob_spec.rb +++ b/sentry-rails/spec/sentry/rails/activejob_spec.rb @@ -463,7 +463,7 @@ def perform(event, hint) allow(Sentry::Rails::ActiveJobExtensions::SentryReporter) .to receive(:capture_exception).and_call_original - assert_performed_jobs 1 do + assert_performed_jobs 3 do FailedJobWithRetryOn.perform_later rescue nil end diff --git a/sentry-rails/spec/spec_helper.rb b/sentry-rails/spec/spec_helper.rb index ee086813f..345137197 100644 --- a/sentry-rails/spec/spec_helper.rb +++ b/sentry-rails/spec/spec_helper.rb @@ -52,6 +52,10 @@ expect(Sentry::Rails::Tracing.subscribed_tracing_events).to be_empty Sentry::Rails::Tracing.remove_active_support_notifications_patch + if defined?(Sentry::Rails::ActiveJobExtensions::SentryReporter) + Sentry::Rails::ActiveJobExtensions::SentryReporter.detach_retry_stopped_subscriber + end + reset_sentry_globals! end From 98855e90956175c4be0a4fb3a43c766b23ad8c63 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Thu, 6 Mar 2025 13:19:55 +0000 Subject: [PATCH 16/20] Fix for jruby --- sentry-rails/spec/spec_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-rails/spec/spec_helper.rb b/sentry-rails/spec/spec_helper.rb index 345137197..e6b578e4f 100644 --- a/sentry-rails/spec/spec_helper.rb +++ b/sentry-rails/spec/spec_helper.rb @@ -52,7 +52,7 @@ expect(Sentry::Rails::Tracing.subscribed_tracing_events).to be_empty Sentry::Rails::Tracing.remove_active_support_notifications_patch - if defined?(Sentry::Rails::ActiveJobExtensions::SentryReporter) + if defined?(Sentry::Rails::ActiveJobExtensions) Sentry::Rails::ActiveJobExtensions::SentryReporter.detach_retry_stopped_subscriber end From 754da2397854fdbf8db654d236b9e889c1153dcf Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Thu, 6 Mar 2025 13:52:57 +0000 Subject: [PATCH 17/20] Update CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3c72f94e..b92ac387e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ ### Features - Add correct breadcrumb levels for 4xx/5xx response codes ([#2549](https://github.com/getsentry/sentry-ruby/pull/2549)) -- [sentry-rails] New configuration option called `report_after_job_retries` for ActiveJob which makes reporting exceptions only happen when the last retry attempt failed ([#2500](https://github.com/getsentry/sentry-ruby/pull/2500)) +- New configuration option called `report_after_job_retries` for ActiveJob which makes reporting exceptions only happen when the last retry attempt failed ([#2500](https://github.com/getsentry/sentry-ruby/pull/2500)) ### Bug Fixes From 8649f69217411a1f9631f9ff53f82c1f1195bf08 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Fri, 7 Mar 2025 13:42:53 +0000 Subject: [PATCH 18/20] Fix flaky specs, I think --- sentry-rails/lib/sentry/rails/railtie.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry-rails/lib/sentry/rails/railtie.rb b/sentry-rails/lib/sentry/rails/railtie.rb index 4c8e5423e..2025c7c88 100644 --- a/sentry-rails/lib/sentry/rails/railtie.rb +++ b/sentry-rails/lib/sentry/rails/railtie.rb @@ -52,7 +52,8 @@ class Railtie < ::Rails::Railtie register_error_subscriber(app) if ::Rails.version.to_f >= 7.0 && Sentry.configuration.rails.register_error_subscriber - register_retry_stopped_subscriber if defined?(ActiveJob) + # Presence of ActiveJob is no longer a reliable cue + register_retry_stopped_subscriber if defined?(Sentry::Rails::ActiveJobExtensions) end runner do From 0763fec8b8d11fd24bd557f3006eaa0e43a3ef70 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Fri, 7 Mar 2025 13:53:33 +0000 Subject: [PATCH 19/20] Fix spec for old rails --- sentry-rails/spec/sentry/rails/activejob_spec.rb | 2 +- sentry-rails/spec/spec_helper.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/sentry-rails/spec/sentry/rails/activejob_spec.rb b/sentry-rails/spec/sentry/rails/activejob_spec.rb index b0b5764b0..e9d65b31f 100644 --- a/sentry-rails/spec/sentry/rails/activejob_spec.rb +++ b/sentry-rails/spec/sentry/rails/activejob_spec.rb @@ -434,7 +434,7 @@ def perform(event, hint) end end - describe "active_job_report_after_job_retries", skip: Rails.version.to_f < 5.1 do + describe "active_job_report_after_job_retries", skip: RAILS_VERSION < 7.0 do context "when active_job_report_after_job_retries is false" do it "reports 3 exceptions" do allow(Sentry::Rails::ActiveJobExtensions::SentryReporter) diff --git a/sentry-rails/spec/spec_helper.rb b/sentry-rails/spec/spec_helper.rb index e6b578e4f..b64dd2c19 100644 --- a/sentry-rails/spec/spec_helper.rb +++ b/sentry-rails/spec/spec_helper.rb @@ -34,6 +34,8 @@ Dir["#{__dir__}/support/**/*.rb"].each { |file| require file } +RAILS_VERSION = Rails.version.to_f + RSpec.configure do |config| # Enable flags like --only-failures and --next-failure config.example_status_persistence_file_path = ".rspec_status" From 4e595699536d8356f4c784d9ad7a15214e5bbbfe Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 11 Mar 2025 14:51:41 +0000 Subject: [PATCH 20/20] Update CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b92ac387e..36952799b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,13 +16,13 @@ ### Internal - Remove `user_segment` from DSC ([#2586](https://github.com/getsentry/sentry-ruby/pull/2586)) +- New configuration option called `report_after_job_retries` for ActiveJob which makes reporting exceptions only happen when the last retry attempt failed ([#2500](https://github.com/getsentry/sentry-ruby/pull/2500)) ## 5.23.0 ### Features - Add correct breadcrumb levels for 4xx/5xx response codes ([#2549](https://github.com/getsentry/sentry-ruby/pull/2549)) -- New configuration option called `report_after_job_retries` for ActiveJob which makes reporting exceptions only happen when the last retry attempt failed ([#2500](https://github.com/getsentry/sentry-ruby/pull/2500)) ### Bug Fixes