Redis instrumentation: support older redis-client

.rubocop.yml

@@ -1341,6 +1341,7 @@ Style/MethodCallWithArgsParentheses:
     - skip
     - source
     - stub
+    - stub_const
   AllowedPatterns: [^assert, ^refute]
 
 Style/MethodCallWithoutArgsParentheses:

lib/new_relic/agent/instrumentation/redis/instrumentation.rb

@@ -27,10 +27,17 @@ def call_pipeline_with_tracing(pipeline)
 
     # Used for Redis 5.x+
     def call_pipelined_with_tracing(pipeline)
+      db = begin
+        _nr_redis_client_config.db
+      rescue StandardError => e
+        NewRelic::Agent.logger.error("Failed to determine configured Redis db value: #{e.class} - #{e.message}")
+        return yield
+      end
+
       operation = pipeline.flatten.include?('MULTI') ? Constants::MULTI_OPERATION : Constants::PIPELINE_OPERATION
       statement = ::NewRelic::Agent::Datastores::Redis.format_pipeline_commands(pipeline)
 
-      with_tracing(operation, statement: statement, database: client.config.db) { yield }
+      with_tracing(operation, statement: statement, database: db) { yield }
     end
 
     private
@@ -52,21 +59,35 @@ def with_tracing(operation, statement: nil, database: nil)
     end
 
     def _nr_hostname
-      _nr_client.path ? Constants::LOCALHOST : _nr_client.host
+      _nr_redis_client_config.path ? Constants::LOCALHOST : _nr_redis_client_config.host
     rescue => e
-      NewRelic::Agent.logger.debug("Failed to retrieve Redis host: #{e}")
+      NewRelic::Agent.logger.debug("Failed to retrieve Redis host: #{e.class} - #{e.message}")
       Constants::UNKNOWN
     end
 
     def _nr_port_path_or_id
-      _nr_client.path || _nr_client.port
+      _nr_redis_client_config.path || _nr_redis_client_config.port
     rescue => e
-      NewRelic::Agent.logger.debug("Failed to retrieve Redis port_path_or_id: #{e}")
+      NewRelic::Agent.logger.debug("Failed to retrieve Redis port_path_or_id: #{e.class} - #{e.message}")
       Constants::UNKNOWN
     end
 
-    def _nr_client
-      @nr_client ||= self.is_a?(::Redis::Client) ? self : client.config
+    def _nr_redis_client_config
+      @nr_config ||= begin
+        # redis gem
+        config = if defined?(::Redis::Client) && self.is_a?(::Redis::Client)
+          self
+        # redis-client gem v0.11+ (self is a RedisClient::Middlewares)
+        elsif respond_to?(:client)
+          client && client.config
+        # redis-client gem <0.11 (self is a RedisClient::Middlewares)
+        elsif defined?(::RedisClient)
+          ::RedisClient.config if ::RedisClient.respond_to?(:config)
+        end
+        raise 'Unable to locate the underlying Redis client configuration.' unless config
+
+        config
+      end
     end
   end
 end

test/multiverse/suites/redis/redis_instrumentation_test.rb

@@ -4,12 +4,17 @@
 
 require 'redis'
 require_relative '../../../helpers/docker'
+require_relative '../../../helpers/misc'
 
 if NewRelic::Agent::Datastores::Redis.is_supported_version?
   class NewRelic::Agent::Instrumentation::RedisInstrumentationTest < Minitest::Test
     include MultiverseHelpers
     setup_and_teardown_agent
 
+    class FakeClient
+      include ::NewRelic::Agent::Instrumentation::Redis
+    end
+
     def after_setup
       super
       # Default timeout is 5 secs; a flushall takes longer on a busy box (i.e. CI)
@@ -388,6 +393,67 @@ def test_instrumentation_returns_expected_values
       assert_equal 2, @redis.del('foo', 'baz')
     end
 
+    def test__nr_redis_client_config_with_redis
+      skip_unless_minitest5_or_above
+
+      client = FakeClient.new
+
+      client.stub :is_a?, true, [::Redis::Client] do
+        assert_equal client, client.send(:_nr_redis_client_config)
+      end
+    end
+
+    def test__nr_redis_client_config_with_redis_client_v0_11
+      skip_unless_minitest5_or_above
+
+      client = FakeClient.new
+      config = 'the config'
+      mock_client = MiniTest::Mock.new
+      mock_client.expect :config, config
+      client.stub :respond_to?, true, [:client] do
+        client.stub :client, mock_client do
+          assert_equal config, client.send(:_nr_redis_client_config)
+        end
+      end
+    end
+
+    def test__nr_redis_client_config_with_redis_client_below_v0_11
+      skip_unless_minitest5_or_above
+
+      client = FakeClient.new
+      config = 'the config'
+      mock_client = MiniTest::Mock.new
+      mock_client.expect :config, config
+
+      Object.stub_const :RedisClient, mock_client do
+        assert_equal config, client.send(:_nr_redis_client_config)
+      end
+    end
+
+    def test__nr_redis_client_config_with_some_unknown_context
+      skip_unless_minitest5_or_above
+
+      client = FakeClient.new
+
+      Object.stub_const :RedisClient, nil do
+        assert_raises StandardError do
+          client.send(:_nr_redis_client_config)
+        end
+      end
+    end
+
+    def test_call_pipelined_with_tracing_yields_early_if_a_db_value_cannot_be_obtained
+      client = FakeClient.new
+      yield_value = 'Vegemite'
+
+      Object.stub_const :RedisClient, nil do
+        # if with_tracing were to be reached, an exception will raise
+        client.stub :with_tracing, -> { raise 'kaboom' } do
+          assert_equal yield_value, client.call_pipelined_with_tracing(nil) { yield_value }
+        end
+      end
+    end
+
     def client
       if Gem::Version.new(Redis::VERSION).segments[0] < 4
         :client