Commit f0287c6b authored by Dylan Griffith's avatar Dylan Griffith

Merge branch '334942-change-logging-per-db-to-config-name-instead-of-database-name' into 'master'

Change to db_config.name for logging metrics per database

See merge request gitlab-org/gitlab!66515
parents e3dcf1c2 805e4096
...@@ -352,11 +352,11 @@ module Gitlab ...@@ -352,11 +352,11 @@ module Gitlab
end end
end end
def self.dbname(ar_connection) def self.db_config_name(ar_connection)
if ar_connection.respond_to?(:pool) && if ar_connection.respond_to?(:pool) &&
ar_connection.pool.respond_to?(:db_config) && ar_connection.pool.respond_to?(:db_config) &&
ar_connection.pool.db_config.respond_to?(:database) ar_connection.pool.db_config.respond_to?(:name)
return ar_connection.pool.db_config.database return ar_connection.pool.db_config.name
end end
'unknown' 'unknown'
......
...@@ -74,9 +74,9 @@ module Gitlab ...@@ -74,9 +74,9 @@ module Gitlab
end end
if Feature.enabled?(:multiple_database_metrics, default_enabled: :yaml) if Feature.enabled?(:multiple_database_metrics, default_enabled: :yaml)
::Gitlab::SafeRequestStore[:duration_by_database]&.each do |dbname, duration_by_role| ::Gitlab::SafeRequestStore[:duration_by_database]&.each do |name, duration_by_role|
duration_by_role.each do |db_role, duration| duration_by_role.each do |db_role, duration|
payload[:"db_#{db_role}_#{dbname}_duration_s"] = duration.to_f.round(3) payload[:"db_#{db_role}_#{name}_duration_s"] = duration.to_f.round(3)
end end
end end
end end
...@@ -113,11 +113,11 @@ module Gitlab ...@@ -113,11 +113,11 @@ module Gitlab
::Gitlab::SafeRequestStore[duration_key] = (::Gitlab::SafeRequestStore[duration_key].presence || 0) + duration ::Gitlab::SafeRequestStore[duration_key] = (::Gitlab::SafeRequestStore[duration_key].presence || 0) + duration
# Per database metrics # Per database metrics
dbname = ::Gitlab::Database.dbname(event.payload[:connection]) name = ::Gitlab::Database.db_config_name(event.payload[:connection])
::Gitlab::SafeRequestStore[:duration_by_database] ||= {} ::Gitlab::SafeRequestStore[:duration_by_database] ||= {}
::Gitlab::SafeRequestStore[:duration_by_database][dbname] ||= {} ::Gitlab::SafeRequestStore[:duration_by_database][name] ||= {}
::Gitlab::SafeRequestStore[:duration_by_database][dbname][db_role] ||= 0 ::Gitlab::SafeRequestStore[:duration_by_database][name][db_role] ||= 0
::Gitlab::SafeRequestStore[:duration_by_database][dbname][db_role] += duration ::Gitlab::SafeRequestStore[:duration_by_database][name][db_role] += duration
end end
def ignored_query?(payload) def ignored_query?(payload)
......
...@@ -487,19 +487,19 @@ RSpec.describe Gitlab::Database do ...@@ -487,19 +487,19 @@ RSpec.describe Gitlab::Database do
end end
end end
describe '.dbname' do describe '.db_config_name' do
it 'returns the dbname for the connection' do it 'returns the db_config name for the connection' do
connection = ActiveRecord::Base.connection connection = ActiveRecord::Base.connection
expect(described_class.dbname(connection)).to be_a(String) expect(described_class.db_config_name(connection)).to be_a(String)
expect(described_class.dbname(connection)).to eq(connection.pool.db_config.database) expect(described_class.db_config_name(connection)).to eq(connection.pool.db_config.name)
end end
context 'when the pool is a NullPool' do context 'when the pool is a NullPool' do
it 'returns unknown' do it 'returns unknown' do
connection = double(:active_record_connection, pool: ActiveRecord::ConnectionAdapters::NullPool.new) connection = double(:active_record_connection, pool: ActiveRecord::ConnectionAdapters::NullPool.new)
expect(described_class.dbname(connection)).to eq('unknown') expect(described_class.db_config_name(connection)).to eq('unknown')
end end
end end
end end
......
...@@ -298,7 +298,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do ...@@ -298,7 +298,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do
allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true)
end end
let(:dbname) { ::Gitlab::Database.dbname(ActiveRecord::Base.connection) } let(:db_config_name) { ::Gitlab::Database.db_config_name(ActiveRecord::Base.connection) }
let(:expected_end_payload_with_db) do let(:expected_end_payload_with_db) do
expected_end_payload.merge( expected_end_payload.merge(
...@@ -314,7 +314,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do ...@@ -314,7 +314,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do
'db_primary_cached_count' => 0, 'db_primary_cached_count' => 0,
'db_primary_wal_count' => 0, 'db_primary_wal_count' => 0,
'db_primary_duration_s' => a_value > 0, 'db_primary_duration_s' => a_value > 0,
"db_primary_#{dbname}_duration_s" => a_value > 0, "db_primary_#{db_config_name}_duration_s" => a_value > 0,
'db_primary_wal_cached_count' => 0, 'db_primary_wal_cached_count' => 0,
'db_replica_wal_cached_count' => 0 'db_replica_wal_cached_count' => 0
) )
......
...@@ -23,7 +23,7 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| ...@@ -23,7 +23,7 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role|
db_replica_wal_cached_count: 0, db_replica_wal_cached_count: 0,
db_replica_wal_count: 0 db_replica_wal_count: 0
} }
expected[:"db_primary_#{::Gitlab::Database.dbname(connection)}_duration_s"] = 0.002 if record_query expected[:"db_primary_#{::Gitlab::Database.db_config_name(connection)}_duration_s"] = 0.002 if record_query
elsif db_role == :replica elsif db_role == :replica
expected = { expected = {
db_count: record_query ? 1 : 0, db_count: record_query ? 1 : 0,
...@@ -40,7 +40,7 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| ...@@ -40,7 +40,7 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role|
db_primary_wal_cached_count: 0, db_primary_wal_cached_count: 0,
db_primary_wal_count: 0 db_primary_wal_count: 0
} }
expected[:"db_replica_#{::Gitlab::Database.dbname(connection)}_duration_s"] = 0.002 if record_query expected[:"db_replica_#{::Gitlab::Database.db_config_name(connection)}_duration_s"] = 0.002 if record_query
else else
expected = { expected = {
db_count: record_query ? 1 : 0, db_count: record_query ? 1 : 0,
...@@ -64,7 +64,7 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| ...@@ -64,7 +64,7 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role|
subscriber.sql(event) subscriber.sql(event)
connection = event.payload[:connection] connection = event.payload[:connection]
expect(described_class.db_counter_payload).not_to include(:"db_replica_#{::Gitlab::Database.dbname(connection)}_duration_s") expect(described_class.db_counter_payload).not_to include(:"db_replica_#{::Gitlab::Database.db_config_name(connection)}_duration_s")
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