Skip to content

Commit

Permalink
[PROF-6447] Improve detection of mysql2 gem incompatibilities with pr…
Browse files Browse the repository at this point in the history
…ofiler

**What does this PR do?**:

As part of #2702 (more specifically, in a7d3b73) we added a check
for the `mysql2` gem which avoided turning on the new profiler when we
detected it.

This check was a bit heavy-handed because the known incompatibility
between the `mysql2` gem and the profiler only happens if the
`mysql2` gem is using a legacy version of the `libmysqlclient` C
library. (Specifically, a version prior to 8.0.0)

In this PR, we improve the check to _actually_ check the
`libmysqlclient` version. Thus, we will only fall back to using the
legacy profiler as needed, not always as before.

**Motivation**:

The `mysql2` gem is a commonly-used database driver for Ruby
(and Ruby on Rails) applications, and we don't want these customers to
get stuck using the old profiler.

**Additional Notes**:

As part of the check, we need to `require 'mysql2'` during profiler
initialization (since we need to call a method provided by the gem).

In the rare just-in-case situation that this causes issues
and customers don't want this to happen, this PR also introduces a
new option to disable this behavior:

* `DD_PROFILING_SKIP_MYSQL2_CHECK` via environment variable
* `c.profiling.advanced.skip_mysql2_check` via code

**How to test the change?**:

Test coverage is included.

To test this with the actual `mysql2` gem, our CI images have a modern
version of `libmysqlclient` (specifically, the mariadb variant):

```
$ DD_PROFILING_ENABLED=true DD_TRACE_DEBUG=true bundle exec ddtracerb exec ruby -e "sleep 1"
DEBUG -- ddtrace: [ddtrace] (/app/lib/datadog/profiling/component.rb:205:in `compatible_libmysqlclient_version?') Requiring `mysql2` to check if the `libmysqlclient` version it uses is compatible with profiling
DEBUG -- ddtrace: [ddtrace] (/app/lib/datadog/profiling/component.rb:218:in `compatible_libmysqlclient_version?') The `mysql2` gem is using a compatible version of the `libmysqlclient` library (10.5.15)
DEBUG -- ddtrace: [ddtrace] (/app/lib/datadog/core/configuration/components.rb:92:in `startup!') Profiling started
DEBUG -- ddtrace: [ddtrace] (/app/lib/datadog/profiling/collectors/cpu_and_wall_time_worker.rb:58:in `block in start') Starting thread for: #<Datadog::Profiling::Collectors::CpuAndWallTimeWorker:0x00005e979dd5e498>
```

...and you can use an older Ubuntu 18.04 image to see the failing case:

```
$ DD_PROFILING_ENABLED=true DD_TRACE_DEBUG=true bundle exec ddtracerb exec ruby -e "sleep 1"
DEBUG -- ddtrace: [ddtrace] (/working/lib/datadog/profiling/component.rb:205:in `compatible_libmysqlclient_version?') Requiring `mysql2` to check if the `libmysqlclient` version it uses is compatible with profiling
DEBUG -- ddtrace: [ddtrace] (/working/lib/datadog/profiling/component.rb:218:in `compatible_libmysqlclient_version?') The `mysql2` gem is using an incompatible version of the `libmysqlclient` library (5.7.41)
 WARN -- ddtrace: [ddtrace] (/working/lib/datadog/profiling/component.rb:170:in `enable_new_profiler?') Falling back to legacy profiler because an incompatible version of the mysql2 gem is installed. Older versions of libmysqlclient (the C library used by the mysql2 gem) have a bug in their signal handling code that the new profiler can trigger. This bug (https://bugs.mysql.com/bug.php?id=83109) is fixed in libmysqlclient versions 8.0.0 and above.
DEBUG -- ddtrace: [ddtrace] (/working/lib/datadog/core/configuration/components.rb:92:in `startup!') Profiling started
DEBUG -- ddtrace: [ddtrace] (/working/lib/datadog/core/workers/async.rb:130:in `start_worker') Starting thread for: #<Datadog::Profiling::Collectors::OldStack:0x00005604f7ac46b0>
```
  • Loading branch information
ivoanjo committed Apr 11, 2023
1 parent 4344616 commit 8225345
Show file tree
Hide file tree
Showing 7 changed files with 188 additions and 11 deletions.
1 change: 1 addition & 0 deletions Steepfile
Original file line number Diff line number Diff line change
Expand Up @@ -658,4 +658,5 @@ target :ddtrace do
library 'sinatra'
library 'google-protobuf'
library 'protobuf-cucumber'
library 'mysql2'
end
10 changes: 10 additions & 0 deletions lib/datadog/core/configuration/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,16 @@ def initialize(*_)
#
# @default `true` on Ruby 2.x, `false` on Ruby 3.x
option :allocation_counting_enabled, default: RUBY_VERSION.start_with?('2.')

# Can be used to disable checking which version of `libmysqlclient` is being used by the `mysql2` gem.
#
# This setting is only used when the `mysql2` gem is installed.
#
# @default `DD_PROFILING_SKIP_MYSQL2_CHECK` environment variable, otherwise `false`
option :skip_mysql2_check do |o|
o.default { env_to_bool('DD_PROFILING_SKIP_MYSQL2_CHECK', false) }
o.lazy
end
end

# @public_api
Expand Down
51 changes: 45 additions & 6 deletions lib/datadog/profiling/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,12 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)

return false if RUBY_VERSION.start_with?('2.3.', '2.4.', '2.5.')

if Gem.loaded_specs['mysql2']
if Gem.loaded_specs['mysql2'] && !compatible_libmysqlclient_version?(settings)
Datadog.logger.warn(
'Falling back to legacy profiler because mysql2 gem is installed. Older versions of libmysqlclient (the C ' \
'Falling back to legacy profiler because an incompatible version of the mysql2 gem is installed. ' \
'Older versions of libmysqlclient (the C ' \
'library used by the mysql2 gem) have a bug in their signal handling code that the new profiler can trigger. ' \
'This bug (https://bugs.mysql.com/bug.php?id=83109) is fixed in libmysqlclient versions 8.0.0 and above. ' \
'If your Linux distribution provides a modern libmysqlclient, you can force-enable the new CPU Profiling 2.0 ' \
'profiler by using the `DD_PROFILING_FORCE_ENABLE_NEW` environment variable or the ' \
'`c.profiling.advanced.force_enable_new_profiler` setting.' \
'This bug (https://bugs.mysql.com/bug.php?id=83109) is fixed in libmysqlclient versions 8.0.0 and above. '
)
return false
end
Expand All @@ -191,6 +189,47 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)

true
end

# Versions of libmysqlclient prior to 8.0.0 are known to have buggy handling of system call interruptions.
# The profiler can sometimes cause system call interruptions, and so this combination can cause queries to fail.
#
# See https://bugs.mysql.com/bug.php?id=83109 and
# https://docs.datadoghq.com/profiler/profiler_troubleshooting/ruby/#unexpected-run-time-failures-and-errors-from-ruby-gems-that-use-native-extensions-in-dd-trace-rb-1110
# for details.
#
# The `mysql2` gem's `info` method can be used to determine which `libmysqlclient` version is in use, and thus to
# detect if it's safe for the profiler to use signals or if we need to employ a fallback.
private_class_method def self.compatible_libmysqlclient_version?(settings)
return false if settings.profiling.advanced.skip_mysql2_check

Datadog.logger.debug(
'Requiring `mysql2` to check if the `libmysqlclient` version it uses is compatible with profiling'
)

begin
require 'mysql2'

return false unless defined?(Mysql2::Client) && Mysql2::Client.respond_to?(:info)

libmysqlclient_version = Gem::Version.new(Mysql2::Client.info[:version])

compatible = libmysqlclient_version >= Gem::Version.new('8.0.0')

Datadog.logger.debug(
"The `mysql2` gem is using #{compatible ? 'a compatible' : 'an incompatible'} version of " \
"the `libmysqlclient` library (#{libmysqlclient_version})"
)

compatible
rescue StandardError, LoadError => e
Datadog.logger.warn(
'Failed to probe `mysql2` gem information. ' \
"Cause: #{e.class.name} #{e.message} Location: #{Array(e.backtrace).first}"
)

false
end
end
end
end
end
2 changes: 2 additions & 0 deletions sig/datadog/profiling/component.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ module Datadog
def self.print_new_profiler_warnings: () -> void

def self.enable_new_profiler?: (untyped settings) -> bool

def self.compatible_libmysqlclient_version?: (untyped settings) -> bool
end
end
end
35 changes: 35 additions & 0 deletions spec/datadog/core/configuration/settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,41 @@
.to(false)
end
end

describe '#skip_mysql2_check' do
subject(:force_enable_gc_profiling) { settings.profiling.advanced.skip_mysql2_check }

context 'when DD_PROFILING_SKIP_MYSQL2_CHECK' do
around do |example|
ClimateControl.modify('DD_PROFILING_SKIP_MYSQL2_CHECK' => environment) do
example.run
end
end

context 'is not defined' do
let(:environment) { nil }

it { is_expected.to be false }
end

{ 'true' => true, 'false' => false }.each do |string, value|
context "is defined as #{string}" do
let(:environment) { string }

it { is_expected.to be value }
end
end
end
end

describe '#skip_mysql2_check=' do
it 'updates the #skip_mysql2_check setting' do
expect { settings.profiling.advanced.skip_mysql2_check = true }
.to change { settings.profiling.advanced.skip_mysql2_check }
.from(false)
.to(true)
end
end
end

describe '#upload' do
Expand Down
95 changes: 90 additions & 5 deletions spec/datadog/profiling/component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -385,14 +385,99 @@
context 'when mysql2 gem is available' do
include_context 'loaded gems', :mysql2 => Gem::Version.new('0.5.5')

before { allow(Datadog.logger).to receive(:warn) }
before do
allow(Datadog.logger).to receive(:warn)
allow(Datadog.logger).to receive(:debug)
end

it { is_expected.to be false }
context 'when skip_mysql2_check is enabled' do
before { settings.profiling.advanced.skip_mysql2_check = true }

it 'logs a warning message mentioning that the legacy profiler is going to be used' do
expect(Datadog.logger).to receive(:warn).with(/Falling back to legacy profiler/)
it { is_expected.to be false }

enable_new_profiler?
it 'logs a warning message mentioning that the legacy profiler is going to be used' do
expect(Datadog.logger).to receive(:warn).with(/Falling back to legacy profiler/)

enable_new_profiler?
end
end

context 'when there is an issue requiring mysql2' do
before { allow(described_class).to receive(:require).and_raise(LoadError.new('Simulated require failure')) }

it { is_expected.to be false }

it 'logs a warning' do
expect(Datadog.logger).to receive(:warn).with(/Failed to probe `mysql2` gem information/)

enable_new_profiler?
end
end

context 'when mysql2 is required successfully' do
before { allow(described_class).to receive(:require).with('mysql2') }

it 'logs a debug message stating mysql2 will be required' do
expect(Datadog.logger).to receive(:debug).with(/Requiring `mysql2` to check/)

enable_new_profiler?
end

context 'when mysql2 gem does not provide the info method' do
before do
stub_const('Mysql2::Client', double('Fake Mysql2::Client'))
end

it { is_expected.to be false }
end

context 'when an error is raised while probing the mysql2 gem' do
before do
fake_client = double('Fake Mysql2::Client')
stub_const('Mysql2::Client', fake_client)
expect(fake_client).to receive(:info).and_raise(ArgumentError.new('Simulated call failure'))
end

it { is_expected.to be false }

it 'logs a warning including the error details' do
expect(Datadog.logger).to receive(:warn).with(/Failed to probe `mysql2` gem information/)

enable_new_profiler?
end
end

context 'when mysql2 gem is using a version of libmysqlclient < 8.0.0' do
before do
fake_client = double('Fake Mysql2::Client')
stub_const('Mysql2::Client', fake_client)
expect(fake_client).to receive(:info).and_return({ version: '7.9.9' })
end

it { is_expected.to be false }

it 'logs a warning message mentioning that the legacy profiler is going to be used' do
expect(Datadog.logger).to receive(:warn).with(/Falling back to legacy profiler/)

enable_new_profiler?
end
end

context 'when mysql2 gem is using a version of libmysqlclient >= 8.0.0' do
before do
fake_client = double('Fake Mysql2::Client')
stub_const('Mysql2::Client', fake_client)
expect(fake_client).to receive(:info).and_return({ version: '8.0.0' })
end

it { is_expected.to be true }

it 'does not log any warning message' do
expect(Datadog.logger).to_not receive(:warn)

enable_new_profiler?
end
end
end

context 'when force_enable_new_profiler is enabled' do
Expand Down
5 changes: 5 additions & 0 deletions vendor/rbs/mysql2/0/mysql2.rbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module Mysql2
class Client
def self.info: () -> { version: ::String }
end
end

0 comments on commit 8225345

Please sign in to comment.