You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by mmiklavc <gi...@git.apache.org> on 2016/05/12 19:59:30 UTC
[GitHub] incubator-metron pull request: METRON-155 Added query filtering ca...
GitHub user mmiklavc opened a pull request:
https://github.com/apache/incubator-metron/pull/119
METRON-155 Added query filtering capability for PCAP via Metron REST API
https://issues.apache.org/jira/browse/METRON-155
Have not run the Vagrant deploy yet, but this is big PR so i wanted to get this reviewed asap.
- Added PCAP filter by query.
- Use Java's Stream, Function, and Predicate impls.
- Added QueryPcapFilterTest
- Added tests in REST api for new query option
- Added integration tests to PCAP topology
- Fixed bug in QueryParser - handle empty string as pass-through impl like WHERE TRUE
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mmiklavc/incubator-metron METRON-155
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-metron/pull/119.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 #119
----
commit b3af68484718a66393f4b0417bc525c462afed5d
Author: Michael Miklavcic <mi...@clevelandflash.com>
Date: 2016-05-12T19:43:09Z
METRON-155 Added query filtering capability for PCAP via Metron REST API
----
---
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: METRON-155 Added query filtering ca...
Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/119#discussion_r63112187
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/query/PredicateProcessor.java ---
@@ -18,12 +18,20 @@
package org.apache.metron.common.query;
-import org.antlr.v4.runtime.*;
-import org.apache.metron.common.query.generated.*;
+import org.antlr.v4.runtime.ANTLRInputStream;
+import org.antlr.v4.runtime.CommonTokenStream;
+import org.antlr.v4.runtime.TokenStream;
+import org.apache.metron.common.query.generated.PredicateLexer;
+import org.apache.metron.common.query.generated.PredicateParser;
+
+import static org.apache.commons.lang3.StringUtils.isEmpty;
public class PredicateProcessor {
public boolean parse(String rule, VariableResolver resolver) {
+ if (isEmpty(rule)) {
--- End diff --
What about NPE on rule.trim()? re: &&
---
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: METRON-155 Added query filtering ca...
Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/119#discussion_r63113925
--- Diff: metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/filter/query/QueryPcapFilter.java ---
@@ -0,0 +1,74 @@
+/**
+ * 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.pcap.filter.query;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.metron.common.Constants;
+import org.apache.metron.common.query.PredicateProcessor;
+import org.apache.metron.common.query.VariableResolver;
+import org.apache.metron.pcap.PacketInfo;
+import org.apache.metron.pcap.PcapHelper;
+import org.apache.metron.pcap.filter.PcapFilter;
+import org.apache.metron.pcap.filter.PcapFilterConfigurator;
+import org.apache.metron.pcap.filter.PcapFilters;
+
+import java.util.EnumMap;
+import java.util.Map;
+
+public class QueryPcapFilter implements PcapFilter {
+ public static final String QUERY_STR_CONFIG = "mql";
+
+ public static class Configurator implements PcapFilterConfigurator<String> {
+ @Override
+ public void addToConfig(String query, Configuration conf) {
+ conf.set(QUERY_STR_CONFIG, query);
+ conf.set(PCAP_FILTER_NAME_CONF, PcapFilters.QUERY.name());
+ }
+
+ @Override
+ public String queryToString(String fields) {
+ return fields.replaceAll("\\w", "_")
--- End diff --
Will definitely want feedback on the formatting on this one since we're using the string rep for filenames.
---
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: METRON-155 Added query filtering ca...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the pull request:
https://github.com/apache/incubator-metron/pull/119#issuecomment-219048690
For some reason, I cannot comment on the original JIRA, so I will comment here.
I think using a custom created language for this purpose is going to be confusing for our users. Most network tools support what is called Berkely Packet Filter (BPF) syntax. The user community is going to be fairly familiar with this syntax. I would suggest that we use BPF syntax for this purpose.
---
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: METRON-155 Added query filtering ca...
Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/119#discussion_r63111019
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/query/PredicateProcessor.java ---
@@ -18,12 +18,20 @@
package org.apache.metron.common.query;
-import org.antlr.v4.runtime.*;
-import org.apache.metron.common.query.generated.*;
+import org.antlr.v4.runtime.ANTLRInputStream;
+import org.antlr.v4.runtime.CommonTokenStream;
+import org.antlr.v4.runtime.TokenStream;
+import org.apache.metron.common.query.generated.PredicateLexer;
+import org.apache.metron.common.query.generated.PredicateParser;
+
+import static org.apache.commons.lang3.StringUtils.isEmpty;
public class PredicateProcessor {
public boolean parse(String rule, VariableResolver resolver) {
+ if (isEmpty(rule)) {
--- End diff --
On second thought, what about rule == null || isEmpty(rule.trim())? I can't think of a reason to handle null different from "" or " ". The trim would only be evaluated if rule != null, and now we're also covering the null condition. Thoughts?
---
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: METRON-155 Added query filtering ca...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the pull request:
https://github.com/apache/incubator-metron/pull/119#issuecomment-218894825
On the whole, this is great. Definitely a great feature and an impressive 2nd PR. Thanks for the contribution!
Please make sure you didn't inadvertently regress the existing pcap functionality on the `full-dev-vagrant` image by following the testing plan in the original PR to move PCAP to hbase [here](https://github.com/apache/incubator-metron/pull/93)
---
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: METRON-155 Added query filtering ca...
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/119#discussion_r63102275
--- Diff: metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/mr/PcapJob.java ---
@@ -33,36 +33,39 @@
import org.apache.hadoop.mapreduce.lib.input.SequenceFileInputFormat;
import org.apache.hadoop.mapreduce.lib.output.SequenceFileOutputFormat;
import org.apache.log4j.Logger;
-import org.apache.metron.common.Constants;
import org.apache.metron.pcap.PcapHelper;
+import org.apache.metron.pcap.filter.PcapFilter;
+import org.apache.metron.pcap.filter.PcapFilterConfigurator;
+import org.apache.metron.pcap.filter.PcapFilters;
-import javax.annotation.Nullable;
import java.io.IOException;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.*;
+import java.util.stream.Collectors;
public class PcapJob {
private static final Logger LOG = Logger.getLogger(PcapJob.class);
-
public static class PcapMapper extends Mapper<LongWritable, BytesWritable, LongWritable, BytesWritable> {
public static final String START_TS_CONF = "start_ts";
public static final String END_TS_CONF = "end_ts";
PcapFilter filter;
long start;
long end;
+
@Override
protected void setup(Context context) throws IOException, InterruptedException {
super.setup(context);
- filter = new PcapFilter(context.getConfiguration());
+ filter = PcapFilters.valueOf(context.getConfiguration().get(PcapFilterConfigurator.PCAP_FILTER_NAME_CONF)).create();
+ filter.configure(context.getConfiguration());
start = Long.parseUnsignedLong(context.getConfiguration().get(START_TS_CONF));
end = Long.parseUnsignedLong(context.getConfiguration().get(END_TS_CONF));
}
@Override
protected void map(LongWritable key, BytesWritable value, Context context) throws IOException, InterruptedException {
if(Long.compareUnsigned(key.get() ,start) >= 0 && Long.compareUnsigned(key.get(), end) <= 0) {
- boolean send = Iterables.size(Iterables.filter(PcapHelper.toPacketInfo(value.copyBytes()), filter)) > 0;
+ boolean send = PcapHelper.toPacketInfo(value.copyBytes()).stream().filter(filter).collect(Collectors.toList()).size() > 0;
--- End diff --
It's also clearer of intent (not empty as opposed to size > 0)
---
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: METRON-155 Added query filtering ca...
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/119#discussion_r63087348
--- Diff: metron-platform/metron-api/src/main/java/org/apache/metron/pcapservice/PcapReceiverImplRestEasy.java ---
@@ -97,6 +97,66 @@ private static boolean isValidPort(String port) {
}
return false;
}
+ /*
+ * (non-Javadoc)
+ *
+ * @see
+ * com.cisco.opensoc.hbase.client.IPcapReceiver#getPcapsByIdentifiers(java.lang
--- End diff --
Could you change the javadocs here to reflect the method?
---
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: METRON-155 Added query filtering ca...
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/119#discussion_r63099841
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/query/PredicateProcessor.java ---
@@ -18,12 +18,20 @@
package org.apache.metron.common.query;
-import org.antlr.v4.runtime.*;
-import org.apache.metron.common.query.generated.*;
+import org.antlr.v4.runtime.ANTLRInputStream;
+import org.antlr.v4.runtime.CommonTokenStream;
+import org.antlr.v4.runtime.TokenStream;
+import org.apache.metron.common.query.generated.PredicateLexer;
+import org.apache.metron.common.query.generated.PredicateParser;
+
+import static org.apache.commons.lang3.StringUtils.isEmpty;
public class PredicateProcessor {
public boolean parse(String rule, VariableResolver resolver) {
+ if (isEmpty(rule)) {
--- End diff --
what about rule != null && isEmpty(rule.trim())?
---
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: METRON-155 Added query filtering ca...
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/119#discussion_r63111447
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/query/PredicateProcessor.java ---
@@ -18,12 +18,20 @@
package org.apache.metron.common.query;
-import org.antlr.v4.runtime.*;
-import org.apache.metron.common.query.generated.*;
+import org.antlr.v4.runtime.ANTLRInputStream;
+import org.antlr.v4.runtime.CommonTokenStream;
+import org.antlr.v4.runtime.TokenStream;
+import org.apache.metron.common.query.generated.PredicateLexer;
+import org.apache.metron.common.query.generated.PredicateParser;
+
+import static org.apache.commons.lang3.StringUtils.isEmpty;
public class PredicateProcessor {
public boolean parse(String rule, VariableResolver resolver) {
+ if (isEmpty(rule)) {
--- End diff --
That works. || or && are equivalent in this situation
On Thu, May 12, 2016 at 18:56 Michael Miklavcic <no...@github.com>
wrote:
> In
> metron-platform/metron-common/src/main/java/org/apache/metron/common/query/PredicateProcessor.java
> <https://github.com/apache/incubator-metron/pull/119#discussion_r63111019>
> :
>
> >
> >
> > public class PredicateProcessor {
> > public boolean parse(String rule, VariableResolver resolver) {
> > + if (isEmpty(rule)) {
>
> On second thought, what about rule == null || isEmpty(rule.trim())? I
> can't think of a reason to handle null different from "" or " ". The trim
> would only be evaluated if rule != null, and now we're also covering the
> null condition. Thoughts?
>
> \u2014
> You are receiving this because you commented.
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-metron/pull/119/files/b3af68484718a66393f4b0417bc525c462afed5d#r63111019>
>
---
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: METRON-155 Added query filtering ca...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the pull request:
https://github.com/apache/incubator-metron/pull/119#issuecomment-219049457
@nickwallen Agreed we should support BPF. This PR makes the filter pluggable and we already have the query language. We can have a follow-on PR for BPF support IMO.
---
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: METRON-155 Added query filtering ca...
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/119#discussion_r63102232
--- Diff: metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/mr/PcapJob.java ---
@@ -33,36 +33,39 @@
import org.apache.hadoop.mapreduce.lib.input.SequenceFileInputFormat;
import org.apache.hadoop.mapreduce.lib.output.SequenceFileOutputFormat;
import org.apache.log4j.Logger;
-import org.apache.metron.common.Constants;
import org.apache.metron.pcap.PcapHelper;
+import org.apache.metron.pcap.filter.PcapFilter;
+import org.apache.metron.pcap.filter.PcapFilterConfigurator;
+import org.apache.metron.pcap.filter.PcapFilters;
-import javax.annotation.Nullable;
import java.io.IOException;
import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.util.*;
+import java.util.stream.Collectors;
public class PcapJob {
private static final Logger LOG = Logger.getLogger(PcapJob.class);
-
public static class PcapMapper extends Mapper<LongWritable, BytesWritable, LongWritable, BytesWritable> {
public static final String START_TS_CONF = "start_ts";
public static final String END_TS_CONF = "end_ts";
PcapFilter filter;
long start;
long end;
+
@Override
protected void setup(Context context) throws IOException, InterruptedException {
super.setup(context);
- filter = new PcapFilter(context.getConfiguration());
+ filter = PcapFilters.valueOf(context.getConfiguration().get(PcapFilterConfigurator.PCAP_FILTER_NAME_CONF)).create();
+ filter.configure(context.getConfiguration());
start = Long.parseUnsignedLong(context.getConfiguration().get(START_TS_CONF));
end = Long.parseUnsignedLong(context.getConfiguration().get(END_TS_CONF));
}
@Override
protected void map(LongWritable key, BytesWritable value, Context context) throws IOException, InterruptedException {
if(Long.compareUnsigned(key.get() ,start) >= 0 && Long.compareUnsigned(key.get(), end) <= 0) {
- boolean send = Iterables.size(Iterables.filter(PcapHelper.toPacketInfo(value.copyBytes()), filter)) > 0;
+ boolean send = PcapHelper.toPacketInfo(value.copyBytes()).stream().filter(filter).collect(Collectors.toList()).size() > 0;
--- End diff --
That `collect(Collectors.toList())` is going to cause the whole stream to be processed (gotta make that list). I think maybe what we want here is `!Iterables.isEmpty(Iterables.filter(...))` as that will shortcut after the first Iterator.hasNext() returns true.
---
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: METRON-155 Added query filtering ca...
Posted by dlyle65535 <gi...@git.apache.org>.
Github user dlyle65535 commented on the pull request:
https://github.com/apache/incubator-metron/pull/119#issuecomment-220288506
+1 worked in EC2.
---
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: METRON-155 Added query filtering ca...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the pull request:
https://github.com/apache/incubator-metron/pull/119#issuecomment-219059773
I think that makes sense, especially since this creates the 'hook' for later contributions of BPF.
The only downside is that we don't want to confuse users by having 8 ways to do things. But I don't think that will be a problem as long as we eventually get the BPF functionality.
---
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: METRON-155 Added query filtering ca...
Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/119#discussion_r63105055
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/query/PredicateProcessor.java ---
@@ -18,12 +18,20 @@
package org.apache.metron.common.query;
-import org.antlr.v4.runtime.*;
-import org.apache.metron.common.query.generated.*;
+import org.antlr.v4.runtime.ANTLRInputStream;
+import org.antlr.v4.runtime.CommonTokenStream;
+import org.antlr.v4.runtime.TokenStream;
+import org.apache.metron.common.query.generated.PredicateLexer;
+import org.apache.metron.common.query.generated.PredicateParser;
+
+import static org.apache.commons.lang3.StringUtils.isEmpty;
public class PredicateProcessor {
public boolean parse(String rule, VariableResolver resolver) {
+ if (isEmpty(rule)) {
--- End diff --
It appears that Hadoop Configuration resolves empty values to null - this handled both circumstances. I can add this for the PredicateProcessor and then add an additional translation in the client where the val is pulled from the Hadoop conf.
---
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.
---
Re: [GitHub] incubator-metron pull request: METRON-155 Added query
filtering ca...
Posted by Casey Stella <ce...@gmail.com>.
Cool thanks for the quick turnaround!
On Thu, May 12, 2016 at 19:58 mmiklavc <gi...@git.apache.org> wrote:
> Github user mmiklavc commented on the pull request:
>
>
> https://github.com/apache/incubator-metron/pull/119#issuecomment-218918935
>
> Ok, made a number of changes for review. Thanks for the feedback!
> Should have the remaining fixes/improvements avail soon and will also test
> per comments on full-dev-vagrant.
>
>
> ---
> 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: METRON-155 Added query filtering ca...
Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the pull request:
https://github.com/apache/incubator-metron/pull/119#issuecomment-218918935
Ok, made a number of changes for review. Thanks for the feedback! Should have the remaining fixes/improvements avail soon and will also test per comments on full-dev-vagrant.
---
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: METRON-155 Added query filtering ca...
Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the pull request:
https://github.com/apache/incubator-metron/pull/119#issuecomment-219250877
fyi, I was able to confirm no regressions in the existing PcapJob query via the instructions located here - https://github.com/apache/incubator-metron/pull/93
Note: I had to up map/reduce heap to 1GB and container mem to 1.2GB to avoid OOM errors.
---
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: METRON-155 Added query filtering ca...
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/119#discussion_r63099913
--- Diff: metron-platform/metron-common/src/test/java/org/apache/metron/common/query/QueryParserTest.java ---
@@ -67,6 +67,7 @@ public void testSimpleOps() throws Exception {
Assert.assertTrue(run("foo== foo", v -> variableMap.get(v)));
Assert.assertTrue(run("empty== ''", v -> variableMap.get(v)));
Assert.assertTrue(run("spaced == 'metron is great'", v -> variableMap.get(v)));
+ Assert.assertTrue(run("", v -> variableMap.get(v)));
--- End diff --
How about a test case for null too?
---
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: METRON-155 Added query filtering ca...
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/119#discussion_r63115005
--- Diff: metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/filter/query/QueryPcapFilter.java ---
@@ -0,0 +1,74 @@
+/**
+ * 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.pcap.filter.query;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.metron.common.Constants;
+import org.apache.metron.common.query.PredicateProcessor;
+import org.apache.metron.common.query.VariableResolver;
+import org.apache.metron.pcap.PacketInfo;
+import org.apache.metron.pcap.PcapHelper;
+import org.apache.metron.pcap.filter.PcapFilter;
+import org.apache.metron.pcap.filter.PcapFilterConfigurator;
+import org.apache.metron.pcap.filter.PcapFilters;
+
+import java.util.EnumMap;
+import java.util.Map;
+
+public class QueryPcapFilter implements PcapFilter {
+ public static final String QUERY_STR_CONFIG = "mql";
+
+ public static class Configurator implements PcapFilterConfigurator<String> {
+ @Override
+ public void addToConfig(String query, Configuration conf) {
+ conf.set(QUERY_STR_CONFIG, query);
+ conf.set(PCAP_FILTER_NAME_CONF, PcapFilters.QUERY.name());
+ }
+
+ @Override
+ public String queryToString(String fields) {
+ return fields.replaceAll("\\w", "_")
--- End diff --
Actually the safest thing here is to return "". There is already a UUID in
the file name to ensure uniqueness.
On Thu, May 12, 2016 at 19:28 Michael Miklavcic <no...@github.com>
wrote:
> In
> metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/filter/query/QueryPcapFilter.java
> <https://github.com/apache/incubator-metron/pull/119#discussion_r63113925>
> :
>
> > +import java.util.EnumMap;
> > +import java.util.Map;
> > +
> > +public class QueryPcapFilter implements PcapFilter {
> > + public static final String QUERY_STR_CONFIG = "mql";
> > +
> > + public static class Configurator implements PcapFilterConfigurator<String> {
> > + @Override
> > + public void addToConfig(String query, Configuration conf) {
> > + conf.set(QUERY_STR_CONFIG, query);
> > + conf.set(PCAP_FILTER_NAME_CONF, PcapFilters.QUERY.name());
> > + }
> > +
> > + @Override
> > + public String queryToString(String fields) {
> > + return fields.replaceAll("\\w", "_")
>
> Will definitely want feedback on the formatting on this one since we're
> using the string rep for filenames.
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-metron/pull/119/files/b3af68484718a66393f4b0417bc525c462afed5d#r63113925>
>
---
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: METRON-155 Added query filtering ca...
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the pull request:
https://github.com/apache/incubator-metron/pull/119#issuecomment-219425116
+1, got my vote.
---
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: METRON-155 Added query filtering ca...
Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/119#discussion_r63114632
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/query/PredicateProcessor.java ---
@@ -18,12 +18,20 @@
package org.apache.metron.common.query;
-import org.antlr.v4.runtime.*;
-import org.apache.metron.common.query.generated.*;
+import org.antlr.v4.runtime.ANTLRInputStream;
+import org.antlr.v4.runtime.CommonTokenStream;
+import org.antlr.v4.runtime.TokenStream;
+import org.apache.metron.common.query.generated.PredicateLexer;
+import org.apache.metron.common.query.generated.PredicateParser;
+
+import static org.apache.commons.lang3.StringUtils.isEmpty;
public class PredicateProcessor {
public boolean parse(String rule, VariableResolver resolver) {
+ if (isEmpty(rule)) {
--- End diff --
To clarify, I've added the following test cases:
```
Assert.assertTrue(run(null, v -> variableMap.get(v)));
Assert.assertTrue(run("", v -> variableMap.get(v)));
Assert.assertTrue(run(" ", v -> variableMap.get(v)));
```
The null results in an NPE from ANTLR.
Either we:
1. Add an else to return true
2. Do this(my vote): if (rule == null || isEmpty(rule.trim())) { return true; }
3. Or throw a new exception for the parser
---
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: METRON-155 Added query filtering ca...
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/119#discussion_r63105345
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/query/PredicateProcessor.java ---
@@ -18,12 +18,20 @@
package org.apache.metron.common.query;
-import org.antlr.v4.runtime.*;
-import org.apache.metron.common.query.generated.*;
+import org.antlr.v4.runtime.ANTLRInputStream;
+import org.antlr.v4.runtime.CommonTokenStream;
+import org.antlr.v4.runtime.TokenStream;
+import org.apache.metron.common.query.generated.PredicateLexer;
+import org.apache.metron.common.query.generated.PredicateParser;
+
+import static org.apache.commons.lang3.StringUtils.isEmpty;
public class PredicateProcessor {
public boolean parse(String rule, VariableResolver resolver) {
+ if (isEmpty(rule)) {
--- End diff --
Right, I wasn't worried about empty values (your case covered that); I was worried about whitespace-only queries. In that case `PredicateProcessor.parse(" ") != PredicateParser.parse("")`
---
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: METRON-155 Added query filtering ca...
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/119#discussion_r63112571
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/query/PredicateProcessor.java ---
@@ -18,12 +18,20 @@
package org.apache.metron.common.query;
-import org.antlr.v4.runtime.*;
-import org.apache.metron.common.query.generated.*;
+import org.antlr.v4.runtime.ANTLRInputStream;
+import org.antlr.v4.runtime.CommonTokenStream;
+import org.antlr.v4.runtime.TokenStream;
+import org.apache.metron.common.query.generated.PredicateLexer;
+import org.apache.metron.common.query.generated.PredicateParser;
+
+import static org.apache.commons.lang3.StringUtils.isEmpty;
public class PredicateProcessor {
public boolean parse(String rule, VariableResolver resolver) {
+ if (isEmpty(rule)) {
--- End diff --
wouldn't be an NPE on `rule != null && isEmpty(rule.trim())` Java would shortcircuit before the `rule.trim()` call.
---
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: METRON-155 Added query filtering ca...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-metron/pull/119
---
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: METRON-155 Added query filtering ca...
Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on a diff in the pull request:
https://github.com/apache/incubator-metron/pull/119#discussion_r63099540
--- Diff: metron-platform/metron-api/src/main/java/org/apache/metron/pcapservice/PcapReceiverImplRestEasy.java ---
@@ -97,6 +97,66 @@ private static boolean isValidPort(String port) {
}
return false;
}
+ /*
+ * (non-Javadoc)
+ *
+ * @see
+ * com.cisco.opensoc.hbase.client.IPcapReceiver#getPcapsByIdentifiers(java.lang
--- End diff --
Looks like older leftover auto-generated javadoc - I'll revise the packaging and make a proper javadoc for the methods in that 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: METRON-155 Added query filtering ca...
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/119#discussion_r63100294
--- Diff: metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/filter/PcapFilters.java ---
@@ -0,0 +1,41 @@
+/**
+ * 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.pcap.filter;
+
+import org.apache.metron.pcap.filter.fixed.FixedPcapFilter;
+import org.apache.metron.pcap.filter.query.QueryPcapFilter;
+
+import java.util.function.Function;
+
+public enum PcapFilters {
+ FIXED(x -> new FixedPcapFilter()),
+ QUERY(x -> new QueryPcapFilter());
+
+ Function<Void, PcapFilter> filter;
+
+ PcapFilters(Function<Void, PcapFilter> filter) {
--- End diff --
It'd be cool if this was an actual interface like, Creator<T> { T create(); } and it lived in metron-common, as opposed to Function<Void, T>. I bet we have use for this pattern elsewhere.
---
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: METRON-155 Added query filtering ca...
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/119#discussion_r63114735
--- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/query/PredicateProcessor.java ---
@@ -18,12 +18,20 @@
package org.apache.metron.common.query;
-import org.antlr.v4.runtime.*;
-import org.apache.metron.common.query.generated.*;
+import org.antlr.v4.runtime.ANTLRInputStream;
+import org.antlr.v4.runtime.CommonTokenStream;
+import org.antlr.v4.runtime.TokenStream;
+import org.apache.metron.common.query.generated.PredicateLexer;
+import org.apache.metron.common.query.generated.PredicateParser;
+
+import static org.apache.commons.lang3.StringUtils.isEmpty;
public class PredicateProcessor {
public boolean parse(String rule, VariableResolver resolver) {
+ if (isEmpty(rule)) {
--- End diff --
I vote for 2
On Thu, May 12, 2016 at 19:36 Michael Miklavcic <no...@github.com>
wrote:
> In
> metron-platform/metron-common/src/main/java/org/apache/metron/common/query/PredicateProcessor.java
> <https://github.com/apache/incubator-metron/pull/119#discussion_r63114632>
> :
>
> >
> >
> > public class PredicateProcessor {
> > public boolean parse(String rule, VariableResolver resolver) {
> > + if (isEmpty(rule)) {
>
> To clarify, I've added the following test cases:
>
> Assert.assertTrue(run(null, v -> variableMap.get(v)));
> Assert.assertTrue(run("", v -> variableMap.get(v)));
> Assert.assertTrue(run(" ", v -> variableMap.get(v)));
>
> The null results in an NPE from ANTLR.
> Either we:
> 1. Add an else to return true
> 2. Do this(my vote): if (rule == null || isEmpty(rule.trim())) { return
> true; }
> 3. Or throw a new exception for the parser
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-metron/pull/119/files/b3af68484718a66393f4b0417bc525c462afed5d#r63114632>
>
---
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: METRON-155 Added query filtering ca...
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/119#discussion_r63101688
--- Diff: metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/filter/query/QueryPcapFilter.java ---
@@ -0,0 +1,74 @@
+/**
+ * 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.pcap.filter.query;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.metron.common.Constants;
+import org.apache.metron.common.query.PredicateProcessor;
+import org.apache.metron.common.query.VariableResolver;
+import org.apache.metron.pcap.PacketInfo;
+import org.apache.metron.pcap.PcapHelper;
+import org.apache.metron.pcap.filter.PcapFilter;
+import org.apache.metron.pcap.filter.PcapFilterConfigurator;
+import org.apache.metron.pcap.filter.PcapFilters;
+
+import java.util.EnumMap;
+import java.util.Map;
+
+public class QueryPcapFilter implements PcapFilter {
+ public static final String QUERY_STR_CONFIG = "mql";
+
+ public static class Configurator implements PcapFilterConfigurator<String> {
+ @Override
+ public void addToConfig(String query, Configuration conf) {
+ conf.set(QUERY_STR_CONFIG, query);
+ conf.set(PCAP_FILTER_NAME_CONF, PcapFilters.QUERY.name());
+ }
+
+ @Override
+ public String queryToString(String fields) {
+ return fields.replaceAll("\\w", "_")
--- End diff --
I think you mean "\\s" there, not "\\w". \\w matches word, so the output of this will be "___ ___" for input "foo bar". Might be worth while to add a unit test around that too.
---
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.
---