Do not rename final Grape segment (#2987)

* Do not rename final segment --------- Co-authored-by: Kayla Reopelle <[email protected]>
newrelic · Dec 6, 2024 · fc26239 · fc26239
1 parent 8032ce4
commit fc26239
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # New Relic Ruby Agent Release Notes
 
+## dev
+
+- **Bugfix: Stop renaming final Grape segment**
+
+  Previously, the agent renamed the final segment in Grape transactions to `"Middleware/Grape/#{class_name}/call"`. This was a part of an old instrumentation pattern that is no longer relevant. Many thanks to [@seriousdev-gh](https://github.com/seriousdev-gh) for bringing this issue to our attention and along with a great reproduction and suggested fix. [PR#2987](https://github.com/newrelic/newrelic-ruby-agent/pull/2987).
+
 ## v9.16.1
 
 - **Bugfix: Add support for Trilogy database adapter**

diff --git a/lib/new_relic/agent/instrumentation/grape/instrumentation.rb b/lib/new_relic/agent/instrumentation/grape/instrumentation.rb
@@ -56,10 +56,7 @@ def handle_transaction(endpoint, class_name, version)
 
       def name_transaction(route, class_name, version)
         txn_name = name_for_transaction(route, class_name, version)
-        segment_name = "Middleware/Grape/#{class_name}/call"
         NewRelic::Agent::Transaction.set_default_transaction_name(txn_name, :grape)
-        txn = NewRelic::Agent::Transaction.tl_current
-        txn.segments.last.name = segment_name
       end
 
       def name_for_transaction(route, class_name, version)

diff --git a/test/multiverse/suites/grape/grape_test.rb b/test/multiverse/suites/grape/grape_test.rb
@@ -36,38 +36,38 @@ def test_route_raises_an_error
 
       expected_txn_name = 'Controller/Grape/GrapeTestApi/self_destruct (GET)'
 
-      assert_grape_metrics(expected_txn_name)
+      assert_metrics_recorded(expected_txn_name)
       assert_metrics_recorded(["Errors/#{expected_txn_name}"])
     end
 
     def test_getting_a_list_of_grape_apes
       get('/grape_ape')
 
-      assert_grape_metrics('Controller/Grape/GrapeTestApi/grape_ape (GET)')
+      assert_metrics_recorded('Controller/Grape/GrapeTestApi/grape_ape (GET)')
     end
 
     def test_showing_a_grape_ape
       get('/grape_ape/1')
 
-      assert_grape_metrics('Controller/Grape/GrapeTestApi/grape_ape/:id (GET)')
+      assert_metrics_recorded('Controller/Grape/GrapeTestApi/grape_ape/:id (GET)')
     end
 
     def test_creating_a_grape_ape
       post('/grape_ape', {})
 
-      assert_grape_metrics('Controller/Grape/GrapeTestApi/grape_ape (POST)')
+      assert_metrics_recorded('Controller/Grape/GrapeTestApi/grape_ape (POST)')
     end
 
     def test_updating_a_grape_ape
       put('/grape_ape/1', {})
 
-      assert_grape_metrics('Controller/Grape/GrapeTestApi/grape_ape/:id (PUT)')
+      assert_metrics_recorded('Controller/Grape/GrapeTestApi/grape_ape/:id (PUT)')
     end
 
     def test_deleting_a_grape_ape
       delete('/grape_ape/1')
 
-      assert_grape_metrics('Controller/Grape/GrapeTestApi/grape_ape/:id (DELETE)')
+      assert_metrics_recorded('Controller/Grape/GrapeTestApi/grape_ape/:id (DELETE)')
     end
 
     def test_transaction_renaming
@@ -81,7 +81,7 @@ def test_transaction_renaming
       # internal representation of the name and category of a transaction as
       # truly separate entities.
       #
-      assert_grape_metrics('Controller/Rack/RenamedTxn')
+      assert_metrics_recorded('Controller/Rack/RenamedTxn')
     end
 
     def test_params_are_not_captured_with_capture_params_disabled
@@ -259,16 +259,6 @@ def test_distinct_routes_make_for_distinct_txn_names
       assert_equal paths.size, names.uniq.size,
         'Expected there to be one unique transaction name per unique full route path'
     end
-
-    def assert_grape_metrics(expected_txn_name)
-      expected_node_name = 'Middleware/Grape/GrapeTestApi/call'
-
-      assert_metrics_recorded([
-        expected_node_name,
-        [expected_node_name, expected_txn_name],
-        expected_txn_name
-      ])
-    end
   end
 end
 
@@ -286,14 +276,7 @@ def app
     def test_subclass_of_grape_api_instance
       get('/banjaxing')
 
-      expected_node_name = 'Middleware/Grape/GrapeApiInstanceTestApi/call'
-      expected_txn_name = 'Controller/Grape/GrapeApiInstanceTestApi/banjaxing (GET)'
-
-      assert_metrics_recorded([
-        expected_node_name,
-        [expected_node_name, expected_txn_name],
-        expected_txn_name
-      ])
+      assert_metrics_recorded('Controller/Grape/GrapeApiInstanceTestApi/banjaxing (GET)')
     end
   end
 end