Merge branch 'remove-peek-pg' into 'master'

Replace peek-pg with our own implementation

Closes #44441

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

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"