Skip to content
This repository has been archived by the owner on Sep 27, 2024. It is now read-only.
This repository has been archived by the owner on Sep 27, 2024. It is now read-only.

Add plugin system to allow users to add their own tracing #96

Open
@hschne

Description

Right now, which traces are profiled, and how they are rendered, is hardcoded.

Add some sort of plugin architecture that allows users to add custom profilers

Activity

created this issue from a note in Backlog on Nov 13, 2021
topherfangio

topherfangio commented on Jun 17, 2024

@topherfangio

On this note, it would be great if we could profile ViewComponents! I would be interested in taking a stab at this PR if you could provide some guidance on your thoughts for a plugin system.

hschne

hschne commented on Jun 21, 2024

@hschne
OwnerAuthor

Hey @topherfangio, that's a great idea. I'd love to see some support for ViewComponents (or other view libraries such as Phlex), and I'll take any help I can get 😁

Now, as you've probably seen I haven't actively worked on RMP for a while, but this seems like a good opportunity. As to guidelines, I'll do my best to recollect what I was thinking when I created this issue originally.


'Plugins' in RMP are basically what Tracers are now. Users can (theoretically even now) create new Tracer classes and register them with Registry (see also Configuration for how built-in tracers are registered). Tracers specify which ActiveSupport events RMP subscribes to and which data from those events gets stored in the database. They also specify a TracePresenter class that governs how the event will be rendered in the UI (see, e.g., SequelTracePresenter).

So, in theory (I haven't tested any of this) you could already create a plugin with the existing code by adding something like this to your app:

class CustomTracer < RailsMiniProfiler::Tracers::Tracer 
   class << self
      def subscribes_to
        `render.viewcomponent`
      end

      def presents
        ViewComponentTracePresenter
      end
   end
end

class ViewComponentTracePresenter < RailsMiniProfiler::Presenters::TracePresenter
  def label 
   'ViewComponentTrace'
   end
end

# RMP Config
RailsMiniProfiler.configure do |config|
  # Customize when Rails Mini Profiler should run
  config.tracers = %i[controller instantiation sequel view rmp view_component] 
end 

Luckily, ViewComponent already provides instrumentation; for other libraries, RMP may have to add instrumentation itself.


Apart from checking if what I wrote above actually works there are two things that need doing (that I can think of on top of my head):

  1. The UI allows filtering traces by type, and there currently is no mechanism to add new tracers/events there (see _trace_list_header.erb). Shouldn't bee to rough adding this though.
            <% trace_names = %w[process_action.action_controller sql.active_record instantiation.active_record render_template.action_view render_partial.action_view] %>
  2. The whole interface of Tracers and registries and stuff isn't user friendly I feel. Might require some naming changes, even if its just aliasing some classes. I'm very open to suggestions! 😄

Let me know your thoughts! I think it's seriously awesome if you were to take a stab at this, be happy to support in any way I can 🙇

topherfangio

topherfangio commented on Jul 8, 2024

@topherfangio

@hschne Thanks! I've been on vacation but will take a look at this this week and see how far I can get!

topherfangio

topherfangio commented on Jul 11, 2024

@topherfangio

@hschne Ok, so I'm running into an issue because the RailsMiniProfiler::TraceProvider isn't defined inside lib, it's inside the app directory, so it can't find it in the initializer.

I tried requiring the file, but it needs the BasePresenter as well.

Here is my current / non-working code. Would love some thoughts on how to address this!

I guess the alternative would be to create a PR into RMP so that it's all in the right place?

class ViewComponentTracer < RailsMiniProfiler::Tracers::Tracer
  class << self
    def subscribes_to
      `render.view_component`
    end

    def presents
      ViewComponentTracePresenter
    end
  end
end

class ViewComponentTracePresenter < RailsMiniProfiler::TracePresenter
  def label
    'ViewComponentTrace'
  end
end
topherfangio

topherfangio commented on Jul 19, 2024

@topherfangio

Ok, slight update. I got the code to load properly, and I have verified that the Registry contains my new tracer! Yay!

Unfortunately...it doesn't seem to appear in the UI. Specifically this page:

Screenshot 2024-07-19 at 9 32 31 AM

Here's my current code.

config/initializers/rails_mini_profiler_extensions.rb

# frozen_string_literal: true
Rails.configuration.to_prepare do

  class RailsMiniProfiler::ViewComponentTracePresenter < RailsMiniProfiler::TracePresenter
    def label
      'ViewComponentTrace'
    end
  end

  # This MUST start with RailsMiniProfiler::Tracers because of the way that it is loaded
  class RailsMiniProfiler::Tracers::ViewComponentTracer < RailsMiniProfiler::Tracers::Tracer
    class << self
      def subscribes_to
        asdf
        'render.view_component'
      end

      def presents
        RailsMiniProfiler::ViewComponentTracePresenter
      end
    end
  end

end

config/initializers/rails_mini_profiler.rb

  # Add our custom ViewComponent tracer
  config.tracers += ['view_component']

Any thoughts on what I'm missing? Thanks!

hschne

hschne commented on Jul 20, 2024

@hschne
OwnerAuthor

@topherfangio Awesome, congratulations on your progress!

On first glance, I'm not sure what's up there; I would need to take an in-depth look. Generally (since you asked) you're welcome to open any and all PRs that you feel might be helpful. In this case I'd love if you could open a draft PR, makes it easier for me to check out what's going on and help debugging :)

Some ideas for figuring this one out:

  • Are traces for ViewComponent stored in the DB? Check the Trace table to see if it's just an issue with rendering.
  • If no traces for 'render.view_component can be found in the DB then your best bet would be to set a breakpoint in the middleware class and see if the event is triggered, and if so, if the correct tracer is loaded. E.g. here:
        def track_trace(event)
          return if traces.nil?
    
          trace = @trace_factory.create(event)
          # Check if breakpoint triggers on render.view_component and if correct trace is created
          binding.pry
          traces.append(trace) unless trace.is_a?(RailsMiniProfiler::Tracers::NullTrace)
        end
  • You have enabled ViewComponent instrumentation, right? Just making sure 😆
    config.view_component.instrumentation_enabled = true
    
topherfangio

topherfangio commented on Jul 22, 2024

@topherfangio

@hschne Finally got it!

Screenshot 2024-07-22 at 12 54 58 PM

Turned out to be a timing issue on when I was defining the tracer class. It needed to be done in the before_configuration hook so that the RMP code could pick it up before the middleware loaded.

Here is what finally worked:

config/environments/development.rb

  config.view_component.instrumentation_enabled = true
  config.view_component.use_deprecated_instrumentation_name = false

config/initializers/rails_mini_profiler.rb

  # ... normal stuff above

  # Add our custom ViewComponent tracer
  config.tracers += [:view_component]

config/initializers/rails_mini_profiler_extensions.rb

# frozen_string_literal: true
Rails.configuration.before_configuration do

  # This MUST start with RailsMiniProfiler::Tracers because of the way that it is loaded
  class RailsMiniProfiler::Tracers::ViewComponentTracer < RailsMiniProfiler::Tracers::Tracer
    class << self
      def subscribes_to
        'render.view_component'
      end

      def presents
        RailsMiniProfiler::ViewComponentTracePresenter
      end
    end
  end
end

Rails.configuration.to_prepare do

  class RailsMiniProfiler::ViewComponentTracePresenter < RailsMiniProfiler::TracePresenter
    def label
      payload["name"]
    end

    def content
      "<pre style='margin-bottom: 16px'>#{payload["identifier"]}</pre>".html_safe
    end

    def backtrace
      model.backtrace.join("\n")
    end
  end

  # Turn off logging of RailsMiniProfiler SQL queries
  module LogSubscriberRMPSilencer
    def sql(event)
      unless event.payload[:name] =~ /RailsMiniProfiler/
        super
      end
    end
  end

  module ActiveRecord
    class LogSubscriber
      prepend LogSubscriberRMPSilencer
    end
  end

end

A couple of notes:

  1. You can see I added a way to silence the SQL Logs for RMP. They were HUGE and made it difficult to look through the output. You definitely don't need that to get the ViewComponent stuff working, I just found it helpful during debugging.
  2. You'll notice my comment about having to prefix my class with RailsMiniProfiler::Tracers:: in order to get it working. This is probably an area where a real plugin system could help.
    constant = "RailsMiniProfiler::Tracers::#{tracer}".safe_constantize
  3. I think the naming is decent as-is, but since we're using TracePresenter, I wonder if TraceProvider might make a bit more sense than Tracer?
  4. As far as a plugin system, I'm thinking something along the lines of the below for a possible API?

Possible Plugin API

  config.add_tracer(:view_component, 'render.view_component', provider: MyProviderClass, presenter: MyPresenterClass)
  
  # OR since add_tracer is already used in the code

  config.plugin_tracer(:view_component, 'render.view_component', provider: MyProviderClass, presenter: MyPresenterClass)

The only other thing I'm thinking that might help with a plugin system is having the classes already defined / available in the Initializer rather than having to do them in a before_configuration or to_prepare. Not sure exactly how we'd execute on that though.

Thanks again for all of your help! I look forward to contributing back to make this process a bit easier!

hschne

hschne commented on Jul 27, 2024

@hschne
OwnerAuthor

Thanks for the fantastic writeup! Great work 🎉

You can see I added a way to silence the SQL Logs for RMP.

IMO great way to go about that 👍

I think the naming is decent as-is, but since we're using TracePresenter, I wonder if TraceProvider might make a bit more sense than Tracer?

Not sure about that, Tracer servers as a base class in the sense that all tracers inherit from it. E.g SequelTracer inheriting from TraceProvider doesn't seem right. Might be misunderstanding what you mean though 😅

As far as a plugin system, I'm thinking something along the lines of the below for a possible API?

That API works for me. At this point you probably have better understanding of what a plugin system needs than I do. So any PR you open I'll probably accept - after review of course, which might take a while 🙈 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Add plugin system to allow users to add their own tracing · Issue #96 · hschne/rails-mini-profiler