Merge branch 'more-elastic-logging' into 'master'

Add logging to error handling in ElasticIndexerWorker

See merge request gitlab-org/gitlab!22414

ee/app/services/elastic/index_record_service.rb

@@ -22,13 +22,16 @@ module Elastic
       update_issue_notes(record, options["changed_fields"]) if record.class == Issue
 
       true
-    rescue Elasticsearch::Transport::Transport::Errors::NotFound, ActiveRecord::RecordNotFound
+    rescue Elasticsearch::Transport::Transport::Errors::NotFound, ActiveRecord::RecordNotFound => e
       # These errors can happen in several cases, including:
       # - A record is updated, then removed before the update is handled
       # - Indexing is enabled, but not every item has been indexed yet - updating
       #   and deleting the un-indexed records will raise exception
       #
       # We can ignore these.
+
+      logger.error(message: 'elastic_index_record_service_caught_exception', error_class: e.class.name, error_message: e.message)
+
       true
     end
 
@@ -88,5 +91,9 @@ module Elastic
       ids = errors.map { |error| error['index']['_id'][/_(\d+)$/, 1] }
       association.id_in(ids).es_import(options)
     end
+
+    def logger
+      @logger ||= ::Gitlab::Elasticsearch::Logger.build
+    end
   end
 end

ee/app/workers/elastic_indexer_worker.rb

@@ -32,13 +32,16 @@ class ElasticIndexerWorker
         client.delete index: klass.index_name, type: klass.document_type, id: es_id
       end
     end
-  rescue Elasticsearch::Transport::Transport::Errors::NotFound, ActiveRecord::RecordNotFound
+  rescue Elasticsearch::Transport::Transport::Errors::NotFound, ActiveRecord::RecordNotFound => e
     # These errors can happen in several cases, including:
     # - A record is updated, then removed before the update is handled
     # - Indexing is enabled, but not every item has been indexed yet - updating
     #   and deleting the un-indexed records will raise exception
     #
     # We can ignore these.
+
+    logger.error(message: 'elastic_indexer_worker_caught_exception', error_class: e.class.name, error_message: e.message)
+
     true
   end
 
@@ -65,4 +68,8 @@ class ElasticIndexerWorker
       }
     })
   end
+
+  def logger
+    @logger ||= ::Gitlab::Elasticsearch::Logger.build
+  end
 end

ee/spec/services/elastic/index_record_service_spec.rb

@@ -50,6 +50,14 @@ describe Elastic::IndexRecordService, :elastic do
           Gitlab::Elastic::Helper.refresh_index
         end.to change { Elasticsearch::Model.search('new').records.size }.by(1)
       end
+
+      it 'ignores Elasticsearch::Transport::Transport::Errors::NotFound errors' do
+        object = create(type)
+
+        allow(object.__elasticsearch__).to receive(:index_document).and_raise(Elasticsearch::Transport::Transport::Errors::NotFound)
+
+        expect(subject.execute(object, true)).to eq(true)
+      end
     end
   end
 

ee/spec/workers/elastic_indexer_worker_spec.rb

@@ -103,4 +103,18 @@ describe ElasticIndexerWorker, :elastic do
       subject.perform("index", 'Project', object.id, object.es_id)
     end.to raise_error
   end
+
+  it 'ignores Elasticsearch::Transport::Transport::Errors::NotFound error' do
+    object = create(:project)
+
+    expect_next_instance_of(Elastic::IndexRecordService) do |service|
+      allow(service).to receive(:execute).and_raise(Elasticsearch::Transport::Transport::Errors::NotFound)
+    end
+
+    expect(subject.perform("index", 'Project', object.id, object.es_id)).to eq(true)
+  end
+
+  it 'ignores missing records' do
+    expect(subject.perform("index", 'Project', -1, 'project_-1')).to eq(true)
+  end
 end