Merge branch 'dev' into custom_log_attributes_api

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## dev
 
-Version <dev> of the agent adds the ability to filter logs by level, an API to add custom attributes to logs, expands instrumentation for Action Cable, provides a bugfix for Code-Level Metrics, and fixes `NewRelic::Agent::Logging::DecoratingFormatter#clear_tags!` being incorrectly private.
+Version <dev> of the agent adds log-level filtering, an API to add custom attributes to logs, and updated instrumentation for Action Cable. It also provides fixes for how `Fiber` args are treated, Code-Level Metrics, and `NewRelic::Agent::Logging::DecoratingFormatter#clear_tags!` being incorrectly private.
 
 - **Feature: Filter forwarded logs based on level**
 
@@ -36,6 +36,10 @@ Version <dev> of the agent adds the ability to filter logs by level, an API to a
 
   As part of a refactor included in a previous release of the agent, the method `NewRelic::Agent::Logging::DecoratingFormatter#clear_tags!` was incorrectly made private. This method is now public again. Thanks to [@dark-panda](https://github.com/dark-panda) for reporting this issue. [PR#](https://github.com/newrelic/newrelic-ruby-agent/pull/2078)
 
+- **Bugfix: Fix the way args are handled for Fibers**
+
+  Previously, the agent treated Fiber args the same as it was treating Thread args, which is not correct. Args are passed to `Fiber#resume`, and not `Fiber.new`. This has been fixed, and the agent will properly preserve args for both Fiber and Thread classes. This also caused an error to occur when using Async 2.6.2, due to mismatching initalize definitions for Fiber prepended modules. This has been fixed as well. Thanks to [@travisbell](https://github.com/travisbell) for bringing this to our attention. [PR#2083](https://github.com/newrelic/newrelic-ruby-agent/pull/2083)
+
 ## v9.2.2
 
   Version 9.2.2 of the agent fixes a bug with the `Transaction#finished?` method.

lib/new_relic/agent/instrumentation/concurrent_ruby/chain.rb

@@ -13,7 +13,7 @@ def self.instrument!
         def post(*args, &task)
           return post_without_new_relic(*args, &task) unless NewRelic::Agent::Tracer.tracing_enabled?
 
-          traced_task = add_task_tracing(*args, &task)
+          traced_task = add_task_tracing(&task)
           post_without_new_relic(*args, &traced_task)
         end
       end

lib/new_relic/agent/instrumentation/concurrent_ruby/instrumentation.rb

@@ -7,11 +7,10 @@ module ConcurrentRuby
     SEGMENT_NAME = 'Concurrent/Task'
     SUPPORTABILITY_METRIC = 'Supportability/ConcurrentRuby/Invoked'
 
-    def add_task_tracing(*args, &task)
+    def add_task_tracing(&task)
       NewRelic::Agent.record_metric_once(SUPPORTABILITY_METRIC)
 
       NewRelic::Agent::Tracer.thread_block_with_current_transaction(
-        *args,
         segment_name: SEGMENT_NAME,
         parent: NewRelic::Agent::Tracer.current_segment,
         &task

lib/new_relic/agent/instrumentation/concurrent_ruby/prepend.rb

@@ -10,7 +10,7 @@ module Prepend
       def post(*args, &task)
         return super(*args, &task) unless NewRelic::Agent::Tracer.tracing_enabled?
 
-        traced_task = add_task_tracing(*args, &task)
+        traced_task = add_task_tracing(&task)
         super(*args, &traced_task)
       end
     end

lib/new_relic/agent/instrumentation/fiber/chain.rb

@@ -10,9 +10,16 @@ def self.instrument!
 
         alias_method(:initialize_without_new_relic, :initialize)
 
-        def initialize(*args, &block)
-          traced_block = add_thread_tracing(*args, &block)
-          initialize_with_newrelic_tracing { initialize_without_new_relic(*args, &traced_block) }
+        if RUBY_VERSION < '2.7.0'
+          def initialize(*_args, &block)
+            traced_block = add_thread_tracing(&block)
+            initialize_with_newrelic_tracing { initialize_without_new_relic(&traced_block) }
+          end
+        else
+          def initialize(**kwargs, &block)
+            traced_block = add_thread_tracing(&block)
+            initialize_with_newrelic_tracing { initialize_without_new_relic(**kwargs, &traced_block) }
+          end
         end
       end
     end

lib/new_relic/agent/instrumentation/fiber/instrumentation.rb

@@ -11,11 +11,10 @@ def initialize_with_newrelic_tracing
       yield
     end
 
-    def add_thread_tracing(*args, &block)
+    def add_thread_tracing(&block)
       return block if !NewRelic::Agent::Tracer.thread_tracing_enabled?
 
       NewRelic::Agent::Tracer.thread_block_with_current_transaction(
-        *args,
         segment_name: 'Ruby/Fiber',
         &block
       )

lib/new_relic/agent/instrumentation/fiber/prepend.rb

@@ -9,9 +9,16 @@ module MonitoredFiber
     module Prepend
       include NewRelic::Agent::Instrumentation::MonitoredFiber
 
-      def initialize(*args, &block)
-        traced_block = add_thread_tracing(*args, &block)
-        initialize_with_newrelic_tracing { super(*args, &traced_block) }
+      if RUBY_VERSION < '2.7.0'
+        def initialize(*_args, &block)
+          traced_block = add_thread_tracing(&block)
+          initialize_with_newrelic_tracing { super(&traced_block) }
+        end
+      else
+        def initialize(**kawrgs, &block)
+          traced_block = add_thread_tracing(&block)
+          initialize_with_newrelic_tracing { super(**kawrgs, &traced_block) }
+        end
       end
     end
   end

lib/new_relic/agent/instrumentation/thread/chain.rb

@@ -14,7 +14,7 @@ def self.instrument!
           alias_method(:initialize_without_new_relic, :initialize)
 
           def initialize(*args, &block)
-            traced_block = add_thread_tracing(*args, &block)
+            traced_block = add_thread_tracing(&block)
             initialize_with_newrelic_tracing { initialize_without_new_relic(*args, &traced_block) }
           end
         end

lib/new_relic/agent/instrumentation/thread/instrumentation.rb

@@ -17,7 +17,6 @@ def add_thread_tracing(*args, &block)
           return block if !NewRelic::Agent::Tracer.thread_tracing_enabled?
 
           NewRelic::Agent::Tracer.thread_block_with_current_transaction(
-            *args,
             segment_name: 'Ruby/Thread',
             &block
           )

lib/new_relic/agent/instrumentation/thread/prepend.rb

@@ -12,7 +12,7 @@ module Prepend
           include NewRelic::Agent::Instrumentation::MonitoredThread
 
           def initialize(*args, &block)
-            traced_block = add_thread_tracing(*args, &block)
+            traced_block = add_thread_tracing(&block)
             initialize_with_newrelic_tracing { super(*args, &traced_block) }
           end
         end

lib/new_relic/agent/tracer.rb

@@ -419,10 +419,10 @@ def thread_tracing_enabled?
           NewRelic::Agent.config[:'instrumentation.thread.tracing']
         end
 
-        def thread_block_with_current_transaction(*args, segment_name:, parent: nil, &block)
+        def thread_block_with_current_transaction(segment_name:, parent: nil, &block)
           parent ||= current_segment
           current_txn = ::Thread.current[:newrelic_tracer_state]&.current_transaction if ::Thread.current[:newrelic_tracer_state]&.is_execution_traced?
-          proc do
+          proc do |*args|
             begin
               if current_txn && !current_txn.finished?
                 NewRelic::Agent::Tracer.state.current_transaction = current_txn

lib/new_relic/traced_thread.rb

@@ -22,15 +22,14 @@ class TracedThread < Thread
     # @api public
     def initialize(*args, &block)
       NewRelic::Agent.record_api_supportability_metric(:traced_thread)
-      traced_block = create_traced_block(*args, &block)
+      traced_block = create_traced_block(&block)
       super(*args, &traced_block)
     end
 
-    def create_traced_block(*args, &block)
+    def create_traced_block(&block)
       return block if NewRelic::Agent.config[:'instrumentation.thread.tracing'] # if this is on, don't double trace
 
       NewRelic::Agent::Tracer.thread_block_with_current_transaction(
-        *args,
         segment_name: 'Ruby/TracedThread',
         &block
       )

test/multiverse/suites/thread/Envfile

@@ -4,4 +4,6 @@
 
 instrumentation_methods :chain, :prepend
 
-gemfile nil
+gemfile <<~RB
+  # this can't be empty or the tests won't run
+RB

test/multiverse/suites/thread/thread_fiber_instrumentation_test.rb

@@ -73,4 +73,16 @@ def test_parents_thread_fiber
   def test_parents_fiber_thread
     run_nested_parent_test(Fiber, Thread)
   end
+
+  def test_thread_instrumentation_args_preserved
+    Thread.new('arg 1', 2) do |arg1, arg2|
+      assert_equal ['arg 1', 2], [arg1, arg2]
+    end.join
+  end
+
+  def test_fiber_instrumentation_args_preserved
+    Fiber.new do |arg1, arg2|
+      assert_equal ['arg 1', 2], [arg1, arg2]
+    end.resume('arg 1', 2)
+  end
 end