Merge branch 'sh-improve-redis-peek' into 'master'

Add Redis call details in Peek performance bar

See merge request gitlab-org/gitlab-ce!30191

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

app/assets/javascripts/performance_bar/components/simple_metric.vue

-<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>

changelogs/unreleased/sh-improve-redis-peek.yml

+---
+title: Add Redis call details in Peek performance bar
+merge_request: 30191
+author:
+type: changed

lib/peek/views/redis.rb

+# 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

spec/javascripts/performance_bar/components/simple_metric_spec.js

-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');
-    });
-  });
-});