Commit 3e7343b1 authored by Sean McGivern's avatar Sean McGivern

Only report MysteryRedisDurationError for shared state Redis

This error reporting (behind a feature flag) was added to investigate
high durations we saw on the shared state Redis on GitLab.com. However,
it did not check the Redis instance. This is a particular problem with
the Sidekiq (queues) Redis instance, which receives many BRPOP commands
with a timeout higher than the reporting threshold for this error.

As a result, when we enabled this flag we got flooded with 'expected'
errors for the queues Redis, which we didn't care about at all. So now
we also check which Redis instance we're using before reporting the
error.
parent 8784b014
...@@ -41,7 +41,10 @@ module Gitlab ...@@ -41,7 +41,10 @@ module Gitlab
instrumentation_class.add_call_details(duration, args) instrumentation_class.add_call_details(duration, args)
end end
if duration > DURATION_ERROR_THRESHOLD && Feature.enabled?(:report_on_long_redis_durations, default_enabled: :yaml) if duration > DURATION_ERROR_THRESHOLD &&
instrumentation_class == ::Gitlab::Instrumentation::Redis::SharedState &&
Feature.enabled?(:report_on_long_redis_durations, default_enabled: :yaml)
Gitlab::ErrorTracking.track_exception(MysteryRedisDurationError.new(caller), Gitlab::ErrorTracking.track_exception(MysteryRedisDurationError.new(caller),
command: command_from_args(args), command: command_from_args(args),
duration: duration, duration: duration,
......
...@@ -130,6 +130,15 @@ RSpec.describe Gitlab::Instrumentation::RedisInterceptor, :clean_gitlab_redis_sh ...@@ -130,6 +130,15 @@ RSpec.describe Gitlab::Instrumentation::RedisInterceptor, :clean_gitlab_redis_sh
end end
context 'when report_on_long_redis_durations is enabled' do context 'when report_on_long_redis_durations is enabled' do
context 'for an instance other than SharedState' do
it 'does nothing' do
expect(Gitlab::ErrorTracking).not_to receive(:track_exception)
Gitlab::Redis::Queues.with { |r| r.mget('foo', 'foo') { sleep threshold + 0.1 } }
end
end
context 'for the SharedState instance' do
it 'tracks an exception and continues' do it 'tracks an exception and continues' do
expect(Gitlab::ErrorTracking) expect(Gitlab::ErrorTracking)
.to receive(:track_exception) .to receive(:track_exception)
...@@ -142,4 +151,5 @@ RSpec.describe Gitlab::Instrumentation::RedisInterceptor, :clean_gitlab_redis_sh ...@@ -142,4 +151,5 @@ RSpec.describe Gitlab::Instrumentation::RedisInterceptor, :clean_gitlab_redis_sh
end end
end end
end end
end
end end
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