Skip to content

Commit

Permalink
Connect Sidekiq's transaction with its parent when possible (#1590)
Browse files Browse the repository at this point in the history
* Connect Sidekiq's transaction with the previous one

If there's a parent transaction on the client side, its sentry_trace
should be passed and picked by the server side middleware, so the 2
transactions can connect.

* Update changelog
  • Loading branch information
st0012 committed Oct 9, 2021
1 parent 5c17674 commit dde4255
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 11 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
- Start Testing Against Rails 7.0 [#1581](https://github.com/getsentry/sentry-ruby/pull/1581)
- Tracing subscribers should be multi-event based [#1587](https://github.com/getsentry/sentry-ruby/pull/1587)

### Bug Fixes

- Connect `Sidekiq`'s transaction with its parent when possible [#1590](https://github.com/getsentry/sentry-ruby/pull/1590)
- Fixes [#1586](https://github.com/getsentry/sentry-ruby/issues/1586)

## 4.7.3

- Avoid leaking tracing timestamp to breadcrumbs [#1575](https://github.com/getsentry/sentry-ruby/pull/1575)
Expand Down
12 changes: 10 additions & 2 deletions sentry-sidekiq/lib/sentry/sidekiq/sentry_context_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ def call(_worker, job, queue)
scope.set_tags(queue: queue, jid: job["jid"])
scope.set_contexts(sidekiq: job.merge("queue" => queue))
scope.set_transaction_name(context_filter.transaction_name)
transaction = Sentry.start_transaction(name: scope.transaction_name, op: "sidekiq")
transaction = start_transaction(scope.transaction_name, job["sentry_trace"])
scope.set_span(transaction) if transaction

begin
yield
rescue => e
rescue
finish_transaction(transaction, 500)
raise
end
Expand All @@ -32,6 +32,12 @@ def call(_worker, job, queue)
scope.clear
end

def start_transaction(transaction_name, sentry_trace)
options = { name: transaction_name, op: "sidekiq" }
transaction = Sentry::Transaction.from_sentry_trace(sentry_trace, **options) if sentry_trace
Sentry.start_transaction(transaction: transaction, **options)
end

def finish_transaction(transaction, status)
return unless transaction

Expand All @@ -45,7 +51,9 @@ def call(_worker_class, job, _queue, _redis_pool)
return yield unless Sentry.initialized?

user = Sentry.get_current_scope.user
transaction = Sentry.get_current_scope.get_transaction
job["sentry_user"] = user unless user.empty?
job["sentry_trace"] = transaction.to_sentry_trace if transaction
yield
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,19 @@
event = transport.events.last
expect(event.user).to eq(user)
end

context "with sentry_trace" do
let(:parent_transaction) { Sentry.start_transaction(op: "sidekiq") }

it "starts the transaction from it" do
execute_worker(processor, HappyWorker, sentry_trace: parent_transaction.to_sentry_trace)

expect(transport.events.count).to eq(1)
transaction = transport.events.first
expect(transaction).not_to be_nil
expect(transaction.contexts.dig(:trace, :trace_id)).to eq(parent_transaction.trace_id)
end
end
end
end

Expand All @@ -70,22 +83,44 @@

before do
queue.clear
perform_basic_setup
perform_basic_setup do |config|
config.traces_sample_rate = 1.0
end
end

it "does not add user to the job if user is absence in the current scope" do
it "does not add user or sentry_trace to the job if they're absence in the current scope" do
client.push('queue' => 'default', 'class' => HappyWorker, 'args' => [])

expect(queue.size).to be(1)
expect(queue.first["sentry_user"]).to be_nil
expect(queue.first["sentry_trace"]).to be_nil
end

it "sets user of the current scope to the job if present" do
Sentry.set_user(user)
describe "with user" do
before do
Sentry.set_user(user)
end

client.push('queue' => 'default', 'class' => HappyWorker, 'args' => [])
it "sets user of the current scope to the job" do
client.push('queue' => 'default', 'class' => HappyWorker, 'args' => [])

expect(queue.size).to be(1)
expect(queue.first["sentry_user"]).to eq(user)
expect(queue.size).to be(1)
expect(queue.first["sentry_user"]).to eq(user)
end
end

describe "with transaction" do
let(:transaction) { Sentry.start_transaction(op: "sidekiq") }

before do
Sentry.get_current_scope.set_span(transaction)
end

it "sets user of the current scope to the job" do
client.push('queue' => 'default', 'class' => HappyWorker, 'args' => [])

expect(queue.size).to be(1)
expect(queue.first["sentry_trace"]).to eq(transaction.to_sentry_trace)
end
end
end
3 changes: 1 addition & 2 deletions sentry-sidekiq/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,8 @@ def perform
end
end

def execute_worker(processor, klass)
def execute_worker(processor, klass, **options)
klass_options = klass.sidekiq_options_hash || {}
options = {}

# for Ruby < 2.6
klass_options.each do |k, v|
Expand Down

0 comments on commit dde4255

Please sign in to comment.