fix: update ActionController handler's span name to follow OTel conve…

…ntions Resolves: open-telemetry#961.
karmingc · May 8, 2024 · 61d534b · 61d534b
1 parent 92277bd
commit 61d534b
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 5 deletions.
diff --git a/...n/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb b/...n/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb
@@ -26,7 +26,7 @@ def start(_name, _id, payload)
 
             request = payload[:request]
 
-            rack_span.name = "#{payload[:controller]}##{payload[:action]}" unless request.env['action_dispatch.exception']
+            rack_span.name = span_name(payload) unless request.env['action_dispatch.exception']
 
             attributes_to_append = {
               OpenTelemetry::SemanticConventions::Trace::CODE_NAMESPACE => String(payload[:controller]),
@@ -52,6 +52,16 @@ def finish(_name, _id, payload)
           rescue StandardError => e
             OpenTelemetry.handle_error(exception: e)
           end
+
+          private
+
+          def span_name(payload)
+            request = payload[:request]
+
+            return "#{request.method} /#{payload[:controller].underscore}/#{payload[:action]}" if ::Rails.version < '7.1'
+
+            "#{request.method} #{request.route_uri_pattern}".gsub('(.:format)', '')
+          end
         end
       end
     end

diff --git a/...on_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb b/...on_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb
@@ -36,7 +36,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'
     _(span.kind).must_equal :server
     _(span.status.ok?).must_equal true
 
@@ -65,7 +65,7 @@
 
     _(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'
     _(span.kind).must_equal :server
     _(span.status.ok?).must_equal true
 
@@ -85,7 +85,7 @@
   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'
     _(span.kind).must_equal :server
     _(span.status.ok?).must_equal false
 
@@ -145,7 +145,7 @@
     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'
       _(span.kind).must_equal :server
       _(span.status.ok?).must_equal false