Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions ext/datadog_profiling_native_extension/extconf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
36 changes: 32 additions & 4 deletions spec/datadog/profiling/collectors/stack_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions spec/datadog/profiling/native_extension_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down