diff --git a/instrumentation/base/lib/opentelemetry/instrumentation/base.rb b/instrumentation/base/lib/opentelemetry/instrumentation/base.rb index ddf8289551..42c6f0bb3c 100644 --- a/instrumentation/base/lib/opentelemetry/instrumentation/base.rb +++ b/instrumentation/base/lib/opentelemetry/instrumentation/base.rb @@ -189,7 +189,7 @@ def infer_version end end - attr_reader :name, :version, :config, :installed, :tracer + attr_reader :name, :version, :config, :installed, :tracer, :meter alias installed? installed @@ -205,6 +205,8 @@ def initialize(name, version, install_blk, present_blk, @installed = false @options = options @tracer = OpenTelemetry::Trace::Tracer.new + # check to see if the API is defined here because the config isn't available yet + @meter = OpenTelemetry::Metrics::Meter.new if defined?(OpenTelemetry::Metrics) end # rubocop:enable Metrics/ParameterLists @@ -221,9 +223,20 @@ def install(config = {}) instance_exec(@config, &@install_blk) @tracer = OpenTelemetry.tracer_provider.tracer(name, version) + install_meter @installed = true 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 + # 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. diff --git a/instrumentation/rack/Gemfile b/instrumentation/rack/Gemfile index c8b2ad72f9..958c84804b 100644 --- a/instrumentation/rack/Gemfile +++ b/instrumentation/rack/Gemfile @@ -12,4 +12,6 @@ group :test do gem 'opentelemetry-instrumentation-base', path: '../base' gem 'rack-test', '~> 2.1.0' gem 'pry-byebug' + gem 'opentelemetry-metrics-sdk' + gem 'opentelemetry-metrics-api' end diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb index 3bfb68a3c9..f4ead759b4 100644 --- a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb @@ -29,7 +29,9 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base option :untraced_requests, default: nil, validate: :callable 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 + 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 # @@ -41,7 +43,9 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base # @return [Array] consisting of a middleware and arguments used in rack builders def middleware_args if config.fetch(:use_rack_events, false) == true && defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandler) - [::Rack::Events, [OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandler.new]] + handlers = [OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandler.new] + handlers << OpenTelemetry::Instrumentation::Rack::Middlewares::MetricsEventHandler.new if metrics_enabled? + [::Rack::Events, handlers] else [OpenTelemetry::Instrumentation::Rack::Middlewares::TracerMiddleware] end @@ -51,6 +55,7 @@ def middleware_args def require_dependencies require_relative 'middlewares/event_handler' if defined?(::Rack::Events) + require_relative 'middlewares/metrics_event_handler' if metrics_enabled? require_relative 'middlewares/tracer_middleware' end diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler.rb index 5475a4fc5e..d767fb32cc 100644 --- a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler.rb +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler.rb @@ -128,6 +128,7 @@ def extract_request_headers(env) end def extract_response_attributes(response) + # TODO: Rack spec states status should always be an integer, so we might not need to coerce attributes = { 'http.status_code' => response.status.to_i } attributes.merge!(extract_response_headers(response.headers)) attributes diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/metrics_event_handler.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/metrics_event_handler.rb new file mode 100644 index 0000000000..6123bdecd1 --- /dev/null +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/metrics_event_handler.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require_relative '../util' + +module OpenTelemetry + module Instrumentation + module Rack + module Middlewares + # OTel Rack Metrics Event Handler + # + # @see Rack::Events + class MetricsEventHandler + include ::Rack::Events::Abstract + + OTEL_METRICS = 'otel.rack.metrics' + + def on_start(request, _) + request.env[OTEL_METRICS] = { start_time: monotonic_time_now_nano } + rescue StandardError => e + OpenTelemetry.handle_error(exception: e) + end + + def on_error(request, _, error) + request.env[OTEL_METRICS][:error] = error.class.to_s + rescue StandardError => e + OpenTelemetry.handle_error(exception: e) + end + + def on_finish(request, response) + record_http_server_request_duration_metric(request, response) + rescue StandardError => e + OpenTelemetry.handle_error(exception: e) + end + + private + + def meter + OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.meter + end + + def http_server_request_duration_histogram + @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(request, response) + metrics_env = request.env[OTEL_METRICS] + duration = (monotonic_time_now_nano - metrics_env[:start_time]) / Float(10**9) + attrs = request_metric_attributes(request.env) + attrs['error.type'] = metrics_env[:error] if metrics_env[:error] + attrs['http.response.status.code'] = response.status + + http_server_request_duration_histogram.record(duration, attributes: attrs) + end + + def monotonic_time_now_nano + Process.clock_gettime(Process::CLOCK_MONOTONIC, :nanosecond) + end + + def request_metric_attributes(env) + { + 'http.method' => env['REQUEST_METHOD'], + 'http.host' => env['HTTP_HOST'] || 'unknown', + 'http.scheme' => env['rack.url_scheme'], + 'http.route' => "#{env['PATH_INFO']}#{('?' + env['QUERY_STRING']) unless env['QUERY_STRING'].empty?}" + } + end + end + end + end + end +end diff --git a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/metric_event_handler_test.rb b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/metric_event_handler_test.rb new file mode 100644 index 0000000000..07a470ca71 --- /dev/null +++ b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/metric_event_handler_test.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require_relative '../../../../test_helper' +require_relative '../../../../../lib/opentelemetry/instrumentation/rack' +require_relative '../../../../../lib/opentelemetry/instrumentation/rack/instrumentation' +require_relative '../../../../../lib/opentelemetry/instrumentation/rack/middlewares/metrics_event_handler' + +# test command: +# be appraisal rack-latest ruby test/opentelemetry/instrumentation/rack/middlewares/metric_event_handler_test.rb +describe 'OpenTelemetry::Instrumentation::Rack::Middlewares::MetricsEventHandler' do + include Rack::Test::Methods + + let(:instrumentation_module) { OpenTelemetry::Instrumentation::Rack } + let(:instrumentation_class) { instrumentation_module::Instrumentation } + let(:instrumentation) { instrumentation_class.instance } + let(:send_metrics) { true } + let(:config) do + { + send_metrics: send_metrics, + use_rack_events: true + } + end + + let(:handler) do + OpenTelemetry::Instrumentation::Rack::Middlewares::MetricsEventHandler.new + end + + let(:exporter) { METRICS_EXPORTER } + + let(:last_snapshot) do + exporter.pull + exporter.metric_snapshots + end + + let(:after_close) { nil } + let(:response_body) { Rack::BodyProxy.new(['Hello World']) { after_close&.call } } + let(:service) do + ->(_arg) { [200, { 'Content-Type' => 'text/plain' }, response_body] } + end + + let(:app) do + Rack::Builder.new.tap do |builder| + builder.use Rack::Events, [handler] + builder.run service + end + end + + let(:uri) { '/' } + let(:headers) { {} } + + before do + exporter.reset + instrumentation.instance_variable_set(:@installed, false) + # TODO: fix this so we don't have to force metrics to be enabled + instrumentation.instance_variable_set(:@metrics_enabled, true) + instrumentation.install(config) + end + + describe '#call' do + before do + get uri, {}, headers + end + + it 'records a metric' do + metric = last_snapshot[0][0] + assert_instance_of OpenTelemetry::SDK::Metrics::State::MetricData, metric + assert_equal metric.name, 'http.server.request.duration' + assert_equal metric.description, 'Duration of HTTP server requests.' + assert_equal metric.unit, 's' + assert_equal metric.instrument_kind, :histogram + assert_equal metric.data_points[0].attributes, { 'http.method' => 'GET', 'http.host' => 'example.org', 'http.scheme' => 'http', 'http.route' => '/', 'http.response.status.code' => 200 } + # assert_equal metric.data_points[0].sum?, expected # to check the duration + end + + # it 'records an error class if raised' {} + # it 'creates the right histogram' {} + # it 'assigns the right attributes' {} + # it 'does not record a metric if send_metrics is false' {} + # # do we need a totally separate testing environment for metrics so that the + # # traces tests do not run with the metrics sdk and api enabled? + # it 'rescues errors raised by OTel on_start' {} + # it 'rescues errors raised by OTel on_error' {} + # it 'rescues errors raised by OTel on_finish' {} + # it 'preserves the :start_time in the rack environment?' {} + # it 'includes a query string where present' {} + # it 'does not include the question mark if the query string is blank' {} + # it 'has a valid duration recorded for the value' {} + # it 'records data points for multiple requests' {} + # it 'creates the instrument only once' {} + end +end diff --git a/instrumentation/rack/test/test_helper.rb b/instrumentation/rack/test/test_helper.rb index 111b030572..3b79427965 100644 --- a/instrumentation/rack/test/test_helper.rb +++ b/instrumentation/rack/test/test_helper.rb @@ -17,6 +17,17 @@ EXPORTER = OpenTelemetry::SDK::Trace::Export::InMemorySpanExporter.new span_processor = OpenTelemetry::SDK::Trace::Export::SimpleSpanProcessor.new(EXPORTER) +METRICS_EXPORTER = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new + +module MetricsPatch + def metrics_configuration_hook + OpenTelemetry.meter_provider = OpenTelemetry::SDK::Metrics::MeterProvider.new(resource: @resource) + OpenTelemetry.meter_provider.add_metric_reader(METRICS_EXPORTER) + end +end + +OpenTelemetry::SDK::Configurator.prepend(MetricsPatch) + OpenTelemetry::SDK.configure do |c| c.error_handler = ->(exception:, message:) { raise(exception || message) } c.logger = Logger.new($stderr, level: ENV.fetch('OTEL_LOG_LEVEL', 'fatal').to_sym)