Skip to content

Commit

Permalink
[PROF-5748] Improve error message when pkg-config tool is not installed
Browse files Browse the repository at this point in the history
In #2068 a customer reported that they were not getting profiling
with the message:

> Profiling was requested but is not supported, profiling disabled:
> Your ddtrace installation is missing support for the Continuous
> Profiler because there was a problem in setting up the libddprof
> dependency.

After investigating this issue (and adding extra tools to debug it,
see #2069) it turns out that the customer was missing the `pkg-config`
tool.

This tool is currently invoked indirectly via Ruby's `mkmf` helper,
and is used to configure linking to libddprof/libdatadog.

I must admit I was surprised that there's not better error logging
in `mkmf` when the `pkg-config` tool is actually missing
(vs it being installed but returning an error when being called).

Thus, to hopefully avoid other customers running into this issue,
I've added a bit of code to detect it, and hopefully present a
better error message in that situation.

I've also learned that `pkg-config` is "old news" on some Linux
distributions, and instead they ship something called `pkgconf`
which is a reimplementation of `pkg-config`.

Beyond the RSpec tests, this can be triggered for testing by:

1. Changing the `pkg_config` call on extconf.rb

```diff
-unless pkg_config('ddprof_ffi_with_rpath')
+unless pkg_config('ddprof_ffi_with_rpath_broken')
```

which triggers the generic message (because pkg-config is
available but returns an error since the configuration file
is not found):

```
+------------------------------------------------------------------------------+
| Could not compile the Datadog Continuous Profiler because                    |
| there was a problem in setting up the `libddprof` dependency.                |
|                                                                              |
| The Datadog Continuous Profiler will not be available,                       |
| but all other ddtrace features will work fine!                               |
|                                                                              |
| For help solving this issue, please contact Datadog support at               |
| <https://docs.datadoghq.com/help/>.                                          |
+------------------------------------------------------------------------------+
```

2. Crippling `pkg_config`:

```bash
$ docker-compose run --no-deps --rm tracer-2.1 /bin/bash
  # doing this inside docker is fine because it doesn't persist
$ rm /usr/bin/pkg-config
$ bundle exec rake clean compile
 # ...
+------------------------------------------------------------------------------+
| Could not compile the Datadog Continuous Profiler because                    |
| the `pkg-config` system tool is missing.                                     |
| This issue can usually be fixed by installing:                               |
| 1. the `pkg-config` package on Homebrew and Debian/Ubuntu-based Linux;       |
| 2. the `pkgconf` package on Arch and Alpine-based Linux;                     |
| 3. the `pkgconf-pkg-config` package on Fedora/Red Hat-based Linux.           |
|                                                                              |
| The Datadog Continuous Profiler will not be available,                       |
| but all other ddtrace features will work fine!                               |
|                                                                              |
| For help solving this issue, please contact Datadog support at               |
| <https://docs.datadoghq.com/help/>.                                          |
+------------------------------------------------------------------------------+
```

Closes #2068
  • Loading branch information
ivoanjo committed Jul 7, 2022
1 parent 96c395d commit d8291ca
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 1 deletion.
9 changes: 8 additions & 1 deletion ext/ddtrace_profiling_native_extension/extconf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,14 @@ def add_compiler_flag(flag)
Logging.message(" [ddtrace] PKG_CONFIG_PATH set to #{ENV['PKG_CONFIG_PATH'].inspect}\n")

unless pkg_config('ddprof_ffi_with_rpath')
skip_building_extension!(Datadog::Profiling::NativeExtensionHelpers::Supported::FAILED_TO_CONFIGURE_LIBDDPROF)
skip_building_extension!(
if Datadog::Profiling::NativeExtensionHelpers::Supported.pkg_config_missing?
Datadog::Profiling::NativeExtensionHelpers::Supported::PKG_CONFIG_IS_MISSING
else
# Less specific error message
Datadog::Profiling::NativeExtensionHelpers::Supported::FAILED_TO_CONFIGURE_LIBDDPROF
end
)
end

# See comments on the helper method being used for why we need to additionally set this
Expand Down
19 changes: 19 additions & 0 deletions ext/ddtrace_profiling_native_extension/native_extension_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,14 @@ def self.render_skipped_reason_file(reason:, suggested:)
[*reason, *suggested].join(' ')
end

# mkmf sets $PKGCONFIG after the `pkg_config` gets used in extconf.rb. When `pkg_config` is unsuccessful, we use
# this helper to decide if we can show more specific error message vs a generic "something went wrong".
def self.pkg_config_missing?(command: $PKGCONFIG) # rubocop:disable Style/GlobalVars
pkg_config_available = command && xsystem("#{command} --version")

pkg_config_available != true
end

CONTACT_SUPPORT = [
'For help solving this issue, please contact Datadog support at',
'<https://docs.datadoghq.com/help/>.',
Expand Down Expand Up @@ -153,6 +161,17 @@ def self.render_skipped_reason_file(reason:, suggested:)
suggested: CONTACT_SUPPORT,
)

# Validation for this check is done in extconf.rb because it relies on mkmf
PKG_CONFIG_IS_MISSING = explain_issue(
#+-----------------------------------------------------------------------------+
'the `pkg-config` system tool is missing.',
'This issue can usually be fixed by installing:',
'1. the `pkg-config` package on Homebrew and Debian/Ubuntu-based Linux;',
'2. the `pkgconf` package on Arch and Alpine-based Linux;',
'3. the `pkgconf-pkg-config` package on Fedora/Red Hat-based Linux.',
suggested: CONTACT_SUPPORT,
)

private_class_method def self.disabled_via_env?
disabled_via_env = explain_issue(
'the `DD_PROFILING_NO_EXTENSION` environment variable is/was set to',
Expand Down
42 changes: 42 additions & 0 deletions spec/datadog/profiling/native_extension_helpers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,46 @@
end
end
end

describe '.pkg_config_missing?' do
subject(:pkg_config_missing) { described_class.pkg_config_missing?(command: command) }

before do
skip_if_profiling_not_supported(self)
end

context 'when command is not available' do
let(:command) { nil }

it { is_expected.to be true }
end

# This spec is semi-realistic, because it actually calls into the pkg-config external process.
#
# We know pkg-config must be avaliable on the machinne running the tests because otherwise profiling would not be
# supported (and thus `skip_if_profiling_not_supported` would've been triggered).
#
# We could also mock the entire interaction, but this seemed like a simple enough way to go.
context 'when command is available' do
before do
# This helper is designed to be called from extconf.rb, which requires mkmf, which defines xsystem.
# When executed in RSpec, mkmf is not required, so we replace it with the regular system call.
without_partial_double_verification do
expect(described_class).to receive(:xsystem) { |*args| system(*args) }
end
end

context 'and pkg-config can successfully be called' do
let(:command) { 'pkg-config' }

it { is_expected.to be false }
end

context 'and pkg-config cannot be called' do
let(:command) { 'does-not-exist' }

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

0 comments on commit d8291ca

Please sign in to comment.