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/02/13 13:19:12 UTC
[GitHub] incubator-metron pull request #453: METRON-694: Index Errors from Topologies
GitHub user merrimanr opened a pull request:
https://github.com/apache/incubator-metron/pull/453
METRON-694: Index Errors from Topologies
This PR addresses METRON-695, including updates to the Ambari MPack. A summary of the changes:
- Defaulted FieldValidator.input to empty list
- Added MetronError abstraction, updated ErrorUtils
- Added MetronGetStrategy abstraction
- Updated Constants to only have 1 error topic and added error field and error type enums
- Updated flux property files for enrichment and indexing topologies to send errors to single error topic
- Added new error topology including start script, index zookeeper config, flux file and flux property file
- Updated integration tests with single error topic and error handling test cases where applicable
- Updated unit tests to cover new error handling code
- Removed invalid data fields, invalid is considered error now
- Exposed Kafka offset as a parameter in ParserTopologyComponent
- Added error indexing topology to Ambari MPack
There are 2 areas of review that I would like to highlight. The first is the abstraction for handling errors in the various Metron topologies. Error handling logic is not consistent across topologies, ranging from calling the ErrorUtils.handleError method to simply logging the exception. The ErrorUtils.handleError was the most common method used so I decided to extend that to accommodate other bolts and topologies. As I worked through the details I found myself either having to add several additional ErrorUtils.handleError methods to cover all the different combinations of error message properties or creating several empty Optional objects, making the code more verbose and confusing than it should be. So I moved the error message generating logic previously in ErrorUtils.generateErrorMessage to a separate MetronError class that follows a builder/fluent design pattern. With this change, capturing an error now follows this pattern:
MetronError error = new MetronError()
.withErrorProperty(errorProperty) // error properties could include caught exception, raw message(s), error type, etc
... // add as many properties as needed
ErrorUtils.handleError(collector, error);
I expect there will be many opinions about the correct approach here.
The other abstraction involves retrieving a message from a tuple. The primary driver for this is logging the original message that caused a failure. The BulkWriterComponent class is passed both a tuple and a message which is then passed to the BulkMessageWriter.write method. There are 2 challenges with this. The first is that the returned BulkWriterResponse object only contains the tuples that failed and not the messages. The other is that a message could have been transformed before being passed to the BulkWriterComponent so we need a way to get the original message again in case of a failure so that it may be replayed. To solve this, I created a MessageGetStrategy interface that can be passed around between components to retrieve the original message if needed. This could become a useful abstraction for other uses cases as well, for example making the BulkMessageWriterBolt configurable through flux.
All applicable unit and integration tests were updated (or created if they didn't already exist). This can be tested in Quick Dev by editing the elasticsearch_error.properties to match the environment and starting the topology with the start_elasticsearch_error_topology.sh script. The Bro sensor in that environment includes messages that fail to parse so error messages should be generated by default as long as that sensor is running. These steps are manual because I'm assuming Ansible is being deprecated. For a more automated approach, this can (and should) be tested with the Ambari MPack. I am still trying to think of ways to simulate errors in other parts of the topology so if anyone has ideas let me know.
I apologize for the size of this PR but error handling cuts across many different modules and classes and required a lot of updates so it was unavoidable. I decided to not update the profiler topology yet because it does not include a bolt for handling errors like the other topologies. This would require changes to the profiler topology architecture, adding to an already very large PR. I am happy to take that on in a separate pull request or this one if everyone feels it should be included now.
A special thanks to @justinleet for helping me with the Ambari MPack and @cestella for helping with the MessageGetStrategy abstraction (that he had already started with MessageGetters). Looking forward to some feedback!
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/merrimanr/incubator-metron METRON-695
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-metron/pull/453.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 #453
----
commit 4a329ed241f60057136de00e1d716fb81d32d337
Author: rmerriman <rm...@hortonworks.com>
Date: 2017-02-06T21:19:45Z
Initial commit
commit a05fdcdd56ff4c1ee33a643ec065be352d88cd3b
Author: rmerriman <rm...@hortonworks.com>
Date: 2017-02-06T21:20:24Z
Merge remote-tracking branch 'mirror/master' into METRON-695
commit a24e621d79a105ebe1a1c69d0fb1601d7940f96b
Author: rmerriman <rm...@hortonworks.com>
Date: 2017-02-08T00:22:16Z
Updated tests to include error conditions
commit 6b897d5afadde00870e2579af9c3c57d9ef9d076
Author: rmerriman <rm...@hortonworks.com>
Date: 2017-02-08T00:22:47Z
Added error topology to Ambari MPack
----
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
So the consensus is to use a single indexing topology for both error and normal messages by default. I will remove the scripts and configuration (flux and properties) files and remove the error indexing topology from the MPack. If a user wants to use a separate error indexing topology it will be their responsibility to change error topic setting and start a separate indexing topology. Does this sound ok to everyone?
A couple questions are still unanswered. How granular should the error topic setting be? I will assume global config is acceptable unless I hear otherwise. Also, as @cestella alluded to earlier, what should the structure of the source.type field be? I see these as possibilities:
1. source.type is 'error' with sensor name and error type in separate fields (1 index config)
2. source.type is error type with sensor name in separate field (index config per error type)
3. source.type is sensor name + '_error' (additional error index config per sensor)
If no one has any strong opinions I will stick with option 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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
METRON-694 includes both the topology changes and the Ambari MPack changes. I started on METRON-695 but decided to include both in a single PR, hence the branch being named METRON-695 but the PR name referencing METRON-694.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I think how you have it as a fine approach. Just need to doc it as such.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I tried running this up and discovered that there's at least one error that doesn't get caught. Json parsing errors, e.g. if someone gives outright badly formatted messages to indexing (e.g. missing closing '}'), don't get caught and indexed right now.
I don't believe we ever handled this type of error, because I don't think it ever occurs from our code directly. I'm inclined to not worry about it for this PR given that we never worried about it to being with, but we may want to create a follow on Jira to ensure that we handle cases like this well. As we add and increase visibility to extension points, we don't want things like this getting tripped by custom code.
Anyone have objections to that?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I like the flexibility of the new MessageGetter stuff. I don't understand when I would use a `DEFAULT_JSON_FROM_FIELD` versus a `JSON_FROM_FIELD`. Some basic javadocs on those new classes would probably answer that question and others. Maybe I am missing them, but I don't see unit tests for those classes either.
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100799809
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/error/MetronError.java ---
@@ -0,0 +1,200 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.error;
+
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.metron.common.Constants;
+import org.apache.metron.common.Constants.ErrorType;
+import org.json.simple.JSONObject;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.metron.common.Constants.ErrorFields;
+
+public class MetronError {
+
+ private String message;
+ private Throwable throwable;
+ private String sensorType = "error";
+ private ErrorType errorType = ErrorType.DEFAULT_ERROR;
+ private Set<String> errorFields;
+ private List<Object> rawMessages;
+
+ public MetronError withMessage(String message) {
+ this.message = message;
+ return this;
+ }
+
+ public MetronError withThrowable(Throwable throwable) {
+ this.throwable = throwable;
+ return this;
+ }
+
+ public MetronError withSensorType(String sensorType) {
+ this.sensorType = sensorType;
+ return this;
+ }
+
+ public MetronError withErrorType(ErrorType errorType) {
+ this.errorType = errorType;
+ return this;
+ }
+
+ public MetronError withErrorFields(Set<String> errorFields) {
+ this.errorFields = errorFields;
+ return this;
+ }
+
+
+ public MetronError addRawMessage(Object rawMessage) {
+ if (rawMessage != null) {
+ if (this.rawMessages == null) {
+ this.rawMessages = new ArrayList<>();
+ }
+ this.rawMessages.add(rawMessage);
+ }
+ return this;
+ }
+
+ public MetronError withRawMessages(List<Object> rawMessages) {
+ this.rawMessages = rawMessages;
+ return this;
+ }
+
+ public Optional<Throwable> getThrowable() {
+ return throwable != null ? Optional.of(throwable) : Optional.empty();
+ }
+
+ @SuppressWarnings({"unchecked"})
+ public JSONObject getJSONObject() {
+ JSONObject errorMessage = new JSONObject();
+ errorMessage.put(Constants.SENSOR_TYPE, "error");
+
+ /*
+ * Save full stack trace in object.
+ */
+ if (throwable != null) {
+ String stackTrace = ExceptionUtils.getStackTrace(throwable);
+ String exception = throwable.toString();
+ errorMessage.put(ErrorFields.EXCEPTION.getName(), exception);
+ errorMessage.put(ErrorFields.STACK.getName(), stackTrace);
+ }
+
+ errorMessage.put(ErrorFields.TIMESTAMP.getName(), System.currentTimeMillis());
+ try {
+ errorMessage.put(ErrorFields.HOSTNAME.getName(), InetAddress.getLocalHost().getHostName());
+ } catch (UnknownHostException ex) {
+
+ }
+ if(rawMessages != null) {
+ for(int i = 0; i < rawMessages.size(); i++) {
+ Object rawMessage = rawMessages.get(i);
+ // If multiple messages are included add an index to the field name, otherwise leave it off
+ String rawMessageField = rawMessages.size() == 1 ? ErrorFields.RAW_MESSAGE.getName() : ErrorFields.RAW_MESSAGE.getName() + "_" + i;
+ String rawMessageBytesField = rawMessages.size() == 1 ? ErrorFields.RAW_MESSAGE_BYTES.getName() : ErrorFields.RAW_MESSAGE_BYTES.getName() + "_" + i;
+ if(rawMessage instanceof byte[]) {
+ errorMessage.put(rawMessageField, Bytes.toString((byte[])rawMessage));
+ errorMessage.put(rawMessageBytesField, toByteArrayList((byte[])rawMessage));
+ } else {
+ errorMessage.put(rawMessageField, rawMessage);
+ }
+ }
+ }
+
+ if (rawMessages != null && rawMessages.size() == 1) {
+ Object rawMessage = rawMessages.get(0);
+ if (rawMessage instanceof JSONObject) {
+ JSONObject rawJSON = (JSONObject) rawMessage;
+ if (errorFields != null) {
+ String errorFieldString = String.join(",", errorFields);
+ errorMessage.put(ErrorFields.ERROR_FIELDS.getName(), errorFieldString);
+ List<String> hashElements = errorFields.stream().map(errorField ->
+ String.format("%s-%s", errorField, rawJSON.get(errorField))).collect(Collectors.toList());
+ errorMessage.put(ErrorFields.ERROR_HASH.getName(), DigestUtils.sha256Hex(String.join("|", hashElements).getBytes(UTF_8)));
+ } else {
+ errorMessage.put(ErrorFields.ERROR_HASH.getName(), DigestUtils.sha256Hex(rawJSON.toJSONString().getBytes(UTF_8)));
+ }
+ } else if (rawMessage instanceof byte[]) {
+ errorMessage.put(ErrorFields.ERROR_HASH.getName(), DigestUtils.sha256Hex((byte[])rawMessage));
+ } else {
+ errorMessage.put(ErrorFields.ERROR_HASH.getName(), DigestUtils.sha256Hex(rawMessage.toString().getBytes(UTF_8)));
+ }
+ }
+
+ if (message != null) {
+ errorMessage.put(ErrorFields.MESSAGE.getName(), message);
+ } else if (throwable != null) {
+ errorMessage.put(ErrorFields.MESSAGE.getName(), throwable.getMessage());
+ }
+
+ errorMessage.put(ErrorFields.FAILED_SENSOR_TYPE.getName(), sensorType);
+ errorMessage.put(ErrorFields.ERROR_TYPE.getName(), errorType.getType());
+
+ return errorMessage;
+ }
+
+ protected List<Byte> toByteArrayList(byte[] list) {
+ List<Byte> ret = new ArrayList<>();
--- End diff --
You can use guava's `Bytes.asList(list)` here
---
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 #453: METRON-694: Index Errors from Topologies
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/453
> I think a configurable error topic is a reasonable request...
Hey @merrimanr - Are you saying that you want to take the approach of using a single indexing topology by default, but allow a user to configure multiple topologies if they so desire? Just want to make sure I understand the go-forward.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Thanks @merrimanr . Error message has tons of useful fields. I like it.
I think your option 2 above ('source.type=error' with a separate field for original source type) would be very useful. I am just not sure if we can assume we always know the original source type.
If a message errored early in parsing then maybe we don't know the `source.type`? Or can we always assume the `source.type` based on the Kafka topic that we got the message from?
I think your option 1 is a good fall-back, if we cannot always assume we know the `source.type`.
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100848948
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/error/MetronError.java ---
@@ -0,0 +1,200 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.error;
+
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.metron.common.Constants;
+import org.apache.metron.common.Constants.ErrorType;
+import org.json.simple.JSONObject;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.metron.common.Constants.ErrorFields;
+
+public class MetronError {
+
+ private String message;
+ private Throwable throwable;
+ private String sensorType = "error";
+ private ErrorType errorType = ErrorType.DEFAULT_ERROR;
+ private Set<String> errorFields;
+ private List<Object> rawMessages;
+
+ public MetronError withMessage(String message) {
+ this.message = message;
+ return this;
+ }
+
+ public MetronError withThrowable(Throwable throwable) {
+ this.throwable = throwable;
+ return this;
+ }
+
+ public MetronError withSensorType(String sensorType) {
+ this.sensorType = sensorType;
+ return this;
+ }
+
+ public MetronError withErrorType(ErrorType errorType) {
+ this.errorType = errorType;
+ return this;
+ }
+
+ public MetronError withErrorFields(Set<String> errorFields) {
+ this.errorFields = errorFields;
+ return this;
+ }
+
+
+ public MetronError addRawMessage(Object rawMessage) {
+ if (rawMessage != null) {
+ if (this.rawMessages == null) {
+ this.rawMessages = new ArrayList<>();
+ }
+ this.rawMessages.add(rawMessage);
+ }
+ return this;
+ }
+
+ public MetronError withRawMessages(List<Object> rawMessages) {
+ this.rawMessages = rawMessages;
+ return this;
+ }
+
+ public Optional<Throwable> getThrowable() {
+ return throwable != null ? Optional.of(throwable) : Optional.empty();
+ }
+
+ @SuppressWarnings({"unchecked"})
+ public JSONObject getJSONObject() {
+ JSONObject errorMessage = new JSONObject();
+ errorMessage.put(Constants.SENSOR_TYPE, "error");
+
+ /*
+ * Save full stack trace in object.
+ */
+ if (throwable != null) {
+ String stackTrace = ExceptionUtils.getStackTrace(throwable);
+ String exception = throwable.toString();
+ errorMessage.put(ErrorFields.EXCEPTION.getName(), exception);
+ errorMessage.put(ErrorFields.STACK.getName(), stackTrace);
+ }
+
+ errorMessage.put(ErrorFields.TIMESTAMP.getName(), System.currentTimeMillis());
+ try {
+ errorMessage.put(ErrorFields.HOSTNAME.getName(), InetAddress.getLocalHost().getHostName());
+ } catch (UnknownHostException ex) {
+
+ }
+ if(rawMessages != null) {
+ for(int i = 0; i < rawMessages.size(); i++) {
+ Object rawMessage = rawMessages.get(i);
+ // If multiple messages are included add an index to the field name, otherwise leave it off
+ String rawMessageField = rawMessages.size() == 1 ? ErrorFields.RAW_MESSAGE.getName() : ErrorFields.RAW_MESSAGE.getName() + "_" + i;
+ String rawMessageBytesField = rawMessages.size() == 1 ? ErrorFields.RAW_MESSAGE_BYTES.getName() : ErrorFields.RAW_MESSAGE_BYTES.getName() + "_" + i;
+ if(rawMessage instanceof byte[]) {
+ errorMessage.put(rawMessageField, Bytes.toString((byte[])rawMessage));
+ errorMessage.put(rawMessageBytesField, toByteArrayList((byte[])rawMessage));
+ } else {
+ errorMessage.put(rawMessageField, rawMessage);
+ }
+ }
+ }
+
+ if (rawMessages != null && rawMessages.size() == 1) {
+ Object rawMessage = rawMessages.get(0);
+ if (rawMessage instanceof JSONObject) {
+ JSONObject rawJSON = (JSONObject) rawMessage;
+ if (errorFields != null) {
+ String errorFieldString = String.join(",", errorFields);
+ errorMessage.put(ErrorFields.ERROR_FIELDS.getName(), errorFieldString);
+ List<String> hashElements = errorFields.stream().map(errorField ->
+ String.format("%s-%s", errorField, rawJSON.get(errorField))).collect(Collectors.toList());
+ errorMessage.put(ErrorFields.ERROR_HASH.getName(), DigestUtils.sha256Hex(String.join("|", hashElements).getBytes(UTF_8)));
+ } else {
+ errorMessage.put(ErrorFields.ERROR_HASH.getName(), DigestUtils.sha256Hex(rawJSON.toJSONString().getBytes(UTF_8)));
+ }
+ } else if (rawMessage instanceof byte[]) {
+ errorMessage.put(ErrorFields.ERROR_HASH.getName(), DigestUtils.sha256Hex((byte[])rawMessage));
+ } else {
+ errorMessage.put(ErrorFields.ERROR_HASH.getName(), DigestUtils.sha256Hex(rawMessage.toString().getBytes(UTF_8)));
+ }
+ }
+
+ if (message != null) {
+ errorMessage.put(ErrorFields.MESSAGE.getName(), message);
+ } else if (throwable != null) {
+ errorMessage.put(ErrorFields.MESSAGE.getName(), throwable.getMessage());
+ }
+
+ errorMessage.put(ErrorFields.FAILED_SENSOR_TYPE.getName(), sensorType);
+ errorMessage.put(ErrorFields.ERROR_TYPE.getName(), errorType.getType());
+
+ return errorMessage;
+ }
+
+ protected List<Byte> toByteArrayList(byte[] list) {
+ List<Byte> ret = new ArrayList<>();
--- End diff --
This was carried over from ErrorUtils. Happy to change it.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Just pushed out a commit to address recent comments. Commit includes:
- unit test and javadoc for MessageGetters
- error index template with the "ignore_above": 8191" setting
- index errors now get put back on indexing topic
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100799375
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/error/MetronError.java ---
@@ -0,0 +1,200 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.error;
+
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.metron.common.Constants;
+import org.apache.metron.common.Constants.ErrorType;
+import org.json.simple.JSONObject;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.metron.common.Constants.ErrorFields;
+
+public class MetronError {
+
+ private String message;
+ private Throwable throwable;
+ private String sensorType = "error";
+ private ErrorType errorType = ErrorType.DEFAULT_ERROR;
+ private Set<String> errorFields;
+ private List<Object> rawMessages;
+
+ public MetronError withMessage(String message) {
+ this.message = message;
+ return this;
+ }
+
+ public MetronError withThrowable(Throwable throwable) {
+ this.throwable = throwable;
+ return this;
+ }
+
+ public MetronError withSensorType(String sensorType) {
+ this.sensorType = sensorType;
+ return this;
+ }
+
+ public MetronError withErrorType(ErrorType errorType) {
+ this.errorType = errorType;
+ return this;
+ }
+
+ public MetronError withErrorFields(Set<String> errorFields) {
+ this.errorFields = errorFields;
+ return this;
+ }
+
+
+ public MetronError addRawMessage(Object rawMessage) {
+ if (rawMessage != null) {
+ if (this.rawMessages == null) {
+ this.rawMessages = new ArrayList<>();
+ }
+ this.rawMessages.add(rawMessage);
+ }
+ return this;
+ }
+
+ public MetronError withRawMessages(List<Object> rawMessages) {
+ this.rawMessages = rawMessages;
+ return this;
+ }
+
+ public Optional<Throwable> getThrowable() {
+ return throwable != null ? Optional.of(throwable) : Optional.empty();
+ }
+
+ @SuppressWarnings({"unchecked"})
+ public JSONObject getJSONObject() {
+ JSONObject errorMessage = new JSONObject();
+ errorMessage.put(Constants.SENSOR_TYPE, "error");
+
+ /*
+ * Save full stack trace in object.
+ */
+ if (throwable != null) {
+ String stackTrace = ExceptionUtils.getStackTrace(throwable);
+ String exception = throwable.toString();
+ errorMessage.put(ErrorFields.EXCEPTION.getName(), exception);
+ errorMessage.put(ErrorFields.STACK.getName(), stackTrace);
+ }
+
+ errorMessage.put(ErrorFields.TIMESTAMP.getName(), System.currentTimeMillis());
+ try {
+ errorMessage.put(ErrorFields.HOSTNAME.getName(), InetAddress.getLocalHost().getHostName());
+ } catch (UnknownHostException ex) {
+
+ }
+ if(rawMessages != null) {
+ for(int i = 0; i < rawMessages.size(); i++) {
+ Object rawMessage = rawMessages.get(i);
+ // If multiple messages are included add an index to the field name, otherwise leave it off
+ String rawMessageField = rawMessages.size() == 1 ? ErrorFields.RAW_MESSAGE.getName() : ErrorFields.RAW_MESSAGE.getName() + "_" + i;
+ String rawMessageBytesField = rawMessages.size() == 1 ? ErrorFields.RAW_MESSAGE_BYTES.getName() : ErrorFields.RAW_MESSAGE_BYTES.getName() + "_" + i;
+ if(rawMessage instanceof byte[]) {
+ errorMessage.put(rawMessageField, Bytes.toString((byte[])rawMessage));
+ errorMessage.put(rawMessageBytesField, toByteArrayList((byte[])rawMessage));
+ } else {
+ errorMessage.put(rawMessageField, rawMessage);
+ }
+ }
+ }
+
+ if (rawMessages != null && rawMessages.size() == 1) {
+ Object rawMessage = rawMessages.get(0);
+ if (rawMessage instanceof JSONObject) {
+ JSONObject rawJSON = (JSONObject) rawMessage;
+ if (errorFields != null) {
+ String errorFieldString = String.join(",", errorFields);
+ errorMessage.put(ErrorFields.ERROR_FIELDS.getName(), errorFieldString);
+ List<String> hashElements = errorFields.stream().map(errorField ->
+ String.format("%s-%s", errorField, rawJSON.get(errorField))).collect(Collectors.toList());
+ errorMessage.put(ErrorFields.ERROR_HASH.getName(), DigestUtils.sha256Hex(String.join("|", hashElements).getBytes(UTF_8)));
--- End diff --
I know there was some discussion around hashing in the messageboard discussion of this, but could you recap for posterity the justification for this again?
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100892848
--- Diff: metron-platform/metron-writer/src/main/java/org/apache/metron/writer/bolt/BulkMessageWriterBolt.java ---
@@ -74,7 +81,11 @@ public void prepare(Map stormConf, TopologyContext context, OutputCollector coll
this.writerComponent = new BulkWriterComponent<>(collector);
this.collector = collector;
super.prepare(stormConf, context, collector);
- messageGetter = MessageGetters.valueOf(messageGetterStr);
+ if (messageGetStrategyType.equals(MessageGetters.JSON_FROM_FIELD.name()) && messageGetField != null) {
--- End diff --
Yes because of the arg.
I like your suggestion. Let me play around with that idea.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/incubator-metron/pull/453
@merrimanr Can we make rawMessage, rawMessage_bytes, and rawMessage_hash to raw_message, raw_message_bytes, and raw_message_hash for consistency with error_type and failed_sensor_type?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/453#discussion_r100817317
--- Diff: metron-platform/metron-elasticsearch/src/main/config/elasticsearch_error.properties ---
@@ -0,0 +1,69 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
--- End diff --
This is duplicated in the MPACK isn't it? Is there not a way to have this defined once and used in multiple places?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
The latest commit includes the changes discussed. Most error messages now go to the indexing topic by default so there is no need for another error indexing topology. The error topic setting for the enrichment and indexing topologies are already exposed through flux properties so I left them there. I added a "parser.error.topic" property to the global config for parser topologies (should indexing topic be the default if that property is missing or should we let it fail?).
For the concern about indexing errors getting in a loop, I thought of a simple solution that required no extra work. If an error happens in the indexing topology, simply send it to a different error topic. Would this be acceptable? I wanted to throw this out there before implementing custom logic in the indexing topology classes.
The error message structure did change slightly. In my testing I realized that the type for the "raw_message" field could be different (byte[] vs JSON) depending on where the error happened (pre vs post parse). If an error message with byte[] as raw_message was indexed first, Elasticsearch would error on subsequent error messages with JSON as raw_message. To address this, I serialized JSON raw messages before storing them in this field. Now raw_message is always and string and raw_message_bytes is always a byte array.
All tests are passing and it has been tested in quick dev. Some tips on testing the different cases:
- the mock Bro sensor in quick dev contains a couple unparsable messages so parser errors should show up in the error index by default
- Invalid parser errors can be generated by adding a field validation to the global config that will always fail
- for testing indexing errors, copy a message from the indexing topic, change a field with number type (ip_src_port for example) to a non-number string, then put that message back into the indexing topic
@cestella, would be good to get your opinion on the MessageGetter classes and where that ended up. Not sure I love the syntax for default message getters.
The MPack changes are now minimal but it should still be tested there and that test is pending. The documentation still needs to be updated so please don't merge this until that is done.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Whoops sorry I see that you did mention a way to test it out. Sorry, on a phone ;)
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
@ottobackwards to answer some of your questions (sorry was at RSA all week):
- An extra topology will take up a couple worker slots but the performance implications should be minimal as long as the number or errors are small (hopefully they are)
- The error topology is not enabled in quick dev (my understanding is that ansible is deprecated) but can be tested there. Instructions are in the original PR description.
- Not in Docker yet but I will add it once we all agree on an approach. Changes should be minimal there.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/453#discussion_r100820671
--- Diff: metron-platform/metron-indexing/src/main/config/zookeeper/indexing/error.json ---
@@ -0,0 +1,17 @@
+{
+ "hdfs" : {
--- End diff --
error is too generic.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
No you are correct, we need a more comprehensive test plan. I'm still thinking about it. Triggering errors at each point in the topologies is not straightforward. Sending in a message that won't parse is an easy one and will cover a lot but it won't test everything.
> On Feb 13, 2017, at 7:35 AM, Casey Stella <no...@github.com> wrote:
>
> Whoops sorry I see that you did mention a way to test it out. Sorry, on a phone ;)
>
> \u2014
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub, or mute the thread.
>
---
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 #453: METRON-694: Index Errors from Topologies
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Can we make the Kafka queue used for errors parameterized in the various topologies? For those who want one indexing topology, we can use the "indexing" queue and for those who want a separate topology we can use an error queue.
If we don't, resource constraints are going to end up with us not spinning this separate topology up in vagrant. This will end in errors slipping through the cracks as this functionality won't be tested during the normal course of testing.
Because it cuts across the entire system, I consider having this functioning in vagrant to be a prerequisite for my acceptance of this PR.
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100796802
--- Diff: metron-platform/metron-indexing/src/main/config/zookeeper/indexing/error.json ---
@@ -0,0 +1,17 @@
+{
+ "hdfs" : {
--- End diff --
I'm a little confused here. Are we sending every error message through with a source type of "error" so that this config can be found in the error writer topology?
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100897656
--- Diff: metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/configuration/metron-env.xml ---
@@ -37,6 +37,12 @@
<display-name>Metron apps indexed HDFS dir</display-name>
</property>
<property>
--- End diff --
Actually you can control the file name in the index config so separate index configs solves that
---
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 #453: METRON-694: Index Errors from Topologies
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/incubator-metron/pull/453
What comes to mind is the 'source' of an error. Is this error wrong because METRON thinks it is invalid, or it it wrong because of some other configuration specific evaluation. I don't think we do this now with stellar ( IF (BUSINESS_RULE(rule_valid_field,field1) == FALSE) DROP_ERROR()) however.
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100801585
--- Diff: metron-platform/metron-writer/src/main/java/org/apache/metron/writer/bolt/BulkMessageWriterBolt.java ---
@@ -74,7 +81,11 @@ public void prepare(Map stormConf, TopologyContext context, OutputCollector coll
this.writerComponent = new BulkWriterComponent<>(collector);
this.collector = collector;
super.prepare(stormConf, context, collector);
- messageGetter = MessageGetters.valueOf(messageGetterStr);
+ if (messageGetStrategyType.equals(MessageGetters.JSON_FROM_FIELD.name()) && messageGetField != null) {
--- End diff --
Now that I think about it, you might want to change the `arg` to `String` from `Object` and either:
* insist that the implementing `MessageGetter`'s provide an appropriate constructor
* convert in the enum (e.g. `arg -> BytesFromPosition(ConversionUtils.convert(arg, Integer.class)`)
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
The error message will look something like this:
{
"exception": "java.lang.Exception: there was an error",
"hostname": "host",
"stack": "java.lang.Exception: ...",
"time": 1485295416563,
"message": "there was an error",
"rawMessage": "raw message",
"rawMessage_bytes": [],
"source.type": "error",
"error_type": "parser_error",
"failed_sensor_type":"bro",
"rawMessage_hash": "dde41b9920954f94066daf6291fb58a9"
}
---
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 #453: METRON-694: Index Errors from Topologies
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/incubator-metron/pull/453
@cestella - how can we utilize the error indexing if we were going to say - output errors or warnings that there were deprecated stellar statements?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
That wasn't an exact structure. Take a look at the ErrorFields enum in https://github.com/apache/incubator-metron/pull/453/files#diff-19fcef3f36d5353a8ad399a128f40f3e. It's very close to what you're suggesting.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/453
> After thinking about it a few minutes more, maybe the answer to the deployment question is to deploy all the necessary scripts, configurations, etc but just not start it by default?
I don't think you need to even create any scripts or configurations as part of this PR to deploy a separate topology for errors. If error indexing works as part of the existing indexing topology out-of-the-box AND we have the 'ability' to deploy an error indexing topology separately, then I'd +1 it.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Based on this PR, what I'd probably do is:
* Walk through setting up the CSV parser
* Set up a field validation
* Send valid data through, make sure it works and is in the index
* Send data which is syntactically correct but fails validation verify that it handles that correctly
* where does invalid data go in this new setup? I notice some changes in that part of the code.
* Send syntactically incorrect data through and verify that it fails that
* send 4 messages through and verify nothing is written
* send the 5th and verify that it is written and that the write configs are honored for indexing
---
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 #453: METRON-694: Index Errors from Topologies
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Right, I am less concerned with the act of finding errors in single node vagrant. I'm more interested in mimicking the mechanisms used in a multi node deployment in our acceptance testing environment. If we don't do that then errors will crop up around this that we do not find because it's not being exercised. We should make vagrant as similar to a real deployment as possible; if we do not then we will pay the price in the form of bugs.
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100799069
--- Diff: metron-platform/metron-writer/src/main/java/org/apache/metron/writer/bolt/BulkMessageWriterBolt.java ---
@@ -74,7 +81,11 @@ public void prepare(Map stormConf, TopologyContext context, OutputCollector coll
this.writerComponent = new BulkWriterComponent<>(collector);
this.collector = collector;
super.prepare(stormConf, context, collector);
- messageGetter = MessageGetters.valueOf(messageGetterStr);
+ if (messageGetStrategyType.equals(MessageGetters.JSON_FROM_FIELD.name()) && messageGetField != null) {
--- End diff --
Why is this special case for JSON_FROM_FIELD required? Is it because of the arg? If so, it's probably better to adjust `MessageGetters` rather than doing it here. I'd suggest making a factory rather than a strategy in this case for it:
```
public enum MessageGetters {
BYTES_FROM_POSITION(arg -> new BytesFromPosition(arg)),
JSON_FROM_POSITION(arg -> new JSONFromPosition(arg)),
JSON_FROM_FIELD(arg -> new JSONFromField(arg));
Function<Object, MessageGetStrategy> messageGetStrategy;
MessageGetters(Function<Object, MessageGetStrategy> messageGetStrategy) {
this.messageGetStrategy = messageGetStrategy;
}
public MessageGetStrategy get(Object arg) {
return messageGetStrategy.apply(arg);
}
}
```
Then it becomes `MessageGetters.valueOf(messageGetStrategyType).get(messageGetField)`
---
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 #453: METRON-694: Index Errors from Topologies
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/453
So from what I see we have a new topology and every error message has a source type of "error", correct? Is there any reason we didn't just use the normal indexing topology and index errors with the usual messages? Presumably their volume will be small.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/453#discussion_r100815725
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/error/MetronError.java ---
@@ -0,0 +1,200 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.error;
+
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.metron.common.Constants;
+import org.apache.metron.common.Constants.ErrorType;
+import org.json.simple.JSONObject;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.metron.common.Constants.ErrorFields;
+
+public class MetronError {
+
+ private String message;
+ private Throwable throwable;
+ private String sensorType = "error";
+ private ErrorType errorType = ErrorType.DEFAULT_ERROR;
+ private Set<String> errorFields;
+ private List<Object> rawMessages;
+
+ public MetronError withMessage(String message) {
+ this.message = message;
+ return this;
+ }
+
+ public MetronError withThrowable(Throwable throwable) {
+ this.throwable = throwable;
+ return this;
+ }
+
+ public MetronError withSensorType(String sensorType) {
+ this.sensorType = sensorType;
+ return this;
+ }
+
+ public MetronError withErrorType(ErrorType errorType) {
+ this.errorType = errorType;
+ return this;
+ }
+
+ public MetronError withErrorFields(Set<String> errorFields) {
+ this.errorFields = errorFields;
+ return this;
+ }
+
+
+ public MetronError addRawMessage(Object rawMessage) {
+ if (rawMessage != null) {
+ if (this.rawMessages == null) {
+ this.rawMessages = new ArrayList<>();
+ }
+ this.rawMessages.add(rawMessage);
+ }
+ return this;
+ }
+
+ public MetronError withRawMessages(List<Object> rawMessages) {
+ this.rawMessages = rawMessages;
+ return this;
+ }
+
+ public Optional<Throwable> getThrowable() {
+ return throwable != null ? Optional.of(throwable) : Optional.empty();
+ }
+
+ @SuppressWarnings({"unchecked"})
+ public JSONObject getJSONObject() {
+ JSONObject errorMessage = new JSONObject();
+ errorMessage.put(Constants.SENSOR_TYPE, "error");
+
+ /*
+ * Save full stack trace in object.
+ */
+ if (throwable != null) {
+ String stackTrace = ExceptionUtils.getStackTrace(throwable);
+ String exception = throwable.toString();
+ errorMessage.put(ErrorFields.EXCEPTION.getName(), exception);
+ errorMessage.put(ErrorFields.STACK.getName(), stackTrace);
+ }
+
+ errorMessage.put(ErrorFields.TIMESTAMP.getName(), System.currentTimeMillis());
+ try {
+ errorMessage.put(ErrorFields.HOSTNAME.getName(), InetAddress.getLocalHost().getHostName());
+ } catch (UnknownHostException ex) {
+
+ }
+ if(rawMessages != null) {
+ for(int i = 0; i < rawMessages.size(); i++) {
+ Object rawMessage = rawMessages.get(i);
+ // If multiple messages are included add an index to the field name, otherwise leave it off
+ String rawMessageField = rawMessages.size() == 1 ? ErrorFields.RAW_MESSAGE.getName() : ErrorFields.RAW_MESSAGE.getName() + "_" + i;
+ String rawMessageBytesField = rawMessages.size() == 1 ? ErrorFields.RAW_MESSAGE_BYTES.getName() : ErrorFields.RAW_MESSAGE_BYTES.getName() + "_" + i;
+ if(rawMessage instanceof byte[]) {
+ errorMessage.put(rawMessageField, Bytes.toString((byte[])rawMessage));
+ errorMessage.put(rawMessageBytesField, toByteArrayList((byte[])rawMessage));
+ } else {
+ errorMessage.put(rawMessageField, rawMessage);
+ }
+ }
+ }
+
+ if (rawMessages != null && rawMessages.size() == 1) {
+ Object rawMessage = rawMessages.get(0);
+ if (rawMessage instanceof JSONObject) {
+ JSONObject rawJSON = (JSONObject) rawMessage;
+ if (errorFields != null) {
+ String errorFieldString = String.join(",", errorFields);
+ errorMessage.put(ErrorFields.ERROR_FIELDS.getName(), errorFieldString);
+ List<String> hashElements = errorFields.stream().map(errorField ->
+ String.format("%s-%s", errorField, rawJSON.get(errorField))).collect(Collectors.toList());
+ errorMessage.put(ErrorFields.ERROR_HASH.getName(), DigestUtils.sha256Hex(String.join("|", hashElements).getBytes(UTF_8)));
--- End diff --
If we do the hashing, it should be done in metron-common so that we can use consistent hashing approaches
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100848754
--- Diff: metron-platform/metron-indexing/src/main/config/zookeeper/indexing/error.json ---
@@ -0,0 +1,17 @@
+{
+ "hdfs" : {
--- End diff --
Yes that is correct. There is an "error_type" type fields that distinguishes between the different type of errors. If you think there is a strong reason to make "source.type" the "error_type" field and add indexing configs for all the different types we can do that.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/incubator-metron/pull/453
@merrimanr Thanks for pointing me there. That looks good.
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100891429
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/error/MetronError.java ---
@@ -0,0 +1,200 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.error;
+
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.metron.common.Constants;
+import org.apache.metron.common.Constants.ErrorType;
+import org.json.simple.JSONObject;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.metron.common.Constants.ErrorFields;
+
+public class MetronError {
+
+ private String message;
+ private Throwable throwable;
+ private String sensorType = "error";
+ private ErrorType errorType = ErrorType.DEFAULT_ERROR;
+ private Set<String> errorFields;
+ private List<Object> rawMessages;
+
+ public MetronError withMessage(String message) {
+ this.message = message;
+ return this;
+ }
+
+ public MetronError withThrowable(Throwable throwable) {
+ this.throwable = throwable;
+ return this;
+ }
+
+ public MetronError withSensorType(String sensorType) {
+ this.sensorType = sensorType;
+ return this;
+ }
+
+ public MetronError withErrorType(ErrorType errorType) {
+ this.errorType = errorType;
+ return this;
+ }
+
+ public MetronError withErrorFields(Set<String> errorFields) {
+ this.errorFields = errorFields;
+ return this;
+ }
+
+
+ public MetronError addRawMessage(Object rawMessage) {
+ if (rawMessage != null) {
+ if (this.rawMessages == null) {
+ this.rawMessages = new ArrayList<>();
+ }
+ this.rawMessages.add(rawMessage);
+ }
+ return this;
+ }
+
+ public MetronError withRawMessages(List<Object> rawMessages) {
+ this.rawMessages = rawMessages;
+ return this;
+ }
+
+ public Optional<Throwable> getThrowable() {
+ return throwable != null ? Optional.of(throwable) : Optional.empty();
+ }
+
+ @SuppressWarnings({"unchecked"})
+ public JSONObject getJSONObject() {
+ JSONObject errorMessage = new JSONObject();
+ errorMessage.put(Constants.SENSOR_TYPE, "error");
+
+ /*
+ * Save full stack trace in object.
+ */
+ if (throwable != null) {
+ String stackTrace = ExceptionUtils.getStackTrace(throwable);
+ String exception = throwable.toString();
+ errorMessage.put(ErrorFields.EXCEPTION.getName(), exception);
+ errorMessage.put(ErrorFields.STACK.getName(), stackTrace);
+ }
+
+ errorMessage.put(ErrorFields.TIMESTAMP.getName(), System.currentTimeMillis());
+ try {
+ errorMessage.put(ErrorFields.HOSTNAME.getName(), InetAddress.getLocalHost().getHostName());
+ } catch (UnknownHostException ex) {
+
+ }
+ if(rawMessages != null) {
+ for(int i = 0; i < rawMessages.size(); i++) {
+ Object rawMessage = rawMessages.get(i);
+ // If multiple messages are included add an index to the field name, otherwise leave it off
+ String rawMessageField = rawMessages.size() == 1 ? ErrorFields.RAW_MESSAGE.getName() : ErrorFields.RAW_MESSAGE.getName() + "_" + i;
+ String rawMessageBytesField = rawMessages.size() == 1 ? ErrorFields.RAW_MESSAGE_BYTES.getName() : ErrorFields.RAW_MESSAGE_BYTES.getName() + "_" + i;
+ if(rawMessage instanceof byte[]) {
+ errorMessage.put(rawMessageField, Bytes.toString((byte[])rawMessage));
+ errorMessage.put(rawMessageBytesField, toByteArrayList((byte[])rawMessage));
+ } else {
+ errorMessage.put(rawMessageField, rawMessage);
+ }
+ }
+ }
+
+ if (rawMessages != null && rawMessages.size() == 1) {
+ Object rawMessage = rawMessages.get(0);
+ if (rawMessage instanceof JSONObject) {
+ JSONObject rawJSON = (JSONObject) rawMessage;
+ if (errorFields != null) {
+ String errorFieldString = String.join(",", errorFields);
+ errorMessage.put(ErrorFields.ERROR_FIELDS.getName(), errorFieldString);
+ List<String> hashElements = errorFields.stream().map(errorField ->
+ String.format("%s-%s", errorField, rawJSON.get(errorField))).collect(Collectors.toList());
+ errorMessage.put(ErrorFields.ERROR_HASH.getName(), DigestUtils.sha256Hex(String.join("|", hashElements).getBytes(UTF_8)));
--- End diff --
Sounds good @ottobackwards. I will move that to a util class.
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100897917
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/error/MetronError.java ---
@@ -0,0 +1,200 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.error;
+
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.metron.common.Constants;
+import org.apache.metron.common.Constants.ErrorType;
+import org.json.simple.JSONObject;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.metron.common.Constants.ErrorFields;
+
+public class MetronError {
+
+ private String message;
+ private Throwable throwable;
+ private String sensorType = "error";
+ private ErrorType errorType = ErrorType.DEFAULT_ERROR;
+ private Set<String> errorFields;
+ private List<Object> rawMessages;
+
+ public MetronError withMessage(String message) {
+ this.message = message;
+ return this;
+ }
+
+ public MetronError withThrowable(Throwable throwable) {
+ this.throwable = throwable;
+ return this;
+ }
+
+ public MetronError withSensorType(String sensorType) {
+ this.sensorType = sensorType;
+ return this;
+ }
+
+ public MetronError withErrorType(ErrorType errorType) {
+ this.errorType = errorType;
+ return this;
+ }
+
+ public MetronError withErrorFields(Set<String> errorFields) {
+ this.errorFields = errorFields;
+ return this;
+ }
+
+
+ public MetronError addRawMessage(Object rawMessage) {
+ if (rawMessage != null) {
+ if (this.rawMessages == null) {
+ this.rawMessages = new ArrayList<>();
+ }
+ this.rawMessages.add(rawMessage);
+ }
+ return this;
+ }
+
+ public MetronError withRawMessages(List<Object> rawMessages) {
+ this.rawMessages = rawMessages;
+ return this;
+ }
+
+ public Optional<Throwable> getThrowable() {
+ return throwable != null ? Optional.of(throwable) : Optional.empty();
+ }
+
+ @SuppressWarnings({"unchecked"})
+ public JSONObject getJSONObject() {
+ JSONObject errorMessage = new JSONObject();
+ errorMessage.put(Constants.SENSOR_TYPE, "error");
+
+ /*
+ * Save full stack trace in object.
--- End diff --
I think that's a good idea. I'll break that method down into smaller methods.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
After thinking about it a few minutes more, maybe the answer to the deployment question is to deploy all the necessary scripts, configurations, etc but just not start it by default?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I think you're right, that could happen (didn't this happen to you at one point?). So what is the correct approach then? Do we leave off the raw message field if the error happens in the indexing topology?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Technically generating an error message and sending it back through the indexing topology should be ok now because the original message is serialized into a string and shouldn't cause another failure. Does the loop problem still exist given this info?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I will create an issue to track the possibility for looping.
Other than that, this looks good. +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 pull request #453: METRON-694: Index Errors from Topologies
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-metron/pull/453
---
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 #453: METRON-694: Index Errors from Topologies
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Can you provide an acceptance test plan for validation on vagrant or a cluster?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I agree, I [recommended the same thing](https://lists.apache.org/thread.html/2a673bc97d975bc7e8e160228742c07a8cf45e43c1b1efb3fea83579@%3Cdev.metron.apache.org%3E) on the dev list and in IRC in the past. In the case where the TTL expires, we should do *something*. Maybe we attempt to place a standard, simple message to be indexed which has a very high likelihood of successfully being stored and that is exempt from any sort of retries or TTLs.
That said, there are no other specific looping scenarios that I'm currently able to envision, which is why I wasn't pushing that concept further for this PR. Given the one that we are aware of, if we only took the TTL route, it would just loop and then fail the entire message, where adding `"ignore_above": 8191` would never loop and only removes the bare minimum from the message in order to get it to properly index, for the given known problem scenario.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I believe you would still have the issue in some cases. The limitation is that the raw_message field could be a long set of characters, processed as a single token. I don't know of a way to configure ES to bypass this limitation, because no matter what you could have a long string that won't get tokenized with the built-ins (i.e. for instance, the URI field of an HTTP message from Bro).
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I would prefer a separate error topology but you and others have provided good reasons for not doing that. Single indexing approach by default is a good compromise and fine with me.
Just want to reconcile all the different opinions and make sure I'm clear on what needs to be done before I start making these changes.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
You need a source.type to look up a parser config right? I don't think you can parse a message without it. I'm relying on the bolts to supply the source.type and I think that's where any missing source.type reconciliation should happen (defaulting to Kafka topic for example). In the event a bolt can't supply a source type, we can just leave that field off.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/453#discussion_r100859285
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/error/MetronError.java ---
@@ -0,0 +1,200 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.error;
+
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.metron.common.Constants;
+import org.apache.metron.common.Constants.ErrorType;
+import org.json.simple.JSONObject;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.metron.common.Constants.ErrorFields;
+
+public class MetronError {
+
+ private String message;
+ private Throwable throwable;
+ private String sensorType = "error";
+ private ErrorType errorType = ErrorType.DEFAULT_ERROR;
+ private Set<String> errorFields;
+ private List<Object> rawMessages;
+
+ public MetronError withMessage(String message) {
+ this.message = message;
+ return this;
+ }
+
+ public MetronError withThrowable(Throwable throwable) {
+ this.throwable = throwable;
+ return this;
+ }
+
+ public MetronError withSensorType(String sensorType) {
+ this.sensorType = sensorType;
+ return this;
+ }
+
+ public MetronError withErrorType(ErrorType errorType) {
+ this.errorType = errorType;
+ return this;
+ }
+
+ public MetronError withErrorFields(Set<String> errorFields) {
+ this.errorFields = errorFields;
+ return this;
+ }
+
+
+ public MetronError addRawMessage(Object rawMessage) {
+ if (rawMessage != null) {
+ if (this.rawMessages == null) {
+ this.rawMessages = new ArrayList<>();
+ }
+ this.rawMessages.add(rawMessage);
+ }
+ return this;
+ }
+
+ public MetronError withRawMessages(List<Object> rawMessages) {
+ this.rawMessages = rawMessages;
+ return this;
+ }
+
+ public Optional<Throwable> getThrowable() {
+ return throwable != null ? Optional.of(throwable) : Optional.empty();
+ }
+
+ @SuppressWarnings({"unchecked"})
+ public JSONObject getJSONObject() {
+ JSONObject errorMessage = new JSONObject();
+ errorMessage.put(Constants.SENSOR_TYPE, "error");
+
+ /*
+ * Save full stack trace in object.
+ */
+ if (throwable != null) {
+ String stackTrace = ExceptionUtils.getStackTrace(throwable);
+ String exception = throwable.toString();
+ errorMessage.put(ErrorFields.EXCEPTION.getName(), exception);
+ errorMessage.put(ErrorFields.STACK.getName(), stackTrace);
+ }
+
+ errorMessage.put(ErrorFields.TIMESTAMP.getName(), System.currentTimeMillis());
+ try {
+ errorMessage.put(ErrorFields.HOSTNAME.getName(), InetAddress.getLocalHost().getHostName());
+ } catch (UnknownHostException ex) {
+
+ }
+ if(rawMessages != null) {
+ for(int i = 0; i < rawMessages.size(); i++) {
+ Object rawMessage = rawMessages.get(i);
+ // If multiple messages are included add an index to the field name, otherwise leave it off
+ String rawMessageField = rawMessages.size() == 1 ? ErrorFields.RAW_MESSAGE.getName() : ErrorFields.RAW_MESSAGE.getName() + "_" + i;
+ String rawMessageBytesField = rawMessages.size() == 1 ? ErrorFields.RAW_MESSAGE_BYTES.getName() : ErrorFields.RAW_MESSAGE_BYTES.getName() + "_" + i;
+ if(rawMessage instanceof byte[]) {
+ errorMessage.put(rawMessageField, Bytes.toString((byte[])rawMessage));
+ errorMessage.put(rawMessageBytesField, toByteArrayList((byte[])rawMessage));
+ } else {
+ errorMessage.put(rawMessageField, rawMessage);
+ }
+ }
+ }
+
+ if (rawMessages != null && rawMessages.size() == 1) {
+ Object rawMessage = rawMessages.get(0);
+ if (rawMessage instanceof JSONObject) {
+ JSONObject rawJSON = (JSONObject) rawMessage;
+ if (errorFields != null) {
+ String errorFieldString = String.join(",", errorFields);
+ errorMessage.put(ErrorFields.ERROR_FIELDS.getName(), errorFieldString);
+ List<String> hashElements = errorFields.stream().map(errorField ->
+ String.format("%s-%s", errorField, rawJSON.get(errorField))).collect(Collectors.toList());
+ errorMessage.put(ErrorFields.ERROR_HASH.getName(), DigestUtils.sha256Hex(String.join("|", hashElements).getBytes(UTF_8)));
--- End diff --
Hashing was suggested to provide a unique identifier of the original information, and thus giving you something to key in on and create a count of unique events/prioritize more prevalent issues without the concern of cyclical issues (for instance, if the issue is with indexing a specific message, and you try to index it again, it will just fail in a loop).
---
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 #453: METRON-694: Index Errors from Topologies
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/453#discussion_r100813285
--- Diff: metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/configuration/metron-env.xml ---
@@ -37,6 +37,12 @@
<display-name>Metron apps indexed HDFS dir</display-name>
</property>
<property>
--- End diff --
Should these HDSF directories more specifically pertain to topology processing? As the other use cases came into being, just 'error' may be too generic
---
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 #453: METRON-694: Index Errors from Topologies
Posted by james-sirota <gi...@git.apache.org>.
Github user james-sirota commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Hi guys, this PR is built on one fundamental assumption: kafka is always available. The source of truth for errors, therefore, is a kafka topic. In a production setting errors should go into their own topic and the retention period (size) of that topic queue should be set very high so that you can retain as many errors as you can. The reason we are are making this configurable is so that we can easier test this in Ansible by throwing both errors and valid telemetry into the same topic. In production we would not do this and would have a dedicated topic and a dedicated topology to error writing with parallelism tuned way down to prioritize ingest of actual valid telemetry over errors. The writing topology should attempt to write errors from the queue exactly to either ES, HDFS, or both exactly once. If it cannot do that then it should ping whatever infrastructure monitoring component that you are using that your ES or HDFS is down. That, however, is a different PR and is ou
t of context here. I will need to file this PR as follow-on work.
With that said, I personally see no problem with the way this PR is implemented. It allows for a dedicated topic and writing of errors into ES or HDFS exactly once if running in production setting. There is an option to configure the topic so you can have telemetry and errors in the same topic for testing on Ansible. So +1 from me
---
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 #453: METRON-694: Index Errors from Topologies
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Did you name this PR correctly? Should it be METRON-695?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Ok, catching up on this:
Regarding invalid vs errors, I'm ok with the notion of treating them the same, but it's different from how we've done this in the past. Prior to this, invalid messages went into a queue untouched. Now, they will be sent along with the errors wrapped in a JSON structure (thought the raw messages will be captured as a field). I'm not necessarily against this, but it's different than as we originally designed and I want to call it out.
I guess I'm ok with adding a new topology, but it's added a fair amount of complexity to this JIRA and I'm not sure I buy that indexing errors (what should be a rare event, IMO, in a healthy set of topologies) would materially change how we tune the indexing topology, which has to be able to handle the unified traffic of all of the parsers anyway.
So, if I have an error in a message with a source type of `bro`, what indexing config am I using as of this JIRA?
* the `bro` indexing config
* the `error` indexing config
* a new `bro_error` indexing config
---
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 #453: METRON-694: Index Errors from Topologies
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Just real quick before I start to review:
- What are the performance implications of this? How can we measure that?
- What is the effect of running this on quick dev? is it to be enabled by default or disabled for load/performance?
- Does this run in Docker?
- I don't see any documentation changes. What needs to be done, should it be in this or another PR?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Does anyone have any outstanding comments that they need addressed?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I think focusing on the one specific error that we've seen is not the right way to think about this. Many different types of errors would cause unexpected looping, no? When unexpected looping occurs, it is very difficult to see and diagnose, right? Considering that, I would suggest adding a simple decrementing TTL counter to avoid unexpected loops.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Yeah. So, I edited my prior message but for clarity, I think we should just set "ignore_above": 8191 ([details](https://www.elastic.co/guide/en/elasticsearch/reference/current/ignore-above.html)) like we did with [METRON-517](https://github.com/apache/incubator-metron/pull/358), and we should be good to go. The worst case is that, if the rawMessage/raw_message is too long, it would be not be indexed or stored, which is what we want.
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100898119
--- Diff: metron-platform/metron-elasticsearch/src/main/config/elasticsearch_error.properties ---
@@ -0,0 +1,69 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
--- End diff --
Unless we decide we only want one indexing topology, we need separate configs. It looks like a duplicate but it's slightly different.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
On the topic of invalid messages, they are now treated as error messages. They can still be distinguished as invalid message though. Is there any reason they should be treated differently? As mentioned above, it's easy enough to change the source.type and add an indexing config.
Is there a case where source.type is not error? If so then that is a mistake.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by james-sirota <gi...@git.apache.org>.
Github user james-sirota commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I think for a single node this is not an issue because all your storm logs are in the same place so you can just pipe to grep. This feature is meant to address gathering errors from different workers in large cluster deployments
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Default in this case is JSON_FROM_FIELD without specifying the field (defaults to "message"). I couldn't think of an ideal way to do this so open to ideas. We could just treat a null field to mean default and get rid of DEFAULT_JSON_FROM_FIELD. I will add unit tests.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/453#discussion_r100815478
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/error/MetronError.java ---
@@ -0,0 +1,200 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.metron.common.error;
+
+import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.metron.common.Constants;
+import org.apache.metron.common.Constants.ErrorType;
+import org.json.simple.JSONObject;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.metron.common.Constants.ErrorFields;
+
+public class MetronError {
+
+ private String message;
+ private Throwable throwable;
+ private String sensorType = "error";
+ private ErrorType errorType = ErrorType.DEFAULT_ERROR;
+ private Set<String> errorFields;
+ private List<Object> rawMessages;
+
+ public MetronError withMessage(String message) {
+ this.message = message;
+ return this;
+ }
+
+ public MetronError withThrowable(Throwable throwable) {
+ this.throwable = throwable;
+ return this;
+ }
+
+ public MetronError withSensorType(String sensorType) {
+ this.sensorType = sensorType;
+ return this;
+ }
+
+ public MetronError withErrorType(ErrorType errorType) {
+ this.errorType = errorType;
+ return this;
+ }
+
+ public MetronError withErrorFields(Set<String> errorFields) {
+ this.errorFields = errorFields;
+ return this;
+ }
+
+
+ public MetronError addRawMessage(Object rawMessage) {
+ if (rawMessage != null) {
+ if (this.rawMessages == null) {
+ this.rawMessages = new ArrayList<>();
+ }
+ this.rawMessages.add(rawMessage);
+ }
+ return this;
+ }
+
+ public MetronError withRawMessages(List<Object> rawMessages) {
+ this.rawMessages = rawMessages;
+ return this;
+ }
+
+ public Optional<Throwable> getThrowable() {
+ return throwable != null ? Optional.of(throwable) : Optional.empty();
+ }
+
+ @SuppressWarnings({"unchecked"})
+ public JSONObject getJSONObject() {
+ JSONObject errorMessage = new JSONObject();
+ errorMessage.put(Constants.SENSOR_TYPE, "error");
+
+ /*
+ * Save full stack trace in object.
--- End diff --
Instead of having a method with multiple null checks....
Maybe have a method that just calls other methods, and put the checks in there
handleThrowable(errorMessage);
handleRawMessages(errorMessage);
etc etc
---
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 #453: METRON-694: Index Errors from Topologies
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/incubator-metron/pull/453
> For the concern about indexing errors getting in a loop, I thought of a simple solution that required no extra work. If an error happens in the indexing topology, simply send it to a different error topic. Would this be acceptable? I wanted to throw this out there before implementing custom logic in the indexing topology classes.
Hmm, that seems like it would be somewhat confusing to me. Unless I'm missing something, that error would just not show up in the "typical" topic/dashboard. How about if there's an error during indexing, retry with more bare-bones data and adding a k/v pair that indicates such?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I think removing the error indexing topology is fine as long as we're careful to avoid error messages getting stuck in a loop in the case of a failed index write. I agree that it will remove complexity and make this PR easier to digest.
As of this JIRA all errors go to a single error index with source type and error type as JSON fields. To answer your question: the error indexing config. I can see the advantage of being able to configure each source type/error type combination separately but I'm not sure it matters that much since we expect it to be very low volume. I will think about that more this weekend. From an implementation standpoint it's easy to modify so the design should be driven by the queries we expect.
Same applies to invalid messages. Easy to do it either way, just need to decide what makes the most sense from a user's perspective.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
In response to "Is there any reason we didn't just use the normal indexing topology". Here are the issues I see with doing that. First, I think we should be careful about putting additional load on the indexing topology. For parser errors I think it's a wash, an error gets indexed with the original message instead of the parsed message. But for enrichment errors, new error messages are generated and passed along. If we send all errors and messages to the same indexing topology a burst of errors could affect the latency of other messages.
Here is another scenario that could cause problems. Imagine a message fails to index and an error is thrown in the indexing topology. If we send all errors and messages to the same topology, that error could be replayed in the indexing topology over and over. We could add some logic to keep that from happening but now writer bolts are more complex and specific to the indexing topology. You will notice the error indexing topology is different in that it does not write back out to Kafka.
I feel like by mixing error messages and legitimate messages we are asking for trouble. Having them separate allows us to tune the topologies separately and avoid the problems I mention above. Tuning the indexing topology as it stands now is challenging enough. I can understand not wanting to allocate additional worker slots though. Maybe I am being too conservative.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Those scripts and configurations have already been created and are part of this PR so the question is should we remove them.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I think a configurable error topic is a reasonable request. The requirements need a little clarification though.
How granular should this configuration property be? Should it be kept in the global config? Do we want to expose this property per topology? In that case it would need to be added to parser configs and flux properties. Do we want it to be per sensor? In that case it would be added to sensor, enrichment and indexing configs.
Should this topology be on by default or not? If it's off by default then there is no ansible work to be done other than deploying configurations. @cestella, in the case that it's off by default, what would you consider functioning in vagrant to be?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/453
Sorry I should have included this in the original description. I still need to update the various READMEs, that task is outstanding and this should not be merged until that is done. This is a big PR so I want to get further along in the review process so I don't have to rewrite the documentation several times.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I'm sorry if this was already covered and I missed it, I'm writing this from my phone. How does this handle errors during indexing itself? For instance, if the failure is due to the content of a specific field and that is placed in rawMessage/raw_message, wouldn't we have a failure loop? I was under the assumption that rawMessage/raw_message was not going to be included because of this sort of cyclical concern. Just wanted to double check on that because I've definitely seen those sort of failures in my environment.
This also may be mitigated by the outcome of the ongoing dev list conversation "[DISCUSS] Management of Elastic and other index schemas" because we could attempt to fix those known sort of issues in the abstraction, but it still leaves us open to unknown issues.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/453
In regards to `source.type`, what does the error message look-like? Is it transformed in any way? What fields are in the error message?
If we set the `source.type` to `error` (which seems perfectly logical to me) how can I tell what the original `source.type` was?
---
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 #453: METRON-694: Index Errors from Topologies
Posted by james-sirota <gi...@git.apache.org>.
Github user james-sirota commented on the issue:
https://github.com/apache/incubator-metron/pull/453
I lean toward having a separate kafka queue for errors. Errors are lower in volume, but are a lot more verbose. You are essentially indexing large text document when you do this and you can easily bring down ES when something goes wrong and you get lots of errors. I want the ability to first tune my cluster enough where the volume of errors is rather low. In my daily workflow I would just monitor the kafka queue via the command line consumer until I reduce the number of errors. Then if i needed to (and i want this to be optional) I can index them into ES. Or I can just dump them out to file straight out of kafka
---
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 #453: METRON-694: Index Errors from Topologies
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/incubator-metron/pull/453
> I guess I'm ok with adding a new topology, but it's added a fair amount of complexity to this JIRA and I'm not sure I buy that indexing errors (what should be a rare event, IMO, in a healthy set of topologies) would materially change how we tune the indexing topology...
I agree with @cestella here. I think by default we should have a single indexing topology, for errors and everything else.
We should be able to make the code configurable enough, so that if a user runs into performance issues caused by indexing telemetry and errors in the same topology, they can easily deploy a second indexing topology that is dedicated to indexing the errors.
This would give them the flexibility that @merrimanr was going for when they need it. But that should be a non-default, "advanced" configuration of Metron.
---
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 #453: METRON-694: Index Errors from Topologies
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/453#discussion_r100894071
--- Diff: metron-deployment/packaging/ambari/metron-mpack/src/main/resources/common-services/METRON/CURRENT/configuration/metron-env.xml ---
@@ -37,6 +37,12 @@
<display-name>Metron apps indexed HDFS dir</display-name>
</property>
<property>
--- End diff --
I think if we move the error type into the source.type field it will solve this. I believe the source type is added to the end of the HDFS path.
---
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 #453: METRON-694: Index Errors from Topologies
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/incubator-metron/pull/453
That makes complete sense, we should call that stuff out pre-review. I think what we are seeing is that folks have some really good ideas and are willing to contribute to documentation on this project, we should take advantage.
---
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.
---