Skip to content

Commit

Permalink
[NO-TICKET] Disable Dir interruption bug workaround on Ruby 3.4+
Browse files Browse the repository at this point in the history
**What does this PR do?**

This PR disables the Dir interruption bug workaround that was added in
 #3720 for Ruby versions 3.4 and above.

**Motivation:**

The bug has been fixed upstream (see
https://bugs.ruby-lang.org/issues/20586 ) and thus is no longer needed!

**Additional Notes:**

N/A

**How to test the change?**

This change includes test coverage.
  • Loading branch information
ivoanjo authored and y9v committed Sep 13, 2024
1 parent d5a6190 commit 74b01b9
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 12 deletions.
6 changes: 2 additions & 4 deletions lib/datadog/core/configuration/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,8 @@ def initialize(*_)
# The profiler gathers data by sending `SIGPROF` unix signals to Ruby application threads.
#
# We've discovered that this can trigger a bug in a number of Ruby APIs in the `Dir` class, as
# described in https://github.com/DataDog/dd-trace-rb/issues/3450 . This workaround prevents the issue
# from happening by monkey patching the affected APIs.
#
# (In the future, once a fix lands upstream, we'll disable this workaround for Rubies that don't need it)
# described in https://bugs.ruby-lang.org/issues/20586 .
# This was fixed for Ruby 3.4+, and this setting is a no-op for those versions.
#
# @default `DD_PROFILING_DIR_INTERRUPTION_WORKAROUND_ENABLED` environment variable as a boolean,
# otherwise `true`
Expand Down
5 changes: 1 addition & 4 deletions lib/datadog/profiling/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -435,10 +435,7 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)
end

private_class_method def self.dir_interruption_workaround_enabled?(settings, no_signals_workaround_enabled)
return false if no_signals_workaround_enabled

# NOTE: In the future this method will evolve to check for Ruby versions affected and not apply the workaround
# when it's not needed but currently all known Ruby versions are affected.
return false if no_signals_workaround_enabled || RUBY_VERSION >= "3.4"

settings.profiling.advanced.dir_interruption_workaround_enabled
end
Expand Down
22 changes: 18 additions & 4 deletions spec/datadog/profiling/component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -543,13 +543,27 @@
let(:no_signals_workaround_enabled) { false }

before do
expect(described_class).to receive(:no_signals_workaround_enabled?).and_return(no_signals_workaround_enabled)
allow(described_class).to receive(:no_signals_workaround_enabled?).and_return(no_signals_workaround_enabled)
end

it "is enabled by default" do
expect(Datadog::Profiling::Ext::DirMonkeyPatches).to receive(:apply!)
context "on Ruby >= 3.4" do
before { skip "Behavior does not apply to current Ruby version" if RUBY_VERSION < "3.4." }

build_profiler_component
it "is never applied" do
expect(Datadog::Profiling::Ext::DirMonkeyPatches).to_not receive(:apply!)

build_profiler_component
end
end

context "on Ruby < 3.4" do
before { skip "Behavior does not apply to current Ruby version" if RUBY_VERSION >= "3.4." }

it "is applied by default" do
expect(Datadog::Profiling::Ext::DirMonkeyPatches).to receive(:apply!)

build_profiler_component
end
end

context "when the no signals workaround is enabled" do
Expand Down

0 comments on commit 74b01b9

Please sign in to comment.