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-5747] Fix broken libddprof linking on Heroku and AWS Elastic Beanstalk #2125

Merged
merged 7 commits into from
Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ gemspec
gem 'addressable', '~> 2.4.0' # locking transitive dependency of webmock
gem 'appraisal', '~> 2.2'
gem 'benchmark-ips', '~> 2.8'
gem 'benchmark-memory', '~> 0.1'
gem 'benchmark-memory', '< 0.2' # V0.2 only works with 2.5+
gem 'builder'
gem 'climate_control', '~> 0.2.0'
# Leave it open as we also have it as an integration and want Appraisal to control the version under test.
gem 'concurrent-ruby'
gem 'json-schema'
gem 'json-schema', '< 3' # V3 only works with 2.5+
gem 'memory_profiler', '~> 0.9'
gem 'os', '~> 1.1'
gem 'pimpmychangelog', '>= 0.1.2'
Expand Down
9 changes: 9 additions & 0 deletions ext/ddtrace_profiling_native_extension/extconf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,19 @@ def add_compiler_flag(flag)
# If we got here, libddprof is available and loaded
ENV['PKG_CONFIG_PATH'] = "#{ENV['PKG_CONFIG_PATH']}:#{Libddprof.pkgconfig_folder}"
Logging.message(" [ddtrace] PKG_CONFIG_PATH set to #{ENV['PKG_CONFIG_PATH'].inspect}\n")

unless pkg_config('ddprof_ffi_with_rpath')
skip_building_extension!(Datadog::Profiling::NativeExtensionHelpers::Supported::FAILED_TO_CONFIGURE_LIBDDPROF)
end

# See comments on the helper method being used for why we need to additionally set this
# The extremely excessive escaping around ORIGIN below seems to be correct and was determined after a lot of
# experimentation. We need to get these special characters across a lot of tools untouched...
$LDFLAGS += \
' -Wl,-rpath,$$$\\\\{ORIGIN\\}/' \
Copy link

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

Copy link
Member Author

Choose a reason for hiding this comment

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

:this-is-fine-fire: ;)

"#{Datadog::Profiling::NativeExtensionHelpers.libddprof_folder_relative_to_native_lib_folder}"
Logging.message(" [ddtrace] After pkg-config $LDFLAGS were set to: #{$LDFLAGS.inspect}\n")

# Tag the native extension library with the Ruby version and Ruby platform.
# This makes it easier for development (avoids "oops I forgot to rebuild when I switched my Ruby") and ensures that
# the wrong library is never loaded.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# typed: ignore

require 'libddprof'
require 'pathname'

module Datadog
module Profiling
Expand All @@ -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.
Copy link

Choose a reason for hiding this comment

The 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
Copy link

Choose a reason for hiding this comment

The 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
Expand Down
3 changes: 0 additions & 3 deletions integration/apps/rack/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ ENV DD_DEMO_ENV_GEM_REF_DDTRACE ${ddtrace_ref}

# Install dependencies
COPY Gemfile /app/Gemfile
# This forces gems with native extensions to be compiled, rather than using pre-compiled binaries; it's needed because
# some google-protobuf versions ship with missing binaries for older rubies.
ENV BUNDLE_FORCE_RUBY_PLATFORM true
RUN bundle install

# Add files
Expand Down
12 changes: 10 additions & 2 deletions integration/apps/rack/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,19 @@ source 'https://rubygems.org' do
gem 'ddtrace', *Datadog::DemoEnv.gem_spec('ddtrace')

# Needed for ddtrace profiling
google_protobuf_versions = [
'~> 3.0',
'!= 3.7.0.rc.2',
'!= 3.7.0.rc.3',
'!= 3.7.0',
'!= 3.7.1',
'!= 3.8.0.rc.1'
]
if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.4')
gem 'google-protobuf'
gem 'google-protobuf', *google_protobuf_versions
else
# Bundler resolves incorrect version (too new, incompatible with Ruby <= 2.3)
gem 'google-protobuf', '< 3.19.2'
gem 'google-protobuf', *google_protobuf_versions, '< 3.19.2'
end

# Development
Expand Down
40 changes: 39 additions & 1 deletion spec/datadog/profiling/native_extension_helpers_spec.rb
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
Copy link

Choose a reason for hiding this comment

The 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
Expand Down