3 files changed, 53 insertions, 39 deletions

diff --git a/lib/gitlab/instrumentation/redis.rb b/lib/gitlab/instrumentation/redis.rb
index 4fee779c767..a371930621d 100644
--- a/lib/gitlab/instrumentation/redis.rb
+++ b/lib/gitlab/instrumentation/redis.rb
@@ -4,15 +4,20 @@ module Gitlab
   module Instrumentation
     # Aggregates Redis measurements from different request storage sources.
     class Redis
+      # Actioncable has it's separate instrumentation, but isn't configurable
+      # in the same way as all the other instances using a class.
       ActionCable = Class.new(RedisBase)
-      Cache = Class.new(RedisBase).enable_redis_cluster_validation
-      Queues = Class.new(RedisBase)
-      SharedState = Class.new(RedisBase).enable_redis_cluster_validation
-      TraceChunks = Class.new(RedisBase).enable_redis_cluster_validation
-      RateLimiting = Class.new(RedisBase).enable_redis_cluster_validation
-      Sessions = Class.new(RedisBase).enable_redis_cluster_validation
-
-      STORAGES = [ActionCable, Cache, Queues, SharedState, TraceChunks, RateLimiting, Sessions].freeze
+
+      STORAGES = (
+        Gitlab::Redis::ALL_CLASSES.map do |redis_instance_class|
+          instrumentation_class = Class.new(RedisBase)
+
+          instrumentation_class.enable_redis_cluster_validation unless redis_instance_class == Gitlab::Redis::Queues
+
+          const_set(redis_instance_class.store_name, instrumentation_class)
+          instrumentation_class
+        end << ActionCable
+      ).freeze
 
       # Milliseconds represented in seconds (from 1 millisecond to 2 seconds).
       QUERY_TIME_BUCKETS = [0.001, 0.0025, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2].freeze
diff --git a/lib/gitlab/instrumentation/redis_base.rb b/lib/gitlab/instrumentation/redis_base.rb
index 0beab008f73..0bd10597f24 100644
--- a/lib/gitlab/instrumentation/redis_base.rb
+++ b/lib/gitlab/instrumentation/redis_base.rb
@@ -20,21 +20,19 @@ module Gitlab
           ::RequestStore[call_duration_key] += duration
         end
 
-        def add_call_details(duration, args)
+        def add_call_details(duration, commands)
           return unless Gitlab::PerformanceBar.enabled_for_request?
-          # redis-rb passes an array (e.g. [[:get, key]])
-          return unless args.length == 1
 
           detail_store << {
-            cmd: args.first,
+            commands: commands,
             duration: duration,
             backtrace: ::Gitlab::BacktraceCleaner.clean_backtrace(caller)
           }
         end
 
-        def increment_request_count
+        def increment_request_count(amount = 1)
           ::RequestStore[request_count_key] ||= 0
-          ::RequestStore[request_count_key] += 1
+          ::RequestStore[request_count_key] += amount
         end
 
         def increment_read_bytes(num_bytes)
@@ -78,9 +76,9 @@ module Gitlab
           self
         end
 
-        def instance_count_request
+        def instance_count_request(amount = 1)
           @request_counter ||= Gitlab::Metrics.counter(:gitlab_redis_client_requests_total, 'Client side Redis request count, per Redis server')
-          @request_counter.increment({ storage: storage_key })
+          @request_counter.increment({ storage: storage_key }, amount)
         end
 
         def instance_count_exception(ex)
diff --git a/lib/gitlab/instrumentation/redis_interceptor.rb b/lib/gitlab/instrumentation/redis_interceptor.rb
index 14474693ddf..7e2acb91b94 100644
--- a/lib/gitlab/instrumentation/redis_interceptor.rb
+++ b/lib/gitlab/instrumentation/redis_interceptor.rb
@@ -13,27 +13,15 @@ module Gitlab
         end
       end
 
-      def call(*args, &block)
-        start = Gitlab::Metrics::System.monotonic_time # must come first so that 'start' is always defined
-        instrumentation_class.instance_count_request
-        instrumentation_class.redis_cluster_validate!(args.first)
-
-        super(*args, &block)
-      rescue ::Redis::BaseError => ex
-        instrumentation_class.instance_count_exception(ex)
-        raise ex
-      ensure
-        duration = Gitlab::Metrics::System.monotonic_time - start
-
-        unless APDEX_EXCLUDE.include?(command_from_args(args))
-          instrumentation_class.instance_observe_duration(duration)
+      def call(command)
+        instrument_call([command]) do
+          super
         end
+      end
 
-        if ::RequestStore.active?
-          # These metrics measure total Redis usage per Rails request / job.
-          instrumentation_class.increment_request_count
-          instrumentation_class.add_duration(duration)
-          instrumentation_class.add_call_details(duration, args)
+      def call_pipeline(pipeline)
+        instrument_call(pipeline.commands) do
+          super
         end
       end
 
@@ -50,6 +38,31 @@ module Gitlab
 
       private
 
+      def instrument_call(commands)
+        start = Gitlab::Metrics::System.monotonic_time # must come first so that 'start' is always defined
+        instrumentation_class.instance_count_request(commands.size)
+
+        commands.each { |c| instrumentation_class.redis_cluster_validate!(c) }
+
+        yield
+      rescue ::Redis::BaseError => ex
+        instrumentation_class.instance_count_exception(ex)
+        raise ex
+      ensure
+        duration = Gitlab::Metrics::System.monotonic_time - start
+
+        unless exclude_from_apdex?(commands)
+          commands.each { instrumentation_class.instance_observe_duration(duration / commands.size) }
+        end
+
+        if ::RequestStore.active?
+          # These metrics measure total Redis usage per Rails request / job.
+          instrumentation_class.increment_request_count(commands.size)
+          instrumentation_class.add_duration(duration)
+          instrumentation_class.add_call_details(duration, commands)
+        end
+      end
+
       def measure_write_size(command)
         size = 0
 
@@ -97,10 +110,8 @@ module Gitlab
         @options[:instrumentation_class] # rubocop:disable Gitlab/ModuleWithInstanceVariables
       end
 
-      def command_from_args(args)
-        command = args[0]
-        command = command[0] if command.is_a?(Array)
-        command.to_s.downcase
+      def exclude_from_apdex?(commands)
+        commands.any? { |command| APDEX_EXCLUDE.include?(command.first.to_s.downcase) }
       end
     end
   end