Skip to content

Commit 6a3e147

Browse files
committed
[PROF-11311] Remove datadog_profiling_loader
**What does this PR do?** This PR removes the `datadog_profiling_loader` native extension, as it's no longer needed. The profiling native loader was added in #2003 . Quoting from that PR's description: > When Ruby loads a native extension (using `require`), it uses > `dlopen(..., RTLD_LAZY | RTLD_GLOBAL)` > (https://github.com/ruby/ruby/blob/67950a4c0a884bdb78d9beb4405ebf7459229b21/dln.c#L362). > > This means that every symbol exposed directly or indirectly by that > native extension becomes visible to every other extension in the > Ruby process. This can cause issues, see > rubyjs/mini_racer#179 . > > Ruby's extension loading mechanism is not configurable -- there's > no way to tell it to use different flags when calling `dlopen`. > To get around this, this commit introduces introduces another > extension (profiling loader) which has only a single responsibility: > mimic Ruby's extension loading mechanism, but when calling > `dlopen` use a different set of flags. > > This idea was shamelessly stolen from @lloeki's work in > rubyjs/mini_racer#179, big thanks! ...and importantly ends with: > Note that, that we know of, the profiling native extension only > exposes one potentially problematic symbol: > `rust_eh_personality` (coming from libddprof/libdatadog). > > Future versions of Rust have been patched not to expose this > (see rust-lang/rust#95604 (comment)) > so we may want to revisit the need for this loader in the future, > and perhaps delete it if we no longer require its services :) And we have reached the situation predicted in that description: 1. Nowadays libdatadog no longer exposes `rust_eh_personality` 2. We have a test that validates that only expected symbols are exported by the libdatadog library (see DataDog/libdatadog#573 ). Any new symbols that show up would break shipping new libdatadog versions to rubygems.org until we review them. 3. The `libdatadog_api` extension, which we've been shipping for customers since release 2.3.0 back in July 2024 has always been loaded directly without a loader without issues. Thus, I think it's the right time to get rid of the loader. **Motivation:** Having the loader around is not zero cost; we've run into/caused a few issues ( #2250 and #3582 come to mind). It also adds overhead to development: every time we need to rebuild the extensions, it's an extra extension that needs to be prepared, rebuilt, copied, etc. I've been meaning to get rid of the loader for some time now, and this came up again this week so I decided it was time to do it. **Additional Notes:** In the future, we can always ressurrect this approach if we figure out we need it again. We've also discussed internally about proposing upstream to the Ruby VM to maybe add an API to do what the loader was doing, so that we wouldn't need the weird workaround. We haven't yet decided if we're going to do that (help welcome!). **How to test the change?** The loader was responsible for loading the rest of profiling. Thus, any test that uses profiling was also validating the loader and now will validate that we're doing fine without it. TL;DR green CI is good.
1 parent a444023 commit 6a3e147

File tree

8 files changed

+2
-315
lines changed

8 files changed

+2
-315
lines changed

.github/labeler.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ dev/testing:
1818
# Changes to Profiling
1919
profiling:
2020
- changed-files:
21-
- any-glob-to-any-file: [ '{lib/datadog/profiling/**,ext/datadog_profiling_loader/**,ext/datadog_profiling_native_extension/**}' ]
21+
- any-glob-to-any-file: [ '{lib/datadog/profiling/**,ext/datadog_profiling_native_extension/**}' ]
2222

2323
# Changes to CI-App
2424
ci-app:

Rakefile

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -435,10 +435,6 @@ NATIVE_EXTS = [
435435
ext.ext_dir = 'ext/datadog_profiling_native_extension'
436436
end,
437437

438-
Rake::ExtensionTask.new("datadog_profiling_loader.#{RUBY_VERSION}_#{RUBY_PLATFORM}") do |ext|
439-
ext.ext_dir = 'ext/datadog_profiling_loader'
440-
end,
441-
442438
Rake::ExtensionTask.new("libdatadog_api.#{RUBY_VERSION[/\d+.\d+/]}_#{RUBY_PLATFORM}") do |ext|
443439
ext.ext_dir = 'ext/libdatadog_api'
444440
end

datadog.gemspec

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ Gem::Specification.new do |spec|
8383

8484
spec.extensions = [
8585
'ext/datadog_profiling_native_extension/extconf.rb',
86-
'ext/datadog_profiling_loader/extconf.rb',
8786
'ext/libdatadog_api/extconf.rb'
8887
]
8988
end

ext/datadog_profiling_loader/.clang-format

Lines changed: 0 additions & 1 deletion
This file was deleted.

ext/datadog_profiling_loader/datadog_profiling_loader.c

Lines changed: 0 additions & 142 deletions
This file was deleted.

ext/datadog_profiling_loader/extconf.rb

Lines changed: 0 additions & 60 deletions
This file was deleted.
Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,9 @@
11
# frozen_string_literal: true
22

3-
# This file is used to load the profiling native extension. It works in two steps:
4-
#
5-
# 1. Load the datadog_profiling_loader extension. This extension will be used to load the actual extension, but in
6-
# a special way that avoids exposing native-level code symbols. See `datadog_profiling_loader.c` for more details.
7-
#
8-
# 2. Use the Datadog::Profiling::Loader exposed by the datadog_profiling_loader extension to load the actual
9-
# profiling native extension.
10-
#
11-
# All code on this file is on-purpose at the top-level; this makes it so this file is executed only once,
12-
# the first time it gets required, to avoid any issues with the native extension being initialized more than once.
13-
143
begin
15-
require "datadog_profiling_loader.#{RUBY_VERSION}_#{RUBY_PLATFORM}"
4+
require "datadog_profiling_native_extension.#{RUBY_VERSION}_#{RUBY_PLATFORM}"
165
rescue LoadError => e
176
raise LoadError,
187
"Failed to load the profiling loader extension. To fix this, please remove and then reinstall datadog " \
198
"(Details: #{e.message})"
209
end
21-
22-
extension_name = "datadog_profiling_native_extension.#{RUBY_VERSION}_#{RUBY_PLATFORM}"
23-
file_name = "#{extension_name}.#{RbConfig::CONFIG["DLEXT"]}"
24-
full_file_path = "#{__dir__}/../../#{file_name}"
25-
26-
unless File.exist?(full_file_path)
27-
extension_dir = Gem.loaded_specs["datadog"].extension_dir
28-
candidate_path = "#{extension_dir}/#{file_name}"
29-
if File.exist?(candidate_path)
30-
full_file_path = candidate_path
31-
else
32-
# We found none of the files. This is unexpected. Let's go ahead anyway, the error is going to be reported further
33-
# down anyway.
34-
end
35-
end
36-
37-
init_function_name = "Init_#{extension_name.split(".").first}"
38-
39-
status, result = Datadog::Profiling::Loader._native_load(full_file_path, init_function_name)
40-
41-
raise "Failure to load #{extension_name} due to #{result}" if status == :error

spec/datadog/profiling/load_native_extension_spec.rb

Lines changed: 0 additions & 73 deletions
This file was deleted.

0 commit comments

Comments
 (0)