Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instrumentation Packaging and Installation #19

Closed
mwear opened this issue Jul 17, 2019 · 13 comments
Closed

Instrumentation Packaging and Installation #19

mwear opened this issue Jul 17, 2019 · 13 comments

Comments

@mwear
Copy link
Member

mwear commented Jul 17, 2019

Third Party Instrumentation Proposal for OpenTelemetry Ruby

Overview

There is an RFC for auto-instrumentation and this document describes a possible approach that we could use for managing instrumentation for the OpenTelemetry Ruby Project.

Packaging

  • Instrumentation should be packaged as gems
  • Packages should list dependencies in their gemspec for
    • OpenTelemetry API Version
    • Framework version
  • This will allow us to use Bundler to do the heavy lifting regarding versioning and compatibility of dependencies

Installation

  • The tracer should be able to auto detect any instrumentation that is listed as a dependency in an application's Gemfile
  • The time at which to install instrumentation in a Ruby application can be challenging to get right
    • To make this straightforward for the reference implementation, the SDK should have an API method that a user can call to install instrumentation at the right time for their application
    • Various SDK implementations can engage in whatever clever means they need to try to install instrumentation at the right time by calling this API method at the right time on behalf of the user

Implementation

The implementation I am proposing is very similar to and inspired by Rails::Railtie.

OpenTelemetry Ruby will ship with a common base class, OpenTelemetry::Instrumentation that

  • registers instrumentation with tracer (via the Class#inherited callback)
  • provides hooks for the instrumentation to be installed
  • provides the ability to enable or disable instrumentation
    • by having an optional, explicit hook that can be defined for complex use cases
    • establishing a naming convention for environment variables users can specify to override whether or not instrumentation is enabled

Example Instrumentation Package

A package should list its dependencies in its gemspec

Gem::Specification.new do |spec|
  spec.add_dependency 'activerecord', '>= 5.0'
  spec.add_dependency 'opentelemetry-api', '~> 1.0'
end

A package will ship with a class that subclasses OpenTelemetry::Instrumentation which will auto-register instrumentation, and provide hooks for installing and enabling it. The base class will automatically check for an environment variable derived off of the instrumentation_name field for overriding an enabled decision. The convention I'm proposing for an environment variable name is OPENTELEMETRY_{insrumentation_name}_ENABLED, which in this example would be OPENTELEMETRY_OTEL_ACTIVE_RECORD_ENABLED.

class ActiveRecordInstrumentation < OpenTelemetry::Instrumentation
  instrumentation_name :otel_active_record

  enabled do
    # optional and overridable by environment variable, but complex logic around 
    # whether this is enabled can go here
    true
  end

  install do |tracer|
    require 'active_record_instrumentation/subscriber'
    subscriber = Subscriber.new tracer
    ActiveSupport::Notifications.subscribe 'sql.active_record' do |*args|
      subscriber.call(*args)
    end
  end
end

Instrumentation installation API

The SDK should provide an API method to install instrumentation that can be called at the right time during application startup. This method should evaluate and install all instrumentation that subclasses OpenTelemetry::Instrumentation if it is enabled.

tracer = TracerImplementation.new
OpenTelemetry::Instrumentation.install tracer

OpenTelemetry::Instrumentation Implementation

Below is a rough-draft of what the OpenTelemetry::Instrumentation class would look like

module OpenTelemetry
  class Instrumentation
    class << self
      private :new
      
      #subclasses are registered via this hook
      def inherited subclass
        subclasses << subclass
      end

      def subclasses
        @subclasses ||= []
      end

      def install_instrumentation tracer
        subclasses.each do |subclass|
          instrumentation = subclass.instance
          if instrumentation.enabled? && !instrumentation.installed?
            instrumentation.install tracer
          end
        end
      end

      #this method is dual purpose, its used by the DSL to setup an install block,
      #but when passed a tracer, will install all registered instrumentation
      def install tracer = nil, &blk
        if tracer && blk
          raise ArgumentError, "Pass a tracer or block, but not both"
        end
        if blk
          instance.install_block = blk
        elsif tracer
          install_instrumentation tracer
        end
      end

      def enabled &blk
        instance.enabled_block = blk
      end

      def instrumentation_name name = nil
        if name.nil?
          @instrumentation_name
        else
          @instrumentation_name = name.to_sym
        end
      end

      def instance
        @instance ||= new
      end
    end

    attr_accessor :install_block
    attr_accessor :enabled_block

    attr_reader :installed
    alias_method :installed?, :installed

    def initialize
      @installed = false
    end

    def install tracer = ::OpenTelemetry.global_tracer
      instance_exec tracer, &install_block
      @installed = true
    end

    def enabled?
      if enabled_block
        enabled_by_env? && instance_eval(&enabled_block)
      else
        enabled_by_env?
      end
    end

    private

    def enabled_by_env?
      key = "OPENTELEMETRY_#{self.class.instrumentation_name.to_s.upcase}_ENABLED"
      if ENV.include? key
        ENV[key] == 'true'
      else
        true
      end
    end
  end
end

Discussion

I've been thinking about how we can package instrumentation for OpenTelemetry Ruby and wanted to put these thoughts in writing as a starting point for a discussion about it. I'm open to any and all suggestions with the end goal of us having the best possible instrumentation distribution story, even if it doesn't end up being this one.

@indrekj
Copy link
Contributor

indrekj commented Jul 17, 2019

So currently the user needs to do:

  • Set up tracer implementation tracer = TracerImplementation.new
  • Set the global tracer OpenTelemetry.tracer = tracer
  • Install extensions OpenTelemetry::Instrumentation.install

I wonder if we could merge steps two and three. Maybe something like

tracer = TracerImplementation.new
OpenTelemetry.setup_tracer(tracer)

which does both steps. We'd still need the underlying methods to support multiple tracers per app.

I'm also curios how can we make it work with metrics API nicely. Lets say we have ActiveRecordInstrumentation < OpenTelemetry::Instrumentation as in the example which has install do |tracer|. If the instrumentation uses both tracing api and also metrics api, then I'd think it will need meter as well beside the tracer.

@fbogsany
Copy link
Contributor

We'd still need the underlying methods to support multiple tracers per app.

Is multiple tracers per app a requirement? The API doesn't appear to support this at present.

@indrekj
Copy link
Contributor

indrekj commented Jul 17, 2019

Is multiple tracers per app a requirement? The API doesn't appear to support this at present.

Not entirely sure. I myself haven't needed it in a real app. It does have some benefits though. E.g. it makes testing easier. Here I have a simple script that creates two tracers and creates a quick trace.

@fbogsany
Copy link
Contributor

I'm also curios how can we make it work with metrics API nicely.

Likewise for the distributed context API, and logging once we get there. Ideally, we could find a suitable lifecycle hook that'd let us initialize all the components, so we'd have something like:

OpenTelemetry.tracer = VendorATracer.new
OpenTelemetry.meter = VendorBMeter.new
OpenTelemetry.logger = VendorCLogger.new
OpenTelemetry.distributed_context_manager = VendorDContextManager.new
# implicit initialization hook

Where we call instrumentation.install from install_instrumentation instead of instrumentation.install tracer, and the instrumentation's install method retrieves the components it needs from OpenTelemetry.*.

The challenge is finding a well-defined point where all the instrumentation has been loaded and all the OT components have been registered. If that has to be done explicitly, we at least need to provide guidance for typical use cases (e.g. Rails apps).

@fbogsany
Copy link
Contributor

Is multiple tracers per app a requirement? The API doesn't appear to support this at present.

Not entirely sure. I myself haven't needed it in a real app. It does have some benefits though. E.g. it makes testing easier. Here I have a simple script that creates two tracers and creates a quick trace.

Interesting. In that case, the tracers aren't registered as the tracer, but I guess you still want them to install the instrumentation.

Perhaps it makes more sense to have a global registry of instrumentation in the API, and a hook (or component-specific hooks) that can be called for non-global components (like your 2 tracer example above)?

@mwear
Copy link
Member Author

mwear commented Jul 17, 2019

I wonder if we could merge steps two and three. Maybe something like

tracer = TracerImplementation.new
OpenTelemetry.setup_tracer(tracer)

I like this idea a lot. We might find other things that need to happen at this point in the application that we can facilitate with this method as well.

I'm also curios how can we make it work with metrics API nicely. Lets say we have ActiveRecordInstrumentation < OpenTelemetry::Instrumentation as in the example which has install do |tracer|. If the instrumentation uses both tracing api and also metrics api, then I'd think it will need meter as well beside the tracer.

We could possibly have a configure block that will setup the tracer, meter, logger, etc, and install instrumentation afterwards. It could yield them individually to the block, or the block could possibly just reference them off the OpenTelemetry module. Something like:

OpenTelemetry.configure do |ot|
  ot.tracer = TracerImplementation.new
  ot.meter = MeterImplementation.new
  ot.logger = LoggerImplementation.new
  ...
end

@mwear
Copy link
Member Author

mwear commented Jul 17, 2019

In terms of multiple tracer support. I think having a configure block that will by default install instrumentation using the tracer that is being configured will solve the majority of use cases. For people who want a more fine-grained tracer approach, this will probably require some custom code on behalf of the user. Since we do effectively have an instrumentation registry with this design, we can expose methods that allow users to interact with the individual pieces of instrumentation. We could provide an each_instrumentation method and a lookup(instrumentation_name) method that would facilitate things requiring a more manual approach.

@indrekj
Copy link
Contributor

indrekj commented Jul 17, 2019

Also one thing to consider is that not all instrumentations can be applied globally. This works fine for activerecord, but e.g. rack expects a middleware. With opentracing we did it like https://github.com/opentracing-contrib/ruby-rack-tracer . Faraday had something similar https://github.com/opentracing-contrib/ruby-faraday-tracer . In rack case it used the global tracer by default, but it was also possible to specify the tracer in the arguments.

@mwear
Copy link
Member Author

mwear commented Jul 17, 2019

There might be a class of instrumentation that is not auto-installable. We can decide if it makes sense to have them subclass OpenTelemetry::Instrumentation so that the SDK is aware of the instrumentation, even if it can't install it. If we decide that would be useful, we can add an additional auto_installable flag to the DSL.

@benedictfischer09
Copy link

@mwear what would you think about encouraging instrumentation authors to not add the library they are instrumenting as a dependency? The benefit of doing that is you can have a tool that auto-instruments whatever libraries it detects are loadable and skip over other libraries. The alternatives are 1) having apps manually list out all of the instrumentation gems they want to use 2) auto instrumenting anyway but pulling in gems the app has no intention of actually using

@mwear
Copy link
Member Author

mwear commented Sep 9, 2019

There are pros and cons to specifying the library version as a dependency. Except in rare cases, instrumentation is only going to be compatible certain versions of the instrumented library and a version check has to happen somewhere.

By making the instrumented library a dependency, we can lean on Bundler to resolve them and report incompatibilities. One huge benefit, is that users will discover incompatibilities at build time, rather than at runtime with other approaches. While this does mean that a user would need to explicitly list instrumentation as a dependency, it doesn't mean this has to be a completely manual process. There could be a method of discovery to help find compatible instrumentation. A prerequisite would be an instrumentation registry of sorts. A way that I would envision this working would be a rake task that introspects your application, queries the registry, and lists compatible instrumentation. This presupposes that someday a registry will exist, and I don't know if that is the case or not.

If the instrumented library isn't an explicit dependency, then a version check still has to happen somewhere. This logic could be part of the OpenTelemetry::Instrumentation class as a valid_versions? check, or something along those lines. This would allow a user to bundle instrumentation that doesn't apply to an application. One downside, is that if a user had incompatible instrumentation they will not know until runtime.

For now, I think all options are on the table and both of these are reasonable approaches. We do need a instrumentation discovery story and the shape of that story will likely inform choices such as this.

@fbogsany
Copy link
Contributor

fbogsany commented Sep 9, 2019

Relevant: #67. open-telemetry/oteps#41 proposes to use https://github.com/DataDog/dd-trace-rb as the basis for auto-instrumentation. I have not 👀 at the code closely.

@mwear
Copy link
Member Author

mwear commented Jan 27, 2020

Revisions of the ideas discussed in this issue have been implemented in #164 and #171.

@mwear mwear closed this as completed Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants