Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .github/workflows/sentry_sidekiq_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ jobs:
redis-version: ${{ (contains(matrix.sidekiq_version, '7.0') || contains(matrix.sidekiq_version, '8.0')) && 6 || 5 }}

- name: Run specs with Sidekiq ${{ matrix.sidekiq_version }}
env:
WITH_SENTRY_RAILS: 1
run: bundle exec rake

- name: Upload Coverage
Expand Down
13 changes: 8 additions & 5 deletions sentry-sidekiq/Rakefile
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
# frozen_string_literal: true

require "bundler/gem_tasks"
require "rspec/core/rake_task"
require_relative "../lib/sentry/test/rake_tasks"

RSpec::Core::RakeTask.new(:spec).tap do |task|
task.rspec_opts = "--order rand"
end
Sentry::Test::RakeTasks.define_spec_tasks(
spec_pattern: "spec/sentry/**/*_spec.rb",
spec_rspec_opts: "--order rand --format progress",
isolated_specs_pattern: "spec/isolated/**/*_spec.rb",
isolated_rspec_opts: "--format progress"
)

task default: :spec
task default: [:spec, :"spec:isolated"]
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
# frozen_string_literal: true

return unless ENV["WITH_SENTRY_RAILS"]

require "logger"
begin
require "simplecov"
SimpleCov.command_name "SidekiqRails"
rescue LoadError
end

require "rails"
require "sentry-rails"
require "spec_helper"

require "action_controller/railtie"
# This MUST be required after sentry-rails because it requires sentry-sidekiq
# which checks if Railtie is defined to properly set things up
require_relative "../spec_helper"

class TestApp < Rails::Application
end
Expand All @@ -23,15 +25,9 @@ def self.name
app.config.hosts = nil
app.config.secret_key_base = "test"
app.config.eager_load = false

app.initializer :configure_sentry do
Sentry.init do |config|
config.release = 'beta'
config.dsn = "dummy://12345:67890@sentry.localdomain:3000/sentry/42"
config.transport.transport_class = Sentry::DummyTransport
# for sending events synchronously
config.background_worker_threads = 0
yield(config, app) if block_given?
end
perform_basic_setup
end

app.initialize!
Expand Down
24 changes: 14 additions & 10 deletions sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,10 @@

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')

span = transport.events.last.spans.detect { |span| span[:op] == "queue.publish" }
expect(span[:op]).to eq("queue.publish")
expect(span[:data]['messaging.destination.name']).to eq('default')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Nil-check missing after detect-span lookup

This test case is missing a nil check after using detect to find a span. If no matching span is found, accessing its properties will raise a NoMethodError. This is inconsistent with other similar span checks in this file and commit.

Fix in Cursor Fix in Web

end

it 'adds job to sidekiq within transaction' do
Expand All @@ -124,12 +124,16 @@
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')

span = events[0].spans.detect { |span| span[:op] == "queue.publish" }
expect(span).not_to be_nil
expect(span[:op]).to eq("queue.publish")
expect(span[:data]['messaging.destination.name']).to eq('default')

span = events[1].spans.detect { |span| span[:op] == "queue.publish" }
expect(span).not_to be_nil
expect(span[:op]).to eq("queue.publish")
expect(span[: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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,12 @@ def ensure_queue_empty(queue, timeout: 0.1)
transaction.finish

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.message.id']).to eq(message_id)
expect(event.spans[0][:data]['messaging.destination.name']).to eq('default')

span = transport.events.last.spans.detect { |span| span[:op] == "queue.publish" }
expect(span).not_to be_nil
expect(span[:op]).to eq("queue.publish")
expect(span[:data]['messaging.message.id']).to eq(message_id)
expect(span[:data]['messaging.destination.name']).to eq('default')
end

it "does not propagate headers with propagate_traces = false" do
Expand Down
1 change: 1 addition & 0 deletions sentry-sidekiq/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ def perform_basic_setup
config.sdk_logger = ::Logger.new(nil)
config.background_worker_threads = 0
config.transport.transport_class = Sentry::DummyTransport

yield config if block_given?
end
end
Loading