You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@metron.apache.org by "ASF GitHub Bot (JIRA)" <ji...@apache.org> on 2018/11/05 20:20:00 UTC
[jira] [Commented] (METRON-1849) Elasticsearch Index Write
Functionality Should be Shared
[ https://issues.apache.org/jira/browse/METRON-1849?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16675714#comment-16675714 ]
ASF GitHub Bot commented on METRON-1849:
----------------------------------------
GitHub user nickwallen opened a pull request:
https://github.com/apache/metron/pull/1254
METRON-1849 Elasticsearch Index Write Functionality Should be Shared
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.
- [ ] #1242
- [ ] #1247
## Changes
* Created the `BulkDocumentWriter` and `ElasticsearchBulkDocumentWriter` that are responsible for writing messages/alerts/documents to an Elasticsearch index.
* Created the `IndexedDocument` which is what the `BulkDocumentWriter` actually writes. This is a `Document` that contains the name of the index it is written to.
* I could not just add `index` to `Document` because the `HbaseDao` does not know how to populate this field. When using a `MultiIndexDao`, the `Document` returned can come from any of the underlying DAOs, including the `HBaseDao`, so this field cannot always be populated with the current archtecture.
* The `ElasticsearchWriter` uses a `BulkDocumentWriter` to index messages in Elasticsearch. This also uses a `TupleBasedDocument` that allows it to maintain the relationship between a tuple and the document created from a tuple. This is needed to properly ack/fail tuples.
* The scope of the `ElasticsearchWriter` tests were increased since this was now easier with the new abstractions.
* The `ElasticsearchUpdateDao` uses a `BulkDocumentWriter` to update alerts from the Alerts UI.
* The `ElasticsearchRetrieveLatestDao` was setting the timestamp of all `Document`s 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](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
- [ ] 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](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nickwallen/metron METRON-1849
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/1254.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1254
----
commit a7c7dc287b4f9c99c6780b934a0b6f433a03aa04
Author: cstella <ce...@...>
Date: 2018-10-09T00:06:52Z
Casey Stella - elasticsearch rest client migration base work
commit 10410ea9718a2a1b1d287fb4f22a6c98efb1fdaa
Author: Michael Miklavcic <mi...@...>
Date: 2018-10-09T00:07:22Z
Update shade plugin version
commit a33a16872118175ed35729df6ddde2959e49ae2f
Author: Michael Miklavcic <mi...@...>
Date: 2018-10-09T15:56:08Z
Fix es update dao test
commit 52c3c96d7657205e65d3bd0c0e35923a851911da
Author: Michael Miklavcic <mi...@...>
Date: 2018-10-09T21:26:16Z
Merge with master. Fix es search integration tests
commit 4742832869f9512e11cab4a32109c15a2b17a92e
Author: Michael Miklavcic <mi...@...>
Date: 2018-10-11T18:45:06Z
Merge branch 'master' into es-rest-client
commit 43809968320e586fd70411776140f3aa13a60195
Author: Michael Miklavcic <mi...@...>
Date: 2018-10-11T23:59:25Z
Get shade plugin working with the new ES client and the ClassIndexTransformer Shade plugin transformer.
commit af03f6f036e96c742db39733bf8ebc2cbf229129
Author: Michael Miklavcic <mi...@...>
Date: 2018-10-19T02:56:52Z
Introduce config classes for managing ES client configuration. Translate properties for new client.
commit 1c8eac2a173e1e24afc9e11163e456a9d90c93db
Author: Michael Miklavcic <mi...@...>
Date: 2018-10-23T22:21:32Z
Resolve merge conflicts with master
commit 1a47ded7a36f9d391227973c7da2921305373283
Author: Michael Miklavcic <mi...@...>
Date: 2018-10-23T22:23:04Z
Remove extra deps in metron-elasticsearch around log4j.
commit 54870d68e6f43e859367879bff7537f97c11d0bd
Author: Michael Miklavcic <mi...@...>
Date: 2018-10-24T00:33:25Z
Fixes for dep version issues.
commit 554de87ae160aae1ae85afb3dbb01a220a9d6838
Author: Nick Allen <ni...@...>
Date: 2018-10-25T18:54:41Z
METRON-1845 Correct Test Data Load in Elasticsearch Integration Tests
commit 3b95d1ece278bf517642a0b0a8eee5a7ea021b9e
Author: Nick Allen <ni...@...>
Date: 2018-10-25T19:06:39Z
Removed dead function
commit 7d9ee2563fdcc0e6b0a3d5106ea568577f21d303
Author: Nick Allen <ni...@...>
Date: 2018-10-26T18:55:03Z
Part way through Solr changes that I am going to roll-back
commit e6993867ebd00a5c09ef1a4da772664a11d1f212
Author: Nick Allen <ni...@...>
Date: 2018-10-26T18:55:15Z
Revert "Part way through Solr changes that I am going to roll-back"
This reverts commit 7d9ee2563fdcc0e6b0a3d5106ea568577f21d303.
commit dd024b818b238e24430385914cef6ea12873bfc6
Author: Michael Miklavcic <mi...@...>
Date: 2018-10-26T19:21:37Z
Merge branch 'master' into es-rest-client
commit 4a7a8370e61e427ac790d2e21bc43a55ac45636b
Author: Nick Allen <ni...@...>
Date: 2018-10-26T19:29:09Z
Resolved integration test differences with Solr
commit ebb7ef57c9fb20fb957b86516734fed1c18cda84
Author: Michael Miklavcic <mi...@...>
Date: 2018-10-26T21:10:54Z
Start addressing pr feedback.
commit 35811175824415c1e37418bb2efb347e6f7a801a
Author: Nick Allen <ni...@...>
Date: 2018-10-29T16:10:05Z
METRON-1849 Added integration test for UpdateDao.update() and UpdateDao.batchUpdate()
commit b7ac957164418d733e6c8fb8aa896186bbc5176f
Author: Michael Miklavcic <mi...@...>
Date: 2018-10-30T23:30:08Z
Addressing review feedback
commit c0faab2aaf04475ffc9e5558018b538fb62a3c7b
Author: Nick Allen <ni...@...>
Date: 2018-10-31T15:43:47Z
METRON-1849 Progress on shared write logic
commit 021810a0edb116c3b9bcf12cb8a77bf4197f24fb
Author: Nick Allen <ni...@...>
Date: 2018-10-31T20:51:32Z
METRON-1849 Progress on tests, still integration tests to fix-up
commit 056497034f7d0b5ebba7d8a66f1b6ff66ebfd119
Author: Nick Allen <ni...@...>
Date: 2018-11-01T13:57:06Z
METRON-1845 Added license header
commit f1c8ce4eb5d404f2a1d99e51e3111586221e13a0
Author: Nick Allen <ni...@...>
Date: 2018-11-01T21:36:56Z
Most tests working except for a few
commit 3e5c12169e8cea5da5349cf699b5519cdc82bca1
Author: Michael Miklavcic <mi...@...>
Date: 2018-11-02T04:56:55Z
Refactor clientfactory. Updates to metron-common, metron-elasticsearch, and kerberos/security documentation
commit d4b02ce80a6c72cdf4595f9a47a4955f59b0a5e4
Author: Nick Allen <ni...@...>
Date: 2018-11-02T20:12:52Z
Fixed all tests
commit fb5610b0aba13cdef7b6440e32e2c838bee3babb
Author: Nick Allen <ni...@...>
Date: 2018-11-02T20:22:09Z
Merge remote-tracking branch 'mike/es-rest-client' into METRON-1849
commit c3b28363e87f75e97bdc9ab14d30156136080af7
Author: Nick Allen <ni...@...>
Date: 2018-11-02T20:27:42Z
Fixed how client is created
commit 4539f43c43334696e33a861e7cbbb76a83c217b6
Author: Nick Allen <ni...@...>
Date: 2018-11-02T21:55:12Z
Improved docs
commit 32467b375f6e6fa3ac268fc0e5f4a6a447a48f76
Author: Nick Allen <ni...@...>
Date: 2018-11-05T16:59:26Z
Small cleanup
----
> Elasticsearch Index Write Functionality Should be Shared
> --------------------------------------------------------
>
> Key: METRON-1849
> URL: https://issues.apache.org/jira/browse/METRON-1849
> Project: Metron
> Issue Type: Bug
> Reporter: Nick Allen
> Assignee: Nick Allen
> Priority: Major
>
> The index write functionality is currently duplicated between the ElasticsearchWriter and the ElasticsearchUpdateDao. This functionality should be de-duplicated and shared between the two.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)