Commit f69d8a3f authored by Sean McGivern's avatar Sean McGivern

Merge branch '1438-add_missing_methods_to_multistore' into 'master'

Fix  logging method_missing commands in Redis::Multistore

See merge request gitlab-org/gitlab!75630
parents b7d55792 f7d2bd0b
...@@ -10,9 +10,9 @@ module Gitlab ...@@ -10,9 +10,9 @@ module Gitlab
def count_session_ip def count_session_ip
redis_store_class.with do |redis| redis_store_class.with do |redis|
redis.pipelined do redis.pipelined do |pipeline|
redis.incr(session_lookup_name) pipeline.incr(session_lookup_name)
redis.expire(session_lookup_name, 24.hours) pipeline.expire(session_lookup_name, 24.hours)
end end
end end
end end
......
...@@ -21,6 +21,8 @@ module Gitlab ...@@ -21,6 +21,8 @@ module Gitlab
FAILED_TO_READ_ERROR_MESSAGE = 'Failed to read from the redis primary_store.' FAILED_TO_READ_ERROR_MESSAGE = 'Failed to read from the redis primary_store.'
FAILED_TO_WRITE_ERROR_MESSAGE = 'Failed to write to the redis primary_store.' FAILED_TO_WRITE_ERROR_MESSAGE = 'Failed to write to the redis primary_store.'
SKIP_LOG_METHOD_MISSING_FOR_COMMANDS = %i(info).freeze
READ_COMMANDS = %i( READ_COMMANDS = %i(
get get
mget mget
...@@ -109,6 +111,8 @@ module Gitlab ...@@ -109,6 +111,8 @@ module Gitlab
end end
def log_method_missing(command_name, *_args) def log_method_missing(command_name, *_args)
return if SKIP_LOG_METHOD_MISSING_FOR_COMMANDS.include?(command_name)
log_error(MethodMissingError.new, command_name) log_error(MethodMissingError.new, command_name)
increment_method_missing_count(command_name) increment_method_missing_count(command_name)
end end
......
...@@ -478,9 +478,7 @@ RSpec.describe Gitlab::Redis::MultiStore do ...@@ -478,9 +478,7 @@ RSpec.describe Gitlab::Redis::MultiStore do
let_it_be(:key) { "redis:counter" } let_it_be(:key) { "redis:counter" }
subject do subject { multi_store.incr(key) }
multi_store.incr(key)
end
it 'executes method missing' do it 'executes method missing' do
expect(multi_store).to receive(:method_missing) expect(multi_store).to receive(:method_missing)
...@@ -488,17 +486,35 @@ RSpec.describe Gitlab::Redis::MultiStore do ...@@ -488,17 +486,35 @@ RSpec.describe Gitlab::Redis::MultiStore do
subject subject
end end
it 'logs MethodMissingError' do context 'when command is not in SKIP_LOG_METHOD_MISSING_FOR_COMMANDS' do
expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(Gitlab::Redis::MultiStore::MethodMissingError), it 'logs MethodMissingError' do
hash_including(command_name: :incr, extra: hash_including(instance_name: instance_name))) expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(Gitlab::Redis::MultiStore::MethodMissingError),
hash_including(command_name: :incr, extra: hash_including(instance_name: instance_name)))
subject subject
end
it 'increments method missing counter' do
expect(counter).to receive(:increment).with(command: :incr, instance_name: instance_name)
subject
end
end end
it 'increments method missing counter' do context 'when command is in SKIP_LOG_METHOD_MISSING_FOR_COMMANDS' do
expect(counter).to receive(:increment).with(command: :incr, instance_name: instance_name) subject { multi_store.info }
subject it 'does not log MethodMissingError' do
expect(Gitlab::ErrorTracking).not_to receive(:log_exception)
subject
end
it 'does not increment method missing counter' do
expect(counter).not_to receive(:increment)
subject
end
end end
context 'with feature flag :use_primary_store_as_default_for_test_store is enabled' do context 'with feature flag :use_primary_store_as_default_for_test_store is enabled' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment