From 76a5e610f9fc2e450534aff851d05c5ba4f18d10 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Wed, 5 Nov 2025 23:40:01 -0500 Subject: [PATCH 1/7] feat: init push for server and client http metrics --- .../lib/opentelemetry/instrumentation/base.rb | 44 ++++++++++++++++- .../net/http/instrumentation.rb | 12 +++++ .../instrumentation/net/http/metrics.rb | 48 +++++++++++++++++++ .../instrumentation/rack/instrumentation.rb | 11 +++++ .../rack/middlewares/stable/event_handler.rb | 4 ++ .../middlewares/stable/tracer_middleware.rb | 3 ++ 6 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/metrics.rb diff --git a/instrumentation/base/lib/opentelemetry/instrumentation/base.rb b/instrumentation/base/lib/opentelemetry/instrumentation/base.rb index 25e9c5e0a8..f09beee211 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,7 @@ def initialize(name, version, install_blk, present_blk, @installed = false @options = options @tracer = OpenTelemetry::Trace::Tracer.new + @meter = nil end # rubocop:enable Metrics/ParameterLists @@ -222,6 +223,18 @@ def install(config = {}) instance_exec(@config, &@install_blk) @tracer = OpenTelemetry.tracer_provider.tracer(name, version) @installed = true + + if enable_metrics?(config) + if defined?(OpenTelemetry.meter_provider) && OpenTelemetry.meter_provider.instance_of?(OpenTelemetry::SDK::Metrics::MeterProvider) + @meter = OpenTelemetry.meter_provider.meter(name, version: version) + end + end + + initialize_metrics + end + + def initialize_metrics + OpenTelemetry.logger.debug "Instrumentation should implement the function" end # Whether or not this instrumentation is installable in the current process. Will @@ -261,6 +274,18 @@ def enabled?(config = nil) true end + # Whether this instrumentation has metrics is enabled. It first checks to see if it's enabled + # by an environment variable and will proceed to check if it's enabled + # by local config, if given. By default, it will be disabled. + # + # @param [optional Hash] config The local config + def enable_metrics?(config = nil) + return true if enabled_metrics_by_env_var? + return config[:metrics] if config&.key?(:metrics) + + false + end + private # The config_options method is responsible for validating that the user supplied @@ -326,6 +351,23 @@ def enabled_by_env_var? ENV[var_name] != 'false' end + + # Checks to see if this instrumentation metrics is enabled by env var. By convention, the + # environment variable will be the instrumentation name upper cased, with '::' + # replaced by underscores, OPENTELEMETRY shortened to OTEL_{LANG} and _ENABLED appended. + # For example, the, environment variable name for OpenTelemetry::Instrumentation::Sinatra + # will be OTEL_RUBY_INSTRUMENTATION_SINATRA_METRICS_ENABLED. A value of 'true' will enable + # the metrics recording in instrumentation, all other values will disable it. + def enabled_metrics_by_env_var? + var_name = name.dup.tap do |n| + n.upcase! + n.gsub!('::', '_') + n.gsub!('OPENTELEMETRY_', 'OTEL_RUBY_') + n << '_METRICS_ENABLED' + end + ENV[var_name] == 'true' + end + # Checks to see if the user has passed any environment variables that set options # for instrumentation. By convention, the environment variable will be the name # of the instrumentation, uppercased, with '::' replaced by underscores, diff --git a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/instrumentation.rb b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/instrumentation.rb index 1c145a0124..9502f5fb2c 100644 --- a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/instrumentation.rb +++ b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/instrumentation.rb @@ -4,6 +4,8 @@ # # SPDX-License-Identifier: Apache-2.0 +require 'opentelemetry/semconv/http' + module OpenTelemetry module Instrumentation module Net @@ -53,6 +55,7 @@ def require_dependencies_old end def require_dependencies_stable + require_relative 'metrics' require_relative 'patches/stable/instrumentation' end @@ -65,8 +68,17 @@ def patch_old end def patch_stable + ::Net::HTTP.prepend(Metrics) ::Net::HTTP.prepend(Patches::Stable::Instrumentation) end + + def initialize_metrics + return if meter.nil? + + config[:client_request_duration] = meter.create_histogram(::OpenTelemetry::SemConv::HTTP::HTTP_CLIENT_REQUEST_DURATION, + unit: 'ms', + description: 'Duration of HTTP client requests.') + end end end end diff --git a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/metrics.rb b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/metrics.rb new file mode 100644 index 0000000000..f9de17162c --- /dev/null +++ b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/metrics.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Instrumentation + module Net + module HTTP + module Metrics + def request(req, body = nil, &block) + return super unless started? + return super if untraced? + + start_time = (Time.now.to_f * 1000).to_i + error_occurred = false + + begin + super + rescue => e + error_occurred = true + raise + ensure + duration_ms = (Time.now.to_f * 1000).to_i - start_time + record_metric(duration_ms, req, error_occurred) + end + end + + private + + def record_metric(duration_ms, req, error_occurred) + instrumentation = ::OpenTelemetry::Instrumentation::Net::HTTP::Instrumentation.instance + return unless instrumentation + + instrumentation.config[:client_request_duration]&.record(duration_ms) + rescue => e + OpenTelemetry.handle_error(exception: e) + end + + def untraced? + OpenTelemetry::Common::Utilities.untraced? + end + end + end + end + end +end diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb index 2089db4284..7b872d7015 100644 --- a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb @@ -5,6 +5,7 @@ # SPDX-License-Identifier: Apache-2.0 require 'opentelemetry' +require 'opentelemetry/semconv/http' module OpenTelemetry module Instrumentation @@ -63,8 +64,10 @@ def middleware_args_dup def middleware_args_stable if config.fetch(:use_rack_events, false) == true && defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler) + puts 'EventHandler' [::Rack::Events, [OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler.new]] else + puts 'TracerMiddleware' [OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::TracerMiddleware] end end @@ -123,6 +126,14 @@ def config_options(user_config) def build_attribute_name(prefix, suffix) prefix + suffix.to_s.downcase.gsub(/[-\s]/, '_') end + + def initialize_metrics + return if meter.nil? + + config[:server_request_duration] = meter.create_histogram(::OpenTelemetry::SemConv::HTTP::HTTP_SERVER_REQUEST_DURATION, + unit: 'ms', + description: 'Duration of HTTP server requests.') + end end end end diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/event_handler.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/event_handler.rb index 3998aa504a..70b6a1be40 100644 --- a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/event_handler.rb +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/event_handler.rb @@ -44,6 +44,7 @@ class EventHandler include ::Rack::Events::Abstract OTEL_TOKEN_AND_SPAN = 'otel.rack.token_and_span' + OTEL_SERVER_START_TIME = 'otel.rack.server.start_time' EMPTY_HASH = {}.freeze # Creates a server span for this current request using the incoming parent context @@ -63,6 +64,7 @@ def on_start(request, _) span_ctx = OpenTelemetry::Trace.context_with_span(span, parent_context: parent_context) rack_ctx = OpenTelemetry::Instrumentation::Rack.context_with_span(span, parent_context: span_ctx) request.env[OTEL_TOKEN_AND_SPAN] = [OpenTelemetry::Context.attach(rack_ctx), span] + request.env[OTEL_SERVER_START_TIME] = (Time.now.to_f * 1000).to_i rescue StandardError => e OpenTelemetry.handle_error(exception: e) end @@ -114,6 +116,8 @@ def on_finish(request, response) rescue StandardError => e OpenTelemetry.handle_error(exception: e) ensure + duration_ms = (Time.now.to_f * 1000).to_i - request.env[OTEL_SERVER_START_TIME] + config[:server_request_duration]&.record(duration_ms) detach_context(request) end diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/tracer_middleware.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/tracer_middleware.rb index f14e819fcc..6ab972fe59 100644 --- a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/tracer_middleware.rb +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/tracer_middleware.rb @@ -58,6 +58,7 @@ def initialize(app) end def call(env) + start_time = (Time.now.to_f * 1000).to_i if untraced_request?(env) OpenTelemetry::Common::Utilities.untraced do return @app.call(env) @@ -89,6 +90,8 @@ def call(env) end end ensure + duration_ms = (Time.now.to_f * 1000).to_i - start_time + config[:server_request_duration]&.record(duration_ms) finish_span(frontend_context) end From db13b1ef4979827d092d980a6c276da28805d2cd Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Thu, 6 Nov 2025 13:22:15 -0500 Subject: [PATCH 2/7] try not to modify original code --- .../lib/opentelemetry/instrumentation/base.rb | 6 +- .../instrumentation/net/http/metrics.rb | 33 ++++++---- .../instrumentation/rack/instrumentation.rb | 18 ++++-- .../middlewares/event_handler_with_metrics.rb | 62 +++++++++++++++++++ .../rack/middlewares/stable/event_handler.rb | 4 -- .../middlewares/stable/tracer_middleware.rb | 3 - .../tracer_middleware_with_metrics.rb | 55 ++++++++++++++++ 7 files changed, 155 insertions(+), 26 deletions(-) create mode 100644 instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics.rb create mode 100644 instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics.rb diff --git a/instrumentation/base/lib/opentelemetry/instrumentation/base.rb b/instrumentation/base/lib/opentelemetry/instrumentation/base.rb index f09beee211..6a7edc79fc 100644 --- a/instrumentation/base/lib/opentelemetry/instrumentation/base.rb +++ b/instrumentation/base/lib/opentelemetry/instrumentation/base.rb @@ -233,8 +233,10 @@ def install(config = {}) initialize_metrics end + # Each instrumentation can implement the initialize_metrics function to + # include the desired metrics instrument def initialize_metrics - OpenTelemetry.logger.debug "Instrumentation should implement the function" + OpenTelemetry.logger.info "Instrumentation should implement the function" end # Whether or not this instrumentation is installable in the current process. Will @@ -329,7 +331,7 @@ def config_options(user_config) h[option_name] = option[:default] end - dropped_config_keys = user_config.keys - validated_config.keys - [:enabled] + dropped_config_keys = user_config.keys - validated_config.keys - [:enabled, :metrics] OpenTelemetry.logger.warn("Instrumentation #{name} ignored the following unknown configuration options #{dropped_config_keys}") unless dropped_config_keys.empty? validated_config diff --git a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/metrics.rb b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/metrics.rb index f9de17162c..4298079605 100644 --- a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/metrics.rb +++ b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/metrics.rb @@ -10,26 +10,35 @@ module Net module HTTP module Metrics def request(req, body = nil, &block) - return super unless started? - return super if untraced? + with_metric_timing { super } + end + + private + + def connect + with_metric_timing { super } + end + + def with_metric_timing + return yield unless started? + return yield if untraced? - start_time = (Time.now.to_f * 1000).to_i - error_occurred = false + start_time = current_time_ms begin - super - rescue => e - error_occurred = true + yield + rescue raise - ensure - duration_ms = (Time.now.to_f * 1000).to_i - start_time - record_metric(duration_ms, req, error_occurred) end + ensure + record_metric(current_time_ms - start_time) if start_time end - private + def current_time_ms + (Time.now.to_f * 1000).to_i + end - def record_metric(duration_ms, req, error_occurred) + def record_metric(duration_ms) instrumentation = ::OpenTelemetry::Instrumentation::Net::HTTP::Instrumentation.instance return unless instrumentation diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb index 7b872d7015..ea680ed80c 100644 --- a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb @@ -63,12 +63,18 @@ def middleware_args_dup end def middleware_args_stable - if config.fetch(:use_rack_events, false) == true && defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler) - puts 'EventHandler' - [::Rack::Events, [OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler.new]] + if config.fetch(:use_rack_events, false) == true \ + && defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler) \ + && defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandlerWithMetrics) + [ + ::Rack::Events, + [ + OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler.new, + OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandlerWithMetrics.new + ] + ] else - puts 'TracerMiddleware' - [OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::TracerMiddleware] + [OpenTelemetry::Instrumentation::Rack::Middlewares::TracerMiddlewareWithMetrics] end end @@ -94,7 +100,9 @@ def require_dependencies_old def require_dependencies_stable require_relative 'middlewares/stable/event_handler' if defined?(::Rack::Events) + require_relative 'middlewares/event_handler_with_metrics' if defined?(::Rack::Events) require_relative 'middlewares/stable/tracer_middleware' + require_relative 'middlewares/tracer_middleware_with_metrics' end def require_dependencies_dup diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics.rb new file mode 100644 index 0000000000..07cbc4e51d --- /dev/null +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Instrumentation + module Rack + module Middlewares + # EventHandler that only records metrics + # This handler is designed to be used alongside the original EventHandler + # in the Rack::Events middleware stack + class EventHandlerWithMetrics + include ::Rack::Events::Abstract + + OTEL_SERVER_START_TIME = 'otel.rack.server.start_time' + + def on_start(request, _) + request.env[OTEL_SERVER_START_TIME] = current_time_ms + rescue StandardError => e + OpenTelemetry.handle_error(exception: e) + end + + def on_commit(_request, _response) + # No-op: metrics are recorded in on_finish + end + + def on_error(_request, _response, _error) + # No-op: metrics are recorded in on_finish + end + + def on_finish(request, _response) + record_metric(request) + rescue StandardError => e + OpenTelemetry.handle_error(exception: e) + end + + private + + def current_time_ms + (Time.now.to_f * 1000).to_i + end + + def record_metric(request) + start_time = request.env[OTEL_SERVER_START_TIME] + return unless start_time + + duration_ms = current_time_ms - start_time + config[:server_request_duration]&.record(duration_ms) + rescue StandardError => e + OpenTelemetry.handle_error(exception: e) + end + + def config + OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.config + end + end + end + end + end +end diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/event_handler.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/event_handler.rb index 70b6a1be40..3998aa504a 100644 --- a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/event_handler.rb +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/event_handler.rb @@ -44,7 +44,6 @@ class EventHandler include ::Rack::Events::Abstract OTEL_TOKEN_AND_SPAN = 'otel.rack.token_and_span' - OTEL_SERVER_START_TIME = 'otel.rack.server.start_time' EMPTY_HASH = {}.freeze # Creates a server span for this current request using the incoming parent context @@ -64,7 +63,6 @@ def on_start(request, _) span_ctx = OpenTelemetry::Trace.context_with_span(span, parent_context: parent_context) rack_ctx = OpenTelemetry::Instrumentation::Rack.context_with_span(span, parent_context: span_ctx) request.env[OTEL_TOKEN_AND_SPAN] = [OpenTelemetry::Context.attach(rack_ctx), span] - request.env[OTEL_SERVER_START_TIME] = (Time.now.to_f * 1000).to_i rescue StandardError => e OpenTelemetry.handle_error(exception: e) end @@ -116,8 +114,6 @@ def on_finish(request, response) rescue StandardError => e OpenTelemetry.handle_error(exception: e) ensure - duration_ms = (Time.now.to_f * 1000).to_i - request.env[OTEL_SERVER_START_TIME] - config[:server_request_duration]&.record(duration_ms) detach_context(request) end diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/tracer_middleware.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/tracer_middleware.rb index 6ab972fe59..f14e819fcc 100644 --- a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/tracer_middleware.rb +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/stable/tracer_middleware.rb @@ -58,7 +58,6 @@ def initialize(app) end def call(env) - start_time = (Time.now.to_f * 1000).to_i if untraced_request?(env) OpenTelemetry::Common::Utilities.untraced do return @app.call(env) @@ -90,8 +89,6 @@ def call(env) end end ensure - duration_ms = (Time.now.to_f * 1000).to_i - start_time - config[:server_request_duration]&.record(duration_ms) finish_span(frontend_context) end diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics.rb new file mode 100644 index 0000000000..a4503ab5e8 --- /dev/null +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require_relative 'stable/tracer_middleware' + +module OpenTelemetry + module Instrumentation + module Rack + module Middlewares + # Middleware that wraps TracerMiddleware and adds metrics recording + # This middleware is designed to be used as a replacement for TracerMiddleware + # It delegates all tracing work to the original TracerMiddleware + class TracerMiddlewareWithMetrics + def initialize(app) + @app = app + # Create the original tracer middleware wrapping our app + @tracer_middleware = Stable::TracerMiddleware.new(@app) + end + + def call(env) + start_time = current_time_ms + + begin + @tracer_middleware.call(env) + ensure + record_metric(start_time) + end + end + + private + + def current_time_ms + (Time.now.to_f * 1000).to_i + end + + def record_metric(start_time) + return unless start_time + + duration_ms = current_time_ms - start_time + config[:server_request_duration]&.record(duration_ms) + rescue StandardError => e + OpenTelemetry.handle_error(exception: e) + end + + def config + OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.config + end + end + end + end + end +end From 7b101ddca4b1e3cf1064f56ee2ba6fb857f940e4 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Mon, 10 Nov 2025 12:35:03 -0500 Subject: [PATCH 3/7] update base test case --- .../lib/opentelemetry/instrumentation/base.rb | 27 +- .../base/test/instrumentation/base_test.rb | 360 ++++++++++++++++++ instrumentation/base/test/test_helper.rb | 72 ++++ instrumentation/net_http/Gemfile | 1 + .../net/http/instrumentation.rb | 4 +- .../instrumentation/net/http/metrics.rb | 11 +- .../net/http/stable/metrics_test.rb | 291 ++++++++++++++ instrumentation/net_http/test/test_helper.rb | 3 + instrumentation/rack/Gemfile | 1 + .../instrumentation/rack/instrumentation.rb | 3 + .../rack/instrumentation_stable_test.rb | 37 +- .../event_handler_with_metrics_test.rb | 214 +++++++++++ .../tracer_middleware_with_metrics_test.rb | 224 +++++++++++ 13 files changed, 1220 insertions(+), 28 deletions(-) create mode 100644 instrumentation/net_http/test/opentelemetry/instrumentation/net/http/stable/metrics_test.rb create mode 100644 instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics_test.rb create mode 100644 instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics_test.rb diff --git a/instrumentation/base/lib/opentelemetry/instrumentation/base.rb b/instrumentation/base/lib/opentelemetry/instrumentation/base.rb index 6a7edc79fc..0caa219d5b 100644 --- a/instrumentation/base/lib/opentelemetry/instrumentation/base.rb +++ b/instrumentation/base/lib/opentelemetry/instrumentation/base.rb @@ -224,10 +224,8 @@ def install(config = {}) @tracer = OpenTelemetry.tracer_provider.tracer(name, version) @installed = true - if enable_metrics?(config) - if defined?(OpenTelemetry.meter_provider) && OpenTelemetry.meter_provider.instance_of?(OpenTelemetry::SDK::Metrics::MeterProvider) - @meter = OpenTelemetry.meter_provider.meter(name, version: version) - end + if enable_metrics?(config) && defined?(OpenTelemetry.meter_provider) && OpenTelemetry.meter_provider.instance_of?(OpenTelemetry::SDK::Metrics::MeterProvider) + @meter = OpenTelemetry.meter_provider.meter(name, version: version) end initialize_metrics @@ -236,7 +234,7 @@ def install(config = {}) # Each instrumentation can implement the initialize_metrics function to # include the desired metrics instrument def initialize_metrics - OpenTelemetry.logger.info "Instrumentation should implement the function" + OpenTelemetry.logger.info 'Instrumentation should implement the function' end # Whether or not this instrumentation is installable in the current process. Will @@ -282,7 +280,8 @@ def enabled?(config = nil) # # @param [optional Hash] config The local config def enable_metrics?(config = nil) - return true if enabled_metrics_by_env_var? + env_value = metrics_env_var_value + return env_value == 'true' if %w[true false].include?(env_value) return config[:metrics] if config&.key?(:metrics) false @@ -331,7 +330,7 @@ def config_options(user_config) h[option_name] = option[:default] end - dropped_config_keys = user_config.keys - validated_config.keys - [:enabled, :metrics] + dropped_config_keys = user_config.keys - validated_config.keys - %i[enabled metrics] OpenTelemetry.logger.warn("Instrumentation #{name} ignored the following unknown configuration options #{dropped_config_keys}") unless dropped_config_keys.empty? validated_config @@ -353,21 +352,19 @@ def enabled_by_env_var? ENV[var_name] != 'false' end - - # Checks to see if this instrumentation metrics is enabled by env var. By convention, the + # Returns the value of the metrics environment variable. By convention, the # environment variable will be the instrumentation name upper cased, with '::' - # replaced by underscores, OPENTELEMETRY shortened to OTEL_{LANG} and _ENABLED appended. - # For example, the, environment variable name for OpenTelemetry::Instrumentation::Sinatra - # will be OTEL_RUBY_INSTRUMENTATION_SINATRA_METRICS_ENABLED. A value of 'true' will enable - # the metrics recording in instrumentation, all other values will disable it. - def enabled_metrics_by_env_var? + # replaced by underscores, OPENTELEMETRY shortened to OTEL_{LANG} and _METRICS_ENABLED appended. + # For example, the environment variable name for OpenTelemetry::Instrumentation::Sinatra + # will be OTEL_RUBY_INSTRUMENTATION_SINATRA_METRICS_ENABLED. + def metrics_env_var_value var_name = name.dup.tap do |n| n.upcase! n.gsub!('::', '_') n.gsub!('OPENTELEMETRY_', 'OTEL_RUBY_') n << '_METRICS_ENABLED' end - ENV[var_name] == 'true' + ENV.fetch(var_name, nil) end # Checks to see if the user has passed any environment variables that set options diff --git a/instrumentation/base/test/instrumentation/base_test.rb b/instrumentation/base/test/instrumentation/base_test.rb index c58bbe5003..d67d0f39c3 100644 --- a/instrumentation/base/test/instrumentation/base_test.rb +++ b/instrumentation/base/test/instrumentation/base_test.rb @@ -442,6 +442,366 @@ def initialize(*args) end end + describe 'metrics support' do + let(:meter_enabled_instrumentation) do + Class.new(OpenTelemetry::Instrumentation::Base) do + instrumentation_name 'test_meter_instrumentation' + instrumentation_version '0.1.0' + + present { true } + compatible { true } + install { true } + end + end + + describe '#enable_metrics?' do + it 'is enabled when METRICS_ENABLED env var is true' do + OpenTelemetry::TestHelpers.with_env('TEST_INSTRUMENTATION_METRICS_ENABLED' => 'true') do + _(instrumentation.instance.enable_metrics?).must_equal(true) + end + end + + it 'is disabled when METRICS_ENABLED env var is false' do + OpenTelemetry::TestHelpers.with_env('TEST_INSTRUMENTATION_METRICS_ENABLED' => 'false') do + _(instrumentation.instance.enable_metrics?).must_equal(false) + end + end + + it 'is disabled when METRICS_ENABLED env var is not set' do + _(instrumentation.instance.enable_metrics?).must_equal(false) + end + + it 'env var true overrides local config false' do + OpenTelemetry::TestHelpers.with_env('TEST_INSTRUMENTATION_METRICS_ENABLED' => 'true') do + _(instrumentation.instance.enable_metrics?(metrics: false)).must_equal(true) + end + end + + it 'env var false overrides local config true' do + OpenTelemetry::TestHelpers.with_env('TEST_INSTRUMENTATION_METRICS_ENABLED' => 'false') do + _(instrumentation.instance.enable_metrics?(metrics: true)).must_equal(false) + end + end + + it 'is enabled when metrics config is true and env var is not set' do + _(instrumentation.instance.enable_metrics?(metrics: true)).must_equal(true) + end + + it 'is disabled when metrics config is false and env var is not set' do + _(instrumentation.instance.enable_metrics?(metrics: false)).must_equal(false) + end + end + + describe '#meter' do + it 'returns nil if not installed' do + _(meter_enabled_instrumentation.instance.meter).must_be_nil + end + + it 'returns nil when metrics are disabled (default)' do + instance = meter_enabled_instrumentation.instance + instance.install + _(instance.meter).must_be_nil + end + + it 'returns nil when metrics config is explicitly false' do + instance = meter_enabled_instrumentation.instance + instance.install(metrics: false) + _(instance.meter).must_be_nil + end + + it 'returns a meter when metrics are enabled and meter provider is available' do + instance = meter_enabled_instrumentation.instance + + meter_provider = OpenTelemetry::SDK::Metrics::MeterProvider.new + OpenTelemetry.stub(:meter_provider, meter_provider) do + instance.instance_variable_set(:@installed, false) + instance.install(metrics: true) + + _(instance.meter).wont_be_nil + _(instance.meter).must_be_kind_of(OpenTelemetry::SDK::Metrics::Meter) + _(instance.meter.name).must_equal('test_meter_instrumentation') + _(instance.meter.instrumentation_version).must_equal('0.1.0') + end + end + + it 'returns nil when meter provider is not an SDK MeterProvider' do + instance = meter_enabled_instrumentation.instance + + fake_meter_provider = Object.new + OpenTelemetry.stub(:meter_provider, fake_meter_provider) do + instance.instance_variable_set(:@installed, false) + instance.install(metrics: true) + _(instance.meter).must_be_nil + end + end + end + end + + describe '#initialize_metrics' do + let(:metrics_instrumentation) do + Class.new(OpenTelemetry::Instrumentation::Base) do + attr_reader :initialize_metrics_called, :meter_at_init + + instrumentation_name 'test_metrics_instrumentation' + instrumentation_version '0.2.0' + + present { true } + compatible { true } + install { true } + + def initialize(*args) + super + @initialize_metrics_called = false + end + + def initialize_metrics + @initialize_metrics_called = true + @meter_at_init = meter + end + end + end + + it 'is called during installation' do + instance = metrics_instrumentation.instance + _(instance.initialize_metrics_called).must_equal(false) + + instance.install + _(instance.initialize_metrics_called).must_equal(true) + end + + it 'is called after meter is set when metrics are enabled' do + instance = metrics_instrumentation.instance + + meter_provider = OpenTelemetry::SDK::Metrics::MeterProvider.new + OpenTelemetry.stub(:meter_provider, meter_provider) do + instance.install(metrics: true) + + _(instance.initialize_metrics_called).must_equal(true) + _(instance.meter_at_init).wont_be_nil + _(instance.meter_at_init).must_be_kind_of(OpenTelemetry::SDK::Metrics::Meter) + end + end + + it 'is called even when metrics are disabled' do + instance = metrics_instrumentation.instance + instance.install(metrics: false) + + _(instance.initialize_metrics_called).must_equal(true) + _(instance.meter_at_init).must_be_nil + end + + it 'has access to config during initialization' do + config_aware_instrumentation = Class.new(OpenTelemetry::Instrumentation::Base) do + attr_reader :config_at_init + + instrumentation_name 'test_config_aware_instrumentation' + instrumentation_version '0.3.0' + + present { true } + compatible { true } + install { true } + + option :test_option, default: 'default_value', validate: :string + + def initialize_metrics + @config_at_init = config.dup + end + end + + instance = config_aware_instrumentation.instance + instance.install(test_option: 'custom_value') + + _(instance.config_at_init).must_equal(test_option: 'custom_value') + end + + it 'does not raise when initialize_metrics raises an error' do + error_instrumentation = Class.new(OpenTelemetry::Instrumentation::Base) do + instrumentation_name 'test_error_instrumentation' + instrumentation_version '0.4.0' + + present { true } + compatible { true } + install { true } + + def initialize_metrics + raise StandardError, 'Metrics initialization error' + end + end + + instance = error_instrumentation.instance + + # Should not raise during install + _(-> { instance.install }).must_raise(StandardError) + end + end + + describe 'metrics integration tests' do + let(:full_metrics_instrumentation) do + Class.new(OpenTelemetry::Instrumentation::Base) do + attr_reader :histogram, :counter + + instrumentation_name 'opentelemetry_instrumentation_full_metrics' + instrumentation_version '1.0.0' + + present { true } + compatible { true } + install { true } + + def initialize_metrics + return unless meter + + @histogram = meter.create_histogram('test.histogram', unit: 'ms', description: 'Test histogram') + @counter = meter.create_counter('test.counter', unit: '1', description: 'Test counter') + end + end + end + + it 'creates metrics instruments when metrics are enabled' do + instance = full_metrics_instrumentation.instance + + meter_provider = OpenTelemetry::SDK::Metrics::MeterProvider.new + OpenTelemetry.stub(:meter_provider, meter_provider) do + instance.install(metrics: true) + + _(instance.histogram).wont_be_nil + _(instance.counter).wont_be_nil + _(instance.histogram).must_respond_to(:record) + _(instance.counter).must_respond_to(:add) + end + end + + it 'does not create metrics instruments when metrics are disabled by default' do + instance = full_metrics_instrumentation.instance + instance.install + + _(instance.meter).must_be_nil + _(instance.histogram).must_be_nil + _(instance.counter).must_be_nil + end + + it 'does not create metrics instruments when metrics config is false' do + instance = full_metrics_instrumentation.instance + instance.install(metrics: false) + + _(instance.histogram).must_be_nil + _(instance.counter).must_be_nil + end + + it 'enables metrics via environment variable (overriding default)' do + instance = full_metrics_instrumentation.instance + + OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_FULL_METRICS_METRICS_ENABLED' => 'true') do + meter_provider = OpenTelemetry::SDK::Metrics::MeterProvider.new + OpenTelemetry.stub(:meter_provider, meter_provider) do + instance.instance_variable_set(:@installed, false) + instance.install + + _(instance.meter).wont_be_nil + _(instance.histogram).wont_be_nil + _(instance.counter).wont_be_nil + end + end + end + + it 'enables metrics via environment variable (overriding local config false)' do + instance = full_metrics_instrumentation.instance + + OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_FULL_METRICS_METRICS_ENABLED' => 'true') do + meter_provider = OpenTelemetry::SDK::Metrics::MeterProvider.new + OpenTelemetry.stub(:meter_provider, meter_provider) do + instance.instance_variable_set(:@installed, false) + instance.install(metrics: false) + + _(instance.meter).wont_be_nil + end + end + end + + it 'env var false overrides local config true' do + instance = full_metrics_instrumentation.instance + + OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_FULL_METRICS_METRICS_ENABLED' => 'false') do + meter_provider = OpenTelemetry::SDK::Metrics::MeterProvider.new + OpenTelemetry.stub(:meter_provider, meter_provider) do + instance.instance_variable_set(:@installed, false) + instance.install(metrics: true) + + _(instance.meter).must_be_nil + end + end + end + + it 'enables metrics via local config when environment variable is not set' do + instance = full_metrics_instrumentation.instance + + meter_provider = OpenTelemetry::SDK::Metrics::MeterProvider.new + OpenTelemetry.stub(:meter_provider, meter_provider) do + instance.instance_variable_set(:@installed, false) + instance.install(metrics: true) + + _(instance.meter).wont_be_nil + end + end + end + + describe 'metrics with namespaced instrumentation' do + before do + module MetricsTest + module Instrumentation + module Custom + VERSION = '1.2.3' + + class Instrumentation < OpenTelemetry::Instrumentation::Base + attr_reader :test_metric + + present { true } + compatible { true } + install { true } + + def initialize_metrics + return unless meter + + @test_metric = meter.create_histogram('custom.metric', unit: 's', description: 'Custom metric') + end + end + end + end + end + end + + after do + Object.send(:remove_const, :MetricsTest) + end + + it 'uses correct environment variable name for namespaced instrumentation' do + instance = MetricsTest::Instrumentation::Custom::Instrumentation.instance + + # The env var name is based on the full namespace: MetricsTest::Instrumentation::Custom + # After transformation: METRICSTEST_INSTRUMENTATION_CUSTOM_METRICS_ENABLED + OpenTelemetry::TestHelpers.with_env('METRICSTEST_INSTRUMENTATION_CUSTOM_METRICS_ENABLED' => 'true') do + meter_provider = OpenTelemetry::SDK::Metrics::MeterProvider.new + + OpenTelemetry.stub(:meter_provider, meter_provider) do + instance.install + + _(instance.meter).wont_be_nil + _(instance.test_metric).wont_be_nil + end + end + end + + it 'does not enable metrics with incorrect environment variable' do + instance = MetricsTest::Instrumentation::Custom::Instrumentation.instance + + OpenTelemetry::TestHelpers.with_env('OTEL_RUBY_INSTRUMENTATION_WRONG_NAME_METRICS_ENABLED' => 'true') do + instance.install + + _(instance.meter).must_be_nil + _(instance.test_metric).must_be_nil + 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 6e20a575a3..f9b60d6067 100644 --- a/instrumentation/base/test/test_helper.rb +++ b/instrumentation/base/test/test_helper.rb @@ -12,3 +12,75 @@ require 'minitest/autorun' OpenTelemetry.logger = Logger.new($stderr, level: ENV.fetch('OTEL_LOG_LEVEL', 'fatal').to_sym) + +# Mock OpenTelemetry::SDK::Metrics module for testing when the SDK is not available +# TODO: This should move to test-helpers gem +unless defined?(OpenTelemetry::SDK::Metrics) + module OpenTelemetry + def self.meter_provider; end + + module SDK + module Metrics + # Mock Meter class + class Meter + attr_reader :name, :instrumentation_version + + def initialize(name, version:) + @name = name + @instrumentation_version = version + end + + def create_histogram(name, unit: nil, description: nil) + MockInstrument.new(name, unit, description) + end + + def create_counter(name, unit: nil, description: nil) + MockInstrument.new(name, unit, description) + end + + def create_up_down_counter(name, unit: nil, description: nil) + MockInstrument.new(name, unit, description) + end + + def create_observable_counter(name, unit: nil, description: nil, &block) + MockInstrument.new(name, unit, description) + end + + def create_observable_gauge(name, unit: nil, description: nil, &block) + MockInstrument.new(name, unit, description) + end + + def create_observable_up_down_counter(name, unit: nil, description: nil, &block) + MockInstrument.new(name, unit, description) + end + end + + # Mock MeterProvider class + class MeterProvider + def meter(name, version: nil) + Meter.new(name, version: version) + end + end + + # Mock Instrument class for histograms, counters, etc. + class MockInstrument + attr_reader :name, :unit, :description + + def initialize(name, unit, description) + @name = name + @unit = unit + @description = description + end + + def record(value, attributes: {}) + # Mock record method + end + + def add(value, attributes: {}) + # Mock add method + end + end + end + end + end +end diff --git a/instrumentation/net_http/Gemfile b/instrumentation/net_http/Gemfile index d425915ffb..7ba40568f7 100644 --- a/instrumentation/net_http/Gemfile +++ b/instrumentation/net_http/Gemfile @@ -13,6 +13,7 @@ group :test do gem 'bundler', '~> 2.4' gem 'minitest', '~> 5.0' gem 'opentelemetry-sdk', '~> 1.1' + gem 'opentelemetry-metrics-sdk', '~> 0.11' gem 'opentelemetry-test-helpers', '~> 0.3' gem 'rake', '~> 13.0.1' gem 'rubocop', '~> 1.81.1' diff --git a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/instrumentation.rb b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/instrumentation.rb index 9502f5fb2c..77dc82fe80 100644 --- a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/instrumentation.rb +++ b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/instrumentation.rb @@ -76,8 +76,8 @@ def initialize_metrics return if meter.nil? config[:client_request_duration] = meter.create_histogram(::OpenTelemetry::SemConv::HTTP::HTTP_CLIENT_REQUEST_DURATION, - unit: 'ms', - description: 'Duration of HTTP client requests.') + unit: 'ms', + description: 'Duration of HTTP client requests.') end end end diff --git a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/metrics.rb b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/metrics.rb index 4298079605..5bffa43903 100644 --- a/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/metrics.rb +++ b/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/metrics.rb @@ -8,8 +8,9 @@ module OpenTelemetry module Instrumentation module Net module HTTP + # Metrics module for patching the instrumentation module Metrics - def request(req, body = nil, &block) + def request(req, body = nil, &) with_metric_timing { super } end @@ -25,11 +26,7 @@ def with_metric_timing start_time = current_time_ms - begin - yield - rescue - raise - end + yield ensure record_metric(current_time_ms - start_time) if start_time end @@ -43,7 +40,7 @@ def record_metric(duration_ms) return unless instrumentation instrumentation.config[:client_request_duration]&.record(duration_ms) - rescue => e + rescue StandardError => e OpenTelemetry.handle_error(exception: e) end diff --git a/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/stable/metrics_test.rb b/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/stable/metrics_test.rb new file mode 100644 index 0000000000..f0c81812b9 --- /dev/null +++ b/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/stable/metrics_test.rb @@ -0,0 +1,291 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' + +require_relative '../../../../../../lib/opentelemetry/instrumentation/net/http' +require_relative '../../../../../../lib/opentelemetry/instrumentation/net/http/patches/stable/instrumentation' + +describe OpenTelemetry::Instrumentation::Net::HTTP::Instrumentation do + let(:instrumentation) { OpenTelemetry::Instrumentation::Net::HTTP::Instrumentation.instance } + let(:exporter) { EXPORTER } + let(:span) { exporter.finished_spans.first } + # let(:metric_exporter) { METRICS_EXPORTER } + + before do + skip unless ENV['BUNDLE_GEMFILE'].include?('stable') + + @metric_exporter = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new + OpenTelemetry.meter_provider.add_metric_reader(@metric_exporter) + + ENV['OTEL_SEMCONV_STABILITY_OPT_IN'] = 'http' + exporter.reset + + stub_request(:get, 'http://example.com/success').to_return(status: 200) + stub_request(:post, 'http://example.com/failure').to_return(status: 500) + stub_request(:get, 'https://example.com/timeout').to_timeout + + # this is currently a noop but this will future proof the test + @orig_propagation = OpenTelemetry.propagation + propagator = OpenTelemetry::Trace::Propagation::TraceContext.text_map_propagator + OpenTelemetry.propagation = propagator + + # Install instrumentation with metrics enabled + instrumentation.install(metrics: true, client_request_duration: true) + end + + after do + # Force re-install of instrumentation + instrumentation.instance_variable_set(:@installed, false) + + OpenTelemetry.meter_provider.instance_variable_set(:@metric_readers, []) + + OpenTelemetry.propagation = @orig_propagation + end + + describe 'metrics integration' do + it 'records metrics alongside spans for successful requests' do + Net::HTTP.get('example.com', '/success') + + # Verify span was created + _(exporter.finished_spans.size).must_equal 1 + _(span.name).must_equal 'GET' + + # Pull and verify metrics + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + + _(metrics).wont_be_empty + duration_metric = metrics[0] + + _(duration_metric).wont_be_nil + _(duration_metric.name).must_equal 'http.client.request.duration' + _(duration_metric.description).must_equal 'Duration of HTTP client requests.' + _(duration_metric.unit).must_equal 'ms' + _(duration_metric.instrument_kind).must_equal :histogram + _(duration_metric.instrumentation_scope.name).must_equal 'OpenTelemetry::Instrumentation::Net::HTTP' + _(duration_metric.data_points).wont_be_empty + _(duration_metric.data_points.first.count).must_equal 1 + end + + it 'records metrics for requests with different status codes' do + # Test successful request + Net::HTTP.get('example.com', '/success') + + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + _(metrics).wont_be_empty + _(metrics[0].data_points.first.count).must_equal 1 + + # Test failed request + Net::HTTP.post(URI('http://example.com/failure'), 'q' => 'ruby') + + _(exporter.finished_spans.last.attributes['http.response.status_code']).must_equal 500 + + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + duration_metric = metrics[0] + + _(duration_metric).wont_be_nil + _(duration_metric.name).must_equal 'http.client.request.duration' + _(duration_metric.data_points).wont_be_empty + _(duration_metric.data_points.first.count).must_equal 1 + end + + it 'records metrics even when request times out' do + expect do + Net::HTTP.get(URI('https://example.com/timeout')) + end.must_raise Net::OpenTimeout + + # Verify span was created + _(exporter.finished_spans.size).must_equal 1 + + # Pull and verify metrics + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + + _(metrics).wont_be_empty + duration_metric = metrics[0] + + _(duration_metric).wont_be_nil + _(duration_metric.name).must_equal 'http.client.request.duration' + _(duration_metric.description).must_equal 'Duration of HTTP client requests.' + _(duration_metric.unit).must_equal 'ms' + _(duration_metric.instrument_kind).must_equal :histogram + _(duration_metric.data_points).wont_be_empty + _(duration_metric.data_points.first.count).must_equal 1 + end + + it 'records metrics for multiple requests' do + # Make multiple requests + 3.times do + Net::HTTP.get('example.com', '/success') + end + + # Pull and verify metrics + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + + _(metrics).wont_be_empty + _(exporter.finished_spans.size).must_equal 3 + + duration_metric = metrics[0] + + _(duration_metric).wont_be_nil + _(duration_metric.name).must_equal 'http.client.request.duration' + _(duration_metric.description).must_equal 'Duration of HTTP client requests.' + _(duration_metric.unit).must_equal 'ms' + _(duration_metric.instrument_kind).must_equal :histogram + _(duration_metric.data_points).wont_be_empty + _(duration_metric.data_points.first.count).must_equal 3 + end + + it 'does not record metrics in untraced context' do + OpenTelemetry::Common::Utilities.untraced do + Net::HTTP.get('example.com', '/success') + end + + # Pull metrics + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + + # Verify that no metrics were recorded in untraced context + duration_metric = metrics.find { |m| m.name == 'http.client.request.duration' } + _(duration_metric).must_be_nil + end + end + + describe '#initialize_metrics' do + let(:meter_provider) { OpenTelemetry.meter_provider } + let(:meter) { meter_provider.meter('test-meter') } + + it 'initializes client_request_duration histogram' do + instrumentation.stub(:meter, meter) do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install + end + + histogram = instrumentation.config[:client_request_duration] + _(histogram).wont_be_nil + _(histogram).must_respond_to :record + end + + it 'does not create metrics when meter is nil' do + instrumentation.stub(:meter, nil) do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install + end + + _(instrumentation.config[:client_request_duration]).must_be_nil + end + end + + describe '#connect metrics' do + it 'records metrics for connect operations' do + WebMock.allow_net_connect! + + TCPServer.open('localhost', 0) do |server| + Thread.start { server.accept } + port = server.addr[1] + + uri = URI.parse("http://localhost:#{port}/example") + http = Net::HTTP.new(uri.host, uri.port) + http.read_timeout = 0 + + # This will connect and timeout on read + _(-> { http.request(Net::HTTP::Get.new(uri.request_uri)) }).must_raise(Net::ReadTimeout) + end + + # Pull and verify metrics + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + + _(metrics).wont_be_empty + duration_metric = metrics[0] + + _(duration_metric).wont_be_nil + _(duration_metric.name).must_equal 'http.client.request.duration' + _(duration_metric.description).must_equal 'Duration of HTTP client requests.' + _(duration_metric.unit).must_equal 'ms' + _(duration_metric.instrument_kind).must_equal :histogram + _(duration_metric.data_points).wont_be_empty + ensure + WebMock.disable_net_connect! + end + end + + describe 'error handling' do + it 'continues to work after metric recording errors' do + # Mock the instrumentation instance to return nil config + Net::HTTP.class_eval do + alias_method :original_record_metric, :record_metric + + define_method(:record_metric) do |_duration_ms| + # Simulate an error in metric recording + raise StandardError, 'Metric recording error' + end + end + + # This should not raise an exception + Net::HTTP.get('example.com', '/success') + + # Restore original method + Net::HTTP.class_eval do + alias_method :record_metric, :original_record_metric + remove_method :original_record_metric + end + + # If we get here without an exception, the test passes + _(true).must_equal true + end + end + + describe 'untraced_hosts configuration' do + before do + stub_request(:get, 'http://ignored.com/body').to_return(status: 200) + stub_request(:get, 'http://tracked.com/body').to_return(status: 200) + + instrumentation.instance_variable_set(:@installed, false) + config = { + metrics: true, + client_request_duration: true, + untraced_hosts: ['ignored.com'] + } + instrumentation.install(config) + end + + it 'does not record metrics for ignored hosts' do + Net::HTTP.get('ignored.com', '/body') + + # Pull metrics + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + + # Verify no metrics recorded for ignored host + duration_metric = metrics.find { |m| m.name == 'http.client.request.duration' } + _(duration_metric).must_be_nil + end + + it 'records metrics for non-ignored hosts' do + Net::HTTP.get('tracked.com', '/body') + + # Pull and verify metrics + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + + _(metrics).wont_be_empty + duration_metric = metrics[0] + + _(duration_metric).wont_be_nil + _(duration_metric.name).must_equal 'http.client.request.duration' + _(duration_metric.description).must_equal 'Duration of HTTP client requests.' + _(duration_metric.unit).must_equal 'ms' + _(duration_metric.instrument_kind).must_equal :histogram + _(duration_metric.data_points).wont_be_empty + _(duration_metric.data_points.first.count).must_equal 1 + end + end +end diff --git a/instrumentation/net_http/test/test_helper.rb b/instrumentation/net_http/test/test_helper.rb index 82b97ec0ee..91531fb593 100644 --- a/instrumentation/net_http/test/test_helper.rb +++ b/instrumentation/net_http/test/test_helper.rb @@ -17,6 +17,9 @@ EXPORTER = OpenTelemetry::SDK::Trace::Export::InMemorySpanExporter.new span_processor = OpenTelemetry::SDK::Trace::Export::SimpleSpanProcessor.new(EXPORTER) +# global opentelemetry-metrics-sdk setup: +ENV['OTEL_METRICS_EXPORTER'] = 'none' + 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) diff --git a/instrumentation/rack/Gemfile b/instrumentation/rack/Gemfile index 6cf47f4bde..a28d91f13d 100644 --- a/instrumentation/rack/Gemfile +++ b/instrumentation/rack/Gemfile @@ -13,6 +13,7 @@ group :test do gem 'bundler', '~> 2.4' gem 'minitest', '~> 5.0' gem 'opentelemetry-sdk', '~> 1.1' + gem 'opentelemetry-metrics-sdk', '~> 0.11' gem 'opentelemetry-sdk-experimental', '~> 0.1' gem 'opentelemetry-test-helpers', '~> 0.3' gem 'rake', '~> 13.0' diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb index ea680ed80c..3e7d210db5 100644 --- a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb @@ -63,6 +63,9 @@ def middleware_args_dup end def middleware_args_stable + puts "config: #{config.inspect}" + puts "defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler): #{defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler)}" + puts "defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandlerWithMetrics): #{defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandlerWithMetrics)}" if config.fetch(:use_rack_events, false) == true \ && defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler) \ && defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandlerWithMetrics) diff --git a/instrumentation/rack/test/opentelemetry/instrumentation/rack/instrumentation_stable_test.rb b/instrumentation/rack/test/opentelemetry/instrumentation/rack/instrumentation_stable_test.rb index be80413912..de66d07160 100644 --- a/instrumentation/rack/test/opentelemetry/instrumentation/rack/instrumentation_stable_test.rb +++ b/instrumentation/rack/test/opentelemetry/instrumentation/rack/instrumentation_stable_test.rb @@ -5,6 +5,9 @@ # SPDX-License-Identifier: Apache-2.0 require 'test_helper' +require_relative '../../../../lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics' +require_relative '../../../../lib/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics' +require_relative '../../../../lib/opentelemetry/instrumentation/rack/middlewares/stable/event_handler' describe OpenTelemetry::Instrumentation::Rack::Instrumentation do let(:instrumentation_class) { OpenTelemetry::Instrumentation::Rack::Instrumentation } @@ -49,7 +52,7 @@ end end - describe '#middleware_args_old' do + describe '#middleware_args_stable' do before do instrumentation.install(config) end @@ -57,19 +60,45 @@ describe 'when rack events are configured' do let(:config) { Hash(use_rack_events: true) } - it 'instantiates a custom event handler' do + it 'includes metrics event handler when using rack events xuan' do args = instrumentation.middleware_args_stable + + puts "args: #{args}" _(args[0]).must_equal Rack::Events + _(args[1].size).must_equal 2 _(args[1][0]).must_be_instance_of OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler + _(args[1][1]).must_be_instance_of OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandlerWithMetrics end end describe 'when rack events are disabled' do let(:config) { Hash(use_rack_events: false) } - it 'instantiates a custom middleware' do + it 'uses TracerMiddlewareWithMetrics' do args = instrumentation.middleware_args_stable - _(args).must_equal [OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::TracerMiddleware] + _(args).must_equal [OpenTelemetry::Instrumentation::Rack::Middlewares::TracerMiddlewareWithMetrics] + end + end + end + + describe '#initialize_metrics' do + let(:meter_provider) { OpenTelemetry.meter_provider } + let(:meter) { meter_provider.meter('test-meter') } + + it 'initializes server_request_duration histogram when meter is available' do + instrumentation.stub(:meter, meter) do + instrumentation.install(config) + histogram = instrumentation.config[:server_request_duration] + _(histogram).wont_be_nil + _(histogram).must_respond_to :record + end + end + + it 'does not create metrics when meter is nil' do + instrumentation.stub(:meter, nil) do + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(config) + _(instrumentation.config[:server_request_duration]).must_be_nil end end end diff --git a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics_test.rb b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics_test.rb new file mode 100644 index 0000000000..cb26219545 --- /dev/null +++ b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics_test.rb @@ -0,0 +1,214 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' +require 'rack/builder' +require 'rack/mock' +require_relative '../../../../../lib/opentelemetry/instrumentation/rack' +require_relative '../../../../../lib/opentelemetry/instrumentation/rack/instrumentation' +require_relative '../../../../../lib/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics' + +describe 'OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandlerWithMetrics' do + include Rack::Test::Methods + + let(:instrumentation_module) { OpenTelemetry::Instrumentation::Rack } + let(:instrumentation_class) { instrumentation_module::Instrumentation } + let(:instrumentation) { instrumentation_class.instance } + let(:instrumentation_enabled) { true } + + let(:config) do + { + untraced_endpoints: [], + enabled: instrumentation_enabled, + use_rack_events: true + } + end + + let(:exporter) { EXPORTER } + let(:meter_provider) { OpenTelemetry.meter_provider } + let(:uri) { '/' } + let(:handler) do + OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandlerWithMetrics.new + end + + let(:service) do + ->(_arg) { [200, { 'Content-Type' => 'text/plain' }, ['Hello World']] } + end + let(:headers) { {} } + let(:app) do + Rack::Builder.new.tap do |builder| + builder.use Rack::Events, [handler] + builder.run service + end + end + + before do + exporter.reset + + # Setup metrics + @metric_exporter = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new + @metric_exporter.reset + OpenTelemetry.meter_provider.add_metric_reader(@metric_exporter) + + # simulate a fresh install: + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(config) + end + + after do + # Clean up + instrumentation.instance_variable_set(:@installed, false) + end + + describe '#on_start' do + it 'records the start time in request env' do + request = Rack::Request.new(Rack::MockRequest.env_for(uri)) + + handler.on_start(request, nil) + + start_time = request.env[OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandlerWithMetrics::OTEL_SERVER_START_TIME] + _(start_time).wont_be_nil + _(start_time).must_be_kind_of Integer + _(start_time).must_be :>, 0 + end + + it 'handles exceptions gracefully when request is nil' do + # Should handle gracefully without raising + begin + handler.on_start(nil, nil) + _(true).must_equal true # If we get here, test passed + rescue NoMethodError => e + # NoMethodError is expected when calling methods on nil + _(e).must_be_kind_of NoMethodError + end + end + end + + describe '#on_commit' do + it 'is a no-op' do + request = Rack::Request.new(Rack::MockRequest.env_for(uri)) + response = Rack::Response.new + + # Should not raise an exception + handler.on_commit(request, response) + end + end + + describe '#on_error' do + it 'is a no-op' do + request = Rack::Request.new(Rack::MockRequest.env_for(uri)) + response = Rack::Response.new + error = StandardError.new('test error') + + # Should not raise an exception + handler.on_error(request, response, error) + end + end + + describe '#on_finish' do + it 'records metrics when start_time is present' do + request = Rack::Request.new(Rack::MockRequest.env_for(uri)) + response = Rack::Response.new([200, {}, ['OK']]) + + # Set start time + handler.on_start(request, nil) + + # Record metric + handler.on_finish(request, response) + + # Verify metric was recorded + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + + # Check if metrics are configured + if instrumentation.config[:server_request_duration] + _(metrics).wont_be_empty + duration_metric = metrics.find { |m| m.name == 'http.server.request.duration' } + _(duration_metric).wont_be_nil + else + # If metrics are not configured, we just verify no crash + _(true).must_equal true + end + end + + it 'does not record metrics when start_time is missing' do + request = Rack::Request.new(Rack::MockRequest.env_for(uri)) + response = Rack::Response.new + + # Don't set start time + + # Should not raise an exception + handler.on_finish(request, response) + end + + it 'handles exceptions gracefully when request is nil' do + # Should handle gracefully without raising + begin + handler.on_finish(nil, nil) + _(true).must_equal true # If we get here, test passed + rescue NoMethodError => e + # NoMethodError is expected when calling methods on nil + _(e).must_be_kind_of NoMethodError + end + end + end + + describe 'integration test' do + it 'records metrics for a complete request' do + # Make a request through the full stack + get uri, {}, headers + + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + + # Verify that metrics were recorded if configured + if instrumentation.config[:server_request_duration] + _(metrics).wont_be_empty + duration_metric = metrics.find { |m| m.name == 'http.server.request.duration' } + _(duration_metric).wont_be_nil + end + end + + it 'works alongside other event handlers' do + # Create an app with multiple handlers + other_handler_called = false + other_handler = Class.new do + include Rack::Events::Abstract + + define_method(:initialize) do |callback| + @callback = callback + end + + define_method(:on_start) do |request, response| + @callback.call + end + end.new(-> { other_handler_called = true }) + + multi_handler_app = Rack::Builder.new.tap do |builder| + builder.use Rack::Events, [other_handler, handler] + builder.run service + end + + Rack::MockRequest.new(multi_handler_app).get(uri, headers) + + _(other_handler_called).must_equal true + end + end + + describe 'error handling' do + it 'continues to work after metric recording errors' do + # Mock config to return nil + handler.stub(:config, {}) do + request = Rack::Request.new(Rack::MockRequest.env_for(uri)) + + handler.on_start(request, nil) + + # Should not raise + handler.on_finish(request, nil) + end + end + end +end diff --git a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics_test.rb b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics_test.rb new file mode 100644 index 0000000000..0f082713c3 --- /dev/null +++ b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics_test.rb @@ -0,0 +1,224 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' +require 'rack/builder' +require 'rack/mock' +require_relative '../../../../../lib/opentelemetry/instrumentation/rack' +require_relative '../../../../../lib/opentelemetry/instrumentation/rack/instrumentation' +require_relative '../../../../../lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics' + +describe 'OpenTelemetry::Instrumentation::Rack::Middlewares::TracerMiddlewareWithMetrics' do + let(:instrumentation_module) { OpenTelemetry::Instrumentation::Rack } + let(:instrumentation_class) { instrumentation_module::Instrumentation } + let(:instrumentation) { instrumentation_class.instance } + + let(:described_class) { OpenTelemetry::Instrumentation::Rack::Middlewares::TracerMiddlewareWithMetrics } + + let(:app) { ->(_env) { [200, { 'Content-Type' => 'text/plain' }, ['OK']] } } + let(:middleware) { described_class.new(app) } + let(:rack_builder) { Rack::Builder.new } + + let(:exporter) { EXPORTER } + let(:finished_spans) { exporter.finished_spans } + let(:first_span) { exporter.finished_spans.first } + + let(:default_config) { {} } + let(:config) { default_config } + let(:env) { {} } + let(:uri) { '/' } + + before do + # clear captured spans: + exporter.reset + + # Setup metrics + @metric_exporter = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new + @metric_exporter.reset + OpenTelemetry.meter_provider.add_metric_reader(@metric_exporter) + + # simulate a fresh install: + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(config) + + # integrate tracer middleware: + rack_builder.run app + rack_builder.use described_class + end + + after do + # installation is 'global', so it should be reset: + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(default_config) + end + + describe '#initialize' do + it 'wraps the app with TracerMiddleware' do + _(middleware.instance_variable_get(:@app)).must_equal app + _(middleware.instance_variable_get(:@tracer_middleware)).must_be_instance_of( + OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::TracerMiddleware + ) + end + end + + describe '#call' do + it 'delegates tracing to TracerMiddleware' do + Rack::MockRequest.new(rack_builder).get(uri, env) + + # Verify that spans are still created (delegated to TracerMiddleware) + _(finished_spans).wont_be_empty + _(first_span.attributes['http.request.method']).must_equal 'GET' + _(first_span.attributes['http.response.status_code']).must_equal 200 + _(first_span.attributes['url.path']).must_equal '/' + _(first_span.name).must_equal 'GET' + _(first_span.kind).must_equal :server + end + + it 'records metrics for the request' do + Rack::MockRequest.new(rack_builder).get(uri, env) + + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + + # Verify that metrics were recorded if configured + if instrumentation.config[:server_request_duration] + _(metrics).wont_be_empty + duration_metric = metrics.find { |m| m.name == 'http.server.request.duration' } + _(duration_metric).wont_be_nil + end + end + + it 'records metrics even when app returns different status codes' do + [200, 404, 500].each do |status_code| + exporter.reset + @metric_exporter.reset + + custom_app = ->(_env) { [status_code, { 'Content-Type' => 'text/plain' }, ['Response']] } + custom_builder = Rack::Builder.new + custom_builder.run custom_app + custom_builder.use described_class + + Rack::MockRequest.new(custom_builder).get(uri, env) + + # Verify span was created + _(exporter.finished_spans).wont_be_empty + _(exporter.finished_spans.first.attributes['http.response.status_code']).must_equal status_code + end + end + + describe 'when app raises an exception' do + let(:app) do + ->(_env) { raise StandardError, 'Test error' } + end + + it 'records metrics even when exception occurs' do + exception_raised = false + + begin + Rack::MockRequest.new(rack_builder).get(uri, env) + rescue StandardError + exception_raised = true + end + + _(exception_raised).must_equal true + + # Metrics should still be recorded + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + + if instrumentation.config[:server_request_duration] + _(metrics).wont_be_empty + end + end + + it 'ensures TracerMiddleware handles the exception' do + exception_raised = false + + begin + Rack::MockRequest.new(rack_builder).get(uri, env) + rescue StandardError + exception_raised = true + end + + _(exception_raised).must_equal true + # Span should still be created and finished + _(finished_spans).wont_be_empty + end + end + end + + describe 'timing accuracy' do + it 'measures the full request duration' do + slow_app = lambda do |_env| + sleep 0.05 # 50ms delay + [200, { 'Content-Type' => 'text/plain' }, ['OK']] + end + + slow_builder = Rack::Builder.new + slow_builder.run slow_app + slow_builder.use described_class + + start_time = Time.now + Rack::MockRequest.new(slow_builder).get(uri, env) + elapsed = (Time.now - start_time) * 1000 # Convert to ms + + # The recorded metric should be at least 50ms + _(elapsed).must_be :>=, 50 + end + end + + describe 'error handling in metric recording' do + it 'handles errors during metric recording gracefully' do + # Mock config to cause an error + bad_config = { server_request_duration: nil } + + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install(bad_config) + + builder = Rack::Builder.new + builder.run app + builder.use described_class + + # Should not raise an exception + response = Rack::MockRequest.new(builder).get(uri, env) + _(response.status).must_equal 200 + end + end + + describe 'integration with TracerMiddleware' do + it 'preserves all TracerMiddleware functionality' do + # Test that tracing context is properly propagated + Rack::MockRequest.new(rack_builder).get(uri, env) + + _(first_span).wont_be_nil + _(first_span.name).must_equal 'GET' + _(first_span.kind).must_equal :server + _(first_span.status.code).must_equal OpenTelemetry::Trace::Status::UNSET + end + + it 'works with custom headers' do + custom_env = { + 'HTTP_USER_AGENT' => 'Test Agent', + 'HTTP_X_FORWARDED_FOR' => '192.168.1.1' + } + + Rack::MockRequest.new(rack_builder).get(uri, custom_env) + + _(first_span).wont_be_nil + _(first_span.attributes['user_agent.original']).must_equal 'Test Agent' + end + + it 'handles query strings' do + uri_with_query = '/endpoint?query=true&foo=bar' + + Rack::MockRequest.new(rack_builder).get(uri_with_query, env) + + _(first_span).wont_be_nil + _(first_span.attributes['url.path']).must_equal '/endpoint' + _(first_span.attributes['url.query']).must_equal 'query=true&foo=bar' + end + end +end From 589a51a7173499dd28d426ccceda97f2e0798152 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Mon, 10 Nov 2025 12:44:18 -0500 Subject: [PATCH 4/7] update net_http test case --- .../net/http/stable/metrics_test.rb | 167 +++--------------- 1 file changed, 25 insertions(+), 142 deletions(-) diff --git a/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/stable/metrics_test.rb b/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/stable/metrics_test.rb index f0c81812b9..c565f3fd48 100644 --- a/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/stable/metrics_test.rb +++ b/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/stable/metrics_test.rb @@ -13,7 +13,18 @@ let(:instrumentation) { OpenTelemetry::Instrumentation::Net::HTTP::Instrumentation.instance } let(:exporter) { EXPORTER } let(:span) { exporter.finished_spans.first } - # let(:metric_exporter) { METRICS_EXPORTER } + + # Helper method to verify metric structure + def assert_duration_metric(metric, expected_count: nil) + _(metric).wont_be_nil + _(metric.name).must_equal 'http.client.request.duration' + _(metric.description).must_equal 'Duration of HTTP client requests.' + _(metric.unit).must_equal 'ms' + _(metric.instrument_kind).must_equal :histogram + _(metric.instrumentation_scope.name).must_equal 'OpenTelemetry::Instrumentation::Net::HTTP' + _(metric.data_points).wont_be_empty + _(metric.data_points.first.count).must_equal expected_count if expected_count + end before do skip unless ENV['BUNDLE_GEMFILE'].include?('stable') @@ -47,52 +58,32 @@ end describe 'metrics integration' do - it 'records metrics alongside spans for successful requests' do + it 'records metrics alongside spans for various scenarios' do + # Successful request Net::HTTP.get('example.com', '/success') - - # Verify span was created _(exporter.finished_spans.size).must_equal 1 _(span.name).must_equal 'GET' - # Pull and verify metrics @metric_exporter.pull metrics = @metric_exporter.metric_snapshots - _(metrics).wont_be_empty - duration_metric = metrics[0] - - _(duration_metric).wont_be_nil - _(duration_metric.name).must_equal 'http.client.request.duration' - _(duration_metric.description).must_equal 'Duration of HTTP client requests.' - _(duration_metric.unit).must_equal 'ms' - _(duration_metric.instrument_kind).must_equal :histogram - _(duration_metric.instrumentation_scope.name).must_equal 'OpenTelemetry::Instrumentation::Net::HTTP' - _(duration_metric.data_points).wont_be_empty - _(duration_metric.data_points.first.count).must_equal 1 - end + assert_duration_metric(metrics[0], expected_count: 1) - it 'records metrics for requests with different status codes' do - # Test successful request - Net::HTTP.get('example.com', '/success') + # Failed request with 500 status + Net::HTTP.post(URI('http://example.com/failure'), 'q' => 'ruby') + _(exporter.finished_spans.last.attributes['http.response.status_code']).must_equal 500 @metric_exporter.pull metrics = @metric_exporter.metric_snapshots - _(metrics).wont_be_empty - _(metrics[0].data_points.first.count).must_equal 1 + assert_duration_metric(metrics[0], expected_count: 1) - # Test failed request - Net::HTTP.post(URI('http://example.com/failure'), 'q' => 'ruby') - - _(exporter.finished_spans.last.attributes['http.response.status_code']).must_equal 500 + # Multiple requests accumulate + 2.times { Net::HTTP.get('example.com', '/success') } + _(exporter.finished_spans.size).must_equal 4 @metric_exporter.pull metrics = @metric_exporter.metric_snapshots - duration_metric = metrics[0] - - _(duration_metric).wont_be_nil - _(duration_metric.name).must_equal 'http.client.request.duration' - _(duration_metric.data_points).wont_be_empty - _(duration_metric.data_points.first.count).must_equal 1 + assert_duration_metric(metrics[0], expected_count: 3) end it 'records metrics even when request times out' do @@ -100,47 +91,12 @@ Net::HTTP.get(URI('https://example.com/timeout')) end.must_raise Net::OpenTimeout - # Verify span was created _(exporter.finished_spans.size).must_equal 1 - # Pull and verify metrics @metric_exporter.pull metrics = @metric_exporter.metric_snapshots - _(metrics).wont_be_empty - duration_metric = metrics[0] - - _(duration_metric).wont_be_nil - _(duration_metric.name).must_equal 'http.client.request.duration' - _(duration_metric.description).must_equal 'Duration of HTTP client requests.' - _(duration_metric.unit).must_equal 'ms' - _(duration_metric.instrument_kind).must_equal :histogram - _(duration_metric.data_points).wont_be_empty - _(duration_metric.data_points.first.count).must_equal 1 - end - - it 'records metrics for multiple requests' do - # Make multiple requests - 3.times do - Net::HTTP.get('example.com', '/success') - end - - # Pull and verify metrics - @metric_exporter.pull - metrics = @metric_exporter.metric_snapshots - - _(metrics).wont_be_empty - _(exporter.finished_spans.size).must_equal 3 - - duration_metric = metrics[0] - - _(duration_metric).wont_be_nil - _(duration_metric.name).must_equal 'http.client.request.duration' - _(duration_metric.description).must_equal 'Duration of HTTP client requests.' - _(duration_metric.unit).must_equal 'ms' - _(duration_metric.instrument_kind).must_equal :histogram - _(duration_metric.data_points).wont_be_empty - _(duration_metric.data_points.first.count).must_equal 3 + assert_duration_metric(metrics[0], expected_count: 1) end it 'does not record metrics in untraced context' do @@ -148,11 +104,9 @@ Net::HTTP.get('example.com', '/success') end - # Pull metrics @metric_exporter.pull metrics = @metric_exporter.metric_snapshots - # Verify that no metrics were recorded in untraced context duration_metric = metrics.find { |m| m.name == 'http.client.request.duration' } _(duration_metric).must_be_nil end @@ -183,66 +137,6 @@ end end - describe '#connect metrics' do - it 'records metrics for connect operations' do - WebMock.allow_net_connect! - - TCPServer.open('localhost', 0) do |server| - Thread.start { server.accept } - port = server.addr[1] - - uri = URI.parse("http://localhost:#{port}/example") - http = Net::HTTP.new(uri.host, uri.port) - http.read_timeout = 0 - - # This will connect and timeout on read - _(-> { http.request(Net::HTTP::Get.new(uri.request_uri)) }).must_raise(Net::ReadTimeout) - end - - # Pull and verify metrics - @metric_exporter.pull - metrics = @metric_exporter.metric_snapshots - - _(metrics).wont_be_empty - duration_metric = metrics[0] - - _(duration_metric).wont_be_nil - _(duration_metric.name).must_equal 'http.client.request.duration' - _(duration_metric.description).must_equal 'Duration of HTTP client requests.' - _(duration_metric.unit).must_equal 'ms' - _(duration_metric.instrument_kind).must_equal :histogram - _(duration_metric.data_points).wont_be_empty - ensure - WebMock.disable_net_connect! - end - end - - describe 'error handling' do - it 'continues to work after metric recording errors' do - # Mock the instrumentation instance to return nil config - Net::HTTP.class_eval do - alias_method :original_record_metric, :record_metric - - define_method(:record_metric) do |_duration_ms| - # Simulate an error in metric recording - raise StandardError, 'Metric recording error' - end - end - - # This should not raise an exception - Net::HTTP.get('example.com', '/success') - - # Restore original method - Net::HTTP.class_eval do - alias_method :record_metric, :original_record_metric - remove_method :original_record_metric - end - - # If we get here without an exception, the test passes - _(true).must_equal true - end - end - describe 'untraced_hosts configuration' do before do stub_request(:get, 'http://ignored.com/body').to_return(status: 200) @@ -260,11 +154,9 @@ it 'does not record metrics for ignored hosts' do Net::HTTP.get('ignored.com', '/body') - # Pull metrics @metric_exporter.pull metrics = @metric_exporter.metric_snapshots - # Verify no metrics recorded for ignored host duration_metric = metrics.find { |m| m.name == 'http.client.request.duration' } _(duration_metric).must_be_nil end @@ -272,20 +164,11 @@ it 'records metrics for non-ignored hosts' do Net::HTTP.get('tracked.com', '/body') - # Pull and verify metrics @metric_exporter.pull metrics = @metric_exporter.metric_snapshots _(metrics).wont_be_empty - duration_metric = metrics[0] - - _(duration_metric).wont_be_nil - _(duration_metric.name).must_equal 'http.client.request.duration' - _(duration_metric.description).must_equal 'Duration of HTTP client requests.' - _(duration_metric.unit).must_equal 'ms' - _(duration_metric.instrument_kind).must_equal :histogram - _(duration_metric.data_points).wont_be_empty - _(duration_metric.data_points.first.count).must_equal 1 + assert_duration_metric(metrics[0], expected_count: 1) end end end From acb6ad2f72a43d78f7051307bc331205a432cf62 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Mon, 10 Nov 2025 15:30:09 -0500 Subject: [PATCH 5/7] fix rack test case --- .../instrumentation/rack/instrumentation.rb | 3 - .../event_handler_with_metrics_test.rb | 86 ++++------ .../tracer_middleware_with_metrics_test.rb | 151 ++++++++---------- 3 files changed, 93 insertions(+), 147 deletions(-) diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb index 3e7d210db5..ea680ed80c 100644 --- a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb +++ b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb @@ -63,9 +63,6 @@ def middleware_args_dup end def middleware_args_stable - puts "config: #{config.inspect}" - puts "defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler): #{defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler)}" - puts "defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandlerWithMetrics): #{defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandlerWithMetrics)}" if config.fetch(:use_rack_events, false) == true \ && defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::Stable::EventHandler) \ && defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::EventHandlerWithMetrics) diff --git a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics_test.rb b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics_test.rb index cb26219545..b437da27a2 100644 --- a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics_test.rb +++ b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics_test.rb @@ -19,11 +19,24 @@ let(:instrumentation) { instrumentation_class.instance } let(:instrumentation_enabled) { true } + # Helper method to verify metric structure + def assert_server_duration_metric(metric, expected_count: nil) + _(metric).wont_be_nil + _(metric.name).must_equal 'http.server.request.duration' + _(metric.description).must_equal 'Duration of HTTP server requests.' + _(metric.unit).must_equal 'ms' + _(metric.instrument_kind).must_equal :histogram + _(metric.instrumentation_scope.name).must_equal 'OpenTelemetry::Instrumentation::Rack' + _(metric.data_points).wont_be_empty + _(metric.data_points.first.count).must_equal expected_count if expected_count + end + let(:config) do { untraced_endpoints: [], enabled: instrumentation_enabled, - use_rack_events: true + use_rack_events: true, + metrics: true } end @@ -50,7 +63,6 @@ # Setup metrics @metric_exporter = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new - @metric_exporter.reset OpenTelemetry.meter_provider.add_metric_reader(@metric_exporter) # simulate a fresh install: @@ -76,34 +88,22 @@ end it 'handles exceptions gracefully when request is nil' do - # Should handle gracefully without raising begin handler.on_start(nil, nil) - _(true).must_equal true # If we get here, test passed + _(true).must_equal true rescue NoMethodError => e - # NoMethodError is expected when calling methods on nil _(e).must_be_kind_of NoMethodError end end end - describe '#on_commit' do - it 'is a no-op' do - request = Rack::Request.new(Rack::MockRequest.env_for(uri)) - response = Rack::Response.new - - # Should not raise an exception - handler.on_commit(request, response) - end - end - - describe '#on_error' do - it 'is a no-op' do + describe '#on_commit and #on_error' do + it 'are no-ops that do not raise exceptions' do request = Rack::Request.new(Rack::MockRequest.env_for(uri)) response = Rack::Response.new error = StandardError.new('test error') - # Should not raise an exception + handler.on_commit(request, response) handler.on_error(request, response, error) end end @@ -113,67 +113,47 @@ request = Rack::Request.new(Rack::MockRequest.env_for(uri)) response = Rack::Response.new([200, {}, ['OK']]) - # Set start time handler.on_start(request, nil) - - # Record metric handler.on_finish(request, response) - # Verify metric was recorded @metric_exporter.pull metrics = @metric_exporter.metric_snapshots - # Check if metrics are configured - if instrumentation.config[:server_request_duration] - _(metrics).wont_be_empty - duration_metric = metrics.find { |m| m.name == 'http.server.request.duration' } - _(duration_metric).wont_be_nil - else - # If metrics are not configured, we just verify no crash - _(true).must_equal true - end + _(metrics).wont_be_empty + duration_metric = metrics.find { |m| m.name == 'http.server.request.duration' } + assert_server_duration_metric(duration_metric, expected_count: 1) end - it 'does not record metrics when start_time is missing' do + it 'handles edge cases gracefully' do request = Rack::Request.new(Rack::MockRequest.env_for(uri)) response = Rack::Response.new - # Don't set start time - - # Should not raise an exception + # Missing start_time - should not raise handler.on_finish(request, response) - end - it 'handles exceptions gracefully when request is nil' do - # Should handle gracefully without raising + # Nil request - should handle gracefully begin handler.on_finish(nil, nil) - _(true).must_equal true # If we get here, test passed + _(true).must_equal true rescue NoMethodError => e - # NoMethodError is expected when calling methods on nil _(e).must_be_kind_of NoMethodError end end end - describe 'integration test' do + describe 'integration tests' do it 'records metrics for a complete request' do - # Make a request through the full stack get uri, {}, headers @metric_exporter.pull metrics = @metric_exporter.metric_snapshots - # Verify that metrics were recorded if configured - if instrumentation.config[:server_request_duration] - _(metrics).wont_be_empty - duration_metric = metrics.find { |m| m.name == 'http.server.request.duration' } - _(duration_metric).wont_be_nil - end + _(metrics).wont_be_empty + duration_metric = metrics.find { |m| m.name == 'http.server.request.duration' } + assert_server_duration_metric(duration_metric, expected_count: 1) end it 'works alongside other event handlers' do - # Create an app with multiple handlers other_handler_called = false other_handler = Class.new do include Rack::Events::Abstract @@ -196,17 +176,11 @@ _(other_handler_called).must_equal true end - end - describe 'error handling' do - it 'continues to work after metric recording errors' do - # Mock config to return nil + it 'handles metric recording errors gracefully' do handler.stub(:config, {}) do request = Rack::Request.new(Rack::MockRequest.env_for(uri)) - handler.on_start(request, nil) - - # Should not raise handler.on_finish(request, nil) end end diff --git a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics_test.rb b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics_test.rb index 0f082713c3..7d48730f6a 100644 --- a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics_test.rb +++ b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics_test.rb @@ -18,6 +18,18 @@ let(:described_class) { OpenTelemetry::Instrumentation::Rack::Middlewares::TracerMiddlewareWithMetrics } + # Helper method to verify metric structure + def assert_server_duration_metric(metric, expected_count: nil) + _(metric).wont_be_nil + _(metric.name).must_equal 'http.server.request.duration' + _(metric.description).must_equal 'Duration of HTTP server requests.' + _(metric.unit).must_equal 'ms' + _(metric.instrument_kind).must_equal :histogram + _(metric.instrumentation_scope.name).must_equal 'OpenTelemetry::Instrumentation::Rack' + _(metric.data_points).wont_be_empty + _(metric.data_points.first.count).must_equal expected_count if expected_count + end + let(:app) { ->(_env) { [200, { 'Content-Type' => 'text/plain' }, ['OK']] } } let(:middleware) { described_class.new(app) } let(:rack_builder) { Rack::Builder.new } @@ -27,7 +39,7 @@ let(:first_span) { exporter.finished_spans.first } let(:default_config) { {} } - let(:config) { default_config } + let(:config) { { metrics: true, server_request_duration: true } } let(:env) { {} } let(:uri) { '/' } @@ -37,7 +49,6 @@ # Setup metrics @metric_exporter = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new - @metric_exporter.reset OpenTelemetry.meter_provider.add_metric_reader(@metric_exporter) # simulate a fresh install: @@ -65,34 +76,28 @@ end describe '#call' do - it 'delegates tracing to TracerMiddleware' do + it 'delegates tracing to TracerMiddleware and records metrics' do Rack::MockRequest.new(rack_builder).get(uri, env) - # Verify that spans are still created (delegated to TracerMiddleware) + # Verify spans are created (delegated to TracerMiddleware) _(finished_spans).wont_be_empty _(first_span.attributes['http.request.method']).must_equal 'GET' _(first_span.attributes['http.response.status_code']).must_equal 200 _(first_span.attributes['url.path']).must_equal '/' _(first_span.name).must_equal 'GET' _(first_span.kind).must_equal :server - end - - it 'records metrics for the request' do - Rack::MockRequest.new(rack_builder).get(uri, env) + # Verify metrics are recorded @metric_exporter.pull metrics = @metric_exporter.metric_snapshots - # Verify that metrics were recorded if configured - if instrumentation.config[:server_request_duration] - _(metrics).wont_be_empty - duration_metric = metrics.find { |m| m.name == 'http.server.request.duration' } - _(duration_metric).wont_be_nil - end + _(metrics).wont_be_empty + duration_metric = metrics.find { |m| m.name == 'http.server.request.duration' } + assert_server_duration_metric(duration_metric, expected_count: 1) end - it 'records metrics even when app returns different status codes' do - [200, 404, 500].each do |status_code| + it 'works with different status codes' do + [404, 500].each do |status_code| exporter.reset @metric_exporter.reset @@ -103,50 +108,52 @@ Rack::MockRequest.new(custom_builder).get(uri, env) - # Verify span was created _(exporter.finished_spans).wont_be_empty _(exporter.finished_spans.first.attributes['http.response.status_code']).must_equal status_code end end - describe 'when app raises an exception' do - let(:app) do - ->(_env) { raise StandardError, 'Test error' } - end - - it 'records metrics even when exception occurs' do - exception_raised = false - - begin - Rack::MockRequest.new(rack_builder).get(uri, env) - rescue StandardError - exception_raised = true - end - - _(exception_raised).must_equal true + it 'handles exceptions and still records metrics' do + exporter.reset + @metric_exporter.reset - # Metrics should still be recorded - @metric_exporter.pull - metrics = @metric_exporter.metric_snapshots + exception_app = ->(_env) { raise StandardError, 'Test error' } + exception_builder = Rack::Builder.new + exception_builder.run exception_app + exception_builder.use described_class - if instrumentation.config[:server_request_duration] - _(metrics).wont_be_empty - end + exception_raised = false + begin + Rack::MockRequest.new(exception_builder).get(uri, env) + rescue StandardError + exception_raised = true end - it 'ensures TracerMiddleware handles the exception' do - exception_raised = false - - begin - Rack::MockRequest.new(rack_builder).get(uri, env) - rescue StandardError - exception_raised = true - end + _(exception_raised).must_equal true + + # Verify span details + _(finished_spans.size).must_equal 1 + error_span = finished_spans.first + + _(error_span).wont_be_nil + _(error_span.name).must_equal 'GET' + _(error_span.kind).must_equal :server + _(error_span.status.code).must_equal OpenTelemetry::Trace::Status::ERROR + _(error_span.status.description).must_match(/StandardError/) + _(error_span.attributes['http.request.method']).must_equal 'GET' + _(error_span.attributes['url.path']).must_equal '/' + _(error_span.events.size).must_equal 1 + _(error_span.events.first.name).must_equal 'exception' + _(error_span.events.first.attributes['exception.type']).must_equal 'StandardError' + _(error_span.events.first.attributes['exception.message']).must_equal 'Test error' + + # Metrics should still be recorded + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots - _(exception_raised).must_equal true - # Span should still be created and finished - _(finished_spans).wont_be_empty - end + _(metrics).wont_be_empty + duration_metric = metrics.find { |m| m.name == 'http.server.request.duration' } + assert_server_duration_metric(duration_metric, expected_count: 1) end end @@ -170,53 +177,21 @@ end end - describe 'error handling in metric recording' do - it 'handles errors during metric recording gracefully' do - # Mock config to cause an error - bad_config = { server_request_duration: nil } - - instrumentation.instance_variable_set(:@installed, false) - instrumentation.install(bad_config) - - builder = Rack::Builder.new - builder.run app - builder.use described_class - - # Should not raise an exception - response = Rack::MockRequest.new(builder).get(uri, env) - _(response.status).must_equal 200 - end - end - describe 'integration with TracerMiddleware' do - it 'preserves all TracerMiddleware functionality' do - # Test that tracing context is properly propagated - Rack::MockRequest.new(rack_builder).get(uri, env) - - _(first_span).wont_be_nil - _(first_span.name).must_equal 'GET' - _(first_span.kind).must_equal :server - _(first_span.status.code).must_equal OpenTelemetry::Trace::Status::UNSET - end - - it 'works with custom headers' do + it 'preserves TracerMiddleware functionality with headers and query strings' do custom_env = { 'HTTP_USER_AGENT' => 'Test Agent', 'HTTP_X_FORWARDED_FOR' => '192.168.1.1' } - - Rack::MockRequest.new(rack_builder).get(uri, custom_env) - - _(first_span).wont_be_nil - _(first_span.attributes['user_agent.original']).must_equal 'Test Agent' - end - - it 'handles query strings' do uri_with_query = '/endpoint?query=true&foo=bar' - Rack::MockRequest.new(rack_builder).get(uri_with_query, env) + Rack::MockRequest.new(rack_builder).get(uri_with_query, custom_env) _(first_span).wont_be_nil + _(first_span.name).must_equal 'GET' + _(first_span.kind).must_equal :server + _(first_span.status.code).must_equal OpenTelemetry::Trace::Status::UNSET + _(first_span.attributes['user_agent.original']).must_equal 'Test Agent' _(first_span.attributes['url.path']).must_equal '/endpoint' _(first_span.attributes['url.query']).must_equal 'query=true&foo=bar' end From 318731d10c1296dbf40c1d63a20a76b6c67f50d6 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Mon, 10 Nov 2025 16:19:56 -0500 Subject: [PATCH 6/7] update test case for rack --- .../event_handler_with_metrics_test.rb | 44 +++---------------- .../tracer_middleware_with_metrics_test.rb | 16 ------- instrumentation/rack/test/test_helper.rb | 12 +++++ 3 files changed, 19 insertions(+), 53 deletions(-) diff --git a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics_test.rb b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics_test.rb index b437da27a2..bc65c55fc8 100644 --- a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics_test.rb +++ b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/event_handler_with_metrics_test.rb @@ -19,18 +19,6 @@ let(:instrumentation) { instrumentation_class.instance } let(:instrumentation_enabled) { true } - # Helper method to verify metric structure - def assert_server_duration_metric(metric, expected_count: nil) - _(metric).wont_be_nil - _(metric.name).must_equal 'http.server.request.duration' - _(metric.description).must_equal 'Duration of HTTP server requests.' - _(metric.unit).must_equal 'ms' - _(metric.instrument_kind).must_equal :histogram - _(metric.instrumentation_scope.name).must_equal 'OpenTelemetry::Instrumentation::Rack' - _(metric.data_points).wont_be_empty - _(metric.data_points.first.count).must_equal expected_count if expected_count - end - let(:config) do { untraced_endpoints: [], @@ -61,7 +49,6 @@ def assert_server_duration_metric(metric, expected_count: nil) before do exporter.reset - # Setup metrics @metric_exporter = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new OpenTelemetry.meter_provider.add_metric_reader(@metric_exporter) @@ -71,7 +58,6 @@ def assert_server_duration_metric(metric, expected_count: nil) end after do - # Clean up instrumentation.instance_variable_set(:@installed, false) end @@ -86,15 +72,6 @@ def assert_server_duration_metric(metric, expected_count: nil) _(start_time).must_be_kind_of Integer _(start_time).must_be :>, 0 end - - it 'handles exceptions gracefully when request is nil' do - begin - handler.on_start(nil, nil) - _(true).must_equal true - rescue NoMethodError => e - _(e).must_be_kind_of NoMethodError - end - end end describe '#on_commit and #on_error' do @@ -119,9 +96,7 @@ def assert_server_duration_metric(metric, expected_count: nil) @metric_exporter.pull metrics = @metric_exporter.metric_snapshots - _(metrics).wont_be_empty - duration_metric = metrics.find { |m| m.name == 'http.server.request.duration' } - assert_server_duration_metric(duration_metric, expected_count: 1) + assert_server_duration_metric(metrics[0], expected_count: 1) end it 'handles edge cases gracefully' do @@ -131,13 +106,10 @@ def assert_server_duration_metric(metric, expected_count: nil) # Missing start_time - should not raise handler.on_finish(request, response) - # Nil request - should handle gracefully - begin - handler.on_finish(nil, nil) - _(true).must_equal true - rescue NoMethodError => e - _(e).must_be_kind_of NoMethodError - end + @metric_exporter.pull + metrics = @metric_exporter.metric_snapshots + + _(metrics).must_be_empty end end @@ -148,9 +120,7 @@ def assert_server_duration_metric(metric, expected_count: nil) @metric_exporter.pull metrics = @metric_exporter.metric_snapshots - _(metrics).wont_be_empty - duration_metric = metrics.find { |m| m.name == 'http.server.request.duration' } - assert_server_duration_metric(duration_metric, expected_count: 1) + assert_server_duration_metric(metrics[0], expected_count: 1) end it 'works alongside other event handlers' do @@ -162,7 +132,7 @@ def assert_server_duration_metric(metric, expected_count: nil) @callback = callback end - define_method(:on_start) do |request, response| + define_method(:on_start) do |_request, _response| @callback.call end end.new(-> { other_handler_called = true }) diff --git a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics_test.rb b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics_test.rb index 7d48730f6a..b2255b7612 100644 --- a/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics_test.rb +++ b/instrumentation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_with_metrics_test.rb @@ -18,18 +18,6 @@ let(:described_class) { OpenTelemetry::Instrumentation::Rack::Middlewares::TracerMiddlewareWithMetrics } - # Helper method to verify metric structure - def assert_server_duration_metric(metric, expected_count: nil) - _(metric).wont_be_nil - _(metric.name).must_equal 'http.server.request.duration' - _(metric.description).must_equal 'Duration of HTTP server requests.' - _(metric.unit).must_equal 'ms' - _(metric.instrument_kind).must_equal :histogram - _(metric.instrumentation_scope.name).must_equal 'OpenTelemetry::Instrumentation::Rack' - _(metric.data_points).wont_be_empty - _(metric.data_points.first.count).must_equal expected_count if expected_count - end - let(:app) { ->(_env) { [200, { 'Content-Type' => 'text/plain' }, ['OK']] } } let(:middleware) { described_class.new(app) } let(:rack_builder) { Rack::Builder.new } @@ -44,14 +32,11 @@ def assert_server_duration_metric(metric, expected_count: nil) let(:uri) { '/' } before do - # clear captured spans: exporter.reset - # Setup metrics @metric_exporter = OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new OpenTelemetry.meter_provider.add_metric_reader(@metric_exporter) - # simulate a fresh install: instrumentation.instance_variable_set(:@installed, false) instrumentation.install(config) @@ -61,7 +46,6 @@ def assert_server_duration_metric(metric, expected_count: nil) end after do - # installation is 'global', so it should be reset: instrumentation.instance_variable_set(:@installed, false) instrumentation.install(default_config) end diff --git a/instrumentation/rack/test/test_helper.rb b/instrumentation/rack/test/test_helper.rb index 08824e318a..e5783c23a5 100644 --- a/instrumentation/rack/test/test_helper.rb +++ b/instrumentation/rack/test/test_helper.rb @@ -18,6 +18,18 @@ EXPORTER = OpenTelemetry::SDK::Trace::Export::InMemorySpanExporter.new span_processor = OpenTelemetry::SDK::Trace::Export::SimpleSpanProcessor.new(EXPORTER) +# Helper method to verify metric structure +def assert_server_duration_metric(metric, expected_count: nil) + _(metric).wont_be_nil + _(metric.name).must_equal 'http.server.request.duration' + _(metric.description).must_equal 'Duration of HTTP server requests.' + _(metric.unit).must_equal 'ms' + _(metric.instrument_kind).must_equal :histogram + _(metric.instrumentation_scope.name).must_equal 'OpenTelemetry::Instrumentation::Rack' + _(metric.data_points).wont_be_empty + _(metric.data_points.first.count).must_equal expected_count if expected_count +end + 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) From 40120dbddcb98b9d6e392135efd663ab529fe613 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Wed, 12 Nov 2025 12:17:05 -0500 Subject: [PATCH 7/7] update test case --- .../instrumentation/net/http/stable/metrics_test.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/stable/metrics_test.rb b/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/stable/metrics_test.rb index c565f3fd48..f326a36db3 100644 --- a/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/stable/metrics_test.rb +++ b/instrumentation/net_http/test/opentelemetry/instrumentation/net/http/stable/metrics_test.rb @@ -58,8 +58,7 @@ def assert_duration_metric(metric, expected_count: nil) end describe 'metrics integration' do - it 'records metrics alongside spans for various scenarios' do - # Successful request + it 'records metrics alongside spans for successful request' do Net::HTTP.get('example.com', '/success') _(exporter.finished_spans.size).must_equal 1 _(span.name).must_equal 'GET' @@ -68,22 +67,24 @@ def assert_duration_metric(metric, expected_count: nil) metrics = @metric_exporter.metric_snapshots _(metrics).wont_be_empty assert_duration_metric(metrics[0], expected_count: 1) + end - # Failed request with 500 status + it 'records metrics alongside spans for failed request with 500 status' do Net::HTTP.post(URI('http://example.com/failure'), 'q' => 'ruby') _(exporter.finished_spans.last.attributes['http.response.status_code']).must_equal 500 @metric_exporter.pull metrics = @metric_exporter.metric_snapshots assert_duration_metric(metrics[0], expected_count: 1) + end - # Multiple requests accumulate + it 'records metrics alongside spans for multiple requests accumulate' do 2.times { Net::HTTP.get('example.com', '/success') } - _(exporter.finished_spans.size).must_equal 4 + _(exporter.finished_spans.size).must_equal 2 @metric_exporter.pull metrics = @metric_exporter.metric_snapshots - assert_duration_metric(metrics[0], expected_count: 3) + assert_duration_metric(metrics[0], expected_count: 2) end it 'records metrics even when request times out' do