diff --git a/ee/app/services/elastic/process_bookkeeping_service.rb b/ee/app/services/elastic/process_bookkeeping_service.rb index 59e188f7b83498c247ae5cda980ab6373a834546..382a47576629d2b0c8f1d6c0d7ffb9fda621acff 100644 --- a/ee/app/services/elastic/process_bookkeeping_service.rb +++ b/ee/app/services/elastic/process_bookkeeping_service.rb @@ -208,7 +208,7 @@ def execute_with_redis(redis) # rubocop:disable Metrics/AbcSize search_indexing_flushing_duration_s: flushing_duration_s ) - Gitlab::Metrics::GlobalSearchIndexingSlis.record_apdex(elapsed: flushing_duration_s, type: klass) + Gitlab::Metrics::GlobalSearchIndexingSlis.record_apdex(elapsed: flushing_duration_s, document_type: klass) end specs_buffer.count diff --git a/ee/app/workers/elastic_commit_indexer_worker.rb b/ee/app/workers/elastic_commit_indexer_worker.rb index 4640a957caeb94e533bf8381369efc62b999014b..4fa78ef0f7bb5603750126eb71b1c8a0f7d208b8 100644 --- a/ee/app/workers/elastic_commit_indexer_worker.rb +++ b/ee/app/workers/elastic_commit_indexer_worker.rb @@ -42,10 +42,11 @@ def perform(project_id, wiki = false, options = {}) search_indexing_duration_s: search_indexing_duration_s, jid: jid ) + + document_type = wiki ? 'Wiki' : 'Code' + Gitlab::Metrics::GlobalSearchIndexingSlis.record_apdex(elapsed: search_indexing_duration_s, document_type: document_type) end - type = wiki ? 'Wiki' : 'Code' - Gitlab::Metrics::GlobalSearchIndexingSlis.record_apdex(elapsed: search_indexing_duration_s, type: type) @ret end end diff --git a/ee/lib/gitlab/metrics/global_search_indexing_slis.rb b/ee/lib/gitlab/metrics/global_search_indexing_slis.rb index 59692de5d6e28a422ede8b0ca96f604952cfb869..d4941bec65505eb387dde7b3cc688e9b4c05a432 100644 --- a/ee/lib/gitlab/metrics/global_search_indexing_slis.rb +++ b/ee/lib/gitlab/metrics/global_search_indexing_slis.rb @@ -18,10 +18,10 @@ def initialize_slis! Gitlab::Metrics::Sli::Apdex.initialize_sli(:global_search_indexing, possible_labels) end - def record_apdex(elapsed:, type:) + def record_apdex(elapsed:, document_type:) Gitlab::Metrics::Sli::Apdex[:global_search_indexing].increment( - labels: labels(type: type), - success: elapsed < duration_target(type) + labels: labels(document_type: document_type), + success: elapsed < duration_target(document_type) ) end @@ -31,7 +31,7 @@ def duration_target(indexing_type) indexing_type == 'Code' ? CODE_INDEXING_TARGET_S : CONTENT_INDEXING_TARGET_S end - def types + def document_types indexable_models + %w[Code Wiki] end @@ -44,16 +44,16 @@ def indexable_models end def possible_labels - types.map do |type| + document_types.map do |document_type| { - type: type + document_type: document_type } end end - def labels(type:) + def labels(document_type:) { - type: type + document_type: document_type } end end diff --git a/ee/spec/lib/gitlab/metrics/global_search_indexing_slis_spec.rb b/ee/spec/lib/gitlab/metrics/global_search_indexing_slis_spec.rb index 5337a1a92a6e08cfe6b14488fe83847853962861..9d564ba13fea1072c2a2a8266e153ea16d6ca4bf 100644 --- a/ee/spec/lib/gitlab/metrics/global_search_indexing_slis_spec.rb +++ b/ee/spec/lib/gitlab/metrics/global_search_indexing_slis_spec.rb @@ -19,14 +19,14 @@ it 'increments the global_search_indexing SLI as a success' do expect(Gitlab::Metrics::Sli::Apdex[:global_search_indexing]).to receive(:increment).with( labels: { - type: 'Code' + document_type: 'Code' }, success: true ) described_class.record_apdex( elapsed: 0.1, - type: 'Code' + document_type: 'Code' ) end end @@ -35,14 +35,14 @@ it 'increments the global_search_indexing SLI as a failure' do expect(Gitlab::Metrics::Sli::Apdex[:global_search_indexing]).to receive(:increment).with( labels: { - type: 'Code' + document_type: 'Code' }, success: false ) described_class.record_apdex( elapsed: 15, - type: 'Code' + document_type: 'Code' ) end end diff --git a/ee/spec/services/elastic/process_bookkeeping_service_spec.rb b/ee/spec/services/elastic/process_bookkeeping_service_spec.rb index 5ec8f8ff972bef66fe29f4afb9ac5d8167bf2252..fb0aae149268b7db7ba28964207bea63bf4d6cbc 100644 --- a/ee/spec/services/elastic/process_bookkeeping_service_spec.rb +++ b/ee/spec/services/elastic/process_bookkeeping_service_spec.rb @@ -275,7 +275,7 @@ expect(Gitlab::Metrics::GlobalSearchIndexingSlis).to receive(:record_apdex).with( elapsed: a_kind_of(Numeric), - type: 'Issue' + document_type: 'Issue' ).exactly(fake_refs.size).times described_class.new.execute @@ -289,7 +289,7 @@ expect(Gitlab::Metrics::GlobalSearchIndexingSlis).to receive(:record_apdex).with( elapsed: a_kind_of(Numeric), - type: 'Issue' + document_type: 'Issue' ).exactly(fake_refs.size - 1).times described_class.new.execute diff --git a/ee/spec/workers/elastic_commit_indexer_worker_spec.rb b/ee/spec/workers/elastic_commit_indexer_worker_spec.rb index 0475cf2ff876f6a8abcedf8662f72a4a5fdd274d..f4c8aff2d4a6e3ce1275eff6e2043eba99afbbbb 100644 --- a/ee/spec/workers/elastic_commit_indexer_worker_spec.rb +++ b/ee/spec/workers/elastic_commit_indexer_worker_spec.rb @@ -14,13 +14,17 @@ end it 'runs indexer' do - expect_any_instance_of(Gitlab::Elastic::Indexer).to receive(:run) + expect_next_instance_of(Gitlab::Elastic::Indexer) do |indexer| + expect(indexer).to receive(:run) + end subject.perform(project.id, false) end it 'logs timing information' do - allow_any_instance_of(Gitlab::Elastic::Indexer).to receive(:run).and_return(true) + allow_next_instance_of(Gitlab::Elastic::Indexer) do |indexer| + allow(indexer).to receive(:run).and_return(true) + end expect(Gitlab::Elasticsearch::Logger).to receive(:build).and_return(logger_double.as_null_object) @@ -34,21 +38,40 @@ subject.perform(project.id, false) end + it 'records the apdex SLI' do + allow_next_instance_of(Gitlab::Elastic::Indexer) do |indexer| + allow(indexer).to receive(:run).and_return(true) + end + + expect(Gitlab::Metrics::GlobalSearchIndexingSlis).to receive(:record_apdex).with( + elapsed: a_kind_of(Numeric), + document_type: 'Code' + ) + + subject.perform(project.id) + end + context 'when ES is disabled' do before do stub_ee_application_setting(elasticsearch_indexing: false) end it 'returns true' do - expect_any_instance_of(Gitlab::Elastic::Indexer).not_to receive(:run) + expect(Gitlab::Elastic::Indexer).not_to receive(:new) - expect(subject.perform(1)).to be_truthy + expect(subject.perform(project.id)).to be_truthy end it 'does not log anything' do expect(logger_double).not_to receive(:info) - subject.perform(1) + subject.perform(project.id) + end + + it 'does not record the apdex SLI' do + expect(Gitlab::Metrics::GlobalSearchIndexingSlis).not_to receive(:record_apdex) + + subject.perform(project.id) end end @@ -79,16 +102,37 @@ subject.perform(project.id) end + + it 'does not record the apdex SLI' do + expect(subject).to receive(:in_lock) # Mock and don't yield + .with("ElasticCommitIndexerWorker/#{project.id}/false", ttl: (Gitlab::Elastic::Indexer::TIMEOUT + 1.minute), retries: 0) + + expect(Gitlab::Metrics::GlobalSearchIndexingSlis).not_to receive(:record_apdex) + + subject.perform(project.id) + end end context 'when the indexer fails' do it 'does not log anything' do - expect_any_instance_of(Gitlab::Elastic::Indexer).to receive(:run).and_return false + expect_next_instance_of(Gitlab::Elastic::Indexer) do |indexer| + expect(indexer).to receive(:run).and_return false + end expect(logger_double).not_to receive(:info) subject.perform(project.id) end + + it 'does not record the apdex SLI' do + expect_next_instance_of(Gitlab::Elastic::Indexer) do |indexer| + expect(indexer).to receive(:run).and_return false + end + + expect(Gitlab::Metrics::GlobalSearchIndexingSlis).not_to receive(:record_apdex) + + subject.perform(project.id) + end end end end