feat: Create MetricsPatch modules to hold metrics

Like the Metrics ConfigurationPatch, include the MetricsPatch modules if
Metrics is enabled/present

instrumentation/base/lib/opentelemetry/instrumentation.rb

@@ -7,6 +7,7 @@
 require 'opentelemetry'
 require 'opentelemetry-registry'
 require 'opentelemetry/instrumentation/base'
+require 'opentelemetry/instrumentation/metrics_patch' if defined?(OpenTelemetry::Metrics) # maybe also add Env var check?
 
 module OpenTelemetry
   # The instrumentation module contains functionality to register and install

instrumentation/base/lib/opentelemetry/instrumentation/base.rb

@@ -4,6 +4,9 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
+# TODO: Maybe add an env var for globally enabling metrics as second switch?
+require_relative 'metrics_patch' if defined?(OpenTelemetry::Metrics)
+
 module OpenTelemetry
   module Instrumentation
     # The Base class holds all metadata and configuration for an
@@ -205,19 +208,12 @@ def initialize(name, version, install_blk, present_blk,
         @installed = false
         @options = options
         @tracer = OpenTelemetry::Trace::Tracer.new
-        # Do we want to conditionally create a meter overall?
-        # @meter = OpenTelemetry::Metrics::Meter.new if metrics_enabled?
+        create_meter
       end
       # rubocop:enable Metrics/ParameterLists
 
-      def metrics_enabled?
-        # We need the API as a dependency to call metrics-y things in instrumentation
-        # However, the user needs to install it separately from base, because we
-        # do not want base to rely on experimental code
-        return @metrics_enabled if defined?(@metrics_enabled)
-
-        @metrics_enabled ||= defined?(OpenTelemetry::Metrics) && @config[:send_metrics]
-      end
+      # no-op, overridden in metrics patch
+      def create_meter; end
 
       # Install instrumentation with the given config. The present? and compatible?
       # will be run first, and install will return false if either fail. Will
@@ -232,10 +228,18 @@ def install(config = {})
 
         instance_exec(@config, &@install_blk)
         @tracer = OpenTelemetry.tracer_provider.tracer(name, version)
-        @meter = OpenTelemetry.meter_provider.meter(name, version: version) if metrics_enabled?
+        install_meter
         @installed = true
       end
 
+      # no-op, defined in metrics patch to use if metrics installed
+      def install_meter; end
+
+      # Metrics should not be enabled if we're trying to load them directly from base
+      def metrics_enabled?
+        false
+      end
+
       # Whether or not this instrumentation is installable in the current process. Will
       # be true when the instrumentation defines an install block, is not disabled
       # by environment or config, and the target library present and compatible.

instrumentation/base/lib/opentelemetry/instrumentation/metrics_patch.rb

@@ -0,0 +1,21 @@
+module OpenTelemetry
+  module Instrumentation
+    module MetricsPatch
+      def create_meter
+        @meter = OpenTelemetry::Metrics::Meter.new
+      end
+
+      def install_meter
+        @meter = OpenTelemetry.meter_provider.meter(name, version: version) if metrics_enabled?
+      end
+
+      def metrics_enabled?
+        return @metrics_enabled if defined?(@metrics_enabled)
+
+        @metrics_enabled ||= defined?(OpenTelemetry::Metrics) && @config[:send_metrics]
+      end
+    end
+  end
+end
+
+OpenTelemetry::Instrumentation::Base.prepend(OpenTelemetry::Instrumentation::MetricsPatch)

instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb

@@ -30,6 +30,7 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base
         option :response_propagators,     default: [],    validate: :array
         # This option is only valid for applications using Rack 2.0 or greater
         option :use_rack_events,          default: true,  validate: :boolean
+        # TODO: This option currently exclusively uses the event handler, should we support old and new Rack?
         option :send_metrics,             default: false, validate: :boolean
         # Temporary Helper for Sinatra and ActionPack middleware to use during installation
         #
@@ -47,10 +48,16 @@ def middleware_args
           end
         end
 
+        # def metrics_enabled?
+        #   super
+        #   # other conditions unique to Rack? Like Events also being available?
+        # end
+
         private
 
         def require_dependencies
           require_relative 'middlewares/event_handler' if defined?(::Rack::Events)
+          require_relative 'middlewares/metrics_patch' if metrics_enabled?
           require_relative 'middlewares/tracer_middleware'
         end
 

instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler.rb

@@ -158,43 +158,8 @@ def untraced_request?(env)
             false
           end
 
-          def record_http_server_request_duration_metric(span)
-            return unless metrics_enabled? && http_server_duration_histogram
-
-            # find span duration
-            # end - start / a billion to convert nanoseconds to seconds
-            duration = (span.end_timestamp - span.start_timestamp) / Float(10**9)
-            # Create attributes
-            #
-            attrs = {}
-            # pattern below goes
-            # stable convention
-            # current span convention
-
-            # attrs['http.request.method']
-            attrs['http.method'] = span.attributes['http.method']
-
-            # attrs['url.scheme']
-            attrs['http.scheme'] = span.attributes['http.scheme']
-
-            # same in stable semconv
-            attrs['http.route'] = span.attributes['http.route']
-
-            # attrs['http.response.status.code']
-            attrs['http.status_code'] = span.attributes['http.status_code']
-
-            # attrs['server.address'] ???
-            # attrs['server.port'] ???
-            # span includes host and port
-            attrs['http.host'] = span.attributes['http.host']
-
-            # attrs not currently in span payload
-            # attrs['network.protocol.version']
-            # attrs['network.protocol.name']
-            attrs['error.type'] = span.status.description if span.status.code == OpenTelemetry::Trace::Status::ERROR
-
-            http_server_duration_histogram.record(duration, attributes: attrs)
-          end
+          # no-op, defined in MetricsPatch and required if metrics enabled
+          def record_http_server_request_duration_metric(span); end
 
           # https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-http.md#name
           #
@@ -286,31 +251,11 @@ def tracer
             OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.tracer
           end
 
-          def metrics_enabled?
-            OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.metrics_enabled?
-          end
-
-          def meter
-            # warn if no meter?
-            return @meter if defined?(@meter)
+          # no-op, overwritten by metrics patch if metrics is enabled
+          def meter; end
 
-            @meter = metrics_enabled? ? OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.meter : nil
-          end
-
-          def http_server_duration_histogram
-            # only want to make the view and the histogram once
-            # OpenTelemetry.meter_provider.add_view(
-            #   'http.server.request.duration',
-            #   aggregation: OpenTelemetry::SDK::Metrics::Aggregation::ExplicitBucketHistogram.new(
-            #     boundaries: [0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10]
-            #     )
-            #   )
-            # Meter might be nil if metrics API isn't installed or isn't configured to send data
-            return @http_server_duration_histogram if defined?(@http_server_duration_histogram)
-
-            @http_server_duration_histogram = nil unless meter
-            @http_server_duration_histogram = meter.create_histogram('http.server.request.duration', unit: 's', description: 'Duration of HTTP server requests.')
-          end
+          # no-op, overwritten by metrics patch if metrics is enabled
+          def http_server_request_duration_histogram; end
 
           def config
             OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.config

instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/metrics_patch.rb

@@ -0,0 +1,58 @@
+module OpenTelemetry
+  module Instrumentation
+    module Rack
+      module Middlewares
+        module MetricsPatch
+          # Don't check in here to see if metrics is enabled, trust that if it's required, metrics will be enabled
+          def meter
+            OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.meter
+          end
+
+          def http_server_request_duration_histogram
+            # TODO: Add Advice to set small explicit histogram bucket boundaries
+            # TODO: Does this need to be memoized?
+            @http_server_request_duration_histogram ||= meter.create_histogram('http.server.request.duration', unit: 's', description: 'Duration of HTTP server requests.')
+          end
+
+          def record_http_server_request_duration_metric(span)
+            # find span duration
+            # end - start / a billion to convert nanoseconds to seconds
+            duration = (span.end_timestamp - span.start_timestamp) / Float(10**9)
+            # Create attributes
+            #
+            attrs = {}
+            # pattern below goes
+            # stable convention
+            # current span convention
+
+            # attrs['http.request.method']
+            attrs['http.method'] = span.attributes['http.method']
+
+            # attrs['url.scheme']
+            attrs['http.scheme'] = span.attributes['http.scheme']
+
+            # same in stable semconv
+            attrs['http.route'] = span.attributes['http.route']
+
+            # attrs['http.response.status.code']
+            attrs['http.status_code'] = span.attributes['http.status_code']
+
+            # attrs['server.address'] ???
+            # attrs['server.port'] ???
+            # span includes host and port
+            attrs['http.host'] = span.attributes['http.host']
+
+            # attrs not currently in span payload
+            # attrs['network.protocol.version']
+            # attrs['network.protocol.name']
+            attrs['error.type'] = span.status.description if span.status.code == OpenTelemetry::Trace::Status::ERROR
+
+            http_server_request_duration_histogram.record(duration, attributes: attrs)
+          end
+        end
+      end
+    end
+  end
+end
+
+OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandler.prepend(OpenTelemetry::Instrumentation::Rack::Middlewares::MetricsPatch)