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
Activity
topherfangio commentedon Jun 17, 2024
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 commentedon Jun 21, 2024
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 newTracer
classes and register them withRegistry
(see alsoConfiguration
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 aTracePresenter
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:
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):
_trace_list_header.erb
). Shouldn't bee to rough adding this though.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 commentedon Jul 8, 2024
@hschne Thanks! I've been on vacation but will take a look at this this week and see how far I can get!
topherfangio commentedon Jul 11, 2024
@hschne Ok, so I'm running into an issue because the
RailsMiniProfiler::TraceProvider
isn't defined insidelib
, it's inside theapp
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?
topherfangio commentedon Jul 19, 2024
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:
Here's my current code.
config/initializers/rails_mini_profiler_extensions.rb
config/initializers/rails_mini_profiler.rb
Any thoughts on what I'm missing? Thanks!
hschne commentedon Jul 20, 2024
@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:
Trace
table to see if it's just an issue with rendering.'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:topherfangio commentedon Jul 22, 2024
@hschne Finally got it!
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/initializers/rails_mini_profiler.rb
config/initializers/rails_mini_profiler_extensions.rb
A couple of notes:
RailsMiniProfiler::Tracers::
in order to get it working. This is probably an area where a real plugin system could help.rails-mini-profiler/lib/rails_mini_profiler/tracers/registry.rb
Line 60 in f088e39
TracePresenter
, I wonder ifTraceProvider
might make a bit more sense thanTracer
?Possible Plugin API
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
orto_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 commentedon Jul 27, 2024
Thanks for the fantastic writeup! Great work 🎉
IMO great way to go about that 👍
Not sure about that,
Tracer
servers as a base class in the sense that all tracers inherit from it. E.gSequelTracer
inheriting fromTraceProvider
doesn't seem right. Might be misunderstanding what you mean though 😅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 🙈 😄