Skip to content

Commit

Permalink
Add config to determine Rails initialization order
Browse files Browse the repository at this point in the history
Starting the agent after :load_config_initializers prevents the dynamic
initializer for roadie-rails from running, making the gem moot. The customer
who used the bugfix also had an error pop up with one of their
initializers. Though this change solves the problem with configs getting
applied, it may interact unpredictably with existing initializers.
We won't turn on this behavior by default until a major version release.
  • Loading branch information
kaylareopelle committed Dec 7, 2022
1 parent bbd78a1 commit 1e989f1
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 24 deletions.
9 changes: 9 additions & 0 deletions lib/new_relic/agent/configuration/default_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 => true,
: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 => [],
Expand Down
2 changes: 1 addition & 1 deletion lib/new_relic/control/instance_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
unless defined?(Rails::VERSION) && NewRelic::Agent.config[:defer_rails_initialization]
Module.send(:include, NewRelic::Agent::MethodTracer::ClassMethods)
Module.send(:include, NewRelic::Agent::MethodTracer)
end
Expand Down
18 changes: 12 additions & 6 deletions lib/newrelic_rpm.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,19 @@
if defined?(Rails::VERSION)
module NewRelic
class Railtie < Rails::Railtie
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
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)
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
Expand Down
72 changes: 55 additions & 17 deletions test/new_relic/newrelic_rpm_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,38 +10,76 @@ class NewRelicRpmTest < Minitest::Test

# 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
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

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?'
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

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

# 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"
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"

# 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"
# 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
refute ActiveRecord::Base.timestamped_migrations,
"ActiveRecord::Base.timestamped_migrations equals false, expected true"
end
end
end
end

0 comments on commit 1e989f1

Please sign in to comment.