Add latest changes from gitlab-org/gitlab@15-7-stable-eev15.7.0-rc42

5 files changed, 67 insertions, 21 deletions

diff --git a/lib/gitlab/instrumentation/redis.rb b/lib/gitlab/instrumentation/redis.rb
index a371930621d..a664656c467 100644
--- a/lib/gitlab/instrumentation/redis.rb
+++ b/lib/gitlab/instrumentation/redis.rb
@@ -39,7 +39,8 @@ module Gitlab
           end
         end
 
-        %i[get_request_count query_time read_bytes write_bytes].each do |method|
+        %i[get_request_count get_cross_slot_request_count get_allowed_cross_slot_request_count query_time read_bytes
+           write_bytes].each do |method|
           define_method method do
             STORAGES.sum(&method)
           end
diff --git a/lib/gitlab/instrumentation/redis_base.rb b/lib/gitlab/instrumentation/redis_base.rb
index 268c6cdf459..de24132a28e 100644
--- a/lib/gitlab/instrumentation/redis_base.rb
+++ b/lib/gitlab/instrumentation/redis_base.rb
@@ -45,6 +45,16 @@ module Gitlab
           ::RequestStore[write_bytes_key] += num_bytes
         end
 
+        def increment_cross_slot_request_count(amount = 1)
+          ::RequestStore[cross_slots_key] ||= 0
+          ::RequestStore[cross_slots_key] += amount
+        end
+
+        def increment_allowed_cross_slot_request_count(amount = 1)
+          ::RequestStore[allowed_cross_slots_key] ||= 0
+          ::RequestStore[allowed_cross_slots_key] += amount
+        end
+
         def get_request_count
           ::RequestStore[request_count_key] || 0
         end
@@ -61,13 +71,32 @@ module Gitlab
           ::RequestStore[call_details_key] ||= []
         end
 
+        def get_cross_slot_request_count
+          ::RequestStore[cross_slots_key] || 0
+        end
+
+        def get_allowed_cross_slot_request_count
+          ::RequestStore[allowed_cross_slots_key] || 0
+        end
+
         def query_time
           query_time = ::RequestStore[call_duration_key] || 0
           query_time.round(::Gitlab::InstrumentationHelper::DURATION_PRECISION)
         end
 
         def redis_cluster_validate!(commands)
-          ::Gitlab::Instrumentation::RedisClusterValidator.validate!(commands) if @redis_cluster_validation
+          return true unless @redis_cluster_validation
+
+          result = ::Gitlab::Instrumentation::RedisClusterValidator.validate(commands)
+          return true if result.nil?
+
+          if !result[:valid] && !result[:allowed] && (Rails.env.development? || Rails.env.test?)
+            raise RedisClusterValidator::CrossSlotError, "Redis command #{result[:command_name]} arguments hash to different slots. See https://docs.gitlab.com/ee/development/redis.html#multi-key-commands"
+          end
+
+          increment_allowed_cross_slot_request_count if result[:allowed]
+
+          result[:valid]
         end
 
         def enable_redis_cluster_validation
@@ -122,6 +151,14 @@ module Gitlab
           strong_memoize(:call_details_key) { build_key(:redis_call_details) }
         end
 
+        def cross_slots_key
+          strong_memoize(:cross_slots_key) { build_key(:redis_cross_slot_request_count) }
+        end
+
+        def allowed_cross_slots_key
+          strong_memoize(:allowed_cross_slots_key) { build_key(:redis_allowed_cross_slot_request_count) }
+        end
+
         def build_key(namespace)
           "#{storage_key}_#{namespace}"
         end
diff --git a/lib/gitlab/instrumentation/redis_cluster_validator.rb b/lib/gitlab/instrumentation/redis_cluster_validator.rb
index 36d3e088956..1567e54d8da 100644
--- a/lib/gitlab/instrumentation/redis_cluster_validator.rb
+++ b/lib/gitlab/instrumentation/redis_cluster_validator.rb
@@ -183,19 +183,22 @@ module Gitlab
       CrossSlotError = Class.new(StandardError)
 
       class << self
-        def validate!(commands)
-          return unless Rails.env.development? || Rails.env.test?
-          return if allow_cross_slot_commands?
+        def validate(commands)
           return if commands.empty?
 
           # early exit for single-command (non-pipelined) if it is a single-key-command
           command_name = commands.size > 1 ? "PIPELINE/MULTI" : commands.first.first.to_s.upcase
           return if commands.size == 1 && REDIS_COMMANDS.dig(command_name, :single_key)
 
-          key_slots = commands.map { |command| key_slots(command) }.flatten
-          if key_slots.uniq.many? # rubocop: disable CodeReuse/ActiveRecord
-            raise CrossSlotError, "Redis command #{command_name} arguments hash to different slots. See https://docs.gitlab.com/ee/development/redis.html#multi-key-commands"
-          end
+          keys = commands.map { |command| extract_keys(command) }.flatten
+
+          {
+            # calculate key-slots only if not allowed
+            valid: allow_cross_slot_commands? || !has_cross_slot_keys?(keys),
+            command_name: command_name,
+            key_count: keys.size,
+            allowed: allow_cross_slot_commands?
+          }
         end
 
         # Keep track of the call stack to allow nested calls to work.
@@ -210,15 +213,17 @@ module Gitlab
 
         private
 
-        def key_slots(command)
+        def extract_keys(command)
           argument_positions = REDIS_COMMANDS[command.first.to_s.upcase]
 
           return [] unless argument_positions
 
           arguments = command.flatten[argument_positions[:first]..argument_positions[:last]]
-          arguments.each_slice(argument_positions[:step]).map do |args|
-            key_slot(args.first)
-          end
+          arguments.each_slice(argument_positions[:step]).map(&:first)
+        end
+
+        def has_cross_slot_keys?(keys)
+          keys.map { |key| key_slot(key) }.uniq.many? # rubocop: disable CodeReuse/ActiveRecord
         end
 
         def allow_cross_slot_commands?
diff --git a/lib/gitlab/instrumentation/redis_interceptor.rb b/lib/gitlab/instrumentation/redis_interceptor.rb
index f19279df2fe..35dd7cbfeb8 100644
--- a/lib/gitlab/instrumentation/redis_interceptor.rb
+++ b/lib/gitlab/instrumentation/redis_interceptor.rb
@@ -33,7 +33,10 @@ module Gitlab
       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)
-        instrumentation_class.redis_cluster_validate!(commands)
+
+        if !instrumentation_class.redis_cluster_validate!(commands) && ::RequestStore.active?
+          instrumentation_class.increment_cross_slot_request_count
+        end
 
         yield
       rescue ::Redis::BaseError => ex
@@ -62,13 +65,11 @@ module Gitlab
         # This count is an approximation that omits the Redis protocol overhead
         # of type prefixes, length prefixes and line endings.
         command.each do |x|
-          size += begin
-            if x.is_a? Array
-              x.inject(0) { |sum, y| sum + y.to_s.bytesize }
-            else
-              x.to_s.bytesize
-            end
-          end
+          size += if x.is_a? Array
+                    x.inject(0) { |sum, y| sum + y.to_s.bytesize }
+                  else
+                    x.to_s.bytesize
+                  end
         end
 
         instrumentation_class.increment_write_bytes(size)
diff --git a/lib/gitlab/instrumentation/redis_payload.rb b/lib/gitlab/instrumentation/redis_payload.rb
index 86a6525c8d0..62a4d1a846f 100644
--- a/lib/gitlab/instrumentation/redis_payload.rb
+++ b/lib/gitlab/instrumentation/redis_payload.rb
@@ -20,6 +20,8 @@ module Gitlab
 
           {
             "#{key_prefix}_calls": -> { get_request_count },
+            "#{key_prefix}_cross_slot_calls": -> { get_cross_slot_request_count },
+            "#{key_prefix}_allowed_cross_slot_calls": -> { get_allowed_cross_slot_request_count },
             "#{key_prefix}_duration_s": -> { query_time },
             "#{key_prefix}_read_bytes": -> { read_bytes },
             "#{key_prefix}_write_bytes": -> { write_bytes }