feat: Externalize config of hooks

kgilpin · kgilpin · commit 8080222ce5b6 · 2021-11-03T12:26:45.000-04:00
diff --git a/lib/appmap/builtin_hooks/json.yml b/lib/appmap/builtin_hooks/json.yml
@@ -0,0 +1,4 @@
+- method: JSON::Ext::Parser#parse
+  label: format.json.parse
+- method: JSON::Ext::Generator::State#generate
+  label: format.json.generate
diff --git a/lib/appmap/builtin_hooks/net/http.yml b/lib/appmap/builtin_hooks/net/http.yml
@@ -0,0 +1,3 @@
+- method: Net::HTTP#request
+  label: protocol.http
+  handler_class: AppMap::Handler::NetHTTP
diff --git a/lib/appmap/builtin_hooks/openssl.yml b/lib/appmap/builtin_hooks/openssl.yml
@@ -0,0 +1,16 @@
+- method: OpenSSL::PKey::PKey#sign
+  label: crypto.pkey
+- methods:
+  - OpenSSL::X509::Request#sign
+  - OpenSSL::X509::Request#verify
+  label: crypto.x509
+- method: OpenSSL::X509::Certificate#sign
+  label: crypto.x509
+- methods:
+  - OpenSSL::PKCS5#pbkdf2_hmac
+  - OpenSSL::PKCS5#pbkdf2_hmac_sha1
+  label: crypto.pkcs5
+- method: OpenSSL::Cipher#encrypt
+  label: crypto.encrypt
+- method: OpenSSL::Cipher#decrypt
+  label: crypto.decrypt
diff --git a/lib/appmap/builtin_hooks/yaml.yml b/lib/appmap/builtin_hooks/yaml.yml
@@ -0,0 +1,10 @@
+- methods:
+  - Psych#load
+  - Psych#load_stream
+  - Psych#parse
+  - Psych#parse_stream
+  label: format.yaml.parse
+- methods:
+  - Psych#dump
+  - Psych#dump_stream
+  label: format.yaml.generate
diff --git a/lib/appmap/config.rb b/lib/appmap/config.rb
@@ -11,18 +11,20 @@
 
 module AppMap
   class Config
-    # Specifies a code +path+ to be mapped.
+    # Specifies a logical code package be mapped.
+    # This can be a project source folder, a Gem, or a builtin.
     #
     # Options:
     #
+    # * +path+ indicates a relative path to a code folder.
     # * +gem+ may indicate a gem name that "owns" the path
     # * +require_name+ can be used to make sure that the code is required so that it can be loaded. This is generally used with
     #   builtins, or when the path to be required is not automatically required when bundler requires the gem.
     # * +exclude+ can be used used to exclude sub-paths. Generally not used with +gem+.
     # * +labels+ is used to apply labels to matching code. This is really only useful when the package will be applied to
     #   specific functions, via TargetMethods.
     # * +shallow+ indicates shallow mapping, in which only the entrypoint to a gem is recorded.
-    Package = Struct.new(:path, :gem, :require_name, :exclude, :labels, :shallow) do
+    Package = Struct.new(:name, :path, :gem, :require_name, :exclude, :labels, :shallow) do
       # This is for internal use only.
       private_methods :gem
 
@@ -45,7 +47,7 @@ class << self
         # Builds a package for a path, such as `app/models` in a Rails app. Generally corresponds to a `path:` entry
         # in appmap.yml. Also used for mapping specific methods via TargetMethods.
         def build_from_path(path, shallow: false, require_name: nil, exclude: [], labels: [])
-          Package.new(path, nil, require_name, exclude, labels, shallow)
+          Package.new(path, path, nil, require_name, exclude, labels, shallow)
         end
 
         # Builds a package for gem. Generally corresponds to a `gem:` entry in appmap.yml. Also used when mapping
@@ -57,7 +59,7 @@ def build_from_gem(gem, shallow: true, require_name: nil, exclude: [], labels: [
           end
           path = gem_path(gem, optional)
           if path
-            Package.new(path, gem, require_name, exclude, labels, shallow)
+            Package.new(gem, path, gem, require_name, exclude, labels, shallow)
           else
             AppMap::Util.startup_message "#{gem} is not available in the bundle"
           end
@@ -75,19 +77,16 @@ def gem_path(gem, optional)
         end
       end
 
-      def name
-        gem || path
-      end
-
       def to_h
         {
+          name: name,
           path: path,
-          require_name: require_name,
           gem: gem,
-          handler_class: handler_class.name,
+          require_name: require_name,
+          handler_class: handler_class ? handler_class.name : nil,
           exclude: Util.blank?(exclude) ? nil : exclude,
           labels: Util.blank?(labels) ? nil : labels,
-          shallow: shallow
+          shallow: shallow.nil? ? nil : shallow,
         }.compact
       end
     end
@@ -97,12 +96,12 @@ class TargetMethods # :nodoc:
       attr_reader :method_names, :package
 
       def initialize(method_names, package)
-        @method_names = method_names
+        @method_names = Array(method_names).map(&:to_sym)
         @package = package
       end
 
       def include_method?(method_name)
-        Array(method_names).include?(method_name)
+        method_names.include?(method_name.to_sym)
       end
 
       def to_h
@@ -139,9 +138,13 @@ def to_h
     private_constant :MethodHook
     
     class << self
-      def package_hooks(gem_name, methods, handler_class: nil, require_name: nil)
+      def package_hooks(methods, path: nil, gem: nil, force: false, handler_class: nil, require_name: nil)
         Array(methods).map do |method|
-          package = Package.build_from_gem(gem_name, require_name: require_name, labels: method.labels, shallow: false, optional: true)
+          package = if gem
+            Package.build_from_gem(gem, require_name: require_name, labels: method.labels, shallow: false, force: force, optional: true)
+          elsif path
+            Package.build_from_path(path, require_name: require_name, labels: method.labels, shallow: false)
+          end
           next unless package
 
           package.handler_class = handler_class if handler_class
@@ -152,85 +155,63 @@ def package_hooks(gem_name, methods, handler_class: nil, require_name: nil)
       def method_hook(cls, method_names, labels)
         MethodHook.new(cls, method_names, labels)
       end
-    end
 
-    # Hook well-known functions. When a function configured here is available in the bundle, it will be hooked with the
-    # predefined labels specified here. If any of these hooks are not desired, they can be disabled in the +exclude+ section
-    # of appmap.yml.
-    METHOD_HOOKS = [
-      package_hooks('actionview',
-        [
-          method_hook('ActionView::Renderer', :render, %w[mvc.view]),
-          method_hook('ActionView::TemplateRenderer', :render, %w[mvc.view]),
-          method_hook('ActionView::PartialRenderer', :render, %w[mvc.view])
-        ],
-        handler_class: AppMap::Handler::Rails::Template::RenderHandler,
-        require_name: 'action_view'
-      ),
-      package_hooks('actionview',
-        [
-          method_hook('ActionView::Resolver', %i[find_all find_all_anywhere], %w[mvc.template.resolver])
-        ],
-        handler_class: AppMap::Handler::Rails::Template::ResolverHandler,
-        require_name: 'action_view'
-      ),
-      package_hooks('actionpack',
-        [
-          method_hook('ActionDispatch::Request::Session', %i[[] dig values fetch], %w[http.session.read]),
-          method_hook('ActionDispatch::Request::Session', %i[destroy []= clear update delete merge], %w[http.session.write]),
-          method_hook('ActionDispatch::Cookies::CookieJar', %i[[] fetch], %w[http.session.read]),
-          method_hook('ActionDispatch::Cookies::CookieJar', %i[[]= clear update delete recycle], %w[http.session.write]),
-          method_hook('ActionDispatch::Cookies::EncryptedCookieJar', %i[[]= clear update delete recycle], %w[http.cookie crypto.encrypt])
-        ],
-        require_name: 'action_dispatch'
-      ),
-      package_hooks('cancancan',
-        [
-          method_hook('CanCan::ControllerAdditions', %i[authorize! can? cannot?], %w[security.authorization]),
-          method_hook('CanCan::Ability', %i[authorize?], %w[security.authorization])
-        ]
-      ),
-      package_hooks('actionpack',
-        [
-          method_hook('ActionController::Instrumentation', %i[process_action send_file send_data redirect_to], %w[mvc.controller])
-        ],
-        require_name: 'action_controller'
-      )
-    ].flatten.freeze
-
-    OPENSSL_PACKAGES = ->(labels) { Package.build_from_path('openssl', require_name: 'openssl', labels: labels) }
-
-    # Hook functions which are builtin to Ruby. Because they are builtins, they may be loaded before appmap.
-    # Therefore, we can't rely on TracePoint to report the loading of this code.
-    BUILTIN_HOOKS = {
-      'OpenSSL::PKey::PKey' => TargetMethods.new(:sign, OPENSSL_PACKAGES.(%w[crypto.pkey])),
-      'OpenSSL::X509::Request' => TargetMethods.new(%i[sign verify], OPENSSL_PACKAGES.(%w[crypto.x509])),
-      'OpenSSL::PKCS5' => TargetMethods.new(%i[pbkdf2_hmac_sha1 pbkdf2_hmac], OPENSSL_PACKAGES.(%w[crypto.pkcs5])),
-      'OpenSSL::Cipher' => [
-        TargetMethods.new(%i[encrypt], OPENSSL_PACKAGES.(%w[crypto.encrypt])),
-        TargetMethods.new(%i[decrypt], OPENSSL_PACKAGES.(%w[crypto.decrypt]))
-      ],
-      'ActiveSupport::Callbacks::CallbackSequence' => [
-        TargetMethods.new(:invoke_before, Package.build_from_gem('activesupport', force: true, require_name: 'active_support', labels: %w[mvc.before_action])),
-        TargetMethods.new(:invoke_after, Package.build_from_gem('activesupport', force: true, require_name: 'active_support', labels: %w[mvc.after_action])),
-      ],
-      'ActiveSupport::SecurityUtils' => TargetMethods.new(:secure_compare, Package.build_from_gem('activesupport', force: true, require_name: 'active_support/security_utils', labels: %w[crypto.secure_compare])),
-      'OpenSSL::X509::Certificate' => TargetMethods.new(:sign, OPENSSL_PACKAGES.(%w[crypto.x509])),
-      'Net::HTTP' => TargetMethods.new(:request, Package.build_from_path('net/http', require_name: 'net/http', labels: %w[protocol.http]).tap do |package|
-        package.handler_class = AppMap::Handler::NetHTTP
-      end),
-      'Net::SMTP' => TargetMethods.new(:send, Package.build_from_path('net/smtp', require_name: 'net/smtp', labels: %w[protocol.email.smtp])),
-      'Net::POP3' => TargetMethods.new(:mails, Package.build_from_path('net/pop3', require_name: 'net/pop', labels: %w[protocol.email.pop])),
-      # This is happening: Method send_command not found on Net::IMAP
-      # 'Net::IMAP' => TargetMethods.new(:send_command, Package.build_from_path('net/imap', require_name: 'net/imap', labels: %w[protocol.email.imap])),
-      # 'Marshal' => TargetMethods.new(%i[dump load], Package.build_from_path('marshal', labels: %w[format.marshal])),
-      'Psych' => [
-        TargetMethods.new(%i[load load_stream parse parse_stream], Package.build_from_path('yaml', require_name: 'psych', labels: %w[format.yaml.parse])),
-        TargetMethods.new(%i[dump dump_stream], Package.build_from_path('yaml', require_name: 'psych', labels: %w[format.yaml.generate])),
-      ],
-      'JSON::Ext::Parser' => TargetMethods.new(:parse, Package.build_from_path('json', require_name: 'json', labels: %w[format.json.parse])),
-      'JSON::Ext::Generator::State' => TargetMethods.new(:generate, Package.build_from_path('json', require_name: 'json', labels: %w[format.json.generate])),
-    }.freeze
+      def declare_hook(hook_decl)
+        hook_decl = YAML.load(hook_decl) if hook_decl.is_a?(String)
+        
+        methods_decl = hook_decl['methods'] || hook_decl['method']
+        methods_decl = Array(methods_decl) unless methods_decl.is_a?(Hash)
+        labels_decl = Array(hook_decl['labels'] || hook_decl['label'])
+
+        methods = methods_decl.map do |name|
+          class_name, method_name, static = name.include?('.') ? name.split('.', 2) + [ true ] : name.split('#', 2) + [ false ]
+          method_hook class_name, [ method_name ], labels_decl
+        end
+
+        require_name = hook_decl['require_name']
+        gem_name = hook_decl['gem']
+        path = hook_decl['path']
+
+        options = {
+          gem: gem_name,
+          path: path,
+          require_name: require_name || gem_name || path,
+          force: hook_decl['force']
+        }.compact
+
+        handler_class = hook_decl['handler_class']
+        options[:handler_class] = Util::class_from_string(handler_class) if handler_class
+        
+        package_hooks(methods, **options)
+      end
+
+      def load_builtin_hooks
+        load_hooks('builtin_hooks') do |path, config|
+          config['path'] = path
+        end
+      end
+
+      def load_gem_hooks
+        load_hooks('gem_hooks') do |path, config|
+          config['gem'] = path
+        end
+      end
+
+      def load_hooks(dir, &block)
+        basedir = [ __dir__, dir ].join('/')
+        [].tap do |hooks|
+          Dir.glob("#{basedir}/**/*.yml").each do |yaml_file|
+            path = yaml_file[basedir.length + 1...-4]
+            YAML.load(File.read(yaml_file)).map do |config|
+              yield path, config
+              config
+            end.each do |config|
+              hooks << declare_hook(config)
+            end
+          end
+        end.compact.flatten
+      end
+    end
 
     attr_reader :name, :appmap_dir, :packages, :exclude, :swagger_config, :depends_config, :hooked_methods, :builtin_hooks
 
@@ -247,10 +228,13 @@ def initialize(name,
       @depends_config = depends_config
       @hook_paths = Set.new(packages.map(&:path))
       @exclude = exclude
-      @builtin_hooks = BUILTIN_HOOKS.dup
       @functions = functions
 
-      @hooked_methods = METHOD_HOOKS.each_with_object(Hash.new { |h,k| h[k] = [] }) do |cls_target_methods, hooked_methods|
+      @builtin_hooks = self.class.load_builtin_hooks.each_with_object(Hash.new { |h,k| h[k] = [] }) do |cls_target_methods, hooked_methods|
+        hooked_methods[cls_target_methods.cls] << cls_target_methods.target_methods
+      end
+
+      @hooked_methods = self.class.load_gem_hooks.each_with_object(Hash.new { |h,k| h[k] = [] }) do |cls_target_methods, hooked_methods|
         hooked_methods[cls_target_methods.cls] << cls_target_methods.target_methods
       end
 
@@ -269,9 +253,7 @@ def initialize(name,
       end
 
       @hooked_methods.each_value do |hooks|
-        Array(hooks).each do |hook|
-          @hook_paths << hook.package.path
-        end
+        @hook_paths += Array(hooks).map { |hook| hook.package.path }.compact
       end
     end
 
@@ -422,8 +404,8 @@ def package
 
       # Hook a method which is specified by class and method name.
       def package_for_code_object
-        Array(config.hooked_methods[cls.name])
-          .compact
+        class_name = cls.to_s.index('#<Class:') == 0 ? cls.to_s['#<Class:'.length...-1] : cls.name
+        Array(config.hooked_methods[class_name])
           .find { |hook| hook.include_method?(method.name) }
           &.package
       end
diff --git a/lib/appmap/gem_hooks/actionpack.yml b/lib/appmap/gem_hooks/actionpack.yml
@@ -0,0 +1,40 @@
+- methods:
+  - ActionDispatch::Request::Session#[]
+  - ActionDispatch::Request::Session#dig
+  - ActionDispatch::Request::Session#values
+  - ActionDispatch::Request::Session#fetch
+  - ActionDispatch::Cookies::CookieJar#[]
+  - ActionDispatch::Cookies::CookieJar#fetch
+  label: http.session.read
+  require_name: action_dispatch
+- methods:
+  - ActionDispatch::Request::Session#destroy
+  - ActionDispatch::Request::Session#[]=
+  - ActionDispatch::Request::Session#clear
+  - ActionDispatch::Request::Session#update
+  - ActionDispatch::Request::Session#delete
+  - ActionDispatch::Request::Session#merge
+  - ActionDispatch::Cookies::CookieJar#[]=
+  - ActionDispatch::Cookies::CookieJar#clear
+  - ActionDispatch::Cookies::CookieJar#update
+  - ActionDispatch::Cookies::CookieJar#delete
+  - ActionDispatch::Cookies::CookieJar#recycle!
+  label: http.session.write
+  require_name: action_dispatch
+- methods:
+  - ActionDispatch::Cookies::EncryptedCookieJar#[]=
+  - ActionDispatch::Cookies::EncryptedCookieJar#clear
+  - ActionDispatch::Cookies::EncryptedCookieJar#update
+  - ActionDispatch::Cookies::EncryptedCookieJar#delete
+  - ActionDispatch::Cookies::EncryptedCookieJar#recycle
+  labels:
+  - http.cookie
+  - crypto.encrypt
+  require_name: action_dispatch
+- methods:
+  - ActionController::Instrumentation#process_action
+  - ActionController::Instrumentation#send_file
+  - ActionController::Instrumentation#send_data
+  - ActionController::Instrumentation#redirect_to
+  label: mvc.controller
+  require_name: action_controller
diff --git a/lib/appmap/gem_hooks/actionview.yml b/lib/appmap/gem_hooks/actionview.yml
@@ -0,0 +1,13 @@
+- methods:
+  - ActionView::Renderer#render
+  - ActionView::TemplateRenderer#render
+  - ActionView::PartialRenderer#render
+  label: mvc.view
+  handler_class: AppMap::Handler::Rails::Template::RenderHandler
+  require_name: action_view
+- methods:
+  - ActionView::Resolver#find_all
+  - ActionView::Resolver#find_all_anywhere
+  label: mvc.template.resolver
+  handler_class: AppMap::Handler::Rails::Template::ResolverHandler
+  require_name: action_view
diff --git a/lib/appmap/gem_hooks/activesupport.yml b/lib/appmap/gem_hooks/activesupport.yml
@@ -0,0 +1,12 @@
+- method: ActiveSupport::Callbacks::CallbackSequence#invoke_before
+  label: mvc.before_action
+  require_name: active_support
+  force: true
+- method: ActiveSupport::Callbacks::CallbackSequence#invoke_after
+  label: mvc.after_action
+  require_name: active_support
+  force: true
+- method: ActiveSupport::SecurityUtils#secure_compare
+  label: crypto.secure_compare
+  require_name: active_support/security_utils
+  force: true
diff --git a/lib/appmap/gem_hooks/cancancan.yml b/lib/appmap/gem_hooks/cancancan.yml
@@ -0,0 +1,6 @@
+- methods:
+  - CanCan::ControllerAdditions#authorize!
+  - CanCan::ControllerAdditions#can?
+  - CanCan::ControllerAdditions#cannot?
+  - CanCan::Ability#authorize?
+  label: security.authorization
diff --git a/lib/appmap/hook.rb b/lib/appmap/hook.rb
diff --git a/lib/appmap/util.rb b/lib/appmap/util.rb
diff --git a/spec/config_spec.rb b/spec/config_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+- method: Net::HTTP#request`
	`2`	`+ label: protocol.http`
	`3`	`+ handler_class: AppMap::Handler::NetHTTP`