Merge branch '43805-list-gitaly-calls-and-arguments-in-the-performance-bar' into 'master'

Resolve "List Gitaly calls and arguments in the performance bar"

Closes #43805

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

14 files changed, 144 insertions, 49 deletions

diff --git a/app/assets/javascripts/performance_bar.js b/app/assets/javascripts/performance_bar.js
index ef44e2323ef..c22598ee665 100644
--- a/app/assets/javascripts/performance_bar.js
+++ b/app/assets/javascripts/performance_bar.js
@@ -14,8 +14,6 @@ export default class PerformanceBar {
 
   init(opts) {
     const $container = $(opts.container);
-    this.$sqlProfileLink = $container.find('.js-toggle-modal-peek-sql');
-    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.initEventListeners();
@@ -23,7 +21,6 @@ export default class PerformanceBar {
   }
 
   initEventListeners() {
-    this.$sqlProfileLink.on('click', () => this.handleSQLProfileLink());
     this.$lineProfileLink.on('click', e => this.handleLineProfileLink(e));
     $(document).on('click', '.js-lineprof-file', PerformanceBar.toggleLineProfileFile);
   }
@@ -36,10 +33,6 @@ export default class PerformanceBar {
     }
   }
 
-  handleSQLProfileLink() {
-    PerformanceBar.toggleModal(this.$sqlProfileModal);
-  }
-
   handleLineProfileLink(e) {
     const lineProfilerParameter = getParameterValues('lineprofiler');
     const lineProfilerParameterRegex = new RegExp(`lineprofiler=${lineProfilerParameter[0]}`);
diff --git a/app/views/peek/views/_gc.html.haml b/app/views/peek/views/_gc.html.haml
new file mode 100644
index 00000000000..9fc83e56ee7
--- /dev/null
+++ b/app/views/peek/views/_gc.html.haml
@@ -0,0 +1,7 @@
+- local_assigns.fetch(:view)
+
+%span.bold
+  %span{ title: 'Invoke Time', data: { defer_to: "#{view.defer_key}-gc_time" } }...
+  \/
+  %span{ title: 'Invoke Count', data: { defer_to: "#{view.defer_key}-invokes" } }...
+gc
diff --git a/app/views/peek/views/_gitaly.html.haml b/app/views/peek/views/_gitaly.html.haml
index a7d040d6821..945bb287429 100644
--- a/app/views/peek/views/_gitaly.html.haml
+++ b/app/views/peek/views/_gitaly.html.haml
@@ -1,7 +1,17 @@
 - local_assigns.fetch(:view)
 
-%strong
-  %span{ data: { defer_to: "#{view.defer_key}-duration" } } ...
+%button.btn-blank.btn-link.bold{ type: 'button', data: { toggle: 'modal', target: '#modal-peek-gitaly-details' } }
+  %span{ data: { defer_to: "#{view.defer_key}-duration" } }...
   \/
-  %span{ data: { defer_to: "#{view.defer_key}-calls" } } ...
-  Gitaly
+  %span{ data: { defer_to: "#{view.defer_key}-calls" } }...
+#modal-peek-gitaly-details.modal{ tabindex: -1, role: 'dialog' }
+  .modal-dialog.modal-full
+    .modal-content
+      .modal-header
+        %button.close{ type: 'button', data: { dismiss: 'modal' }, 'aria-label' => 'Close' }
+          %span{ 'aria-hidden' => 'true' }
+            ×
+        %h4
+          Gitaly requests
+      .modal-body{ data: { defer_to: "#{view.defer_key}-details" } }...
+gitaly
diff --git a/app/views/peek/views/_redis.html.haml b/app/views/peek/views/_redis.html.haml
new file mode 100644
index 00000000000..f7fba6c95fc
--- /dev/null
+++ b/app/views/peek/views/_redis.html.haml
@@ -0,0 +1,7 @@
+- local_assigns.fetch(:view)
+
+%span.bold
+  %span{ data: { defer_to: "#{view.defer_key}-duration" } }...
+  \/
+  %span{ data: { defer_to: "#{view.defer_key}-calls" } }...
+redis
diff --git a/app/views/peek/views/_sidekiq.html.haml b/app/views/peek/views/_sidekiq.html.haml
new file mode 100644
index 00000000000..7efbc05890d
--- /dev/null
+++ b/app/views/peek/views/_sidekiq.html.haml
@@ -0,0 +1,7 @@
+- local_assigns.fetch(:view)
+
+%span.bold
+  %span{ data: { defer_to: "#{view.defer_key}-duration" } }...
+  \/
+  %span{ data: { defer_to: "#{view.defer_key}-calls" } }...
+sidekiq
diff --git a/app/views/peek/views/_sql.html.haml b/app/views/peek/views/_sql.html.haml
index dd8b524064f..36583df898a 100644
--- a/app/views/peek/views/_sql.html.haml
+++ b/app/views/peek/views/_sql.html.haml
@@ -1,13 +1,14 @@
-%strong
-  %a.js-toggle-modal-peek-sql
-    %span{ data: { defer_to: "#{view.defer_key}-duration" } }...
-    \/
-    %span{ data: { defer_to: "#{view.defer_key}-calls" } }...
+%button.btn-blank.btn-link.bold{ type: 'button', data: { toggle: 'modal', target: '#modal-peek-pg-queries' } }
+  %span{ data: { defer_to: "#{view.defer_key}-duration" } }...
+  \/
+  %span{ data: { defer_to: "#{view.defer_key}-calls" } }...
 #modal-peek-pg-queries.modal{ tabindex: -1 }
   .modal-dialog.modal-full
     .modal-content
       .modal-header
-        %button.close.btn.btn-link.btn-sm{ type: 'button', data: { dismiss: 'modal' } } X
+        %button.close{ type: 'button', data: { dismiss: 'modal' }, 'aria-label' => 'Close' }
+          %span{ 'aria-hidden' => 'true' }
+            ×
         %h4
           SQL queries
       .modal-body{ data: { defer_to: "#{view.defer_key}-queries" } }...
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/config/initializers/peek.rb b/config/initializers/peek.rb
index 11759801112..ba04a2bf5fa 100644
--- a/config/initializers/peek.rb
+++ b/config/initializers/peek.rb
@@ -16,11 +16,11 @@ else
 end
 
 Peek.into PEEK_DB_VIEW
+Peek.into Peek::Views::Gitaly
+Peek.into Peek::Views::Rblineprof
 Peek.into Peek::Views::Redis
 Peek.into Peek::Views::Sidekiq
-Peek.into Peek::Views::Rblineprof
 Peek.into Peek::Views::GC
-Peek.into Peek::Views::Gitaly
 
 # rubocop:disable Naming/ClassAndModuleCamelCase
 class PEEK_DB_CLIENT
diff --git a/doc/administration/monitoring/performance/img/performance_bar.png b/doc/administration/monitoring/performance/img/performance_bar.png
index b3c6bc474e3..48212f6276a 100644
--- a/doc/administration/monitoring/performance/img/performance_bar.png
+++ b/doc/administration/monitoring/performance/img/performance_bar.png
diff --git a/doc/administration/monitoring/performance/img/performance_bar_gitaly_calls.png b/doc/administration/monitoring/performance/img/performance_bar_gitaly_calls.png
new file mode 100644
index 00000000000..52176df9ecd
--- /dev/null
+++ b/doc/administration/monitoring/performance/img/performance_bar_gitaly_calls.png
diff --git a/doc/administration/monitoring/performance/performance_bar.md b/doc/administration/monitoring/performance/performance_bar.md
index b9464945cea..ec1cbce1bad 100644
--- a/doc/administration/monitoring/performance/performance_bar.md
+++ b/doc/administration/monitoring/performance/performance_bar.md
@@ -11,10 +11,12 @@ It allows you to see (from left to right):
 - the timing of the page (backend, frontend)
 - time taken and number of DB queries, click through for details of these queries
 ![SQL profiling using the Performance Bar](img/performance_bar_sql_queries.png)
-- time taken and number of calls to Redis
-- time taken and number of background jobs created by Sidekiq
+- time taken and number of [Gitaly] calls, click through for details of these calls
+![Gitaly profiling using the Performance Bar](img/performance_bar_gitaly_calls.png)
 - profile of the code used to generate the page, line by line for either _all_, _app & lib_ , or _views_. In the profile view, the numbers in the left panel represent wall time, cpu time, and number of calls (based on [rblineprof](https://github.com/tmm1/rblineprof)).
 ![Line profiling using the Performance Bar](img/performance_bar_line_profiling.png)
+- time taken and number of calls to Redis
+- time taken and number of background jobs created by Sidekiq
 - time taken and number of Ruby GC calls
 
 ## Enable the Performance Bar via the Admin panel
@@ -39,3 +41,5 @@ You can toggle the Bar using the same shortcut.
 ![GitLab Performance Bar Admin Settings](img/performance_bar_configuration_settings.png)
 
 ---
+
+[Gitaly]: ../../gitaly/index.md
diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb
index 9cd76630484..8ca30ffc232 100644
--- a/lib/gitlab/gitaly_client.rb
+++ b/lib/gitlab/gitaly_client.rb
@@ -119,6 +119,9 @@ module Gitlab
     #
     def self.call(storage, service, rpc, request, remote_storage: nil, timeout: nil)
       start = Gitlab::Metrics::System.monotonic_time
+      request_hash = request.is_a?(Google::Protobuf::MessageExts) ? request.to_h : {}
+      @current_call_id ||= SecureRandom.uuid
+
       enforce_gitaly_request_limits(:call)
 
       kwargs = request_kwargs(storage, timeout, remote_storage: remote_storage)
@@ -135,6 +138,10 @@ module Gitlab
       gitaly_controller_action_duration_seconds.observe(
         current_transaction_labels.merge(gitaly_service: service.to_s, rpc: rpc.to_s),
         duration)
+
+      add_call_details(id: @current_call_id, feature: service, duration: duration, request: request_hash)
+
+      @current_call_id = nil
     end
 
     def self.handle_grpc_unavailable!(ex)
@@ -252,12 +259,16 @@ module Gitlab
           feature_stack.unshift(feature)
           begin
             start = Gitlab::Metrics::System.monotonic_time
+            @current_call_id = SecureRandom.uuid
+            call_details = { id: @current_call_id }
             yield is_enabled
           ensure
             total_time = Gitlab::Metrics::System.monotonic_time - start
             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(call_details.merge(feature: feature, duration: total_time))
           end
         end
       end
@@ -344,6 +355,22 @@ module Gitlab
       end
     end
 
+    def self.add_call_details(details)
+      id = details.delete(:id)
+
+      return unless id && RequestStore.active? && RequestStore.store[:peek_enabled]
+
+      RequestStore.store['gitaly_call_details'] ||= {}
+      RequestStore.store['gitaly_call_details'][id] ||= {}
+      RequestStore.store['gitaly_call_details'][id].merge!(details)
+    end
+
+    def self.list_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..ab35f7a2258 100644
--- a/lib/peek/views/gitaly.rb
+++ b/lib/peek/views/gitaly.rb
@@ -10,11 +10,29 @@ module Peek
       end
 
       def results
-        { duration: formatted_duration, calls: calls }
+        {
+          duration: formatted_duration,
+          calls: calls,
+          details: details
+        }
       end
 
       private
 
+      def details
+        ::Gitlab::GitalyClient.list_call_details
+          .values
+          .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..7c6d226fa6a 100644
--- a/vendor/assets/javascripts/peek.js
+++ b/vendor/assets/javascripts/peek.js
@@ -1,14 +1,14 @@
 /*
- * This is a modified version of https://github.com/peek/peek/blob/master/app/assets/javascripts/peek.js
+ * this is a modified version of https://github.com/peek/peek/blob/master/app/assets/javascripts/peek.js
  *
  * - 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,55 @@
     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) {
-        data = results.data[key][label];
+        var data = results.data[key][label];
+        var table = createTable(key, label, data);
+        var target = $('[data-defer-to="' + key + '-' + label + '"]');
+
+        if (table) {
+          target.html(table);
+        } else {
+          target.text(data);
+        }
+      });
+    });
+    return $(document).trigger('peek:render', [getRequestId(), results]);
+  };
+  createTable = function(key, label, data) {
+    if (label !== 'queries' && label !== 'details') {
+      return;
+    }
 
-        if (label == 'queries') {
-          table = document.createElement('table');
+    var table = document.createElement('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');
+    for (var i = 0; i < data.length; i += 1) {
+      table.appendChild(createTableRow(data[i]));
+    }
 
-            strong.append(data[i]['duration'] + 'ms');
-            duration_td.appendChild(strong);
-            tr.appendChild(duration_td);
+    table.className = 'table';
 
-            sql_td.appendChild(document.createTextNode(data[i]['sql']));
-            tr.appendChild(sql_td);
+    return table;
+  };
+  createTableRow = function(row) {
+    var tr = document.createElement('tr');
+    var durationTd = document.createElement('td');
+    var strong = document.createElement('strong');
 
-            table.appendChild(tr);
-          }
+    strong.append(row['duration'] + 'ms');
+    durationTd.appendChild(strong);
+    tr.appendChild(durationTd);
 
-          table.className = 'table';
-          $("[data-defer-to=" + key + "-" + label + "]").html(table);
-        } else {
-          $("[data-defer-to=" + key + "-" + label + "]").text(results.data[key][label]);
-        }
-      });
+    ['sql', 'feature', 'enabled', 'request'].forEach(function(key) {
+      if (!row[key]) { return; }
+
+      var 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', {