diff options
Diffstat (limited to 'spec/lib/gitlab/metrics')
8 files changed, 297 insertions, 177 deletions
diff --git a/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb b/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb index ff8f5797f9d..c15e717b126 100644 --- a/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/importers/prometheus_metrics_spec.rb @@ -12,12 +12,6 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do subject { described_class.new(dashboard_hash, project: project, dashboard_path: dashboard_path) } - before do - allow_next_instance_of(::Clusters::Applications::ScheduleUpdateService) do |update_service| - allow(update_service).to receive(:execute) - end - end - context 'valid dashboard' do let(:dashboard_hash) { load_sample_dashboard } @@ -45,13 +39,6 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do create(:prometheus_metric, existing_metric_attributes) end - let!(:existing_alert) do - alert = create(:prometheus_alert, project: project, prometheus_metric: existing_metric) - existing_metric.prometheus_alerts << alert - - alert - end - it 'updates existing PrometheusMetrics' do subject.execute @@ -68,15 +55,6 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do expect { subject.execute }.to change { PrometheusMetric.count }.by(2) end - it 'updates affected environments' do - expect(::Clusters::Applications::ScheduleUpdateService).to receive(:new).with( - existing_alert.environment.cluster_prometheus_adapter, - project - ).and_return(double('ScheduleUpdateService', execute: true)) - - subject.execute - end - context 'with stale metrics' do let!(:stale_metric) do create(:prometheus_metric, @@ -87,13 +65,6 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do ) end - let!(:stale_alert) do - alert = create(:prometheus_alert, project: project, prometheus_metric: stale_metric) - stale_metric.prometheus_alerts << alert - - alert - end - it 'updates existing PrometheusMetrics' do subject.execute @@ -111,21 +82,6 @@ RSpec.describe Gitlab::Metrics::Dashboard::Importers::PrometheusMetrics do expect { stale_metric.reload }.to raise_error(ActiveRecord::RecordNotFound) end - - it 'deletes stale alert' do - subject.execute - - expect { stale_alert.reload }.to raise_error(ActiveRecord::RecordNotFound) - end - - it 'updates affected environments' do - expect(::Clusters::Applications::ScheduleUpdateService).to receive(:new).with( - existing_alert.environment.cluster_prometheus_adapter, - project - ).and_return(double('ScheduleUpdateService', execute: true)) - - subject.execute - end end end end diff --git a/spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb b/spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb index c7afc02f0af..66fba7ab683 100644 --- a/spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb +++ b/spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb @@ -152,8 +152,6 @@ RSpec.describe Gitlab::Metrics::Exporter::BaseExporter do where(:method_class, :path, :http_status) do Net::HTTP::Get | '/metrics' | 200 - Net::HTTP::Get | '/liveness' | 200 - Net::HTTP::Get | '/readiness' | 200 Net::HTTP::Get | '/' | 404 end diff --git a/spec/lib/gitlab/metrics/exporter/health_checks_middleware_spec.rb b/spec/lib/gitlab/metrics/exporter/health_checks_middleware_spec.rb deleted file mode 100644 index 9ee46a45e7a..00000000000 --- a/spec/lib/gitlab/metrics/exporter/health_checks_middleware_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -# frozen_string_literal: true - -require 'fast_spec_helper' - -RSpec.describe Gitlab::Metrics::Exporter::HealthChecksMiddleware do - let(:app) { double(:app) } - let(:env) { { 'PATH_INFO' => path } } - - let(:readiness_probe) { double(:readiness_probe) } - let(:liveness_probe) { double(:liveness_probe) } - let(:probe_result) { Gitlab::HealthChecks::Probes::Status.new(200, { status: 'ok' }) } - - subject(:middleware) { described_class.new(app, readiness_probe, liveness_probe) } - - describe '#call' do - context 'handling /readiness requests' do - let(:path) { '/readiness' } - - it 'handles the request' do - expect(readiness_probe).to receive(:execute).and_return(probe_result) - - response = middleware.call(env) - - expect(response).to eq([200, { 'Content-Type' => 'application/json; charset=utf-8' }, ['{"status":"ok"}']]) - end - end - - context 'handling /liveness requests' do - let(:path) { '/liveness' } - - it 'handles the request' do - expect(liveness_probe).to receive(:execute).and_return(probe_result) - - response = middleware.call(env) - - expect(response).to eq([200, { 'Content-Type' => 'application/json; charset=utf-8' }, ['{"status":"ok"}']]) - end - end - - context 'handling other requests' do - let(:path) { '/other_path' } - - it 'forwards them to the next middleware' do - expect(app).to receive(:call).with(env).and_return([201, {}, []]) - - response = middleware.call(env) - - expect(response).to eq([201, {}, []]) - end - end - end -end diff --git a/spec/lib/gitlab/metrics/methods_spec.rb b/spec/lib/gitlab/metrics/methods_spec.rb index 71135a6e9c5..eb7c8891e98 100644 --- a/spec/lib/gitlab/metrics/methods_spec.rb +++ b/spec/lib/gitlab/metrics/methods_spec.rb @@ -35,7 +35,7 @@ RSpec.describe Gitlab::Metrics::Methods do context 'metric is not cached' do it 'calls fetch_metric' do - expect(subject).to receive(:init_metric).with(metric_type, metric_name, docstring: docstring) + expect(subject).to receive(:init_metric).with(metric_type, metric_name, { docstring: docstring }) subject.public_send(metric_name) end diff --git a/spec/lib/gitlab/metrics/rails_slis_spec.rb b/spec/lib/gitlab/metrics/rails_slis_spec.rb index 2ba06316507..b30eb57101f 100644 --- a/spec/lib/gitlab/metrics/rails_slis_spec.rb +++ b/spec/lib/gitlab/metrics/rails_slis_spec.rb @@ -36,18 +36,8 @@ RSpec.describe Gitlab::Metrics::RailsSlis do } end - expect(Gitlab::Metrics::Sli).to receive(:initialized?).with(:rails_request_apdex) { false } - expect(Gitlab::Metrics::Sli).to receive(:initialized?).with(:graphql_query_apdex) { false } - expect(Gitlab::Metrics::Sli).to receive(:initialize_sli).with(:rails_request_apdex, array_including(*possible_labels)).and_call_original - expect(Gitlab::Metrics::Sli).to receive(:initialize_sli).with(:graphql_query_apdex, array_including(*possible_graphql_labels)).and_call_original - - described_class.initialize_request_slis! - end - - it 'does not initialize the SLI if they were initialized already', :aggregate_failures do - expect(Gitlab::Metrics::Sli).to receive(:initialized?).with(:rails_request_apdex) { true } - expect(Gitlab::Metrics::Sli).to receive(:initialized?).with(:graphql_query_apdex) { true } - expect(Gitlab::Metrics::Sli).not_to receive(:initialize_sli) + expect(Gitlab::Metrics::Sli::Apdex).to receive(:initialize_sli).with(:rails_request, array_including(*possible_labels)).and_call_original + expect(Gitlab::Metrics::Sli::Apdex).to receive(:initialize_sli).with(:graphql_query, array_including(*possible_graphql_labels)).and_call_original described_class.initialize_request_slis! end diff --git a/spec/lib/gitlab/metrics/sli_spec.rb b/spec/lib/gitlab/metrics/sli_spec.rb index 8ba4bf29568..9b776d6738d 100644 --- a/spec/lib/gitlab/metrics/sli_spec.rb +++ b/spec/lib/gitlab/metrics/sli_spec.rb @@ -10,72 +10,151 @@ RSpec.describe Gitlab::Metrics::Sli do end describe 'Class methods' do - before do - described_class.instance_variable_set(:@known_slis, nil) + it 'does not allow them to be called on the parent module' do + expect(described_class).not_to respond_to(:[]) + expect(described_class).not_to respond_to(:initialize_sli) end - describe '.[]' do - it 'warns about an uninitialized SLI but returns and stores a new one' do - sli = described_class[:bar] + it 'allows different SLIs to be defined on each subclass' do + apdex_counters = [ + fake_total_counter('foo', 'apdex'), + fake_numerator_counter('foo', 'apdex', 'success') + ] - expect(described_class[:bar]).to be(sli) - end + error_rate_counters = [ + fake_total_counter('foo', 'error_rate'), + fake_numerator_counter('foo', 'error_rate', 'error') + ] - it 'returns the same object for multiple accesses' do - sli = described_class.initialize_sli(:huzzah, []) + apdex = described_class::Apdex.initialize_sli(:foo, [{ hello: :world }]) - 2.times do - expect(described_class[:huzzah]).to be(sli) - end - end - end + expect(apdex_counters).to all(have_received(:get).with(hello: :world)) - describe '.initialized?' do - before do - fake_total_counter(:boom) - fake_success_counter(:boom) - end + error_rate = described_class::ErrorRate.initialize_sli(:foo, [{ other: :labels }]) - it 'is true when an SLI was initialized with labels' do - expect { described_class.initialize_sli(:boom, [{ hello: :world }]) } - .to change { described_class.initialized?(:boom) }.from(false).to(true) - end + expect(error_rate_counters).to all(have_received(:get).with(other: :labels)) - it 'is false when an SLI was not initialized with labels' do - expect { described_class.initialize_sli(:boom, []) } - .not_to change { described_class.initialized?(:boom) }.from(false) - end + expect(described_class::Apdex[:foo]).to be(apdex) + expect(described_class::ErrorRate[:foo]).to be(error_rate) end end - describe '#initialize_counters' do - it 'initializes counters for the passed label combinations' do - counters = [fake_total_counter(:hey), fake_success_counter(:hey)] + subclasses = { + Gitlab::Metrics::Sli::Apdex => :success, + Gitlab::Metrics::Sli::ErrorRate => :error + } - described_class.new(:hey).initialize_counters([{ foo: 'bar' }, { foo: 'baz' }]) + subclasses.each do |subclass, numerator_type| + subclass_type = subclass.to_s.demodulize.underscore - expect(counters).to all(have_received(:get).with({ foo: 'bar' })) - expect(counters).to all(have_received(:get).with({ foo: 'baz' })) - end - end + describe subclass do + describe 'Class methods' do + before do + described_class.instance_variable_set(:@known_slis, nil) + end - describe "#increment" do - let!(:sli) { described_class.new(:heyo) } - let!(:total_counter) { fake_total_counter(:heyo) } - let!(:success_counter) { fake_success_counter(:heyo) } + describe '.[]' do + it 'returns and stores a new, uninitialized SLI' do + sli = described_class[:bar] - it 'increments both counters for labels successes' do - sli.increment(labels: { hello: "world" }, success: true) + expect(described_class[:bar]).to be(sli) + expect(described_class[:bar]).not_to be_initialized + end - expect(total_counter).to have_received(:increment).with({ hello: 'world' }) - expect(success_counter).to have_received(:increment).with({ hello: 'world' }) - end + it 'returns the same object for multiple accesses' do + sli = described_class.initialize_sli(:huzzah, []) + + 2.times do + expect(described_class[:huzzah]).to be(sli) + end + end + end + + describe '.initialize_sli' do + it 'returns and stores a new initialized SLI' do + counters = [ + fake_total_counter(:bar, subclass_type), + fake_numerator_counter(:bar, subclass_type, numerator_type) + ] + + sli = described_class.initialize_sli(:bar, [{ hello: :world }]) + + expect(sli).to be_initialized + expect(counters).to all(have_received(:get).with(hello: :world)) + expect(counters).to all(have_received(:get).with(hello: :world)) + end + + it 'does not change labels for an already-initialized SLI' do + counters = [ + fake_total_counter(:bar, subclass_type), + fake_numerator_counter(:bar, subclass_type, numerator_type) + ] + + sli = described_class.initialize_sli(:bar, [{ hello: :world }]) - it 'only increments the total counters for labels when not successful' do - sli.increment(labels: { hello: "world" }, success: false) + expect(sli).to be_initialized + expect(counters).to all(have_received(:get).with(hello: :world)) + expect(counters).to all(have_received(:get).with(hello: :world)) - expect(total_counter).to have_received(:increment).with({ hello: 'world' }) - expect(success_counter).not_to have_received(:increment).with({ hello: 'world' }) + counters.each do |counter| + expect(counter).not_to receive(:get) + end + + expect(described_class.initialize_sli(:bar, [{ other: :labels }])).to eq(sli) + end + end + + describe '.initialized?' do + before do + fake_total_counter(:boom, subclass_type) + fake_numerator_counter(:boom, subclass_type, numerator_type) + end + + it 'is true when an SLI was initialized with labels' do + expect { described_class.initialize_sli(:boom, [{ hello: :world }]) } + .to change { described_class.initialized?(:boom) }.from(false).to(true) + end + + it 'is false when an SLI was not initialized with labels' do + expect { described_class.initialize_sli(:boom, []) } + .not_to change { described_class.initialized?(:boom) }.from(false) + end + end + end + + describe '#initialize_counters' do + it 'initializes counters for the passed label combinations' do + counters = [ + fake_total_counter(:hey, subclass_type), + fake_numerator_counter(:hey, subclass_type, numerator_type) + ] + + described_class.new(:hey).initialize_counters([{ foo: 'bar' }, { foo: 'baz' }]) + + expect(counters).to all(have_received(:get).with({ foo: 'bar' })) + expect(counters).to all(have_received(:get).with({ foo: 'baz' })) + end + end + + describe "#increment" do + let!(:sli) { described_class.new(:heyo) } + let!(:total_counter) { fake_total_counter(:heyo, subclass_type) } + let!(:numerator_counter) { fake_numerator_counter(:heyo, subclass_type, numerator_type) } + + it "increments both counters for labels when #{numerator_type} is true" do + sli.increment(labels: { hello: "world" }, numerator_type => true) + + expect(total_counter).to have_received(:increment).with({ hello: 'world' }) + expect(numerator_counter).to have_received(:increment).with({ hello: 'world' }) + end + + it "only increments the total counters for labels when #{numerator_type} is false" do + sli.increment(labels: { hello: "world" }, numerator_type => false) + + expect(total_counter).to have_received(:increment).with({ hello: 'world' }) + expect(numerator_counter).not_to have_received(:increment).with({ hello: 'world' }) + end + end end end @@ -89,11 +168,11 @@ RSpec.describe Gitlab::Metrics::Sli do fake_counter end - def fake_total_counter(name) - fake_prometheus_counter("gitlab_sli:#{name}:total") + def fake_total_counter(name, type) + fake_prometheus_counter("gitlab_sli:#{name}_#{type}:total") end - def fake_success_counter(name) - fake_prometheus_counter("gitlab_sli:#{name}:success_total") + def fake_numerator_counter(name, type, numerator_name) + fake_prometheus_counter("gitlab_sli:#{name}_#{type}:#{numerator_name}_total") 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 389b0ef1044..28c3ef229ab 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -10,6 +10,124 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do let(:connection) { ActiveRecord::Base.retrieve_connection } let(:db_config_name) { ::Gitlab::Database.db_config_name(connection) } + describe '.load_balancing_metric_counter_keys' do + context 'multiple databases' do + before do + skip_if_multiple_databases_not_setup + end + + it 'has expected keys' do + expect(described_class.load_balancing_metric_counter_keys).to include( + :db_replica_count, + :db_primary_count, + :db_main_count, + :db_main_replica_count, + :db_ci_count, + :db_ci_replica_count, + :db_replica_cached_count, + :db_primary_cached_count, + :db_main_cached_count, + :db_main_replica_cached_count, + :db_ci_cached_count, + :db_ci_replica_cached_count, + :db_replica_wal_count, + :db_primary_wal_count, + :db_main_wal_count, + :db_main_replica_wal_count, + :db_ci_wal_count, + :db_ci_replica_wal_count, + :db_replica_wal_cached_count, + :db_primary_wal_cached_count, + :db_main_wal_cached_count, + :db_main_replica_wal_cached_count, + :db_ci_wal_cached_count, + :db_ci_replica_wal_cached_count + ) + end + end + + context 'single database' do + before do + skip_if_multiple_databases_are_setup + end + + it 'has expected keys' do + expect(described_class.load_balancing_metric_counter_keys).to include( + :db_replica_count, + :db_primary_count, + :db_main_count, + :db_main_replica_count, + :db_replica_cached_count, + :db_primary_cached_count, + :db_main_cached_count, + :db_main_replica_cached_count, + :db_replica_wal_count, + :db_primary_wal_count, + :db_main_wal_count, + :db_main_replica_wal_count, + :db_replica_wal_cached_count, + :db_primary_wal_cached_count, + :db_main_wal_cached_count, + :db_main_replica_wal_cached_count + ) + end + + it 'does not have ci keys' do + expect(described_class.load_balancing_metric_counter_keys).not_to include( + :db_ci_count, + :db_ci_replica_count, + :db_ci_cached_count, + :db_ci_replica_cached_count, + :db_ci_wal_count, + :db_ci_replica_wal_count, + :db_ci_wal_cached_count, + :db_ci_replica_wal_cached_count + ) + end + end + end + + describe '.load_balancing_metric_duration_keys' do + context 'multiple databases' do + before do + skip_if_multiple_databases_not_setup + end + + it 'has expected keys' do + expect(described_class.load_balancing_metric_duration_keys).to include( + :db_replica_duration_s, + :db_primary_duration_s, + :db_main_duration_s, + :db_main_replica_duration_s, + :db_ci_duration_s, + :db_ci_replica_duration_s + ) + end + end + + context 'single database' do + before do + skip_if_multiple_databases_are_setup + end + + it 'has expected keys' do + expect(described_class.load_balancing_metric_duration_keys).to include( + :db_replica_duration_s, + :db_primary_duration_s, + :db_main_duration_s, + :db_main_replica_duration_s + ) + end + + it 'does not have ci keys' do + expect(described_class.load_balancing_metric_duration_keys).not_to include( + :db_ci_duration_s, + :db_ci_replica_duration_s + ) + end + end + end + describe '#transaction' do let(:web_transaction) { double('Gitlab::Metrics::WebTransaction') } let(:background_transaction) { double('Gitlab::Metrics::WebTransaction') } @@ -37,7 +155,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do end it 'captures the metrics for web only' do - expect(web_transaction).to receive(:observe).with(:gitlab_database_transaction_seconds, 0.23, db_config_name: db_config_name) + 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(:increment) @@ -77,7 +195,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ActiveRecord do end it 'captures the metrics for web only' do - expect(background_transaction).to receive(:observe).with(:gitlab_database_transaction_seconds, 0.23, db_config_name: db_config_name) + 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(:increment) diff --git a/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb b/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb index fda4b94bd78..9f939d0d7d6 100644 --- a/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb @@ -77,8 +77,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do end it 'logs request information' do - expect(Gitlab::AuthLogger).to receive(:error).with( - include( + expect(Gitlab::AuthLogger).to receive(:error) do |arguments| + expect(arguments).to include( message: 'Rack_Attack', env: match_type, remote_ip: '1.2.3.4', @@ -86,7 +86,14 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do path: '/api/v4/internal/authorized_keys', matched: 'throttle_unauthenticated' ) - ) + + if expected_status + expect(arguments).to include(status: expected_status) + else + expect(arguments).not_to have_key(:status) + end + end + subscriber.send(match_type, event) end end @@ -111,8 +118,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do end it 'logs request information and user id' do - expect(Gitlab::AuthLogger).to receive(:error).with( - include( + expect(Gitlab::AuthLogger).to receive(:error) do |arguments| + expect(arguments).to include( message: 'Rack_Attack', env: match_type, remote_ip: '1.2.3.4', @@ -121,7 +128,14 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do matched: 'throttle_authenticated_api', user_id: non_existing_record_id ) - ) + + if expected_status + expect(arguments).to include(status: expected_status) + else + expect(arguments).not_to have_key(:status) + end + end + subscriber.send(match_type, event) end end @@ -145,8 +159,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do end it 'logs request information and user meta' do - expect(Gitlab::AuthLogger).to receive(:error).with( - include( + expect(Gitlab::AuthLogger).to receive(:error) do |arguments| + expect(arguments).to include( message: 'Rack_Attack', env: match_type, remote_ip: '1.2.3.4', @@ -156,7 +170,14 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do user_id: user.id, 'meta.user' => user.username ) - ) + + if expected_status + expect(arguments).to include(status: expected_status) + else + expect(arguments).not_to have_key(:status) + end + end + subscriber.send(match_type, event) end end @@ -182,8 +203,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do end it 'logs request information and user meta' do - expect(Gitlab::AuthLogger).to receive(:error).with( - include( + expect(Gitlab::AuthLogger).to receive(:error) do |arguments| + expect(arguments).to include( message: 'Rack_Attack', env: match_type, remote_ip: '1.2.3.4', @@ -192,7 +213,14 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do matched: 'throttle_authenticated_api', deploy_token_id: deploy_token.id ) - ) + + if expected_status + expect(arguments).to include(status: expected_status) + else + expect(arguments).not_to have_key(:status) + end + end + subscriber.send(match_type, event) end end @@ -202,6 +230,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do describe '#throttle' do let(:match_type) { :throttle } + let(:expected_status) { 429 } let(:event_name) { 'throttle.rack_attack' } it_behaves_like 'log into auth logger' @@ -209,6 +238,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do describe '#blocklist' do let(:match_type) { :blocklist } + let(:expected_status) { 403 } let(:event_name) { 'blocklist.rack_attack' } it_behaves_like 'log into auth logger' @@ -216,6 +246,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do describe '#track' do let(:match_type) { :track } + let(:expected_status) { nil } let(:event_name) { 'track.rack_attack' } it_behaves_like 'log into auth logger' |