-
Notifications
You must be signed in to change notification settings - Fork 375
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-5747] Fix broken libddprof linking on Heroku and AWS Elastic Beanstalk #2125
Changes from all commits
e8a9387
03ae999
88a7d4d
fd4621e
0aa109c
389eb53
3020a57
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
# typed: ignore | ||
|
||
require 'libddprof' | ||
require 'pathname' | ||
|
||
module Datadog | ||
module Profiling | ||
|
@@ -20,6 +21,51 @@ def self.fail_install_if_missing_extension? | |
ENV[ENV_FAIL_INSTALL_IF_MISSING_EXTENSION].to_s.strip.downcase == 'true' | ||
end | ||
|
||
# Used as an workaround for a limitation with how dynamic linking works in environments where ddtrace and | ||
# libddprof are moved after the extension gets compiled. | ||
# | ||
# Because the libddpprof native library is installed on a non-standard system path, in order for it to be | ||
# found by the system dynamic linker (e.g. what takes care of dlopen(), which is used to load the profiling | ||
# native extension), we need to add a "runpath" -- a list of folders to search for libddprof. | ||
# | ||
# This runpath gets hardcoded at native library linking time. You can look at it using the `readelf` tool in | ||
# Linux: e.g. `readelf -d ddtrace_profiling_native_extension.2.7.3_x86_64-linux.so`. | ||
# | ||
# In ddtrace 1.1.0, we only set as runpath an absolute path to libddprof. (This gets set automatically by the call | ||
# to `pkg_config('ddprof_ffi_with_rpath')` in `extconf.rb`). This worked fine as long as libddprof was **NOT** | ||
# moved from the folder it was present at ddtrace installation/linking time. | ||
# | ||
# Unfortunately, environments such as Heroku and AWS Elastic Beanstalk move gems around in the filesystem after | ||
# installation. Thus, the profiling native extension could not be loaded in these environments | ||
# (see https://github.com/DataDog/dd-trace-rb/issues/2067) because libddprof could not be found. | ||
# | ||
# To workaround this issue, this method computes the **relative** path between the folder where the profiling | ||
# native extension is going to be installed and the folder where libddprof is installed, and returns it | ||
# to be set as an additional runpath. (Yes, you can set multiple runpath folders to be searched). | ||
# | ||
# This way, if both gems are moved together (and it turns out that they are in these environments), | ||
# the relative path can still be traversed to find libddprof. | ||
# | ||
# This is incredibly awful, and it's kinda bizarre how it's not possible to just find these paths at runtime | ||
# and set them correctly; rather than needing to set stuff at linking-time and then praying to $deity that | ||
# weird moves don't happen. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good note, it's completely possible for a sufficiently bizarre setup to still fail. |
||
# | ||
# As a curiosity, `LD_LIBRARY_PATH` can be used to influence the folders that get searched but **CANNOT BE | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good note, since this is one of the obvious ways of avoiding this headache but it doesn't work. |
||
# SET DYNAMICALLY**, e.g. it needs to be set at the start of the process (Ruby VM) and thus it's not something | ||
# we could setup when doing a `require`. | ||
# | ||
def self.libddprof_folder_relative_to_native_lib_folder( | ||
current_folder: __dir__, | ||
libddprof_pkgconfig_folder: Libddprof.pkgconfig_folder | ||
) | ||
return unless libddprof_pkgconfig_folder | ||
|
||
profiling_native_lib_folder = "#{current_folder}/../../lib/" | ||
libddprof_lib_folder = "#{libddprof_pkgconfig_folder}/../" | ||
|
||
Pathname.new(libddprof_lib_folder).relative_path_from(Pathname.new(profiling_native_lib_folder)).to_s | ||
end | ||
|
||
# Used to check if profiler is supported, including user-visible clear messages explaining why their | ||
# system may not be supported. | ||
# rubocop:disable Metrics/ModuleLength | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,45 @@ | ||
# typed: ignore | ||
|
||
require 'ext/ddtrace_profiling_native_extension/native_extension_helpers' | ||
require 'libddprof' | ||
|
||
require 'datadog/profiling/spec_helper' | ||
|
||
RSpec.describe Datadog::Profiling::NativeExtensionHelpers do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice |
||
describe '.libddprof_folder_relative_to_native_lib_folder' do | ||
context 'when libddprof is available' do | ||
before do | ||
skip_if_profiling_not_supported(self) | ||
if PlatformHelpers.mac? && Libddprof.pkgconfig_folder.nil? && ENV['LIBDDPROF_VENDOR_OVERRIDE'].nil? | ||
raise 'You have a libddprof setup without macOS support. Did you forget to set LIBDDPROF_VENDOR_OVERRIDE?' | ||
end | ||
end | ||
|
||
it 'returns a relative path to libddprof folder from the gem lib folder' do | ||
relative_path = described_class.libddprof_folder_relative_to_native_lib_folder | ||
|
||
# RbConfig::CONFIG['SOEXT'] was only introduced in Ruby 2.5, so we have a fallback for older Rubies... | ||
libddprof_extension = | ||
RbConfig::CONFIG['SOEXT'] || | ||
('so' if PlatformHelpers.linux?) || | ||
('dylib' if PlatformHelpers.mac?) || | ||
raise('Missing SOEXT for current platform') | ||
|
||
gem_lib_folder = "#{Gem.loaded_specs['ddtrace'].gem_dir}/lib" | ||
full_libddprof_path = "#{gem_lib_folder}/#{relative_path}/libddprof_ffi.#{libddprof_extension}" | ||
|
||
expect(relative_path).to start_with('../') | ||
expect(File.exist?(full_libddprof_path)) | ||
.to be(true), "Libddprof not available in expected path: #{full_libddprof_path.inspect}" | ||
end | ||
end | ||
|
||
context 'when libddprof is unsupported' do | ||
it do | ||
expect(described_class.libddprof_folder_relative_to_native_lib_folder(libddprof_pkgconfig_folder: nil)).to be nil | ||
end | ||
end | ||
end | ||
end | ||
|
||
RSpec.describe Datadog::Profiling::NativeExtensionHelpers::Supported do | ||
describe '.supported?' do | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we've all had to do this kind of escaping garbage before, and all I can say is: LOL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:this-is-fine-fire: ;)