You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by cestella <gi...@git.apache.org> on 2016/06/03 03:09:40 UTC
[GitHub] incubator-metron pull request #143: METRON-197: Validation should be the las...
GitHub user cestella opened a pull request:
https://github.com/apache/incubator-metron/pull/143
METRON-197: Validation should be the last step in the ParserBolt
Right now we are doing the validation prior to the messageFilter. We should only validate the parsed messages which passes through the filter.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cestella/incubator-metron METRON-197
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-metron/pull/143.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 #143
----
commit 53e3f61fe795b1620ee5847448ba1a2a13274120
Author: cstella <ce...@gmail.com>
Date: 2016-06-03T02:25:16Z
Fixing global validation to run at the end of the validation pipeline.
commit 134e4cd02a149452f9f774a6a85d6c7ccefabc17
Author: cstella <ce...@gmail.com>
Date: 2016-06-03T03:07:24Z
Fixing ParserBolt to do validations in the proper place.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron pull request #143: METRON-197: Validation should be the las...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/143#discussion_r66248395
--- Diff: metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/bolt/ParserBolt.java ---
@@ -143,14 +140,14 @@ public void execute(Tuple tuple) {
boolean ackTuple = true;
if(sensorParserConfig != null) {
List<FieldValidator> fieldValidations = getConfigurations().getFieldValidations();
- List<JSONObject> messages = parser.parse(originalMessage);
- for (JSONObject message : messages) {
- if (parser.validate(message)) {
+ Optional<List<JSONObject>> messages = parser.parseOptional(originalMessage);
+ for (JSONObject message : messages.orElse(Collections.emptyList())) {
+ if (parser.validate(message) && filter != null && filter.emitTuple(message)) {
if(!isGloballyValid(message, fieldValidations)) {
message.put(Constants.SENSOR_TYPE, getSensorType()+ ".invalid");
collector.emit(Constants.INVALID_STREAM, new Values(message));
}
- else if (filter != null && filter.emitTuple(message)) {
+ else {
ackTuple = !isBulk;
--- End diff --
The tuple will be acked. Note that ackTuple is initialized to true and is only set to false if we are in a situation where we are doing a bulk write, in which case the tuple will be acked in the bulk writer upon the writing of a batch of entries. So, when this loop is ended, it will ack on line 164.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron pull request #143: METRON-197: Validation should be the las...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-metron/pull/143
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron pull request #143: METRON-197: Validation should be the las...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/143#discussion_r65940558
--- Diff: metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/bolt/ParserBolt.java ---
@@ -143,14 +140,14 @@ public void execute(Tuple tuple) {
boolean ackTuple = true;
if(sensorParserConfig != null) {
List<FieldValidator> fieldValidations = getConfigurations().getFieldValidations();
- List<JSONObject> messages = parser.parse(originalMessage);
- for (JSONObject message : messages) {
- if (parser.validate(message)) {
+ Optional<List<JSONObject>> messages = parser.parseOptional(originalMessage);
+ for (JSONObject message : messages.orElse(Collections.emptyList())) {
+ if (parser.validate(message) && filter != null && filter.emitTuple(message)) {
if(!isGloballyValid(message, fieldValidations)) {
message.put(Constants.SENSOR_TYPE, getSensorType()+ ".invalid");
collector.emit(Constants.INVALID_STREAM, new Values(message));
}
- else if (filter != null && filter.emitTuple(message)) {
+ else {
ackTuple = !isBulk;
--- End diff --
Shouldn't the tuple get acked regardless of whether it passed the validations or not?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #143: METRON-197: Validation should be the last step ...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/143
I was under the impression that validations should happen after the field transformations. Is that not the case?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #143: METRON-197: Validation should be the last step ...
Posted by james-sirota <gi...@git.apache.org>.
Github user james-sirota commented on the issue:
https://github.com/apache/incubator-metron/pull/143
If a message or a tuple fails validation then the associated message should get routed to a dead letter queue. I believe there is a different PR that is required to get the dead letter queue established. The tuple should be acked, though. We need to differentiate between if the tuple is failed due to not being able to be parsed vs. not validated. So if we fail based on parser issues then we need to ack for validation issues and then land the message in the dead letter queue. The important thing is for the messages that fail validation not to go through the rest of the pipeline.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #143: METRON-197: Validation should be the last step ...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/143
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #143: METRON-197: Validation should be the last step ...
Posted by james-sirota <gi...@git.apache.org>.
Github user james-sirota commented on the issue:
https://github.com/apache/incubator-metron/pull/143
+1. This is a simple code change in the order of method calls. Builds and all tests pass
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-metron issue #143: METRON-197: Validation should be the last step ...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/143
@merrimanr @james-sirota I tend to agree, validations should happen prior to field transformations since the transformation may affect the validity of the message. I'll adjust that. Good feedback!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---