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

Profiler incompatible with Ruby 3.3-head #3053

Closed
ivoanjo opened this issue Aug 17, 2023 · 0 comments · Fixed by #3167
Closed

Profiler incompatible with Ruby 3.3-head #3053

ivoanjo opened this issue Aug 17, 2023 · 0 comments · Fixed by #3167
Assignees
Labels
bug Involves a bug profiling Involves Datadog profiling

Comments

@ivoanjo
Copy link
Member

ivoanjo commented Aug 17, 2023

Current behaviour

Profiler is incompatible with Ruby 3.3-head. It's not possible to build or run the profiler on the latest development Ruby versions.

Expected behaviour

Profiler should work on Ruby 3.3-head.


I'm opening this issue for wider community visibility that we're aware of this issue.

I spotted two problems:

  1. Current ruby-head is incompatible with the 3.3.0-preview1 headers in the debase-ruby_core_source gem. This is an easy fix, once preview2/rc1 gets released, the updated headers can be added to the gem.

  2. Even with the correct headers, the profiler is failing to load with

RuntimeError Failure to load ddtrace_profiling_native_extension.3.3.0_x86_64-linux due to ddtrace_profiling_native_extension.3.3.0_x86_64-linux.so: undefined symbol: ruby_current_ec' at 'lib/datadog/profiling/load_native_extension.rb:26

... this one seems a bit more harder to fix.

I plan to open a PR so the profiler is disabled on Ruby 3.3 until we can address these issues.

@ivoanjo ivoanjo added bug Involves a bug community Was opened by a community member labels Aug 17, 2023
ivoanjo added a commit that referenced this issue Aug 17, 2023
**What does this PR do?**:

In #3053 I reported the discovery that the profiler is currently not
working on Ruby 3.3-head (reported by a customer that tests with
head in CI).

A full fix for #3053 seems like it may take a while, so until it's
available, let's gracefully disable the profiler in Ruby 3.3, so that
instead of a compilation failure, customers just get a nice message
saying we don't support 3.3 right now.

**Motivation**:

Improve experience for current customers testing with 3.3-head and
future customers trying to profile 3.3.

**Additional Notes**:

Customers will be shown the following message when trying to profile
on Ruby 3.3:

```
WARN -- ddtrace: [ddtrace] Profiling was requested but is not supported,
profiling disabled: Your ddtrace installation is missing support for the
Continuous Profiler because the profiler in the current version of
ddtrace does not yet support Ruby version 3.3. (See
#3053 for details). Try
upgrading to the latest ddtrace, as this issue may have been fixed by
now. For help solving this issue, please contact Datadog support at
<https://docs.datadoghq.com/help/>. You can also check out the
Continuous Profiler troubleshooting page at
<https://dtdg.co/ruby-profiler-troubleshooting>.
```

**How to test the change?**:

Try to profile ruby-3.3 (any version) and confirm the profiler
disables itself gracefully.
@ivoanjo ivoanjo self-assigned this Aug 18, 2023
@ivoanjo ivoanjo added profiling Involves Datadog profiling and removed community Was opened by a community member labels Aug 18, 2023
ivoanjo added a commit that referenced this issue Sep 27, 2023
**What does this PR do?**:

This PR bumps our dependency on `debase-ruby_core_source` to version
3.2.2. This version is needed to support profiling Ruby 3.3.0-preview2.

**Motivation**:

Support profiling on Ruby 3.3.0-preview2.

**Additional Notes**:

I've updated the appraisal gemfiles as well.

The profiler is currently disabled on Ruby 3.3 (see #3053 for
details). This PR is the first step towards re-enabling it again.

**How to test the change?**:

Validate that CI is green.
ivoanjo added a commit that referenced this issue Sep 28, 2023
**What does this PR do?**

Back in #3053, a customer reported that profiling was not working
on ruby-head (for Ruby 3.3).

At the time, I tracked it down to two issues:
1. A bunch of headers had changed, and thus we needed a new
   release of `debase-ruby_core_source` to include them
2. We were getting errors when loading the profiler due to the
   `ruby_current_ec` VM symbol no longer being available.

At the time, I chose to disable profiling on Ruby 3.3 (#3054) until
we could tackle these two items.

Item 1. was fixed in #3163, that pulls in a new release of
`debase-ruby_core_source` that includes the 3.3-preview2 headers.

This PR fixes item 2. and re-enables profiling on Ruby 3.3
(and thus reverts #3054).

**Motivation:**

Make sure we're in good shape for the Ruby 3.3 final release in
December.

**Additional Notes:**

We didn't use the magic `ruby_current_ec` VM symbol directly.
Instead, we were using `GET_RACTOR()` and `GET_EC()` which call
a few methods that are defined as `inline` in `vm_core.h`.

Those methods all relied on `ruby_current_ec`, and thus hiding
that symbol made them break.

As an alternative, we can get to the same objects by starting
with the current thread, and navigating the object graph a bit.

The `rb_thread_current()` also relies on `ruby_current_ec` but
crucially **is not inline** and is actually a public API of the VM,
so that one kept working.

I hesitated on if we should use `rb_thread_current()` on all
Rubies with Ractors, or if we should keep the fast path for
3.0 to 3.2. I ended up deciding to keep the fast path since
it was quite easy to do, but in the future we could instead
choose to simplify this whole thing.

P.s.: This is one of those PRs where a lot of work went into
a handful of unimpressive lines of code 😅 .

**How to test the change?**

Validate that CI is passing on Ruby 3.3!

Fixes #3053
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug profiling Involves Datadog profiling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant