getappmap / appmap-ruby

AppMap client agent for Ruby
https://appland.org
Other
98 stars 11 forks source link

Proper way of hooking methods named `call` #344

Open ikuo opened 9 months ago

ikuo commented 9 months ago

It looks that we globally ignore methods named call here.

When we want to hook methods named call in our application Ruby classes (./app or ./lib in our case), is there any proper way to do this?

Currently, I'm working around it with a monkey patch like this:

# lib/appmap/hook.rb
module AppMap
  class Hook
    def trace_end(...)
      ...
      local_path = location.delete_prefix(Dir.pwd + '/')
      is_app = local_path.start_with?('app') || local_path.start_with?('lib')
      next if !is_app && method_id == :call

But I think there must be a better way to do this.

kgilpin commented 9 months ago

Hi, great question. I believe this restriction is here because hooking methods named call can interfere with the recording process itself.

In AppMap configuration there is an explicit list of functions to be recorded:

https://appmap.io/docs/reference/appmap-ruby.html#configuration

Maybe functions listed here should not be subject to the call restriction? In that case it will be “caveat emptor” about possible interference.

We could also look at renaming internal call methods of AppMap to mitigate the need for this restriction.

ikuo commented 9 months ago

@kgilpin Hi, thank you for letting me know the background and possible approaches.

I feel like the first approach (explicit list with caveat emptor) helps us a lot if it supports glob patterns based on source_location etc. since my app has a lot of call methods defined. And with existing or new test programs, I hope we can mitigate the possible interference.