Merge pull request #2997 from newrelic/backtrace-3-4-0-fixes

Thread Profile / Backtrace 3.4.0 fixes

.rubocop.yml

@@ -877,7 +877,7 @@ Naming/VariableNumber:
 Performance/BindCall:
   Enabled: false
 
-# TODO: OLD RUBIES - Requites 2.5
+# TODO: OLD RUBIES - Requires 2.5
 Performance/DeletePrefix:
   Enabled: false
 

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## dev
 
+- **Feature: Support Ruby 3.4.0**
+
+  The agent now supports Ruby 3.4.0. We've made incremental changes throughout the preview stage to reach compatibility. This release includes an update to the Thread Profiler for compatibility with Ruby 3.4.0's new backtrace format. [Issue#2992](https://github.com/newrelic/newrelic-ruby-agent/issues/2992) [PR#2997](https://github.com/newrelic/newrelic-ruby-agent/pull/2997)
+
 - **Bugfix: Do not attempt to decorate logs with `nil` messages**
 
   The agent no longer attempts to add New Relic linking metadata to logs with `nil` messages. Thank you, [@arlando](https://github.com/arlando) for bringing this to our attention! [Issue#2985](https://github.com/newrelic/newrelic-ruby-agent/issues/2985) [PR#2986](https://github.com/newrelic/newrelic-ruby-agent/pull/2986)

lib/new_relic/agent/threading/backtrace_node.rb

@@ -125,7 +125,16 @@ def dump_string(indent = 0)
 
         # Returns [filename, method, line number]
         def parse_backtrace_frame(frame)
-          frame =~ /([^:]*)(\:(\d+))?\:in `(.*)'/
+          # TODO: OLD RUBIES - Ruby 3.3
+          # The (?:`|') non-capturing group can be removed when the agent
+          # drops support for Ruby 3.3
+          # This group is used to capture the pre-Ruby 3.4.0 backtrace syntax.
+          # Example frame:
+          # Ruby 3.3.0 and below
+          # "irb.rb:69:in `catch'"
+          # Ruby 3.4.0+
+          # "irb.rb:69:in 'Kernel#catch'"
+          frame =~ /([^:]*)(\:(\d+))?\:in (?:`|')(.*)'/
           [$1, $4, $3] # sic
         end
       end

test/new_relic/agent/threading/backtrace_node_test.rb

@@ -7,15 +7,22 @@
 
 module NewRelic::Agent::Threading
   class BacktraceNodeTest < Minitest::Test
-    SINGLE_LINE = "irb.rb:69:in `catch'"
-
     def setup
+      @single_line = ruby_3_4_0_or_above? ? "irb.rb:69:in 'Kernel#catch'" : "irb.rb:69:in `catch'"
       @node = BacktraceRoot.new
-      @single_trace = [
-        "irb.rb:69:in `catch'",
-        "irb.rb:69:in `start'",
-        "irb:12:in `<main>'"
-      ]
+      @single_trace = if ruby_3_4_0_or_above?
+        [
+          "irb.rb:69:in 'Kernel#catch'",
+          "irb.rb:69:in 'Object#start'",
+          "irb:12:in '<main>'"
+        ]
+      else
+        [
+          "irb.rb:69:in `catch'",
+          "irb.rb:69:in `start'",
+          "irb:12:in `<main>'"
+        ]
+      end
     end
 
     def assert_backtrace_trees_equal(a, b, original_a = a, original_b = b)
@@ -43,31 +50,49 @@ def convert_nodes_to_array(nodes)
     end
 
     def test_single_node_converts_to_array
-      line = "irb.rb:69:in `catch'"
+      line = ruby_3_4_0_or_above? ? "irb.rb:69:in 'Kernel#catch'" : "irb.rb:69:in `catch'"
       node = BacktraceNode.new(line)
       convert_nodes_to_array([node])
 
+      expected = if ruby_3_4_0_or_above?
+        ['irb.rb', 'Kernel#catch', 69]
+      else
+        ['irb.rb', 'catch', 69]
+      end
+
       assert_equal([
-        ['irb.rb', 'catch', 69],
+        expected,
         0, 0,
         []
       ],
         node.as_array)
     end
 
     def test_multiple_nodes_converts_to_array
-      line = "irb.rb:69:in `catch'"
-      child_line = "bacon.rb:42:in `yum'"
+      line = ruby_3_4_0_or_above? ? "irb.rb:69:in 'Kernel#catch'" : "irb.rb:69:in `catch'"
+      child_line = ruby_3_4_0_or_above? ? "bacon.rb:42:in 'Bacon#yum'" : "bacon.rb:42:in `yum'"
       node = create_node(line)
       child_node = create_node(child_line, node)
       convert_nodes_to_array([node, child_node])
 
+      expected_1 = if ruby_3_4_0_or_above?
+        ['irb.rb', 'Kernel#catch', 69]
+      else
+        ['irb.rb', 'catch', 69]
+      end
+
+      expected_2 = if ruby_3_4_0_or_above?
+        ['bacon.rb', 'Bacon#yum', 42]
+      else
+        ['bacon.rb', 'yum', 42]
+      end
+
       assert_equal([
-        ['irb.rb', 'catch', 69],
+        expected_1,
         0, 0,
         [
           [
-            ['bacon.rb', 'yum', 42],
+            expected_2,
             0, 0,
             []
           ]
@@ -76,21 +101,30 @@ def test_multiple_nodes_converts_to_array
         node.as_array)
     end
 
+    def test_nodes_in_ruby_3_4_work_with_class_methods
+    end
+
     def test_nodes_without_line_numbers
-      line = "transaction_sample_buffer.rb:in `visit_node'"
+      line = ruby_3_4_0_or_above? ? "transaction_sample_buffer.rb:in 'TransactionSampleBuffer#visit_node'" : "transaction_sample_buffer.rb:in `visit_node'"
       node = create_node(line)
       convert_nodes_to_array([node])
 
+      expected = if ruby_3_4_0_or_above?
+        ['transaction_sample_buffer.rb', 'TransactionSampleBuffer#visit_node', -1]
+      else
+        ['transaction_sample_buffer.rb', 'visit_node', -1]
+      end
+
       assert_equal([
-        ['transaction_sample_buffer.rb', 'visit_node', -1],
+        expected,
         0, 0,
         []
       ],
         node.as_array)
     end
 
     def test_gracefully_handle_bad_values_in_to_array
-      node = BacktraceNode.new(SINGLE_LINE)
+      node = BacktraceNode.new(@single_line)
       node.stubs(:parse_backtrace_frame).returns(['irb.rb', 'catch', 'blarg'])
       node.runnable_count = Rational(10, 1)
       convert_nodes_to_array([node])
@@ -104,8 +138,8 @@ def test_gracefully_handle_bad_values_in_to_array
     end
 
     def test_add_child_twice
-      parent = BacktraceNode.new(SINGLE_LINE)
-      child = BacktraceNode.new(SINGLE_LINE)
+      parent = BacktraceNode.new(@single_line)
+      child = BacktraceNode.new(@single_line)
 
       parent.add_child_unless_present(child)
       parent.add_child_unless_present(child)
@@ -137,17 +171,33 @@ def test_aggregate_builds_tree_from_overlapping_traces
     end
 
     def test_aggregate_builds_tree_from_diverging_traces
-      backtrace1 = [
-        "baz.rb:3:in `baz'",
-        "bar.rb:2:in `bar'",
-        "foo.rb:1:in `foo'"
-      ]
-
-      backtrace2 = [
-        "wiggle.rb:3:in `wiggle'",
-        "qux.rb:2:in `qux'",
-        "foo.rb:1:in `foo'"
-      ]
+      backtrace1 = if ruby_3_4_0_or_above?
+        [
+          "baz.rb:3:in 'Object#baz'",
+          "bar.rb:2:in 'Object#bar'",
+          "foo.rb:1:in 'Object#foo'"
+        ]
+      else
+        [
+          "baz.rb:3:in `baz'",
+          "bar.rb:2:in `bar'",
+          "foo.rb:1:in `foo'"
+        ]
+      end
+
+      backtrace2 = if ruby_3_4_0_or_above?
+        [
+          "wiggle.rb:3:in 'Object#wiggle'",
+          "qux.rb:2:in 'Object#qux'",
+          "foo.rb:1:in 'Object#foo'"
+        ]
+      else
+        [
+          "wiggle.rb:3:in `wiggle'",
+          "qux.rb:2:in `qux'",
+          "foo.rb:1:in `foo'"
+        ]
+      end
 
       @node.aggregate(backtrace1)
       @node.aggregate(backtrace2)
@@ -178,12 +228,22 @@ def test_aggregate_doesnt_create_duplicate_children
     end
 
     def test_aggregate_limits_recorded_depth
-      deep_backtrace = (0..2000).to_a.map { |i| "foo.rb:#{i}:in `foo'" }
+      deep_backtrace = (0..2000).to_a.map { |i| ruby_3_4_0_or_above? ? "foo.rb:#{i}:in 'Foo#foo'" : "foo.rb:#{i}:in `foo'" }
 
       root = BacktraceRoot.new
       root.aggregate(deep_backtrace)
 
       assert_equal(MAX_THREAD_PROFILE_DEPTH, root.flattened.size)
     end
+
+    private
+
+    # TODO: OLD RUBIES < 3.4
+    # Ruby 3.4 introduced a new format for backtraces
+    # These tests have examples with the old the and new formats
+    # When we drop support for Ruby 3.3, we can remove the condition
+    def ruby_3_4_0_or_above?
+      Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.4.0')
+    end
   end
 end

test/new_relic/agent/threading/thread_profile_test.rb

@@ -16,11 +16,19 @@ class ThreadProfileTest < Minitest::Test
       def setup
         setup_fake_threads
 
-        @single_trace = [
-          "irb.rb:69:in `catch'",
-          "irb.rb:69:in `start'",
-          "irb:12:in `<main>'"
-        ]
+        @single_trace = if ruby_3_4_0_or_above?
+          [
+            "irb.rb:69:in 'Kernel#catch'",
+            "irb.rb:69:in 'Object#start'",
+            "irb:12:in '<main>'"
+          ]
+        else
+          [
+            "irb.rb:69:in `catch'",
+            "irb.rb:69:in `start'",
+            "irb:12:in `<main>'"
+          ]
+        end
 
         @profile = ThreadProfile.new
 
@@ -240,6 +248,14 @@ def rec_count_tree_nodes(tree)
 
         count
       end
+
+      # TODO: OLD RUBIES < 3.4
+      # Ruby 3.4 introduced a new format for backtraces
+      # These tests have examples with the old the and new formats
+      # When we drop support for Ruby 3.3, we can remove the condition
+      def ruby_3_4_0_or_above?
+        Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.4.0')
+      end
     end
   end
 end