Add testing for enabling the GVL profiling hook

+ a small refactor for skipping GVL profiling specs on legacy Rubies

ext/datadog_profiling_native_extension/collectors_cpu_and_wall_time_worker.c

@@ -230,6 +230,7 @@ static VALUE _native_resume_signals(DDTRACE_UNUSED VALUE self);
 static void on_gvl_event(rb_event_flag_t event_id, const rb_internal_thread_event_data_t *event_data, DDTRACE_UNUSED void *_unused);
 #endif
 static void after_gvl_running_from_postponed_job(DDTRACE_UNUSED void *_unused);
+static VALUE _native_gvl_profiling_hook_active(DDTRACE_UNUSED VALUE self, VALUE instance);
 
 // We're using `on_newobj_event` function with `rb_add_event_hook2`, which requires in its public signature a function
 // with signature `rb_event_hook_func_t` which doesn't match `on_newobj_event`.
@@ -325,6 +326,7 @@ void collectors_cpu_and_wall_time_worker_init(VALUE profiling_module) {
   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);
   rb_define_singleton_method(testing_module, "_native_delayed_error", _native_delayed_error, 2);
+  rb_define_singleton_method(testing_module, "_native_gvl_profiling_hook_active", _native_gvl_profiling_hook_active, 1);
 }
 
 // This structure is used to define a Ruby object that stores a pointer to a struct cpu_and_wall_time_worker_state
@@ -1338,3 +1340,14 @@ static VALUE _native_resume_signals(DDTRACE_UNUSED VALUE self) {
     state->during_sample = false;
   }
 #endif
+
+static VALUE _native_gvl_profiling_hook_active(DDTRACE_UNUSED VALUE self, VALUE instance) {
+  #ifndef NO_GVL_INSTRUMENTATION
+    struct cpu_and_wall_time_worker_state *state;
+    TypedData_Get_Struct(instance, struct cpu_and_wall_time_worker_state, &cpu_and_wall_time_worker_typed_data, state);
+
+    return state->gvl_profiling_hook != NULL ? Qtrue : Qfalse;
+  #else
+    return Qfalse;
+  #endif
+}

spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb

@@ -136,6 +136,28 @@
       end
     end
 
+    context "when gvl_profiling_enabled is true" do
+      before { skip_if_gvl_profiling_not_supported(self) }
+
+      let(:gvl_profiling_enabled) { true }
+
+      it "enables the gvl profiling hook" do
+        start
+
+        expect(described_class::Testing._native_gvl_profiling_hook_active(cpu_and_wall_time_worker)).to be true
+      end
+    end
+
+    context "when gvl_profiling_enabled is false" do
+      let(:gvl_profiling_enabled) { false }
+
+      it "does not enable the gvl profiling hook" do
+        start
+
+        expect(described_class::Testing._native_gvl_profiling_hook_active(cpu_and_wall_time_worker)).to be false
+      end
+    end
+
     context "when a previous signal handler existed" do
       before do
         described_class::Testing._native_install_testing_signal_handler
@@ -374,11 +396,7 @@
       end
 
       context "when GVL profiling is enabled" do
-        before do
-          if min_ruby_for_gvl_profiling > RUBY_VERSION
-            skip "GVL profiling is is only supported on Ruby >= #{min_ruby_for_gvl_profiling}"
-          end
-        end
+        before { skip_if_gvl_profiling_not_supported(self) }
 
         let(:gvl_profiling_enabled) { true }
 
@@ -947,6 +965,18 @@
         # come from us -- it's the default message for an unhandled SIGPROF. Pretty confusing UNIX/POSIX behavior...)
         Process.kill("SIGPROF", Process.pid)
       end
+
+      context "when GVL profiling is enabled" do
+        before { skip_if_gvl_profiling_not_supported(self) }
+
+        let(:gvl_profiling_enabled) { true }
+
+        it "disables the GVL profiling hook" do
+          expect { stop }
+            .to change { described_class::Testing._native_gvl_profiling_hook_active(cpu_and_wall_time_worker) }
+            .from(true).to(false)
+        end
+      end
     end
 
     it "unblocks SIGPROF signal handling from the worker thread" do
@@ -993,6 +1023,18 @@
         .from(true).to(false)
     end
 
+    context "when gvl_profiling_enabled is true" do
+      before { skip_if_gvl_profiling_not_supported(self) }
+
+      let(:gvl_profiling_enabled) { true }
+
+      it "disables the gvl profiling hook" do
+        expect { reset_after_fork }
+          .to change { described_class::Testing._native_gvl_profiling_hook_active(cpu_and_wall_time_worker) }
+          .from(true).to(false)
+      end
+    end
+
     it "resets the CpuAndWallTime collector only after disabling the tracepoint" do
       expect(thread_context_collector).to receive(:reset_after_fork) do
         expect(described_class::Testing._native_gc_tracepoint(cpu_and_wall_time_worker)).to_not be_enabled

spec/datadog/profiling/collectors/thread_context_spec.rb

@@ -780,7 +780,7 @@ def self.otel_sdk_available?
 
         context "when thread starts Waiting for GVL" do
           before do
-            skip "Behavior does not apply to current Ruby version" if min_ruby_for_gvl_profiling > RUBY_VERSION
+            skip_if_gvl_profiling_not_supported(self)
 
             sample # trigger context creation
             samples_from_pprof(recorder.serialize!) # flush sample
@@ -843,7 +843,7 @@ def self.otel_sdk_available?
 
         context "when thread is Waiting for GVL" do
           before do
-            skip "Behavior does not apply to current Ruby version" if min_ruby_for_gvl_profiling > RUBY_VERSION
+            skip_if_gvl_profiling_not_supported(self)
 
             sample # trigger context creation
             on_gvl_waiting(t1)
@@ -1465,7 +1465,7 @@ def sample_and_check(expected_state:)
   end
 
   describe "#on_gvl_waiting" do
-    before { skip "Behavior does not apply to current Ruby version" if min_ruby_for_gvl_profiling > RUBY_VERSION }
+    before { skip_if_gvl_profiling_not_supported(self) }
 
     context "if thread has not been sampled before" do
       it "does not record anything in the internal_thread_specific value" do
@@ -1491,7 +1491,7 @@ def sample_and_check(expected_state:)
   end
 
   describe "#on_gvl_running" do
-    before { skip "Behavior does not apply to current Ruby version" if min_ruby_for_gvl_profiling > RUBY_VERSION }
+    before { skip_if_gvl_profiling_not_supported(self) }
 
     context "if thread has not been sampled before" do
       it "does not record anything in the internal_thread_specific value" do
@@ -1571,7 +1571,7 @@ def sample_and_check(expected_state:)
   end
 
   describe "#sample_after_gvl_running" do
-    before { skip "Behavior does not apply to current Ruby version" if min_ruby_for_gvl_profiling > RUBY_VERSION }
+    before { skip_if_gvl_profiling_not_supported(self) }
 
     let(:timeline_enabled) { true }
 
@@ -1823,7 +1823,7 @@ def sample_and_check(expected_state:)
 
       describe ":gvl_waiting_at" do
         context "on supported Rubies" do
-          before { skip "Behavior does not apply to current Ruby version" if min_ruby_for_gvl_profiling > RUBY_VERSION }
+          before { skip_if_gvl_profiling_not_supported(self) }
 
           it "is initialized to GVL_WAITING_ENABLED_EMPTY (INTPTR_MAX)" do
             expect(per_thread_context.values).to all(
@@ -1833,7 +1833,7 @@ def sample_and_check(expected_state:)
         end
 
         context "on legacy Rubies" do
-          before { skip "Behavior does not apply to current Ruby version" if min_ruby_for_gvl_profiling <= RUBY_VERSION }
+          before { skip "Behavior does not apply to current Ruby version" if RUBY_VERSION >= "3.3." }
 
           it "is not set" do
             per_thread_context.each do |_thread, context|

spec/datadog/profiling/spec_helper.rb

@@ -122,8 +122,8 @@ def self.maybe_fix_label_range(key, value)
     end
   end
 
-  def min_ruby_for_gvl_profiling
-    "3.3."
+  def skip_if_gvl_profiling_not_supported(testcase)
+    testcase.skip "GVL profiling is only supported on Ruby >= 3.3" if RUBY_VERSION < "3.3."
   end
 end