diff --git a/ext/datadog_profiling_native_extension/extconf.rb b/ext/datadog_profiling_native_extension/extconf.rb index 35f0210405d..2323498ed9c 100644 --- a/ext/datadog_profiling_native_extension/extconf.rb +++ b/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" diff --git a/spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb b/spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb index 408abf1ddd8..9eaac908e9c 100644 --- a/spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb +++ b/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)) diff --git a/spec/datadog/profiling/collectors/stack_spec.rb b/spec/datadog/profiling/collectors/stack_spec.rb index 7a9ecf8997a..292a596b09e 100644 --- a/spec/datadog/profiling/collectors/stack_spec.rb +++ b/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 + # `` (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 "" + 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 diff --git a/spec/datadog/profiling/native_extension_spec.rb b/spec/datadog/profiling/native_extension_spec.rb index b3fa0c8163a..669b12319ec 100644 --- a/spec/datadog/profiling/native_extension_spec.rb +++ b/spec/datadog/profiling/native_extension_spec.rb @@ -190,7 +190,7 @@ 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") @@ -198,7 +198,7 @@ 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)"