You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by justinleet <gi...@git.apache.org> on 2017/09/29 15:06:01 UTC
[GitHub] metron pull request #780: METRON-1220: Create documentation around alert nes...
GitHub user justinleet opened a pull request:
https://github.com/apache/metron/pull/780
METRON-1220: Create documentation around alert nested field
## Contributor Comments
Adding some docs around the requirement of an "alert" field on new sensors.
There's an open discussion thread [[DISCUSS] Meta alert Elasticsearch new template requirement ramifications](https://lists.apache.org/thread.html/418ec07c67670f685cc42acfedde85cbd90c80301ea7819093e787f8@%3Cdev.metron.apache.org%3E) that has at least one question about what this should look like, so feel free to chime in there.
Outstanding Question
* Where should this live? I have this put up, so people can see what it says, but I'm pretty sure it should live somewhere else, under a different heading. I'm just not sure what the best fit is.
* Should additional documentation around our ES templates and other sensors requirements be added now or in the future.
I'll be running the proposed curl up in full dev, so that's slightly subject to change. Normally, updates to temples don't affect existing ES indices, but since it's a dummy field without data I cautiously expect it to function.
## 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?
- [ ] 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)?
- [ ] 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/justinleet/metron METRON-1220
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/780.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 #780
----
commit bc4feff87f1a2e8380419f48ea6aa24aba0fa034
Author: justinjleet <ju...@gmail.com>
Date: 2017-09-29T14:57:27Z
Adding some documentation around ES alert template
----
---
[GitHub] metron issue #780: METRON-1220: Create documentation around alert nested fie...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron/pull/780
> Honestly, I think it should at the very least be linked from the documentation which tells people how to create their own parsers...which we do not seem to have. ;)
Yes @cestella, I was thinking the same thing too. Right now, I think that means just updating the Squid Getting Started blog series in the Metron Wiki/Blogs.
We really should capture that entire Squid Getting Started Guide in our version controlled docs at some point.
---
[GitHub] metron pull request #780: METRON-1220: Create documentation around alert nes...
Posted by justinleet <gi...@git.apache.org>.
GitHub user justinleet reopened a pull request:
https://github.com/apache/metron/pull/780
METRON-1220: Create documentation around alert nested field
## Contributor Comments
Adding some docs around the requirement of an "alert" field on new sensors.
There's an open discussion thread [[DISCUSS] Meta alert Elasticsearch new template requirement ramifications](https://lists.apache.org/thread.html/418ec07c67670f685cc42acfedde85cbd90c80301ea7819093e787f8@%3Cdev.metron.apache.org%3E) that has at least one question about what this should look like, so feel free to chime in there.
Outstanding Question
* Where should this live? I have this put up, so people can see what it says, but I'm pretty sure it should live somewhere else, under a different heading. I'm just not sure what the best fit is.
* Should additional documentation around our ES templates and other sensors requirements be added now or in the future.
I'll be running the proposed curl up in full dev, so that's slightly subject to change. Normally, updates to temples don't affect existing ES indices, but since it's a dummy field without data I cautiously expect it to function.
## 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/justinleet/metron METRON-1220
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/780.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 #780
----
----
---
[GitHub] metron pull request #780: METRON-1220: Create documentation around alert nes...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet closed the pull request at:
https://github.com/apache/metron/pull/780
---
[GitHub] metron pull request #780: METRON-1220: Create documentation around alert nes...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/780#discussion_r143064409
--- Diff: README.md ---
@@ -118,3 +118,9 @@ Some useful utilities that cross all of these parts of the architecture:
* [Model as a Service](metron-analytics/metron-maas-service) : A Yarn application which can deploy machine learning and statistical models onto the cluster along with the associated Stellar functions to be able to call out to them in a scalable manner.
* [Data management](metron-platform/metron-data-management) : A set of data management utilities aimed at getting data into HBase in a format which will allow data flowing through metron to be enriched with the results. Contains integrations with threat intelligence feeds exposed via TAXII as well as simple flat file structures.
* [Profiler](metron-analytics/metron-profiler) : A feature extraction mechanism that can generate a profile describing the behavior of an entity. An entity might be a server, user, subnet or application. Once a profile has been generated defining what normal behavior looks-like, models can be built that identify anomalous behavior.
+
+# Notes on Adding a New Sensor
+In order to allow for meta alerts to be queries alongside regular alerts in Elasticsearch 2.x,
+it is necessary to add an additional field to the templates and mapping for existing sensors.
+
+Please see a description of the steps necessary to make this change in the metron-elasticsearch [README](./metron-platform/metron-elasticsearch#using-metron-with-elasticsearch-2x)
--- End diff --
You are correct, and I didn't catch it because it still goes to the correct readme, but without going to the section.
---
[GitHub] metron pull request #780: METRON-1220: Create documentation around alert nes...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron/pull/780#discussion_r142215715
--- Diff: metron-platform/metron-indexing/README.md ---
@@ -163,6 +163,36 @@ Both of these functions are handled under the hood.
In addition, an API endpoint is added for the meta alert specific features of creation and going from meta alert to alert.
The denormalization handles the case of going from meta alert to alert automatically.
+With Elasticsearch 2.x, there is an additional requirement that all sensors templates have a nested alert field defined. This field is a dummy field, and will be obsolete in Elasticsearch 5.x. See [Ignoring Unmapped Fields](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-sort.html#_ignoring_unmapped_fields) for more information
+
+Definition of the expected field:
+```
+ "alert": {
+ "type": "nested"
+ }
+```
+
+Without this field, an error will be thrown during ALL searches (including from UIs, resulting in no alerts being found for any sensor):
+
+Exception seen:
+```
+QueryParsingException[[nested] failed to find nested object under path [alert]];
+```
+
+To put a new template into Elasticsearch to resolve this issue, update the template with the new field:
+```
+curl -XPUT 'http://node1:9200/$SENSOR*/_mapping/$SENSOR_doc' -d '
--- End diff --
Can we make it more clear that `node1:9200` is the Elasticsearch API host:port that may be different outside of Full Dev?
There may be a better way, but maybe something like the following. If I install Metron using the MPack, do we call the Elasticsearch REST API something in there that might make sense here?
```
export ELASTICSEARCH="node1:9200"
curl -XPUT 'http://$ELASTICSEARCH/$SENSOR*/_mapping/$SENSOR_doc ...
```
---
[GitHub] metron pull request #780: METRON-1220: Create documentation around alert nes...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/metron/pull/780
---
[GitHub] metron issue #780: METRON-1220: Create documentation around alert nested fie...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/780
Kicking Travis.
---
[GitHub] metron issue #780: METRON-1220: Create documentation around alert nested fie...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/780
Honestly, I think it should at the very least be linked from the documentation which tells people how to create their own parsers...which we do not seem to have. ;)
---
[GitHub] metron issue #780: METRON-1220: Create documentation around alert nested fie...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/780
@nickwallen I figured out what the problem was. I'd replaced dashes with spaces and lowercased it, but left the period in `2.x`. After removing the period, links worked.
@cestella Added sections to the various files. `Upgrading.md` is a more stripped down version of what's in the main README addition. Minor catch is is we don't really have a nice way to point to a more permanent (i.e. github) link since we don't actually have a permanent link to point to yet. I just referred to metron-platform/metron-elasticsearch/README.md without an actual link. Let me know if there's something better you'd like to see there.
---
[GitHub] metron pull request #780: METRON-1220: Create documentation around alert nes...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron/pull/780#discussion_r143062420
--- Diff: README.md ---
@@ -118,3 +118,9 @@ Some useful utilities that cross all of these parts of the architecture:
* [Model as a Service](metron-analytics/metron-maas-service) : A Yarn application which can deploy machine learning and statistical models onto the cluster along with the associated Stellar functions to be able to call out to them in a scalable manner.
* [Data management](metron-platform/metron-data-management) : A set of data management utilities aimed at getting data into HBase in a format which will allow data flowing through metron to be enriched with the results. Contains integrations with threat intelligence feeds exposed via TAXII as well as simple flat file structures.
* [Profiler](metron-analytics/metron-profiler) : A feature extraction mechanism that can generate a profile describing the behavior of an entity. An entity might be a server, user, subnet or application. Once a profile has been generated defining what normal behavior looks-like, models can be built that identify anomalous behavior.
+
+# Notes on Adding a New Sensor
+In order to allow for meta alerts to be queries alongside regular alerts in Elasticsearch 2.x,
+it is necessary to add an additional field to the templates and mapping for existing sensors.
+
+Please see a description of the steps necessary to make this change in the metron-elasticsearch [README](./metron-platform/metron-elasticsearch#using-metron-with-elasticsearch-2x)
--- End diff --
I am pretty sure that to make this work correctly when we generate the site book, you have to name the link "Using Metron with Elasticsearch 2.x" or something close to that. If you've already vetted the site book, then ignore me.
---
[GitHub] metron issue #780: METRON-1220: Create documentation around alert nested fie...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron/pull/780
> Where should this live? I have this put up, so people can see what it says, but I'm pretty sure it should live somewhere else, under a different heading. I'm just not sure what the best fit is.
Whichever README this lands in, I would prefer to see it in a section entitled something like "Using Metron with Elasticsearch 2.x".
Does this make sense in the (currently non-existent) `metron-elasticsearch/README.md`?
We could link to `metron-elasticsearch/README.md` from the current location (**The MetaAlertDao** section) if you think that it makes sense. My thought is that a user encountering this problem, won't even know what the `MetaAlertDao` is, so I think the current location is not very user friendly.
---
[GitHub] metron issue #780: METRON-1220: Create documentation around alert nested fie...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/780
Also, should be linked from metron-parsers makes sense IMO as any time you are adding a new sensor, you're going to need to add this field. This is the use-case that keeps biting us in the butt over the last week.
---
[GitHub] metron issue #780: METRON-1220: Create documentation around alert nested fie...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/780
Three things IMO:
1. Could we add a link to the documentation you have from the front page README.md under the heading "Notes on Adding a New Sensor"
2. Could we also add a link to the documentation you have from the metron-parsers README.md maybe under a heading "Notes on Adding a New Sensor"
3. Add a blurb in Upgrading.md around what users need to do to get their existing sensors working.
---
[GitHub] metron issue #780: METRON-1220: Create documentation around alert nested fie...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron/pull/780
> @justinleet: Sidenote, does anybody know how to actually link to a head in a different doc? Anything inside the same doc works fine, but across docs would be better.
The following example is used to link from the Profiler Client README to a section in the Profiler README. Is this what you are looking for?
```
[Getting Started](../metron-profiler#getting-started)
```
---
[GitHub] metron pull request #780: METRON-1220: Create documentation around alert nes...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/780#discussion_r143064505
--- Diff: Upgrading.md ---
@@ -2,6 +2,48 @@
This document constitutes a per-version listing of changes of
configuration which are non-backwards compatible.
+## 0.4.1 to 0.4.2
+
+### [METRON-1158: Build backend for grouping alerts into meta alerts](https://issues.apache.org/jira/browse/METRON-1158)
+In order to allow for meta alerts to be queries alongside regular alerts in Elasticsearch 2.x,
+it is necessary to add an additional field to the templates and mapping for existing sensors.
+
+Two steps must be done for each sensor, but not on each index for each sensor.
+
+First is to update the Elasticsearch template for each sensor, so any new indices have the field:
+
+```
+export SENSOR="bro"
--- End diff --
Screwed it up when I move stuff around. Moved a few statements up, along with the export that was later.
---
[GitHub] metron issue #780: METRON-1220: Create documentation around alert nested fie...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/780
@cestella @nickwallen Hopefully took care of comments that don't involve migrating wiki docs. Let me know if I looked over anything. Sidenote, does anybody know how to actually link to a head in a different doc? Anything inside the same doc works fine, but across docs would be better.
## Test Plan
### Setup
Rather than creating a new parser, let's just break the Bro one.
1. Spin up full dev
1. Stop bro in monit.
1. Pull down the bro template and delete the extra JSON tag we don't need
```
curl -XGET "http://node1:9200/_template/bro_index*?pretty" -o "bro.template"
sed -i '' '2d;$d' ./bro.template
```
1. Open it in an editor and remove the `alert` field under `properties`
1. Push the template back.
```
curl -XPUT "http://node1:9200/_template/bro_index" -d @${SENSOR}.template
```
1. Ensure the `alert` field does **NOT** show up in the template:
```
curl -XGET 'node1:9200/_template/bro_index*?pretty' | grep '"alert"'
```
1. Delete any existing bro indices
```
curl -XDELETE 'localhost:9200/bro*?pretty'
```
1. Turn bro back on in Monit.
1. Wait until new data flows through the system into the bro indexes.
1. Make sure that the mapping has no `alert` field (i.e. Did our updated template apply as expected?). At this point we should have a sensor without the proper template and existing data.
```
curl -XGET 'node1:9200/bro*/_mapping?pretty' | grep '"alert"'
```
1. Go into the Swagger UI and run the following search. It should fail with a 500:
```
{
"fields": [
"*"
],
"from": 0,
"indices": [
"bro"
],
"query": "*",
"size": 10
}
```
1. Run through the commands given in the metron-elasticsearch README to update both the template and the mappings.
1. Ensure the `alert` field shows up in the mappings:
```
curl -XGET 'node1:9200/bro*/_mapping?pretty' | grep '"alert"'
```
1. Ensure the `alert` field shows up in the template:
```
curl -XGET 'node1:9200/_template/bro_index*?pretty' | grep '"alert"'
```
1. Rerun the query from the Swagger UI. Results should be returned now with no error.
---
[GitHub] metron issue #780: METRON-1220: Create documentation around alert nested fie...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron/pull/780
+1 LGTM. Thanks!
---
[GitHub] metron pull request #780: METRON-1220: Create documentation around alert nes...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron/pull/780#discussion_r143063049
--- Diff: Upgrading.md ---
@@ -2,6 +2,48 @@
This document constitutes a per-version listing of changes of
configuration which are non-backwards compatible.
+## 0.4.1 to 0.4.2
+
+### [METRON-1158: Build backend for grouping alerts into meta alerts](https://issues.apache.org/jira/browse/METRON-1158)
+In order to allow for meta alerts to be queries alongside regular alerts in Elasticsearch 2.x,
+it is necessary to add an additional field to the templates and mapping for existing sensors.
+
+Two steps must be done for each sensor, but not on each index for each sensor.
+
+First is to update the Elasticsearch template for each sensor, so any new indices have the field:
+
+```
+export SENSOR="bro"
--- End diff --
Tiny issue, but I think we're missing an export.
```
export ELASTICSEARCH="node1"
```
---
[GitHub] metron pull request #780: METRON-1220: Create documentation around alert nes...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron/pull/780#discussion_r142214035
--- Diff: metron-platform/metron-indexing/README.md ---
@@ -163,6 +163,36 @@ Both of these functions are handled under the hood.
In addition, an API endpoint is added for the meta alert specific features of creation and going from meta alert to alert.
The denormalization handles the case of going from meta alert to alert automatically.
+With Elasticsearch 2.x, there is an additional requirement that all sensors templates have a nested alert field defined. This field is a dummy field, and will be obsolete in Elasticsearch 5.x. See [Ignoring Unmapped Fields](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-sort.html#_ignoring_unmapped_fields) for more information
+
+Definition of the expected field:
+```
+ "alert": {
+ "type": "nested"
+ }
+```
+
+Without this field, an error will be thrown during ALL searches (including from UIs, resulting in no alerts being found for any sensor):
--- End diff --
Where exactly would I see this error message? In the UI itself or is it only logged by the REST API?
Maybe this error message with a link to your explanation could go in a separate **FAQ** section (in whatever README you decide to land this in.)
---