diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d2360ffd..af25e2044 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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+ diff --git a/sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb b/sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb index 937fd797c..954e98311 100644 --- a/sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb +++ b/sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb @@ -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 @@ -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 diff --git a/sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb b/sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb index 4e4f65202..85fcaeb99 100644 --- a/sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb +++ b/sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb @@ -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 }) @@ -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