METRON-1849 Elasticsearch Index Write Functionality Should be Shared

[nickwallen]

The Elasticsearch index write functionality is currently duplicated between the ElasticsearchWriter and the ElasticsearchUpdateDao. This functionality needs to be de-duplicated and shared between the two. This will help ensure that any changes that need to be made to how alerts are written, for instance whether the client sets a document ID, are implemented and tested in the same way across Metron.

This change depends on the following PRs. Once these predecessors are addressed, review of this will be much simpler.

• METRON-1834: Migrate Elasticsearch from TransportClient to new Java REST API #1242
• METRON-1845 Correct Test Data Load in Elasticsearch Integration Tests #1247

Changes

• Created the BulkDocumentWriter and ElasticsearchBulkDocumentWriter that are responsible for writing messages/alerts/documents to an Elasticsearch index.

• The ElasticsearchWriter uses a BulkDocumentWriter<TupleBasedDocument> to index messages in Elasticsearch.

• The ElasticsearchWriter actually writes TupleBasedDocument POJOS that extend Document. This allows it to maintain the relationship between a tuple and the document created from a tuple. If a Document fails to write, we need to fail the associated tuple.

• The scope of the ElasticsearchWriter tests were increased since this was now easier with the new abstractions.

• The ElasticsearchUpdateDao uses a BulkDocumentWriter<Document> to update alerts for the Alerts UI.

• The ElasticsearchRetrieveLatestDao was setting the timestamp of all Documents returned to 0. This was causing the tests that I added to fail. I added logic to extract the timestamp from the message and set the Document.timestamp if one exists.

Instead of creating the BulkDocumentWriter abstraction, I also looked at a couple alternatives.

• Another approach I looked at was to simply have the ElasticsearchWriter use an ElasticsearchUpdateDao. Unfortunately, this quickly led to a large number of changes. The ElasticsearchUpdateDao does not track and record individual failures in a batch. It instead fails fast if any single update in a batch fails. To track individual failures, as needed by the ElasticsearchWriter, would have required changes to the signature of UpdateDao.

• Another approach was to have the ElasticsearchUpdateDao use an ElasticsearchWriter. Unfortunately, the ElasticsearchWriter is a BulkMessageWriter and requires Storm as a dependency due to its use of TopologyContext. To clean this up would have required a large number of changes.

Testing

Spin-up the Full Dev environment. Use the Alerts UI to:

• Search for alerts
• Escalate alerts
• Add comments to alerts
• Remove comments from alerts
• Create meta-alerts
• Escalate meta-alerts

Pull Request Checklist

• Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
• Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
• Has your PR been rebased against the latest commit within the target branch (typically master)?
• Have you included steps to reproduce the behavior or problem that is being changed or addressed?
• Have you included steps or a guide to how the change may be verified and tested manually?
• Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
• Have you written or updated unit tests and or integration tests to verify your changes?
• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
• Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?

[justinleet]

+1, pending Travis and @mmiklavc. Thanks a lot, it's great to see this cleaned up and refactored!

[mmiklavc]

+1 lgtm with the latest changes, thanks for the submission @nickwallen!

[nickwallen]

Thanks for the feedback @justinleet and @mmiklavc . It turned out better for it. I will merge once the latest CI build completes.