split init_plugin into load/start (#662)

CHANGELOG.md

@@ -10,6 +10,15 @@
 
     Integers not wrapped in quotation marks can be passed to `error_collector.ignore_status_codes` in the `newrelic.yml` file. Our thanks goes to @elaguerta and @brammerl for resolving this issue!
 
+  * **Bugfix: initiate agent start after Rails config initializers are run**
+
+    Previously `NewRelic::Control.instance.init_plugin` installed instrumentation prior to Rails'
+    `config/initializers/*` files being loaded. This caused ActiveRecord, ActiveJob and ActionView to
+    fire their lazy-load hooks prematurely, which resulted in config settings made in initializers for
+    those components to be ignored.
+
+    The agent now defers installing instrumentation until after `load_config_initializers`. Thank you
+    @jdelStrother for reporting this issue!
 
   ## v8.0.0
 

lib/new_relic/control/instance_methods.rb

@@ -48,9 +48,14 @@ module InstanceMethods
       # init_config({}) which is called one or more times.
       #
       def init_plugin(options={})
+        setup_agent(options)
+        start_agent
+      end
+
+      def setup_agent(options)
         env = determine_env(options)
 
-        configure_agent(env, options)
+        load_configs(env, options)
 
         # Be sure to only create once! RUBY-1020
         if ::NewRelic::Agent.logger.is_startup_logger?
@@ -68,11 +73,33 @@ def init_plugin(options={})
         Module.send :include, NewRelic::Agent::MethodTracer
         init_config(options)
         NewRelic::Agent.agent = NewRelic::Agent::Agent.instance
+      end
+
+      def load_configs(env, options)
+        manual = Agent::Configuration::ManualSource.new(options)
+        Agent.config.replace_or_add_config(manual)
+
+        config_file_path = @config_file_override || Agent.config[:config_path]
+        yaml_source = Agent::Configuration::YamlSource.new(config_file_path, env)
+        if yaml_source.failed?
+          yaml_source.failures.each do |msg|
+            stdout.puts Agent::AgentLogger.format_fatal_error(msg)
+          end
+        end
+        Agent.config.replace_or_add_config(yaml_source)
 
+        if security_settings_valid? && Agent.config[:high_security]
+          Agent.logger.info("Installing high security configuration based on local configuration")
+          Agent.config.replace_or_add_config(Agent::Configuration::HighSecuritySource.new(Agent.config))
+        end
+      end
+
+      def start_agent
         if !security_settings_valid?
           handle_invalid_security_settings
         elsif Agent.config[:agent_enabled] && !NewRelic::Agent.instance.started?
-          start_agent
+          @started_in_env = self.env
+          NewRelic::Agent.agent.start
           install_instrumentation
         elsif !Agent.config[:agent_enabled]
           install_shim
@@ -96,25 +123,6 @@ def determine_env(options)
         env
       end
 
-      def configure_agent(env, options)
-        manual = Agent::Configuration::ManualSource.new(options)
-        Agent.config.replace_or_add_config(manual)
-
-        config_file_path = @config_file_override || Agent.config[:config_path]
-        yaml_source = Agent::Configuration::YamlSource.new(config_file_path, env)
-        if yaml_source.failed?
-          yaml_source.failures.each do |msg|
-            stdout.puts Agent::AgentLogger.format_fatal_error(msg)
-          end
-        end
-        Agent.config.replace_or_add_config(yaml_source)
-
-        if security_settings_valid? && Agent.config[:high_security]
-          Agent.logger.info("Installing high security configuration based on local configuration")
-          Agent.config.replace_or_add_config(Agent::Configuration::HighSecuritySource.new(Agent.config))
-        end
-      end
-
       def security_settings_valid?
         !Agent.config[:high_security] ||
           Agent.config[:security_policies_token].empty?
@@ -129,12 +137,6 @@ def handle_invalid_security_settings
         install_shim
       end
 
-      # Install the real agent into the Agent module, and issue the start command.
-      def start_agent
-        @started_in_env = self.env
-        NewRelic::Agent.agent.start
-      end
-
       def app
         Agent.config[:framework]
       end

lib/newrelic_rpm.rb

@@ -20,8 +20,12 @@
 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)
+      initializer "newrelic_rpm.load_plugin", before: :load_config_initializers do |app|
+        NewRelic::Control.instance.setup_agent(config: app.config)
+      end
+
+      initializer "newrelic_rpm.start_plugin", after: :load_config_initializers do |app|
+        NewRelic::Control.instance.start_agent
       end
     end
   end

test/new_relic/control/instance_methods_test.rb

@@ -19,43 +19,43 @@ def setup
     @test = ::TestClass.new(nil)
   end
 
-  def test_configure_agent_adds_the_yaml_config
+  def test_load_configs_adds_the_yaml_config
     refute_has_config NewRelic::Agent::Configuration::YamlSource
-    @test.configure_agent('test', {})
+    @test.load_configs('test', {})
     assert_has_config NewRelic::Agent::Configuration::YamlSource
   end
 
-  def test_configure_agent_adds_the_manual_config
+  def test_load_configs_adds_the_manual_config
     refute_has_config NewRelic::Agent::Configuration::ManualSource
-    @test.configure_agent('test', {})
+    @test.load_configs('test', {})
     assert_has_config NewRelic::Agent::Configuration::ManualSource
   end
 
   def test_no_high_security_config_by_default
     refute_has_config NewRelic::Agent::Configuration::HighSecuritySource
-    @test.configure_agent('test', {:high_security => false})
+    @test.load_configs('test', {:high_security => false})
     refute_has_config NewRelic::Agent::Configuration::HighSecuritySource
   end
 
   def test_high_security_config_added_if_requested
     refute_has_config NewRelic::Agent::Configuration::HighSecuritySource
-    @test.configure_agent('test', {:high_security => true})
+    @test.load_configs('test', {:high_security => true})
     assert_has_config NewRelic::Agent::Configuration::HighSecuritySource
   end
 
-  def test_configure_agent_yaml_parse_error_logs_to_stdout
+  def test_load_configs_yaml_parse_error_logs_to_stdout
     NewRelic::Agent::Configuration::YamlSource.any_instance.stubs(:failed?).returns(true)
     NewRelic::Agent::Configuration::YamlSource.any_instance.stubs(:failures).returns(['failure'])
-    @test.configure_agent('invalid', {})
+    @test.load_configs('invalid', {})
     assert_equal "** [NewRelic] FATAL : failure\n", @test.stdout.string
   end
 
-  def test_configure_agent_invalid_yaml_value_logs_to_stdout
+  def test_load_configs_invalid_yaml_value_logs_to_stdout
     config_path = File.expand_path(File.join(
       File.dirname(__FILE__),
       '..','..', 'config','newrelic.yml')
     )
-    @test.configure_agent('invalid', {:config_path => config_path})
+    @test.load_configs('invalid', {:config_path => config_path})
     assert NewRelic::Agent.config.instance_variable_get(:@yaml_source).failed?
     expected_err = "** [NewRelic] FATAL : Unexpected value (cultured groats) for 'enabled' in #{config_path}\n"
     assert_equal expected_err, @test.stdout.string