Merge pull request newrelic#2906 from newrelic/acinarius

Create helper to access Bundler.rubygems specs

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## dev
 
-Version <dev> updates View Componment instrumentation to use a default metric name when one is unavaliable and resolves a bug in rdkafka instrumentation when using the karafka-rdkafka gem.  
+Version <dev> updates View Componment instrumentation to use a default metric name when one is unavaliable, resolves a bug in rdkafka instrumentation when using the karafka-rdkafka gem and fixes a bug with Grape instrumentation.
 
 - **Feature: Use default `View/component` metric name for unidentified View Components**
 
@@ -12,6 +12,9 @@ Version <dev> updates View Componment instrumentation to use a default metric na
 
   Due to version differences between the rdkafka gem and karafka-rdkafka gem, the agent could encounter an error when it tried to install rdkafka instrumentation. This has now been resolved. Thank you to @krisdigital for bringing this issue to our attention. [PR#2880](https://github.com/newrelic/newrelic-ruby-agent/pull/2880)
 
+- **Bugfix: Stop calling deprecated all_specs method to check for the presence of newrelic-grape**
+
+  In 9.14.0, we released a fix for calls to the deprecated `Bundler.rubygems.all_specs`, but the fix fell short for the agent's Grape instrumentation and deprecation warnings could still be raised. The condition has been simplified and deprecation warnings should no longer be raised. Thank you, [@excelsior](https://github.com/excelsior) for bringing this to our attention. [Issue#](https://github.com/newrelic/newrelic-ruby-agent/issues/2885) [PR#2906](https://github.com/newrelic/newrelic-ruby-agent/pull/2906)
 
 ## v9.14.0
 

lib/new_relic/agent/instrumentation/grape.rb

@@ -19,9 +19,7 @@
 
   depends_on do
     begin
-      if defined?(Bundler) &&
-          ((Bundler.rubygems.respond_to?(:installed_specs) && Bundler.rubygems.installed_specs.map(&:name).include?('newrelic-grape')) ||
-            Bundler.rubygems.all_specs.map(&:name).include?('newrelic-grape'))
+      if NewRelic::Helper.rubygems_specs.map(&:name).include?('newrelic-grape')
         NewRelic::Agent.logger.info('Not installing New Relic supported Grape instrumentation because the third party newrelic-grape gem is present')
         false
       else

lib/new_relic/agent/instrumentation/ruby_kafka/prepend.rb

@@ -20,10 +20,20 @@ module RubyKafkaConsumer
     module Prepend
       include NewRelic::Agent::Instrumentation::RubyKafka
 
-      def each_message(*args)
-        super do |message|
-          each_message_with_new_relic(message) do
-            yield(message)
+      if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3')
+        def each_message(**args)
+          super do |message|
+            each_message_with_new_relic(message) do
+              yield(message)
+            end
+          end
+        end
+      else
+        def each_message(*args)
+          super do |message|
+            each_message_with_new_relic(message) do
+              yield(message)
+            end
           end
         end
       end

lib/new_relic/control/frameworks/rails4.rb

@@ -9,11 +9,7 @@ class Control
     module Frameworks
       class Rails4 < NewRelic::Control::Frameworks::Rails3
         def rails_gem_list
-          if Bundler.rubygems.respond_to?(:installed_specs)
-            Bundler.rubygems.installed_specs.map { |gem| "#{gem.name} (#{gem.version})" }
-          else
-            Bundler.rubygems.all_specs.map { |gem| "#{gem.name} (#{gem.version})" }
-          end
+          NewRelic::Helper.rubygems_specs.map { |gem| "#{gem.name} (#{gem.version})" }
         end
 
         def append_plugin_list

lib/new_relic/environment_report.rb

@@ -44,11 +44,7 @@ def self.registered_reporters=(logic)
     ####################################
     report_on('Gems') do
       begin
-        if Bundler.rubygems.respond_to?(:installed_specs)
-          Bundler.rubygems.installed_specs.map { |gem| "#{gem.name}(#{gem.version})" }
-        else
-          Bundler.rubygems.all_specs.map { |gem| "#{gem.name}(#{gem.version})" }
-        end
+        NewRelic::Helper.rubygems_specs.map { |gem| "#{gem.name}(#{gem.version})" }
       rescue
         # There are certain rubygem, bundler, rails combinations (e.g. gem
         # 1.6.2, rails 2.3, bundler 1.2.3) where the code above throws an error

lib/new_relic/helper.rb

@@ -82,5 +82,20 @@ def executable_in_path?(executable)
         File.exist?(executable_path) && File.file?(executable_path) && File.executable?(executable_path)
       end
     end
+
+    # Bundler version 2.5.12 deprecated all_specs and added installed_specs.
+    # To support newer Bundler versions, try to use installed_specs first,
+    # then fall back to all_specs.
+    # All callers expect this to be an array, so return an array if Bundler isn't defined
+    # @api private
+    def rubygems_specs
+      return [] unless defined?(Bundler)
+
+      if Bundler.rubygems.respond_to?(:installed_specs)
+        Bundler.rubygems.installed_specs
+      else
+        Bundler.rubygems.all_specs
+      end
+    end
   end
 end

lib/new_relic/language_support.rb

@@ -90,11 +90,7 @@ def snakeize(string)
     def bundled_gem?(gem_name)
       return false unless defined?(Bundler)
 
-      if Bundler.rubygems.respond_to?(:installed_specs)
-        Bundler.rubygems.installed_specs.map(&:name).include?(gem_name)
-      else
-        Bundler.rubygems.all_specs.map(&:name).include?(gem_name)
-      end
+      NewRelic::Helper.rubygems_specs.map(&:name).include?(gem_name)
     rescue => e
       ::NewRelic::Agent.logger.info("Could not determine if third party #{gem_name} gem is installed", e)
       false

test/new_relic/helper_test.rb

@@ -77,4 +77,28 @@ def test_run_command_sad_exception
       NewRelic::Helper.run_command('executable that existed at detection time but is not there now')
     end
   end
+
+  #
+  # rubygems_specs
+  #
+  def test_rubygems_specs_returns_empty_array_without_bundler
+    stub(:defined?, nil, ['Bundler']) do
+      result = NewRelic::Helper.rubygems_specs
+
+      assert_instance_of Array, result
+      assert_empty Array, result
+    end
+  end
+
+  def test_rubygems_specs_works_with_all_specs_when_installed_specs_is_absent
+    Bundler.rubygems.stub(:respond_to?, nil) do
+      assert_equal Bundler.rubygems.all_specs, NewRelic::Helper.rubygems_specs
+    end
+  end
+
+  def test_rubygems_specs_works_with_installed_specs
+    skip 'running a version of Bundler that has not defined installed_specs' unless Bundler.rubygems.respond_to?(:installed_specs)
+
+    assert_equal Bundler.rubygems.installed_specs, NewRelic::Helper.rubygems_specs
+  end
 end