Include ruby class names in frame name output for ruby 3.3.0+ #397

dalehamel · 2025-08-20T20:49:50Z

Fixes #379

Updates bindgen ruby structs for 3.3.0+ to include the necessary structs for parsing out the class name. 3.3.0 was the first release to include ruby/ruby@abff5f6, which makes looking up the classpath trivial compared to prior versions, where it was stored as an instance variable.

I used the method that @acj had been pursuing in his comment on #379, but updated it for ruby refactoring over the last 2 years. I copied the algorithm that ruby/ruby is doing, I was able to leverage some code that already existed for checking the method entry for c functions.

Note - I had to update the test fixtures / sample stacks for ruby 3.3.0+ as they now include the class name! (Object#, defined at the top level).

Here's a basic test program, and i've added a more complete one in rbspy/rbspy-testdata#7, but I had to fix a bug in how we were reading the absolute path in order to get that to work.

class Foo
  class Bar
    def bar()
      loop {}
    end
  end
end
Foo::Bar.new.bar

$ sudo cargo run -- snapshot --pid 75198
<main> - /Users/dalehamel/src/github.com/rbspy/rbspy/test.rb:15
Foo::Bar#bar - /Users/dalehamel/src/github.com/rbspy/rbspy/test.rb:12
loop - unknown:173

dalehamel · 2025-08-20T21:37:31Z

src/core/ruby_version.rs

    )
 );

+macro_rules! get_stack_frame_3_3_0(


The classext approach used here is only valid in ruby 3.3.0+, it may be possible to get the info in another way in earlier versions, but the struct i'm using wasn't added until 3.3.0

ruby/ruby@abff5f6

src/core/ruby_version.rs

dalehamel · 2025-08-21T03:16:54Z

src/core/ruby_version.rs

+            ).context("couldn't get ruby string from iseq body")?;
+
+            let method_name = get_ruby_string(body.location.label as usize, source)?;
+            let name = if class_name != "" {


again this is a bit lazy but it is simple for composing the class name with the method name.

I guess it needs to be a bit smarter to match https://github.com/ruby/ruby/blob/master/vm_backtrace.c#L1974-L1975, and check if it is a singleton or not and use "." instead of "#"? I'll see if i can add that easily enough, seems like it uses a bitmask on the flags attribute or something?

dalehamel · 2025-08-21T13:39:28Z

@acj when you have a sec, please take a look! I was hoping to have more complete tests but as i said in the desc, couldn't get the coredump tests to work for a new coredump with a more complete test file.

Technically the 3.3.0 test is updated and includes classes for the top level methods (eg, aaa -> Object#aaa), but i wanted to have something with nested classes, etc. as i mentioned in rbspy/rbspy-testdata#7

dalehamel · 2025-08-22T20:03:56Z

FYI i think there is a legit issue here, but haven't pinned it down yet - the issue i'm seeing with the coredump tests seems to be related to reading the class name. We might need some more checks for safety, will try and repro more locally.

dalehamel · 2025-08-25T15:12:49Z

FYI i think there is a legit issue here, but haven't pinned it down yet - the issue i'm seeing with the coredump tests seems to be related to reading the class name. We might need some more checks for safety, will try and repro more locally.

Ok i think i've fixed this, I also refactored reading the class names into their own function, and improved the error handling.

dalehamel · 2025-08-25T17:23:44Z

Cargo.toml

 [dev-dependencies]
 byteorder = "1.4.3"
-rbspy-testdata = "0.2.2"
+rbspy-testdata = { git = "https://github.com/dalehamel/rbspy-testdata.git", branch = "class-data" }


Ideally we'd land this patch first, release a new version, then update this before merging.

I just tagged and published 0.2.4 with the enriched 3.3.0 coredump.

dalehamel · 2025-08-25T17:25:01Z

src/core/ruby_version.rs

+    )
+);
+
+macro_rules! get_ruby_string_array_3_3_0(


Needed to add these as I was getting a string allocation error when trying to read the absolute path in my coredump, rstring was reporting its size as enormous (might have been reported as max string length, or max string length -1?)

I added a sanity test where we can read the value only if it doesn't exceed a maximum size, and then pass 4096 as that is the maximum path size on linux.

Sounds good. There are a few places in the code where we're defensive about unreasonably large values, either clamping them to a sane value or dropping them depending on the situation.

test ui::summary::tests::stats_by_line_number ... ok memory allocation of 139633090689240 bytes failed error: test failed, to rerun pass `--lib` Caused by: process didn't exit successfully: `/home/runner/work/rbspy/rbspy/target/x86_64-unknown-linux-gnu/release/deps/rbspy-310591bc6f33f4f3` (signal: 6, SIGABRT: process abort signal

Hmm another test failure with this memory allocation issue... but it doesn't seem related to the code change here 🤨 looks like another function might need to add the maximum string read

dalehamel · 2025-08-25T17:25:24Z

src/core/ruby_version.rs

+            let rel_path = get_ruby_string(path_addr, source)?;
+            // In the case of internal ruby functions (and maybe others), we may not get a valid
+            // pointer here
+            let abs_path = get_ruby_string_limit(abs_path_addr, 4096, source)


function is identical to 3_2 except for this line.

dalehamel · 2025-08-25T17:37:18Z

freebsd failure seems like a flake

acj · 2025-08-31T01:09:00Z

I think the FreeBSD failure is a workflow bug that I introduced recently. I'll take a closer look.

acj · 2025-08-31T01:56:22Z

Okay, the CI bug should be fixed on main, so please rebase. I ran your branch here with the testdata version bump and everything was green.

I need to review the version-specific code more closely and do some local testing, but I'm optimistic that we'll be able to merge this in the next week. Thanks for working on it!

dalehamel · 2025-09-02T13:25:20Z

Thanks @acj

Okay, the CI bug should be fixed on main, so please rebase.

FYI looks like you only fixed that on your fork, didn't push to main on rbspy/rbspy yet. I fetched and rebased your main though.

I ran your branch here with the testdata version bump and everything was green.

Awesome, I moved the testdata back to the versioned crate. Commit looks the same as acj@9cb6983, including the lockfile bumps.

I need to review the version-specific code more closely and do some local testing, but I'm optimistic that we'll be able to merge this in the next week. Thanks for working on it!

Sounds great! Please give it a good look and if you can, try it on a real workload - I haven't had a chance to yet.

acj

This has worked well in my local testing. I'll try to find a good production load to test it on this week, but that doesn't need to hold us up.

src/core/ruby_version.rs

dalehamel commented Aug 20, 2025

View reviewed changes

dalehamel force-pushed the class-names branch from d62e2e5 to 55bb5d3 Compare August 21, 2025 03:01

dalehamel commented Aug 21, 2025

View reviewed changes

src/core/ruby_version.rs Outdated Show resolved Hide resolved

dalehamel commented Aug 21, 2025

View reviewed changes

src/core/ruby_version.rs Outdated Show resolved Hide resolved

dalehamel commented Aug 21, 2025

View reviewed changes

dalehamel marked this pull request as ready for review August 21, 2025 13:38

dalehamel changed the title ~~Add ruby class names~~ Include ruby class names in frame name output for ruby 3.3.0+ Aug 21, 2025

dalehamel force-pushed the class-names branch from 1b25f15 to 55bb5d3 Compare August 22, 2025 20:02

dalehamel commented Aug 25, 2025

View reviewed changes

dalehamel mentioned this pull request Aug 27, 2025

[feat] Include class name for ruby method calls open-telemetry/opentelemetry-ebpf-profiler#714

Open

CI: Fix cross install failure when there's a partial cache hit

89f8c66

dalehamel and others added 6 commits September 2, 2025 09:21

Add ruby class names

de81a75

Regenerate ruby C bindings

3ed2e67

Update stack test traces for 3.3.0+ to include Object#

7f48e4f

Refactor extraction of classname into its own function

d64ff9f

Add coredump test for ready ruby class names

55e152d

Move rbspy-testdata back to versioned crate

cc6de92

dalehamel force-pushed the class-names branch from a41043e to cc6de92 Compare September 2, 2025 13:21

dalehamel mentioned this pull request Sep 2, 2025

Obtain execution context from TLS - is it doable? #398

Open

acj reviewed Sep 8, 2025

View reviewed changes

src/core/ruby_version.rs Outdated Show resolved Hide resolved

Simplify error handling when reading classname

665890d

acj approved these changes Sep 9, 2025

View reviewed changes

acj merged commit 63ed893 into rbspy:main Sep 9, 2025
9 checks passed

dalehamel mentioned this pull request Sep 29, 2025

[bug] Mislabeled frames when class is detected #399

Closed

Include ruby class names in frame name output for ruby 3.3.0+ #397

Include ruby class names in frame name output for ruby 3.3.0+ #397

Uh oh!

Conversation

dalehamel commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dalehamel commented Aug 21, 2025

Uh oh!

dalehamel commented Aug 22, 2025

Uh oh!

dalehamel commented Aug 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dalehamel commented Aug 25, 2025

Uh oh!

acj commented Aug 31, 2025

Uh oh!

acj commented Aug 31, 2025

Uh oh!

dalehamel commented Sep 2, 2025

Uh oh!

acj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dalehamel commented Aug 20, 2025 •

edited

Loading