From d30576c5a7197b167d1f2c472361dd4ce77b8262 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Tue, 6 Mar 2018 14:02:18 +0000 Subject: [PATCH] Add Gitaly call details to the performance bar The same as the SQL queries, show the details of Gitaly calls in the performance bar, as a modal that can be opened in the same way. --- app/assets/javascripts/performance_bar.js | 7 ++ app/views/peek/views/_gitaly.html.haml | 17 +++-- ...s-and-arguments-in-the-performance-bar.yml | 5 ++ lib/gitlab/gitaly_client.rb | 18 +++++ lib/peek/views/gitaly.rb | 19 ++++- vendor/assets/javascripts/peek.js | 72 ++++++++++++------- 6 files changed, 108 insertions(+), 30 deletions(-) create mode 100644 changelogs/unreleased/43805-list-gitaly-calls-and-arguments-in-the-performance-bar.yml diff --git a/app/assets/javascripts/performance_bar.js b/app/assets/javascripts/performance_bar.js index ef44e2323ef..a224a3440b4 100644 --- a/app/assets/javascripts/performance_bar.js +++ b/app/assets/javascripts/performance_bar.js @@ -18,6 +18,8 @@ export default class PerformanceBar { this.$sqlProfileModal = $container.find('#modal-peek-pg-queries'); this.$lineProfileLink = $container.find('.js-toggle-modal-peek-line-profile'); this.$lineProfileModal = $('#modal-peek-line-profile'); + this.$gitalyProfileLink = $container.find('.js-toggle-modal-peek-gitaly'); + this.$gitalyProfileModal = $container.find('#modal-peek-gitaly-details'); this.initEventListeners(); this.showModalOnLoad(); } @@ -25,6 +27,7 @@ export default class PerformanceBar { initEventListeners() { this.$sqlProfileLink.on('click', () => this.handleSQLProfileLink()); this.$lineProfileLink.on('click', e => this.handleLineProfileLink(e)); + this.$gitalyProfileLink.on('click', () => this.handleGitalyProfileLink()); $(document).on('click', '.js-lineprof-file', PerformanceBar.toggleLineProfileFile); } @@ -52,6 +55,10 @@ export default class PerformanceBar { } } + handleGitalyProfileLink() { + PerformanceBar.toggleModal(this.$gitalyProfileModal); + } + static toggleModal($modal) { if ($modal.length) { $modal.modal('toggle'); diff --git a/app/views/peek/views/_gitaly.html.haml b/app/views/peek/views/_gitaly.html.haml index a7d040d6821..5e7d1565825 100644 --- a/app/views/peek/views/_gitaly.html.haml +++ b/app/views/peek/views/_gitaly.html.haml @@ -1,7 +1,16 @@ - local_assigns.fetch(:view) %strong - %span{ data: { defer_to: "#{view.defer_key}-duration" } } ... - \/ - %span{ data: { defer_to: "#{view.defer_key}-calls" } } ... - Gitaly + %a.js-toggle-modal-peek-gitaly + %span{ data: { defer_to: "#{view.defer_key}-duration" } }... + \/ + %span{ data: { defer_to: "#{view.defer_key}-calls" } }... +#modal-peek-gitaly-details.modal{ tabindex: -1 } + .modal-dialog.modal-full + .modal-content + .modal-header + %button.close.btn.btn-link.btn-sm{ type: 'button', data: { dismiss: 'modal' } } X + %h4 + Gitaly requests + .modal-body{ data: { defer_to: "#{view.defer_key}-details" } }... +Gitaly diff --git a/changelogs/unreleased/43805-list-gitaly-calls-and-arguments-in-the-performance-bar.yml b/changelogs/unreleased/43805-list-gitaly-calls-and-arguments-in-the-performance-bar.yml new file mode 100644 index 00000000000..4c63e69f0bb --- /dev/null +++ b/changelogs/unreleased/43805-list-gitaly-calls-and-arguments-in-the-performance-bar.yml @@ -0,0 +1,5 @@ +--- +title: Add Gitaly call details to performance bar +merge_request: +author: +type: added diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 9cd76630484..c3aa3ccd7b1 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -119,6 +119,8 @@ module Gitlab # def self.call(storage, service, rpc, request, remote_storage: nil, timeout: nil) start = Gitlab::Metrics::System.monotonic_time + @last_request = request.is_a?(Google::Protobuf::MessageExts) ? request.to_h : nil + enforce_gitaly_request_limits(:call) kwargs = request_kwargs(storage, timeout, remote_storage: remote_storage) @@ -258,6 +260,9 @@ module Gitlab gitaly_migrate_call_duration_seconds.observe({ gitaly_enabled: is_enabled, feature: feature }, total_time) feature_stack.shift Thread.current[:gitaly_feature_stack] = nil if feature_stack.empty? + add_call_details(feature: feature, + duration: total_time, + request: is_enabled ? @last_request : {}) end end end @@ -344,6 +349,19 @@ module Gitlab end end + def self.add_call_details(details) + return unless RequestStore.active? && RequestStore.store[:peek_enabled] + + RequestStore.store['gitaly_call_details'] ||= [] + RequestStore.store['gitaly_call_details'] << details + end + + def self.call_details + return [] unless RequestStore.active? && RequestStore.store[:peek_enabled] + + RequestStore.store['gitaly_call_details'] || [] + end + def self.expected_server_version path = Rails.root.join(SERVER_VERSION_FILE) path.read.chomp diff --git a/lib/peek/views/gitaly.rb b/lib/peek/views/gitaly.rb index d519d8e86fa..79851640318 100644 --- a/lib/peek/views/gitaly.rb +++ b/lib/peek/views/gitaly.rb @@ -10,11 +10,28 @@ module Peek end def results - { duration: formatted_duration, calls: calls } + { + duration: formatted_duration, + calls: calls, + details: details + } end private + def details + ::Gitlab::GitalyClient.call_details + .sort { |a, b| b[:duration] <=> a[:duration] } + .map(&method(:format_call_details)) + end + + 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) + end + def formatted_duration ms = duration * 1000 if ms >= 1000 diff --git a/vendor/assets/javascripts/peek.js b/vendor/assets/javascripts/peek.js index 695eeb27c17..607517f045f 100644 --- a/vendor/assets/javascripts/peek.js +++ b/vendor/assets/javascripts/peek.js @@ -3,12 +3,12 @@ * * - Removed the dependency on jquery.tipsy * - Removed the initializeTipsy and toggleBar functions - * - Customized updatePerformanceBar to handle SQL queries report specificities + * - Customized updatePerformanceBar to handle SQL query and Gitaly call lists * - Changed /peek/results to /-/peek/results * - Removed the keypress, pjax:end, page:change, and turbolinks:load handlers */ (function($) { - var fetchRequestResults, getRequestId, peekEnabled, updatePerformanceBar; + var fetchRequestResults, getRequestId, peekEnabled, updatePerformanceBar, createTable, createTableRow; getRequestId = function() { return $('#peek').data('requestId'); }; @@ -16,39 +16,61 @@ return $('#peek').length; }; updatePerformanceBar = function(results) { - var key, label, data, table, html, tr, duration_td, sql_td, strong; - Object.keys(results.data).forEach(function(key) { Object.keys(results.data[key]).forEach(function(label) { + var data, table, target; + data = results.data[key][label]; + table = createTable(key, label, data); + target = $("[data-defer-to=" + key + "-" + label + "]"); - if (label == 'queries') { - table = document.createElement('table'); + if (table) { + target.html(table); + } else { + target.text(data); + } + }); + }); + return $(document).trigger('peek:render', [getRequestId(), results]); + }; + createTable = function(key, label, data) { + var table; - for (var i = 0; i < data.length; i += 1) { - tr = document.createElement('tr'); - duration_td = document.createElement('td'); - sql_td = document.createElement('td'); - strong = document.createElement('strong'); + if (label != 'queries' && label != 'details') { return; } - strong.append(data[i]['duration'] + 'ms'); - duration_td.appendChild(strong); - tr.appendChild(duration_td); + table = document.createElement('table'); - sql_td.appendChild(document.createTextNode(data[i]['sql'])); - tr.appendChild(sql_td); + for (var i = 0; i < data.length; i += 1) { + table.appendChild(createTableRow(data[i])); + } - table.appendChild(tr); - } + table.className = 'table'; - table.className = 'table'; - $("[data-defer-to=" + key + "-" + label + "]").html(table); - } else { - $("[data-defer-to=" + key + "-" + label + "]").text(results.data[key][label]); - } - }); + return table; + }; + createTableRow = function(row) { + var tr, duration_td, strong; + + tr = document.createElement('tr'); + duration_td = document.createElement('td'); + strong = document.createElement('strong'); + + strong.append(row['duration'] + 'ms'); + duration_td.appendChild(strong); + tr.appendChild(duration_td); + + ['sql', 'feature', 'enabled', 'request'].forEach(function(key) { + var td; + + if (!row[key]) { return; } + + td = document.createElement('td'); + td.appendChild(document.createTextNode(row[key])); + + tr.appendChild(td); }); - return $(document).trigger('peek:render', [getRequestId(), results]); + + return tr; }; fetchRequestResults = function() { return $.ajax('/-/peek/results', { -- GitLab