Merge pull request #1658 from newrelic/bugfix/rails-frameworks-forced…

…-to-load-during-agent-initialization

Defer agent initialization in Rails until after :load_config_initializers

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## v8.14.0
 
-Version 8.14.0 of the agent restores desired Capistrano-based changelog lookup functionalty when a deployment is performed, delivers support for instrumenting Rails custom event notifications, and fixes potential compatibility issues with Redis gems, and reformats the Changelog to match Kayla's ghost linter.
+Version 8.14.0 of the agent restores desired Capistrano-based changelog lookup functionalty when a deployment is performed, delivers support for instrumenting Rails custom event notifications, and fixes potential compatibility issues with Redis gems, and fixes bugs related to initialization in Rails.
 
 - **Deployment Recipe: Restore desired Capistrano-based changelog lookup behavior**
 
@@ -16,9 +16,15 @@ Version 8.14.0 of the agent restores desired Capistrano-based changelog lookup f
 
   With version 8.13.0 of the agent, support was added for the [redis-rb](https://github.com/redis/redis-rb) gem v5+ and the new [RedisClient](https://rubygems.org/gems/redis-client) gem. With versions of RedisClient older than v0.11, the agent could cause the monitored application to crash when attempting to determine the Redis database index. Version 8.14.0 adds two related improvements. Firstly, support for RedisClient versions older than v0.11 has been added to get at the database index value. Secondly, the agent will no longer crash or impact the monitored application in the event that the database index cannot be obtained. Thank you very much to our community members [@mbsmartee](https://github.com/mbsmartee) and [@patatepartie](https://github.com/patatepartie) for bringing this issue to our attention, for helping us determine how to best reproduce it, and for testing out the update. We appreciate your help! [Issue#1650](https://github.com/newrelic/newrelic-ruby-agent/issues/1650) [PR#1673](https://github.com/newrelic/newrelic-ruby-agent/pull/1673)
 
-- **Test**
+- **Bugfix: Defer agent startup in Rails until after application-defined initializers have run**
 
-  This is a test entry to force the ghost linter to run. Remove this before release.
+  In Rails, the agent previously loaded before any application-defined initializers. This allowed initializers to reference the `add_method_tracer` API. However, this had the side-effect of forcing some framework libraries to load before initializers ran, preventing any configuration values related to these libraries from being applied. This fix provides an option to split initialization into two parts: load `add_method_tracer` before application-defined initializers and start the agent after application-defined initializers. This may cause other initializers to behave differently.
+
+  If you'd like to use this feature, set `defer_rails_initialization` to `true`. It is `false` by default, but may become `true` by default in a future release.
+
+  Furthermore, our Action View instrumentation was missing an `ActiveSupport.on_load` block around the code that loads our instrumentation.
+
+  Thank you [@jdelStrother](https://github.com/jdelStrother) for bringing this to our attention and collaborating with us on a fix. [PR#1658](https://github.com/newrelic/newrelic-ruby-agent/pull/1658)
 
 ## v8.13.1
 

lib/new_relic/agent/configuration/default_source.rb

@@ -1850,6 +1850,15 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil)
           :allowed_from_server => false,
           :description => 'Specify a custom host name for [display in the New Relic UI](/docs/apm/new-relic-apm/maintenance/add-rename-remove-hosts#display_name).'
         },
+        # Rails
+        :'defer_rails_initialization' => {
+          :default => false,
+          :public => true,
+          :type => Boolean,
+          :allowed_from_server => false,
+          :description => 'If `true`, when the agent is in an application using Ruby on Rails, it will start after ' \
+            'config/initializers are run.'
+        },
         # Rake
         :'rake.tasks' => {
           :default => [],

lib/new_relic/agent/instrumentation/rails_notifications/action_view.rb

@@ -22,7 +22,9 @@
   end
 
   executes do
-    NewRelic::Agent::Instrumentation::ActionViewSubscriber.subscribe(/render_.+\.action_view$/)
-    NewRelic::Agent::PrependSupportability.record_metrics_for(ActionView::Base, ActionView::Template, ActionView::Renderer)
+    ActiveSupport.on_load(:action_view) do
+      NewRelic::Agent::Instrumentation::ActionViewSubscriber.subscribe(/render_.+\.action_view$/)
+      NewRelic::Agent::PrependSupportability.record_metrics_for(ActionView::Base, ActionView::Template, ActionView::Renderer)
+    end
   end
 end

lib/new_relic/control/instance_methods.rb

@@ -64,8 +64,12 @@ def init_plugin(options = {})
 
         # An artifact of earlier implementation, we put both #add_method_tracer and #trace_execution
         # methods in the module methods.
-        Module.send(:include, NewRelic::Agent::MethodTracer::ClassMethods)
-        Module.send(:include, NewRelic::Agent::MethodTracer)
+        # Rails applications load the next two lines before any other initializers are run
+        unless defined?(Rails::VERSION) && NewRelic::Agent.config[:defer_rails_initialization]
+          Module.send(:include, NewRelic::Agent::MethodTracer::ClassMethods)
+          Module.send(:include, NewRelic::Agent::MethodTracer)
+        end
+
         init_config(options)
         NewRelic::Agent.agent = NewRelic::Agent::Agent.instance
         init_instrumentation

lib/newrelic_rpm.rb

@@ -20,8 +20,19 @@
 if defined?(Rails::VERSION)
   module NewRelic
     class Railtie < Rails::Railtie
-      initializer "newrelic_rpm.start_plugin", before: :load_config_initializers do |app|
-        NewRelic::Control.instance.init_plugin(config: app.config)
+      if NewRelic::Agent.config[:defer_rails_initialization]
+        initializer "newrelic_rpm.include_method_tracers", before: :load_config_initializers do |app|
+          Module.send(:include, NewRelic::Agent::MethodTracer::ClassMethods)
+          Module.send(:include, NewRelic::Agent::MethodTracer)
+        end
+
+        initializer "newrelic_rpm.start_plugin", after: :load_config_initializers do |app|
+          NewRelic::Control.instance.init_plugin(config: app.config)
+        end
+      else
+        initializer "newrelic_rpm.start_plugin", before: :load_config_initializers do |app|
+          NewRelic::Control.instance.init_plugin(config: app.config)
+        end
       end
     end
   end

test/README.md

@@ -28,9 +28,9 @@ In CI, these unit tests are run against all supported major.minor versions of Ra
 
 ### Running Specific Tests
 
-The file parameter can be added to the test:env invocation to run a specific unit file.  It can be exact file name, or a wildcard pattern.  Multiple file patterns can be specified by separating with a comma with no spaces surrounding:
+The file environment variable can be added to the test:env invocation to run a specific unit file.  It can be exact file name, or a wildcard pattern.  Multiple file patterns can be specified by separating with a comma with no spaces surrounding:
 
-    bundle exec rake 'test:env[rails60]' file=test/new_relic/agent/distributed_tracing/*  # everything in this folder
-    bundle exec rake 'test:env[rails60]' file=test/new_relic/agent/tracer_state_test.rb   # single file
-    bundle exec rake 'test:env[rails60]' file=test/new_relic/agent/*_test.rb              # all *_test.rb files in this folder
-    bundle exec rake 'test:env[rails60]' file=test/new_relic/agent/distributed_tracing/*,test/new_relic/agent/datastores/*  # all files in two folders
+    file=test/new_relic/agent/distributed_tracing/* bundle exec rake 'test:env[rails60]'  # everything in this folder
+    file=test/new_relic/agent/tracer_state_test.rb bundle exec rake 'test:env[rails60]'   # single file
+    file=test/new_relic/agent/*_test.rb  bundle exec rake 'test:env[rails60]'             # all *_test.rb files in this folder
+    file=test/new_relic/agent/distributed_tracing/*,test/new_relic/agent/datastores/* bundle exec rake 'test:env[rails60]' # all files in two folders

test/environments/rails40/config/initializers/test.rb

@@ -0,0 +1,20 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+require 'new_relic/agent/method_tracer'
+
+class Bloodhound < ActiveRecord::Base
+  include ::NewRelic::Agent::MethodTracer
+
+  def sniff
+    puts "When a bloodhound sniffs a scent article (a piece of clothing or item touched only by the subject), " \
+    "air rushes through its nasal cavity and chemical vapors — or odors — lodge in the mucus and bombard the dog's " \
+    "scent receptors. " \
+    "Source: https://www.pbs.org/wnet/nature/underdogs-the-bloodhounds-amazing-sense-of-smell/350/"
+  end
+
+  add_method_tracer :sniff
+end
+
+Rails.application.config.active_record.timestamped_migrations = false

test/environments/rails41/config/initializers/test.rb

@@ -0,0 +1,19 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+require 'new_relic/agent/method_tracer'
+
+class Bloodhound < ActiveRecord::Base
+  include ::NewRelic::Agent::MethodTracer
+
+  def sniff
+    puts "When a bloodhound sniffs a scent article (a piece of clothing or item touched only by the subject)," \
+    "air rushes through its nasal cavity and chemical vapors — or odors — lodge in the mucus and bombard the dog's scent receptors. " \
+    "Source: https://www.pbs.org/wnet/nature/underdogs-the-bloodhounds-amazing-sense-of-smell/350/"
+  end
+
+  add_method_tracer :sniff
+end
+
+Rails.application.config.active_record.timestamped_migrations = false

test/environments/rails42/config/initializers/test.rb

@@ -0,0 +1,20 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+require 'new_relic/agent/method_tracer'
+
+class Bloodhound < ActiveRecord::Base
+  include ::NewRelic::Agent::MethodTracer
+
+  def sniff
+    puts "When a bloodhound sniffs a scent article (a piece of clothing or item touched only by the subject), " \
+    "air rushes through its nasal cavity and chemical vapors — or odors — lodge in the mucus and bombard the dog's " \
+    "scent receptors. " \
+    "Source: https://www.pbs.org/wnet/nature/underdogs-the-bloodhounds-amazing-sense-of-smell/350/"
+  end
+
+  add_method_tracer :sniff
+end
+
+Rails.application.config.active_record.timestamped_migrations = false

test/environments/rails50/config/initializers/test.rb

@@ -0,0 +1,20 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+require 'new_relic/agent/method_tracer'
+
+class Bloodhound < ActiveRecord::Base
+  include ::NewRelic::Agent::MethodTracer
+
+  def sniff
+    puts "When a bloodhound sniffs a scent article (a piece of clothing or item touched only by the subject), " \
+    "air rushes through its nasal cavity and chemical vapors — or odors — lodge in the mucus and bombard the dog's " \
+    "scent receptors. " \
+    "Source: https://www.pbs.org/wnet/nature/underdogs-the-bloodhounds-amazing-sense-of-smell/350/"
+  end
+
+  add_method_tracer :sniff
+end
+
+Rails.application.config.active_record.timestamped_migrations = false

test/environments/rails51/config/initializers/test.rb

@@ -0,0 +1,20 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+require 'new_relic/agent/method_tracer'
+
+class Bloodhound < ActiveRecord::Base
+  include ::NewRelic::Agent::MethodTracer
+
+  def sniff
+    puts "When a bloodhound sniffs a scent article (a piece of clothing or item touched only by the subject), " \
+    "air rushes through its nasal cavity and chemical vapors — or odors — lodge in the mucus and bombard the dog's " \
+    "scent receptors. " \
+    "Source: https://www.pbs.org/wnet/nature/underdogs-the-bloodhounds-amazing-sense-of-smell/350/"
+  end
+
+  add_method_tracer :sniff
+end
+
+Rails.application.config.active_record.timestamped_migrations = false

test/environments/rails52/config/initializers/test.rb

@@ -0,0 +1,20 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+require 'new_relic/agent/method_tracer'
+
+class Bloodhound < ActiveRecord::Base
+  include ::NewRelic::Agent::MethodTracer
+
+  def sniff
+    puts "When a bloodhound sniffs a scent article (a piece of clothing or item touched only by the subject), " \
+    "air rushes through its nasal cavity and chemical vapors — or odors — lodge in the mucus and bombard the dog's " \
+    "scent receptors. " \
+    "Source: https://www.pbs.org/wnet/nature/underdogs-the-bloodhounds-amazing-sense-of-smell/350/"
+  end
+
+  add_method_tracer :sniff
+end
+
+Rails.application.config.active_record.timestamped_migrations = false

test/environments/rails60/config/initializers/test.rb

@@ -0,0 +1,20 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+require 'new_relic/agent/method_tracer'
+
+class Bloodhound < ActiveRecord::Base
+  include ::NewRelic::Agent::MethodTracer
+
+  def sniff
+    puts "When a bloodhound sniffs a scent article (a piece of clothing or item touched only by the subject), " \
+    "air rushes through its nasal cavity and chemical vapors — or odors — lodge in the mucus and bombard the dog's " \
+    "scent receptors. " \
+    "Source: https://www.pbs.org/wnet/nature/underdogs-the-bloodhounds-amazing-sense-of-smell/350/"
+  end
+
+  add_method_tracer :sniff
+end
+
+Rails.application.config.active_record.timestamped_migrations = false

test/environments/rails61/config/initializers/test.rb

@@ -0,0 +1,20 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+require 'new_relic/agent/method_tracer'
+
+class Bloodhound < ActiveRecord::Base
+  include ::NewRelic::Agent::MethodTracer
+
+  def sniff
+    puts "When a bloodhound sniffs a scent article (a piece of clothing or item touched only by the subject), " \
+    "air rushes through its nasal cavity and chemical vapors — or odors — lodge in the mucus and bombard the dog's " \
+    "scent receptors. " \
+    "Source: https://www.pbs.org/wnet/nature/underdogs-the-bloodhounds-amazing-sense-of-smell/350/"
+  end
+
+  add_method_tracer :sniff
+end
+
+Rails.application.config.active_record.timestamped_migrations = false

test/environments/rails70/config/initializers/test.rb

@@ -0,0 +1,20 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+require 'new_relic/agent/method_tracer'
+
+class Bloodhound < ActiveRecord::Base
+  include ::NewRelic::Agent::MethodTracer
+
+  def sniff
+    puts "When a bloodhound sniffs a scent article (a piece of clothing or item touched only by the subject), " \
+    "air rushes through its nasal cavity and chemical vapors — or odors — lodge in the mucus and bombard the dog's " \
+    "scent receptors. " \
+    "Source: https://www.pbs.org/wnet/nature/underdogs-the-bloodhounds-amazing-sense-of-smell/350/"
+  end
+
+  add_method_tracer :sniff
+end
+
+Rails.application.config.active_record.timestamped_migrations = false

test/environments/railsedge/config/initializers/test.rb

@@ -0,0 +1,20 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+require 'new_relic/agent/method_tracer'
+
+class Bloodhound < ActiveRecord::Base
+  include ::NewRelic::Agent::MethodTracer
+
+  def sniff
+    puts "When a bloodhound sniffs a scent article (a piece of clothing or item touched only by the subject), " \
+    "air rushes through its nasal cavity and chemical vapors — or odors — lodge in the mucus and bombard the dog's " \
+    "scent receptors. " \
+    "Source: https://www.pbs.org/wnet/nature/underdogs-the-bloodhounds-amazing-sense-of-smell/350/"
+  end
+
+  add_method_tracer :sniff
+end
+
+Rails.application.config.active_record.timestamped_migrations = false