hschne / rails-mini-profiler

Performance profiling for Rails, made simple 🦔
MIT License
411 stars 22 forks source link

GET requestes are not sabved into the respective table #234

Closed oscarlaf03 closed 3 months ago

oscarlaf03 commented 4 months ago

Found a bug? Please fill out the sections below! 👍

Issue Summary

All the POST, OPTIONS and other requests are shown are as expected on the /rails_mini_profiler page, but every GET requested is missing due to the fact it cannot get added to the rmp_profiled_request table

I get the following error message:

E, [2024-05-29T14:32:10.392477 #58380] ERROR -- : [RailsMiniProfiler] Could not save profile: Write query attempted while in readonly mode: INSERT INTO `rmp_profiled_request` [.....]

Steps to Reproduce

  1. Start a new Rails project with Rails Mini Profiler
  2. issue several GET requests
  3. realize GET requests are ignored from the rails_mini_profiler UI

Technical details

on my config/initializers/rails_mini_profiler.rb file


# Rails Mini Profiler Initializer (0.7.3)

# Customize to your hearts content. If you remove this file, Rails Mini Profiler will use sensible defaults.
# For more information see https://github.com/hschne/rails-mini-profiler#configuration
RailsMiniProfiler.configure do |config|
  # Customize when Rails Mini Profiler should run
  config.enabled = proc { |env| Rails.env.development? || env['HTTP_RMP_ENABLED'].present? }

  # Configure Flamegraph generation
  config.flamegraph_enabled = true
  # config.flamegraph_sample_rate = 0.5

  # Configure endpoints to profile
  config.skip_paths = []

  # Configure how Rails Mini Profiler stores profiling information
  # config.storage.database = :rmp_database
  config.storage.profiled_requests_table = :rmp_profiled_request
  config.storage.traces_table = :rmp_traces
  config.storage.flamegraphs_table = :rmp_flamegraphs

  # Configure the Rails Mini Profiler User Interface
  # config.ui.badge_enabled = true
  # config.ui.badge_position = 'top-left'
  # config.ui.base_controller = ApplicationController
  # config.ui.page_size = 25
  # config.ui.webpacker_enabled = true

  # Customize how users are detected
  config.user_provider = proc { |env| Rack::Request.new(env).ip }
end
hschne commented 4 months ago

Thanks for opening this issue!

My first instinct says that this could be related to a particular database configuration. The SO thread you linked seems to point in the same direction. Could you share your respective config files?

oscarlaf03 commented 3 months ago

Hi @hschne as far as I could tell is not a database config issue but rather one of the Rails 7 expected behaviors: to prevent writing to the database while receiving a GET request to enforce Restful as per here: https://github.com/rails/rails/blob/ea757ae7023fd0f89b6c086831a6f34c80f61f1c/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb#L230

the way I got around it was through a monkey patch on my development.rb file

module ActiveRecord
  module ConnectionAdapters
    class AbstractAdapter
      def preventing_writes?
        false 
      end
    end
  end
end

This is not as elegant as I would like I also encapsulated that further under and environment variable to be extra safe and only run RMP as optional under flag ie PROFILER=truer bundle exec rails server

But yeah I would like to have been available as a config variable on rails

Have you seen this type of issue while implementing RMP on any Rails 7 or higher app?

hschne commented 3 months ago

@oscarlaf03 Unfortunately, I failed to reproduce this behaviour with a new application, using Rails 7.1, though.

Here's my baseline and steps for trying to reproduce (using development). I also tried using PedantMysQLAdapter as well as SQlite, with the same results.

  1. Create a new Rails app with a similar setup to yours.

    rails -v
    Rails 7.1.3.4
    
    rails new Testing -d mysql 
    bundle add rails_mini_profiler
    rails rails_mini_profiler:install
  2. Generate some traffic, verifying that GET requests work as expected. I'm not getting any errors.

Screen Shot 2024-06-21 at 19 15 13

As you already found a workaround I'll close this. I'd love to dig deeper into this, so if you find a way to consistently reproduce this issue with a new app let me know :)