Skip to content

Commit 0cf8374

Browse files
committed
fix(sidekiq): trace failures before ActiveJob performs
1 parent 75acb3d commit 0cf8374

5 files changed

Lines changed: 57 additions & 11 deletions

File tree

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,15 @@ def perform_now
2121
super
2222
else
2323
data = _sentry || {}
24+
transaction = Sentry.get_current_scope.get_span
25+
transaction = nil unless transaction.is_a?(Sentry::Transaction) && transaction.span_id == data["transaction_span_id"]
2426
SentryReporter.record(
2527
self,
2628
trace_headers: data["trace_propagation_headers"],
2729
user: data["user"],
2830
delivery_queue: data["delivery_queue"],
29-
payload: _sentry_job_data
31+
payload: _sentry_job_data,
32+
transaction: transaction
3033
) { super }
3134
end
3235
end
@@ -140,7 +143,7 @@ def log_producer_span_error(e)
140143
Sentry.sdk_logger&.error("sentry-rails: failed to record producer span: #{e.class}: #{e.message}\n #{Array(e.backtrace).first(5).join("\n ")}")
141144
end
142145

143-
def record(job, trace_headers: nil, user: nil, delivery_queue: nil, payload: nil, &block)
146+
def record(job, trace_headers: nil, user: nil, delivery_queue: nil, payload: nil, transaction: nil, &block)
144147
original_hub = Sentry.get_current_hub_internal
145148
Sentry.clone_hub_to_current_thread
146149

@@ -162,6 +165,7 @@ def record(job, trace_headers: nil, user: nil, delivery_queue: nil, payload: nil
162165

163166
transaction = consumer.start_transaction(
164167
scope, job,
168+
transaction: transaction.is_a?(Sentry::Transaction) ? transaction : nil,
165169
trace_headers: trace_headers,
166170
payload: payload,
167171
messaging_data: -> {

‎sentry-ruby/lib/sentry/job_extensions/consumer.rb‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def transaction_name(job)
2424
registry.call(registry.for(job), :transaction_name, job: job)
2525
end
2626

27-
def start_transaction(scope, job, trace_headers: nil, payload: nil, always_continue_trace: false, messaging_data: {}, default_retry_count: nil)
27+
def start_transaction(scope, job, transaction: nil, trace_headers: nil, payload: nil, always_continue_trace: false, messaging_data: {}, default_retry_count: nil)
2828
extension = registry.for(job)
2929
options = {
3030
name: scope.transaction_name,
@@ -42,11 +42,16 @@ def start_transaction(scope, job, trace_headers: nil, payload: nil, always_conti
4242
end
4343
end
4444

45-
transaction = if always_continue_trace || (trace_headers && !trace_headers.empty?)
46-
continued = Sentry.continue_trace(trace_headers, **options)
47-
Sentry.start_transaction(transaction: continued, **options)
45+
if transaction
46+
transaction.set_name(options[:name], source: options[:source])
47+
transaction.set_origin(options[:origin])
4848
else
49-
Sentry.start_transaction(**options)
49+
transaction = if always_continue_trace || (trace_headers && !trace_headers.empty?)
50+
continued = Sentry.continue_trace(trace_headers, **options)
51+
Sentry.start_transaction(transaction: continued, **options)
52+
else
53+
Sentry.start_transaction(**options)
54+
end
5055
end
5156
return unless transaction
5257

@@ -61,7 +66,7 @@ def start_transaction(scope, job, trace_headers: nil, payload: nil, always_conti
6166
end
6267

6368
def finish_transaction(transaction, status)
64-
return unless transaction
69+
return if !transaction || transaction.timestamp
6570

6671
transaction.set_http_status(status)
6772
transaction.finish

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,12 @@ def performed_by_active_job?(sidekiq_job)
3030
!!sidekiq_job["wrapped"] && active_job_owns_adapter?
3131
end
3232

33-
def active_job_args(sidekiq_job, sidekiq_config: nil)
33+
def active_job_args(sidekiq_job, sidekiq_config: nil, transaction: nil)
3434
payload, *arguments = sidekiq_job["args"]
3535
return sidekiq_job["args"] unless payload.is_a?(Hash)
3636

3737
sentry_data = (payload["_sentry"] || {}).merge("delivery_queue" => sidekiq_job["queue"])
38+
sentry_data["transaction_span_id"] = transaction.span_id if transaction
3839
[payload.merge(
3940
SIDEKIQ_JOB_KEY => sidekiq_job,
4041
SIDEKIQ_RETRY_LIMIT_KEY => @retry_policy.limit(sidekiq_job, sidekiq_config),

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,25 @@ def perform_through_active_job(job, sidekiq_job)
5656
scope.clear_span
5757
consumer.apply_scope_data(scope, sidekiq_job, default_scope_data: job_extension.scope_data(job: sidekiq_job))
5858
scope.set_transaction_name(job_extension.transaction_name(job: job), source: :task)
59-
Sentry.continue_trace(job_extension.extract_trace_headers(job: sidekiq_job, payload: sidekiq_job))
60-
job["args"] = job_extension.active_job_args(sidekiq_job, sidekiq_config: sidekiq_config)
59+
trace_headers = args.first.is_a?(Hash) && args.first.dig("_sentry", "trace_propagation_headers")
60+
trace_headers = job_extension.extract_trace_headers(job: sidekiq_job, payload: sidekiq_job) if !trace_headers || trace_headers.empty?
61+
transaction = consumer.start_transaction(
62+
scope, sidekiq_job,
63+
payload: sidekiq_job,
64+
trace_headers: trace_headers,
65+
messaging_data: -> { job_extension.consumer_messaging_data(sidekiq_job) },
66+
default_retry_count: job_extension.retry_info(job: sidekiq_job)[:count],
67+
always_continue_trace: true
68+
)
69+
job["args"] = job_extension.active_job_args(sidekiq_job, sidekiq_config: sidekiq_config, transaction: transaction)
6170

6271
completed = false
6372
begin
6473
result = yield
6574
completed = true
6675
result
6776
ensure
77+
consumer.finish_transaction(transaction, completed ? 200 : 500)
6878
if !completed && (exception = $!)
6979
context = { context: JobExtension::JOB_ERROR_CONTEXT, job: sidekiq_job }
7080
context[:jobstr] = ::Sidekiq.dump_json(sidekiq_job) if JobExtension::JOB_STRING_IN_ERROR_CONTEXT

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,32 @@ def perform; end
483483
end
484484
end
485485

486+
it "records one failed consumer transaction when the queued job class is missing" do
487+
Sentry.configuration.traces_sample_rate = 1.0
488+
message = sidekiq_message(
489+
parity_job, retry: false, queue: "delivery",
490+
trace_propagation_headers: { "sentry-trace" => "#{"a" * 32}-#{"b" * 16}-1" }
491+
)
492+
message["wrapped"] = "RemovedJob"
493+
message["args"].first["job_class"] = "RemovedJob"
494+
495+
[true, false].each do |adapter_skipped|
496+
events = process(message, adapter_skipped: adapter_skipped)
497+
transactions = consumer_transactions(events)
498+
errors = events.grep(Sentry::ErrorEvent)
499+
500+
expect(transactions.size).to eq(1)
501+
expect(errors.size).to eq(1)
502+
expect(transactions.first.transaction).to eq("Sidekiq/RemovedJob")
503+
expect(transactions.first.contexts.dig(:trace, :status)).to eq("internal_error")
504+
expect(transactions.first.contexts.dig(:trace, :trace_id)).to eq("a" * 32)
505+
expect(transactions.first.contexts.dig(:trace, :data)).to include(
506+
"messaging.message.id" => message["jid"], "messaging.destination.name" => "delivery"
507+
)
508+
expect(errors.first.contexts.dig(:trace, :span_id)).to eq(transactions.first.contexts.dig(:trace, :span_id))
509+
end
510+
end
511+
486512
it "restores the worker hub and payload after a failure before perform_now" do
487513
skip_sidekiq_adapter(false)
488514
message = sidekiq_message(parity_job, retry: false)

0 commit comments

Comments
 (0)