avoid set status in action controller (let rack set the span status);…

… add more test case for 4xx error
open-telemetry · Sep 12, 2023 · c549724 · c549724
1 parent 26d1dd5
commit c549724
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 1 deletion.
diff --git a/...ion_pack/lib/opentelemetry/instrumentation/action_pack/patches/action_controller/metal.rb b/...ion_pack/lib/opentelemetry/instrumentation/action_pack/patches/action_controller/metal.rb
@@ -27,7 +27,6 @@ def dispatch(name, request, response)
               super(name, request, response)
             rescue Exception => e # rubocop:disable Lint/RescueException
               rack_span.record_exception(e)
-              rack_span.status = OpenTelemetry::Trace::Status.error
               raise
             end
 

diff --git a/...ck/test/opentelemetry/instrumentation/action_pack/patches/action_controller/metal_test.rb b/...ck/test/opentelemetry/instrumentation/action_pack/patches/action_controller/metal_test.rb
@@ -121,6 +121,78 @@
     _(span.name).must_equal 'HTTP GET'
   end
 
+  it 'does not set span to error when 404 is raised from controller' do
+    get '/internal_page_not_found'
+
+    _(span.name).must_equal 'ExampleController#internal_page_not_found'
+    _(span.kind).must_equal :server
+    _(span.status.ok?).must_equal true
+
+    _(span.instrumentation_library.name).must_equal 'OpenTelemetry::Instrumentation::Rack'
+    _(span.instrumentation_library.version).must_equal OpenTelemetry::Instrumentation::Rack::VERSION
+
+    _(span.attributes['http.method']).must_equal 'GET'
+    _(span.attributes['http.host']).must_equal 'example.org'
+    _(span.attributes['http.scheme']).must_equal 'http'
+    _(span.attributes['http.target']).must_equal '/internal_page_not_found'
+    _(span.attributes['http.status_code']).must_equal 404
+    _(span.attributes['http.user_agent']).must_be_nil
+    _(span.attributes['code.namespace']).must_equal 'ExampleController'
+    _(span.attributes['code.function']).must_equal 'internal_page_not_found'
+
+    _(span.events.size).must_equal 1
+    _(span.events.first.name).must_equal 'exception'
+    _(span.events.first.attributes['exception.type']).must_equal 'ActionController::RoutingError'
+    _(span.events.first.attributes['exception.message']).must_equal 'Not Found'
+    _(span.events.first.attributes['exception.stacktrace'].nil?).must_equal false
+  end
+
+  it 'does not set span to error when wrong url is requested' do
+    get '/not_found_url'
+
+    _(span.name).must_equal 'HTTP GET'
+    _(span.kind).must_equal :server
+    _(span.status.ok?).must_equal true
+
+    _(span.instrumentation_library.name).must_equal 'OpenTelemetry::Instrumentation::Rack'
+    _(span.instrumentation_library.version).must_equal OpenTelemetry::Instrumentation::Rack::VERSION
+
+    _(span.attributes['http.method']).must_equal 'GET'
+    _(span.attributes['http.host']).must_equal 'example.org'
+    _(span.attributes['http.scheme']).must_equal 'http'
+    _(span.attributes['http.target']).must_equal '/not_found_url'
+    _(span.attributes['http.status_code']).must_equal 404
+    _(span.attributes['http.user_agent']).must_be_nil
+
+    _(span.events).must_be_nil
+  end
+
+  it 'does not set span to error when 422 is raised from controller' do
+    get '/internal_invalid_auth'
+
+    _(span.name).must_equal 'ExampleController#internal_invalid_auth'
+    _(span.kind).must_equal :server
+    _(span.status.ok?).must_equal true
+
+    _(span.instrumentation_library.name).must_equal 'OpenTelemetry::Instrumentation::Rack'
+    _(span.instrumentation_library.version).must_equal OpenTelemetry::Instrumentation::Rack::VERSION
+
+    _(span.attributes['http.method']).must_equal 'GET'
+    _(span.attributes['http.host']).must_equal 'example.org'
+    _(span.attributes['http.scheme']).must_equal 'http'
+    _(span.attributes['http.target']).must_equal '/internal_invalid_auth'
+    _(span.attributes['http.status_code']).must_equal 422
+    _(span.attributes['http.user_agent']).must_be_nil
+    _(span.attributes['code.namespace']).must_equal 'ExampleController'
+    _(span.attributes['code.function']).must_equal 'internal_invalid_auth'
+
+    _(span.events.size).must_equal 1
+    _(span.events.first.name).must_equal 'exception'
+    _(span.events.first.attributes['exception.type']).must_equal 'ActionController::InvalidAuthenticityToken'
+    _(span.events.first.attributes['exception.message']).must_equal 'Invalid Authentication'
+    _(span.events.first.attributes['exception.stacktrace'].nil?).must_equal false
+  end
+
   describe 'when the application has exceptions_app configured' do
     let(:rails_app) { AppConfig.initialize_app(use_exceptions_app: true) }
 

diff --git a/instrumentation/action_pack/test/test_helpers/controllers/example_controller.rb b/instrumentation/action_pack/test/test_helpers/controllers/example_controller.rb
@@ -22,4 +22,12 @@ def new_item
   def internal_server_error
     raise :internal_server_error
   end
+
+  def internal_page_not_found
+    raise ::ActionController::RoutingError.new("Not Found")
+  end
+
+  def internal_invalid_auth
+    raise ::ActionController::InvalidAuthenticityToken.new("Invalid Authentication")
+  end
 end
diff --git a/instrumentation/action_pack/test/test_helpers/routes.rb b/instrumentation/action_pack/test/test_helpers/routes.rb
@@ -11,5 +11,7 @@ def draw_routes(rails_app)
     get '/items/new', to: 'example#new_item'
     get '/items/:id', to: 'example#item'
     get '/internal_server_error', to: 'example#internal_server_error'
+    get '/internal_page_not_found', to: 'example#internal_page_not_found'
+    get '/internal_invalid_auth', to: 'example#internal_invalid_auth'
   end
 end