diff --git a/CHANGELOG.md b/CHANGELOG.md index dd49d50659..523e323a00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,15 @@ Community member [@fchatterji](https://github.com/fchatterji) helped standardize how we reference `NewRelic` throughout our codebase. Thanks fchatterji! [PR#1795](https://github.com/newrelic/newrelic-ruby-agent/pull/1795) +- **Bugfix: Allow rails initialization to be deferred by environment variable** + + The Ruby agent may force some Rails libraries to load on agent initialization, preventing some settings defined in `config/initializers` from being applied. Changing the initialization process to run after `config/initializers`, however, may break the configuration for other gems (ex. Roadie Rails). + + For those having troubles with agent initialization and Rails initializers, you can now pass the environment variable `NEW_RELIC_DEFER_RAILS_INITIALIZATION=true` to make the agent initialize after `config/initializers` are run. This config option can only be set using an environment variable and can't be set using YAML. + + Thanks to [@jdelStrother](https://github.com/jdelStrother) for bringing this issue to our attention and testing our fixes along the way. [Issue#662](https://github.com/newrelic/newrelic-ruby-agent/issues/662) [PR#1791](https://github.com/newrelic/newrelic-ruby-agent/pull/1791) + + ## 8.16.0 Version 8.16.0 introduces more Ruby on Rails instrumentation (especially for Rails 6 and 7) for various Action\*/Active\* libraries whose actions produce [Active Support notifications events](https://guides.rubyonrails.org/active_support_instrumentation.html). diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 24a9032864..1b0c6a5666 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -1614,12 +1614,17 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) # Rails :'defer_rails_initialization' => { :default => false, - :public => false, + :public => true, :type => Boolean, + :external => true, # this config is used directly from the ENV variables :allowed_from_server => false, - :description => 'This configuration option is currently unreachable. Please do not use. ' \ - 'If `true`, when the agent is in an application using Ruby on Rails, it will start after ' \ - 'config/initializers have run.' + :description => <<-DESCRIPTION + If `true`, when the agent is in an application using Ruby on Rails, it will start after `config/initializers` run. + + + This option may only be set by environment variable. + + DESCRIPTION }, # Rake :'rake.tasks' => { diff --git a/lib/new_relic/agent/instrumentation/net_http.rb b/lib/new_relic/agent/instrumentation/net_http.rb index 104bb72b5a..53e2f83fa8 100644 --- a/lib/new_relic/agent/instrumentation/net_http.rb +++ b/lib/new_relic/agent/instrumentation/net_http.rb @@ -27,7 +27,7 @@ end conflicts_with_prepend do - defined?(Rack::MiniProfiler) + defined?(Rack::MiniProfiler) && !defined?(Rack::MINI_PROFILER_PREPEND_NET_HTTP_PATCH) end conflicts_with_prepend do diff --git a/lib/new_relic/control/instance_methods.rb b/lib/new_relic/control/instance_methods.rb index bab3ef1032..1fe5738984 100644 --- a/lib/new_relic/control/instance_methods.rb +++ b/lib/new_relic/control/instance_methods.rb @@ -65,7 +65,7 @@ def init_plugin(options = {}) # An artifact of earlier implementation, we put both #add_method_tracer and #trace_execution # methods in the module methods. # Rails applications load the next two lines before any other initializers are run - unless defined?(Rails::VERSION) && NewRelic::Agent.config[:defer_rails_initialization] + unless defined?(Rails::VERSION) && ENV['NEW_RELIC_DEFER_RAILS_INITIALIZATION'] Module.send(:include, NewRelic::Agent::MethodTracer::ClassMethods) Module.send(:include, NewRelic::Agent::MethodTracer) end diff --git a/lib/newrelic_rpm.rb b/lib/newrelic_rpm.rb index c6917c2e93..67c472cd9a 100644 --- a/lib/newrelic_rpm.rb +++ b/lib/newrelic_rpm.rb @@ -20,7 +20,7 @@ if defined?(Rails::VERSION) module NewRelic class Railtie < Rails::Railtie - if NewRelic::Agent.config[:defer_rails_initialization] + if ENV['NEW_RELIC_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) diff --git a/newrelic.yml b/newrelic.yml index 5f5aabc31a..de12401750 100644 --- a/newrelic.yml +++ b/newrelic.yml @@ -162,10 +162,6 @@ common: &default_settings # or port_path_or_id parameters to transaction or slow SQL traces. # datastore_tracer.instance_reporting.enabled: true - # If true, when the agent is in an application using Ruby on Rails, it will start after - # config/initializers have run. - # defer_rails_initialization: false - # If true, disables Action Cable instrumentation. # disable_action_cable_instrumentation: false diff --git a/test/environments/rails60/config/initializers/test.rb b/test/environments/rails60/config/initializers/test.rb index 1d88e77b0a..d5d1a23d95 100644 --- a/test/environments/rails60/config/initializers/test.rb +++ b/test/environments/rails60/config/initializers/test.rb @@ -16,5 +16,6 @@ def sniff add_method_tracer :sniff end - +puts Rails.application.config.active_record.timestamped_migrations Rails.application.config.active_record.timestamped_migrations = false +puts Rails.application.config.active_record.timestamped_migrations diff --git a/test/new_relic/newrelic_rpm_test.rb b/test/new_relic/newrelic_rpm_test.rb index 4ea73ee1b7..abfac7b43f 100644 --- a/test/new_relic/newrelic_rpm_test.rb +++ b/test/new_relic/newrelic_rpm_test.rb @@ -5,81 +5,21 @@ require_relative '../test_helper' class NewRelicRpmTest < Minitest::Test + RAILS_32_SKIP_MESSAGE = 'MySQL error for Rails 3.2 when we add an initializer to config/initializers' # This test examines the behavior of an initializer when the agent is started in a Rails context # The initializer used for these tests is defined in test/environments/*/config/inititalizers/test.rb # We have documentation recommending customers call add_method_tracer # in their initializers, let's make sure this works - def test_add_method_tracer_in_initializer_gets_traced_when_agent_initialized_before_config_initializers - skip unless defined?(Rails::VERSION) - skip 'MySQL error for Rails 3.2' if Rails::VERSION::MAJOR == 3 - - with_config(defer_rails_initialization: false) do - assert Bloodhound.newrelic_method_exists?('sniff'), - 'Bloodhound#sniff not found by' \ - 'NewRelic::Agent::MethodTracer::ClassMethods::AddMethodTracer.newrelic_method_exists?' - assert Bloodhound.method_traced?('sniff'), - 'Bloodhound#sniff not found by' \ - 'NewRelic::Agent::MethodTracer::ClassMethods::AddMethodTracer.method_traced?' - end - end - def test_add_method_tracer_in_initializer_gets_traced_when_agent_initialized_after_config_initializers skip unless defined?(Rails::VERSION) - skip 'MySQL error for Rails 3.2 if we define an initializer in config/initializers' if Rails::VERSION::MAJOR == 3 - - with_config(defer_rails_initialization: true) do - assert Bloodhound.newrelic_method_exists?('sniff'), - 'Bloodhound#sniff not found by' \ - 'NewRelic::Agent::MethodTracer::ClassMethods::AddMethodTracer.newrelic_method_exists?' - assert Bloodhound.method_traced?('sniff'), - 'Bloodhound#sniff not found by' \ - 'NewRelic::Agent::MethodTracer::ClassMethods::AddMethodTracer.method_traced?' - end - end - - # All supported Rails versions have the default value for - # timestamped_migrations as true. Our initializer sets it to false. - # Test to resolve: https://github.com/newrelic/newrelic-ruby-agent/issues/662 - - def test_active_record_initializer_config_change_saved_when_agent_initialized_after_config_initializers - skip unless defined?(Rails::VERSION) - # TODO: This test passes in a Rails console in a playground app and in the customer's environment - # but fails in this unit test context - skip if Gem::Version.new(NewRelic::VERSION::STRING) >= Gem::Version.new('8.13.0') - skip 'MySQL error for Rails 3.2' if Rails::VERSION::MAJOR == 3 - - with_config(defer_rails_initialization: true) do - # Verify the configuration value was set to the initializer value - refute Rails.application.config.active_record.timestamped_migrations, - "Rails.application.config.active_record.timestamped_migrations equals true, expected false" - - # Verify the configuration value was applied to the ActiveRecord class variable - if Gem::Version.new(Rails::VERSION::STRING) >= Gem::Version.new('7.1') - refute ActiveRecord.timestamped_migrations, "ActiveRecord.timestamped_migrations equals true, expected false" - else - refute ActiveRecord::Base.timestamped_migrations, - "ActiveRecord::Base.timestamped_migrations equals true, expected false" - end - end - end - - def test_active_record_initializer_config_change_not_saved_when_agent_initialized_before_config_initializers - skip unless defined?(Rails::VERSION) - skip 'MySQL error for Rails 3.2' if Rails::VERSION::MAJOR == 3 - - with_config(defer_rails_initialization: false) do - # Verify the configuration value was set to the initializer value - refute Rails.application.config.active_record.timestamped_migrations, - "Rails.application.config.active_record.timestamped_migrations equals true, expected false" - - # Rails.application.config value should not be applied (this is the state of the original bug) - if Gem::Version.new(Rails::VERSION::STRING) >= Gem::Version.new('7.1') - assert ActiveRecord.timestamped_migrations, "ActiveRecord.timestamped_migrations equals false, expected true" - else - assert ActiveRecord::Base.timestamped_migrations, - "ActiveRecord::Base.timestamped_migrations equals false, expected true" - end - end + skip RAILS_32_SKIP_MESSAGE if Rails::VERSION::MAJOR == 3 + + assert Bloodhound.newrelic_method_exists?('sniff'), + 'Bloodhound#sniff not found by' \ + 'NewRelic::Agent::MethodTracer::ClassMethods::AddMethodTracer.newrelic_method_exists?' + assert Bloodhound.method_traced?('sniff'), + 'Bloodhound#sniff not found by' \ + 'NewRelic::Agent::MethodTracer::ClassMethods::AddMethodTracer.method_traced?' end end