Add Redis call details in Peek performance barsh-improve-redis-peek

Since Redis timings appear to be increasing in production, this change
makes it easier to see what exactly which queries are being called and
where.

This is done by prepending modules in peek-redis to store the call
details.

This commit redact values for all SET commands (e.g. HMSET, GETSET,
etc.).

5 files changed, 93 insertions, 89 deletions

diff --git a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue b/app/assets/javascripts/performance_bar/components/performance_bar_app.vue
index 185003c306e..015c1527500 100644
--- a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue
+++ b/app/assets/javascripts/performance_bar/components/performance_bar_app.vue
@@ -4,14 +4,12 @@ import { glEmojiTag } from '~/emoji';
 
 import detailedMetric from './detailed_metric.vue';
 import requestSelector from './request_selector.vue';
-import simpleMetric from './simple_metric.vue';
 import { s__ } from '~/locale';
 
 export default {
   components: {
     detailedMetric,
     requestSelector,
-    simpleMetric,
   },
   props: {
     store: {
@@ -43,8 +41,13 @@ export default {
       details: 'details',
       keys: ['feature', 'request'],
     },
+    {
+      metric: 'redis',
+      header: 'Redis calls',
+      details: 'details',
+      keys: ['cmd'],
+    },
   ],
-  simpleMetrics: ['redis'],
   data() {
     return { currentRequestId: '' };
   },
@@ -124,12 +127,6 @@ export default {
         </button>
         <a v-else :href="profileUrl">{{ s__('PerformanceBar|profile') }}</a>
       </div>
-      <simple-metric
-        v-for="metric in $options.simpleMetrics"
-        :key="metric"
-        :current-request="currentRequest"
-        :metric="metric"
-      />
       <div id="peek-view-gc" class="view">
         <span v-if="currentRequest.details" class="bold">
           <span title="Invoke Time">{{ currentRequest.details.gc.gc_time }}</span
diff --git a/app/assets/javascripts/performance_bar/components/simple_metric.vue b/app/assets/javascripts/performance_bar/components/simple_metric.vue
deleted file mode 100644
index 358a57d5bc5..00000000000
--- a/app/assets/javascripts/performance_bar/components/simple_metric.vue
+++ /dev/null
@@ -1,33 +0,0 @@
-<script>
-export default {
-  props: {
-    currentRequest: {
-      type: Object,
-      required: true,
-    },
-    metric: {
-      type: String,
-      required: true,
-    },
-  },
-  computed: {
-    duration() {
-      return (
-        this.currentRequest.details[this.metric] &&
-        this.currentRequest.details[this.metric].duration
-      );
-    },
-    calls() {
-      return (
-        this.currentRequest.details[this.metric] && this.currentRequest.details[this.metric].calls
-      );
-    },
-  },
-};
-</script>
-<template>
-  <div :id="`peek-view-${metric}`" class="view">
-    <span v-if="currentRequest.details" class="bold"> {{ duration }} / {{ calls }} </span>
-    {{ metric }}
-  </div>
-</template>
diff --git a/changelogs/unreleased/sh-improve-redis-peek.yml b/changelogs/unreleased/sh-improve-redis-peek.yml
new file mode 100644
index 00000000000..940be103ab7
--- /dev/null
+++ b/changelogs/unreleased/sh-improve-redis-peek.yml
@@ -0,0 +1,5 @@
+---
+title: Add Redis call details in Peek performance bar
+merge_request: 30191
+author:
+type: changed
diff --git a/lib/peek/views/redis.rb b/lib/peek/views/redis.rb
new file mode 100644
index 00000000000..ad3c3c9fe01
--- /dev/null
+++ b/lib/peek/views/redis.rb
@@ -0,0 +1,82 @@
+# frozen_string_literal: true
+
+require 'redis'
+require 'peek-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
+    module RedisDetailed
+      def results
+        super.merge(details: details)
+      end
+
+      def details
+        detail_store
+          .sort { |a, b| b[:duration] <=> a[:duration] }
+          .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))
+      end
+
+      def format_command(cmd)
+        # Scrub out the value of the SET calls to avoid binary
+        # data or large data from spilling into the view
+        if cmd.length >= 2 && cmd.first =~ /set/i
+          cmd[-1] = "<redacted>"
+        end
+
+        cmd.join(' ')
+      end
+    end
+  end
+end
+
+class Redis::Client
+  prepend Gitlab::Peek::RedisInstrumented
+end
+
+module Peek
+  module Views
+    class Redis < View
+      prepend Peek::Views::RedisDetailed
+    end
+  end
+end
diff --git a/spec/javascripts/performance_bar/components/simple_metric_spec.js b/spec/javascripts/performance_bar/components/simple_metric_spec.js
deleted file mode 100644
index 98b843e9711..00000000000
--- a/spec/javascripts/performance_bar/components/simple_metric_spec.js
+++ /dev/null
@@ -1,47 +0,0 @@
-import Vue from 'vue';
-import simpleMetric from '~/performance_bar/components/simple_metric.vue';
-import mountComponent from 'spec/helpers/vue_mount_component_helper';
-
-describe('simpleMetric', () => {
-  let vm;
-
-  afterEach(() => {
-    vm.$destroy();
-  });
-
-  describe('when the current request has no details', () => {
-    beforeEach(() => {
-      vm = mountComponent(Vue.extend(simpleMetric), {
-        currentRequest: {},
-        metric: 'gitaly',
-      });
-    });
-
-    it('does not display details', () => {
-      expect(vm.$el.innerText).not.toContain('/');
-    });
-
-    it('displays the metric name', () => {
-      expect(vm.$el.innerText).toContain('gitaly');
-    });
-  });
-
-  describe('when the current request has details', () => {
-    beforeEach(() => {
-      vm = mountComponent(Vue.extend(simpleMetric), {
-        currentRequest: {
-          details: { gitaly: { duration: '123ms', calls: '456' } },
-        },
-        metric: 'gitaly',
-      });
-    });
-
-    it('diplays details', () => {
-      expect(vm.$el.innerText.replace(/\s+/g, ' ')).toContain('123ms / 456');
-    });
-
-    it('displays the metric name', () => {
-      expect(vm.$el.innerText).toContain('gitaly');
-    });
-  });
-});