From 80d5b611e1502b4edb0750c9bae1cbc337e7c5d7 Mon Sep 17 00:00:00 2001
From: Ivo Anjo <ivo.anjo@datadoghq.com>
Date: Fri, 24 Nov 2023 12:11:49 +0000
Subject: [PATCH] Allow profiler to run without "no signals" workaround on
 passenger 6.0.19+

**What does this PR do?**

This PR is the last step to fully fixing the issue reported in #2976.

In that issue, we discovered that he passenger web server had an
incompatibility with the profiler. At the time, we added code to
detect when passenger was in use and apply the "no signals" workaround
to avoid this issue out-of-the-box.

Upstream passenger has since accepted our PR to fix the issue, allowing
us in this PR to change our detection logic to:

a) Not enable the "no signals" workaround on passenger 6.0.19+
b) Provide an actionable error message to customers on older passenger
   versions to tell them that the best option is to upgrade

**Motivation:**

Improve the out-of-the-box experience of profiler users that use the
passenger web server.

**Additional Notes:**

N/A

**How to test the change?**

I used the following `config.ru`:

```ruby
def self.fib(n)
  n <= 1 ? n : fib(n-1) + fib(n-2)
end

app = ->(env) {
  puts " ** Got request!"

  [200, {}, ["Hello, World! #{fib(30)}"]]
}
run app
```

and `Gemfile`:

```ruby
source 'https://rubygems.org'

gem 'ddtrace', path: 'path/to/local/repo'
 #gem 'passenger', '= 6.0.18'
gem 'passenger', '= 6.0.19'
gem 'puma'
```

to validate that the passenger versions are correctly detected +
request processing is not impacted on either.

Fixes #2976
---
 lib/datadog/profiling/component.rb       | 18 ++++++++++-----
 sig/datadog/profiling/component.rbs      |  1 +
 spec/datadog/profiling/component_spec.rb | 28 ++++++++++++++++++++++--
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/lib/datadog/profiling/component.rb b/lib/datadog/profiling/component.rb
index fe2ba74f758..6b03ea564c1 100644
--- a/lib/datadog/profiling/component.rb
+++ b/lib/datadog/profiling/component.rb
@@ -171,12 +171,11 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)
           return true
         end
 
-        if defined?(::PhusionPassenger)
+        if (defined?(::PhusionPassenger) || Gem.loaded_specs['passenger']) && incompatible_passenger_version?
           Datadog.logger.warn(
-            'Enabling the profiling "no signals" workaround because the passenger web server is in use. ' \
-            'This is needed because passenger is currently incompatible with the normal working mode ' \
-            'of the profiler, as detailed in <https://github.com/DataDog/dd-trace-rb/issues/2976>. ' \
-            'Profiling data will have lower quality.'
+            'Enabling the profiling "no signals" workaround because an incompatible version of the passenger gem is ' \
+            'installed. Profiling data will have lower quality.' \
+            'To fix this, upgrade the passenger gem to version 6.0.19 or above.'
           )
           return true
         end
@@ -237,6 +236,15 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:)
           true
         end
       end
+
+      # See https://github.com/datadog/dd-trace-rb/issues/2976 for details.
+      private_class_method def self.incompatible_passenger_version?
+        if Gem.loaded_specs['passenger']
+          Gem.loaded_specs['passenger'].version < Gem::Version.new('6.0.19')
+        else
+          true
+        end
+      end
     end
   end
 end
diff --git a/sig/datadog/profiling/component.rbs b/sig/datadog/profiling/component.rbs
index c3b8bc92240..75f739c8784 100644
--- a/sig/datadog/profiling/component.rbs
+++ b/sig/datadog/profiling/component.rbs
@@ -23,6 +23,7 @@ module Datadog
       def self.no_signals_workaround_enabled?: (untyped settings) -> bool
 
       def self.incompatible_libmysqlclient_version?: (untyped settings) -> bool
+      def self.incompatible_passenger_version?: () -> bool
     end
   end
 end
diff --git a/spec/datadog/profiling/component_spec.rb b/spec/datadog/profiling/component_spec.rb
index 7d806e3b543..7c64aa27f21 100644
--- a/spec/datadog/profiling/component_spec.rb
+++ b/spec/datadog/profiling/component_spec.rb
@@ -480,7 +480,9 @@
           end
         end
 
-        context 'when running inside the passenger web server' do
+        context 'when running inside the passenger web server, even when gem is not available' do
+          include_context('loaded gems', passenger: nil, rugged: nil, mysql2: nil)
+
           before do
             stub_const('::PhusionPassenger', Module.new)
             allow(Datadog.logger).to receive(:warn)
@@ -495,8 +497,30 @@
           end
         end
 
+        context 'when passenger gem is available' do
+          context 'on passenger >= 6.0.19' do
+            include_context('loaded gems', passenger: Gem::Version.new('6.0.19'), rugged: nil, mysql2: nil)
+
+            it { is_expected.to be false }
+          end
+
+          context 'on passenger < 6.0.19' do
+            include_context('loaded gems', passenger: Gem::Version.new('6.0.18'), rugged: nil, mysql2: nil)
+
+            before { allow(Datadog.logger).to receive(:warn) }
+
+            it { is_expected.to be true }
+
+            it 'logs a warning message mentioning that the no signals workaround is going to be used' do
+              expect(Datadog.logger).to receive(:warn).with(/Enabling the profiling "no signals" workaround/)
+
+              no_signals_workaround_enabled?
+            end
+          end
+        end
+
         context 'when mysql2 / rugged gems + passenger are not available' do
-          include_context('loaded gems', mysql2: nil, rugged: nil)
+          include_context('loaded gems', passenger: nil, mysql2: nil, rugged: nil)
 
           it { is_expected.to be false }
         end