While investigating #1369 I half-suspected that once sucker_punch was fixed upstream that our instrumentation would not be correct, and now that sucker_punch 3.0.1 is out and does it correctly, I coded up a quick test that shows this:
puts RUBY_DESCRIPTION
require 'sucker_punch'
class FooJob
include SuckerPunch::Job
def perform(*args, required:)
puts [args, required].inspect
end
end
FooJob.perform_async(1, required: 2)
FooJob.perform_in(0, 1, required: 2)
FooJob.__run_perform(1, required: 2)
sleep 1
require 'ddtrace'
Datadog.configure do |c|
c.use :sucker_punch
end
FooJob.perform_async(1, required: 2)
FooJob.perform_in(0, 1, required: 2)
FooJob.__run_perform(1, required: 2)
sleep 1Behavior on Ruby 2.x:
$ bundle exec ruby sucker_punch_instrumentation.rb
ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-darwin19]
[[1], 2]
[[1], 2]
[[1], 2]
[[1], 2]
[[1], 2]
[[1], 2]
Behavior on Ruby 3.x:
$ bundle exec ruby sucker_punch_instrumentation.rb
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin19]
[[1], 2]
[[1], 2]
[[1], 2]
E, [2021-02-18T09:00:41.528096 #15995] ERROR -- : Sucker Punch job error for class: 'FooJob' args: [1, {:required=>2}]
ArgumentError missing keyword: :required
sucker_punch_instrumentation.rb:8:in `perform'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/sucker_punch-3.0.1/lib/sucker_punch/job.rb:63:in `__run_perform'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:32:in `block in __run_perform'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:77:in `block in __with_instrumentation'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/tracer.rb:284:in `trace'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:73:in `__with_instrumentation'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:21:in `__run_perform'
sucker_punch_instrumentation.rb:27:in `<main>'
E, [2021-02-18T09:00:41.528391 #15995] ERROR -- : Sucker Punch job error for class: 'FooJob' args: [1, {:required=>2}]
ArgumentError missing keyword: :required
sucker_punch_instrumentation.rb:8:in `perform'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/sucker_punch-3.0.1/lib/sucker_punch/job.rb:63:in `__run_perform'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:32:in `block in __run_perform'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:77:in `block in __with_instrumentation'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/tracer.rb:284:in `trace'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:73:in `__with_instrumentation'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:21:in `__run_perform'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/sucker_punch-3.0.1/lib/sucker_punch/job.rb:38:in `block in perform_async'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:363:in `run_task'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:352:in `block (3 levels) in create_worker'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:335:in `loop'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:335:in `block (2 levels) in create_worker'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:334:in `catch'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:334:in `block in create_worker'
E, [2021-02-18T09:00:41.529427 #15995] ERROR -- : Sucker Punch job error for class: 'FooJob' args: [1, {:required=>2}]
ArgumentError missing keyword: :required
sucker_punch_instrumentation.rb:8:in `perform'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/sucker_punch-3.0.1/lib/sucker_punch/job.rb:63:in `__run_perform'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:32:in `block in __run_perform'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:77:in `block in __with_instrumentation'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/tracer.rb:284:in `trace'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:73:in `__with_instrumentation'
/Users/ivo.anjo/datadog/dd-trace-rb/lib/ddtrace/contrib/sucker_punch/instrumentation.rb:21:in `__run_perform'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/sucker_punch-3.0.1/lib/sucker_punch/job.rb:46:in `block in perform_in'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/safe_task_executor.rb:24:in `block in execute'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/synchronization/mutex_lockable_object.rb:41:in `block in synchronize'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/synchronization/mutex_lockable_object.rb:41:in `synchronize'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/synchronization/mutex_lockable_object.rb:41:in `synchronize'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/safe_task_executor.rb:19:in `execute'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/ivar.rb:169:in `safe_execute'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/scheduled_task.rb:285:in `process_task'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/timer_set.rb:98:in `block in ns_post_task'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:363:in `run_task'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:352:in `block (3 levels) in create_worker'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:335:in `loop'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:335:in `block (2 levels) in create_worker'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:334:in `catch'
/Users/ivo.anjo/.rvm/gems/ruby-3.0.0/gems/concurrent-ruby-1.1.8/lib/concurrent-ruby/concurrent/executor/ruby_thread_pool_executor.rb:334:in `block in create_worker'The fix is probably quite simple: sprinkle ruby2_keywords :foo if respond_to?(:ruby2_keywords, true) on a bunch of our methods, see https://eregon.me/blog/2021/02/13/correct-delegation-in-ruby-2-27-3.html , and add tests to cover it ;)
While investigating #1369 I half-suspected that once
sucker_punchwas fixed upstream that our instrumentation would not be correct, and now thatsucker_punch3.0.1 is out and does it correctly, I coded up a quick test that shows this:Behavior on Ruby 2.x:
Behavior on Ruby 3.x:
The fix is probably quite simple: sprinkle
ruby2_keywords :foo if respond_to?(:ruby2_keywords, true)on a bunch of our methods, see https://eregon.me/blog/2021/02/13/correct-delegation-in-ruby-2-27-3.html , and add tests to cover it ;)