Commit c5b31389 authored by Thong Kuah's avatar Thong Kuah

Merge branch '333066-include-dbname-in-performance-bar' into 'master'

Include database name in performance bar

See merge request gitlab-org/gitlab!63819
parents 44c83f7d 3afa2700
...@@ -40,7 +40,7 @@ export default { ...@@ -40,7 +40,7 @@ export default {
metric: 'active-record', metric: 'active-record',
title: 'pg', title: 'pg',
header: s__('PerformanceBar|SQL queries'), header: s__('PerformanceBar|SQL queries'),
keys: ['sql', 'cached', 'transaction', 'db_role'], keys: ['sql', 'cached', 'transaction', 'db_role', 'db_config_name'],
}, },
{ {
metric: 'bullet', metric: 'bullet',
......
...@@ -21,7 +21,20 @@ From left to right, the performance bar displays: ...@@ -21,7 +21,20 @@ From left to right, the performance bar displays:
- **Current Host**: the current host serving the page. - **Current Host**: the current host serving the page.
- **Database queries**: the time taken (in milliseconds) and the total number - **Database queries**: the time taken (in milliseconds) and the total number
of database queries, displayed in the format `00ms / 00 (00 cached) pg`. Click to display of database queries, displayed in the format `00ms / 00 (00 cached) pg`. Click to display
a modal window with more details. a modal window with more details. You can use this to see the following
details for each query:
- **In a transaction**: shows up below the query if it was executed in
the context of a transaction
- **Role**: shows up when [database load
balancing](../../database_load_balancing.md) is enabled. It shows
which server role was used for the query. "Primary" means that the query
was sent to the read/write primary server. "Replica" means it was sent
to a read-only replica.
- **Config name**: shows up only when the
`multiple_database_metrics` feature flag is enabled. This is used to
distinguish between different databases configured for different GitLab
features. The name shown is the same name used to configure database
connections in GitLab.
- **Gitaly calls**: the time taken (in milliseconds) and the total number of - **Gitaly calls**: the time taken (in milliseconds) and the total number of
[Gitaly](../../gitaly/index.md) calls. Click to display a modal window with more [Gitaly](../../gitaly/index.md) calls. Click to display a modal window with more
details. details.
......
...@@ -66,7 +66,8 @@ module Peek ...@@ -66,7 +66,8 @@ module Peek
backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller), backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller),
cached: data[:cached] ? 'Cached' : '', cached: data[:cached] ? 'Cached' : '',
transaction: data[:connection].transaction_open? ? 'In a transaction' : '', transaction: data[:connection].transaction_open? ? 'In a transaction' : '',
db_role: db_role(data) db_role: db_role(data),
db_config_name: "Config name: #{::Gitlab::Database.db_config_name(data[:connection])}"
} }
end end
...@@ -76,7 +77,15 @@ module Peek ...@@ -76,7 +77,15 @@ module Peek
role = ::Gitlab::Database::LoadBalancing.db_role_for_connection(data[:connection]) || role = ::Gitlab::Database::LoadBalancing.db_role_for_connection(data[:connection]) ||
::Gitlab::Database::LoadBalancing::ROLE_UNKNOWN ::Gitlab::Database::LoadBalancing::ROLE_UNKNOWN
role.to_s.capitalize "Role: #{role.to_s.capitalize}"
end
def format_call_details(call)
if Feature.enabled?(:multiple_database_metrics, default_enabled: :yaml)
super
else
super.except(:db_config_name)
end
end end
end end
end end
......
...@@ -52,6 +52,7 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do ...@@ -52,6 +52,7 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
allow(connection_primary_1).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) allow(connection_primary_2).to receive(:transaction_open?).and_return(true)
allow(connection_unknown).to receive(:transaction_open?).and_return(false) allow(connection_unknown).to receive(:transaction_open?).and_return(false)
allow(::Gitlab::Database).to receive(:db_config_name).and_return('the_db_config_name')
end end
context 'when database load balancing is not enabled' do context 'when database load balancing is not enabled' do
...@@ -77,32 +78,48 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do ...@@ -77,32 +78,48 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
cached: '', cached: '',
transaction: '', transaction: '',
duration: 1000.0, duration: 1000.0,
sql: 'SELECT * FROM users WHERE id = 10' sql: 'SELECT * FROM users WHERE id = 10',
db_config_name: "Config name: the_db_config_name"
), ),
a_hash_including( a_hash_including(
start: be_a(Time), start: be_a(Time),
cached: 'Cached', cached: 'Cached',
transaction: '', transaction: '',
duration: 2000.0, duration: 2000.0,
sql: 'SELECT * FROM users WHERE id = 10' sql: 'SELECT * FROM users WHERE id = 10',
db_config_name: "Config name: the_db_config_name"
), ),
a_hash_including( a_hash_including(
start: be_a(Time), start: be_a(Time),
cached: '', cached: '',
transaction: 'In a transaction', transaction: 'In a transaction',
duration: 3000.0, duration: 3000.0,
sql: 'UPDATE users SET admin = true WHERE id = 10' sql: 'UPDATE users SET admin = true WHERE id = 10',
db_config_name: "Config name: the_db_config_name"
), ),
a_hash_including( a_hash_including(
start: be_a(Time), start: be_a(Time),
cached: '', cached: '',
transaction: '', transaction: '',
duration: 4000.0, duration: 4000.0,
sql: 'SELECT VERSION()' sql: 'SELECT VERSION()',
db_config_name: "Config name: the_db_config_name"
) )
) )
) )
end end
context 'when the multiple_database_metrics feature flag is disabled' do
before do
stub_feature_flags(multiple_database_metrics: false)
end
it 'does not include db_config_name field' do
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1)
expect(subject.results[:details][0][:db_config_name]).to be_nil
end
end
end end
context 'when database load balancing is enabled' do context 'when database load balancing is enabled' do
...@@ -114,7 +131,7 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do ...@@ -114,7 +131,7 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_unknown).and_return(nil) allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_unknown).and_return(nil)
end end
it 'includes db role data' do it 'includes db role data and db_config_name name' do
Timecop.freeze(2021, 2, 23, 10, 0) do Timecop.freeze(2021, 2, 23, 10, 0) do
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1)
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2) ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2)
...@@ -127,9 +144,9 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do ...@@ -127,9 +144,9 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
summary: { summary: {
"Cached" => 1, "Cached" => 1,
"In a transaction" => 1, "In a transaction" => 1,
"Primary" => 2, "Role: Primary" => 2,
"Replica" => 1, "Role: Replica" => 1,
"Unknown" => 1 "Role: Unknown" => 1
}, },
duration: '10000.00ms', duration: '10000.00ms',
warnings: ["active-record duration: 10000.0 over 3000"], warnings: ["active-record duration: 10000.0 over 3000"],
...@@ -140,7 +157,8 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do ...@@ -140,7 +157,8 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
transaction: '', transaction: '',
duration: 1000.0, duration: 1000.0,
sql: 'SELECT * FROM users WHERE id = 10', sql: 'SELECT * FROM users WHERE id = 10',
db_role: 'Primary' db_role: 'Role: Primary',
db_config_name: "Config name: the_db_config_name"
), ),
a_hash_including( a_hash_including(
start: be_a(Time), start: be_a(Time),
...@@ -148,7 +166,8 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do ...@@ -148,7 +166,8 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
transaction: '', transaction: '',
duration: 2000.0, duration: 2000.0,
sql: 'SELECT * FROM users WHERE id = 10', sql: 'SELECT * FROM users WHERE id = 10',
db_role: 'Replica' db_role: 'Role: Replica',
db_config_name: "Config name: the_db_config_name"
), ),
a_hash_including( a_hash_including(
start: be_a(Time), start: be_a(Time),
...@@ -156,7 +175,8 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do ...@@ -156,7 +175,8 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
transaction: 'In a transaction', transaction: 'In a transaction',
duration: 3000.0, duration: 3000.0,
sql: 'UPDATE users SET admin = true WHERE id = 10', sql: 'UPDATE users SET admin = true WHERE id = 10',
db_role: 'Primary' db_role: 'Role: Primary',
db_config_name: "Config name: the_db_config_name"
), ),
a_hash_including( a_hash_including(
start: be_a(Time), start: be_a(Time),
...@@ -164,10 +184,23 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do ...@@ -164,10 +184,23 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
transaction: '', transaction: '',
duration: 4000.0, duration: 4000.0,
sql: 'SELECT VERSION()', sql: 'SELECT VERSION()',
db_role: 'Unknown' db_role: 'Role: Unknown',
db_config_name: "Config name: the_db_config_name"
) )
) )
) )
end end
context 'when the multiple_database_metrics feature flag is disabled' do
before do
stub_feature_flags(multiple_database_metrics: false)
end
it 'does not include db_config_name field' do
ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1)
expect(subject.results[:details][0][:db_config_name]).to be_nil
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment