Replace libddprof dependency with libdatadog, and upgrade to 0.7.0

ddtrace.gemspec

@@ -64,8 +64,8 @@ Gem::Specification.new do |spec|
   # Used by appsec
   spec.add_dependency 'libddwaf', '~> 1.3.0.2.0'
 
-  # Used by profiling
-  spec.add_dependency 'libddprof', '~> 0.6.0.1.0'
+  # Used by profiling (and possibly others in the future)
+  spec.add_dependency 'libdatadog', '~> 0.7.0.1.0'
 
   spec.extensions = ['ext/ddtrace_profiling_native_extension/extconf.rb', 'ext/ddtrace_profiling_loader/extconf.rb']
 end

ext/ddtrace_profiling_loader/ddtrace_profiling_loader.c

@@ -20,7 +20,7 @@
 // This idea was shamelessly stolen from @lloeki's work in https://github.com/rubyjs/mini_racer/pull/179, big thanks!
 //
 // Extra note: Currently (May 2022), that we know of, the profiling native extension only exposes one potentially
-// problematic symbol: `rust_eh_personality` (coming from libddprof/libdatadog).
+// problematic symbol: `rust_eh_personality` (coming from libdatadog).
 // Future versions of Rust have been patched not to expose this
 // (see https://github.com/rust-lang/rust/pull/95604#issuecomment-1108563434) 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 :)

ext/ddtrace_profiling_native_extension/NativeExtensionDesign.md

@@ -2,7 +2,7 @@
 
 The profiling native extension is used to:
 1. Implement features which are expensive (in terms of resources) or otherwise impossible to implement using Ruby code.
-2. Bridge between Ruby-specific profiling features and [`libddprof`](https://github.com/DataDog/libddprof), a Rust-based
+2. Bridge between Ruby-specific profiling features and [`libdatadog`](https://github.com/DataDog/libdatadog), a Rust-based
 library with common profiling functionality.
 
 Due to (1), this extension is quite coupled with MRI Ruby ("C Ruby") internals, and is not intended to support other rubies such as

ext/ddtrace_profiling_native_extension/extconf.rb

@@ -138,8 +138,8 @@ def add_compiler_flag(flag)
 # In Ruby 2.1, living_threads were stored in a hashmap (st)
 $defs << '-DUSE_LEGACY_LIVING_THREADS_ST' if RUBY_VERSION < '2.2'
 
-# If we got here, libddprof is available and loaded
-ENV['PKG_CONFIG_PATH'] = "#{ENV['PKG_CONFIG_PATH']}:#{Libddprof.pkgconfig_folder}"
+# If we got here, libdatadog is available and loaded
+ENV['PKG_CONFIG_PATH'] = "#{ENV['PKG_CONFIG_PATH']}:#{Libdatadog.pkgconfig_folder}"
 Logging.message(" [ddtrace] PKG_CONFIG_PATH set to #{ENV['PKG_CONFIG_PATH'].inspect}\n")
 
 unless pkg_config('ddprof_ffi_with_rpath')
@@ -148,17 +148,17 @@ def add_compiler_flag(flag)
       Datadog::Profiling::NativeExtensionHelpers::Supported::PKG_CONFIG_IS_MISSING
     else
       # Less specific error message
-      Datadog::Profiling::NativeExtensionHelpers::Supported::FAILED_TO_CONFIGURE_LIBDDPROF
+      Datadog::Profiling::NativeExtensionHelpers::Supported::FAILED_TO_CONFIGURE_LIBDATADOG
     end
   )
 end
 
-# See comments on the helper method being used for why we need to additionally set this
+# 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\\}/' \
-  "#{Datadog::Profiling::NativeExtensionHelpers.libddprof_folder_relative_to_native_lib_folder}"
+  "#{Datadog::Profiling::NativeExtensionHelpers.libdatadog_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.

ext/ddtrace_profiling_native_extension/http_transport.c

@@ -166,7 +166,7 @@ static ddprof_ffi_Vec_tag convert_tags(VALUE tags_as_array) {
       VALUE err_details = ruby_string_from_vec_u8(push_result.err);
       ddprof_ffi_PushTagResult_drop(push_result);
 
-      // libddprof validates tags and may catch invalid tags that ddtrace didn't actually catch.
+      // libdatadog validates tags and may catch invalid tags that ddtrace didn't actually catch.
       // We warn users about such tags, and then just ignore them.
       safely_log_failure_to_process_tag(tags, err_details);
     } else {
@@ -193,7 +193,7 @@ static void safely_log_failure_to_process_tag(ddprof_ffi_Vec_tag tags, VALUE err
   }
 }
 
-// Note: This function handles a bunch of libddprof dynamically-allocated objects, so it MUST not use any Ruby APIs
+// Note: This function handles a bunch of libdatadog dynamically-allocated objects, so it MUST not use any Ruby APIs
 // which can raise exceptions, otherwise the objects will be leaked.
 static VALUE perform_export(
   ddprof_ffi_NewProfileExporterV3Result valid_exporter_result, // Must be called with a valid exporter result
@@ -229,39 +229,31 @@ static VALUE perform_export(
 
   while (!args.send_ran && !pending_exception) {
     rb_thread_call_without_gvl2(call_exporter_without_gvl, &args, interrupt_exporter_call, cancel_token);
+
+    // To make sure we don't leak memory, we never check for pending exceptions if send ran
     if (!args.send_ran) pending_exception = check_if_pending_exception();
   }
 
-  VALUE ruby_status;
-  VALUE ruby_result;
+  // Cleanup exporter and token, no longer needed
+  ddprof_ffi_CancellationToken_drop(cancel_token);
+  ddprof_ffi_NewProfileExporterV3Result_drop(valid_exporter_result);
 
   if (pending_exception) {
-    // We're in a weird situation that libddprof doesn't quite support. The ddprof_ffi_Request payload is dynamically
-    // allocated and needs to be freed, but libddprof doesn't have an API for dropping a request.
-    //
-    // There's plans to add a `ddprof_ffi_Request_drop`
-    // (https://github.com/DataDog/dd-trace-rb/pull/1923#discussion_r882096221); once that happens, we can use it here.
-    //
-    // As a workaround, we get libddprof to clean up the request by asking for the send to be cancelled, and then calling
-    // it anyway. This will make libddprof free the request and return immediately which gets us the expected effect.
-    interrupt_exporter_call((void *) cancel_token);
-    call_exporter_without_gvl((void *) &args);
+    // If we got here send did not run, so we need to explicitly dispose of the request
+    ddprof_ffi_Request_drop(request);
+
+    // Let Ruby propagate the exception. This will not return.
+    rb_jump_tag(pending_exception);
   }
 
   ddprof_ffi_SendResult result = args.result;
   bool success = result.tag == DDPROF_FFI_SEND_RESULT_HTTP_RESPONSE;
 
-  ruby_status = success ? ok_symbol : error_symbol;
-  ruby_result = success ? UINT2NUM(result.http_response.code) : ruby_string_from_vec_u8(result.failure);
+  VALUE ruby_status = success ? ok_symbol : error_symbol;
+  VALUE ruby_result = success ? UINT2NUM(result.http_response.code) : ruby_string_from_vec_u8(result.err);
 
-  // Clean up all dynamically-allocated things
   ddprof_ffi_SendResult_drop(args.result);
-  ddprof_ffi_CancellationToken_drop(cancel_token);
-  ddprof_ffi_NewProfileExporterV3Result_drop(valid_exporter_result);
-  // The request itself does not need to be freed as libddprof takes care of it.
-
-  // We've cleaned up everything, so if there's an exception to be raised, let's have it
-  if (pending_exception) rb_jump_tag(pending_exception);
+  // The request itself does not need to be freed as libdatadog takes care of it as part of sending.
 
   return rb_ary_new_from_args(2, ruby_status, ruby_result);
 }

ext/ddtrace_profiling_native_extension/native_extension_helpers.rb

@@ -2,7 +2,7 @@
 
 # typed: ignore
 
-require 'libddprof'
+require 'libdatadog'
 require 'pathname'
 
 module Datadog
@@ -22,29 +22,29 @@ def self.fail_install_if_missing_extension?
       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.
+      # libdatadog 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.
+      # native extension), we need to add a "runpath" -- a list of folders to search for libdatadog.
       #
       # 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**
+      # In ddtrace 1.1.0, we only set as runpath an absolute path to libdatadog. (This gets set automatically by the call
+      # to `pkg_config('ddprof_ffi_with_rpath')` in `extconf.rb`). This worked fine as long as libdatadog 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.
+      # (see https://github.com/DataDog/dd-trace-rb/issues/2067) because libdatadog 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
+      # native extension is going to be installed and the folder where libdatadog 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.
+      # the relative path can still be traversed to find libdatadog.
       #
       # 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
@@ -54,16 +54,16 @@ def self.fail_install_if_missing_extension?
       # 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(
+      def self.libdatadog_folder_relative_to_native_lib_folder(
         current_folder: __dir__,
-        libddprof_pkgconfig_folder: Libddprof.pkgconfig_folder
+        libdatadog_pkgconfig_folder: Libdatadog.pkgconfig_folder
       )
-        return unless libddprof_pkgconfig_folder
+        return unless libdatadog_pkgconfig_folder
 
         profiling_native_lib_folder = "#{current_folder}/../../lib/"
-        libddprof_lib_folder = "#{libddprof_pkgconfig_folder}/../"
+        libdatadog_lib_folder = "#{libdatadog_pkgconfig_folder}/../"
 
-        Pathname.new(libddprof_lib_folder).relative_path_from(Pathname.new(profiling_native_lib_folder)).to_s
+        Pathname.new(libdatadog_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
@@ -87,7 +87,7 @@ def self.unsupported_reason
             on_unknown_os? ||
             not_on_amd64_or_arm64? ||
             expected_to_use_mjit_but_mjit_is_disabled? ||
-            libddprof_not_usable?
+            libdatadog_not_usable?
         end
 
         # This banner will show up in the logs/terminal while compiling the native extension
@@ -149,8 +149,8 @@ def self.pkg_config_missing?(command: $PKGCONFIG) # rubocop:disable Style/Global
         ].freeze
 
         # Validation for this check is done in extconf.rb because it relies on mkmf
-        FAILED_TO_CONFIGURE_LIBDDPROF = explain_issue(
-          'there was a problem in setting up the `libddprof` dependency.',
+        FAILED_TO_CONFIGURE_LIBDATADOG = explain_issue(
+          'there was a problem in setting up the `libdatadog` dependency.',
           suggested: CONTACT_SUPPORT,
         )
 
@@ -255,17 +255,17 @@ def self.pkg_config_missing?(command: $PKGCONFIG) # rubocop:disable Style/Global
           ruby_without_mjit if CAN_USE_MJIT_HEADER && RbConfig::CONFIG['MJIT_SUPPORT'] != 'yes'
         end
 
-        private_class_method def self.libddprof_not_usable?
+        private_class_method def self.libdatadog_not_usable?
           no_binaries_for_current_platform = explain_issue(
-            'the `libddprof` gem installed on your system is missing binaries for your',
+            'the `libdatadog` gem installed on your system is missing binaries for your',
             'platform variant.',
             "(Your platform: `#{Gem::Platform.local}`)",
             '(Available binaries: ',
-            "`#{Libddprof.available_binaries.join('`, `')}`)",
+            "`#{Libdatadog.available_binaries.join('`, `')}`)",
             suggested: CONTACT_SUPPORT,
           )
 
-          no_binaries_for_current_platform unless Libddprof.pkgconfig_folder
+          no_binaries_for_current_platform unless Libdatadog.pkgconfig_folder
         end
       end
       # rubocop:enable Metrics/ModuleLength

ext/ddtrace_profiling_native_extension/stack_recorder.c

@@ -31,9 +31,9 @@ void stack_recorder_init(VALUE profiling_module) {
 
   // Instances of the StackRecorder class are "TypedData" objects.
   // "TypedData" objects are special objects in the Ruby VM that can wrap C structs.
-  // In our case, we're going to keep a libddprof profile reference inside our object.
+  // In our case, we're going to keep a libdatadog profile reference inside our object.
   //
-  // Because Ruby doesn't know how to initialize libddprof profiles, we MUST override the allocation function for objects
+  // Because Ruby doesn't know how to initialize libdatadog profiles, we MUST override the allocation function for objects
   // of this class so that we can manage this part. Not overriding or disabling the allocation function is a common
   // gotcha for "TypedData" objects that can very easily lead to VM crashes, see for instance
   // https://bugs.ruby-lang.org/issues/18007 for a discussion around this.
@@ -61,7 +61,7 @@ static const rb_data_type_t stack_recorder_typed_data = {
 static VALUE _native_new(VALUE klass) {
   ddprof_ffi_Slice_value_type sample_types = {.ptr = enabled_value_types, .len = ENABLED_VALUE_TYPES_COUNT};
 
-  ddprof_ffi_Profile *profile = ddprof_ffi_Profile_new(sample_types, NULL /* Period is optional */);
+  ddprof_ffi_Profile *profile = ddprof_ffi_Profile_new(sample_types, NULL /* period is optional */, NULL /* start_time is optional */);
 
   return TypedData_Wrap_Struct(klass, &stack_recorder_typed_data, profile);
 }
@@ -89,7 +89,7 @@ static VALUE _native_serialize(VALUE self, VALUE recorder_instance) {
 
     // We use rb_thread_call_without_gvl2 here because unlike the regular _gvl variant, gvl2 does not process
     // interruptions and thus does not raise exceptions after running our code.
-    rb_thread_call_without_gvl2(call_serialize_without_gvl, &args, /* No interruption function supported */ NULL, NULL);
+    rb_thread_call_without_gvl2(call_serialize_without_gvl, &args, NULL /* No interruption function needed in this case */, NULL /* Not needed */);
   }
 
   ddprof_ffi_SerializeResult serialized_profile = args.result;
@@ -105,13 +105,15 @@ static VALUE _native_serialize(VALUE self, VALUE recorder_instance) {
   ddprof_ffi_Timespec ddprof_start = serialized_profile.ok.start;
   ddprof_ffi_Timespec ddprof_finish = serialized_profile.ok.end;
 
-  // Clean up libddprof object to avoid leaking in case ruby_time_from raises an exception
+  // Clean up libdatadog object to avoid leaking in case ruby_time_from raises an exception
   ddprof_ffi_SerializeResult_drop(serialized_profile);
 
   VALUE start = ruby_time_from(ddprof_start);
   VALUE finish = ruby_time_from(ddprof_finish);
 
-  if (!ddprof_ffi_Profile_reset(profile)) return rb_ary_new_from_args(2, error_symbol, rb_str_new_cstr("Failed to reset profile"));
+  if (!ddprof_ffi_Profile_reset(profile, NULL /* start_time is optional */ )) {
+    return rb_ary_new_from_args(2, error_symbol, rb_str_new_cstr("Failed to reset profile"));
+  }
 
   return rb_ary_new_from_args(2, ok_symbol, rb_ary_new_from_args(3, start, finish, encoded_pprof));
 }
@@ -136,7 +138,7 @@ void record_sample(VALUE recorder_instance, ddprof_ffi_Sample sample) {
 static void *call_serialize_without_gvl(void *call_args) {
   struct call_serialize_without_gvl_arguments *args = (struct call_serialize_without_gvl_arguments *) call_args;
 
-  args->result = ddprof_ffi_Profile_serialize(args->profile);
+  args->result = ddprof_ffi_Profile_serialize(args->profile, NULL /* end_time is optional */, NULL /* duration_nanos is optional */);
   args->serialize_ran = true;
 
   return NULL; // Unused

lib/datadog/profiling/http_transport.rb

@@ -28,7 +28,7 @@ def export(flush)
           upload_timeout_milliseconds: @upload_timeout_milliseconds,
 
           # why "timespec"?
-          # libddprof represents time using POSIX's struct timespec, see
+          # libdatadog represents time using POSIX's struct timespec, see
           # https://www.gnu.org/software/libc/manual/html_node/Time-Types.html
           # aka it represents the seconds part separate from the nanoseconds part
           start_timespec_seconds: flush.start.tv_sec,

spec/datadog/profiling/http_transport_spec.rb

@@ -12,8 +12,8 @@
 # between the Ruby code and the native methods, and thus in this class we have a bunch of tests to make sure the
 # native methods are invoked correctly.
 #
-# We also have "integration" specs, where we exercise the Ruby code together with the C code and libddprof to ensure
-# that things come out of libddprof as we expected.
+# We also have "integration" specs, where we exercise the Ruby code together with the C code and libdatadog to ensure
+# that things come out of libdatadog as we expected.
 RSpec.describe Datadog::Profiling::HttpTransport do
   before { skip_if_profiling_not_supported(self) }