Add summary to peek ActiveRecord

Issue https://gitlab.com/gitlab-org/gitlab/-/issues/324649

ee/lib/ee/peek/views/active_record.rb

@@ -11,11 +11,23 @@ module EE
           detail = super
 
           if ::Gitlab::Database::LoadBalancing.enable?
-            detail[:db_role] = ::Gitlab::Database::LoadBalancing.db_role_for_connection(data[:connection])
+            detail[:db_role] = ::Gitlab::Database::LoadBalancing.db_role_for_connection(data[:connection]).to_s.capitalize
           end
 
           detail
         end
+
+        override :summary
+        def summary
+          if ::Gitlab::Database::LoadBalancing.enable?
+            detail_store.each_with_object(super) do |item, count|
+              count[item[:db_role]] ||= 0
+              count[item[:db_role]] += 1
+            end
+          else
+            super
+          end
+        end
       end
     end
   end

ee/spec/lib/peek/views/active_record_spec.rb

@@ -6,14 +6,15 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
   subject { Peek.views.find { |v| v.instance_of?(Peek::Views::ActiveRecord) } }
 
   let(:connection_replica) { double(:connection_replica) }
-  let(:connection_primary) { double(:connection_primary) }
+  let(:connection_primary_1) { double(:connection_primary) }
+  let(:connection_primary_2) { double(:connection_primary) }
 
   let(:event_1) do
     {
       name: 'SQL',
       sql: 'SELECT * FROM users WHERE id = 10',
       cached: false,
-      connection: connection_primary
+      connection: connection_primary_1
     }
   end
 
@@ -31,12 +32,15 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
       name: 'SQL',
       sql: 'UPDATE users SET admin = true WHERE id = 10',
       cached: false,
-      connection: connection_primary
+      connection: connection_primary_2
     }
   end
 
   before do
     allow(Gitlab::PerformanceBar).to receive(:enabled_for_request?).and_return(true)
+    allow(connection_replica).to receive(:transaction_open?).and_return(false)
+    allow(connection_primary_1).to receive(:transaction_open?).and_return(false)
+    allow(connection_primary_2).to receive(:transaction_open?).and_return(true)
   end
 
   context 'when database load balancing is not enabled' do
@@ -48,22 +52,32 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
       end
 
       expect(subject.results).to match(
-        calls: '3 (1 cached)',
+        calls: 3,
+        summary: {
+          "Cached" => 1,
+          "In a transaction" => 1
+        },
         duration: '6000.00ms',
         warnings: ["active-record duration: 6000.0 over 3000"],
         details: contain_exactly(
           a_hash_including(
+            start: be_a(Time),
             cached: '',
+            transaction: '',
             duration: 1000.0,
             sql: 'SELECT * FROM users WHERE id = 10'
           ),
           a_hash_including(
-            cached: 'cached',
+            start: be_a(Time),
+            cached: 'Cached',
+            transaction: '',
             duration: 2000.0,
             sql: 'SELECT * FROM users WHERE id = 10'
           ),
           a_hash_including(
+            start: be_a(Time),
             cached: '',
+            transaction: 'In a transaction',
             duration: 3000.0,
             sql: 'UPDATE users SET admin = true WHERE id = 10'
           )
@@ -76,7 +90,8 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
     before do
       allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
       allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_replica).and_return(:replica)
-      allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary).and_return(:primary)
+      allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_1).and_return(:primary)
+      allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_2).and_return(:primary)
     end
 
     it 'includes db role data' do
@@ -87,27 +102,39 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
       end
 
       expect(subject.results).to match(
-        calls: '3 (1 cached)',
+        calls: 3,
+        summary: {
+          "Cached" => 1,
+          "In a transaction" => 1,
+          "Primary" => 2,
+          "Replica" => 1
+        },
         duration: '6000.00ms',
         warnings: ["active-record duration: 6000.0 over 3000"],
         details: contain_exactly(
           a_hash_including(
+            start: be_a(Time),
             cached: '',
+            transaction: '',
             duration: 1000.0,
             sql: 'SELECT * FROM users WHERE id = 10',
-            db_role: :primary
+            db_role: 'Primary'
           ),
           a_hash_including(
-            cached: 'cached',
+            start: be_a(Time),
+            cached: 'Cached',
+            transaction: '',
             duration: 2000.0,
             sql: 'SELECT * FROM users WHERE id = 10',
-            db_role: :replica
+            db_role: 'Replica'
           ),
           a_hash_including(
+            start: be_a(Time),
             cached: '',
+            transaction: 'In a transaction',
             duration: 3000.0,
             sql: 'UPDATE users SET admin = true WHERE id = 10',
-            db_role: :primary
+            db_role: 'Primary'
           )
         )
       )

lib/peek/views/active_record.rb

@@ -17,22 +17,28 @@ module Peek
         }
       }.freeze
 
-      def results
-        super.merge(calls: detailed_calls)
-      end
-
       def self.thresholds
         @thresholds ||= THRESHOLDS.fetch(Rails.env.to_sym, DEFAULT_THRESHOLDS)
       end
 
+      def results
+        super.merge(summary: summary)
+      end
+
       private
 
-      def detailed_calls
-        "#{calls} (#{cached_calls} cached)"
+      def summary
+        detail_store.each_with_object({}) do |item, count|
+          if item[:cached].present?
+            count[item[:cached]] ||= 0
+            count[item[:cached]] += 1
           end
 
-      def cached_calls
-        detail_store.count { |item| item[:cached] == 'cached' }
+          if item[:transaction].present?
+            count[item[:transaction]] ||= 0
+            count[item[:transaction]] += 1
+          end
+        end
       end
 
       def setup_subscribers
@@ -45,10 +51,12 @@ module Peek
 
       def generate_detail(start, finish, data)
         {
+          start: start,
           duration: finish - start,
           sql: data[:sql].strip,
           backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller),
-          cached: data[:cached] ? 'cached' : ''
+          cached: data[:cached] ? 'Cached' : '',
+          transaction: data[:connection].transaction_open? ? 'In a transaction' : ''
         }
       end
     end

spec/lib/peek/views/active_record_spec.rb

@@ -5,14 +5,16 @@ require 'spec_helper'
 RSpec.describe Peek::Views::ActiveRecord, :request_store do
   subject { Peek.views.find { |v| v.instance_of?(Peek::Views::ActiveRecord) } }
 
-  let(:connection) { double(:connection) }
+  let(:connection_1) { double(:connection) }
+  let(:connection_2) { double(:connection) }
+  let(:connection_3) { double(:connection) }
 
   let(:event_1) do
     {
       name: 'SQL',
       sql: 'SELECT * FROM users WHERE id = 10',
       cached: false,
-      connection: connection
+      connection: connection_1
     }
   end
 
@@ -21,7 +23,7 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
       name: 'SQL',
       sql: 'SELECT * FROM users WHERE id = 10',
       cached: true,
-      connection: connection
+      connection: connection_2
     }
   end
 
@@ -30,12 +32,15 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
       name: 'SQL',
       sql: 'UPDATE users SET admin = true WHERE id = 10',
       cached: false,
-      connection: connection
+      connection: connection_3
     }
   end
 
   before do
     allow(Gitlab::PerformanceBar).to receive(:enabled_for_request?).and_return(true)
+    allow(connection_1).to receive(:transaction_open?).and_return(false)
+    allow(connection_2).to receive(:transaction_open?).and_return(false)
+    allow(connection_3).to receive(:transaction_open?).and_return(true)
   end
 
   it 'subscribes and store data into peek views' do
@@ -46,22 +51,32 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
     end
 
     expect(subject.results).to match(
-      calls: '3 (1 cached)',
+      calls: 3,
+      summary: {
+        "Cached" => 1,
+        "In a transaction" => 1
+      },
       duration: '6000.00ms',
       warnings: ["active-record duration: 6000.0 over 3000"],
       details: contain_exactly(
         a_hash_including(
+          start: be_a(Time),
           cached: '',
+          transaction: '',
           duration: 1000.0,
           sql: 'SELECT * FROM users WHERE id = 10'
         ),
         a_hash_including(
-          cached: 'cached',
+          start: be_a(Time),
+          cached: 'Cached',
+          transaction: '',
           duration: 2000.0,
           sql: 'SELECT * FROM users WHERE id = 10'
         ),
         a_hash_including(
+          start: be_a(Time),
           cached: '',
+          transaction: 'In a transaction',
           duration: 3000.0,
           sql: 'UPDATE users SET admin = true WHERE id = 10'
         )