You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by nickwallen <gi...@git.apache.org> on 2017/11/20 18:14:43 UTC
[GitHub] metron pull request #845: METRON-1321 Metaalert Threat Score Type Does Not M...
GitHub user nickwallen opened a pull request:
https://github.com/apache/metron/pull/845
METRON-1321 Metaalert Threat Score Type Does Not Match Sensor Indices
After creating Metaalerts in the Alerts UI, I am unable to sort by threat triage score. The exception that is logged is shown in the issue. When opening the Developer view in Chrome, I can see the API responds with an error. An error message is logged under `/var/log/metron/metron-rest.log`.
## What happened?
An overall threat score is added to a Metaalert whenever it has any child alerts. This overall threat score is a summary of the threat scores from each of the child alerts.
This field is also named the same as the threat score fields from the sensor indices; 'threat:triage:score'. The type of this field must be the same as the 'threat:triage:score' fields from each of the sensor indices. Otherwise the Alerts UI cannot properly sort alerts and metaalerts in the same table/view in the UI.
## Changes
I updated the `ElasticsearchMetaalertDao` so that when it creates the overall threat score, it is added as a Float to match the type expected from the other sensor indices.
## Testing
1. Spin up Full Dev
2. Open up the Alerts UI
3. Sort by triage score and assure no errors occur
4. Create a Metaalert.
5. Sort by triage score and assure no errors occur
## Pull Request Checklist
- [ ] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
- [ ] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
- [ ] Have you included steps to reproduce the behavior or problem that is being changed or addressed?
- [ ] Have you included steps or a guide to how the change may be verified and tested manually?
- [ ] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
- [ ] Have you written or updated unit tests and or integration tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nickwallen/metron METRON-1321
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/845.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 #845
----
commit c780058aad1611edb0ea7a4fae84038ee07d50fd
Author: Nick Allen <ni...@nickallen.org>
Date: 2017-11-20T18:05:37Z
METRON-1321 Metaalert Threat Score Type Does Not Match Sensor Indices
----
---
[GitHub] metron pull request #845: METRON-1321 Metaalert Threat Score Type Does Not M...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron/pull/845#discussion_r152077236
--- Diff: metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertDao.java ---
@@ -614,8 +625,15 @@ protected void calculateMetaScores(Document metaAlert) {
}
metaScores = new MetaScores(scores);
}
+
+ // add a summary (max, min, avg, count, sum) of all the threat scores from the child alerts
metaAlert.getDocument().putAll(metaScores.getMetaScores());
- metaAlert.getDocument().put(threatTriageField, metaScores.getMetaScores().get(threatSort));
+
+ // the overall threat score for the metaalert; either max, min, avg, count or sum of all child scores
--- End diff --
The `ElasticsearchMetaAlertDao` adds an overall threat score to the Metaalert. The overall threat score can be any one of the following summary aggregations of the child alerts; sum, min, max, count, average, or median. These summary values are calculated in `MetaScores` and result in Double values. Since the other sensor indices currently define `threat:triage:score` as a float, this solution just casts this to a Double to match those.
I think an alternative way to solve this is to just make the `threat:triage:score` in each of the sensor indices a Double as you mentioned. I think your approach seems a little cleaner to me. Although, I am not sure if there are other down sides I am not thinking about. Can anyone else think of a problem with this approach? @justinleet @merrimanr ?
---
[GitHub] metron issue #845: METRON-1321 Metaalert Threat Score Type Does Not Match Se...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/845
Could we also add the threat score to the metaalert template, to match the other templates?
---
[GitHub] metron issue #845: METRON-1321 Metaalert Threat Score Type Does Not Match Se...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/845
+1 by inspection, assuming @ottobackwards is good.
Thanks for expanding the comments out, it's definitely helpful.
---
[GitHub] metron pull request #845: METRON-1321 Metaalert Threat Score Type Does Not M...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron/pull/845#discussion_r152069979
--- Diff: metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertDao.java ---
@@ -614,8 +625,15 @@ protected void calculateMetaScores(Document metaAlert) {
}
metaScores = new MetaScores(scores);
}
+
+ // add a summary (max, min, avg, count, sum) of all the threat scores from the child alerts
metaAlert.getDocument().putAll(metaScores.getMetaScores());
- metaAlert.getDocument().put(threatTriageField, metaScores.getMetaScores().get(threatSort));
+
+ // the overall threat score for the metaalert; either max, min, avg, count or sum of all child scores
--- End diff --
why isn't it a float to start with? isn't that the real issue?
---
[GitHub] metron pull request #845: METRON-1321 Metaalert Threat Score Type Does Not M...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron/pull/845#discussion_r152083195
--- Diff: metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertDao.java ---
@@ -614,8 +625,15 @@ protected void calculateMetaScores(Document metaAlert) {
}
metaScores = new MetaScores(scores);
}
+
+ // add a summary (max, min, avg, count, sum) of all the threat scores from the child alerts
metaAlert.getDocument().putAll(metaScores.getMetaScores());
- metaAlert.getDocument().put(threatTriageField, metaScores.getMetaScores().get(threatSort));
+
+ // the overall threat score for the metaalert; either max, min, avg, count or sum of all child scores
--- End diff --
I think it is really just a matter of what we'd expect a user to define as scores for their threat triage rules. Are they really going to define values greater than a 32-bit float? Or values that sum to greater than a 32-bit float?
I think it is a good point to discuss, Otto. I'd suggest we go with the float approach now, as it minimizes the scope of change in this PR. But we can revisit whether a double should be used after we migrate to ES 5.x.
---
[GitHub] metron pull request #845: METRON-1321 Metaalert Threat Score Type Does Not M...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/metron/pull/845
---
[GitHub] metron pull request #845: METRON-1321 Metaalert Threat Score Type Does Not M...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron/pull/845#discussion_r152081656
--- Diff: metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertDao.java ---
@@ -614,8 +625,15 @@ protected void calculateMetaScores(Document metaAlert) {
}
metaScores = new MetaScores(scores);
}
+
+ // add a summary (max, min, avg, count, sum) of all the threat scores from the child alerts
metaAlert.getDocument().putAll(metaScores.getMetaScores());
- metaAlert.getDocument().put(threatTriageField, metaScores.getMetaScores().get(threatSort));
+
+ // the overall threat score for the metaalert; either max, min, avg, count or sum of all child scores
--- End diff --
I would not hold up the PR for this point if it is too much. Just seems that we are coding around something else.
---
[GitHub] metron issue #845: METRON-1321 Metaalert Threat Score Type Does Not Match Se...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron/pull/845
I ran this up according to my testing instructions and it addresses the problem. Please take a look-see.
---
[GitHub] metron issue #845: METRON-1321 Metaalert Threat Score Type Does Not Match Se...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron/pull/845
I am still testing this in Full Dev. Will respond once I verify this completely.
---
[GitHub] metron issue #845: METRON-1321 Metaalert Threat Score Type Does Not Match Se...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/metron/pull/845
+1
---
[GitHub] metron pull request #845: METRON-1321 Metaalert Threat Score Type Does Not M...
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/845#discussion_r152075990
--- Diff: metron-platform/metron-elasticsearch/src/main/java/org/apache/metron/elasticsearch/dao/ElasticsearchMetaAlertDao.java ---
@@ -614,8 +625,15 @@ protected void calculateMetaScores(Document metaAlert) {
}
metaScores = new MetaScores(scores);
}
+
+ // add a summary (max, min, avg, count, sum) of all the threat scores from the child alerts
metaAlert.getDocument().putAll(metaScores.getMetaScores());
- metaAlert.getDocument().put(threatTriageField, metaScores.getMetaScores().get(threatSort));
+
+ // the overall threat score for the metaalert; either max, min, avg, count or sum of all child scores
--- End diff --
The calculations were done as Double and given to ES. However, there's no definition of the field in ES (It just used automatic mapping), so it was given the ES double.
---