Merge branch 'sc1-log-cache-store-error' into 'master'

Log error in ActiveSupport cache via error_handler

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/111807



Merged-by: George Koltsov <gkoltsov@gitlab.com>
Approved-by: Bob Van Landuyt <bob@gitlab.com>
Approved-by: George Koltsov <gkoltsov@gitlab.com>
Reviewed-by: Bob Van Landuyt <bob@gitlab.com>
Co-authored-by: Sylvester Chin <schin@gitlab.com>

lib/gitlab/redis/cache.rb

@@ -2,6 +2,16 @@
 module Gitlab
   module Redis
+    # Match signature in
+    # https://github.com/rails/rails/blob/v6.1.7.2/activesupport/lib/active_support/cache/redis_cache_store.rb#L59
+    ERROR_HANDLER = ->(method:, returning:, exception:) do
+      Gitlab::ErrorTracking.log_exception(
+        exception,
+        method: method,
+        returning: returning.inspect
+      )
+    end
+
     class Cache < ::Gitlab::Redis::Wrapper
       CACHE_NAMESPACE = 'cache:gitlab'
@@ -12,7 +22,8 @@ def self.active_support_config
           redis: pool,
           compress: Gitlab::Utils.to_boolean(ENV.fetch('ENABLE_REDIS_CACHE_COMPRESSION', '1')),
           namespace: CACHE_NAMESPACE,
-          expires_in: default_ttl_seconds
+          expires_in: default_ttl_seconds,
+          error_handler: ::Gitlab::Redis::ERROR_HANDLER
         }
       end

lib/gitlab/redis/rate_limiting.rb

@@ -10,7 +10,11 @@ def config_fallback
         end
         def cache_store
-          @cache_store ||= ActiveSupport::Cache::RedisCacheStore.new(redis: pool, namespace: Cache::CACHE_NAMESPACE)
+          @cache_store ||= ActiveSupport::Cache::RedisCacheStore.new(
+            redis: pool,
+            namespace: Cache::CACHE_NAMESPACE,
+            error_handler: ::Gitlab::Redis::ERROR_HANDLER
+          )
         end
         private

lib/gitlab/redis/repository_cache.rb

@@ -14,7 +14,8 @@ def cache_store
             redis: pool,
             compress: Gitlab::Utils.to_boolean(ENV.fetch('ENABLE_REDIS_CACHE_COMPRESSION', '1')),
             namespace: Cache::CACHE_NAMESPACE,
-            expires_in: Cache.default_ttl_seconds
+            expires_in: Cache.default_ttl_seconds,
+            error_handler: ::Gitlab::Redis::ERROR_HANDLER
           )
         end
       end

spec/lib/gitlab/redis/cache_spec.rb

@@ -26,5 +26,22 @@
       expect(described_class.active_support_config[:expires_in]).to eq(1.day)
     end
+
+    context 'when encountering an error' do
+      let(:cache) { ActiveSupport::Cache::RedisCacheStore.new(**described_class.active_support_config) }
+
+      subject { cache.read('x') }
+
+      before do
+        described_class.with do |redis|
+          allow(redis).to receive(:get).and_raise(::Redis::CommandError)
+        end
+      end
+
+      it 'logs error' do
+        expect(::Gitlab::ErrorTracking).to receive(:log_exception)
+        subject
+      end
+    end
   end
 end

spec/lib/gitlab/redis/rate_limiting_spec.rb

@@ -4,4 +4,21 @@
 RSpec.describe Gitlab::Redis::RateLimiting do
   include_examples "redis_new_instance_shared_examples", 'rate_limiting', Gitlab::Redis::Cache
+
+  describe '.cache_store' do
+    context 'when encountering an error' do
+      subject { described_class.cache_store.read('x') }
+
+      before do
+        described_class.with do |redis|
+          allow(redis).to receive(:get).and_raise(::Redis::CommandError)
+        end
+      end
+
+      it 'logs error' do
+        expect(::Gitlab::ErrorTracking).to receive(:log_exception)
+        subject
+      end
+    end
+  end
 end

spec/lib/gitlab/redis/repository_cache_spec.rb

@@ -17,5 +17,20 @@
     it 'has a default ttl of 8 hours' do
       expect(described_class.cache_store.options[:expires_in]).to eq(8.hours)
     end
+
+    context 'when encountering an error' do
+      subject { described_class.cache_store.read('x') }
+
+      before do
+        described_class.with do |redis|
+          allow(redis).to receive(:get).and_raise(::Redis::CommandError)
+        end
+      end
+
+      it 'logs error' do
+        expect(::Gitlab::ErrorTracking).to receive(:log_exception)
+        subject
+      end
+    end
   end
 end