diff options
Diffstat (limited to 'spec/lib/gitlab/metrics')
| -rw-r--r-- | spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb | 38 | ||||
| -rw-r--r-- | spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb | 141 | ||||
| -rw-r--r-- | spec/lib/gitlab/metrics/subscribers/active_record_spec.rb | 136 | ||||
| -rw-r--r-- | spec/lib/gitlab/metrics/transaction_spec.rb | 30 | ||||
| -rw-r--r-- | spec/lib/gitlab/metrics/web_transaction_spec.rb | 40 |
5 files changed, 203 insertions, 182 deletions
diff --git a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb index 1f7daaa308d..9d5c4bdf9e2 100644 --- a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb @@ -7,8 +7,16 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do subject { described_class.new(app) } + around do |example| + # Simulate application context middleware + # In fact, this middleware cleans up the contexts after a request lifecycle + ::Gitlab::ApplicationContext.with_context({}) do + example.run + end + end + describe '#call' do - let(:status) { 100 } + let(:status) { 200 } let(:env) { { 'REQUEST_METHOD' => 'GET' } } let(:stack_result) { [status, {}, 'body'] } @@ -71,6 +79,17 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do end end + context '@app.call returns an error code' do + let(:status) { '500' } + + it 'tracks count but not duration' do + expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '500', feature_category: 'unknown') + expect(described_class).not_to receive(:http_request_duration_seconds) + + subject.call(env) + end + end + context '@app.call throws exception' do let(:http_request_duration_seconds) { double('http_request_duration_seconds') } let(:http_requests_total) { double('http_requests_total') } @@ -91,9 +110,9 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do end context 'feature category header' do - context 'when a feature category header is present' do + context 'when a feature category context is present' do before do - allow(app).to receive(:call).and_return([200, { described_class::FEATURE_CATEGORY_HEADER => 'issue_tracking' }, nil]) + ::Gitlab::ApplicationContext.push(feature_category: 'issue_tracking') end it 'adds the feature category to the labels for http_requests_total' do @@ -113,11 +132,20 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do end end - context 'when the feature category header is an empty string' do + context 'when application raises an exception when the feature category context is present' do before do - allow(app).to receive(:call).and_return([200, { described_class::FEATURE_CATEGORY_HEADER => '' }, nil]) + ::Gitlab::ApplicationContext.push(feature_category: 'issue_tracking') + allow(app).to receive(:call).and_raise(StandardError) end + it 'adds the feature category to the labels for http_requests_total' do + expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: 'undefined', feature_category: 'issue_tracking') + + expect { subject.call(env) }.to raise_error(StandardError) + end + end + + context 'when the feature category context is not available' do it 'sets the feature category to unknown' do expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '200', feature_category: 'unknown') expect(described_class).not_to receive(:http_health_requests_total) diff --git a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb deleted file mode 100644 index 7971a7cabd5..00000000000 --- a/spec/lib/gitlab/metrics/samplers/unicorn_sampler_spec.rb +++ /dev/null @@ -1,141 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Metrics::Samplers::UnicornSampler do - subject { described_class.new(1.second) } - - it_behaves_like 'metrics sampler', 'UNICORN_SAMPLER' - - describe '#sample' do - let(:unicorn) { Module.new } - let(:raindrops) { double('raindrops') } - let(:stats) { double('stats') } - - before do - stub_const('Unicorn', unicorn) - stub_const('Raindrops::Linux', raindrops) - allow(raindrops).to receive(:unix_listener_stats).and_return({}) - allow(raindrops).to receive(:tcp_listener_stats).and_return({}) - end - - context 'unicorn listens on unix sockets' do - let(:socket_address) { '/some/sock' } - let(:sockets) { [socket_address] } - - before do - allow(unicorn).to receive(:listener_names).and_return(sockets) - end - - it 'samples socket data' do - expect(raindrops).to receive(:unix_listener_stats).with(sockets) - - subject.sample - end - - context 'stats collected' do - before do - allow(stats).to receive(:active).and_return('active') - allow(stats).to receive(:queued).and_return('queued') - allow(raindrops).to receive(:unix_listener_stats).and_return({ socket_address => stats }) - end - - it 'updates metrics type unix and with addr' do - labels = { socket_type: 'unix', socket_address: socket_address } - - expect(subject.metrics[:unicorn_active_connections]).to receive(:set).with(labels, 'active') - expect(subject.metrics[:unicorn_queued_connections]).to receive(:set).with(labels, 'queued') - - subject.sample - end - end - end - - context 'unicorn listens on tcp sockets' do - let(:tcp_socket_address) { '0.0.0.0:8080' } - let(:tcp_sockets) { [tcp_socket_address] } - - before do - allow(unicorn).to receive(:listener_names).and_return(tcp_sockets) - end - - it 'samples socket data' do - expect(raindrops).to receive(:tcp_listener_stats).with(tcp_sockets) - - subject.sample - end - - context 'stats collected' do - before do - allow(stats).to receive(:active).and_return('active') - allow(stats).to receive(:queued).and_return('queued') - allow(raindrops).to receive(:tcp_listener_stats).and_return({ tcp_socket_address => stats }) - end - - it 'updates metrics type unix and with addr' do - labels = { socket_type: 'tcp', socket_address: tcp_socket_address } - - expect(subject.metrics[:unicorn_active_connections]).to receive(:set).with(labels, 'active') - expect(subject.metrics[:unicorn_queued_connections]).to receive(:set).with(labels, 'queued') - - subject.sample - end - end - end - - context 'unicorn workers' do - before do - allow(unicorn).to receive(:listener_names).and_return([]) - end - - context 'without http server' do - it "does set unicorn_workers to 0" do - expect(subject.metrics[:unicorn_workers]).to receive(:set).with({}, 0) - - subject.sample - end - end - - context 'with http server' do - let(:http_server_class) { Struct.new(:worker_processes) } - let!(:http_server) { http_server_class.new(5) } - - before do - stub_const('Unicorn::HttpServer', http_server_class) - end - - it "sets additional metrics" do - expect(subject.metrics[:unicorn_workers]).to receive(:set).with({}, 5) - - subject.sample - end - end - end - end - - describe '#start' do - context 'when enabled' do - before do - allow(subject).to receive(:enabled?).and_return(true) - end - - it 'creates new thread' do - expect(Thread).to receive(:new) - - subject.start - end - end - - context 'when disabled' do - before do - allow(subject).to receive(:enabled?).and_return(false) - end - - it "doesn't create new thread" do - expect(Thread).not_to receive(:new) - - subject.start - end - end - end -end diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb index 6bfcfa21289..cffa62c3a52 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -150,4 +150,140 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do it_behaves_like 'track generic sql events' end end + + context 'Database Load Balancing enabled' do + let(:payload) { { sql: 'SELECT * FROM users WHERE id = 10', connection: connection } } + + let(:event) do + double( + :event, + name: 'sql.active_record', + duration: 2, + payload: payload + ) + 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 sql events for each role' do + where(:name, :sql_query, :record_query, :record_write_query, :record_cached_query, :record_wal_query) do + 'SQL' | 'SELECT * FROM users WHERE id = 10' | true | false | false | false + 'SQL' | 'WITH active_milestones AS (SELECT COUNT(*), state FROM milestones GROUP BY state) SELECT * FROM active_milestones' | true | false | false | false + 'SQL' | 'SELECT * FROM users WHERE id = 10 FOR UPDATE' | true | true | false | false + 'SQL' | 'WITH archived_rows AS (SELECT * FROM users WHERE archived = true) INSERT INTO products_log SELECT * FROM archived_rows' | true | true | false | false + 'SQL' | 'DELETE FROM users where id = 10' | true | true | false | false + 'SQL' | 'INSERT INTO project_ci_cd_settings (project_id) SELECT id FROM projects' | true | true | false | false + 'SQL' | 'UPDATE users SET admin = true WHERE id = 10' | true | true | false | false + 'SQL' | 'SELECT pg_current_wal_insert_lsn()::text AS location' | true | false | false | true + 'SQL' | 'SELECT pg_last_wal_replay_lsn()::text AS location' | true | false | false | true + 'CACHE' | 'SELECT * FROM users WHERE id = 10' | true | false | true | false + '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 | false + nil | 'BEGIN' | false | false | false | false + nil | 'COMMIT' | false | false | false | false + end + + with_them do + let(:payload) { { name: name, sql: sql(sql_query, comments: comments), connection: connection } } + + before do + allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) + end + + context 'query using a connection to a replica' do + before do + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).and_return(:replica) + end + + it 'queries connection db role' do + subscriber.sql(event) + + if record_query + expect(Gitlab::Database::LoadBalancing).to have_received(:db_role_for_connection).with(connection) + end + end + + it_behaves_like 'record ActiveRecord metrics', :replica + it_behaves_like 'store ActiveRecord info in RequestStore', :replica + end + + context 'query using a connection to a primary' do + before do + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).and_return(:primary) + end + + it 'queries connection db role' do + subscriber.sql(event) + + if record_query + expect(Gitlab::Database::LoadBalancing).to have_received(:db_role_for_connection).with(connection) + end + end + + it_behaves_like 'record ActiveRecord metrics', :primary + it_behaves_like 'store ActiveRecord info in RequestStore', :primary + end + + context 'query using a connection to an unknown source' do + let(:transaction) { double('Gitlab::Metrics::WebTransaction') } + + before do + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).and_return(nil) + + allow(::Gitlab::Metrics::WebTransaction).to receive(:current).and_return(transaction) + allow(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(nil) + + allow(transaction).to receive(:increment) + allow(transaction).to receive(:observe) + end + + it 'does not record DB role metrics' do + expect(transaction).not_to receive(:increment).with("gitlab_transaction_db_primary_count_total".to_sym, any_args) + expect(transaction).not_to receive(:increment).with("gitlab_transaction_db_replica_count_total".to_sym, any_args) + + expect(transaction).not_to receive(:increment).with("gitlab_transaction_db_primary_cached_count_total".to_sym, any_args) + expect(transaction).not_to receive(:increment).with("gitlab_transaction_db_replica_cached_count_total".to_sym, any_args) + + expect(transaction).not_to receive(:observe).with("gitlab_sql_primary_duration_seconds".to_sym, any_args) + expect(transaction).not_to receive(:observe).with("gitlab_sql_replica_duration_seconds".to_sym, any_args) + + subscriber.sql(event) + end + + it 'does not store DB roles into into RequestStore' do + Gitlab::WithRequestStore.with_request_store do + subscriber.sql(event) + + expect(described_class.db_counter_payload).to include( + db_primary_cached_count: 0, + db_primary_count: 0, + db_primary_duration_s: 0, + db_replica_cached_count: 0, + db_replica_count: 0, + db_replica_duration_s: 0 + ) + end + end + end + end + end + + context 'without Marginalia comments' do + let(:comments) { false } + + it_behaves_like 'track sql events for each role' + end + + context 'with Marginalia comments' do + let(:comments) { true } + + it_behaves_like 'track sql events for each role' + end + end end diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index d4e5a1a94f2..2ff8efcd7cb 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -12,32 +12,6 @@ RSpec.describe Gitlab::Metrics::Transaction do } end - describe '#duration' do - it 'returns the duration of a transaction in seconds' do - transaction.run { } - - expect(transaction.duration).to be > 0 - end - end - - describe '#run' do - it 'yields the supplied block' do - expect { |b| transaction.run(&b) }.to yield_control - end - - it 'stores the transaction in the current thread' do - transaction.run do - expect(described_class.current).to eq(transaction) - end - end - - it 'removes the transaction from the current thread upon completion' do - transaction.run { } - - expect(described_class.current).to be_nil - end - end - describe '#method_call_for' do it 'returns a MethodCall' do method = transaction.method_call_for('Foo#bar', :Foo, '#bar') @@ -46,6 +20,10 @@ RSpec.describe Gitlab::Metrics::Transaction do end end + describe '#run' do + specify { expect { transaction.run }.to raise_error(NotImplementedError) } + end + describe '#add_event' do let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, increment: nil, base_labels: {}) } diff --git a/spec/lib/gitlab/metrics/web_transaction_spec.rb b/spec/lib/gitlab/metrics/web_transaction_spec.rb index 6ee9564ef75..5261d04c879 100644 --- a/spec/lib/gitlab/metrics/web_transaction_spec.rb +++ b/spec/lib/gitlab/metrics/web_transaction_spec.rb @@ -38,16 +38,6 @@ RSpec.describe Gitlab::Metrics::WebTransaction do end end - describe '#duration' do - include_context 'transaction observe metrics' - - it 'returns the duration of a transaction in seconds' do - transaction.run { sleep(0.5) } - - expect(transaction.duration).to be >= 0.5 - end - end - describe '#run' do include_context 'transaction observe metrics' @@ -58,6 +48,9 @@ RSpec.describe Gitlab::Metrics::WebTransaction do it 'stores the transaction in the current thread' do transaction.run do expect(Thread.current[described_class::THREAD_KEY]).to eq(transaction) + expect(described_class.current).to eq(transaction) + + ['200', {}, ''] end end @@ -65,6 +58,33 @@ RSpec.describe Gitlab::Metrics::WebTransaction do transaction.run { } expect(Thread.current[described_class::THREAD_KEY]).to be_nil + expect(described_class.current).to be_nil + end + + it 'records the duration of the transaction if the request was successful' do + expect(transaction).to receive(:observe).with(:gitlab_transaction_duration_seconds, instance_of(Float)) + + transaction.run { ['200', {}, ''] } + end + + it 'does not record the duration of the transaction if the request failed' do + expect(transaction).not_to receive(:observe).with(:gitlab_transaction_duration_seconds, instance_of(Float)) + + transaction.run { ['500', {}, ''] } + end + + it 'does not record the duration of the transaction if it raised' do + expect(transaction).not_to receive(:observe).with(:gitlab_transaction_duration_seconds, instance_of(Float)) + + expect do + transaction.run { raise 'broken' } + end.to raise_error('broken') + end + + it 'returns the rack response' do + response = ['500', {}, ''] + + expect(transaction.run { response }).to eq(response) end end |