igorkasyanchuk / rails_performance

Monitor performance of you Rails applications (self-hosted and free)
https://www.railsjazz.com/
MIT License
976 stars 53 forks source link

Fix initialization and make "enabled" setting work #23

Closed haffla closed 3 years ago

haffla commented 3 years ago

Hey!

I noticed that setting enabled to false actually doesn't work as expected. The problem is that when the engine code is executed the initializer hasn't even run yet, so RailsPerformance.enabled is always true.

Fixed it by delaying the middleware configuration using the "rails_performance.middleware" initializer.

Also replaced

haffla commented 3 years ago

Hello @igorkasyanchuk

Did you get a chance to have a look at this?

igorkasyanchuk commented 3 years ago

Hi @haffla , sorry not enogh time

So as I see https://github.com/igorkasyanchuk/rails_performance/blob/8c7f8a2cdda26992627c830e0db950d88298b27c/app/controllers/rails_performance/rails_performance_controller.rb#L7

this option is just disabling access to the admin panel.

And here: https://github.com/igorkasyanchuk/rails_performance/blob/8917f317e33930c74bbc08180104ee7fa634bb66/lib/rails_performance/engine.rb#L10

Have you created an initializer? Are you saying that this option if set to false in initialized later "true" in the code?

haffla commented 3 years ago

I am saying that testing for enabled inside of the Engine class (rails_performance/lib/rails_performance/engine.rb) does not work because when you start your Rails app, the Engine class is loaded before any of the Rails initializer files like config/initializers/rails_performance.rb are loaded. Just test it in your own app and set enabled to false. You will see that rails_performance is still collecting metrics because it is actually not disabled.

igorkasyanchuk commented 3 years ago

Ohh, I see. I thought I've tested and it was working. Good catch on this. Thanks

igorkasyanchuk commented 3 years ago

a new version was just released. Thanks for support