Merge pull request #5091 from DataDog/ivoanjo/prof-13115-ruby-4_0_0-preview2-profiler

[PROF-13115] Fix profiler support for Ruby 4.0.0-preview2

Author: ivoanjo

ext/datadog_profiling_native_extension/extconf.rb

@@ -141,8 +141,10 @@ def skip_building_extension!(reason)
 # On Ruby 3.5, we can't ask the object_id from IMEMOs (https://github.com/ruby/ruby/pull/13347)
 $defs << "-DNO_IMEMO_OBJECT_ID" unless RUBY_VERSION < "3.5"
 
-# On Ruby 2.5 and 3.3, this symbol was not visible. It is on 2.6 to 3.2, as well as 3.4+
-$defs << "-DNO_RB_OBJ_INFO" if RUBY_VERSION.start_with?("2.5", "3.3")
+# This symbol is exclusively visible on certain Ruby versions: 2.6 to 3.2, as well as 3.4 and 3.5-preview1 (but not 4.0)
+# It's only used to get extra information about an object when a failure happens, so it's a "very nice to have" but not
+# actually required for correct behavior of the profiler.
+$defs << "-DNO_RB_OBJ_INFO" if RUBY_VERSION.start_with?("2.5", "3.3", "4.0")
 
 # On older Rubies, rb_postponed_job_preregister/rb_postponed_job_trigger did not exist
 $defs << "-DNO_POSTPONED_TRIGGER" if RUBY_VERSION < "3.3"

spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb

@@ -683,8 +683,8 @@
       end
 
       it "records allocated objects" do
-        # TODO: Ruby 3.5 - Remove this skip after investigation.
-        pending('Allocation profiling call not working correctly on Ruby 3.5.0-preview1') if RUBY_DESCRIPTION.include?('preview')
+        # TODO: Remove this when Ruby 3.5.0-preview1 is removed from CI
+        pending('Allocation profiling call not working correctly on Ruby 3.5.0-preview1') if RUBY_DESCRIPTION.include?('3.5.0preview1')
 
         stub_const("CpuAndWallTimeWorkerSpec::TestStruct", Struct.new(:foo))
 

spec/datadog/profiling/collectors/stack_spec.rb

@@ -15,9 +15,9 @@
   let(:metric_values) { {"cpu-time" => 123, "cpu-samples" => 456, "wall-time" => 789} }
   let(:labels) { {"label_a" => "value_a", "label_b" => "value_b", "state" => "unknown"}.to_a }
 
-  let(:raw_reference_stack) { stacks.fetch(:reference) }
-  let(:reference_stack) { convert_reference_stack(raw_reference_stack) }
-  let(:gathered_stack) { stacks.fetch(:gathered) }
+  let(:raw_reference_stack) { stacks.fetch(:reference).freeze }
+  let(:reference_stack) { convert_reference_stack(raw_reference_stack).freeze }
+  let(:gathered_stack) { stacks.fetch(:gathered).freeze }
   let(:native_filenames_enabled) { false }
 
   def sample(thread, recorder_instance, metric_values_hash, labels_array, **options)
@@ -220,7 +220,35 @@ def call_sleep
 
       # I opted to join these two expects to avoid running the `load` above more than once
       it "matches the Ruby backtrace API AND has a sleeping frame at the top of the stack" do
-        expect(gathered_stack).to eq reference_stack
+        if RUBY_VERSION.start_with?("4.")
+          # In Ruby 4, due to https://bugs.ruby-lang.org/issues/20968 while internally Integer#times has the path
+          # `<internal:numeric>` (and this is what the profiler observes), Ruby actually hides this and "blames" it
+          # on the last ruby file/line that was on the stack.
+          #
+          # @ivoanjo: At this point I'm not sure we want to match that behavior as we don't match it either when
+          # using the "native filenames" feature. So for now we adjust the assertions to account for that
+          unmatched_indexes =
+            reference_stack.each_with_index.select { |frame, index| frame.base_label == "times" }.map(&:last)
+          expect(unmatched_indexes).to_not be_empty
+
+          gathered_stack_without_unmatched = gathered_stack.dup
+          reference_stack_without_unmatched = reference_stack.dup
+
+          # Check the expected frames are different -- and remove them from the match
+          unmatched_indexes.sort.reverse_each do |index|
+            expect(gathered_stack[index].path).to eq "<internal:numeric>"
+            expect(reference_stack[index].path).to end_with "/interesting_backtrace_helper.rb"
+
+            gathered_stack_without_unmatched.delete_at(index)
+            reference_stack_without_unmatched.delete_at(index)
+          end
+
+          # ...match the rest of the frames
+          expect(gathered_stack_without_unmatched).to eq reference_stack_without_unmatched
+        else
+          expect(gathered_stack).to eq reference_stack
+        end
+
         expect(reference_stack.first.base_label).to eq "sleep"
       end
     end

spec/datadog/profiling/native_extension_spec.rb

@@ -190,15 +190,15 @@
     subject(:safe_object_info) { described_class::Testing._native_safe_object_info(object_to_inspect) }
 
     context "on a Ruby with rb_obj_info" do
-      before { skip "Behavior does not apply to current Ruby version" if RUBY_VERSION.start_with?("2.5", "3.3") }
+      before { skip "Behavior does not apply to current Ruby version" if RUBY_VERSION.start_with?("2.5", "3.3", "4.0") }
 
       it "returns a string with information about the object" do
         expect(safe_object_info).to include("T_STRING")
       end
     end
 
     context "on a Ruby without rb_obj_info" do
-      before { skip "Behavior does not apply to current Ruby version" unless RUBY_VERSION.start_with?("2.5", "3.3") }
+      before { skip "Behavior does not apply to current Ruby version" unless RUBY_VERSION.start_with?("2.5", "3.3", "4.0") }
 
       it "returns a placeholder string and does not otherwise fail" do
         expect(safe_object_info).to eq "(No rb_obj_info for current Ruby)"