Skip to content

Commit 7db2c29

Browse files
solnicmodosc
andauthored
[active_job] support exception reporting only after last retry failed (#2573)
* add report_after_job_retries support for activejob * require configuration * handle already_supported_by_sentry_integration * copy other logic * changelog * changelog * flatten configuration to rails.actibve_job_report_after_job_retries * add config default / spec * add tests * skip retry_on tests with rails < 5.1 * move class declaration into rail > 5.1 block * try a different way * Fix formatting * Update CHANGELOG.md * Properly detach retry_stopped.active_job subscriber Otherwise specs would be failing randomly * Fix for jruby * Update CHANGELOG * Fix flaky specs, I think * Fix spec for old rails * Update CHANGELOG --------- Co-authored-by: jonathan schatz <[email protected]>
1 parent a884cae commit 7db2c29

File tree

7 files changed

+111
-9
lines changed

7 files changed

+111
-9
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
### Internal
1717

1818
- Remove `user_segment` from DSC ([#2586](https://github.com/getsentry/sentry-ruby/pull/2586))
19+
- 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))
1920

2021
## 5.23.0
2122

sentry-rails/lib/sentry/rails/active_job.rb

+41-8
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ def already_supported_by_sentry_integration?
2020
class SentryReporter
2121
OP_NAME = "queue.active_job"
2222
SPAN_ORIGIN = "auto.queue.active_job"
23+
NOTIFICATION_NAME = "retry_stopped.active_job"
2324

2425
class << self
2526
def record(job, &block)
@@ -46,19 +47,51 @@ def record(job, &block)
4647
rescue Exception => e # rubocop:disable Lint/RescueException
4748
finish_sentry_transaction(transaction, 500)
4849

49-
Sentry::Rails.capture_exception(
50-
e,
51-
extra: sentry_context(job),
52-
tags: {
53-
job_id: job.job_id,
54-
provider_job_id: job.provider_job_id
55-
}
56-
)
50+
unless Sentry.configuration.rails.active_job_report_after_job_retries
51+
capture_exception(job, e)
52+
end
53+
5754
raise
5855
end
5956
end
6057
end
6158

59+
def capture_exception(job, e)
60+
Sentry::Rails.capture_exception(
61+
e,
62+
extra: sentry_context(job),
63+
tags: {
64+
job_id: job.job_id,
65+
provider_job_id: job.provider_job_id
66+
}
67+
)
68+
end
69+
70+
def register_retry_stopped_subscriber
71+
unless @retry_stopped_subscriber
72+
@retry_stopped_subscriber = ActiveSupport::Notifications.subscribe(NOTIFICATION_NAME) do |*args|
73+
retry_stopped_handler(*args)
74+
end
75+
end
76+
end
77+
78+
def detach_retry_stopped_subscriber
79+
if @retry_stopped_subscriber
80+
ActiveSupport::Notifications.unsubscribe(@retry_stopped_subscriber)
81+
@retry_stopped_subscriber = nil
82+
end
83+
end
84+
85+
def retry_stopped_handler(*args)
86+
event = ActiveSupport::Notifications::Event.new(*args)
87+
job = event.payload[:job]
88+
error = event.payload[:error]
89+
90+
return if !Sentry.initialized? || job.already_supported_by_sentry_integration?
91+
return unless Sentry.configuration.rails.active_job_report_after_job_retries
92+
capture_exception(job, error)
93+
end
94+
6295
def finish_sentry_transaction(transaction, status)
6396
return unless transaction
6497

sentry-rails/lib/sentry/rails/configuration.rb

+5
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,10 @@ class Configuration
156156
# @return [Hash<String, Array<Symbol>>]
157157
attr_accessor :active_support_logger_subscription_items
158158

159+
# Set this option to true if you want Sentry to only capture the last job
160+
# retry if it fails.
161+
attr_accessor :active_job_report_after_job_retries
162+
159163
def initialize
160164
@register_error_subscriber = false
161165
@report_rescued_exceptions = true
@@ -172,6 +176,7 @@ def initialize
172176
@enable_db_query_source = true
173177
@db_query_source_threshold_ms = 100
174178
@active_support_logger_subscription_items = Sentry::Rails::ACTIVE_SUPPORT_LOGGER_SUBSCRIPTION_ITEMS_DEFAULT.dup
179+
@active_job_report_after_job_retries = false
175180
end
176181
end
177182
end

sentry-rails/lib/sentry/rails/railtie.rb

+7
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ class Railtie < ::Rails::Railtie
5151
activate_tracing
5252

5353
register_error_subscriber(app) if ::Rails.version.to_f >= 7.0 && Sentry.configuration.rails.register_error_subscriber
54+
55+
# Presence of ActiveJob is no longer a reliable cue
56+
register_retry_stopped_subscriber if defined?(Sentry::Rails::ActiveJobExtensions)
5457
end
5558

5659
runner do
@@ -137,5 +140,9 @@ def register_error_subscriber(app)
137140
require "sentry/rails/error_subscriber"
138141
app.executor.error_reporter.subscribe(Sentry::Rails::ErrorSubscriber.new)
139142
end
143+
144+
def register_retry_stopped_subscriber
145+
Sentry::Rails::ActiveJobExtensions::SentryReporter.register_retry_stopped_subscriber
146+
end
140147
end
141148
end

sentry-rails/spec/sentry/rails/activejob_spec.rb

+45-1
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ class FailedJobWithCron < FailedJob
6767
sentry_monitor_check_ins slug: "failed_job", monitor_config: Sentry::Cron::MonitorConfig.from_crontab("5 * * * *")
6868
end
6969

70+
class FailedJobWithRetryOn < FailedJob
71+
if respond_to? :retry_on
72+
retry_on StandardError, attempts: 3, wait: 0
73+
end
74+
end
7075

7176
RSpec.describe "without Sentry initialized", type: :job do
7277
it "runs job" do
@@ -361,7 +366,6 @@ def perform(event, hint)
361366

362367
it "does not trigger sentry and re-raises" do
363368
expect { FailedJob.perform_now }.to raise_error(FailedJob::TestError)
364-
365369
expect(transport.events.size).to eq(0)
366370
end
367371
end
@@ -429,4 +433,44 @@ def perform(event, hint)
429433
end
430434
end
431435
end
436+
437+
describe "active_job_report_after_job_retries", skip: RAILS_VERSION < 7.0 do
438+
context "when active_job_report_after_job_retries is false" do
439+
it "reports 3 exceptions" do
440+
allow(Sentry::Rails::ActiveJobExtensions::SentryReporter)
441+
.to receive(:capture_exception).and_call_original
442+
443+
assert_performed_jobs 3 do
444+
FailedJobWithRetryOn.perform_later rescue nil
445+
end
446+
447+
expect(Sentry::Rails::ActiveJobExtensions::SentryReporter)
448+
.to have_received(:capture_exception)
449+
.exactly(3).times
450+
end
451+
end
452+
453+
context "when active_job_report_after_job_retries is true" do
454+
before do
455+
Sentry.configuration.rails.active_job_report_after_job_retries = true
456+
end
457+
458+
after do
459+
Sentry.configuration.rails.active_job_report_after_job_retries = false
460+
end
461+
462+
it "reports 1 exception" do
463+
allow(Sentry::Rails::ActiveJobExtensions::SentryReporter)
464+
.to receive(:capture_exception).and_call_original
465+
466+
assert_performed_jobs 3 do
467+
FailedJobWithRetryOn.perform_later rescue nil
468+
end
469+
470+
expect(Sentry::Rails::ActiveJobExtensions::SentryReporter)
471+
.to have_received(:capture_exception)
472+
.exactly(1).times
473+
end
474+
end
475+
end
432476
end

sentry-rails/spec/sentry/rails/configuration_spec.rb

+6
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,10 @@ class MySubscriber; end
5959
expect(subject.active_support_logger_subscription_items["foo"]).to include(:bar)
6060
end
6161
end
62+
63+
describe "#active_job_report_after_job_retries" do
64+
it "has correct default value" do
65+
expect(subject.active_job_report_after_job_retries).to eq(false)
66+
end
67+
end
6268
end

sentry-rails/spec/spec_helper.rb

+6
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434

3535
Dir["#{__dir__}/support/**/*.rb"].each { |file| require file }
3636

37+
RAILS_VERSION = Rails.version.to_f
38+
3739
RSpec.configure do |config|
3840
# Enable flags like --only-failures and --next-failure
3941
config.example_status_persistence_file_path = ".rspec_status"
@@ -52,6 +54,10 @@
5254
expect(Sentry::Rails::Tracing.subscribed_tracing_events).to be_empty
5355
Sentry::Rails::Tracing.remove_active_support_notifications_patch
5456

57+
if defined?(Sentry::Rails::ActiveJobExtensions)
58+
Sentry::Rails::ActiveJobExtensions::SentryReporter.detach_retry_stopped_subscriber
59+
end
60+
5561
reset_sentry_globals!
5662
end
5763

0 commit comments

Comments
 (0)