Merge pull request #2134 from DataDog/ivoanjo/prof-5748-handle-missin…

…g-pkgconfig

[PROF-5748] Improve error message when pkg-config tool is not installed

ext/ddtrace_profiling_native_extension/extconf.rb

@@ -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

ext/ddtrace_profiling_native_extension/native_extension_helpers.rb

@@ -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/>.',
@@ -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',

spec/datadog/profiling/native_extension_helpers_spec.rb

@@ -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 available on the machine 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