Merge pull request #2741 from DataDog/ivoanjo/prof-7409-rugged-gem-wo…

…rkaround

[PROF-7409] Do not auto-enable new profiler when rugged gem is detected

ext/ddtrace_profiling_native_extension/collectors_cpu_and_wall_time_worker.c

@@ -170,6 +170,7 @@ static void sleep_for(uint64_t time_ns);
 static VALUE _native_allocation_count(DDTRACE_UNUSED VALUE self);
 static void on_newobj_event(DDTRACE_UNUSED VALUE tracepoint_data, DDTRACE_UNUSED void *unused);
 static void disable_tracepoints(struct cpu_and_wall_time_worker_state *state);
+static VALUE _native_with_blocked_sigprof(DDTRACE_UNUSED VALUE self);
 
 // Note on sampler global state safety:
 //
@@ -222,6 +223,7 @@ void collectors_cpu_and_wall_time_worker_init(VALUE profiling_module) {
   rb_define_singleton_method(testing_module, "_native_simulate_handle_sampling_signal", _native_simulate_handle_sampling_signal, 0);
   rb_define_singleton_method(testing_module, "_native_simulate_sample_from_postponed_job", _native_simulate_sample_from_postponed_job, 0);
   rb_define_singleton_method(testing_module, "_native_is_sigprof_blocked_in_current_thread", _native_is_sigprof_blocked_in_current_thread, 0);
+  rb_define_singleton_method(testing_module, "_native_with_blocked_sigprof", _native_with_blocked_sigprof, 0);
 }
 
 // This structure is used to define a Ruby object that stores a pointer to a struct cpu_and_wall_time_worker_state
@@ -869,3 +871,16 @@ static void disable_tracepoints(struct cpu_and_wall_time_worker_state *state) {
   rb_tracepoint_disable(state->gc_tracepoint);
   rb_tracepoint_disable(state->object_allocation_tracepoint);
 }
+
+static VALUE _native_with_blocked_sigprof(DDTRACE_UNUSED VALUE self) {
+  block_sigprof_signal_handler_from_running_in_current_thread();
+  int exception_state;
+  VALUE result = rb_protect(rb_yield, Qundef, &exception_state);
+  unblock_sigprof_signal_handler_from_running_in_current_thread();
+
+  if (exception_state) {
+    rb_jump_tag(exception_state);
+  } else {
+    return result;
+  }
+}

lib/datadog/profiling/component.rb

@@ -172,8 +172,19 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)
             'library used by the mysql2 gem) have a bug in their signal handling code that the new profiler can trigger. ' \
             'This bug (https://bugs.mysql.com/bug.php?id=83109) is fixed in libmysqlclient versions 8.0.0 and above. ' \
             'If your Linux distribution provides a modern libmysqlclient, you can force-enable the new CPU Profiling 2.0 ' \
-            'profiler by using the `DD_PROFILING_FORCE_ENABLE_NEW` or `c.profiling.advanced.force_enable_new_profiler` ' \
-            'settings.'
+            'profiler by using the `DD_PROFILING_FORCE_ENABLE_NEW` environment variable or the ' \
+            '`c.profiling.advanced.force_enable_new_profiler` setting.' \
+          )
+          return false
+        end
+
+        if Gem.loaded_specs['rugged']
+          Datadog.logger.warn(
+            'Falling back to legacy profiler because rugged gem is installed. Some operations on this gem are ' \
+            'currently incompatible with the new CPU Profiling 2.0 profiler, as detailed in ' \
+            '<https://github.com/datadog/dd-trace-rb/issues/2721>. If you still want to try out the new profiler, you ' \
+            'can force-enable it by using the `DD_PROFILING_FORCE_ENABLE_NEW` environment variable or the ' \
+            '`c.profiling.advanced.force_enable_new_profiler` setting.'
           )
           return false
         end

spec/datadog/profiling/component_spec.rb

@@ -402,8 +402,28 @@
           end
         end
 
-        context 'when mysql2 gem is not available' do
-          include_context 'loaded gems', :mysql2 => nil
+        context 'when rugged gem is available' do
+          include_context 'loaded gems', :rugged => Gem::Version.new('1.6.3')
+
+          before { allow(Datadog.logger).to receive(:warn) }
+
+          it { is_expected.to be false }
+
+          it 'logs a warning message mentioning that the legacy profiler is going to be used' do
+            expect(Datadog.logger).to receive(:warn).with(/Falling back to legacy profiler/)
+
+            enable_new_profiler?
+          end
+
+          context 'when force_enable_new_profiler is enabled' do
+            before { settings.profiling.advanced.force_enable_new_profiler = true }
+
+            it { is_expected.to be true }
+          end
+        end
+
+        context 'when mysql2 / rugged gem are not available' do
+          include_context('loaded gems', mysql2: nil, rugged: nil)
 
           it { is_expected.to be true }
         end