Initialize NewRelic before Rails' config-initializers #279
Conversation
Before, the initializer for NewRelic did not specify any ordering. In some situations, Rails will order its load_config_initializers before NewRelic's newrelic_rpm.start_plugin. This is a problem if NewRelic is called in a config-initializer (e.g. when defining method tracers) and will output the following warning: Agent unavailable as it hasn't been started. This change ensures that newrelic_rpm.start_plugin always gets loaded before config-intiializers are.
Multiverse::Suite#ordered_ruby_files depends on Dir.glob, which does not
guarantee order. Since TestWorker depends on SidekiqServer, the tests
will fail is the former is loaded before the latter.
>> Dir[File.join("test/multiverse/suites/sidekiq", '*.rb')]
=> ["test/multiverse/suites/sidekiq/test_worker.rb",
"test/multiverse/suites/sidekiq/sidekiq_server.rb",
"test/multiverse/suites/sidekiq/sidekiq_instrumentation_test.rb",
"test/multiverse/suites/sidekiq/after_suite.rb",
"test/multiverse/suites/sidekiq/test_model.rb"]
Adding an explicit require before TestWorker will circumvent this issue
while also declaring the explicit dependency.
|
The changes between 4b3c200 and cc9f74a don't seem to account for the new test failures (which might be related to issues on the Rubygems server). The failures are likely nondeterministic and could probably just be retried. ¯\(ツ)/¯ Hope this helps! |
|
Hi @mwear! Anything blocking this? Anything I can do to help? |
|
Hi @tonyta. Thanks for checking in. We've pulled this branch into our private repo and are currently discussing it. We are mostly in favor of the change as it makes the point at the which the agent is started better defined. There is a small amount of concern about changing things around agent startup, especially if there are users relying on some of the idiosyncrasies around how it is currently initialized. That's mainly the source of our discussions for now. We'll keep you updated on the status. Also, I wanted to give a brief run down of how we handle public PRs as it is likely a slightly different workflow from other projects you may have contributed to. We do not merge PRs directly to the public repo. We first pull them into our private repo, discuss the changes, and if we accept them, we'll merge it into our development branch on our private repo. At that point we would put an Thanks for your submission and we'll be in touch if we need anything. |
|
Hi @tonyta. Thanks for the contribution and the patience. This will be going into our next release. While we don't have an exact date for you, I'd expect to see it in the next couple of weeks. Thank you! |
|
Hi @tonyta. I wanted to let you know that your changes have been integrated into version 4.8.0 of the agent, which was released this morning. Thank you for your contribution! |
|
Thanks @mwear! 💛💚💙💜❤️ |
Before, the initializer for NewRelic did not specify any ordering. In some situations, Rails will order its
load_config_initializersbefore NewRelic'snewrelic_rpm.start_plugin. This is a problem if NewRelic is called in a config-initializer (e.g. when defining method tracers) and will output the following warning:This change ensures that
newrelic_rpm.start_pluginalways gets loaded before config-intiializers are.The problem is detailed and the fix is demonstrated here: tonyta/newrelic_warning#1
An unrelated dependency loading issue is fixed in cc9f74a to allow tests to pass.