From 5f393f883ab5f10eb9535bb7d459a3f64333b1d4 Mon Sep 17 00:00:00 2001 From: zvkemp Date: Tue, 7 Jan 2025 14:02:19 -0500 Subject: [PATCH] feat: Gradually add metrics capabilities to Instrumentation::Base --- instrumentation/base/Appraisals | 13 ++ instrumentation/base/Gemfile | 3 + .../lib/opentelemetry/instrumentation/base.rb | 34 +++- .../opentelemetry/instrumentation/metrics.rb | 192 ++++++++++++++++++ ...opentelemetry-instrumentation-base.gemspec | 1 + .../base/test/instrumentation/base_test.rb | 124 +++++++++++ instrumentation/base/test/test_helper.rb | 6 +- 7 files changed, 362 insertions(+), 11 deletions(-) create mode 100644 instrumentation/base/Appraisals create mode 100644 instrumentation/base/lib/opentelemetry/instrumentation/metrics.rb diff --git a/instrumentation/base/Appraisals b/instrumentation/base/Appraisals new file mode 100644 index 000000000..f99256b2a --- /dev/null +++ b/instrumentation/base/Appraisals @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +appraise 'base' do + remove_gem 'opentelemetry-metrics-api' + remove_gem 'opentelemetry-metrics-sdk' +end + +appraise 'metrics-api' do + remove_gem 'opentelemetry-metrics-sdk' +end + +appraise 'metrics-sdk' do # rubocop: disable Lint/EmptyBlock +end diff --git a/instrumentation/base/Gemfile b/instrumentation/base/Gemfile index f649e2f64..c2a88bb22 100644 --- a/instrumentation/base/Gemfile +++ b/instrumentation/base/Gemfile @@ -6,4 +6,7 @@ source 'https://rubygems.org' +gem 'opentelemetry-metrics-api', '~> 0.2' +gem 'opentelemetry-metrics-sdk' + gemspec diff --git a/instrumentation/base/lib/opentelemetry/instrumentation/base.rb b/instrumentation/base/lib/opentelemetry/instrumentation/base.rb index ddf828955..3c0a226c9 100644 --- a/instrumentation/base/lib/opentelemetry/instrumentation/base.rb +++ b/instrumentation/base/lib/opentelemetry/instrumentation/base.rb @@ -69,8 +69,9 @@ class << self integer: ->(v) { v.is_a?(Integer) }, string: ->(v) { v.is_a?(String) } }.freeze + SINGLETON_MUTEX = Thread::Mutex.new - private_constant :NAME_REGEX, :VALIDATORS + private_constant :NAME_REGEX, :VALIDATORS, :SINGLETON_MUTEX private :new @@ -163,8 +164,10 @@ def option(name, default:, validate:) end def instance - @instance ||= new(instrumentation_name, instrumentation_version, install_blk, - present_blk, compatible_blk, options) + @instance || SINGLETON_MUTEX.synchronize do + @instance ||= new(instrumentation_name, instrumentation_version, install_blk, + present_blk, compatible_blk, options) + end end private @@ -189,13 +192,15 @@ def infer_version end end - attr_reader :name, :version, :config, :installed, :tracer + attr_reader :name, :version, :config, :installed, :tracer, :meter alias installed? installed + require_relative 'metrics' + prepend(OpenTelemetry::Instrumentation::Metrics) + # rubocop:disable Metrics/ParameterLists - def initialize(name, version, install_blk, present_blk, - compatible_blk, options) + def initialize(name, version, install_blk, present_blk, compatible_blk, options) @name = name @version = version @install_blk = install_blk @@ -204,7 +209,8 @@ def initialize(name, version, install_blk, present_blk, @config = {} @installed = false @options = options - @tracer = OpenTelemetry::Trace::Tracer.new + @tracer = OpenTelemetry::Trace::Tracer.new # default no-op tracer + @meter = OpenTelemetry::Metrics::Meter.new if defined?(OpenTelemetry::Metrics::Meter) # default no-op meter end # rubocop:enable Metrics/ParameterLists @@ -217,10 +223,12 @@ def install(config = {}) return true if installed? @config = config_options(config) + return false unless installable?(config) + prepare_install instance_exec(@config, &@install_blk) - @tracer = OpenTelemetry.tracer_provider.tracer(name, version) + @installed = true end @@ -263,6 +271,10 @@ def enabled?(config = nil) private + def prepare_install + @tracer = OpenTelemetry.tracer_provider.tracer(name, version) + end + # The config_options method is responsible for validating that the user supplied # config hash is valid. # Unknown configuration keys are not included in the final config hash. @@ -317,13 +329,17 @@ def config_options(user_config) # will be OTEL_RUBY_INSTRUMENTATION_SINATRA_ENABLED. A value of 'false' will disable # the instrumentation, all other values will enable it. def enabled_by_env_var? + !disabled_by_env_var? + end + + def disabled_by_env_var? var_name = name.dup.tap do |n| n.upcase! n.gsub!('::', '_') n.gsub!('OPENTELEMETRY_', 'OTEL_RUBY_') n << '_ENABLED' end - ENV[var_name] != 'false' + ENV[var_name] == 'false' end # Checks to see if the user has passed any environment variables that set options diff --git a/instrumentation/base/lib/opentelemetry/instrumentation/metrics.rb b/instrumentation/base/lib/opentelemetry/instrumentation/metrics.rb new file mode 100644 index 000000000..7f91bfa48 --- /dev/null +++ b/instrumentation/base/lib/opentelemetry/instrumentation/metrics.rb @@ -0,0 +1,192 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Instrumentation + # Extensions to Instrumentation::Base that handle metrics instruments. + # The goal here is to allow metrics to be added gradually to instrumentation libraries, + # without requiring that the metrics-sdk or metrics-api gems are present in the bundle + # (if they are not, or if the metrics-api gem does not meet the minimum version requirement, + # the no-op edition is installed.) + module Metrics + METER_TYPES = %i[ + counter + observable_counter + histogram + gauge + observable_gauge + up_down_counter + observable_up_down_counter + ].freeze + + def self.prepended(base) + base.prepend(Compatibility) + base.extend(Compatibility) + base.extend(Registration) + + if base.metrics_compatible? + base.prepend(Extensions) + else + base.prepend(NoopExtensions) + end + end + + # Methods to check whether the metrics API is defined + # and is a compatible version + module Compatibility + METRICS_API_MINIMUM_GEM_VERSION = Gem::Version.new('0.2.0') + + def metrics_defined? + defined?(OpenTelemetry::Metrics) + end + + def metrics_compatible? + metrics_defined? && Gem.loaded_specs['opentelemetry-metrics-api'].version >= METRICS_API_MINIMUM_GEM_VERSION + end + + extend(self) + end + + # class-level methods to declare and register metrics instruments. + # This can be extended even if metrics is not active or present. + module Registration + METER_TYPES.each do |instrument_kind| + define_method(instrument_kind) do |name, **opts, &block| + opts[:callback] ||= block if block + register_instrument(instrument_kind, name, **opts) + end + end + + def register_instrument(kind, name, **opts) + key = [kind, name] + if instrument_configs.key?(key) + warn("Duplicate instrument configured for #{self}: #{key.inspect}") + else + instrument_configs[key] = opts + end + end + + def instrument_configs + @instrument_configs ||= {} + end + end + + # No-op instance methods for metrics instruments. + module NoopExtensions + METER_TYPES.each do |kind| + define_method(kind) {} # rubocop: disable Lint/EmptyBlock + end + + def with_meter; end + + def metrics_enabled? + false + end + end + + # Instance methods for metrics instruments. + module Extensions + %i[ + counter + observable_counter + histogram + gauge + observable_gauge + up_down_counter + observable_up_down_counter + ].each do |kind| + define_method(kind) do |name| + get_metrics_instrument(kind, name) + end + end + + # This is based on a variety of factors, and should be invalidated when @config changes. + # It should be explicitly set in `prepare_install` for now. + def metrics_enabled? + !!@metrics_enabled + end + + # @api private + # ONLY yields if the meter is enabled. + def with_meter + yield @meter if metrics_enabled? + end + + private + + def compute_metrics_enabled + return false unless metrics_compatible? + return false if metrics_disabled_by_env_var? + + !!@config[:metrics] || metrics_enabled_by_env_var? + end + + # Checks if this instrumentation's metrics are enabled by env var. + # This follows the conventions as outlined above, using `_METRICS_ENABLED` as a suffix. + # Unlike INSTRUMENTATION_*_ENABLED variables, these are explicitly opt-in (i.e. + # if the variable is unset, and `metrics: true` is not in the instrumentation's config, + # the metrics will not be enabled) + def metrics_enabled_by_env_var? + ENV.key?(metrics_env_var_name) && ENV[metrics_env_var_name] != 'false' + end + + def metrics_disabled_by_env_var? + ENV[metrics_env_var_name] == 'false' + end + + def metrics_env_var_name + @metrics_env_var_name ||= + begin + var_name = name.dup + var_name.upcase! + var_name.gsub!('::', '_') + var_name.gsub!('OPENTELEMETRY_', 'OTEL_RUBY_') + var_name << '_METRICS_ENABLED' + var_name + end + end + + def prepare_install + @metrics_enabled = compute_metrics_enabled + if metrics_defined? + @metrics_instruments = {} + @instrument_mutex = Mutex.new + end + + @meter = OpenTelemetry.meter_provider.meter(name, version: version) if metrics_enabled? + + super + end + + def get_metrics_instrument(kind, name) + # TODO: we should probably return *something* + # if metrics is not enabled, but if the api is undefined, + # it's unclear exactly what would be suitable. + # For now, there are no public methods that call this + # if metrics isn't defined. + return unless metrics_defined? + + @metrics_instruments.fetch([kind, name]) do |key| + @instrument_mutex.synchronize do + @metrics_instruments[key] ||= create_configured_instrument(kind, name) + end + end + end + + def create_configured_instrument(kind, name) + config = self.class.instrument_configs[[kind, name]] + + if config.nil? + Kernel.warn("unconfigured instrument requested: #{kind} of '#{name}'") + return + end + + meter.public_send(:"create_#{kind}", name, **config) + end + end + end + end +end diff --git a/instrumentation/base/opentelemetry-instrumentation-base.gemspec b/instrumentation/base/opentelemetry-instrumentation-base.gemspec index 0362d3821..58502f1b3 100644 --- a/instrumentation/base/opentelemetry-instrumentation-base.gemspec +++ b/instrumentation/base/opentelemetry-instrumentation-base.gemspec @@ -29,6 +29,7 @@ Gem::Specification.new do |spec| spec.add_dependency 'opentelemetry-common', '~> 0.21' spec.add_dependency 'opentelemetry-registry', '~> 0.1' + spec.add_development_dependency 'appraisal', '~> 2.5' spec.add_development_dependency 'bundler', '~> 2.4' spec.add_development_dependency 'minitest', '~> 5.0' spec.add_development_dependency 'opentelemetry-test-helpers', '~> 0.3' diff --git a/instrumentation/base/test/instrumentation/base_test.rb b/instrumentation/base/test/instrumentation/base_test.rb index c58bbe500..ac0306ed3 100644 --- a/instrumentation/base/test/instrumentation/base_test.rb +++ b/instrumentation/base/test/instrumentation/base_test.rb @@ -53,15 +53,89 @@ def initialize(*args) end end + let(:instrumentation_with_metrics) do + Class.new(OpenTelemetry::Instrumentation::Base) do + instrumentation_name 'test_instrumentation' + instrumentation_version '0.1.1' + + option :metrics, default: false, validate: :boolean + + install { true } + present { true } + + if defined?(OpenTelemetry::Metrics) + counter 'example.counter' + observable_counter 'example.observable_counter' + histogram 'example.histogram' + gauge 'example.gauge' + observable_gauge 'example.observable_gauge' + up_down_counter 'example.up_down_counter' + observable_up_down_counter 'example.observable_up_down_counter' + end + + def example_counter + counter 'example.counter' + end + + def example_observable_counter + observable_counter 'example.observable_counter' + end + + def example_histogram + histogram 'example.histogram' + end + + def example_gauge + gauge 'example.gauge' + end + + def example_observable_gauge + observable_gauge 'example.observable_gauge' + end + + def example_up_down_counter + up_down_counter 'example.up_down_counter' + end + + def example_observable_up_down_counter + observable_up_down_counter 'example.observable_up_down_counter' + end + end + end + it 'is auto-registered' do instance = instrumentation.instance _(OpenTelemetry::Instrumentation.registry.lookup('test_instrumentation')).must_equal(instance) end describe '.instance' do + let(:instrumentation) do + Class.new(OpenTelemetry::Instrumentation::Base) do + instrumentation_name 'test_instrumentation' + instrumentation_version '0.1.1' + + def initialize(*args) + # Simulate latency by hinting the VM should switch tasks + # (this can also be accomplished by something like `sleep(0.1)`). + # This replicates the worst-case scenario when using default assignment + # to obtain a singleton, i.e. that the scheduler switches threads between + # the nil check and object initialization. + Thread.pass + super + end + end + end + it 'returns an instance' do _(instrumentation.instance).must_be_instance_of(instrumentation) end + + it 'returns the same singleton instance to every thread' do + object_ids = Array.new(2).map { Thread.new { instrumentation.instance } } + .map { |thr| thr.join.value } + + _(object_ids.uniq.count).must_equal(1) + end end describe '.option' do @@ -442,6 +516,56 @@ def initialize(*args) end end + describe 'metrics' do + let(:config) { {} } + let(:instance) { instrumentation_with_metrics.instance } + + before do + instance.install(config) + end + + if defined?(OpenTelemetry::Metrics) + describe 'with the metrics api' do + it 'is disabled by default' do + _(instance.metrics_enabled?).must_equal false + end + + it 'returns a no-op counter' do + counter = instance.example_counter + _(counter).must_be_kind_of(OpenTelemetry::Metrics::Instrument::Counter) + end + + describe 'with the option enabled' do + let(:config) { { metrics: true } } + + it 'will be enabled' do + _(instance.metrics_enabled?).must_equal true + end + + it 'returns a counter' do + counter = instance.example_counter + + _(counter).must_be_kind_of(OpenTelemetry::Internal::ProxyInstrument) + end + end + end + else + describe 'without the metrics api' do + it 'will not be enabled' do + _(instance.metrics_enabled?).must_equal false + end + + describe 'with the option enabled' do + let(:config) { { metrics: true } } + + it 'will not be enabled' do + _(instance.metrics_enabled?).must_equal false + end + end + end + end + end + def define_instrumentation_subclass(name, version = nil) names = name.split('::').map(&:to_sym) names.inject(Object) do |object, const| diff --git a/instrumentation/base/test/test_helper.rb b/instrumentation/base/test/test_helper.rb index 9efdc59b3..7febed6ea 100644 --- a/instrumentation/base/test/test_helper.rb +++ b/instrumentation/base/test/test_helper.rb @@ -4,12 +4,14 @@ # # SPDX-License-Identifier: Apache-2.0 -require 'bundler/setup' -Bundler.require(:default, :development, :test) +require 'simplecov' SimpleCov.start SimpleCov.minimum_coverage 85 +require 'bundler/setup' +Bundler.require(:default, :development, :test) + require 'opentelemetry-instrumentation-base' require 'minitest/autorun'