diff options
author | Sean McGivern <sean@gitlab.com> | 2019-07-26 14:03:00 +0100 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2019-07-26 14:37:26 +0100 |
commit | ad1c71663f7780838c7c90979419f3e3cf5ec580 (patch) | |
tree | 59443b9ea0f860511610acf181e1eadbc8c88d34 | |
parent | 55f99e930e1c147ec191a234ff4881ea7e70ea61 (diff) | |
download | gitlab-ce-ad1c71663f7780838c7c90979419f3e3cf5ec580.tar.gz |
Replace peek-pg with our own implementationremove-peek-pg
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.
-rw-r--r-- | Gemfile | 3 | ||||
-rw-r--r-- | Gemfile.lock | 6 | ||||
-rw-r--r-- | app/assets/javascripts/performance_bar/components/detailed_metric.vue | 13 | ||||
-rw-r--r-- | app/assets/javascripts/performance_bar/components/performance_bar_app.vue | 12 | ||||
-rw-r--r-- | config/initializers/peek.rb | 40 | ||||
-rw-r--r-- | lib/gitlab/performance_bar/peek_query_tracker.rb | 45 | ||||
-rw-r--r-- | lib/peek/views/active_record.rb | 25 | ||||
-rw-r--r-- | lib/peek/views/detailed_view.rb | 12 | ||||
-rw-r--r-- | lib/peek/views/gitaly.rb | 3 | ||||
-rw-r--r-- | lib/peek/views/redis_detailed.rb | 19 | ||||
-rw-r--r-- | lib/peek/views/rugged.rb | 3 | ||||
-rw-r--r-- | spec/javascripts/performance_bar/components/detailed_metric_spec.js | 27 | ||||
-rw-r--r-- | vendor/licenses.csv | 1 |
13 files changed, 82 insertions, 127 deletions
@@ -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 diff --git a/Gemfile.lock b/Gemfile.lock index 7e26c5bbc45..24dee04833c 100644 --- a/Gemfile.lock +++ b/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) diff --git a/app/assets/javascripts/performance_bar/components/detailed_metric.vue b/app/assets/javascripts/performance_bar/components/detailed_metric.vue index d5f1cea8356..5bc1d5e0533 100644 --- a/app/assets/javascripts/performance_bar/components/detailed_metric.vue +++ b/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> 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 73c9c60765a..769ddb21277 100644 --- a/app/assets/javascripts/performance_bar/components/performance_bar_app.vue +++ b/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"> diff --git a/config/initializers/peek.rb b/config/initializers/peek.rb index 8416ae430c7..d51d553c939 100644 --- a/config/initializers/peek.rb +++ b/config/initializers/peek.rb @@ -1,44 +1,14 @@ -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 diff --git a/lib/gitlab/performance_bar/peek_query_tracker.rb b/lib/gitlab/performance_bar/peek_query_tracker.rb deleted file mode 100644 index 3a27e26eaba..00000000000 --- a/lib/gitlab/performance_bar/peek_query_tracker.rb +++ /dev/null @@ -1,45 +0,0 @@ -# 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 diff --git a/lib/peek/views/active_record.rb b/lib/peek/views/active_record.rb new file mode 100644 index 00000000000..2d78818630d --- /dev/null +++ b/lib/peek/views/active_record.rb @@ -0,0 +1,25 @@ +# 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 diff --git a/lib/peek/views/detailed_view.rb b/lib/peek/views/detailed_view.rb index ebaf46478df..f4ca1cb5075 100644 --- a/lib/peek/views/detailed_view.rb +++ b/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 diff --git a/lib/peek/views/gitaly.rb b/lib/peek/views/gitaly.rb index 067aaf31fbc..6ad6ddfd89d 100644 --- a/lib/peek/views/gitaly.rb +++ b/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 diff --git a/lib/peek/views/redis_detailed.rb b/lib/peek/views/redis_detailed.rb index c61a1e91282..a64ea672895 100644 --- a/lib/peek/views/redis_detailed.rb +++ b/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) diff --git a/lib/peek/views/rugged.rb b/lib/peek/views/rugged.rb index f0cd520fb8b..6b9d3e7b1a3 100644 --- a/lib/peek/views/rugged.rb +++ b/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) diff --git a/spec/javascripts/performance_bar/components/detailed_metric_spec.js b/spec/javascripts/performance_bar/components/detailed_metric_spec.js index 8a7aa057186..0486b5fa3db 100644 --- a/spec/javascripts/performance_bar/components/detailed_metric_spec.js +++ b/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'); + }); + }); }); }); diff --git a/vendor/licenses.csv b/vendor/licenses.csv index 0c52cb5a947..bf6d05c6b6e 100644 --- a/vendor/licenses.csv +++ b/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" |