Skip to content

Commit 4f51c1c

Browse files
committed
fix(sidekiq): enrich failures before ActiveJob runs
1 parent 0f926b4 commit 4f51c1c

3 files changed

Lines changed: 77 additions & 2 deletions

File tree

‎sentry-ruby/lib/sentry/scope.rb‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,10 @@ def set_span(span)
218218
@span = span
219219
end
220220

221+
def clear_span
222+
@span = nil
223+
end
224+
221225
# @!macro set_user
222226
def set_user(user_hash)
223227
check_argument_type!(user_hash, Hash)

‎sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,28 @@ def call(worker, job, queue)
4949

5050
def perform_through_active_job(job, sidekiq_job)
5151
args = job["args"]
52-
job["args"] = job_extension.active_job_args(sidekiq_job, sidekiq_config: sidekiq_config)
53-
yield
52+
original_hub = Sentry.get_current_hub_internal
53+
Sentry.clone_hub_to_current_thread
54+
55+
Sentry.with_scope do |scope|
56+
scope.clear_span
57+
consumer.apply_scope_data(scope, sidekiq_job, default_scope_data: job_extension.scope_data(sidekiq_job))
58+
scope.set_transaction_name(job_extension.transaction_name(job), source: :task)
59+
Sentry.continue_trace(job_extension.extract_trace_headers(sidekiq_job))
60+
job["args"] = job_extension.active_job_args(sidekiq_job, sidekiq_config: sidekiq_config)
61+
62+
begin
63+
yield
64+
rescue => e
65+
context = { context: JobExtension::JOB_ERROR_CONTEXT, job: sidekiq_job }
66+
context[:jobstr] = ::Sidekiq.dump_json(sidekiq_job) if JobExtension::JOB_STRING_IN_ERROR_CONTEXT
67+
ErrorHandler.new.call(e, context, sidekiq_config)
68+
raise
69+
end
70+
end
5471
ensure
5572
job["args"] = args
73+
Sentry.set_current_hub_internal(original_hub)
5674
end
5775

5876
def consumer

‎sentry-sidekiq/spec/isolated/active_job_spec.rb‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,59 @@ def reported_executions(job_class, error, adapter_skipped:)
395395
end
396396
end
397397

398+
context "when the queued ActiveJob class cannot be loaded" do
399+
let(:traces_sample_rate) { nil }
400+
401+
[nil, 1.0].each do |sample_rate|
402+
it "preserves native error enrichment and the incoming trace at sample rate #{sample_rate.inspect}" do
403+
Sentry.configuration.traces_sample_rate = sample_rate
404+
worker_transaction = Sentry.start_transaction(name: "worker", op: "test")
405+
Sentry.get_main_hub.current_scope.set_span(worker_transaction) if worker_transaction
406+
407+
message = sidekiq_message(
408+
parity_job,
409+
retry: false,
410+
tags: ["marvel"],
411+
sentry_user: { "id" => "42" },
412+
trace_propagation_headers: { "sentry-trace" => "#{"a" * 32}-#{"b" * 16}-0" }
413+
)
414+
message["wrapped"] = "RemovedJob"
415+
message["args"].first["job_class"] = "RemovedJob"
416+
417+
native_errors = process(message, adapter_skipped: true).grep(Sentry::ErrorEvent)
418+
active_job_errors = process(message, adapter_skipped: false).grep(Sentry::ErrorEvent)
419+
420+
expect(native_errors.size).to eq(1)
421+
expect(active_job_errors.size).to eq(1)
422+
expect(active_job_errors.first.tags).to include(native_errors.first.tags)
423+
expect(active_job_errors.first.user).to eq(native_errors.first.user)
424+
expect(active_job_errors.first.contexts.dig(:trace, :trace_id)).to eq("a" * 32)
425+
end
426+
end
427+
428+
it "restores the worker hub and payload after a failure before perform_now" do
429+
skip_sidekiq_adapter(false)
430+
message = sidekiq_message(parity_job, retry: false)
431+
message["args"].first["job_class"] = "RemovedJob"
432+
original_args = message["args"]
433+
original_payload = Marshal.load(Marshal.dump(original_args))
434+
hub = Sentry.get_current_hub
435+
Sentry.set_tags(worker: "outside")
436+
437+
expect do
438+
Sentry::Sidekiq::SentryContextServerMiddleware.new.call(nil, message, "events") do
439+
ActiveJob::Base.execute(message["args"].first)
440+
end
441+
end.to raise_error(NameError)
442+
443+
expect(message["args"]).to equal(original_args)
444+
expect(message["args"]).to eq(original_payload)
445+
expect(Sentry.get_current_hub).to equal(hub)
446+
expect(Sentry.get_current_scope.tags).to eq(worker: "outside")
447+
expect(transport.events.grep(Sentry::ErrorEvent).size).to eq(1)
448+
end
449+
end
450+
398451
context "with a separate running Sidekiq configuration", skip: !WITH_SIDEKIQ_7 do
399452
let(:running_config) do
400453
Sidekiq::Config.new(max_retries: 5).tap do |config|

0 commit comments

Comments
 (0)