diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb index ccfe48ff3e..6999068815 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb @@ -23,6 +23,9 @@ def initialize(config) # @param payload [Hash] the payload passed as a method argument # @return [Hash] the payload passed as a method argument def start(_name, _id, payload) + span = OpenTelemetry::Instrumentation::Rack.current_span + return unless span.recording? + request = payload[:request] # It seems that there are cases in Rails functional tests where it bypasses the routing system and the `action_dispatch.route_uri_pattern` header not being set. # Our Test suite executes the routing system so we are unable to recreate this error case. @@ -36,22 +39,18 @@ def start(_name, _id, payload) attributes[OpenTelemetry::SemanticConventions::Trace::HTTP_ROUTE] = http_route if http_route attributes[OpenTelemetry::SemanticConventions::Trace::HTTP_TARGET] = request.filtered_path if request.filtered_path != request.fullpath - span = OpenTelemetry::Instrumentation::Rack.current_span - span_name = if @span_naming == :semconv - if http_route + if @span_naming == :semconv + span.name = if http_route "#{request.method} #{http_route}" else "#{request.method} /#{payload.dig(:params, :controller)}/#{payload.dig(:params, :action)}" end - # If there is an exception we want to keep the original span name - # so it is easier to see where the request was routed to. - elsif request.env['action_dispatch.exception'] - span.name - else - "#{payload[:controller]}##{payload[:action]}" - end + # If there is an exception we want to keep the original span name + # so it is easier to see where the request was routed to. + elsif !request.env['action_dispatch.exception'] + span.name = "#{payload[:controller]}##{payload[:action]}" + end - span.name = span_name span.add_attributes(attributes) rescue StandardError => e OpenTelemetry.handle_error(exception: e) diff --git a/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb b/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb index 099debf55d..47cfe648a3 100644 --- a/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb +++ b/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb @@ -236,6 +236,12 @@ _(span.attributes['code.namespace']).must_equal 'ExceptionsController' _(span.attributes['code.function']).must_equal 'show' end + + it 'does not raise with api/non recording spans' do + with_sampler(OpenTelemetry::SDK::Trace::Samplers::ALWAYS_OFF) do + get 'internal_server_error' + end + end end it 'sets filters `http.target`' do diff --git a/instrumentation/action_pack/test/test_helper.rb b/instrumentation/action_pack/test/test_helper.rb index 992871d77d..8ea8f9ec92 100644 --- a/instrumentation/action_pack/test/test_helper.rb +++ b/instrumentation/action_pack/test/test_helper.rb @@ -24,3 +24,11 @@ c.use 'OpenTelemetry::Instrumentation::ActionPack' c.add_span_processor span_processor end + +def with_sampler(sampler) + previous_sampler = OpenTelemetry.tracer_provider.sampler + OpenTelemetry.tracer_provider.sampler = sampler + yield +ensure + OpenTelemetry.tracer_provider.sampler = previous_sampler +end