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

Gemfile

@@ -299,7 +299,6 @@ gem 'peek-gc', '~> 0.0.2'
 gem 'peek-mysql2', '~> 1.2.0', group: :mysql
 gem 'peek-pg', '~> 1.3.0', group: :postgres
 gem 'peek-rblineprof', '~> 0.2.0'
-gem 'peek-redis', '~> 1.2.0'
 
 # Memory benchmarks
 gem 'derailed_benchmarks', require: false

Gemfile.lock

@@ -76,7 +76,6 @@ GEM
     asciidoctor-plantuml (0.0.9)
       asciidoctor (>= 1.5.6, < 3.0.0)
     ast (2.4.0)
-    atomic (1.1.99)
     attr_encrypted (3.1.0)
       encryptor (~> 3.0.0)
     attr_required (1.0.1)
@@ -658,10 +657,6 @@ GEM
     peek-rblineprof (0.2.0)
       peek
       rblineprof
-    peek-redis (1.2.0)
-      atomic (>= 1.0.0)
-      peek
-      redis
     pg (1.1.4)
     po_to_json (1.0.1)
       json (>= 1.6.0)
@@ -1199,7 +1194,6 @@ DEPENDENCIES
   peek-mysql2 (~> 1.2.0)
   peek-pg (~> 1.3.0)
   peek-rblineprof (~> 0.2.0)
-  peek-redis (~> 1.2.0)
   pg (~> 1.1)
   premailer-rails (~> 1.9.7)
   prometheus-client-mmap (~> 0.9.8)

config/initializers/peek.rb

@@ -29,7 +29,7 @@ end
 Peek.into PEEK_DB_VIEW
 Peek.into Peek::Views::Gitaly
 Peek.into Peek::Views::Rblineprof
-Peek.into Peek::Views::Redis
+Peek.into Peek::Views::RedisDetailed
 Peek.into Peek::Views::GC
 Peek.into Peek::Views::Tracing if Labkit::Tracing.tracing_url_enabled?
 

lib/peek/views/redis.rb → lib/peek/views/redis_detailed.rb

 # frozen_string_literal: true
 
 require 'redis'
-require 'peek-redis'
 
 module Gitlab
   module Peek
@@ -36,11 +35,42 @@ end
 
 module Peek
   module Views
-    module RedisDetailed
+    class RedisDetailed < View
       REDACTED_MARKER = "<redacted>"
 
+      def key
+        'redis'
+      end
+
       def results
-        super.merge(details: details)
+        {
+          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
@@ -49,10 +79,6 @@ module Peek
           .map(&method(:format_call_details))
       end
 
-      def detail_store
-        ::Gitlab::SafeRequestStore['redis_call_details'] ||= []
-      end
-
       def format_call_details(call)
         call.merge(cmd: format_command(call[:cmd]),
                    duration: (call[:duration] * 1000).round(3))
@@ -76,11 +102,3 @@ end
 class Redis::Client
   prepend Gitlab::Peek::RedisInstrumented
 end
-
-module Peek
-  module Views
-    class Redis < View
-      prepend Peek::Views::RedisDetailed
-    end
-  end
-end

spec/lib/peek/views/redis_detailed_spec.rb

@@ -2,14 +2,8 @@
 
 require 'spec_helper'
 
-describe Peek::Views::RedisDetailed do
-  let(:redis_detailed_class) do
-    Class.new do
-      include Peek::Views::RedisDetailed
-    end
-  end
-
-  subject { redis_detailed_class.new }
+describe Peek::Views::RedisDetailed, :request_store do
+  subject { described_class.new }
 
   using RSpec::Parameterized::TableSyntax
 
@@ -22,15 +16,24 @@ describe Peek::Views::RedisDetailed do
   end
 
   with_them do
-    it 'scrubs Redis commands', :request_store do
+    it 'scrubs Redis commands' do
       subject.detail_store << { cmd: cmd, duration: 1.second }
 
-      expect(subject.details.count).to eq(1)
-      expect(subject.details.first)
+      expect(subject.results[:details].count).to eq(1)
+      expect(subject.results[:details].first)
         .to eq({
                  cmd: expected,
                  duration: 1000
                })
     end
   end
+
+  it 'returns aggregated results' do
+    subject.detail_store << { cmd: [:get, 'test'], duration: 0.001 }
+    subject.detail_store << { cmd: [:get, 'test'], duration: 1.second }
+
+    expect(subject.results[:calls]).to eq(2)
+    expect(subject.results[:duration]).to eq('1001.00ms')
+    expect(subject.results[:details].count).to eq(2)
+  end
 end