diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-06-18 14:18:50 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-06-18 14:18:50 +0300 |
commit | 8c7f4e9d5f36cff46365a7f8c4b9c21578c1e781 (patch) | |
tree | a77e7fe7a93de11213032ed4ab1f33a3db51b738 /spec/lib/gitlab/metrics | |
parent | 00b35af3db1abfe813a778f643dad221aad51fca (diff) |
Add latest changes from gitlab-org/gitlab@13-1-stable-ee
Diffstat (limited to 'spec/lib/gitlab/metrics')
-rw-r--r-- | spec/lib/gitlab/metrics/dashboard/finder_spec.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/dashboard/processor_spec.rb | 24 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb | 57 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/method_call_spec.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/methods_spec.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb | 61 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb | 12 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb | 12 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb | 12 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb | 20 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/subscribers/active_record_spec.rb | 132 | ||||
-rw-r--r-- | spec/lib/gitlab/metrics/transaction_spec.rb | 40 |
12 files changed, 349 insertions, 33 deletions
diff --git a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb index d772b0c7a5f..2703339d89c 100644 --- a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb @@ -142,7 +142,7 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi describe '.find_all_paths' do let(:all_dashboard_paths) { described_class.find_all_paths(project) } - let(:system_dashboard) { { path: system_dashboard_path, display_name: 'Default', default: true, system_dashboard: true } } + let(:system_dashboard) { { path: system_dashboard_path, display_name: 'Default dashboard', default: true, system_dashboard: true } } it 'includes only the system dashboard by default' do expect(all_dashboard_paths).to eq([system_dashboard]) @@ -163,7 +163,7 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi let(:self_monitoring_dashboard) do { path: self_monitoring_dashboard_path, - display_name: 'Default', + display_name: 'Default dashboard', default: true, system_dashboard: false } diff --git a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb index b2fca0b5954..7250cefb9ff 100644 --- a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb @@ -16,7 +16,8 @@ describe Gitlab::Metrics::Dashboard::Processor do Gitlab::Metrics::Dashboard::Stages::EndpointInserter, Gitlab::Metrics::Dashboard::Stages::Sorter, Gitlab::Metrics::Dashboard::Stages::AlertsInserter, - Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter + Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter, + Gitlab::Metrics::Dashboard::Stages::UrlValidator ] end @@ -201,6 +202,27 @@ describe Gitlab::Metrics::Dashboard::Processor do it_behaves_like 'errors with message', 'Each "metric" must define one of :query or :query_range' end + + describe 'validating links' do + context 'when the links contain a blocked url' do + let(:dashboard_yml_links) do + [{ 'url' => 'http://1.1.1.1.1' }, { 'url' => 'https://gitlab.com' }] + end + + let(:expected) do + [{ url: '' }, { url: 'https://gitlab.com' }] + end + + before do + stub_env('RSPEC_ALLOW_INVALID_URLS', 'false') + dashboard_yml['links'] = dashboard_yml_links + end + + it 'replaces the blocked url with an empty string' do + expect(dashboard[:links]).to eq(expected) + end + end + end end private diff --git a/spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb new file mode 100644 index 00000000000..305768ef060 --- /dev/null +++ b/spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Metrics::ElasticsearchRackMiddleware do + let(:app) { double(:app, call: 'app call result') } + let(:middleware) { described_class.new(app) } + let(:env) { {} } + let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } + + describe '#call' do + let(:counter) { instance_double(Prometheus::Client::Counter, increment: nil) } + let(:histogram) { instance_double(Prometheus::Client::Histogram, observe: nil) } + let(:elasticsearch_query_time) { 0.1 } + let(:elasticsearch_requests_count) { 2 } + + before do + allow(Gitlab::Instrumentation::ElasticsearchTransport).to receive(:query_time) { elasticsearch_query_time } + allow(Gitlab::Instrumentation::ElasticsearchTransport).to receive(:get_request_count) { elasticsearch_requests_count } + + allow(Gitlab::Metrics).to receive(:counter) + .with(:http_elasticsearch_requests_total, + an_instance_of(String), + Gitlab::Metrics::Transaction::BASE_LABELS) + .and_return(counter) + + allow(Gitlab::Metrics).to receive(:histogram) + .with(:http_elasticsearch_requests_duration_seconds, + an_instance_of(String), + Gitlab::Metrics::Transaction::BASE_LABELS, + described_class::HISTOGRAM_BUCKETS) + .and_return(histogram) + + allow(Gitlab::Metrics).to receive(:current_transaction).and_return(transaction) + end + + it 'calls the app' do + expect(middleware.call(env)).to eq('app call result') + end + + it 'records elasticsearch metrics' do + expect(counter).to receive(:increment).with(transaction.labels, elasticsearch_requests_count) + expect(histogram).to receive(:observe).with(transaction.labels, elasticsearch_query_time) + + middleware.call(env) + end + + it 'records elasticsearch metrics if an error is raised' do + expect(counter).to receive(:increment).with(transaction.labels, elasticsearch_requests_count) + expect(histogram).to receive(:observe).with(transaction.labels, elasticsearch_query_time) + + allow(app).to receive(:call).with(env).and_raise(StandardError) + + expect { middleware.call(env) }.to raise_error(StandardError) + end + end +end diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index 229db67ec88..035d875258c 100644 --- a/spec/lib/gitlab/metrics/method_call_spec.rb +++ b/spec/lib/gitlab/metrics/method_call_spec.rb @@ -26,7 +26,7 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is enabled' do before do - Feature.get(:prometheus_metrics_method_instrumentation).enable + stub_feature_flags(prometheus_metrics_method_instrumentation: true) end around do |example| @@ -50,7 +50,7 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is disabled' do before do - Feature.get(:prometheus_metrics_method_instrumentation).disable + stub_feature_flags(prometheus_metrics_method_instrumentation: false) end it 'observes using NullMetric' do diff --git a/spec/lib/gitlab/metrics/methods_spec.rb b/spec/lib/gitlab/metrics/methods_spec.rb index bca94deb1d8..5cf8db55142 100644 --- a/spec/lib/gitlab/metrics/methods_spec.rb +++ b/spec/lib/gitlab/metrics/methods_spec.rb @@ -104,7 +104,7 @@ describe Gitlab::Metrics::Methods do context 'when feature is enabled' do before do - Feature.get(feature_name).enable + stub_feature_flags(feature_name => true) end it "initializes #{metric_type} metric" do @@ -118,7 +118,7 @@ describe Gitlab::Metrics::Methods do context 'when feature is disabled' do before do - Feature.get(feature_name).disable + stub_feature_flags(feature_name => false) end it "returns NullMetric" do diff --git a/spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb new file mode 100644 index 00000000000..f2f36ccad20 --- /dev/null +++ b/spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Metrics::RedisRackMiddleware do + let(:app) { double(:app) } + let(:middleware) { described_class.new(app) } + let(:env) { {} } + let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } + + before do + allow(app).to receive(:call).with(env).and_return('wub wub') + end + + describe '#call' do + let(:counter) { double(Prometheus::Client::Counter, increment: nil) } + let(:histogram) { double(Prometheus::Client::Histogram, observe: nil) } + let(:redis_query_time) { 0.1 } + let(:redis_requests_count) { 2 } + + before do + allow(Gitlab::Instrumentation::Redis).to receive(:query_time) { redis_query_time } + allow(Gitlab::Instrumentation::Redis).to receive(:get_request_count) { redis_requests_count } + + allow(Gitlab::Metrics).to receive(:counter) + .with(:http_redis_requests_total, + an_instance_of(String), + Gitlab::Metrics::Transaction::BASE_LABELS) + .and_return(counter) + + allow(Gitlab::Metrics).to receive(:histogram) + .with(:http_redis_requests_duration_seconds, + an_instance_of(String), + Gitlab::Metrics::Transaction::BASE_LABELS, + Gitlab::Instrumentation::Redis::QUERY_TIME_BUCKETS) + .and_return(histogram) + + allow(Gitlab::Metrics).to receive(:current_transaction).and_return(transaction) + end + + it 'calls the app' do + expect(middleware.call(env)).to eq('wub wub') + end + + it 'records redis metrics' do + expect(counter).to receive(:increment).with(transaction.labels, redis_requests_count) + expect(histogram).to receive(:observe).with(transaction.labels, redis_query_time) + + middleware.call(env) + end + + it 'records redis metrics if an error is raised' do + expect(counter).to receive(:increment).with(transaction.labels, redis_requests_count) + expect(histogram).to receive(:observe).with(transaction.labels, redis_query_time) + + allow(app).to receive(:call).with(env).and_raise(StandardError) + + expect { middleware.call(env) }.to raise_error(StandardError) + end + end +end diff --git a/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb index fdf3b5bd045..087a0bfbac5 100644 --- a/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/database_sampler_spec.rb @@ -3,7 +3,17 @@ require 'spec_helper' describe Gitlab::Metrics::Samplers::DatabaseSampler do - subject { described_class.new(described_class::SAMPLING_INTERVAL_SECONDS) } + subject { described_class.new } + + describe '#interval' do + it 'samples every five seconds by default' do + expect(subject.interval).to eq(5) + end + + it 'samples at other intervals if requested' do + expect(described_class.new(11).interval).to eq(11) + end + end describe '#sample' do before do diff --git a/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb index 1097d26c320..df63f2ebe28 100644 --- a/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/puma_sampler_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::Metrics::Samplers::PumaSampler do - subject { described_class.new(5) } + subject { described_class.new } let(:null_metric) { double('null_metric', set: nil, observe: nil) } @@ -11,6 +11,16 @@ describe Gitlab::Metrics::Samplers::PumaSampler do allow(Gitlab::Metrics::NullMetric).to receive(:instance).and_return(null_metric) end + describe '#interval' do + it 'samples every five seconds by default' do + expect(subject.interval).to eq(5) + end + + it 'samples at other intervals if requested' do + expect(described_class.new(11).interval).to eq(11) + end + end + describe '#sample' do before do expect(subject).to receive(:puma_stats).and_return(puma_stats) diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index ead650a27f0..9fc8dd10922 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::Metrics::Samplers::RubySampler do - let(:sampler) { described_class.new(5) } + let(:sampler) { described_class.new } let(:null_metric) { double('null_metric', set: nil, observe: nil) } before do @@ -18,6 +18,16 @@ describe Gitlab::Metrics::Samplers::RubySampler do end end + describe '#interval' do + it 'samples every sixty seconds by default' do + expect(subject.interval).to eq(60) + end + + it 'samples at other intervals if requested' do + expect(described_class.new(11).interval).to eq(11) + end + end + describe '#sample' do it 'adds a metric containing the process resident memory bytes' do expect(Gitlab::Metrics::System).to receive(:memory_usage_rss).and_return(9000) diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb index 67336cf83e6..ea9e8fa6795 100644 --- a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb @@ -10,17 +10,19 @@ describe Gitlab::Metrics::SidekiqMiddleware do it 'tracks the transaction' do worker = double(:worker, class: double(:class, name: 'TestWorker')) - expect(Gitlab::Metrics::BackgroundTransaction).to receive(:new) - .with(worker.class) - .and_call_original + expect_next_instance_of(Gitlab::Metrics::BackgroundTransaction) do |transaction| + expect(transaction).to receive(:set).with(:sidekiq_queue_duration, instance_of(Float)) + expect(transaction).to receive(:increment).with(:db_count, 1) + end - expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:set) - .with(:sidekiq_queue_duration, instance_of(Float)) + middleware.call(worker, message, :test) do + ActiveRecord::Base.connection.execute('SELECT pg_sleep(0.1);') + end - middleware.call(worker, message, :test) { nil } + expect(message).to include(:db_count, :db_write_count, :db_cached_count) end - it 'tracks the transaction (for messages without `enqueued_at`)' do + it 'tracks the transaction (for messages without `enqueued_at`)', :aggregate_failures do worker = double(:worker, class: double(:class, name: 'TestWorker')) expect(Gitlab::Metrics::BackgroundTransaction).to receive(:new) @@ -33,7 +35,7 @@ describe Gitlab::Metrics::SidekiqMiddleware do middleware.call(worker, {}, :test) { nil } end - it 'tracks any raised exceptions' do + it 'tracks any raised exceptions', :aggregate_failures do worker = double(:worker, class: double(:class, name: 'TestWorker')) expect_any_instance_of(Gitlab::Metrics::Transaction) @@ -44,6 +46,8 @@ describe Gitlab::Metrics::SidekiqMiddleware do expect { middleware.call(worker, message, :test) } .to raise_error(RuntimeError) + + expect(message).to include(:db_count, :db_write_count, :db_cached_count) 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 1624cea8bda..a78d048908d 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -6,10 +6,15 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do let(:env) { {} } let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) } let(:subscriber) { described_class.new } + let(:payload) { { sql: 'SELECT * FROM users WHERE id = 10' } } let(:event) do - double(:event, duration: 2, - payload: { sql: 'SELECT * FROM users WHERE id = 10' }) + double( + :event, + name: 'sql.active_record', + duration: 2, + payload: payload + ) end describe '#sql' do @@ -23,6 +28,63 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do end describe 'with a current transaction' do + shared_examples 'read only query' do + it 'increments only db count value' do + allow(subscriber).to receive(:current_transaction) + .at_least(:once) + .and_return(transaction) + + expect(transaction).to receive(:increment) + .with(:db_count, 1) + + expect(transaction).not_to receive(:increment) + .with(:db_cached_count, 1) + + expect(transaction).not_to receive(:increment) + .with(:db_write_count, 1) + + subscriber.sql(event) + end + end + + shared_examples 'write query' do + it 'increments db_write_count and db_count value' do + expect(subscriber).to receive(:current_transaction) + .at_least(:once) + .and_return(transaction) + + expect(transaction).to receive(:increment) + .with(:db_count, 1) + + expect(transaction).not_to receive(:increment) + .with(:db_cached_count, 1) + + expect(transaction).to receive(:increment) + .with(:db_write_count, 1) + + subscriber.sql(event) + end + end + + shared_examples 'cached query' do + it 'increments db_cached_count and db_count value' do + expect(subscriber).to receive(:current_transaction) + .at_least(:once) + .and_return(transaction) + + expect(transaction).to receive(:increment) + .with(:db_count, 1) + + expect(transaction).to receive(:increment) + .with(:db_cached_count, 1) + + expect(transaction).not_to receive(:increment) + .with(:db_write_count, 1) + + subscriber.sql(event) + end + end + it 'observes sql_duration metric' do expect(subscriber).to receive(:current_transaction) .at_least(:once) @@ -31,18 +93,66 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do subscriber.sql(event) end - it 'increments the :sql_duration value' do - expect(subscriber).to receive(:current_transaction) - .at_least(:once) - .and_return(transaction) + it_behaves_like 'read only query' + + context 'with select for update sql event' do + let(:payload) { { sql: 'SELECT * FROM users WHERE id = 10 FOR UPDATE' } } - expect(transaction).to receive(:increment) - .with(:sql_duration, 2, false) + it_behaves_like 'write query' + end - expect(transaction).to receive(:increment) - .with(:sql_count, 1, false) + 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' } } - subscriber.sql(event) + it_behaves_like 'write query' + end + + 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 'read only query' + end + end + + context 'with delete sql event' do + let(:payload) { { sql: 'DELETE FROM users where id = 10' } } + + it_behaves_like 'write query' + 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 'write query' + end + + context 'with update sql event' do + let(:payload) { { sql: 'UPDATE users SET admin = true WHERE id = 10' } } + + it_behaves_like 'write query' + end + + context 'with cached payload ' do + let(:payload) do + { + sql: 'SELECT * FROM users WHERE id = 10', + cached: true + } + end + + it_behaves_like 'cached query' + end + + context 'with cached payload name' do + let(:payload) do + { + sql: 'SELECT * FROM users WHERE id = 10', + name: 'CACHE' + } + end + + it_behaves_like 'cached query' end context 'events are internal to Rails or irrelevant' do diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index cf46fa3e91c..693ec3cb7e7 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -65,18 +65,16 @@ describe Gitlab::Metrics::Transaction do describe '#add_event' do let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, increment: nil) } - before do - allow(described_class).to receive(:transaction_metric).and_return(prometheus_metric) - end - it 'adds a metric' do expect(prometheus_metric).to receive(:increment) + expect(described_class).to receive(:fetch_metric).with(:counter, :gitlab_transaction_event_meow_total).and_return(prometheus_metric) transaction.add_event(:meow) end it 'allows tracking of custom tags' do expect(prometheus_metric).to receive(:increment).with(hash_including(animal: "dog")) + expect(described_class).to receive(:fetch_metric).with(:counter, :gitlab_transaction_event_bau_total).and_return(prometheus_metric) transaction.add_event(:bau, animal: 'dog') end @@ -84,6 +82,7 @@ describe Gitlab::Metrics::Transaction do context 'with sensitive tags' do before do transaction.add_event(:baubau, **sensitive_tags.merge(sane: 'yes')) + allow(described_class).to receive(:transaction_metric).and_return(prometheus_metric) end it 'filters tags' do @@ -93,4 +92,37 @@ describe Gitlab::Metrics::Transaction do end end end + + describe '#increment' do + let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, increment: nil) } + + it 'adds a metric' do + expect(prometheus_metric).to receive(:increment).with(hash_including(:action, :controller), 1) + expect(described_class).to receive(:fetch_metric).with(:counter, :gitlab_transaction_meow_total).and_return(prometheus_metric) + + transaction.increment(:meow, 1) + end + end + + describe '#set' do + let(:prometheus_metric) { instance_double(Prometheus::Client::Gauge, set: nil) } + + it 'adds a metric' do + expect(prometheus_metric).to receive(:set).with(hash_including(:action, :controller), 1) + expect(described_class).to receive(:fetch_metric).with(:gauge, :gitlab_transaction_meow_total).and_return(prometheus_metric) + + transaction.set(:meow, 1) + end + end + + describe '#get' do + let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, get: nil) } + + it 'gets a metric' do + expect(described_class).to receive(:fetch_metric).with(:counter, :gitlab_transaction_meow_total).and_return(prometheus_metric) + expect(prometheus_metric).to receive(:get) + + transaction.get(:meow, :counter) + end + end end |