Merge branch 'dev' into add_aws_account_id_config

CHANGELOG.md

@@ -2,9 +2,9 @@
 
 ## dev
 
-
-Version <dev> adds a configuration option to associate the AWS account ID with the DynamoDB calls from the AWS SDK, 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 unavailable, adds a configuration option to associate the AWS account ID with the DynamoDB calls from the AWS SDK, resolves a bug in rdkafka instrumentation when using the karafka-rdkafka gem, resolves a bug in the ruby-kafka instrumentation, and fixes a bug with Grape instrumentation.
+  
+  
 - **Feature: New configuration option cloud.aws.account_id**
 
   A new configuration option has been added, `cloud.aws.account_id`, that will allow New Relic to provide more details about certain calls made using the AWS SDK. Currently, the DynamoDB instrumentation is the only instrumentation that will make use of this configuration option, but this will be used in future instrumentation as well. [PR#2904](https://github.com/newrelic/newrelic-ruby-agent/pull/2904)
@@ -17,6 +17,14 @@ Version <dev> adds a configuration option to associate the AWS account ID with t
 
   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)
+
+- **Bugfix: Instrumentation errors when using the ruby-kafka gem**
+
+  Kafka::Consumer#each_message takes keyword arguments, while the prepended method is defined with a single splat positional argument. In Ruby >= 3.0, this signature mismatch raises an ArgumentError. Thank you [@patrickarnett](https://github.com/patrickarnett) for providing this bugfix. [PR#2915](https://github.com/newrelic/newrelic-ruby-agent/pull/2915)
+
 
 ## 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/dependency_detection.rb

@@ -25,11 +25,9 @@ def defer(&block)
 
   def detect!
     @items.each do |item|
-      if item.dependencies_satisfied?
-        item.execute
-      else
-        item.configure_as_unsatisfied unless item.disabled_configured?
-      end
+      next if item.executed || item.disabled_configured?
+
+      item.dependencies_satisfied? ? item.execute : item.configure_as_unsatisfied
     end
   end
 
@@ -65,6 +63,13 @@ def dependencies_satisfied?
     end
 
     def configure_as_unsatisfied
+      # TODO: currently using :unsatisfied for Padrino will clobber the value
+      #       already set for Sinatra, so skip Padrino and circle back with a
+      #       new Padrino specific solution in the future.
+      #
+      #       https://github.com/newrelic/newrelic-ruby-agent/issues/2912
+      return if name == :padrino
+
       NewRelic::Agent.config.instance_variable_get(:@cache)[config_key] = :unsatisfied
     end
 

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/multiverse/suites/sinatra/sinatra_test_cases.rb

@@ -128,6 +128,13 @@ def test_with_regex_pattern
     assert_metrics_recorded(["Controller/Sinatra/#{app_name}/#{regex_segment}"])
   end
 
+  def test_that_the_actively_configured_instrumentation_is_not_marked_as_unsatsfied
+    get('/pass')
+
+    assert_equal 200, last_response.status
+    assert_includes(%w[chain prepend], NewRelic::Agent.config[:'instrumentation.sinatra'].to_s)
+  end
+
   # https://support.newrelic.com/tickets/31061
   def test_precondition_not_over_called
     get('/precondition')

test/new_relic/agent/configuration/manager_test.rb

@@ -492,20 +492,22 @@ def test_apply_transformations_reraises_errors
     end
 
     def test_auto_determined_values_stay_cached
-      name = :knockbreck_manse
+      with_config(:'security.agent.enabled' => true) do
+        name = :knockbreck_manse
 
-      DependencyDetection.defer do
-        named(name)
-        executes { use_prepend? }
-      end
+        DependencyDetection.defer do
+          named(name)
+          executes { use_prepend? }
+        end
 
-      key = :"instrumentation.#{name}"
-      with_config(key => 'auto') do
-        DependencyDetection.detect!
+        key = :"instrumentation.#{name}"
+        with_config(key => 'auto') do
+          DependencyDetection.detect!
 
-        @manager.replace_or_add_config(ServerSource.new({}))
+          @manager.replace_or_add_config(ServerSource.new({}))
 
-        assert_equal :prepend, @manager.instance_variable_get(:@cache)[key]
+          assert_equal :prepend, @manager.instance_variable_get(:@cache)[key]
+        end
       end
     end
 

test/new_relic/dependency_detection_test.rb

@@ -488,4 +488,21 @@ def test_prepend_is_replaced_by_unsatisfied_when_appropriate
       assert_equal :unsatisfied, dd.config_value
     end
   end
+
+  def test_already_executed_items_are_not_executed_again
+    unexecuted = Minitest::Mock.new
+    unexecuted.expect :executed, false
+    unexecuted.expect :dependencies_satisfied?, true
+    unexecuted.expect :disabled_configured?, false
+    unexecuted.expect :execute, -> { execution_took_place = true }
+    executed = Minitest::Mock.new
+    executed.expect :executed, true
+    unexecuted.expect :disabled_configured?, false
+
+    DependencyDetection.instance_variable_set(:@items, [unexecuted, executed])
+    DependencyDetection.detect!
+
+    unexecuted.verify
+    executed.verify
+  end
 end

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