From 40f05ead89d3e4587735b197f4900f4ccb613ac7 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 11 Sep 2026 08:19:33 +0000 Subject: [PATCH 1/6] [NO-TICKET] Profiling: Add integration test with GC.stress **What does this PR do?** This PR adds a new "integration" test to the `cpu_and_wall_time_worker_spec.rb` where we run the profiler for a brief period with all features enabled under Ruby's `GC.stress` setting. **Motivation:** In https://github.com/DataDog/dd-trace-rb/pull/6242 and https://github.com/DataDog/dd-trace-rb/pull/6245 we were able to reproduce those bugs using `GC.stress`. To try to catch possible similar issues in the future, it seemed useful to have an integration test where we ran the profiler with `GC.stress`. **Additional Notes:** The big downside of `GC.stress` is how slow it makes the test suite. On my workspace, it takes around 20-30s to run just this one new testcase, which is why I've made it only run in CI as I really value quick iteration on our testsuite and having mega-slow tests breaks that. I also evaluated how feasible it would be to run the entire `cpu_and_wall_time_worker_spec.rb` with `GC.stress` and unfortunately the answer isn't great -- it would take hours AND in particular it would need modifications since we have a bunch of timeouts for cross-thread stuff that would need heavy tweaking. Ideas on how we could get more coverage without a lot of work are welcome ;) **How to test the change?** Validate CI is green and this test passes! --- .../cpu_and_wall_time_worker_spec.rb | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) 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 fb7227538c4..46dd1be678c 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 @@ -1195,6 +1195,46 @@ def skip_if_signal_handler_sampling_not_supported end end + context "GC stress enabled integration test", :memcheck_valgrind_skip do + before do + unless ENV["DATADOG_GEM_CI"] == "true" + skip "Test is slow so we only run it when " \ + "DATADOG_GEM_CI env var is true" + end + end + + let(:allocation_profiling_enabled) { true } + let(:allocation_counting_enabled) { true } + let(:heap_profiling_enabled) { RubyVersion.is?(">= 3.1") } + let(:gvl_profiling_enabled) { RubyVersion.is?(">= 3.2") } + let(:sighandler_sampling_enabled) do + !(RubyVersion.is?("< 3.2.5") || RubyVersion.is?(">= 3.3", "< 3.3.4")) + end + + it "runs the profiler successfully" do + on_failure_proc_called = false + cpu_and_wall_time_worker # pre-create instances before enabling stress + + begin + GC.stress = true + + cpu_and_wall_time_worker.start(on_failure_proc: proc { on_failure_proc_called = true }) + cpu_and_wall_time_worker.wait_until_running(timeout_seconds: 30) + + 10.times { |i| i.to_s } + recorder.serialize! + 10.times { |i| i.to_s } + + cpu_and_wall_time_worker.stop + recorder.serialize! + ensure + GC.stress = false + end + + expect(on_failure_proc_called).to be false + end + end + context "when the _native_sampling_loop terminates with an exception" do it "calls the on_failure_proc" do expect(described_class).to receive(:_native_sampling_loop).and_raise(StandardError.new("Simulated error")) From 0de0fb67848e62d1865c669a2c36d1b937632e12 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 11 Sep 2026 09:36:39 +0000 Subject: [PATCH 2/6] Minor: Improve error messages if something fails --- .../collectors/cpu_and_wall_time_worker_spec.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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 46dd1be678c..251d0a7d2ab 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 @@ -1231,7 +1231,14 @@ def skip_if_signal_handler_sampling_not_supported GC.stress = false end - expect(on_failure_proc_called).to be false + expect(on_failure_proc_called).to( + be(false), + -> { + failure_exception = cpu_and_wall_time_worker.send(:failure_exception) + "Profiler failed to run cleanly, failure_exception: #{failure_exception.inspect}\n" \ + "#{failure_exception&.backtrace&.join("\n")}" + } + ) end end From 0857e9ce6c9ae430a984d62dac7f03a59d66ef2e Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Fri, 11 Sep 2026 10:39:10 +0100 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Benoit Daloze --- .../profiling/collectors/cpu_and_wall_time_worker_spec.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) 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 251d0a7d2ab..1b1e4f310a9 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 @@ -1198,8 +1198,7 @@ def skip_if_signal_handler_sampling_not_supported context "GC stress enabled integration test", :memcheck_valgrind_skip do before do unless ENV["DATADOG_GEM_CI"] == "true" - skip "Test is slow so we only run it when " \ - "DATADOG_GEM_CI env var is true" + skip "Test is slow so we only run it with DATADOG_GEM_CI=true" end end @@ -1215,8 +1214,8 @@ def skip_if_signal_handler_sampling_not_supported on_failure_proc_called = false cpu_and_wall_time_worker # pre-create instances before enabling stress - begin - GC.stress = true + GC.stress = true + begin cpu_and_wall_time_worker.start(on_failure_proc: proc { on_failure_proc_called = true }) cpu_and_wall_time_worker.wait_until_running(timeout_seconds: 30) From 9b5c2a490d27499557e2b19f36ce3633256ae43c Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Wed, 23 Sep 2026 11:52:45 +0000 Subject: [PATCH 4/6] Make rubocop happy --- .../profiling/collectors/cpu_and_wall_time_worker_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 1b1e4f310a9..e007699a801 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 @@ -1214,9 +1214,8 @@ def skip_if_signal_handler_sampling_not_supported on_failure_proc_called = false cpu_and_wall_time_worker # pre-create instances before enabling stress - GC.stress = true - begin - + GC.stress = true + begin cpu_and_wall_time_worker.start(on_failure_proc: proc { on_failure_proc_called = true }) cpu_and_wall_time_worker.wait_until_running(timeout_seconds: 30) From b449f7b26be40f5727e2197c10731e14f93978f8 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Wed, 23 Sep 2026 14:36:46 +0000 Subject: [PATCH 5/6] Add simple sanity check that we did collect allocation samples --- .../profiling/collectors/cpu_and_wall_time_worker_spec.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 e007699a801..28810f5cd55 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 @@ -1224,7 +1224,7 @@ def skip_if_signal_handler_sampling_not_supported 10.times { |i| i.to_s } cpu_and_wall_time_worker.stop - recorder.serialize! + profile = recorder.serialize! ensure GC.stress = false end @@ -1237,6 +1237,11 @@ def skip_if_signal_handler_sampling_not_supported "#{failure_exception&.backtrace&.join("\n")}" } ) + + samples = samples_from_pprof(profile).select do |sample| + sample.locations.any? { |location| location.path == __FILE__ } + end + expect(samples.map(&:values)).to include(include("alloc-samples": be > 0)) end end From 8631f6a5ec04f1e2cf5a8be88365b62703ce9a92 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Wed, 23 Sep 2026 15:46:51 +0000 Subject: [PATCH 6/6] Make sure dynamic sampling rate doesn't decide to skip samples ...otherwise we might (as just happened in CI) end up in a situation where e.g. no allocation samples are taken. --- .../profiling/collectors/cpu_and_wall_time_worker_spec.rb | 2 ++ 1 file changed, 2 insertions(+) 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 28810f5cd55..0e881c2f94d 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 @@ -1202,6 +1202,8 @@ def skip_if_signal_handler_sampling_not_supported end end + # Make sure the profiler doesn't skip samples during GC.stress + let(:options) { {dynamic_sampling_rate_enabled: false} } let(:allocation_profiling_enabled) { true } let(:allocation_counting_enabled) { true } let(:heap_profiling_enabled) { RubyVersion.is?(">= 3.1") }