From 2f590905a95eee46071d435673f517df2472bb04 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 08:26:52 -0800 Subject: [PATCH 01/12] 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 953af632b..61c44f723 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 62bdc4d2a7652e26a99ecf7aaf5cb0ef5b8a36b5 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 08:37:27 -0800 Subject: [PATCH 02/12] 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 61c44f723..ddb899ed4 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 3c2f78afb49bd2e6023a29a113791ea96a5b1d2f Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 08:49:56 -0800 Subject: [PATCH 03/12] 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 ddb899ed4..b9b447500 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 dd2a2f53d91d403fab2a5d86d71d6e7e7f99231c Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 08:53:11 -0800 Subject: [PATCH 04/12] 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 b9b447500..89f240909 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 b243abb74d277b6bf50c0f44cdcb7d309a8f5aec Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 08:54:22 -0800 Subject: [PATCH 05/12] 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 d495c114d586961b6476803aa3aa1bff029595a2 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 08:58:11 -0800 Subject: [PATCH 06/12] 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 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/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 08d3b406f47c26f9f896800523acbeb957915c3b Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 10:43:27 -0800 Subject: [PATCH 07/12] 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 89f240909..dcaf89d52 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 6d0cc35c34c4f509d32dd177de8f12ffa1170f82 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 10:46:07 -0800 Subject: [PATCH 08/12] 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 daee3d87e5f4eb78631035b695ea77019157ea23 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Wed, 18 Dec 2024 15:21:32 -0800 Subject: [PATCH 09/12] 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 dcaf89d52..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| @@ -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 ffcb4d44c..e3a500fe4 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 @@ -318,7 +321,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 +388,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 b0695703d04c4959f0fce951fbace12ca5c168a4 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Fri, 10 Jan 2025 16:18:44 -0800 Subject: [PATCH 10/12] 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 e3a500fe4..d2b263737 100644 --- a/sentry-rails/spec/sentry/rails/activejob_spec.rb +++ b/sentry-rails/spec/sentry/rails/activejob_spec.rb @@ -389,7 +389,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 8c05200786fd4bbd4f0df304098ffd7e8dfed21a Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Mon, 13 Jan 2025 08:49:22 -0800 Subject: [PATCH 11/12] 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 d2b263737..d69a3b109 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) @@ -390,6 +386,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 f8a24b279f60b6b3b43749fe9df004315406d2d8 Mon Sep 17 00:00:00 2001 From: jonathan schatz Date: Mon, 13 Jan 2025 08:51:30 -0800 Subject: [PATCH 12/12] 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 d69a3b109..38a5ce236 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) @@ -386,10 +392,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)