Merge branch 'jv-count-redis-exceptions' into 'master'

Count Redis errors and requests per Redis server

See merge request gitlab-org/gitlab!35992

doc/administration/monitoring/prometheus/gitlab_metrics.md

@@ -251,6 +251,19 @@ When Puma is used instead of Unicorn, the following metrics are available:
 | `puma_idle_threads`               | Gauge   | 12.0  | Number of spawned threads which are not processing a request |
 | `puma_killer_terminations_total`  | Gauge   | 12.0  | Number of workers terminated by PumaWorkerKiller |
 
+## Redis metrics
+
+These client metrics are meant to complement Redis server metrics.
+These metrics are broken down per [Redis
+instance](https://docs.gitlab.com/omnibus/settings/redis.html#running-with-multiple-redis-instances).
+These metrics all have a `storage` label which indicates the Redis
+instance (`cache`, `shared_state` etc.).
+
+| Metric                            | Type    | Since | Description |
+|:--------------------------------- |:------- |:----- |:----------- |
+| `redis_client_exceptions_total`                    | Counter   | 13.2  | Number of Redis client exceptions, broken down by exception class |
+| `redis_client_requests_total`                    | Counter   | 13.2  | Number of Redis client requests |
+
 ## Metrics shared directory
 
 GitLab's Prometheus client requires a directory to store metrics data shared between multi-process services.

lib/gitlab/instrumentation/redis_base.rb

@@ -81,6 +81,19 @@ module Gitlab
           self
         end
 
+        def count_request
+          @request_counter ||= Gitlab::Metrics.counter(:redis_client_requests_total, 'Client side Redis request count, per Redis server')
+          @request_counter.increment({ storage: storage_key })
+        end
+
+        def 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(: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
+
         private
 
         def request_count_key

lib/gitlab/instrumentation/redis_interceptor.rb

@@ -6,11 +6,14 @@ module Gitlab
   module Instrumentation
     module RedisInterceptor
       def call(*args, &block)
-        start = Time.now
-
+        instrumentation_class.count_request
         instrumentation_class.redis_cluster_validate!(args.first)
+        start = Time.now
 
         super(*args, &block)
+      rescue ::Redis::BaseError => ex
+        instrumentation_class.count_exception(ex)
+        raise ex
       ensure
         duration = (Time.now - start)
 

spec/lib/gitlab/instrumentation/redis_interceptor_spec.rb

@@ -42,4 +42,26 @@ RSpec.describe Gitlab::Instrumentation::RedisInterceptor, :clean_gitlab_redis_sh
       end
     end
   end
+
+  describe 'counting' do
+    let(:instrumentation_class) { Gitlab::Redis::SharedState.instrumentation_class }
+
+    it 'counts successful requests' do
+      expect(instrumentation_class).to receive(:count_request).and_call_original
+
+      Gitlab::Redis::SharedState.with { |redis| redis.call(:get, 'foobar') }
+    end
+
+    it 'counts exceptions' do
+      expect(instrumentation_class).to receive(:count_exception)
+        .with(instance_of(Redis::CommandError)).and_call_original
+      expect(instrumentation_class).to receive(:count_request).and_call_original
+
+      expect do
+        Gitlab::Redis::SharedState.with do |redis|
+          redis.call(:auth, 'foo', 'bar')
+        end
+      end.to raise_exception(Redis::CommandError)
+    end
+  end
 end