Commit 3afa2700 authored by Dylan Griffith's avatar Dylan Griffith

Include database config name in performance bar

As part of https://gitlab.com/groups/gitlab-org/-/epics/5759 we're
preparing the GitLab application to handle connections to multiple
databases. As such we want to improve our observability to understand
these multiple databases.

This MR just adds the database name to the performance bar details.

Unfortunately the default rails database config name is `primary`.
Eventually we will force this to be `main` and it will be distinct from
the new `ci` database. This does create a little bit of confusion now as
we also use the words `primary/replica` to distinguish between our
read/write and read-only database servers. This means you may see
"Replica" next to "Config name: primary" which could be confusing. I
hope that the usage of "Config name" should help to disambiguate. I have
also used a feature flag to be less disruptive.
parent 440e4556
......@@ -40,7 +40,7 @@ export default {
metric: 'active-record',
title: 'pg',
header: s__('PerformanceBar|SQL queries'),
keys: ['sql', 'cached', 'transaction', 'db_role'],
keys: ['sql', 'cached', 'transaction', 'db_role', 'db_config_name'],
},
{
metric: 'bullet',
......
......@@ -19,7 +19,20 @@ From left to right, it displays:
- **Current Host**: the current host serving the page.
- **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
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](../../gitaly/index.md) calls. Click to display a modal window with more
details.
......
......@@ -66,7 +66,8 @@ module Peek
backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller),
cached: data[:cached] ? 'Cached' : '',
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
......@@ -76,7 +77,15 @@ module Peek
role = ::Gitlab::Database::LoadBalancing.db_role_for_connection(data[:connection]) ||
::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
......
......@@ -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_2).to receive(:transaction_open?).and_return(true)
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
context 'when database load balancing is not enabled' do
......@@ -77,32 +78,48 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
cached: '',
transaction: '',
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(
start: be_a(Time),
cached: 'Cached',
transaction: '',
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(
start: be_a(Time),
cached: '',
transaction: 'In a transaction',
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(
start: be_a(Time),
cached: '',
transaction: '',
duration: 4000.0,
sql: 'SELECT VERSION()'
sql: 'SELECT VERSION()',
db_config_name: "Config name: the_db_config_name"
)
)
)
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
context 'when database load balancing is enabled' 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)
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
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)
......@@ -127,9 +144,9 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
summary: {
"Cached" => 1,
"In a transaction" => 1,
"Primary" => 2,
"Replica" => 1,
"Unknown" => 1
"Role: Primary" => 2,
"Role: Replica" => 1,
"Role: Unknown" => 1
},
duration: '10000.00ms',
warnings: ["active-record duration: 10000.0 over 3000"],
......@@ -140,7 +157,8 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
transaction: '',
duration: 1000.0,
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(
start: be_a(Time),
......@@ -148,7 +166,8 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
transaction: '',
duration: 2000.0,
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(
start: be_a(Time),
......@@ -156,7 +175,8 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
transaction: 'In a transaction',
duration: 3000.0,
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(
start: be_a(Time),
......@@ -164,10 +184,23 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do
transaction: '',
duration: 4000.0,
sql: 'SELECT VERSION()',
db_role: 'Unknown'
db_role: 'Role: Unknown',
db_config_name: "Config name: the_db_config_name"
)
)
)
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
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