Commit ddb27c18 authored by Dylan Griffith's avatar Dylan Griffith

Add db_config_name label to prometheus database metrics

The sharding team is adding support for multiple databases and in order
for us to have good visibility into performance we'll want to be able to
distinguish between the different databases in our prometheus metrics.

Adding a prometheus label allows us to filter/group by the different
databases.

By default there will only be 1 `db_config_name` which is `primary` by
default. But this is configured by `config/database.yml` and in future
there will be 2 databases called `main` and `ci`.

This MR also ensures the new label is disabled by default to allow us to
introduce this to production in a slightly more controlled way. We opted
to use an environment variable instead of feature flag in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66885 because it's
too expensive to query the databases so frequently to check the feature
flag value.
parent ea72d0ff
...@@ -134,7 +134,12 @@ module Gitlab ...@@ -134,7 +134,12 @@ module Gitlab
:"gitlab_transaction_db_#{counter}_total" :"gitlab_transaction_db_#{counter}_total"
end end
current_transaction&.increment(prometheus_key, 1) if ENV['GITLAB_MULTIPLE_DATABASE_METRICS']
current_transaction&.increment(prometheus_key, 1, { db_config_name: db_config_name })
else
current_transaction&.increment(prometheus_key, 1)
end
Gitlab::SafeRequestStore[log_key] = Gitlab::SafeRequestStore[log_key].to_i + 1 Gitlab::SafeRequestStore[log_key] = Gitlab::SafeRequestStore[log_key].to_i + 1
# To avoid confusing log keys we only log the db_config_name metrics # To avoid confusing log keys we only log the db_config_name metrics
...@@ -147,7 +152,13 @@ module Gitlab ...@@ -147,7 +152,13 @@ module Gitlab
end end
def observe(histogram, event, &block) def observe(histogram, event, &block)
current_transaction&.observe(histogram, event.duration / 1000.0, &block) db_config_name = db_config_name(event.payload)
if ENV['GITLAB_MULTIPLE_DATABASE_METRICS']
current_transaction&.observe(histogram, event.duration / 1000.0, { db_config_name: db_config_name }, &block)
else
current_transaction&.observe(histogram, event.duration / 1000.0, &block)
end
end end
def current_transaction def current_transaction
......
...@@ -8,6 +8,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do ...@@ -8,6 +8,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do
let(:env) { {} } let(:env) { {} }
let(:subscriber) { described_class.new } let(:subscriber) { described_class.new }
let(:connection) { ActiveRecord::Base.connection } let(:connection) { ActiveRecord::Base.connection }
let(:db_config_name) { ::Gitlab::Database.db_config_name(connection) }
describe '#transaction' do describe '#transaction' do
let(:web_transaction) { double('Gitlab::Metrics::WebTransaction') } let(:web_transaction) { double('Gitlab::Metrics::WebTransaction') }
...@@ -36,7 +37,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do ...@@ -36,7 +37,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do
end end
it 'captures the metrics for web only' do it 'captures the metrics for web only' do
expect(web_transaction).to receive(:observe).with(:gitlab_database_transaction_seconds, 0.23) expect(web_transaction).to receive(:observe).with(:gitlab_database_transaction_seconds, 0.23, db_config_name: db_config_name)
expect(background_transaction).not_to receive(:observe) expect(background_transaction).not_to receive(:observe)
expect(background_transaction).not_to receive(:increment) expect(background_transaction).not_to receive(:increment)
...@@ -56,7 +57,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do ...@@ -56,7 +57,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do
end end
it 'captures the metrics for web only' do it 'captures the metrics for web only' do
expect(web_transaction).to receive(:observe).with(:gitlab_database_transaction_seconds, 0.23) expect(web_transaction).to receive(:observe).with(:gitlab_database_transaction_seconds, 0.23, { db_config_name: db_config_name })
expect(background_transaction).not_to receive(:observe) expect(background_transaction).not_to receive(:observe)
expect(background_transaction).not_to receive(:increment) expect(background_transaction).not_to receive(:increment)
...@@ -76,7 +77,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do ...@@ -76,7 +77,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do
end end
it 'captures the metrics for web only' do it 'captures the metrics for web only' do
expect(background_transaction).to receive(:observe).with(:gitlab_database_transaction_seconds, 0.23) expect(background_transaction).to receive(:observe).with(:gitlab_database_transaction_seconds, 0.23, db_config_name: db_config_name)
expect(web_transaction).not_to receive(:observe) expect(web_transaction).not_to receive(:observe)
expect(web_transaction).not_to receive(:increment) expect(web_transaction).not_to receive(:increment)
......
...@@ -81,36 +81,38 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role| ...@@ -81,36 +81,38 @@ RSpec.shared_examples 'store ActiveRecord info in RequestStore' do |db_role|
end end
RSpec.shared_examples 'record ActiveRecord metrics in a metrics transaction' do |db_role| RSpec.shared_examples 'record ActiveRecord metrics in a metrics transaction' do |db_role|
let(:db_config_name) { ::Gitlab::Database.db_config_name(ApplicationRecord.connection) }
it 'increments only db counters' do it 'increments only db counters' do
if record_query if record_query
expect(transaction).to receive(:increment).with(:gitlab_transaction_db_count_total, 1) expect(transaction).to receive(:increment).with(:gitlab_transaction_db_count_total, 1, { db_config_name: db_config_name })
expect(transaction).to receive(:increment).with("gitlab_transaction_db_#{db_role}_count_total".to_sym, 1) if db_role expect(transaction).to receive(:increment).with("gitlab_transaction_db_#{db_role}_count_total".to_sym, 1, { db_config_name: db_config_name }) if db_role
else else
expect(transaction).not_to receive(:increment).with(:gitlab_transaction_db_count_total, 1) expect(transaction).not_to receive(:increment).with(:gitlab_transaction_db_count_total, 1, { db_config_name: db_config_name })
expect(transaction).not_to receive(:increment).with("gitlab_transaction_db_#{db_role}_count_total".to_sym, 1) if db_role expect(transaction).not_to receive(:increment).with("gitlab_transaction_db_#{db_role}_count_total".to_sym, 1, { db_config_name: db_config_name }) if db_role
end end
if record_write_query if record_write_query
expect(transaction).to receive(:increment).with(:gitlab_transaction_db_write_count_total, 1) expect(transaction).to receive(:increment).with(:gitlab_transaction_db_write_count_total, 1, { db_config_name: db_config_name })
else else
expect(transaction).not_to receive(:increment).with(:gitlab_transaction_db_write_count_total, 1) expect(transaction).not_to receive(:increment).with(:gitlab_transaction_db_write_count_total, 1, { db_config_name: db_config_name })
end end
if record_cached_query if record_cached_query
expect(transaction).to receive(:increment).with(:gitlab_transaction_db_cached_count_total, 1) expect(transaction).to receive(:increment).with(:gitlab_transaction_db_cached_count_total, 1, { db_config_name: db_config_name })
expect(transaction).to receive(:increment).with("gitlab_transaction_db_#{db_role}_cached_count_total".to_sym, 1) if db_role expect(transaction).to receive(:increment).with("gitlab_transaction_db_#{db_role}_cached_count_total".to_sym, 1, { db_config_name: db_config_name }) if db_role
else else
expect(transaction).not_to receive(:increment).with(:gitlab_transaction_db_cached_count_total, 1) expect(transaction).not_to receive(:increment).with(:gitlab_transaction_db_cached_count_total, 1, { db_config_name: db_config_name })
expect(transaction).not_to receive(:increment).with("gitlab_transaction_db_#{db_role}_cached_count_total".to_sym, 1) if db_role expect(transaction).not_to receive(:increment).with("gitlab_transaction_db_#{db_role}_cached_count_total".to_sym, 1, { db_config_name: db_config_name }) if db_role
end end
if record_wal_query if record_wal_query
if db_role if db_role
expect(transaction).to receive(:increment).with("gitlab_transaction_db_#{db_role}_wal_count_total".to_sym, 1) expect(transaction).to receive(:increment).with("gitlab_transaction_db_#{db_role}_wal_count_total".to_sym, 1, { db_config_name: db_config_name })
expect(transaction).to receive(:increment).with("gitlab_transaction_db_#{db_role}_wal_cached_count_total".to_sym, 1) if record_cached_query expect(transaction).to receive(:increment).with("gitlab_transaction_db_#{db_role}_wal_cached_count_total".to_sym, 1, { db_config_name: db_config_name }) if record_cached_query
end end
else else
expect(transaction).not_to receive(:increment).with("gitlab_transaction_db_#{db_role}_wal_count_total".to_sym, 1) if db_role expect(transaction).not_to receive(:increment).with("gitlab_transaction_db_#{db_role}_wal_count_total".to_sym, 1, { db_config_name: db_config_name }) if db_role
end end
subscriber.sql(event) subscriber.sql(event)
...@@ -118,14 +120,34 @@ RSpec.shared_examples 'record ActiveRecord metrics in a metrics transaction' do ...@@ -118,14 +120,34 @@ RSpec.shared_examples 'record ActiveRecord metrics in a metrics transaction' do
it 'observes sql_duration metric' do it 'observes sql_duration metric' do
if record_query if record_query
expect(transaction).to receive(:observe).with(:gitlab_sql_duration_seconds, 0.002) expect(transaction).to receive(:observe).with(:gitlab_sql_duration_seconds, 0.002, { db_config_name: db_config_name })
expect(transaction).to receive(:observe).with("gitlab_sql_#{db_role}_duration_seconds".to_sym, 0.002) if db_role expect(transaction).to receive(:observe).with("gitlab_sql_#{db_role}_duration_seconds".to_sym, 0.002, { db_config_name: db_config_name }) if db_role
else else
expect(transaction).not_to receive(:observe) expect(transaction).not_to receive(:observe)
end end
subscriber.sql(event) subscriber.sql(event)
end end
context 'when the GITLAB_MULTIPLE_DATABASE_METRICS env var is disabled' do
before do
stub_env('GITLAB_MULTIPLE_DATABASE_METRICS', nil)
end
it 'does not include db_config_name label' do
allow(transaction).to receive(:increment) do |*args|
labels = args[2] || {}
expect(labels).not_to include(:db_config_name)
end
allow(transaction).to receive(:observe) do |*args|
labels = args[2] || {}
expect(labels).not_to include(:db_config_name)
end
subscriber.sql(event)
end
end
end end
RSpec.shared_examples 'record ActiveRecord metrics' do |db_role| RSpec.shared_examples 'record ActiveRecord metrics' do |db_role|
......
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