Merge pull request #728 from newrelic/method_tracer_prepend

Refactor method tracers to use Module#prepend, no longer class_eval strings

CHANGELOG.md

@@ -17,6 +17,24 @@
   * **Remove Merb Support**
     This release removes the remaining support for the [Merb](https://weblog.rubyonrails.org/2008/12/23/merb-gets-merged-into-rails-3/) framework. It merged with Rails during the 3.0 release. Now that the Ruby agent supports Rails 3.2 and above, we thought it was time to say goodbye.
 
+  * **`add_method_tracer` refactored to use prepend over alias_method chaining**
+    This release overhauls the implementation of `add_method_tracer`, as detailed in [issue #502](https://github.com/newrelic/newrelic-ruby-agent/issues/502). The main breaking updates are as follows:
+    - A metric name passed to `add_method_tracer` will no longer be interpolated in an instance context as before. To maintain this behavior, pass a Proc object with the same arity as the method being traced. For example:
+
+        # OLD
+        add_method_tracer :foo, '#{args[0]}.#{args[1]}'
+
+        # NEW
+        add_method_tracer :foo, -> (*args) { "#{args[0].#{args[1]" }
+
+    - Similarly, the `:code_header` and `:code_footer` options to `add_method_tracer` will *only* accept a Proc object, which will be bound to the calling instance when the traced method is invoked.
+
+    - Calling `add_method_tracer` for a method will overwrite any previously defined tracers for that method. To specify multiple metric names for a single method tracer, pass them to `add_method_tracer` as an array.
+
+    See updated documentation on the following pages for full details:
+    https://docs.newrelic.com/docs/agents/ruby-agent/api-guides/ruby-custom-instrumentation/#method_tracers
+    https://rubydoc.info/github/newrelic/newrelic-ruby-agent/NewRelic/Agent/MethodTracer/ClassMethods#add_method_tracer-instance_method
+
   ## v7.2.0
 
   * **Expected Errors and Ignore Errors**

lib/new_relic/agent.rb

@@ -139,20 +139,20 @@ def logger=(log)
     # is initialized; these methods enable us to defer these calls
     # until we have started up and can process them.
     #
-    def add_or_defer_method_tracer(receiver, method_name, metric_name_code, options)
+    def add_or_defer_method_tracer(receiver, method_name, metric_name, options)
       @tracer_lock.synchronize do
         if @agent
-          receiver.send(:_add_method_tracer_now, method_name, metric_name_code, options)
+          receiver.send(:_nr_add_method_tracer_now, method_name, metric_name, options)
         else
-          @tracer_queue << [receiver, method_name, metric_name_code, options]
+          @tracer_queue << [receiver, method_name, metric_name, options]
         end
       end
     end
 
     def add_deferred_method_tracers_now
       @tracer_lock.synchronize do
-        @tracer_queue.each do |receiver, method_name, metric_name_code, options|
-          receiver.send(:_add_method_tracer_now, method_name, metric_name_code, options)
+        @tracer_queue.each do |receiver, method_name, metric_name, options|
+          receiver.send(:_nr_add_method_tracer_now, method_name, metric_name, options)
         end
 
         @tracer_queue = []

lib/new_relic/agent/instrumentation/active_merchant.rb

@@ -26,9 +26,9 @@ class ActiveMerchant::Billing::Gateway
         gateway_name = self.name.split('::').last
         [:authorize, :purchase, :credit, :void, :capture, :recurring, :store, :unstore, :update].each do |operation|
           if implemented_methods.include?(operation)
-            add_method_tracer operation, "ActiveMerchant/gateway/#{gateway_name}/#{operation}"
-            add_method_tracer operation, "ActiveMerchant/gateway/#{gateway_name}", :push_scope => false
-            add_method_tracer operation, "ActiveMerchant/operation/#{operation}", :push_scope => false
+            add_method_tracer operation, [-> (*) { "ActiveMerchant/gateway/#{gateway_name}/#{operation}" },
+                                          -> (*) { "ActiveMerchant/gateway/#{gateway_name}" },
+                                          -> (*) { "ActiveMerchant/operation/#{operation}" }]
           end
         end
       end

lib/new_relic/agent/instrumentation/rails/action_controller.rb

@@ -20,13 +20,13 @@
   executes do
     ActionView::PartialTemplate.class_eval do
       include NewRelic::Agent::MethodTracer
-      add_method_tracer :render, 'View/#{path_without_extension[%r{^(/.*/)?(.*)$},2]}.#{@view.template_format}.#{extension}/Partial'
+      add_method_tracer :render, -> (*) { "View/#{path_without_extension[%r{^(/.*/)?(.*)$},2]}.#{@view.template_format}.#{extension}/Partial" }
     end
 
     # this is for template rendering, as opposed to partial rendering.
     ActionView::Template.class_eval do
       include NewRelic::Agent::MethodTracer
-      add_method_tracer :render, 'View/#{(path_without_extension || @view.controller.newrelic_metric_path)[%r{^(/.*/)?(.*)$},2]}.#{@view.template_format}.#{extension}/Rendering'
+      add_method_tracer :render, -> (*) { "View/#{(path_without_extension || @view.controller.newrelic_metric_path)[%r{^(/.*/)?(.*)$},2]}.#{@view.template_format}.#{extension}/Rendering" }
     end
   end
 end
@@ -47,7 +47,7 @@
   executes do
     ActionController::Base.class_eval do
       include NewRelic::Agent::MethodTracer
-      add_method_tracer :render, 'View/#{newrelic_metric_path}/Rendering'
+      add_method_tracer :render, -> (*) { "View/#{newrelic_metric_path}/Rendering" }
     end
   end
 end
@@ -68,12 +68,12 @@
   executes do
     ActionView::RenderablePartial.module_eval do
       include NewRelic::Agent::MethodTracer
-      add_method_tracer :render_partial, 'View/#{path[%r{^(/.*/)?(.*)$},2]}/Partial'
+      add_method_tracer :render_partial, -> (*) { "View/#{path[%r{^(/.*/)?(.*)$},2]}/Partial" }
     end
 
     ActionView::Template.class_eval do
       include NewRelic::Agent::MethodTracer
-      add_method_tracer :render, 'View/#{path[%r{^(/.*/)?(.*)$},2]}/Rendering'
+      add_method_tracer :render, -> (*) { "View/#{path[%r{^(/.*/)?(.*)$},2]}/Rendering" }
     end
   end
 end

lib/new_relic/agent/instrumentation/rails/action_web_service.rb

@@ -38,7 +38,7 @@
     if defined?(ActionController) && defined?(ActionController::Base)
       ActionController::Base.class_eval do
         if method_defined? :perform_invocation
-          add_method_tracer :perform_invocation, 'WebService/#{controller_name}/#{args.first}'
+          add_method_tracer :perform_invocation, -> (*args) { "WebService/#{controller_name}/#{args.first}" }
         end
       end
     end