Add latest changes from gitlab-org/gitlab@13-2-stable-ee

5 files changed, 172 insertions, 14 deletions

diff --git a/lib/gitlab/instrumentation/elasticsearch_transport.rb b/lib/gitlab/instrumentation/elasticsearch_transport.rb
index deee0127c0c..56179eda22d 100644
--- a/lib/gitlab/instrumentation/elasticsearch_transport.rb
+++ b/lib/gitlab/instrumentation/elasticsearch_transport.rb
@@ -5,8 +5,10 @@ require 'elasticsearch-transport'
 module Gitlab
   module Instrumentation
     module ElasticsearchTransportInterceptor
-      def perform_request(*args)
+      def perform_request(method, path, params = {}, body = nil, headers = nil)
         start = Time.now
+        headers = (headers || {})
+          .reverse_merge({ 'X-Opaque-Id': Labkit::Correlation::CorrelationId.current_or_new_id })
         super
       ensure
         if ::Gitlab::SafeRequestStore.active?
@@ -14,7 +16,7 @@ module Gitlab
 
           ::Gitlab::Instrumentation::ElasticsearchTransport.increment_request_count
           ::Gitlab::Instrumentation::ElasticsearchTransport.add_duration(duration)
-          ::Gitlab::Instrumentation::ElasticsearchTransport.add_call_details(duration, args)
+          ::Gitlab::Instrumentation::ElasticsearchTransport.add_call_details(duration, method, path, params, body)
         end
       end
     end
@@ -47,14 +49,14 @@ module Gitlab
         ::Gitlab::SafeRequestStore[ELASTICSEARCH_CALL_DURATION] += duration
       end
 
-      def self.add_call_details(duration, args)
+      def self.add_call_details(duration, method, path, params, body)
         return unless Gitlab::PerformanceBar.enabled_for_request?
 
         detail_store << {
-          method: args[0],
-          path: args[1],
-          params: args[2],
-          body: args[3],
+          method: method,
+          path: path,
+          params: params,
+          body: body,
           duration: duration,
           backtrace: ::Gitlab::BacktraceCleaner.clean_backtrace(caller)
         }
diff --git a/lib/gitlab/instrumentation/redis.rb b/lib/gitlab/instrumentation/redis.rb
index 82b4701872f..4a85a313fd7 100644
--- a/lib/gitlab/instrumentation/redis.rb
+++ b/lib/gitlab/instrumentation/redis.rb
@@ -5,9 +5,9 @@ module Gitlab
     # Aggregates Redis measurements from different request storage sources.
     class Redis
       ActionCable = Class.new(RedisBase)
-      Cache = Class.new(RedisBase)
+      Cache = Class.new(RedisBase).enable_redis_cluster_validation
       Queues = Class.new(RedisBase)
-      SharedState = Class.new(RedisBase)
+      SharedState = Class.new(RedisBase).enable_redis_cluster_validation
 
       STORAGES = [ActionCable, Cache, Queues, SharedState].freeze
 
diff --git a/lib/gitlab/instrumentation/redis_base.rb b/lib/gitlab/instrumentation/redis_base.rb
index 012543e1645..1df899747e0 100644
--- a/lib/gitlab/instrumentation/redis_base.rb
+++ b/lib/gitlab/instrumentation/redis_base.rb
@@ -25,9 +25,6 @@ module Gitlab
           # redis-rb passes an array (e.g. [[:get, key]])
           return unless args.length == 1
 
-          # TODO: Add information about current Redis client
-          # being instrumented.
-          # https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/316.
           detail_store << {
             cmd: args.first,
             duration: duration,
@@ -71,6 +68,40 @@ module Gitlab
           query_time.round(::Gitlab::InstrumentationHelper::DURATION_PRECISION)
         end
 
+        def redis_cluster_validate!(command)
+          ::Gitlab::Instrumentation::RedisClusterValidator.validate!(command) if @redis_cluster_validation
+        end
+
+        def enable_redis_cluster_validation
+          @redis_cluster_validation = true
+
+          self
+        end
+
+        def instance_count_request
+          @request_counter ||= Gitlab::Metrics.counter(:gitlab_redis_client_requests_total, 'Client side Redis request count, per Redis server')
+          @request_counter.increment({ storage: storage_key })
+        end
+
+        def instance_count_exception(ex)
+          # This metric is meant to give a client side view of how the Redis
+          # server is doing. Redis itself does not expose error counts. This
+          # metric can be used for Redis alerting and service health monitoring.
+          @exception_counter ||= Gitlab::Metrics.counter(:gitlab_redis_client_exceptions_total, 'Client side Redis exception count, per Redis server, per exception class')
+          @exception_counter.increment({ storage: storage_key, exception: ex.class.to_s })
+        end
+
+        def instance_observe_duration(duration)
+          @request_latency_histogram ||= Gitlab::Metrics.histogram(
+            :gitlab_redis_client_requests_duration_seconds,
+            'Client side Redis request latency, per Redis server, excluding blocking commands',
+            {},
+            [0.005, 0.01, 0.1, 0.5]
+          )
+
+          @request_latency_histogram.observe({ storage: storage_key }, duration)
+        end
+
         private
 
         def request_count_key
diff --git a/lib/gitlab/instrumentation/redis_cluster_validator.rb b/lib/gitlab/instrumentation/redis_cluster_validator.rb
new file mode 100644
index 00000000000..6800e5667f6
--- /dev/null
+++ b/lib/gitlab/instrumentation/redis_cluster_validator.rb
@@ -0,0 +1,106 @@
+# frozen_string_literal: true
+
+require 'rails'
+require 'redis'
+
+module Gitlab
+  module Instrumentation
+    module RedisClusterValidator
+      # Generate with:
+      #
+      # Gitlab::Redis::Cache
+      #   .with { |redis| redis.call('COMMAND') }
+      #   .select { |command| command[3] != command[4] }
+      #   .map { |command| [command[0].upcase, { first: command[3], last: command[4], step: command[5] }] }
+      #   .sort_by(&:first)
+      #   .to_h
+      #
+      MULTI_KEY_COMMANDS = {
+        "BITOP" => { first: 2, last: -1, step: 1 },
+        "BLPOP" => { first: 1, last: -2, step: 1 },
+        "BRPOP" => { first: 1, last: -2, step: 1 },
+        "BRPOPLPUSH" => { first: 1, last: 2, step: 1 },
+        "BZPOPMAX" => { first: 1, last: -2, step: 1 },
+        "BZPOPMIN" => { first: 1, last: -2, step: 1 },
+        "DEL" => { first: 1, last: -1, step: 1 },
+        "EXISTS" => { first: 1, last: -1, step: 1 },
+        "MGET" => { first: 1, last: -1, step: 1 },
+        "MSET" => { first: 1, last: -1, step: 2 },
+        "MSETNX" => { first: 1, last: -1, step: 2 },
+        "PFCOUNT" => { first: 1, last: -1, step: 1 },
+        "PFMERGE" => { first: 1, last: -1, step: 1 },
+        "RENAME" => { first: 1, last: 2, step: 1 },
+        "RENAMENX" => { first: 1, last: 2, step: 1 },
+        "RPOPLPUSH" => { first: 1, last: 2, step: 1 },
+        "SDIFF" => { first: 1, last: -1, step: 1 },
+        "SDIFFSTORE" => { first: 1, last: -1, step: 1 },
+        "SINTER" => { first: 1, last: -1, step: 1 },
+        "SINTERSTORE" => { first: 1, last: -1, step: 1 },
+        "SMOVE" => { first: 1, last: 2, step: 1 },
+        "SUNION" => { first: 1, last: -1, step: 1 },
+        "SUNIONSTORE" => { first: 1, last: -1, step: 1 },
+        "UNLINK" => { first: 1, last: -1, step: 1 },
+        "WATCH" => { first: 1, last: -1, step: 1 }
+      }.freeze
+
+      CrossSlotError = Class.new(StandardError)
+
+      class << self
+        def validate!(command)
+          return unless Rails.env.development? || Rails.env.test?
+          return if allow_cross_slot_commands?
+
+          command_name = command.first.to_s.upcase
+          argument_positions = MULTI_KEY_COMMANDS[command_name]
+
+          return unless argument_positions
+
+          arguments = command.flatten[argument_positions[:first]..argument_positions[:last]]
+
+          key_slots = arguments.each_slice(argument_positions[:step]).map do |args|
+            key_slot(args.first)
+          end
+
+          unless key_slots.uniq.length == 1
+            raise CrossSlotError.new("Redis command #{command_name} arguments hash to different slots. See https://docs.gitlab.com/ee/development/redis.html#multi-key-commands")
+          end
+        end
+
+        # Keep track of the call stack to allow nested calls to work.
+        def allow_cross_slot_commands
+          Thread.current[:allow_cross_slot_commands] ||= 0
+          Thread.current[:allow_cross_slot_commands] += 1
+
+          yield
+        ensure
+          Thread.current[:allow_cross_slot_commands] -= 1
+        end
+
+        private
+
+        def allow_cross_slot_commands?
+          Thread.current[:allow_cross_slot_commands].to_i > 0
+        end
+
+        def key_slot(key)
+          ::Redis::Cluster::KeySlotConverter.convert(extract_hash_tag(key))
+        end
+
+        # This is almost identical to Redis::Cluster::Command#extract_hash_tag,
+        # except that it returns the original string if no hash tag is found.
+        #
+        def extract_hash_tag(key)
+          s = key.index('{')
+
+          return key unless s
+
+          e = key.index('}', s + 1)
+
+          return key unless e
+
+          key[s + 1..e - 1]
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/instrumentation/redis_interceptor.rb b/lib/gitlab/instrumentation/redis_interceptor.rb
index a36aade59c3..b5a5f8fd984 100644
--- a/lib/gitlab/instrumentation/redis_interceptor.rb
+++ b/lib/gitlab/instrumentation/redis_interceptor.rb
@@ -5,13 +5,26 @@ require 'redis'
 module Gitlab
   module Instrumentation
     module RedisInterceptor
+      APDEX_EXCLUDE = %w[brpop blpop brpoplpush bzpopmin bzpopmax xread xreadgroup].freeze
+
       def call(*args, &block)
-        start = Time.now
+        start = Time.now # 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 = (Time.now - start)
+        duration = Time.now - start
+
+        unless APDEX_EXCLUDE.include?(command_from_args(args))
+          instrumentation_class.instance_observe_duration(duration)
+        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)
@@ -77,6 +90,12 @@ module Gitlab
       def instrumentation_class
         @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
+      end
     end
   end
 end