charkost / prosopite

:mag: Rails N+1 queries auto-detection with zero false positives / false negatives
Apache License 2.0
1.48k stars 45 forks source link

FEATURE: Make output more configurable #46

Open bf4 opened 2 years ago

bf4 commented 2 years ago

Feature: I want to be able to report a summary of the found issues.

For example, here's my code I've hacked right now. Ideally I could do this via some post-processor callback prior to logger.warn which let's be post-process the generated message.

class TestLogger < ActiveSupport::Logger
  def initialize
    @file = StringIO.new
    super(@file)
  end

  def messages
    @file.rewind
    @file.read
  end
end

and in the test env

  require Rails.root.join("lib", "test_logger")
  prosopite_logger = Class.new(TestLogger) do
    # Shrink down size of notification.
    def warn(progname, &block)
      progname = extract_interesting_lines(progname)
      super
    end

    def extract_interesting_lines(progname)
      lines = progname.split("\n")
      interesting_traces = {}
      lines.each_with_index do |call_stack_line, index|
        next unless /Call stack/i.match?(call_stack_line)
        interesting_lines = lines[index - 1..index + 1]
        interesting_lines.each do |interesting_line|
          interesting_line.gsub!(/#{Rails.root.to_path}\//, "")
        end
        interesting_traces[interesting_lines.last] = interesting_lines.join("\n")
      end
      interesting_traces
        .to_a
        .sort_by(&:first)
        .map(&:last)
        .join("\n\n")
    end
    # def add(severity, message = nil, progname = nil, &block) # :nodoc:
    # end
  end.new
  Rails.application.configure do
    config.after_initialize do
      Prosopite.custom_logger = prosopite_logger
      # TODO(BF): When all the warnings are gone
      # Prosopite.rails_logger = true
      # Prosopite.raise = true
    end
  end
  RSpec.configure do |config|
    config.before(:each) do
      Prosopite.scan
    end

    config.after(:each) do
      Prosopite.finish
    end

    config.after(:suite) do
      messages = prosopite_logger.messages
      next if messages.blank?
      message = <<~EOS
        ---------------- N+1 WARNINGS -------------------
        #{messages}
        ---------------------------------------------
      EOS
      if ENV["LOG"]
        ::Rails.logger.info message
      else
        $stderr.puts message # rubocop:disable Style/StderrPuts
        if !ENV["CI"]
          File.write(Rails.root.join("log", "prosopite_test.log"), message)
        end
      end
    end
  end
technicalpickles commented 1 year ago

I think this can be implemented with the (recently merged) Prosopite.backtrace_cleaner (https://github.com/charkost/prosopite/pull/54)

I also suspect that now we are using Rails.backtrace_cleaner, you may not even need to add your custom logic for cleaning the output.

bf4 commented 1 year ago

Thanks @technicalpickles for your đŸ‘€ Would you accept a PR for to back-out from requiring the entire rails meta gem and just railties (which defines Rails, which I think is what you wanted?). Our Rails app doesn't require "rails" so installing the change would cause us to install a bunch of stuff we don't currently use.

technicalpickles commented 1 year ago

@bf4 there isn't a runtime dependency on rails, just a development dependency required in test/test_helper.rb

Are you seeing errors with the latest release?