DataDog / dd-trace-rb

Datadog Tracing Ruby Client
https://docs.datadoghq.com/tracing/
Other
299 stars 368 forks source link

"Incompatible version of the passenger gem" even when Passenger has the right version #3721

Closed 23tux closed 1 week ago

23tux commented 2 weeks ago

Current behaviour We have passenger 6.0.22 installed in our Dockerfile on Ruby 3.3.3-bullseye via

apt-get install -y nginx libnginx-mod-http-passenger

We don't have the passenger gem itself in our Gemfile. The PhusionPassenger constant get's injected by the nginx module.

At startup ddtrace complains with this warning:

WARN -- ddtrace: [ddtrace] 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.

Expected behaviour ddtrace should look at the PhusionPassenger::VERSION_STRING constant instead of Gem.loaded_specs['passenger'].version:

https://github.com/DataDog/dd-trace-rb/blob/75278d628143c3063904002e72e3af5fa0574dbe/lib/datadog/profiling/component.rb#L399-L405

So maybe these lines could be

      private_class_method def self.incompatible_passenger_version?
        if defined?(PhusionPassenger)
          PhusionPassenger::VERSION_STRING < Gem::Version.new('6.0.19')
        else
          true
        end
      end

Steps to reproduce

Environment

ivoanjo commented 2 weeks ago

Hey! Thanks for getting in touch. Will definitely fix :)

In the meanwhile, it's possible to use DD_PROFILING_NO_SIGNALS_WORKAROUND_ENABLED=false or via code:

Datadog.configure do |c|
  c.profiling.advanced.no_signals_workaround_enabled = false
end

To override the incorrect check!

23tux commented 1 week ago

Thanks for the quick response, I'll try disable the config and let you know if it worked!

ivoanjo commented 1 week ago

Oops! I've updated my comment above -- I had a bit too much copy pasta in the code example and I had set it to = true instead of = false. (So make sure to actually set it to false and not true)

23tux commented 1 week ago

too late 😅 Gonna switch it to false. This double negation stuff is always confusing

23tux commented 1 week ago

I changed it to false, but I still get this warning

WARN -- ddtrace: [ddtrace] Profiling "no signals" workaround disabled via configuration

I'm not sure what to do here, do you have an idea?

ivoanjo commented 1 week ago

I'm not sure what to do here, do you have an idea?

That warning confirms that it was disabled successfully, so you're getting the full data quality from the profiler, but I guess the aim was also to get rid of the warning, and I kinda just directed you to replace one warning with the other :sweat_smile:.

For now you'll have to live with this warning -- I'll open the PR with the fix ASAP so you'll be able to remove the manual disable and have no warnings :)

23tux commented 1 week ago

Yes, getting rid of the warning would be nice. But anyway, thanks for the quick response, and the main goal is to get full data quality. Then I'll have an eye on the releases and update once the PR is merged. Thanks again!