Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable defer_rails_initialization config by environment variable only #1791

Merged
merged 10 commits into from
Feb 13, 2023
Merged

Enable defer_rails_initialization config by environment variable only #1791

merged 10 commits into from
Feb 13, 2023

Conversation

kaylareopelle
Copy link
Contributor

@kaylareopelle kaylareopelle commented Feb 3, 2023 •
edited by tannalynn
Loading

Configs are loaded during initialization. This fix was placed behind a feature flag referencing the config value, but that value will always be false at initialization, so the bug fix was unreachable.

Environment variables are accessible before initialization. The environment variable NEW_RELIC_DEFER_RAILS_INITIALIZATION may be set to true to enable this option.

Resolves #662 #1800

Todo:

  • Fix unit tests
  • Make sure base branch is correct
  • Add changelog entry:
**Bugfix: Allow rails initialization to be deferred by environment variable**

The Ruby agent may force some Rails frameworks 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)

kaylareopelle and others added 2 commits February 3, 2023 12:22
@kaylareopelle
Enable defer_rails_initialization config by environment variable only
8b75702 
Configs are loaded during initialization. This fix was placed behind a
feature flag referencing the config value, but that value will always
be falsey at initialization, so the bugfix was unreachable.

Environment variables are accessible before initialization.
The environment variable NEW_RELIC_DEFER_RAILS_INITIALIZATION may be
set to true to enable this option.
@tannalynn
Merge branch 'dev' into bugfix/rails-init-env-toggle
aa6e37c
@tannalynn tannalynn mentioned this pull request Feb 9, 2023
Closed
@tannalynn tannalynn changed the base branch from dev to major-release-9 February 13, 2023 17:53
tannalynn
tannalynn reviewed Feb 13, 2023
View reviewed changes
test/new_relic/newrelic_rpm_test.rb
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')
Copy link
Contributor

@tannalynn tannalynn Feb 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was removed because

  1. it has never worked and was always being skipped
  2. it is not possible for this test to work because the rails instrumentation is loaded prior to any tests running in the env tests, therefore configuration options that are used while loading instrumentation cannot be changed and tested in this way. This would have be changed to be tested in a multiverse situation.

fallwith
fallwith reviewed Feb 13, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@tannalynn @fallwith
Update CHANGELOG.md
319c909 
Co-authored-by: James Bunch <[email protected]>
@github-actions
Copy link
Contributor

SimpleCov Report

Coverage Threshold
Line 93.94% 93%
Branch 85.39% 84%

@tannalynn tannalynn merged commit b61e30b into major-release-9 Feb 13, 2023
This was referenced Feb 13, 2023
Closed
Open
@kaylareopelle kaylareopelle deleted the bugfix/rails-init-env-toggle branch February 22, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tannalynn tannalynn tannalynn left review comments

@fallwith fallwith fallwith approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

RPM forces Rails frameworks to load during initialization
3 participants
@kaylareopelle @fallwith @tannalynn