diff options
author | Stan Hu <stanhu@gmail.com> | 2019-07-17 22:33:49 +0300 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-07-18 01:11:01 +0300 |
commit | 9dd59df6991b9d82bcbb95bf406194aab8ecf743 (patch) | |
tree | 8e1331802676781b266be121e8e228cb272fa054 /lib/peek/views/redis_detailed.rb | |
parent | c11eb0c3a42dba550764e96426dd9bf21347b917 (diff) |
Fix inconsistency in Redis performance bar stats
peek-redis resets its counters at the start of an ActionController
notification (`start_processing.action_controller`), which causes it to
miss some Redis queries that precede it, such as the database load
balancer and Rack Attack queries. This produces inconsistencies in the
performance bar between the number of calls and their durations with the
actual calls in the detailed view.
We fix this by getting rid of peek-redis in favor of consolidating all
logic into the `RedisDetailed` view, which tracks Redis queries using
`RequestStore`. This has the nice property of removing thread-specific
counters as well.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/64707
Diffstat (limited to 'lib/peek/views/redis_detailed.rb')
-rw-r--r-- | lib/peek/views/redis_detailed.rb | 104 |
1 files changed, 104 insertions, 0 deletions
diff --git a/lib/peek/views/redis_detailed.rb b/lib/peek/views/redis_detailed.rb new file mode 100644 index 00000000000..12760c9b75e --- /dev/null +++ b/lib/peek/views/redis_detailed.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'redis' + +module Gitlab + module Peek + module RedisInstrumented + def call(*args, &block) + start = Time.now + super(*args, &block) + ensure + duration = (Time.now - start) + add_call_details(duration, args) + end + + private + + def add_call_details(duration, args) + # redis-rb passes an array (e.g. [:get, key]) + return unless args.length == 1 + + detail_store << { + cmd: args.first, + duration: duration, + backtrace: Gitlab::Profiler.clean_backtrace(caller) + } + end + + def detail_store + ::Gitlab::SafeRequestStore['redis_call_details'] ||= [] + end + end + end +end + +module Peek + module Views + class RedisDetailed < View + REDACTED_MARKER = "<redacted>" + + def key + 'redis' + end + + def results + { + calls: calls, + duration: formatted_duration, + details: details + } + end + + def detail_store + ::Gitlab::SafeRequestStore['redis_call_details'] ||= [] + end + + private + + def formatted_duration + ms = duration * 1000 + if ms >= 1000 + "%.2fms" % ms + else + "%.0fms" % ms + end + end + + def duration + detail_store.map { |entry| entry[:duration] }.sum # rubocop:disable CodeReuse/ActiveRecord + end + + def calls + detail_store.count + end + + def details + detail_store + .sort { |a, b| b[:duration] <=> a[:duration] } + .map(&method(:format_call_details)) + end + + def format_call_details(call) + call.merge(cmd: format_command(call[:cmd]), + duration: (call[:duration] * 1000).round(3)) + end + + def format_command(cmd) + if cmd.length >= 2 && cmd.first =~ /^auth$/i + cmd[-1] = REDACTED_MARKER + # Scrub out the value of the SET calls to avoid binary + # data or large data from spilling into the view + elsif cmd.length >= 3 && cmd.first =~ /set/i + cmd[2..-1] = REDACTED_MARKER + end + + cmd.join(' ') + end + end + end +end + +class Redis::Client + prepend Gitlab::Peek::RedisInstrumented +end |