[NO-TICKET] Check extension dir as well when loading profiler

[ivoanjo]

What does this PR do?

This PR tweaks our existing logic for loading the profiling native extension to try to find the file in two directories:

1. The lib/ directory
2. The Ruby extensions directory

Previously we would only check 1.

Motivation:

I'll admit I don't 100% understand when one or the other folder gets used.

For instance, on a fresh install of ddtrace on my machine I get

$ bundle exec gem contents ddtrace | grep so$
.rvm/gems/ruby-3.1.4/gems/ddtrace-1.21.1/lib/datadog_profiling_loader.3.1.4_x86_64-linux.so
.rvm/gems/ruby-3.1.4/gems/ddtrace-1.21.1/lib/datadog_profiling_native_extension.3.1.4_x86_64-linux.so

(this is 1., above)

but also...

$ find `bundle exec ruby -e "require 'ddtrace'; puts Gem::loaded_specs['ddtrace'].extension_dir"` | grep so$
.rvm/gems/ruby-3.1.4/extensions/x86_64-linux/3.1.0/ddtrace-1.21.1/datadog_profiling_native_extension.3.1.4_x86_64-linux.so
.rvm/gems/ruby-3.1.4/extensions/x86_64-linux/3.1.0/ddtrace-1.21.1/datadog_profiling_loader.3.1.4_x86_64-linux.so

(this is 2., above)

(Aside: These are not leftovers -- if I uninstall the gem, the files get removed, and get recreated when I install them.)

So, on my machine, the files get installed to two different folders.

And for most of our customers, I'm pretty sure they get installed into 1. because otherwise the profiler won't work.

But one customer reported the following error:

Profiling was requested but is not supported, profiling disabled: There was an error loading the profiling native extension due to 'RuntimeError Failure to load ddtrace_profiling_native_extension.3.2.2_x86_64-linux due to /var/app/current/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.20.0/lib/datadog/profiling/../../ddtrace_profiling_native_extension.3.2.2_x86_64-linux.so: cannot open shared object file: No such file or directory' at '/var/app/current/vendor/bundle/ruby/3.2.0/gems/ddtrace-1.20.0/lib/datadog/profiling/load_native_extension.rb:26:in <top (required)>''`

Now my first intuition was to suspect -- something is wrong, the native extension didn't get built. But, actually, that's not the case, and here's a very important detail that took me a while to fully understand: the error message says that the
ddtrace_profiling_native_extension.3.2.2_x86_64-linux.so was not found, not the datadog_profiling_loader.3.2.2_x86_64-linux.so.

This distinction is important, because it's the loader that attempts to load the extension. So if the loader didn't fail first, it means the loader was found; but the extension wasn't.

With a bit more input from the customer, and some testing on my side, it looks like Ruby puts both folders above (1. and 2.) in the $LOAD_PATH, so when using require to load a file, both get checked.

And for this one customer, the extension only existed in folder 2., not 1. (even after reinstalling the gem).

Since we use a regular require to load the loader, it was still found, regardless of where it was. BUT because for the extension we try to load by using a specific path, and that path only checked lib (1.), then the extension failed to load.

This is why the customer saw an error message that pointed to the loader working, but the extension not being found.

Now because our previous logic of only checking folder 1. worked for all other customers we have, I'm not really sure why this one customer had the extension only in folder 2., but not in 1. . But regardless, having either folder in use is something that Ruby does, so I think it's reasonable for us to emulate that logic.

Additional Notes:

If you're curious why we are reimplementing Ruby logic for loading extensions, check the comments on datadog_profiling_loader.c.

TL;DR is, we want to load the library with specific system flags, and Ruby doesn't provide a way to do so, so we need to basically duplicate a bunch of Ruby's library loading logic ourselves to be able to emulate Ruby's behavior but change the system flags.

How to test the change?

I was able to reproduce the issue locally by installing ddtrace, and then manually deleting the files in 1., leaving them only in 2.

Without this change, I saw exactly the same error message as the customer did; with this change, profiling works.

On top of the added specs, I manually tested this fix on Ruby versions 2.3, 3.1 and 3.3.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 97.91667% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 98.24%. Comparing base (07c75b8) to head (c6270c9).
Report is 3 commits behind head on master.

Files	Patch %	Lines
...ec/datadog/profiling/load_native_extension_spec.rb	97.56%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3582      +/-   ##
==========================================
- Coverage   98.24%   98.24%   -0.01%     
==========================================
  Files        1254     1255       +1     
  Lines       74355    74402      +47     
  Branches     3529     3536       +7     
==========================================
+ Hits        73052    73098      +46     
- Misses       1303     1304       +1     

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

[AlexJF]

Love these weird behaviours! LGTM!

[p-datadog]

Should this not use File.join for best results?

[ivoanjo]

Indeed... The profiler really only targets Linux/Unix right now, so a number of linux/unixisms have crept in.

The native extension loader that's getting called using this path, for instance, uses dlopen, so that's another thing that wouldn't work.

So yeah, I'm not sure it's worth trying to piecemeal support all OSs for the profiler in preparation for maybe supporting them in the future, I think it's easier to evaluate the whole thing once we want that (e.g. for instance not using the load_native_extension.rb on Windows at all).