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

[PROF-10967] Fix profiler not loading due to "rb_obj_info" symbol not found #4161

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Nov 26, 2024

What does this PR do?

This PR fixes a bug introduced in #4020, specifically in f581076 .

We started using the rb_obj_info to print debug information about objects in some cases, BUT I failed to notice that this API is not really available on Ruby 2.5 and 3.3 (but is on all others, which is why it tripped me).

This manifested in the following error reported by a customer:

WARN -- datadog: [datadog] Profiling was requested but is not supported, profiling disabled: There was an error loading the profiling native extension due to 'RuntimeError Failure to load datadog_profiling_native_extension.3.3.5_x86_64-linux-musl due to Error relocating /app/vendor/bundle/ruby/3.3.0/gems/datadog-2.6.0/lib/datadog/profiling/../../datadog_profiling_native_extension.3.3.5_x86_64-linux-musl.so: rb_obj_info: symbol not found' at '/app/vendor/bundle/ruby/3.3.0/gems/datadog-2.6.0/lib/datadog/profiling/load_native_extension.rb:41:in `<main>''

This PR fixes this issue by never referencing rb_obj_info on those Rubies. Since this API is only used for printing information during errors, this should be fine (and is better than the alternative of not printing info on any Rubies).

Motivation:

Fix profiling not loading in certain situations on Ruby 2.5 and 3.3.

Change log entry

Fix profiling not loading in certain situations on Ruby 2.5 and 3.3

Additional Notes:

Interestingly, this issue did not show up on glibc systems. I guess musl libc is being a bit more eager about trying to resolve symbols?

How to test the change?

This change includes test coverage. Disabling the added check in extconf.rb will produce a failing test.

… found

**What does this PR do?**

This PR fixes a bug introduced in #4020, specifically in f581076 .

We started using the `rb_obj_info` to print debug information about
objects in some cases, BUT I failed to notice that this API is not
really available on Ruby 2.5 and 3.3 (but is on all others, which
is why it tripped me).

This manifested in the following error reported by a customer:

> WARN -- datadog: [datadog] Profiling was requested but is not
> supported, profiling disabled: There was an error loading the
> profiling native extension due to 'RuntimeError Failure to load
> datadog_profiling_native_extension.3.3.5_x86_64-linux-musl due
> to Error relocating
> /app/vendor/bundle/ruby/3.3.0/gems/datadog-2.6.0/lib/datadog/profiling/../../datadog_profiling_native_extension.3.3.5_x86_64-linux-musl.so:
> rb_obj_info: symbol not found' at
> '/app/vendor/bundle/ruby/3.3.0/gems/datadog-2.6.0/lib/datadog/profiling/load_native_extension.rb:41:in `<main>''

This PR fixes this issue by never referencing `rb_obj_info` on
those Rubies. Since this API is only used for printing information
during errors, this should be fine (and is better than the alternative
of not printing info on any Rubies).

**Motivation:**

Fix profiling not loading in certain situations on Ruby 2.5 and 3.3.

**Additional Notes:**

Interestingly, this issue did not show up on glibc systems. I guess
musl libc is being a bit more eager about trying to resolve symbols?

**How to test the change?**

This change includes test coverage. Disabling the added check
in `extconf.rb` will produce a failing test.
@ivoanjo ivoanjo requested review from a team as code owners November 26, 2024 12:42
@github-actions github-actions bot added the profiling Involves Datadog profiling label Nov 26, 2024
Copy link
Member

@Strech Strech left a comment

Choose a reason for hiding this comment

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

LGTM 👏🏼

The `rb_obj_info` got re-exposed later in the 3.4 development cycle,
so our test that was still running with 3.4.0-preview1 was failing
because it still had not been changed.
@ivoanjo ivoanjo enabled auto-merge November 26, 2024 12:56
@ivoanjo ivoanjo added this to the 2.8.0 milestone Nov 26, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.78%. Comparing base (77b9d8f) to head (94104b8).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4161      +/-   ##
==========================================
+ Coverage   97.77%   97.78%   +0.01%     
==========================================
  Files        1353     1353              
  Lines       81785    81796      +11     
  Branches     4141     4143       +2     
==========================================
+ Hits        79963    79983      +20     
+ Misses       1822     1813       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ivoanjo ivoanjo merged commit c5232cb into master Nov 26, 2024
287 of 289 checks passed
@ivoanjo ivoanjo deleted the ivoanjo/prof-10967-fix-rb-obj-info-usage branch November 26, 2024 13:13
@ivoanjo ivoanjo added the bug Involves a bug label Nov 28, 2024
TonyCTHsu pushed a commit that referenced this pull request Nov 28, 2024
…nfo-usage

[PROF-10967] Fix profiler not loading due to "rb_obj_info" symbol not found
@TonyCTHsu TonyCTHsu mentioned this pull request Nov 28, 2024
@ivoanjo ivoanjo modified the milestones: 2.8.0, 2.7.1 Dec 5, 2024
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 this pull request may close these issues.

3 participants