Skip to content

Commit

Permalink
Reset trace and transaction for sidekiq-cron
Browse files Browse the repository at this point in the history
Fixes #2391
  • Loading branch information
frederikspang authored Nov 5, 2024
1 parent 9446a30 commit ceba0da
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- Add `include_sentry_event` matcher for RSpec [#2424](https://github.com/getsentry/sentry-ruby/pull/2424)
- Add support for Sentry Cache instrumentation, when using Rails.cache ([#2380](https://github.com/getsentry/sentry-ruby/pull/2380))
- Add support for Queue Instrumentation for Sidekiq. [#2403](https://github.com/getsentry/sentry-ruby/pull/2403)
- Reset trace_id and add root transaction for sidekiq-cron [#2446](https://github.com/getsentry/sentry-ruby/pull/2446)

Note: MemoryStore and FileStore require Rails 8.0+

Expand Down
42 changes: 42 additions & 0 deletions sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,32 @@ module Sentry
module Sidekiq
module Cron
module Job
def self.enqueueing_method
::Sidekiq::Cron::Job.instance_methods.include?(:enque!) ? :enque! : :enqueue!
end

define_method(enqueueing_method) do |*args|
# make sure the current thread has a clean hub
Sentry.clone_hub_to_current_thread

Sentry.with_scope do |scope|
Sentry.with_session_tracking do
begin
scope.set_transaction_name("#{name} (#{klass})")

transaction = start_transaction(scope)
scope.set_span(transaction) if transaction
super(*args)

finish_transaction(transaction, 200)
rescue
finish_transaction(transaction, 500)
raise
end
end
end
end

def save
# validation failed, do nothing
return false unless super
Expand All @@ -34,6 +60,22 @@ def save

true
end

def start_transaction(scope)
Sentry.start_transaction(
name: scope.transaction_name,
source: scope.transaction_source,
op: "queue.sidekiq-cron",
origin: "auto.queue.sidekiq.cron"
)
end

def finish_transaction(transaction, status_code)
return unless transaction

transaction.set_http_status(status_code)
transaction.finish
end
end
end
end
Expand Down
48 changes: 47 additions & 1 deletion sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,24 @@
return unless defined?(Sidekiq::Cron::Job)

RSpec.describe Sentry::Sidekiq::Cron::Job do
let(:processor) do
new_processor
end

let(:transport) do
Sentry.get_current_client.transport
end

before do
perform_basic_setup { |c| c.enabled_patches += [:sidekiq_cron] }
perform_basic_setup do |c|
c.enabled_patches += [:sidekiq_cron]
c.traces_sample_rate = 1.0
end
end

before do
Sidekiq::Cron::Job.destroy_all!
Sidekiq::Queue.all.each(&:clear)
schedule_file = 'spec/fixtures/sidekiq-cron-schedule.yml'
schedule = Sidekiq::Cron::Support.load_yaml(ERB.new(IO.read(schedule_file)).result)
schedule = schedule.merge(symbol_name: { cron: '* * * * *', class: HappyWorkerWithSymbolName })
Expand Down Expand Up @@ -79,4 +92,37 @@
it 'does not patch ReportingWorker because of invalid schedule' do
expect(ReportingWorker.ancestors).not_to include(Sentry::Cron::MonitorSchedule)
end

describe 'sidekiq-cron' do
it 'adds job to sidekiq within transaction' do
job = Sidekiq::Cron::Job.new(name: 'test', cron: 'not a crontab', class: 'HappyWorkerForCron')
job.send(Sentry::Sidekiq::Cron::Job.enqueueing_method)

expect(::Sidekiq::Queue.new.size).to eq(1)
expect(transport.events.count).to eq(1)
event = transport.events.last
expect(event.spans.count).to eq(1)
expect(event.spans[0][:op]).to eq("queue.publish")
expect(event.spans[0][:data]['messaging.destination.name']).to eq('default')
end

it 'adds job to sidekiq within transaction' do
job = Sidekiq::Cron::Job.new(name: 'test', cron: 'not a crontab', class: 'HappyWorkerForCron')
job.send(Sentry::Sidekiq::Cron::Job.enqueueing_method)
# Time passes.
job.send(Sentry::Sidekiq::Cron::Job.enqueueing_method)

expect(::Sidekiq::Queue.new.size).to eq(2)
expect(transport.events.count).to eq(2)
events = transport.events
expect(events[0].spans.count).to eq(1)
expect(events[0].spans[0][:op]).to eq("queue.publish")
expect(events[0].spans[0][:data]['messaging.destination.name']).to eq('default')
expect(events[1].spans.count).to eq(1)
expect(events[1].spans[0][:op]).to eq("queue.publish")
expect(events[1].spans[0][:data]['messaging.destination.name']).to eq('default')

expect(events[0].dynamic_sampling_context['trace_id']).to_not eq(events[1].dynamic_sampling_context['trace_id'])
end
end
end

0 comments on commit ceba0da

Please sign in to comment.