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.

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

rails: install all performance hooks by default #1112

Merged
merged 2 commits into from
Aug 4, 2020

Conversation

kyrylo
Copy link
Contributor

@kyrylo kyrylo commented Aug 3, 2020

Previously, we were installing performance hooks based on the value from the
Airbrake config. If performance_stats is true, then the hooks will be
installed during initialization, if it's false, the hooks would never be
installed.

Now, airbrake-ruby supports remote configuration, which can change the value
of performance_stats at runtime. In order to support this feature we must
change the approach and install the hooks in either case. The
performance_stats check is now performed inside the hooks, since this will
allow us to enable/disable the feature during the runtime.

A pleasant effect of this change is that we no longer need to read the config
inside our railtie. This fixes #1105, which reports an issue with our existing
code where we hook into after: :load_config_initializers. With this new
approach we don't need to hook into that event anymore because we don't care at
what stage the config/initializers/airbrake.rb is loaded.

@kyrylo kyrylo force-pushed the performance-stats-notifier-config-support branch 3 times, most recently from 4528080 to e2b3746 Compare August 4, 2020 02:34
There was a mismatch between the real name and the label.
@kyrylo kyrylo force-pushed the performance-stats-notifier-config-support branch 6 times, most recently from 18ac61f to b39243f Compare August 4, 2020 07:46
Previously, we were installing performance hooks based on the value from the
Airbrake config. If `performance_stats` is `true`, then the hooks will be
installed during initialization, if it's `false`, the hooks would never be
installed.

Now, `airbrake-ruby` supports remote configuration, which can change the value
of `performance_stats` at runtime. In order to support this feature we must
change the approach and install the hooks in either case. The
`performance_stats` check is now performed inside the hooks, since this will
allow us to enable/disable the feature during the runtime.

A pleasant effect of this change is that we no longer need to read the config
inside our railtie. This fixes #1105, which reports an issue with our existing
code where we hook into `after: :load_config_initializers`. With this new
approach we don't need to hook into that event anymore because we don't care at
what stage the `config/initializers/airbrake.rb` is loaded.
@kyrylo kyrylo force-pushed the performance-stats-notifier-config-support branch from b39243f to 681cf87 Compare August 4, 2020 08:23
@kyrylo kyrylo merged commit 2b26317 into master Aug 4, 2020
@kyrylo kyrylo deleted the performance-stats-notifier-config-support branch August 4, 2020 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App fails to initialize with 10.0.1+
1 participant