feat: name ActionPack spans with the HTTP method and route

instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/patches/action_controller/metal.rb

@@ -14,9 +14,14 @@ module Metal
             def dispatch(name, request, response)
               rack_span = OpenTelemetry::Instrumentation::Rack.current_span
               if rack_span.recording?
-                rack_span.name = "#{self.class.name}##{name}" unless request.env['action_dispatch.exception']
+                unless request.env['action_dispatch.exception']
+                  rack_span.name = case instrumentation_config[:span_naming]
+                                   when :controller_action then "#{self.class.name}##{name}"
+                                   else "#{request.method} #{rails_route(request)}"
+                                   end
+                end
 
-                add_rails_route(rack_span, request) if instrumentation_config[:enable_recognize_route]
+                rack_span.set_attribute('http.route', rails_route(request)) if instrumentation_config[:enable_recognize_route]
 
                 rack_span.set_attribute('http.target', request.filtered_path) if request.filtered_path != request.fullpath
               end
@@ -26,11 +31,10 @@ def dispatch(name, request, response)
 
             private
 
-            def add_rails_route(rack_span, request)
-              ::Rails.application.routes.router.recognize(request) do |route, _params|
-                rack_span.set_attribute('http.route', route.path.spec.to_s)
+            def rails_route(request)
+              @rails_route ||= ::Rails.application.routes.router.recognize(request) do |route, _params|
+                return route.path.spec.to_s
                 # Rails will match on the first route - see https://guides.rubyonrails.org/routing.html#crud-verbs-and-actions
-                break
               end
             end
 

instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/patches/action_controller/metal_test.rb

@@ -25,7 +25,7 @@
 
     _(last_response.body).must_equal 'actually ok'
     _(last_response.ok?).must_equal true
-    _(span.name).must_equal 'ExampleController#ok'
+    _(span.name).must_equal 'GET /ok(.:format)'
     _(span.kind).must_equal :server
     _(span.status.ok?).must_equal true
 
@@ -41,10 +41,32 @@
     _(span.attributes['http.route']).must_be_nil
   end
 
+  it 'does not memoize data across requests' do
+    get '/ok'
+    get '/items/new'
+
+    _(last_response.body).must_equal 'created new item'
+    _(last_response.ok?).must_equal true
+    _(span.name).must_equal 'GET /items/new(.:format)'
+    _(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 '/items/new'
+    _(span.attributes['http.status_code']).must_equal 200
+    _(span.attributes['http.user_agent']).must_be_nil
+    _(span.attributes['http.route']).must_be_nil
+  end
+
   it 'sets the span name when the controller raises an exception' do
     get 'internal_server_error'
 
-    _(span.name).must_equal 'ExampleController#internal_server_error'
+    _(span.name).must_equal 'GET /internal_server_error(.:format)'
   end
 
   it 'does not set the span name when an exception is raised in middleware' do
@@ -59,13 +81,47 @@
     _(span.name).must_equal 'HTTP GET'
   end
 
+  describe 'when the application has span_naming set with controller_action' do
+    before do
+      OpenTelemetry::Instrumentation::ActionPack::Instrumentation.instance.config[:span_naming] = :controller_action
+    end
+
+    after do
+      OpenTelemetry::Instrumentation::ActionPack::Instrumentation.instance.config[:span_naming] = nil
+    end
+
+    it 'sets the span name to the HTTP method and route' do
+      get '/ok'
+
+      _(span.name).must_equal 'ExampleController#ok'
+    end
+
+    it 'sets the span name when the controller raises an exception' do
+      get 'internal_server_error'
+
+      _(span.name).must_equal 'ExampleController#internal_server_error'
+    end
+
+    it 'does not set the span name when an exception is raised in middleware' do
+      get '/ok?raise_in_middleware'
+
+      _(span.name).must_equal 'HTTP GET'
+    end
+
+    it 'does not set the span name when the request is redirected in middleware' do
+      get '/ok?redirect_in_middleware'
+
+      _(span.name).must_equal 'HTTP GET'
+    end
+  end
+
   describe 'when the application has exceptions_app configured' do
     let(:rails_app) { AppConfig.initialize_app(use_exceptions_app: true) }
 
     it 'does not overwrite the span name from the controller that raised' do
       get 'internal_server_error'
 
-      _(span.name).must_equal 'ExampleController#internal_server_error'
+      _(span.name).must_equal 'GET /internal_server_error(.:format)'
     end
   end
 
@@ -82,7 +138,7 @@
       get '/items/new'
       _(last_response.body).must_equal 'created new item'
       _(last_response.ok?).must_equal true
-      _(span.name).must_equal 'ExampleController#new_item'
+      _(span.name).must_equal 'GET /items/new(.:format)'
       _(span.kind).must_equal :server
       _(span.status.ok?).must_equal true
 

instrumentation/rails/test/instrumentation/opentelemetry/instrumentation/rails/patches/action_controller/metal_test.rb

@@ -22,7 +22,7 @@
 
     _(last_response.body).must_equal 'actually ok'
     _(last_response.ok?).must_equal true
-    _(span.name).must_equal 'ExampleController#ok'
+    _(span.name).must_equal 'GET /ok(.:format)'
     _(span.kind).must_equal :server
     _(span.status.ok?).must_equal true