newrelic-ruby-agent icon indicating copy to clipboard operation
newrelic-ruby-agent copied to clipboard

Count the number of times methods from instrumented libraries are called

Open kaylareopelle opened this issue 3 years ago • 5 comments

We'd like to find a way to use data to identify new methods to instrument within the libraries we currently instrument.

One idea to accomplish this would be to create a metric that would register the class name and method name called and increment the number of times it was called. This would give us potential methods to investigate for new instrumentation and provide feedback about the usage of the methods we already instrument.

Potential gotchas:

  • How can we avoid causing a metrics explosion (too many unique metrics) for our customers?
  • How can we keep this calculation performant/low overhead?

kaylareopelle avatar Jun 02 '22 22:06 kaylareopelle

The scope of this ticket as part of the Core Technologies - Clean and Shine milestone has been limited to discovery so that we can pick this up at a later time with a clearer technical direction.

kaylareopelle avatar Jul 05 '22 16:07 kaylareopelle

Here is my preference for an updated new scope:

  • Define a String array of class names
    • These names will come from existing gems that we instrument (but might not instrument all methods for) AND gems that we have not yet instrumented
    • Example: %w[InstrumentedGem NotYetInstrumentedGem]
  • Define a String array or Hash of class name and method name combinations that are to be ignored. These combinations will come from methods that have already been instrumented, as well as methods we simply want to ignore.
    • This list will behave as a blocklist for methods to not report usage info for
    • The list will need to convey whether each method is a class or instance method
    • Example (String array): %w[InstrumentedGem.already_instrumented_method InstrumentedGem#already_instrumented_instance_method NotYetInstrumentedGem.ignored_method]
    • Example (Hash): { class: 'InstrumentedGem', method: already_instrumented_method, self: true }
  • Define a common agent method that will accept as input information about the method (class name, method name, is the method a class method) and automatically auto-increment a usage metric.
  • At boot time, if supported, and unless configured not to, the agent will use module prepends to automatically set up all public methods belonging to the class name allowlist to be evaluated.

I have a working proof of concept that demonstrates the desired prepend behavior from a given input of a String array.

fallwith avatar Jul 05 '22 17:07 fallwith

This POC performs a puts before calling super, but is contains the core logic needed to automatically hook into all public class and instance methods for any given class.

class CoolNewGem
  def self.awesome; end
  def fantastic; end
end

class SuperGem
  def self.neato; end
  def stellar; end
end

module An
  class ExcellentGem
    def self.sweet; end
    def perfect; end
  end
end





module NewRelic
  module LanguageSupport
    extend self
    def constantize(const_name)
      const_name.to_s.sub(/\A::/, '').split('::').inject(Object) do |namespace, name|
        begin
          result = namespace.const_get(name)

          # const_get looks up the inheritence chain, so if it's a class
          # in the constant make sure we found the one in our namespace.
          #
          # Can't help if the constant isn't a class...
          if result.is_a?(Module)
            expected_name = "#{namespace}::#{name}".gsub(/^Object::/, "")
            return unless expected_name == result.to_s
          end

          result
        rescue NameError
          nil
        end
      end
    end
  end
end





def evaluation_method(name, class_method = false)
  <<-METHOD
    def #{name}
      puts "NR evaluation code for method #{name} (class method = #{class_method})..."

      super
    end
  METHOD
end

def evaluate_instance_methods(klass)
  code_to_eval = String.new("prepend(Module.new do\n")
  klass.instance_methods(false).each do |method|
    code_to_eval += evaluation_method(method)
  end
  code_to_eval += 'end)'
  klass.class_eval(code_to_eval)
end

def evaluate_class_methods(klass)
  code_to_eval = String.new("#{klass}.singleton_class.prepend(Module.new do\n")
  klass.methods(false).each do |method|
    code_to_eval += evaluation_method(method, true)
  end
  code_to_eval += 'end)'
  klass.class_eval(code_to_eval)
end

def evaluate_gem(klass)
  evaluate_class_methods(klass)
  evaluate_instance_methods(klass)
end


evaluated_gems = %w[CoolNewGem SuperGem An::ExcellentGem MissingGem]

evaluated_gems.each do |name|


  const = NewRelic::LanguageSupport.constantize(name)
  next unless const

  evaluate_gem(const)
end

CoolNewGem.awesome
CoolNewGem.new.fantastic

SuperGem.neato
SuperGem.new.stellar

An::ExcellentGem.sweet
An::ExcellentGem.new.perfect

fallwith avatar Jul 06 '22 02:07 fallwith

Team discussion

To avoid metric grouping issues:

  • maximum number of methods per class
  • if you don't run that gem, the constantization would fail
  • would have a maximum number of class names times the methods we set

Alternately, for new gems, just check to see if the gem is being used, keep the footprint small Or just use this for existing gems to limit the footprint

Could include just the gem, see how many times the gems are used, which could help with the number of metrics problem, and see instead the picture of the gem as a whole

Dimensional metrics for an individual gem with sub-artifacts for each method or class

We need to have chain options also in the case that prepend isn't compatible with the gems we want to generate the metrics about. Could be more uncertain about the results for gems we haven't instrumented before.

Could also try to hook into the Ruby internals to increment a counter whenever our chosen list of methods are called.

What kind of footprint will this have on our customers? Could it cause a crash or other unexpected issues? Decouple this issue with what our customers are using and what we need to instrument.

Decision

  • Put this on hold until after we implement dimensional metrics to see if this data collection is more feasible with that data structure

kaylareopelle avatar Jul 06 '22 17:07 kaylareopelle

https://issues.newrelic.com/browse/NEWRELIC-3446

Nah

fallwith avatar Jun 05 '23 23:06 fallwith