diff options
Diffstat (limited to 'spec/lib/gitlab/metrics/subscribers/active_record_spec.rb')
-rw-r--r-- | spec/lib/gitlab/metrics/subscribers/active_record_spec.rb | 315 |
1 files changed, 36 insertions, 279 deletions
diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb index edcd5b31941..dffd37eeb9d 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -3,10 +3,12 @@ require 'spec_helper' RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do + using RSpec::Parameterized::TableSyntax + let(:env) { {} } - let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } - let(:subscriber) { described_class.new } - let(:payload) { { sql: 'SELECT * FROM users WHERE id = 10' } } + let(:subscriber) { described_class.new } + let(:connection) { double(:connection) } + let(:payload) { { sql: 'SELECT * FROM users WHERE id = 10', connection: connection } } let(:event) do double( @@ -17,82 +19,32 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do ) end - describe '#sql' do - shared_examples 'track query in metrics' do - before do - allow(subscriber).to receive(:current_transaction) - .at_least(:once) - .and_return(transaction) - end - - it 'increments only db count value' do - described_class::DB_COUNTERS.each do |counter| - prometheus_counter = "gitlab_transaction_#{counter}_total".to_sym - if expected_counters[counter] > 0 - expect(transaction).to receive(:increment).with(prometheus_counter, 1) - else - expect(transaction).not_to receive(:increment).with(prometheus_counter, 1) - end - end - - subscriber.sql(event) - end + # Emulate Marginalia pre-pending comments + def sql(query, comments: true) + if comments && !%w[BEGIN COMMIT].include?(query) + "/*application:web,controller:badges,action:pipeline,correlation_id:01EYN39K9VMJC56Z7808N7RSRH*/ #{query}" + else + query end + end - shared_examples 'track query in RequestStore' do - context 'when RequestStore is enabled' do - it 'caches db count value', :request_store, :aggregate_failures do - subscriber.sql(event) - - described_class::DB_COUNTERS.each do |counter| - expect(Gitlab::SafeRequestStore[counter].to_i).to eq expected_counters[counter] - end - end - - it 'prevents db counters from leaking to the next transaction' do - 2.times do - Gitlab::WithRequestStore.with_request_store do - subscriber.sql(event) - - described_class::DB_COUNTERS.each do |counter| - expect(Gitlab::SafeRequestStore[counter].to_i).to eq expected_counters[counter] - end - end - end - end - end - end - - describe 'without a current transaction' do - it 'does not track any metrics' do - expect_any_instance_of(Gitlab::Metrics::Transaction) - .not_to receive(:increment) - - subscriber.sql(event) - end - - context 'with read query' do - let(:expected_counters) do - { - db_count: 1, - db_write_count: 0, - db_cached_count: 0 - } - end - - it_behaves_like 'track query in RequestStore' - end + shared_examples 'track generic sql events' do + where(:name, :sql_query, :record_query, :record_write_query, :record_cached_query) do + 'SQL' | 'SELECT * FROM users WHERE id = 10' | true | false | false + 'SQL' | 'WITH active_milestones AS (SELECT COUNT(*), state FROM milestones GROUP BY state) SELECT * FROM active_milestones' | true | false | false + 'SQL' | 'SELECT * FROM users WHERE id = 10 FOR UPDATE' | true | true | false + 'SQL' | 'WITH archived_rows AS (SELECT * FROM users WHERE archived = true) INSERT INTO products_log SELECT * FROM archived_rows' | true | true | false + 'SQL' | 'DELETE FROM users where id = 10' | true | true | false + 'SQL' | 'INSERT INTO project_ci_cd_settings (project_id) SELECT id FROM projects' | true | true | false + 'SQL' | 'UPDATE users SET admin = true WHERE id = 10' | true | true | false + 'CACHE' | 'SELECT * FROM users WHERE id = 10' | true | false | true + 'SCHEMA' | "SELECT attr.attname FROM pg_attribute attr INNER JOIN pg_constraint cons ON attr.attrelid = cons.conrelid AND attr.attnum = any(cons.conkey) WHERE cons.contype = 'p' AND cons.conrelid = '\"projects\"'::regclass" | false | false | false + nil | 'BEGIN' | false | false | false + nil | 'COMMIT' | false | false | false end - describe 'with a current transaction' do - it 'observes sql_duration metric' do - expect(subscriber).to receive(:current_transaction) - .at_least(:once) - .and_return(transaction) - expect(transaction).to receive(:observe).with(:gitlab_sql_duration_seconds, 0.002) - - subscriber.sql(event) - end + with_them do + let(:payload) { { name: name, sql: sql(sql_query, comments: comments), connection: connection } } it 'marks the current thread as using the database' do # since it would already have been toggled by other specs @@ -101,215 +53,20 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do expect { subscriber.sql(event) }.to change { Thread.current[:uses_db_connection] }.from(nil).to(true) end - context 'with read query' do - let(:expected_counters) do - { - db_count: 1, - db_write_count: 0, - db_cached_count: 0 - } - end - - it_behaves_like 'track query in metrics' - it_behaves_like 'track query in RequestStore' - - context 'with only select' do - let(:payload) { { sql: 'WITH active_milestones AS (SELECT COUNT(*), state FROM milestones GROUP BY state) SELECT * FROM active_milestones' } } - - it_behaves_like 'track query in metrics' - it_behaves_like 'track query in RequestStore' - end - end - - context 'write query' do - let(:expected_counters) do - { - db_count: 1, - db_write_count: 1, - db_cached_count: 0 - } - end - - context 'with select for update sql event' do - let(:payload) { { sql: 'SELECT * FROM users WHERE id = 10 FOR UPDATE' } } - - it_behaves_like 'track query in metrics' - it_behaves_like 'track query in RequestStore' - end - - context 'with common table expression' do - context 'with insert' do - let(:payload) { { sql: 'WITH archived_rows AS (SELECT * FROM users WHERE archived = true) INSERT INTO products_log SELECT * FROM archived_rows' } } - - it_behaves_like 'track query in metrics' - it_behaves_like 'track query in RequestStore' - end - end - - context 'with delete sql event' do - let(:payload) { { sql: 'DELETE FROM users where id = 10' } } - - it_behaves_like 'track query in metrics' - it_behaves_like 'track query in RequestStore' - end - - context 'with insert sql event' do - let(:payload) { { sql: 'INSERT INTO project_ci_cd_settings (project_id) SELECT id FROM projects' } } - - it_behaves_like 'track query in metrics' - it_behaves_like 'track query in RequestStore' - end - - context 'with update sql event' do - let(:payload) { { sql: 'UPDATE users SET admin = true WHERE id = 10' } } - - it_behaves_like 'track query in metrics' - it_behaves_like 'track query in RequestStore' - end - end - - context 'with cached query' do - let(:expected_counters) do - { - db_count: 1, - db_write_count: 0, - db_cached_count: 1 - } - end - - context 'with cached payload ' do - let(:payload) do - { - sql: 'SELECT * FROM users WHERE id = 10', - cached: true - } - end - - it_behaves_like 'track query in metrics' - it_behaves_like 'track query in RequestStore' - end - - context 'with cached payload name' do - let(:payload) do - { - sql: 'SELECT * FROM users WHERE id = 10', - name: 'CACHE' - } - end - - it_behaves_like 'track query in metrics' - it_behaves_like 'track query in RequestStore' - end - end - - context 'events are internal to Rails or irrelevant' do - let(:schema_event) do - double( - :event, - name: 'sql.active_record', - payload: { - sql: "SELECT attr.attname FROM pg_attribute attr INNER JOIN pg_constraint cons ON attr.attrelid = cons.conrelid AND attr.attnum = any(cons.conkey) WHERE cons.contype = 'p' AND cons.conrelid = '\"projects\"'::regclass", - name: 'SCHEMA', - connection_id: 135, - statement_name: nil, - binds: [] - }, - duration: 0.7 - ) - end - - let(:begin_event) do - double( - :event, - name: 'sql.active_record', - payload: { - sql: "BEGIN", - name: nil, - connection_id: 231, - statement_name: nil, - binds: [] - }, - duration: 1.1 - ) - end - - let(:commit_event) do - double( - :event, - name: 'sql.active_record', - payload: { - sql: "COMMIT", - name: nil, - connection_id: 212, - statement_name: nil, - binds: [] - }, - duration: 1.6 - ) - end - - it 'skips schema/begin/commit sql commands' do - allow(subscriber).to receive(:current_transaction) - .at_least(:once) - .and_return(transaction) - - expect(transaction).not_to receive(:increment) - - subscriber.sql(schema_event) - subscriber.sql(begin_event) - subscriber.sql(commit_event) - end - end + it_behaves_like 'record ActiveRecord metrics' + it_behaves_like 'store ActiveRecord info in RequestStore' end end - describe 'self.db_counter_payload' do - before do - allow(subscriber).to receive(:current_transaction) - .at_least(:once) - .and_return(transaction) - end - - context 'when RequestStore is enabled', :request_store do - context 'when query is executed' do - let(:expected_payload) do - { - db_count: 1, - db_cached_count: 0, - db_write_count: 0 - } - end - - it 'returns correct payload' do - subscriber.sql(event) - - expect(described_class.db_counter_payload).to eq(expected_payload) - end - end + context 'without Marginalia comments' do + let(:comments) { false } - context 'when query is not executed' do - let(:expected_payload) do - { - db_count: 0, - db_cached_count: 0, - db_write_count: 0 - } - end - - it 'returns correct payload' do - expect(described_class.db_counter_payload).to eq(expected_payload) - end - end - end - - context 'when RequestStore is disabled' do - let(:expected_payload) { {} } + it_behaves_like 'track generic sql events' + end - it 'returns empty payload' do - subscriber.sql(event) + context 'with Marginalia comments' do + let(:comments) { true } - expect(described_class.db_counter_payload).to eq(expected_payload) - end - end + it_behaves_like 'track generic sql events' end end |