Skip to content

Commit

Permalink
fix!: Drop DelayedJob ActiveRecord in Tests
Browse files Browse the repository at this point in the history
The DelayedJob tests suite relied on the ActiveRecord Backend
gem for testing, however incompatabilities with ActiveRecord/Rails 7.1 means would
result in failures in the test suite.

It is not the responsibility of OTel Ruby Instrumentation to ensure compatability
of 3rd party gems or ensure our test suite to come up with workarounds for bugs.

For this reason I have rewritten the test suite to use the Test backend that Delayed Job
used to test the upstream gem.

This does introduce one change, the `created_at` attribute, which is _specific_ to the ActiveRecord backend
will _no longer_ be emitted. Users that need this event should pin to earlier versions of the gem.

There is a risk here that the upstream DelayedJob gem will no longer be included in future releases,
but I think that is less likely than to run into incompatabilities with future Rails versions.
  • Loading branch information
arielvalentin committed Oct 7, 2023
1 parent d3690bc commit a3ff8ed
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 61 deletions.
12 changes: 10 additions & 2 deletions instrumentation/delayed_job/Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@
#
# SPDX-License-Identifier: Apache-2.0

appraise 'delayed_job-4.1' do
gem 'delayed_job', '~> 4.1.0'
appraise 'delayed_job_4.1-rails-7.1' do
gem 'activejob', '~> 7.1.0'
end

appraise 'delayed_job_4.1-rails-7.0' do
gem 'activejob', '~> 7.0.0'
end

appraise 'delayed_job-4.1-rails-6.1' do
gem 'activejob', '~> 6.1.0'
end
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ def build_attributes(job)
end

def add_events(span, job)
span.add_event('created_at', timestamp: job.created_at)
span.add_event('run_at', timestamp: job.run_at) if job.run_at
span.add_event('locked_at', timestamp: job.locked_at) if job.locked_at
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ Gem::Specification.new do |spec|

spec.add_development_dependency 'appraisal', '~> 2.5'
spec.add_development_dependency 'bundler', '~> 2.4'
spec.add_development_dependency 'delayed_job', '~> 4.1.0'
spec.add_development_dependency 'delayed_job_active_record'
spec.add_development_dependency 'delayed_job', '~> 4.1.7'
spec.add_development_dependency 'minitest', '~> 5.0'
spec.add_development_dependency 'opentelemetry-sdk', '~> 1.1'
spec.add_development_dependency 'opentelemetry-test-helpers', '~> 0.3'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@
let(:span) { exporter.finished_spans.last }

before do
TestHelper.setup_active_record

Delayed::Worker.backend.delete_all
stub_const('BasicPayload', Class.new do
def perform; end
end)
Expand Down Expand Up @@ -50,8 +49,6 @@ def job_data

after do
OpenTelemetry.propagation = @orig_propagation

TestHelper.teardown_active_record
end

describe 'enqueue callback' do
Expand All @@ -74,11 +71,9 @@ def job_data
_(span.attributes['messaging.operation']).must_equal 'publish'
_(span.attributes['messaging.message_id']).must_be_kind_of String

_(span.events.size).must_equal 2
_(span.events[0].name).must_equal 'created_at'
_(span.events.size).must_equal 1
_(span.events[0].name).must_equal 'run_at'
_(span.events[0].timestamp).must_be_kind_of Integer
_(span.events[1].name).must_equal 'run_at'
_(span.events[1].timestamp).must_be_kind_of Integer
end

describe 'when queue name is set' do
Expand Down Expand Up @@ -124,7 +119,6 @@ def job_data
_(exporter.finished_spans.size).must_equal 1
_(exporter.finished_spans.first.name).must_equal 'default publish'
job_run
_(exporter.finished_spans.size).must_equal 2

_(span).must_be_kind_of OpenTelemetry::SDK::Trace::SpanData
_(span.name).must_equal 'default process'
Expand All @@ -138,17 +132,15 @@ def job_data
_(span.attributes['messaging.operation']).must_equal 'process'
_(span.attributes['messaging.message_id']).must_be_kind_of String

_(span.events.size).must_equal 3
_(span.events[0].name).must_equal 'created_at'
_(span.events[0].name).must_equal 'run_at'
_(span.events[0].timestamp).must_be_kind_of Integer
_(span.events[1].name).must_equal 'run_at'
_(span.events[1].name).must_equal 'locked_at'
_(span.events[1].timestamp).must_be_kind_of Integer
_(span.events[2].name).must_equal 'locked_at'
_(span.events[2].timestamp).must_be_kind_of Integer
end

describe 'when queue name is set' do
let(:job_params) { { queue: 'foobar_queue' } }
let(:job_enqueue) { Delayed::Job.enqueue(@basic_payload.new, job_params) }

it 'span tags include queue name' do
job_run
Expand Down Expand Up @@ -181,11 +173,10 @@ def job_data
it 'has resource name equal to underlying ActiveJob class name' do
job_run
_(span.attributes['messaging.delayed_job.name']).must_equal 'ErrorPayload'
_(span.events.size).must_equal 4
_(span.events[3].name).must_equal 'exception'
_(span.events[3].timestamp).must_be_kind_of Integer
_(span.events[3].attributes['exception.type']).must_equal 'ArgumentError'
_(span.events[3].attributes['exception.message']).must_equal 'This job failed'
_(span.events[2].name).must_equal 'exception'
_(span.events[2].timestamp).must_be_kind_of Integer
_(span.events[2].attributes['exception.type']).must_equal 'ArgumentError'
_(span.events[2].attributes['exception.message']).must_equal 'This job failed'
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
let(:exporter) { EXPORTER }

before do
Delayed::Worker.backend.delete_all
instrumentation.install
exporter.reset
end
Expand Down Expand Up @@ -48,14 +49,6 @@
end

describe 'tracing' do
before do
TestHelper.setup_active_record
end

after do
TestHelper.teardown_active_record
end

it 'before job' do
_(exporter.finished_spans.size).must_equal 0
end
Expand Down
40 changes: 11 additions & 29 deletions instrumentation/delayed_job/test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@
require 'bundler/setup'
Bundler.require(:default, :development, :test)

# These are dependencies that delayed job assumes are already loaded
# We are compensating for that here in this test... that is a smell
# NoMethodError: undefined method `extract_options!' for [#<ActiveJobPayload:0x0000000108bf5d48>, {}]:Array
# delayed_job-4.1.11/lib/delayed/backend/job_preparer.rb:7:in `initialize'0
require 'active_support/core_ext/array'

require 'opentelemetry-instrumentation-delayed_job'
require 'active_support/core_ext/kernel/reporting'

require 'minitest/autorun'
require 'rspec/mocks/minitest_integration'
Expand All @@ -24,31 +29,8 @@
c.add_span_processor span_processor
end

ActiveRecord::Migration.verbose = false

module TestHelper
extend self

def setup_active_record
::ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
::ActiveRecord::Schema.define do
create_table 'delayed_jobs', force: :cascade do |t|
t.integer 'priority', default: 0, null: false
t.integer 'attempts', default: 0, null: false
t.text 'handler', null: false
t.text 'last_error'
t.datetime 'run_at'
t.datetime 'locked_at'
t.datetime 'failed_at'
t.string 'locked_by'
t.string 'queue'
t.datetime 'created_at'
t.datetime 'updated_at'
end
end
end

def teardown_active_record
::ActiveRecord::Base.connection.close
end
end
gem_dir = Gem::Specification.find_by_name('delayed_job').gem_dir
require "#{gem_dir}/spec/delayed/serialization/test"
require "#{gem_dir}/spec/delayed/backend/test"

Delayed::Worker.backend = Delayed::Backend::Test::Job

0 comments on commit a3ff8ed

Please sign in to comment.