Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thread Profile / Backtrace 3.4.0 fixes #2997

Merged
merged 6 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 10 additions & 1 deletion lib/new_relic/agent/threading/backtrace_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,16 @@

# 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 (?:`|')(.*)'/
Dismissed Show dismissed Hide dismissed
[$1, $4, $3] # sic
end
end
Expand Down
120 changes: 90 additions & 30 deletions test/new_relic/agent/threading/backtrace_node_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fun instance variable setup!

@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)
Expand Down Expand Up @@ -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,
[]
]
Expand All @@ -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])
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
26 changes: 21 additions & 5 deletions test/new_relic/agent/threading/thread_profile_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Loading