Replace peek-pg with our own implementation

This uses an ActiveRecord subscriber to get queries and calculate the
total query time from that. This means that the total will always be
consistent with the queries in the table. It does however mean that we
could potentially miss some queries that don't go through ActiveRecord.

Making this change also allows us to unify the response JSON a little
bit, making the frontend slightly simpler as a result.

Gemfile

@@ -16,7 +16,7 @@ gem 'sprockets', '~> 3.7.0'
 gem 'default_value_for', '~> 3.2.0'
 
 # Supported DBs
-gem 'pg', '~> 1.1', group: :postgres
+gem 'pg', '~> 1.1'
 
 gem 'rugged', '~> 0.28'
 gem 'grape-path-helpers', '~> 1.1'
@@ -297,7 +297,6 @@ gem 'batch-loader', '~> 1.4.0'
 # Perf bar
 gem 'peek', '~> 1.0.1'
 gem 'peek-gc', '~> 0.0.2'
-gem 'peek-pg', '~> 1.3.0', group: :postgres
 gem 'peek-rblineprof', '~> 0.2.0'
 
 # Memory benchmarks

Gemfile.lock

@@ -643,11 +643,6 @@ GEM
       railties (>= 4.0.0)
     peek-gc (0.0.2)
       peek
-    peek-pg (1.3.0)
-      concurrent-ruby
-      concurrent-ruby-ext
-      peek
-      pg
     peek-rblineprof (0.2.0)
       peek
       rblineprof
@@ -1184,7 +1179,6 @@ DEPENDENCIES
   org-ruby (~> 0.9.12)
   peek (~> 1.0.1)
   peek-gc (~> 0.0.2)
-  peek-pg (~> 1.3.0)
   peek-rblineprof (~> 0.2.0)
   pg (~> 1.1)
   premailer-rails (~> 1.9.7)

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

@@ -16,11 +16,14 @@ export default {
       type: String,
       required: true,
     },
-    header: {
+    title: {
       type: String,
-      required: true,
+      required: false,
+      default() {
+        return this.metric;
+      },
     },
-    details: {
+    header: {
       type: String,
       required: true,
     },
@@ -34,7 +37,7 @@ export default {
       return this.currentRequest.details[this.metric];
     },
     detailsList() {
-      return this.metricDetails[this.details];
+      return this.metricDetails.details;
     },
   },
 };
@@ -101,6 +104,6 @@ export default {
 
       <div slot="footer"></div>
     </gl-modal>
-    {{ metric }}
+    {{ title }}
   </div>
 </template>

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

@@ -34,23 +34,25 @@ export default {
     },
   },
   detailedMetrics: [
-    { metric: 'pg', header: s__('PerformanceBar|SQL queries'), details: 'queries', keys: ['sql'] },
+    {
+      metric: 'active-record',
+      title: 'pg',
+      header: s__('PerformanceBar|SQL queries'),
+      keys: ['sql'],
+    },
     {
       metric: 'gitaly',
       header: s__('PerformanceBar|Gitaly calls'),
-      details: 'details',
       keys: ['feature', 'request'],
     },
     {
       metric: 'rugged',
       header: s__('PerformanceBar|Rugged calls'),
-      details: 'details',
       keys: ['feature', 'args'],
     },
     {
       metric: 'redis',
       header: s__('PerformanceBar|Redis calls'),
-      details: 'details',
       keys: ['cmd'],
     },
   ],
@@ -118,8 +120,8 @@ export default {
         :key="metric.metric"
         :current-request="currentRequest"
         :metric="metric.metric"
+        :title="metric.title"
         :header="metric.header"
-        :details="metric.details"
         :keys="metric.keys"
       />
       <div v-if="initialRequest" id="peek-view-rblineprof" class="view">

config/initializers/peek.rb

-Rails.application.config.peek.adapter = :redis, { client: ::Redis.new(Gitlab::Redis::Cache.params) }
-
-Peek.into Peek::Views::Host
+require 'peek/adapters/redis'
 
-if Gitlab::Database.postgresql?
-  require 'peek-pg'
-  PEEK_DB_CLIENT = ::PG::Connection
-  PEEK_DB_VIEW = Peek::Views::PG
+Peek::Adapters::Redis.prepend ::Gitlab::PerformanceBar::RedisAdapterWhenPeekEnabled
 
-  # Remove once we have https://github.com/peek/peek-pg/pull/10
-  module ::Peek::PGInstrumented
-    def exec_params(*args)
-      start = Time.now
-      super(*args)
-    ensure
-      duration = (Time.now - start)
-      PEEK_DB_CLIENT.query_time.update { |value| value + duration }
-      PEEK_DB_CLIENT.query_count.update { |value| value + 1 }
-    end
-  end
-else
-  raise "Unsupported database adapter for peek!"
-end
+Rails.application.config.peek.adapter = :redis, { client: ::Redis.new(Gitlab::Redis::Cache.params) }
 
-Peek.into PEEK_DB_VIEW
+Peek.into Peek::Views::Host
+Peek.into Peek::Views::ActiveRecord
 Peek.into Peek::Views::Gitaly
 Peek.into Peek::Views::Rblineprof
 Peek.into Peek::Views::RedisDetailed
 Peek.into Peek::Views::Rugged
 Peek.into Peek::Views::GC
 Peek.into Peek::Views::Tracing if Labkit::Tracing.tracing_url_enabled?
-
-# rubocop:disable Naming/ClassAndModuleCamelCase
-class PEEK_DB_CLIENT
-  class << self
-    attr_accessor :query_details
-  end
-  self.query_details = Concurrent::Array.new
-end
-
-PEEK_DB_VIEW.prepend ::Gitlab::PerformanceBar::PeekQueryTracker
-
-require 'peek/adapters/redis'
-Peek::Adapters::Redis.prepend ::Gitlab::PerformanceBar::RedisAdapterWhenPeekEnabled

lib/gitlab/performance_bar/peek_query_tracker.rb

-# frozen_string_literal: true
-
-# Inspired by https://github.com/peek/peek-pg/blob/master/lib/peek/views/pg.rb
-# PEEK_DB_CLIENT is a constant set in config/initializers/peek.rb
-module Gitlab
-  module PerformanceBar
-    module PeekQueryTracker
-      def sorted_queries
-        PEEK_DB_CLIENT.query_details
-          .sort { |a, b| b[:duration] <=> a[:duration] }
-      end
-
-      def results
-        super.merge(queries: sorted_queries)
-      end
-
-      private
-
-      def setup_subscribers
-        super
-
-        # Reset each counter when a new request starts
-        before_request do
-          PEEK_DB_CLIENT.query_details = []
-        end
-
-        subscribe('sql.active_record') do |_, start, finish, _, data|
-          if Gitlab::SafeRequestStore.store[:peek_enabled]
-            unless data[:cached]
-              backtrace = Gitlab::Profiler.clean_backtrace(caller)
-              track_query(data[:sql].strip, data[:binds], backtrace, start, finish)
-            end
-          end
-        end
-      end
-
-      def track_query(raw_query, bindings, backtrace, start, finish)
-        duration = (finish - start) * 1000.0
-        query_info = { duration: duration.round(3), sql: raw_query, backtrace: backtrace }
-
-        PEEK_DB_CLIENT.query_details << query_info
-      end
-    end
-  end
-end

lib/peek/views/active_record.rb

+# frozen_string_literal: true
+
+module Peek
+  module Views
+    class ActiveRecord < DetailedView
+      private
+
+      def setup_subscribers
+        super
+
+        subscribe('sql.active_record') do |_, start, finish, _, data|
+          if Gitlab::SafeRequestStore.store[:peek_enabled]
+            unless data[:cached]
+              detail_store << {
+                duration: finish - start,
+                sql: data[:sql].strip,
+                backtrace: Gitlab::Profiler.clean_backtrace(caller)
+              }
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/peek/views/detailed_view.rb

@@ -11,22 +11,26 @@ module Peek
         }
       end
 
+      def detail_store
+        ::Gitlab::SafeRequestStore["#{key}_call_details"] ||= []
+      end
+
       private
 
       def duration
-        raise NotImplementedError
+        detail_store.map { |entry| entry[:duration] }.sum # rubocop:disable CodeReuse/ActiveRecord
       end
 
       def calls
-        raise NotImplementedError
+        detail_store.count
       end
 
       def call_details
-        raise NotImplementedError
+        detail_store
       end
 
       def format_call_details(call)
-        raise NotImplementedError
+        call.merge(duration: (call[:duration] * 1000).round(3))
       end
 
       def details

lib/peek/views/gitaly.rb

@@ -20,8 +20,7 @@ module Peek
       def format_call_details(call)
         pretty_request = call[:request]&.reject { |k, v| v.blank? }.to_h.pretty_inspect
 
-        call.merge(duration: (call[:duration] * 1000).round(3),
-                   request: pretty_request || {})
+        super.merge(request: pretty_request || {})
       end
 
       def setup_subscribers

lib/peek/views/redis_detailed.rb

@@ -42,27 +42,10 @@ module Peek
         'redis'
       end
 
-      def detail_store
-        ::Gitlab::SafeRequestStore['redis_call_details'] ||= []
-      end
-
       private
 
-      def duration
-        detail_store.map { |entry| entry[:duration] }.sum # rubocop:disable CodeReuse/ActiveRecord
-      end
-
-      def calls
-        detail_store.count
-      end
-
-      def call_details
-        detail_store
-      end
-
       def format_call_details(call)
-        call.merge(cmd: format_command(call[:cmd]),
-                   duration: (call[:duration] * 1000).round(3))
+        super.merge(cmd: format_command(call[:cmd]))
       end
 
       def format_command(cmd)

lib/peek/views/rugged.rb

@@ -24,8 +24,7 @@ module Peek
       end
 
       def format_call_details(call)
-        call.merge(duration: (call[:duration] * 1000).round(3),
-                   args: format_args(call[:args]))
+        super.merge(args: format_args(call[:args]))
       end
 
       def format_args(args)

spec/javascripts/performance_bar/components/detailed_metric_spec.js

@@ -44,7 +44,6 @@ describe('detailedMetric', () => {
         },
         metric: 'gitaly',
         header: 'Gitaly calls',
-        details: 'details',
         keys: ['feature', 'request'],
       });
     });
@@ -79,8 +78,32 @@ describe('detailedMetric', () => {
       });
     });
 
-    it('displays the metric name', () => {
+    it('displays the metric title', () => {
       expect(vm.$el.innerText).toContain('gitaly');
     });
+
+    describe('when using a custom metric title', () => {
+      beforeEach(() => {
+        vm = mountComponent(Vue.extend(detailedMetric), {
+          currentRequest: {
+            details: {
+              gitaly: {
+                duration: '123ms',
+                calls: '456',
+                details: requestDetails,
+              },
+            },
+          },
+          metric: 'gitaly',
+          title: 'custom',
+          header: 'Gitaly calls',
+          keys: ['feature', 'request'],
+        });
+      });
+
+      it('displays the custom title', () => {
+        expect(vm.$el.innerText).toContain('custom');
+      });
+    });
   });
 });

vendor/licenses.csv

@@ -816,7 +816,6 @@ pbkdf2,3.0.14,MIT
 peek,1.0.1,MIT
 peek-gc,0.0.2,MIT
 peek-mysql2,1.1.0,MIT
-peek-pg,1.3.0,MIT
 peek-rblineprof,0.2.0,MIT
 peek-redis,1.2.0,MIT
 pg,0.18.4,"BSD,ruby,GPL"