You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by merrimanr <gi...@git.apache.org> on 2017/11/01 21:25:49 UTC
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
GitHub user merrimanr opened a pull request:
https://github.com/apache/metron/pull/824
METRON-1289: Alert fields are lost when a MetaAlert is created
## Contributor Comments
This PR fixes a bug in the ElasticsearchMetaAlertDao that incorrectly updates the included alerts. To reproduce this error, pick any alert and record it's guid. Then create a metaalert with the http://node1:8082/swagger-ui.html#!/meta-alert-controller/createUsingPOST endpoint:
```
{
"groups": [
"string"
],
"guidToIndices": {"dcb3afed-1b68-d88a-7adb-f38183867920":"bro_index_2017.11.01.13"}
}
```
Now perform a lookup on the same alert with the http://node1:8082/swagger-ui.html#!/search-controller/getLatestUsingPOST endpoint:
```
{
"guid": "dcb3afed-1b68-d88a-7adb-f38183867920",
"sensorType": "bro"
}
```
The result only contains the "metaalert" field:
```
{
"metaalerts": [
"87ce1d82-aa09-4a1a-8be9-b9e7b76859b9"
]
}
```
This PR corrects this error by updating the alert with the full object instead of a partial object. After this PR, following the steps above should return the full object including the new "metaalert" field.
A `getAllLatest` function was added to make IndexDao match the pattern used for findOne. An HBase implementation was not added because it seemed unnecessary but it could be if it makes sense. Test cases were added to catch the error above and for the new `getAllLatest` function. This has been verified in full dev.
## Pull Request Checklist
Thank you for submitting a contribution to Apache Metron.
Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions.
Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) for complete smoke testing guides.
In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:
### For all changes:
- [x] 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).
- [x] 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.
- [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
### For code changes:
- [x] Have you included steps to reproduce the behavior or problem that is being changed or addressed?
- [x] Have you included steps or a guide to how the change may be verified and tested manually?
- [x] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
```
mvn -q clean integration-test install && build_utils/verify_licenses.sh
```
- [x] Have you written or updated unit tests and or integration tests to verify your changes?
- [x] 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)?
- [x] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
### For documentation related changes:
- [x] Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via `site-book/target/site/index.html`:
```
cd site-book
mvn site
```
#### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
It is also recommended that [travis-ci](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/merrimanr/incubator-metron METRON-1289
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/824.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 #824
----
commit c26a36f8d1a5d0a4cec9166f777013247b6ca199
Author: merrimanr <me...@gmail.com>
Date: 2017-11-01T21:09:42Z
initial commit
----
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/824
This looks good to me. I will not require attribution to my PR against this branch and I give this a +1 pending travis.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
@iraghumitra I'm unable to duplicate the grouping on the current code (although I admittedly ran through a pretty basic example). Here's what I did, so let me know if I missed something, or you have a particular query you made.
### Find an alert
```
/api/v1/search/search
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"fields": [
"guid"
],
"from": 0,
"indices": [
"snort"
],
"query": "ip_dst_addr:192.168.66.121",
"size": 1
}' 'http://node1:8082/api/v1/search/search'
```
In this case, I just grabbed one:
```
"d29925a7-da80-49d0-b7b5-0663380d526f":"snort_index_2017.11.14.15"
```
### Retrieve that alert from findOne to get something to group by
```
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "d29925a7-da80-49d0-b7b5-0663380d526f",
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/search/findOne'
```
I'll group, arbitrarily, with:
```
"ip_dst_addr": "192.168.66.121"
```
### Run a group by
```
/api/v1/search/group
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"groups": [
{
"field": "ip_dst_addr"
}
],
"indices": [
"snort",
"metaalert"
],
"query":"ip_dst_addr:192.168.66.121"
}' 'http://node1:8082/api/v1/search/group'
```
Results in:
```
{
"groupedBy": "ip_dst_addr",
"groupResults": [
{
"key": "192.168.66.121",
"total": 1299,
"score": null
}
]
}
```
### Create a metaalert with that GUID
```
/api/v1/metaalert/create
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"groups": [
"test"
],
"guidToIndices": {
"adf65dbb-73fd-4347-a61d-a990166fbbb1":"snort_index_2017.11.14.15"
}
}' 'http://node1:8082/api/v1/metaalert/create'
```
It returns the guid, in this case:
```
715538d9-7f4f-4316-b149-a9659df06de4
```
### (Optionally) Look up the metaalert to double check it was created as expected.
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "715538d9-7f4f-4316-b149-a9659df06de4",
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Run the group by from before
```
/api/v1/search/group
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"groups": [
{
"field": "ip_dst_addr"
}
],
"indices": [
"snort",
"metaalert"
],
"query":"ip_dst_addr:192.168.66.121"
}' 'http://node1:8082/api/v1/search/group'
```
Validate that the count has gone down by one (since the metaalert doesn't return and the child alert is now hidden:
```
{
"groupedBy": "ip_dst_addr",
"groupResults": [
{
"key": "3232252537",
"total": 1298,
"score": null
}
]
}
```
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150892216
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -135,8 +138,9 @@ private Document getDocumentFromResult(Result result) throws IOException {
Map.Entry<byte[], byte[]> entry= columns.lastEntry();
Long ts = Bytes.toLong(entry.getKey());
if(entry.getValue()!= null) {
- String json = new String(entry.getValue());
- return new Document(json, Bytes.toString(result.getRow()), null, ts);
+ Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()), new TypeReference<Map<String, Object>>() {
+ });
+ return new Document(json, Bytes.toString(result.getRow()), (String) json.get(SOURCE_TYPE), ts);
--- End diff --
Yeah, let's go with this. It solves several problems nicely
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150854643
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -135,8 +138,9 @@ private Document getDocumentFromResult(Result result) throws IOException {
Map.Entry<byte[], byte[]> entry= columns.lastEntry();
Long ts = Bytes.toLong(entry.getKey());
if(entry.getValue()!= null) {
- String json = new String(entry.getValue());
- return new Document(json, Bytes.toString(result.getRow()), null, ts);
+ Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()), new TypeReference<Map<String, Object>>() {
+ });
+ return new Document(json, Bytes.toString(result.getRow()), (String) json.get(SOURCE_TYPE), ts);
--- End diff --
I would prefer to see one of two things happen here. Either we keep the constant in the ES specific classes (which is admittedly less than ideal, but it does limit the pollution of ES knowledge into HBase classes) and populate source type from there (basically moving the loading and source type population there). Alternatively, we pass in a more general function that can be applied to the fields and configure and handle it appropriately.
I think the second one is probably more general useful to be able to do, but given the state of ES5 upgrade making this particular case obsolete, I'm amenable to doing the first option.
At bare minimum we should replace the '.'s with ':'s only if present. Even if there's not a Solr implementation, I don't want HBaseDao tied to ES so directly.
@cestella Do you have a preference on implementation? I know you'd had some comments earlier, but I don't want to put words in your mouth.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
## Removing alerts and removing an already removed alert
### Find two alerts
```
/api/v1/search/search
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"fields": [
"guid"
],
"from": 0,
"indices": [
"snort"
],
"query": "ip_dst_addr:192.168.66.121",
"size": 2
}' 'http://node1:8082/api/v1/search/search'
```
Results in two guids:
```
8b8314d4-277b-44dc-a75b-04b0cdcedb40
4ac26cf7-ab93-4940-9a0e-8e7f4d67736d
```
### Create a metaalert with the alerts
```
/api/v1/metaalert/create
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"alerts": [
{
"guid": "8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid":"4ac26cf7-ab93-4940-9a0e-8e7f4d67736d",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
}
],
"groups": [
"test"
]
}' 'http://node1:8082/api/v1/metaalert/create'
```
Make sure to get the resulting guid from the response.
```
b25b663e-39c9-42d5-a52c-e6380235d43f
```
### Retrieve the meta alert and ensure it contains the provided alerts
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "b25b663e-39c9-42d5-a52c-e6380235d43f",
"index": "metaalert_index",
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Remove one of the alerts
```
/api/v1/metaalert/remove/alert
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"alerts": [
{
"guid": "8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
}
],
"metaAlertGuid": "b25b663e-39c9-42d5-a52c-e6380235d43f"
}' 'http://node1:8082/api/v1/metaalert/remove/alert'
```
### Retrieve the meta alert again, and ensure it only contains the second alert.
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "b25b663e-39c9-42d5-a52c-e6380235d43f",
"index": "metaalert_index",
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Rerun the delete
```
/api/v1/metaalert/remove/alert
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"alerts": [
{
"guid": "8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
}
],
"metaAlertGuid": "b25b663e-39c9-42d5-a52c-e6380235d43f"
}' 'http://node1:8082/api/v1/metaalert/remove/alert'
```
### Retrieve the meta alert again, and ensure it only contains the second alert.
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "b25b663e-39c9-42d5-a52c-e6380235d43f",
"index": "metaalert_index",
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Retrieve the child alerts
Ensure only the second alert has the 'metaalerts' field populated with the parent met alert.
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid":"8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"sensorType": "snort"
}' 'http://node1:8082/api/v1/search/findOne'
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid":"4ac26cf7-ab93-4940-9a0e-8e7f4d67736d",
"sensorType": "snort"
}' 'http://node1:8082/api/v1/search/findOne'
```
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/824
The latest commits address some outstanding tasks and address a couple other issues:
- disabling updates to meta alert objects means we can't update a meta alert name or comments (or other fields that may be added in the future). The patch implementation was added to the ElasticsearchMetaAlertDao to allow updates to fields other than `alert` and `status` which require special handling and have dedicated endpoints
- a `getAllLatest` implementation was added to the HBaseDao along with an integration test
- a bug was discovered where the number of search results returned defaults to 10 meaning changes to alerts are only propagated to the first 10 meta alerts and vice-versa. This was corrected in the ElasticsearchDao.searchByGuids by explicitly setting the size and fixed in the alert/meta alert lookup queries by paging through all the results if necessary. A test was added to the integration test to catch this case in the future.
I feel like this PR is in pretty good shape from a functional perspective. Still working on javadocs/documentation but it's ready for functional testing/code review.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/824
@justinleet looking at your results I would expect the count to go down by 2 instead of 1: 1 for the alert that was added to the meta alert and another for the meta alert itself since they shouldn't be included in group by queries. I think the bug @iraghumitra reported is valid.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/824
No problem. I am still actively working on this PR and will add the HBaseDao.findAllLatest implementation task to my list.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
Do we know why the partial update doesn't work? I'm not necessarily opposed to doing this as a short term fix, but I'd like to know root cause.
It seems like overkill to submit a full object update on every alert that goes into a metaalert (e.g. that could be a hundred+ full document reindexes happening every time)
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150239607
--- Diff: metron-interface/metron-rest/src/main/java/org/apache/metron/rest/controller/MetaAlertController.java ---
@@ -60,5 +63,37 @@
) throws RestException {
return new ResponseEntity<>(metaAlertService.create(createRequest), HttpStatus.OK);
}
+
+ @ApiOperation(value = "Create a meta alert")
+ @ApiResponse(message = "Created meta alert", code = 200)
+ @RequestMapping(value = "/add/alert", method = RequestMethod.POST)
+ ResponseEntity<Boolean> addAlertsToMetaAlert(
+ @ApiParam(name = "request", value = "Meta Alert Create Request", required = true)
+ @RequestBody final MetaAlertAddRemoveRequest metaAlertAddRemoveRequest
+ ) throws RestException {
+ return new ResponseEntity<>(metaAlertService.addAlertsToMetaAlert(metaAlertAddRemoveRequest), HttpStatus.OK);
+ }
+
+ @ApiOperation(value = "Create a meta alert")
+ @ApiResponse(message = "Created meta alert", code = 200)
+ @RequestMapping(value = "/remove/alert", method = RequestMethod.POST)
+ ResponseEntity<Boolean> removeAlertsFromMetaAlert(
+ @ApiParam(name = "request", value = "Meta Alert Create Request", required = true)
+ @RequestBody final MetaAlertAddRemoveRequest metaAlertAddRemoveRequest
+ ) throws RestException {
+ return new ResponseEntity<>(metaAlertService.removeAlertsFromMetaAlert(metaAlertAddRemoveRequest), HttpStatus.OK);
+ }
+
+ @ApiOperation(value = "Create a meta alert")
+ @ApiResponse(message = "Created meta alert", code = 200)
+ @RequestMapping(value = "/update/status/{guid}/{status}", method = RequestMethod.POST)
+ ResponseEntity<Boolean> updateMetaAlertStatus(
+ final @ApiParam(name = "guid", value = "Kafka topic name", required = true)
+ @PathVariable String guid,
+ final @ApiParam(name = "status", value = "Kafka topic name", required = true)
+ @PathVariable String status) throws RestException {
+ return new ResponseEntity<>(metaAlertService.updateMetaAlertStatus(guid,
+ MetaAlertStatus.valueOf(status.toUpperCase())), HttpStatus.OK);
+ }
--- End diff --
I think we'll also need to be able to add comments and a name to the metaalert as a whole, to support work in https://github.com/apache/metron/pull/803/
@iraghumitra Is there anything else needed in terms of updates on meta alerts themselves (underlying alerts aren't affected by this stuff)?
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150998084
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -138,9 +142,17 @@ private Document getDocumentFromResult(Result result) throws IOException {
Map.Entry<byte[], byte[]> entry= columns.lastEntry();
Long ts = Bytes.toLong(entry.getKey());
if(entry.getValue()!= null) {
- Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()), new TypeReference<Map<String, Object>>() {
- });
- return new Document(json, Bytes.toString(result.getRow()), (String) json.get(SOURCE_TYPE), ts);
+ Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()),
+ new TypeReference<Map<String, Object>>() {});
+ ByteArrayInputStream baos = new ByteArrayInputStream(result.getRow());
--- End diff --
Ok, I submitted a [PR](https://github.com/merrimanr/incubator-metron/pull/34) against your branch that abstracts it out the key into an object and adds some explicit unit tests around it.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
@merrimanr Can you merge master into this PR? There is at least one fix in here that caused me problems spinning this up that's in master, so I'd like to have it pulled in so testing is as stable as possible.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/824
The latest commit fixes the bug in the previous comment. The root cause was that HBaseDao.getAllLatest was not properly returning the sensor type and causing subsequent updates in Elasticsearch to fail. The solution was simple: look up the sensor type from the document and add it to the return Document. I added a constant for this field (essentially a copy of ElasticsearchMetaAlertDao.SOURCE_TYPE) to the HBaseDao in anticipation of the ES 5 upgrade where it will no longer be required to change .'s to :'s. At that point we can just change the constant to Constants.SENSOR_TYPE and it should continue to function.
There are a couple other solutions to this problem that I can think of:
- change the getAllLatest interface to include guid/sensorType mappings instead of separate guid and sensorType lists
- add a column family to store the sensor type
- other more complicated ways of getting the correct sensor type field name
I felt this temporary constant was the simplest but feel free to give opinions on other options. I also update the HBaseDaoIntegrationTest to expect this field to be returned.
The PR description has been updated to reflect the various interface changes and new testing procedure.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
## Patch alert and status
Create a metaalerts and get the GUID for the following steps.
### Attempt to update status field
```
/api/v1/update/patch
curl -X PATCH --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "00eae5ba-6137-4601-ae3a-fbf0003e58e6",
"index": "metaalert_index",
"patch": [
{
"op": "replace"
, "path": "/status"
, "value": "failure"
}
],
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/update/patch'
```
Should return
```
{
"responseCode": 500,
"message": "Meta alert patches are not allowed for /alert or /status paths. Please use the add/remove alert or update status functions instead.",
"fullMessage": "IllegalArgumentException: Meta alert patches are not allowed for /alert or /status paths. Please use the add/remove alert or update status functions instead."
}
```
### Attempt to update the alert list
```
/api/v1/update/patch
curl -X PATCH --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "00eae5ba-6137-4601-ae3a-fbf0003e58e6",
"index": "metaalert_index",
"patch": [
{
"op": "replace"
, "path": "/alert"
, "value": [{
"alertOne":"test"
},
{
"alertTwo":"test"
}
]
}
],
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/update/patch'
```
Should return
```
{
"responseCode": 500,
"message": "Meta alert patches are not allowed for /alert or /status paths. Please use the add/remove alert or update status functions instead.",
"fullMessage": "IllegalArgumentException: Meta alert patches are not allowed for /alert or /status paths. Please use the add/remove alert or update status functions instead."
}
```
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150644110
--- Diff: metron-platform/metron-indexing/src/test/java/org/apache/metron/indexing/dao/InMemoryMetaAlertDao.java ---
@@ -200,4 +207,23 @@ public MetaAlertCreateResponse createMetaAlert(MetaAlertCreateRequest request)
createResponse.setCreated(true);
return createResponse;
}
+
+ @Override
+ public boolean addAlertsToMetaAlert(String metaAlertGuid, Collection<String> alertGuids,
+ Collection<String> sensorTypes) throws IOException {
+ return true;
--- End diff --
I think @cestella's suggestion is good, I will add some REST integration tests that ensure request/responses are serialized/deserialized correctly. There are tests included in MetaAlertControllerIntegrationTest that exercise these functions.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/824
@justinleet same here. My +1 is imminent pending docs.
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150889867
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -135,8 +138,9 @@ private Document getDocumentFromResult(Result result) throws IOException {
Map.Entry<byte[], byte[]> entry= columns.lastEntry();
Long ts = Bytes.toLong(entry.getKey());
if(entry.getValue()!= null) {
- String json = new String(entry.getValue());
- return new Document(json, Bytes.toString(result.getRow()), null, ts);
+ Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()), new TypeReference<Map<String, Object>>() {
+ });
+ return new Document(json, Bytes.toString(result.getRow()), (String) json.get(SOURCE_TYPE), ts);
--- End diff --
I like this solution much better.
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r149422318
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/Constants.java ---
@@ -29,6 +29,7 @@
public static final String ZOOKEEPER_TOPOLOGY_ROOT = ZOOKEEPER_ROOT + "/topology";
public static final long DEFAULT_CONFIGURED_BOLT_TIMEOUT = 5000;
public static final String SENSOR_TYPE = "source.type";
+ public static final String SOURCE_TYPE = SENSOR_TYPE.replace('.', ':');
--- End diff --
This doesn't feel like it should exist in Constants.java as it's very ES specific (solr would not work this way, for instance). Maybe we could have an ElasticsearchConstants or just move this to a constant in the ESDao?
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150872247
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -135,8 +138,9 @@ private Document getDocumentFromResult(Result result) throws IOException {
Map.Entry<byte[], byte[]> entry= columns.lastEntry();
Long ts = Bytes.toLong(entry.getKey());
if(entry.getValue()!= null) {
- String json = new String(entry.getValue());
- return new Document(json, Bytes.toString(result.getRow()), null, ts);
+ Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()), new TypeReference<Map<String, Object>>() {
+ });
+ return new Document(json, Bytes.toString(result.getRow()), (String) json.get(SOURCE_TYPE), ts);
--- End diff --
I agree with @cestella, I'd rather use the existing abstraction. Setting a field transformer is a cleaner way of doing things, in my opinion, than requiring the user to know know and input where all the GUIDs are coming from.
Another concern about ES5 is that PR may live for awhile, even after we're fairly confident in it (there's going to be a lot of testing involved there). Casey's PR is a pretty simple way of handling it (set a config, pass it in, use it basically the same way you set it up).
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150239332
--- Diff: metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchDao.java ---
@@ -256,59 +256,91 @@ public Document getLatest(final String guid, final String sensorType) throws IOE
return ret.orElse(null);
}
+ @Override
+ public Iterable<Document> getAllLatest(final Collection<String> guids, final Collection<String> sensorTypes) throws IOException {
+ List<Document> documents = searchByGuids(
+ guids
+ , sensorTypes
+ , hit -> {
+ Long ts = 0L;
+ String doc = hit.getSourceAsString();
+ String sourceType = Iterables.getFirst(Splitter.on("_doc").split(hit.getType()), null);
+ try {
+ return Optional.of(new Document(doc, hit.getId(), sourceType, ts));
+ } catch (IOException e) {
+ throw new IllegalStateException("Unable to retrieve latest: " + e.getMessage(), e);
+ }
+ }
+
+ );
+ return documents;
+ }
+
+ <T> Optional<T> searchByGuid(String guid, String sensorType,
+ Function<SearchHit, Optional<T>> callback) {
+ Collection<String> sensorTypes = sensorType != null ? Collections.singleton(sensorType) : null;
+ List<T> results = searchByGuids(Collections.singleton(guid), sensorTypes, callback);
+ if (results.size() > 0) {
+ return Optional.of(results.get(0));
+ } else {
+ return Optional.empty();
+ }
+ }
+
/**
* Return the search hit based on the UUID and sensor type.
* A callback can be specified to transform the hit into a type T.
* If more than one hit happens, the first one will be returned.
*/
- <T> Optional<T> searchByGuid(String guid, String sensorType,
+ <T> List<T> searchByGuids(Collection<String> guids, Collection<String> sensorTypes,
Function<SearchHit, Optional<T>> callback) {
QueryBuilder query;
- if (sensorType != null) {
- query = QueryBuilders.idsQuery(sensorType + "_doc").ids(guid);
+ if (sensorTypes != null) {
+ String[] types = sensorTypes.stream().map(sensorType -> sensorType + "_doc").toArray(String[]::new);
+ query = QueryBuilders.idsQuery(types).ids(guids);
} else {
- query = QueryBuilders.idsQuery().ids(guid);
+ query = QueryBuilders.idsQuery().ids(guids);
}
SearchRequestBuilder request = client.prepareSearch()
--- End diff --
Good catch. I will add a fix and test case.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/824
I also found what I believe are 2 fairly significant architectural issues with our DAO abstraction:
- https://issues.apache.org/jira/browse/METRON-1314
- https://issues.apache.org/jira/browse/METRON-1315
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r149423305
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/IndexDao.java ---
@@ -65,6 +65,8 @@
*/
Document getLatest(String guid, String sensorType) throws IOException;
+ Iterable<Document> getAllLatest(Map<String, String> guidToIndices) throws IOException;
--- End diff --
Can we add a javadoc similar to getLatest?
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150628792
--- Diff: metron-interface/metron-rest/src/main/java/org/apache/metron/rest/controller/MetaAlertController.java ---
@@ -60,5 +63,37 @@
) throws RestException {
return new ResponseEntity<>(metaAlertService.create(createRequest), HttpStatus.OK);
}
+
+ @ApiOperation(value = "Create a meta alert")
+ @ApiResponse(message = "Created meta alert", code = 200)
+ @RequestMapping(value = "/add/alert", method = RequestMethod.POST)
+ ResponseEntity<Boolean> addAlertsToMetaAlert(
+ @ApiParam(name = "request", value = "Meta Alert Create Request", required = true)
+ @RequestBody final MetaAlertAddRemoveRequest metaAlertAddRemoveRequest
+ ) throws RestException {
+ return new ResponseEntity<>(metaAlertService.addAlertsToMetaAlert(metaAlertAddRemoveRequest), HttpStatus.OK);
+ }
+
+ @ApiOperation(value = "Create a meta alert")
+ @ApiResponse(message = "Created meta alert", code = 200)
+ @RequestMapping(value = "/remove/alert", method = RequestMethod.POST)
+ ResponseEntity<Boolean> removeAlertsFromMetaAlert(
+ @ApiParam(name = "request", value = "Meta Alert Create Request", required = true)
+ @RequestBody final MetaAlertAddRemoveRequest metaAlertAddRemoveRequest
+ ) throws RestException {
+ return new ResponseEntity<>(metaAlertService.removeAlertsFromMetaAlert(metaAlertAddRemoveRequest), HttpStatus.OK);
+ }
+
+ @ApiOperation(value = "Create a meta alert")
+ @ApiResponse(message = "Created meta alert", code = 200)
+ @RequestMapping(value = "/update/status/{guid}/{status}", method = RequestMethod.POST)
+ ResponseEntity<Boolean> updateMetaAlertStatus(
+ final @ApiParam(name = "guid", value = "Kafka topic name", required = true)
+ @PathVariable String guid,
+ final @ApiParam(name = "status", value = "Kafka topic name", required = true)
+ @PathVariable String status) throws RestException {
+ return new ResponseEntity<>(metaAlertService.updateMetaAlertStatus(guid,
+ MetaAlertStatus.valueOf(status.toUpperCase())), HttpStatus.OK);
+ }
--- End diff --
We will need to add sections for these in `metron-interface/metron-rest/README.md`
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150868283
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -135,8 +138,9 @@ private Document getDocumentFromResult(Result result) throws IOException {
Map.Entry<byte[], byte[]> entry= columns.lastEntry();
Long ts = Bytes.toLong(entry.getKey());
if(entry.getValue()!= null) {
- String json = new String(entry.getValue());
- return new Document(json, Bytes.toString(result.getRow()), null, ts);
+ Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()), new TypeReference<Map<String, Object>>() {
+ });
+ return new Document(json, Bytes.toString(result.getRow()), (String) json.get(SOURCE_TYPE), ts);
--- End diff --
I will amend my constant briefly in that I don't like the first option there as the major flaw in all of these designs is coupling the ES logic within HBase logic. I think given this is all going away after ES 5 (IMO), we should probably just use the existing abstractions used in the writers to handle field name transformations here where we need to transform field names.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
@merrimanr I'm okay with excluding metaalerts (although I need to review what you did there). I wouldn't expect it to go down by two though.
Say I have two matches, I put one in a metaalert (so it should be hidden). I make the query again. I would still expect to get the remaining, standalone match. The metaalert should never have showed up and the child alert should be hidden. So one result.
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150874132
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -135,8 +138,9 @@ private Document getDocumentFromResult(Result result) throws IOException {
Map.Entry<byte[], byte[]> entry= columns.lastEntry();
Long ts = Bytes.toLong(entry.getKey());
if(entry.getValue()!= null) {
- String json = new String(entry.getValue());
- return new Document(json, Bytes.toString(result.getRow()), null, ts);
+ Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()), new TypeReference<Map<String, Object>>() {
+ });
+ return new Document(json, Bytes.toString(result.getRow()), (String) json.get(SOURCE_TYPE), ts);
--- End diff --
That seems generally useful but it's a pretty significant change (several files and several lines of code) plus it adds another setting to our config. There is no unit test for IndexConfig so it also adds untested code (or would require writing a new test).
I prefer a simple 2-3 line change that we can easily revert when ES 5 upgrade makes it in. This should be a separate PR in my opinion. This PR is already very large and complicated.
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r149423571
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/IndexDao.java ---
@@ -65,6 +65,8 @@
*/
Document getLatest(String guid, String sensorType) throws IOException;
+ Iterable<Document> getAllLatest(Map<String, String> guidToIndices) throws IOException;
--- End diff --
Actually, even a default implementation here that calls getLatest might be useful.
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150994812
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -138,9 +142,17 @@ private Document getDocumentFromResult(Result result) throws IOException {
Map.Entry<byte[], byte[]> entry= columns.lastEntry();
Long ts = Bytes.toLong(entry.getKey());
if(entry.getValue()!= null) {
- Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()), new TypeReference<Map<String, Object>>() {
- });
- return new Document(json, Bytes.toString(result.getRow()), (String) json.get(SOURCE_TYPE), ts);
+ Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()),
+ new TypeReference<Map<String, Object>>() {});
+ ByteArrayInputStream baos = new ByteArrayInputStream(result.getRow());
--- End diff --
Can we pull this out into a `readRowKey()` method, like we have a `buildRowKey()`?
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150230733
--- Diff: metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchDao.java ---
@@ -256,59 +256,91 @@ public Document getLatest(final String guid, final String sensorType) throws IOE
return ret.orElse(null);
}
+ @Override
+ public Iterable<Document> getAllLatest(final Collection<String> guids, final Collection<String> sensorTypes) throws IOException {
+ List<Document> documents = searchByGuids(
+ guids
+ , sensorTypes
+ , hit -> {
+ Long ts = 0L;
+ String doc = hit.getSourceAsString();
+ String sourceType = Iterables.getFirst(Splitter.on("_doc").split(hit.getType()), null);
+ try {
+ return Optional.of(new Document(doc, hit.getId(), sourceType, ts));
+ } catch (IOException e) {
+ throw new IllegalStateException("Unable to retrieve latest: " + e.getMessage(), e);
+ }
+ }
+
+ );
+ return documents;
+ }
+
+ <T> Optional<T> searchByGuid(String guid, String sensorType,
+ Function<SearchHit, Optional<T>> callback) {
+ Collection<String> sensorTypes = sensorType != null ? Collections.singleton(sensorType) : null;
+ List<T> results = searchByGuids(Collections.singleton(guid), sensorTypes, callback);
+ if (results.size() > 0) {
+ return Optional.of(results.get(0));
+ } else {
+ return Optional.empty();
+ }
+ }
+
/**
* Return the search hit based on the UUID and sensor type.
* A callback can be specified to transform the hit into a type T.
* If more than one hit happens, the first one will be returned.
*/
- <T> Optional<T> searchByGuid(String guid, String sensorType,
+ <T> List<T> searchByGuids(Collection<String> guids, Collection<String> sensorTypes,
Function<SearchHit, Optional<T>> callback) {
QueryBuilder query;
- if (sensorType != null) {
- query = QueryBuilders.idsQuery(sensorType + "_doc").ids(guid);
+ if (sensorTypes != null) {
+ String[] types = sensorTypes.stream().map(sensorType -> sensorType + "_doc").toArray(String[]::new);
+ query = QueryBuilders.idsQuery(types).ids(guids);
} else {
- query = QueryBuilders.idsQuery().ids(guid);
+ query = QueryBuilders.idsQuery().ids(guids);
}
SearchRequestBuilder request = client.prepareSearch()
--- End diff --
This is only going to return 10 max results, from the default size on the query. This was fine before (since we only returned one), but it's going to be incomplete with the refactor.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/824
The 2nd most recent commits updates the MetaAlertDao interface by adding additional methods for interacting with metaalerts. This should make it easier to use and make the underlying implementation simpler and easier to understand by eliminating the need for complex logic that must catch all of the different use cases in the MetaAlertDao.update. The high-level changes include:
- dedicated methods that allow adding/removing alerts to/from a metaalert
- dedicated method for changing the status of a metaalert
- direct updates through IndexDao.update are no longer allowed on metaalerts, the other MetaAlertDao methods must be used instead
The latest commit updates the ElasticsearchMetaAlertIntegrationTest with several changes:
- test data setup and test style is now consistent across all tests
- test cases for the new interface methods were added
- old test cases were reconciled with the new test cases
- test coverage for ElasticsearchMetaAlertDao is now close to 100% with tests added for all the bugs found so far
I am still working on adding these changes:
- updated javadocs and comments in all relevant classes and tests
- an HBaseDao.getAllLatest implementation and test
- updated ElasticsearchMetaAlertDaoTest adapted for the new MetaAlertDao interface (it's currently all commented out for the purpose of running the integration tests)
For now reviewers can start looking at the new MetaAlertDao interface methods and the accompanying integration tests in ElasticsearchMetaAlertIntegrationTest. Hope to have the rest pushed out soon.
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r151203066
--- Diff: metron-interface/metron-rest/src/main/java/org/apache/metron/rest/controller/MetaAlertController.java ---
@@ -60,5 +63,37 @@
) throws RestException {
return new ResponseEntity<>(metaAlertService.create(createRequest), HttpStatus.OK);
}
+
+ @ApiOperation(value = "Create a meta alert")
--- End diff --
The descriptions and so on need to be updated on all of these. I know you're working on documentation, but I wanted to call it out so it doesn't slip through.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
+1, looks good. Thanks for all the work on the supplemental fixes. Feel free to skip attribution on the testing PR.
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150887809
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -135,8 +138,9 @@ private Document getDocumentFromResult(Result result) throws IOException {
Map.Entry<byte[], byte[]> entry= columns.lastEntry();
Long ts = Bytes.toLong(entry.getKey());
if(entry.getValue()!= null) {
- String json = new String(entry.getValue());
- return new Document(json, Bytes.toString(result.getRow()), null, ts);
+ Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()), new TypeReference<Map<String, Object>>() {
+ });
+ return new Document(json, Bytes.toString(result.getRow()), (String) json.get(SOURCE_TYPE), ts);
--- End diff --
The problem with the "we're going to revert after ES 5" argument is that it is likely not going to be immediate. Until then we have coupled HBaseDao and this feels like a kludge.
If you don't like this solution, can I propose a compromise that will sidestep this entirely? How about changing the hbase key in HBaseDao to pass along the sensor (guid + sensor type). This has a couple of advantages:
* It requires no parsing of the document as it can retrieve the sensor type from the key
* It does not couple our field name conversion from ES with HBase in any way
* It's a small change
I think this neatly sidesteps the issue. Thoughts?
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150866003
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -135,8 +138,9 @@ private Document getDocumentFromResult(Result result) throws IOException {
Map.Entry<byte[], byte[]> entry= columns.lastEntry();
Long ts = Bytes.toLong(entry.getKey());
if(entry.getValue()!= null) {
- String json = new String(entry.getValue());
- return new Document(json, Bytes.toString(result.getRow()), null, ts);
+ Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()), new TypeReference<Map<String, Object>>() {
+ });
+ return new Document(json, Bytes.toString(result.getRow()), (String) json.get(SOURCE_TYPE), ts);
--- End diff --
Option 1 is not possible the way things are now. This constant is already in an ES specific class in metron-elasticsearch but metron-elasticsearch depends on metron-indexing. To get access to that constant we would need to add metron-elasticsearch as a dependency to metron-indexing thus creating a circular dependency.
Option 2 would be my least favorite option and I would rather just change the method signature of getAllLatest to include all guid/sensorType relationships.
Since we currently have a PR in review for the ES 5 upgrade that will allow us to just remove this constant, I don't feel like we should spend much time on it. I would also argue that this was an issue long before this PR so anything more than a simple workaround should be a follow-on.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
Double check me on that logic though. I could definitely be masking an off by one error there.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/824
The latest commit fixes the group by bug found by @iraghumitra and addresses the feedback [above](https://github.com/apache/metron/pull/824#pullrequestreview-76466048).
The group by fix was simple. We are now just excluding the "metaalert" sensor type from group by queries.
Refactoring the HBaseDao was more involved. One of the side effects of storing the sensorType in the row key is that now we require a map of guids to sensorTypes for an IndexDao.getAllLatest call. These means the IndexDao.getAllLatest interface needed to change. It made sense to just create a standard interface that can be used for looking up records since it is used in many different places and can be confusing as to what is needed to perform an operation on a document. So this PR introduces a GetRequest that has the following structure:
```
{
"guid": "some guid",
"sensorType" : " some sensor type",
"index": "some optional index, not required"
}
```
Since now keep the sensorType in the row key it is required for a look up. HBase still doesn't have any concept of an index so that is kept optional. In the ElasticsearchDao this is handled by using the index of a GetRequest if present or looking it up in Elasticsearch if not. Now it should be clear what is needed to get a document since the interface is more consistent across that DAO methods.
Because of the GetRequest change the testing process and REST interface has changed slightly. The PR description has been updated to reflect the changes. I am still planning on doing a fresh vagrant build but this should be ready for testing.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
## Changing Metaalert status
### Find two alerts
```
/api/v1/search/search
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"fields": [
"guid"
],
"from": 0,
"indices": [
"snort"
],
"query": "ip_dst_addr:192.168.66.121",
"size": 2
}' 'http://node1:8082/api/v1/search/search'
```
Results in two guids:
```
8b8314d4-277b-44dc-a75b-04b0cdcedb40
4ac26cf7-ab93-4940-9a0e-8e7f4d67736d
```
### Create a metaalert with the alerts
```
/api/v1/metaalert/create
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"alerts": [
{
"guid": "8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid":"4ac26cf7-ab93-4940-9a0e-8e7f4d67736d",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
}
],
"groups": [
"test"
]
}' 'http://node1:8082/api/v1/metaalert/create'
```
Make sure to get the resulting guid from the response.
```
da60ccc9-9e79-45c5-be07-0a322c8791f0
```
### Retrieve the meta alert and ensure it contains the provided alerts
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "da60ccc9-9e79-45c5-be07-0a322c8791f0",
"index": "metaalert_index",
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Change the meta alert status to active
This makes sure nothing happens when we set active status to the same active status.
```
/api/v1/metaalert/update/status/{guid}/{status}
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' 'http://node1:8082/api/v1/metaalert/update/status/da60ccc9-9e79-45c5-be07-0a322c8791f0/active'
```
It should return false, as no status has changed.
### Retrieve the metaalert and ensure it is still active
Look for the 'status' field.
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "da60ccc9-9e79-45c5-be07-0a322c8791f0",
"index": "metaalert_index",
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Retrieve the child alerts
Ensure both alerts have the 'metaalerts' field populated with the parent meta alert.
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid":"8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"sensorType": "snort"
}' 'http://node1:8082/api/v1/search/findOne'
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid":"4ac26cf7-ab93-4940-9a0e-8e7f4d67736d",
"sensorType": "snort"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Change the meta alert status to inactive
Look for the 'status' field. Running this once will set it to 'inactive'. Subsequent runs have no effect ('inactive' -> 'inactive' does nothing).
```
/api/v1/metaalert/update/status/{guid}/{status}
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' 'http://node1:8082/api/v1/metaalert/update/status/da60ccc9-9e79-45c5-be07-0a322c8791f0/inactive'
```
It should return true, because the status has changed.
### Retrieve the metaalert and ensure it is inactive
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "da60ccc9-9e79-45c5-be07-0a322c8791f0",
"index": "metaalert_index",
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Retrieve the child alerts
Ensure neither alert has the 'metaalerts' field populated with the parent meta alert.
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid":"8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"sensorType": "snort"
}' 'http://node1:8082/api/v1/search/findOne'
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid":"4ac26cf7-ab93-4940-9a0e-8e7f4d67736d",
"sensorType": "snort"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Change status back to active
```
/api/v1/metaalert/update/status/{guid}/{status}
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' 'http://node1:8082/api/v1/metaalert/update/status/da60ccc9-9e79-45c5-be07-0a322c8791f0/active'
```
It should return false, as no status has changed.
### Retrieve the metaalert and ensure it is active again
Look for the 'status' field.
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "da60ccc9-9e79-45c5-be07-0a322c8791f0",
"index": "metaalert_index",
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Retrieve the child alerts
Ensure both alerts have the 'metaalerts' field populated with the parent meta alert.
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid":"8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"sensorType": "snort"
}' 'http://node1:8082/api/v1/search/findOne'
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid":"4ac26cf7-ab93-4940-9a0e-8e7f4d67736d",
"sensorType": "snort"
}' 'http://node1:8082/api/v1/search/findOne'
```
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by iraghumitra <gi...@git.apache.org>.
Github user iraghumitra commented on the issue:
https://github.com/apache/metron/pull/824
@merrimanr @justinleet tested the new API's they are working fine the only issue I see is
>>When a meta alert is created the meta alert is returned as part of the group api which was not the case before.
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/metron/pull/824
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150638251
--- Diff: metron-platform/metron-indexing/src/test/java/org/apache/metron/indexing/dao/InMemoryMetaAlertDao.java ---
@@ -200,4 +207,23 @@ public MetaAlertCreateResponse createMetaAlert(MetaAlertCreateRequest request)
createResponse.setCreated(true);
return createResponse;
}
+
+ @Override
+ public boolean addAlertsToMetaAlert(String metaAlertGuid, Collection<String> alertGuids,
+ Collection<String> sensorTypes) throws IOException {
+ return true;
--- End diff --
Given the non-implementation of these, do we not have any integration tests in metron-rest exercising the add/remove alerts calls?
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by iraghumitra <gi...@git.apache.org>.
Github user iraghumitra commented on the issue:
https://github.com/apache/metron/pull/824
+1 for functionality all the API's are working fine. Great job @merrimanr 👍
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by iraghumitra <gi...@git.apache.org>.
Github user iraghumitra commented on the issue:
https://github.com/apache/metron/pull/824
I have tested the rest-api I am able to see the alert details for all alerts under meta alerts
I see a couple of new issues:
- When a meta alert is created the meta alert is returned as part of the group api which was not the case before.
- When a meta alert is deleted (status set to inactive) the meta alert gets deleted but the details are missing for all alerts under meta alert. I feel this bug might be in there all along but never surfaced since the details were missing when the meta alert is created. It would be nice if you can handle this in this PR.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
@merrimanr For the findAllLatest, can we add a default implementation to the interface that would cover HBase (i.e. just do a for loop lookup), even if it's not efficient. Otherwise, it should just be a MultiGet right if we want to do something more efficient / HBase specific, right?
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/824
whoops, nevermind, I see you already merged master. my bad. :)
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
@iraghumitra Do you have the specific metaalerts and calls that you made? I'm spinning this up again, but it'll be a bit before I can test something myself.
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150867558
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -135,8 +138,9 @@ private Document getDocumentFromResult(Result result) throws IOException {
Map.Entry<byte[], byte[]> entry= columns.lastEntry();
Long ts = Bytes.toLong(entry.getKey());
if(entry.getValue()!= null) {
- String json = new String(entry.getValue());
- return new Document(json, Bytes.toString(result.getRow()), null, ts);
+ Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()), new TypeReference<Map<String, Object>>() {
+ });
+ return new Document(json, Bytes.toString(result.getRow()), (String) json.get(SOURCE_TYPE), ts);
--- End diff --
I submitted a PR against this PR with my proposed solution of coupling the HBaseDao with the Elasticsearch specific field name transformation logic. I propose that we do this:
* Migrate the FieldNameConverter as a field in the REST config
* Make the IndexDao take the FieldNameConverter in the AccessConfig
* Have the HBaseDao use the parameterized field name converter to transform `source.type` to the appropriate field.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
Code-wise, I'm pretty good at this point. Once the docs come in, I'll give them a once-over and hopefully we're good to go soon.
Thanks a lot for the hard work here!
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
## Adding alerts and adding a preexisting alert
### Find two alerts
```
/api/v1/search/search
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"fields": [
"guid"
],
"from": 0,
"indices": [
"snort"
],
"query": "ip_dst_addr:192.168.66.121",
"size": 2
}' 'http://node1:8082/api/v1/search/search'
```
Results in two guids:
```
8b8314d4-277b-44dc-a75b-04b0cdcedb40
4ac26cf7-ab93-4940-9a0e-8e7f4d67736d
```
### Create a metaalert with only one of the alerts
```
/api/v1/metaalert/create
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"alerts": [
{
"guid": "8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
}
],
"groups": [
"test"
]
}' 'http://node1:8082/api/v1/metaalert/create'
```
Make sure to get the resulting guid from the response.
```
6a4affe4-02ce-4d25-80b1-bfc4ca53f557
```
### Retrieve the meta alert and ensure it contains the provided alert
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "6a4affe4-02ce-4d25-80b1-bfc4ca53f557",
"index": "metaalert_index",
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Retrieve the child alert and ensure the 'metaalerts' field contains the new GUID of the new metaalert
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid":"8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"sensorType": "snort"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Add the same alert to the meta alert
```
/api/v1/metaalert/add/alert
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"alerts": [
{
"guid": "8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
}
],
"metaAlertGuid": "6a4affe4-02ce-4d25-80b1-bfc4ca53f557"
}' 'http://node1:8082/api/v1/metaalert/add/alert'
```
It should return "false" as no alerts have been added. The meta alert should be retrieved again to validate.
### Run the add alert again but with the second alert
```
/api/v1/metaalert/add/alert
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"alerts": [
{
"guid": "8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid":"4ac26cf7-ab93-4940-9a0e-8e7f4d67736d",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
}
],
"metaAlertGuid": "6a4affe4-02ce-4d25-80b1-bfc4ca53f557"
}' 'http://node1:8082/api/v1/metaalert/add/alert'
```
It should return true, because the second alert will be added. The meta alert should be retrieved again to validate.
### Retrieve the child alerts
Ensure they have the 'metaalerts' field populated with their parent.
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid":"8b8314d4-277b-44dc-a75b-04b0cdcedb40",
"sensorType": "snort"
}' 'http://node1:8082/api/v1/search/findOne'
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid":"4ac26cf7-ab93-4940-9a0e-8e7f4d67736d",
"sensorType": "snort"
}' 'http://node1:8082/api/v1/search/findOne'
```
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
## Patch neither alert and status
Create a metaalerts and get the GUID for the following steps.
### Patch in new field
```
/api/v1/update/patch
curl -X PATCH --header 'Content-Type: application/json' --header 'Accept: */*' -d '{
"guid": "00eae5ba-6137-4601-ae3a-fbf0003e58e6",
"index": "metaalert_index",
"patch": [
{
"op": "add"
, "path": "/name"
, "value": "My new meta alert name"
}
],
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/update/patch'
```
### Retrieve the meta alert and ensure it contains the new 'name' field
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "00eae5ba-6137-4601-ae3a-fbf0003e58e6",
"index": "metaalert_index",
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/search/findOne'
```
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/824
I agree with @justinleet. I think this is well within the scope of this PR since you're introducing a new method at an interface level. In that situation, we should have either a default implementation or a specific implementation for every datastore where it makes sense to do the operation. In this case, HBase certainly can and should have a `findAllLatest` implementation. As Justin said, I'd be ok with a default implementation at the interface level.
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
## Create meta alert with more than 10 alerts
### Find more than 10 alerts alerts
```
/api/v1/search/search
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"fields": [
"guid"
],
"from": 0,
"indices": [
"snort"
],
"query": "ip_dst_addr:192.168.66.121",
"size": 11
}' 'http://node1:8082/api/v1/search/search'
```
Note the alerts that come back
```
62a53a5f-78e6-417a-8078-fb850baa3e84
876b72cb-9d72-4706-ac99-46cf91a8f359
5fd8b0a0-1f68-494a-ae20-633542a7045d
aee597a0-4255-499a-a4e2-ec7d756babb2
bf9e0e73-e64c-4759-b4f7-efad0a60be82
5ab9ce98-30db-45b2-a4e6-6489f136c839
0a4a7019-04f8-4a8c-af0b-d2e3908ecdc9
3423fdca-cefa-402a-b57d-60b75a15f046
2eb63002-e5f2-467a-8675-30b653ae145b
53f38cfd-aa89-4e49-ba5f-827eb73774cd
5f71a515-4976-4b0d-be85-bb6879b1e151
```
### Create a metaalert with the alerts
```
/api/v1/metaalert/create
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"alerts": [
{
"guid": "62a53a5f-78e6-417a-8078-fb850baa3e84",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid":"876b72cb-9d72-4706-ac99-46cf91a8f359",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid": "5fd8b0a0-1f68-494a-ae20-633542a7045d",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid": "aee597a0-4255-499a-a4e2-ec7d756babb2",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid":"bf9e0e73-e64c-4759-b4f7-efad0a60be82",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid": "5ab9ce98-30db-45b2-a4e6-6489f136c839",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid":"0a4a7019-04f8-4a8c-af0b-d2e3908ecdc9",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid": "3423fdca-cefa-402a-b57d-60b75a15f046",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid": "2eb63002-e5f2-467a-8675-30b653ae145b",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid":"53f38cfd-aa89-4e49-ba5f-827eb73774cd",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
},
{
"guid":"5f71a515-4976-4b0d-be85-bb6879b1e151",
"index": "snort_index_2017.11.15.17",
"sensorType": "snort"
}
],
"groups": [
"test"
]
}' 'http://node1:8082/api/v1/metaalert/create'
```
Make sure to get the resulting guid from the response.
```
00eae5ba-6137-4601-ae3a-fbf0003e58e6
```
### Retrieve the meta alert and ensure it contains the provided alerts
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid": "00eae5ba-6137-4601-ae3a-fbf0003e58e6",
"index": "metaalert_index",
"sensorType": "metaalert"
}' 'http://node1:8082/api/v1/search/findOne'
```
### Retrieve the child alerts
Ensure all alerts have the 'metaalerts' field populated with the parent meta alert.
```
/api/v1/search/findOne
curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
"guid":"62a53a5f-78e6-417a-8078-fb850baa3e84",
"sensorType": "snort"
}' 'http://node1:8082/api/v1/search/findOne'
... // 10 more times
```
---
[GitHub] metron issue #824: METRON-1289: Alert fields are lost when a MetaAlert is cr...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/824
I've spun this up, and `add/alert` throws this exception:
```
{
"responseCode": 500,
"message": "class org.apache.metron.elasticsearch.dao.ElasticsearchDao: ElasticsearchDao upsert failed: failure in bulk execution:
[1]: index [metaalert_index], type [null_doc], id [be44748e-6471-465f-bd08-7f2c164b6755], message [java.lang.IllegalArgumentException: object mapping [alert] can't be changed from nested to non-nested]
java.io.IOException: ElasticsearchDao upsert failed: failure in bulk execution:
[1]: index [metaalert_index], type [null_doc], id [be44748e-6471-465f-bd08-7f2c164b6755], message [java.lang.IllegalArgumentException: object mapping [alert] can't be changed from nested to non-nested]
at org.apache.metron.elasticsearch.dao.ElasticsearchDao.batchUpdate(ElasticsearchDao.java:368)
at org.apache.metron.indexing.dao.MultiIndexDao.lambda$batchUpdate$3(MultiIndexDao.java:79)
at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1374)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
at java.util.stream.ReduceOps$ReduceTask.doLeaf(ReduceOps.java:747)
at java.util.stream.ReduceOps$ReduceTask.doLeaf(ReduceOps.java:721)
at java.util.stream.AbstractTask.compute(AbstractTask.java:316)
at java.util.concurrent.CountedCompleter.exec(CountedCompleter.java:731)
at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)
",
"fullMessage": "IOException: class org.apache.metron.elasticsearch.dao.ElasticsearchDao: ElasticsearchDao upsert failed: failure in bulk execution:
[1]: index [metaalert_index], type [null_doc], id [be44748e-6471-465f-bd08-7f2c164b6755], message [java.lang.IllegalArgumentException: object mapping [alert] can't be changed from nested to non-nested]
java.io.IOException: ElasticsearchDao upsert failed: failure in bulk execution:
[1]: index [metaalert_index], type [null_doc], id [be44748e-6471-465f-bd08-7f2c164b6755], message [java.lang.IllegalArgumentException: object mapping [alert] can't be changed from nested to non-nested]
at org.apache.metron.elasticsearch.dao.ElasticsearchDao.batchUpdate(ElasticsearchDao.java:368)
at org.apache.metron.indexing.dao.MultiIndexDao.lambda$batchUpdate$3(MultiIndexDao.java:79)
at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1374)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
at java.util.stream.ReduceOps$ReduceTask.doLeaf(ReduceOps.java:747)
at java.util.stream.ReduceOps$ReduceTask.doLeaf(ReduceOps.java:721)
at java.util.stream.AbstractTask.compute(AbstractTask.java:316)
at java.util.concurrent.CountedCompleter.exec(CountedCompleter.java:731)
at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)
"
}
```
Request
```
{
"alertGuids": [
"9b2e9b29-41a5-45fc-aabe-fd49c63af943",
"0fbea2b2-c4f3-43a8-a717-8ceb3467ae10",
"698289ca-43cf-4add-bf35-2a391ae1c244"
],
"metaAlertGuid": "be44748e-6471-465f-bd08-7f2c164b6755",
"sensorTypes": [
"bro"
]
}
```
I'll dig into it a bit, but I assume something isn't quite being constructed properly.
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150641377
--- Diff: metron-platform/metron-indexing/src/test/java/org/apache/metron/indexing/dao/InMemoryMetaAlertDao.java ---
@@ -200,4 +207,23 @@ public MetaAlertCreateResponse createMetaAlert(MetaAlertCreateRequest request)
createResponse.setCreated(true);
return createResponse;
}
+
+ @Override
+ public boolean addAlertsToMetaAlert(String metaAlertGuid, Collection<String> alertGuids,
+ Collection<String> sensorTypes) throws IOException {
+ return true;
--- End diff --
MetaAlertControllerIntegrationTest would be the best place to add these integration tests, and I think they should be added to make sure the REST calls line up the way we do everything else.
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r149423194
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -121,13 +121,16 @@ public synchronized Document getLatest(String guid, String sensorType) throws IO
}
@Override
+ public Iterable<Document> getAllLatest(Map<String, String> guidToIndices) throws IOException {
+ return null;
--- End diff --
Why is there no implementation here? HBase certainly should be able to have a multiget for the documents, no?
---
[GitHub] metron pull request #824: METRON-1289: Alert fields are lost when a MetaAler...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/824#discussion_r150995571
--- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/HBaseDao.java ---
@@ -138,9 +142,17 @@ private Document getDocumentFromResult(Result result) throws IOException {
Map.Entry<byte[], byte[]> entry= columns.lastEntry();
Long ts = Bytes.toLong(entry.getKey());
if(entry.getValue()!= null) {
- Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()), new TypeReference<Map<String, Object>>() {
- });
- return new Document(json, Bytes.toString(result.getRow()), (String) json.get(SOURCE_TYPE), ts);
+ Map<String, Object> json = JSONUtils.INSTANCE.load(new String(entry.getValue()),
+ new TypeReference<Map<String, Object>>() {});
+ ByteArrayInputStream baos = new ByteArrayInputStream(result.getRow());
--- End diff --
I initially tried to do this but if felt sort of awkward since we need to return both the guid and sensorType. Would it make sense to just return a Document with those fields set and set the other fields after that method is called? Or we could accept the json and ts as inputs to that method too.
---