diff options
Diffstat (limited to 'spec/lib/gitlab')
20 files changed, 117 insertions, 249 deletions
diff --git a/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb b/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb index 18ac8a7b42b..2064a592246 100644 --- a/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb +++ b/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb @@ -7,20 +7,12 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, featu let(:supported_dast_versions) { described_class::SUPPORTED_VERSIONS[:dast].join(', ') } - let(:analyzer_vendor) do - { 'name' => 'A DAST analyzer' } - end - - let(:scanner_vendor) do - { 'name' => 'A DAST scanner' } - end - let(:scanner) do { 'id' => 'my-dast-scanner', 'name' => 'My DAST scanner', 'version' => '0.2.0', - 'vendor' => scanner_vendor + 'vendor' => { 'name' => 'A DAST scanner' } } end @@ -33,7 +25,7 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, featu 'id' => 'my-dast-analyzer', 'name' => 'My DAST analyzer', 'version' => '0.1.0', - 'vendor' => analyzer_vendor + 'vendor' => { 'name' => 'A DAST analyzer' } }, 'end_time' => '2020-01-28T03:26:02', 'scanned_resources' => [], @@ -441,22 +433,6 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, featu it_behaves_like 'report with expected warnings' end - - context 'and the report passes schema validation as a GitLab-vendored analyzer' do - let(:analyzer_vendor) do - { 'name' => 'GitLab' } - end - - it { is_expected.to be_empty } - end - - context 'and the report passes schema validation as a GitLab-vendored scanner' do - let(:scanner_vendor) do - { 'name' => 'GitLab' } - end - - it { is_expected.to be_empty } - end end context 'when given an unsupported schema version' do diff --git a/spec/lib/gitlab/database/load_balancing/logger_spec.rb b/spec/lib/gitlab/database/load_balancing/logger_spec.rb new file mode 100644 index 00000000000..81883fa6f1a --- /dev/null +++ b/spec/lib/gitlab/database/load_balancing/logger_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::LoadBalancing::Logger, feature_category: :database do + subject { described_class.new('/dev/null') } + + it_behaves_like 'a json logger', {} + + it 'excludes context' do + expect(described_class.exclude_context?).to be(true) + end +end diff --git a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb index b39b273bba9..fa7645d581c 100644 --- a/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb +++ b/spec/lib/gitlab/database/obsolete_ignored_columns_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Database::ObsoleteIgnoredColumns do +RSpec.describe Gitlab::Database::ObsoleteIgnoredColumns, feature_category: :database do before do stub_const('Testing', Module.new) stub_const('Testing::MyBase', Class.new(ActiveRecord::Base)) @@ -16,11 +16,10 @@ RSpec.describe Gitlab::Database::ObsoleteIgnoredColumns do Testing.module_eval do Testing::MyBase.class_eval do + include IgnorableColumns end SomeAbstract.class_eval do - include IgnorableColumns - self.abstract_class = true self.table_name = 'projects' @@ -29,8 +28,6 @@ RSpec.describe Gitlab::Database::ObsoleteIgnoredColumns do end Testing::B.class_eval do - include IgnorableColumns - self.table_name = 'issues' ignore_column :id, :other, remove_after: '2019-01-01', remove_with: '12.0' diff --git a/spec/lib/gitlab/database/pg_depend_spec.rb b/spec/lib/gitlab/database/pg_depend_spec.rb index f8b0e1af3a5..547a2c84b76 100644 --- a/spec/lib/gitlab/database/pg_depend_spec.rb +++ b/spec/lib/gitlab/database/pg_depend_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Gitlab::Database::PgDepend, type: :model, feature_category: :data connection.execute('CREATE EXTENSION IF NOT EXISTS pg_stat_statements;') end - it 'returns pg_stat_statements' do + it 'returns pg_stat_statements', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/410508' do expect(subject.pluck('relname')).to eq(['pg_stat_statements']) end end diff --git a/spec/lib/gitlab/database/reflection_spec.rb b/spec/lib/gitlab/database/reflection_spec.rb index 779bdbe50f0..641dd48be36 100644 --- a/spec/lib/gitlab/database/reflection_spec.rb +++ b/spec/lib/gitlab/database/reflection_spec.rb @@ -191,9 +191,15 @@ RSpec.describe Gitlab::Database::Reflection, feature_category: :database do expect(database.postgresql_minimum_supported_version?).to eq(false) end - it 'returns true when using PostgreSQL 12' do + it 'returns false when using PostgreSQL 12' do allow(database).to receive(:version).and_return('12') + expect(database.postgresql_minimum_supported_version?).to eq(false) + end + + it 'returns true when using PostgreSQL 13' do + allow(database).to receive(:version).and_return('13') + expect(database.postgresql_minimum_supported_version?).to eq(true) end end diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index 1ba526a6eb3..0073d2ebe80 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -534,7 +534,7 @@ RSpec.describe Gitlab::GitalyClient, feature_category: :gitaly do context 'when RequestStore is enabled with empty git_env', :request_store do before do - ::Gitlab::Instrumentation::Storage[:gitlab_git_env] = {} + Gitlab::SafeRequestStore[:gitlab_git_env] = {} end it 'disables force-routing to primary' do @@ -544,7 +544,7 @@ RSpec.describe Gitlab::GitalyClient, feature_category: :gitaly do context 'when RequestStore is enabled with populated git_env', :request_store do before do - ::Gitlab::Instrumentation::Storage[:gitlab_git_env] = { + Gitlab::SafeRequestStore[:gitlab_git_env] = { "GIT_OBJECT_DIRECTORY_RELATIVE" => "foo/bar" } end diff --git a/spec/lib/gitlab/github_import/logger_spec.rb b/spec/lib/gitlab/github_import/logger_spec.rb index 6fd0f5db93e..97806872746 100644 --- a/spec/lib/gitlab/github_import/logger_spec.rb +++ b/spec/lib/gitlab/github_import/logger_spec.rb @@ -5,37 +5,5 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Logger do subject(:logger) { described_class.new('/dev/null') } - let(:now) { Time.zone.now } - - describe '#format_message' do - before do - allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('new-correlation-id') - end - - it 'formats strings' do - output = subject.format_message('INFO', now, 'test', 'Hello world') - - expect(Gitlab::Json.parse(output)).to eq({ - 'severity' => 'INFO', - 'time' => now.utc.iso8601(3), - 'message' => 'Hello world', - 'correlation_id' => 'new-correlation-id', - 'feature_category' => 'importers', - 'import_type' => 'github' - }) - end - - it 'formats hashes' do - output = subject.format_message('INFO', now, 'test', { hello: 1 }) - - expect(Gitlab::Json.parse(output)).to eq({ - 'severity' => 'INFO', - 'time' => now.utc.iso8601(3), - 'hello' => 1, - 'correlation_id' => 'new-correlation-id', - 'feature_category' => 'importers', - 'import_type' => 'github' - }) - end - end + it_behaves_like 'a json logger', { 'feature_category' => 'importers', 'import_type' => 'github' } end diff --git a/spec/lib/gitlab/graphql/calls_gitaly/field_extension_spec.rb b/spec/lib/gitlab/graphql/calls_gitaly/field_extension_spec.rb index 1899697c78e..33f49dbc8d4 100644 --- a/spec/lib/gitlab/graphql/calls_gitaly/field_extension_spec.rb +++ b/spec/lib/gitlab/graphql/calls_gitaly/field_extension_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Gitlab::Graphql::CallsGitaly::FieldExtension, :request_store do context 'when the field calls gitaly' do before do owner.define_method :value do - ::Gitlab::Instrumentation::Storage['gitaly_call_actual'] = 1 + Gitlab::SafeRequestStore['gitaly_call_actual'] = 1 'fresh-from-the-gitaly-mines!' end end @@ -64,22 +64,22 @@ RSpec.describe Gitlab::Graphql::CallsGitaly::FieldExtension, :request_store do object = :anything arguments = :any_args - ::Gitlab::Instrumentation::Storage['gitaly_call_actual'] = 3 - ::Gitlab::Instrumentation::Storage['graphql_gitaly_accounted_for'] = 0 + ::Gitlab::SafeRequestStore['gitaly_call_actual'] = 3 + ::Gitlab::SafeRequestStore['graphql_gitaly_accounted_for'] = 0 expect do |b| extension.resolve(object: object, arguments: arguments, &b) end.to yield_with_args(object, arguments, [3, 0]) - ::Gitlab::Instrumentation::Storage['gitaly_call_actual'] = 13 - ::Gitlab::Instrumentation::Storage['graphql_gitaly_accounted_for'] = 10 + ::Gitlab::SafeRequestStore['gitaly_call_actual'] = 13 + ::Gitlab::SafeRequestStore['graphql_gitaly_accounted_for'] = 10 expect { extension.after_resolve(value: 'foo', memo: [3, 0]) }.not_to raise_error end it 'is unacceptable if some of the calls are unaccounted for' do - ::Gitlab::Instrumentation::Storage['gitaly_call_actual'] = 10 - ::Gitlab::Instrumentation::Storage['graphql_gitaly_accounted_for'] = 9 + ::Gitlab::SafeRequestStore['gitaly_call_actual'] = 10 + ::Gitlab::SafeRequestStore['graphql_gitaly_accounted_for'] = 9 expect { extension.after_resolve(value: 'foo', memo: [0, 0]) }.to raise_error(include('Object.value')) end diff --git a/spec/lib/gitlab/import/logger_spec.rb b/spec/lib/gitlab/import/logger_spec.rb index 60978aaa25c..a85ba84108e 100644 --- a/spec/lib/gitlab/import/logger_spec.rb +++ b/spec/lib/gitlab/import/logger_spec.rb @@ -5,35 +5,5 @@ require 'spec_helper' RSpec.describe Gitlab::Import::Logger do subject { described_class.new('/dev/null') } - let(:now) { Time.zone.now } - - describe '#format_message' do - before do - allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('new-correlation-id') - end - - it 'formats strings' do - output = subject.format_message('INFO', now, 'test', 'Hello world') - - expect(Gitlab::Json.parse(output)).to eq({ - 'severity' => 'INFO', - 'time' => now.utc.iso8601(3), - 'message' => 'Hello world', - 'correlation_id' => 'new-correlation-id', - 'feature_category' => 'importers' - }) - end - - it 'formats hashes' do - output = subject.format_message('INFO', now, 'test', { hello: 1 }) - - expect(Gitlab::Json.parse(output)).to eq({ - 'severity' => 'INFO', - 'time' => now.utc.iso8601(3), - 'hello' => 1, - 'correlation_id' => 'new-correlation-id', - 'feature_category' => 'importers' - }) - end - end + it_behaves_like 'a json logger', { 'feature_category' => 'importers' } end diff --git a/spec/lib/gitlab/instrumentation/storage_spec.rb b/spec/lib/gitlab/instrumentation/storage_spec.rb deleted file mode 100644 index afff8f6251b..00000000000 --- a/spec/lib/gitlab/instrumentation/storage_spec.rb +++ /dev/null @@ -1,69 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ::Gitlab::Instrumentation::Storage, :request_store, feature_category: :shared do - subject(:storage) { described_class } - - describe '.active?' do - context 'when SafeRequestStore is active' do - it 'returns true' do - allow(Gitlab::SafeRequestStore).to receive(:active?).and_return(true) - - expect(storage.active?).to be(true) - end - end - - context 'when SafeRequestStore is not active' do - it 'returns false' do - allow(Gitlab::SafeRequestStore).to receive(:active?).and_return(false) - - expect(storage.active?).to be(false) - end - end - end - - it 'stores data' do - storage[:a] = 1 - storage[:b] = 'hey' - - expect(storage[:a]).to eq(1) - expect(storage[:b]).to eq('hey') - end - - describe '.clear!' do - it 'removes all values' do - storage[:a] = 1 - storage[:b] = 'hey' - - storage.clear! - - expect(storage[:a]).to be_nil - expect(storage[:b]).to be_nil - end - end - - # This is testing implementation details, but until we have a truly segregated - # instrumentation data store, we need to make sure we do not "pollute" the - # underlying RequestStore or interfere with other co-located data. - describe 'backing storage' do - it 'stores data in the instrumentation bucket' do - storage[:a] = 1 - - expect(::RequestStore[:instrumentation]).to eq({ a: 1 }) - end - - describe '.clear!' do - it 'resets only the instrumentation bucket' do - storage[:a] = 1 - storage[:b] = 'hey' - ::RequestStore[:b] = 2 - - storage.clear! - - expect(::RequestStore[:instrumentation]).to eq({}) - expect(::RequestStore[:b]).to eq(2) - end - end - end -end diff --git a/spec/lib/gitlab/instrumentation_helper_spec.rb b/spec/lib/gitlab/instrumentation_helper_spec.rb index b934f2261a4..8a88328e0c1 100644 --- a/spec/lib/gitlab/instrumentation_helper_spec.rb +++ b/spec/lib/gitlab/instrumentation_helper_spec.rb @@ -8,14 +8,6 @@ RSpec.describe Gitlab::InstrumentationHelper, :clean_gitlab_redis_repository_cac :use_null_store_as_repository_cache, feature_category: :scalability do using RSpec::Parameterized::TableSyntax - describe '.init_instrumentation_data' do - it 'clears instrumentation storage' do - expect(::Gitlab::Instrumentation::Storage).to receive(:clear!) - - described_class.init_instrumentation_data - end - end - describe '.add_instrumentation_data', :request_store do let(:payload) { {} } @@ -34,7 +26,7 @@ RSpec.describe Gitlab::InstrumentationHelper, :clean_gitlab_redis_repository_cac context 'when Gitaly calls are made' do it 'adds Gitaly and Redis data' do project = create(:project) - ::Gitlab::Instrumentation::Storage.clear! + RequestStore.clear! project.repository.exists? subject @@ -189,8 +181,8 @@ RSpec.describe Gitlab::InstrumentationHelper, :clean_gitlab_redis_repository_cac context 'when replica caught up search was made' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_ok] = 2 - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_fail] = 1 + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 2 + Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = 1 end it 'includes related metrics' do @@ -203,8 +195,8 @@ RSpec.describe Gitlab::InstrumentationHelper, :clean_gitlab_redis_repository_cac context 'when only a single counter was updated' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_ok] = 1 - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_fail] = nil + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 1 + Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = nil end it 'includes only that counter into logging' do diff --git a/spec/lib/gitlab/json_logger_spec.rb b/spec/lib/gitlab/json_logger_spec.rb index 801de357ddc..87df20c066b 100644 --- a/spec/lib/gitlab/json_logger_spec.rb +++ b/spec/lib/gitlab/json_logger_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe Gitlab::JsonLogger do subject { described_class.new('/dev/null') } - let(:now) { Time.now } + it_behaves_like 'a json logger', {} describe '#file_name' do let(:subclass) do @@ -26,31 +26,4 @@ RSpec.describe Gitlab::JsonLogger do expect(subclass.file_name).to eq('testlogger.log') end end - - describe '#format_message' do - before do - allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('new-correlation-id') - end - - it 'formats strings' do - output = subject.format_message('INFO', now, 'test', 'Hello world') - data = Gitlab::Json.parse(output) - - expect(data['severity']).to eq('INFO') - expect(data['time']).to eq(now.utc.iso8601(3)) - expect(data['message']).to eq('Hello world') - expect(data['correlation_id']).to eq('new-correlation-id') - end - - it 'formats hashes' do - output = subject.format_message('INFO', now, 'test', { hello: 1 }) - data = Gitlab::Json.parse(output) - - expect(data['severity']).to eq('INFO') - expect(data['time']).to eq(now.utc.iso8601(3)) - expect(data['hello']).to eq(1) - expect(data['message']).to be_nil - expect(data['correlation_id']).to eq('new-correlation-id') - end - end end diff --git a/spec/lib/gitlab/metrics/subscribers/external_http_spec.rb b/spec/lib/gitlab/metrics/subscribers/external_http_spec.rb index cccfb5d8c99..18a5d2c2c3f 100644 --- a/spec/lib/gitlab/metrics/subscribers/external_http_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/external_http_spec.rb @@ -67,7 +67,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ExternalHttp, :request_store, featu context 'when external HTTP detail store has some values' do before do Gitlab::SafeRequestStore[:peek_enabled] = true - ::Gitlab::Instrumentation::Storage[:external_http_detail_store] = [{ + Gitlab::SafeRequestStore[:external_http_detail_store] = [{ method: 'POST', code: "200", duration: 0.321 }] end @@ -87,8 +87,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::ExternalHttp, :request_store, featu context 'when external HTTP recorded some values' do before do - ::Gitlab::Instrumentation::Storage[:external_http_count] = 7 - ::Gitlab::Instrumentation::Storage[:external_http_duration_s] = 1.2 + Gitlab::SafeRequestStore[:external_http_count] = 7 + Gitlab::SafeRequestStore[:external_http_duration_s] = 1.2 end it 'returns the external http detailed store' do @@ -133,7 +133,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ExternalHttp, :request_store, featu ) expect(described_class.top_slowest_requests).to eq(slow_requests) - expect(::Gitlab::Instrumentation::Storage[:external_http_slow_requests].length).to eq(3) + expect(Gitlab::SafeRequestStore[:external_http_slow_requests].length).to eq(3) end end end @@ -181,8 +181,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::ExternalHttp, :request_store, featu subscriber.request(event_2) subscriber.request(event_3) - expect(::Gitlab::Instrumentation::Storage[:external_http_count]).to eq(3) - expect(::Gitlab::Instrumentation::Storage[:external_http_duration_s]).to eq(5.741) # 0.321 + 0.12 + 5.3 + expect(Gitlab::SafeRequestStore[:external_http_count]).to eq(3) + expect(Gitlab::SafeRequestStore[:external_http_duration_s]).to eq(5.741) # 0.321 + 0.12 + 5.3 end it 'stores a portion of events into the detail store' do @@ -190,22 +190,22 @@ RSpec.describe Gitlab::Metrics::Subscribers::ExternalHttp, :request_store, featu subscriber.request(event_2) subscriber.request(event_3) - expect(::Gitlab::Instrumentation::Storage[:external_http_detail_store].length).to eq(3) - expect(::Gitlab::Instrumentation::Storage[:external_http_detail_store][0]).to match a_hash_including( + expect(Gitlab::SafeRequestStore[:external_http_detail_store].length).to eq(3) + expect(Gitlab::SafeRequestStore[:external_http_detail_store][0]).to match a_hash_including( start: be_like_time(Time.current), method: 'POST', code: "200", duration: 0.321, scheme: 'https', host: 'gitlab.com', port: 443, path: '/api/v4/projects', query: 'current=true', exception_object: nil, backtrace: be_a(Array) ) - expect(::Gitlab::Instrumentation::Storage[:external_http_detail_store][1]).to match a_hash_including( + expect(Gitlab::SafeRequestStore[:external_http_detail_store][1]).to match a_hash_including( start: be_like_time(Time.current), method: 'GET', code: "301", duration: 0.12, scheme: 'http', host: 'gitlab.com', port: 80, path: '/api/v4/projects/2', query: 'current=true', exception_object: nil, backtrace: be_a(Array) ) - expect(::Gitlab::Instrumentation::Storage[:external_http_detail_store][2]).to match a_hash_including( + expect(Gitlab::SafeRequestStore[:external_http_detail_store][2]).to match a_hash_including( start: be_like_time(Time.current), method: 'POST', duration: 5.3, scheme: 'http', host: 'gitlab.com', port: 80, path: '/api/v4/projects/2/issues', @@ -225,7 +225,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::ExternalHttp, :request_store, featu subscriber.request(event_2) subscriber.request(event_3) - expect(::Gitlab::Instrumentation::Storage[:external_http_detail_store]).to be(nil) + expect(Gitlab::SafeRequestStore[:external_http_detail_store]).to be(nil) end end end diff --git a/spec/lib/gitlab/metrics/subscribers/ldap_spec.rb b/spec/lib/gitlab/metrics/subscribers/ldap_spec.rb index 1db8659943e..fb822c8d779 100644 --- a/spec/lib/gitlab/metrics/subscribers/ldap_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/ldap_spec.rb @@ -66,7 +66,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::Ldap, :request_store, feature_categ end describe ".payload" do - context "when instrumentation storage is empty" do + context "when SafeRequestStore is empty" do it "returns an empty array" do expect(described_class.payload).to eql(net_ldap_count: 0, net_ldap_duration_s: 0.0) end @@ -74,8 +74,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::Ldap, :request_store, feature_categ context "when LDAP recorded some values" do before do - ::Gitlab::Instrumentation::Storage[:net_ldap_count] = 7 - ::Gitlab::Instrumentation::Storage[:net_ldap_duration_s] = 1.2 + Gitlab::SafeRequestStore[:net_ldap_count] = 7 + Gitlab::SafeRequestStore[:net_ldap_duration_s] = 1.2 end it "returns the populated payload" do @@ -117,8 +117,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::Ldap, :request_store, feature_categ subscriber.observe_event(event_2) subscriber.observe_event(event_3) - expect(::Gitlab::Instrumentation::Storage[:net_ldap_count]).to eq(3) - expect(::Gitlab::Instrumentation::Storage[:net_ldap_duration_s]).to eq(0.005741) # (0.321 + 0.12 + 5.3) / 1000 + expect(Gitlab::SafeRequestStore[:net_ldap_count]).to eq(3) + expect(Gitlab::SafeRequestStore[:net_ldap_duration_s]).to eq(0.005741) # (0.321 + 0.12 + 5.3) / 1000 end end end diff --git a/spec/lib/gitlab/metrics/subscribers/load_balancing_spec.rb b/spec/lib/gitlab/metrics/subscribers/load_balancing_spec.rb index be04167a588..c2c3bb29b16 100644 --- a/spec/lib/gitlab/metrics/subscribers/load_balancing_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/load_balancing_spec.rb @@ -22,7 +22,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store, feat it 'stores per-request caught up replica search result' do subject - expect(::Gitlab::Instrumentation::Storage[counter_name]).to eq(1) + expect(Gitlab::SafeRequestStore[counter_name]).to eq(1) end end @@ -50,7 +50,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store, feat context 'when no data in request store' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick] = nil + Gitlab::SafeRequestStore[:caught_up_replica_pick] = nil end it 'does not change the counters' do @@ -60,8 +60,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store, feat context 'when request store was updated' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_ok] = 2 - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_fail] = 1 + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 2 + Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = 1 end it 'increments :caught_up_replica_pick count with proper label' do @@ -78,8 +78,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store, feat context 'when no data in request store' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_ok] = nil - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_fail] = nil + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = nil + Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = nil end it 'returns empty hash' do @@ -89,7 +89,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store, feat context 'when request store was updated for a single counter' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_ok] = 2 + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 2 end it 'returns proper payload with only that counter' do @@ -99,8 +99,8 @@ RSpec.describe Gitlab::Metrics::Subscribers::LoadBalancing, :request_store, feat context 'when both counters were updated' do before do - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_ok] = 2 - ::Gitlab::Instrumentation::Storage[:caught_up_replica_pick_fail] = 1 + Gitlab::SafeRequestStore[:caught_up_replica_pick_ok] = 2 + Gitlab::SafeRequestStore[:caught_up_replica_pick_fail] = 1 end it 'return proper payload' do diff --git a/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb b/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb index a87b1ab4a71..13965bf1244 100644 --- a/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/rack_attack_spec.rb @@ -17,7 +17,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do context 'when the request store already has data' do before do - ::Gitlab::Instrumentation::Storage[:rack_attack_instrumentation] = { + Gitlab::SafeRequestStore[:rack_attack_instrumentation] = { rack_attack_redis_count: 10, rack_attack_redis_duration_s: 9.0 } @@ -239,7 +239,7 @@ RSpec.describe Gitlab::Metrics::Subscribers::RackAttack, :request_store do it 'adds the matched name to safe request store' do subscriber.safelist(event) - expect(::Gitlab::Instrumentation::Storage[:instrumentation_throttle_safelist]).to eql('throttle_unauthenticated') + expect(Gitlab::SafeRequestStore[:instrumentation_throttle_safelist]).to eql('throttle_unauthenticated') end end end diff --git a/spec/lib/gitlab/middleware/request_context_spec.rb b/spec/lib/gitlab/middleware/request_context_spec.rb index 6d5b581feaa..cd21209bcee 100644 --- a/spec/lib/gitlab/middleware/request_context_spec.rb +++ b/spec/lib/gitlab/middleware/request_context_spec.rb @@ -1,10 +1,10 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'spec_helper' require 'rack' require 'request_store' require_relative '../../../support/helpers/next_instance_of' -RSpec.describe Gitlab::Middleware::RequestContext do +RSpec.describe Gitlab::Middleware::RequestContext, feature_category: :application_instrumentation do include NextInstanceOf let(:app) { -> (env) {} } @@ -55,6 +55,10 @@ RSpec.describe Gitlab::Middleware::RequestContext do it 'sets the `request_start_time`' do expect { subject }.to change { instance.request_start_time }.from(nil).to(Float) end + + it 'sets the `spam_params`' do + expect { subject }.to change { instance.spam_params }.from(nil).to(::Spam::SpamParams) + end end end end diff --git a/spec/lib/gitlab/request_context_spec.rb b/spec/lib/gitlab/request_context_spec.rb index b9acfa4a841..44664be7d39 100644 --- a/spec/lib/gitlab/request_context_spec.rb +++ b/spec/lib/gitlab/request_context_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::RequestContext, :request_store do +RSpec.describe Gitlab::RequestContext, :request_store, feature_category: :application_instrumentation do subject { described_class.instance } before do @@ -11,6 +11,44 @@ RSpec.describe Gitlab::RequestContext, :request_store do it { is_expected.to have_attributes(client_ip: nil, start_thread_cpu_time: nil, request_start_time: nil) } + describe '.start_request_context' do + let(:request) { ActionDispatch::Request.new({ 'REMOTE_ADDR' => '1.2.3.4' }) } + let(:start_request_context) { described_class.start_request_context(request: request) } + + before do + allow(Gitlab::Metrics::System).to receive(:real_time).and_return(123) + end + + it 'sets the client IP' do + expect { start_request_context }.to change { subject.client_ip }.from(nil).to('1.2.3.4') + end + + it 'sets the spam params' do + expect { start_request_context }.to change { subject.spam_params }.from(nil).to(::Spam::SpamParams) + end + + it 'sets the request start time' do + expect { start_request_context }.to change { subject.request_start_time }.from(nil).to(123) + end + end + + describe '.start_thread_context' do + let(:start_thread_context) { described_class.start_thread_context } + + before do + allow(Gitlab::Metrics::System).to receive(:thread_cpu_time).and_return(123) + allow(Gitlab::Memory::Instrumentation).to receive(:start_thread_memory_allocations).and_return(456) + end + + it 'sets the thread cpu time' do + expect { start_thread_context }.to change { subject.start_thread_cpu_time }.from(nil).to(123) + end + + it 'sets the thread memory allocations' do + expect { start_thread_context }.to change { subject.thread_memory_allocations }.from(nil).to(456) + end + end + describe '#request_deadline' do let(:request_start_time) { 1575982156.206008 } diff --git a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb index 459c926fa89..4b589dc43af 100644 --- a/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb +++ b/spec/lib/gitlab/sidekiq_logging/structured_logger_spec.rb @@ -328,7 +328,7 @@ RSpec.describe Gitlab::SidekiqLogging::StructuredLogger do ApplicationRecord.connection.execute('SELECT pg_sleep(0.1);') end - ::Gitlab::Instrumentation::Storage.clear! + Gitlab::SafeRequestStore.clear! call_subject(job.dup, 'test_queue') {} end diff --git a/spec/lib/gitlab/sidekiq_middleware_spec.rb b/spec/lib/gitlab/sidekiq_middleware_spec.rb index 7f22dea8528..af9075f5aa0 100644 --- a/spec/lib/gitlab/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware_spec.rb @@ -18,8 +18,8 @@ RSpec.describe Gitlab::SidekiqMiddleware do include ApplicationWorker def perform(*args) - ::Gitlab::Instrumentation::Storage['gitaly_call_actual'] = 1 - ::Gitlab::Instrumentation::Storage[:gitaly_query_time] = 5 + Gitlab::SafeRequestStore['gitaly_call_actual'] = 1 + Gitlab::SafeRequestStore[:gitaly_query_time] = 5 end end end |