[GitHub] metron issue #1268: METRON-1877: Nested IF ELSE statements can cause parse e...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1268 This looks good to me. Good catch, +1 by inspection. ---
[GitHub] metron issue #1242: METRON-1834: Migrate Elasticsearch from TransportClient ...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1242 Just chiming in here in support of taking some care around javadoc. I tend to agree that it goes stale and it can become misleading. I think the metric (and this is just personal opinion here) should be that any public API should be well javadoc'd and otherwise documented (e.g. the REST API) and we should seriously consider javadoc'ing classes which are used broadly. I tend to prefer a compromise whereby the class itself has documentation around the intent and use of the class in Javadoc and the methods are named well and broken up well. From there, I'm ok if there's less effort put around javadoc of individual methods. Just my $0.02 ---
[GitHub] metron issue #1014: METRON-1563 : Base Stellar assign for feature branch
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1014 Ok, this looks good. +1 to go to the feature branch ---
[GitHub] metron pull request #870: METRON-1364: Add an implementation of Robust PCA o...
Github user cestella closed the pull request at: https://github.com/apache/metron/pull/870 ---
[GitHub] metron issue #1175: METRON-1750 Metron Parser for valid RFC 5424 Syslog mess...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1175 This looks good to me; I'm +1 on it by inspection. Good job, otto ;) ---
[GitHub] metron pull request #1171: METRON-1740 make parser support CONFIG and SYSTEM...
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1171#discussion_r213704549 --- Diff: metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/paloalto/README.md --- @@ -0,0 +1,15 @@ +# BasicPaloAltoFirewallParser --- End diff -- This needs a license header on line 1: ``` ``` ---
[GitHub] metron issue #1178: METRON-1757 Storm Profiler Serialization Exception
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1178 This is really good work, @nickwallen. I'm +1 by inspection here, pending @JonZeolla's +1 ---
[GitHub] metron issue #1176: METRON-1751 Storm Profiler dies when consuming null mess...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1176 +1 by inspection pending full-dev validation. ---
[GitHub] metron issue #1134: METRON-1696: Create the HDFS directory for pcap sequence...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1134 Should this be closed out? ---
[GitHub] metron pull request #1175: METRON-1750 Metron Parser for valid RFC 5424 Sysl...
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1175#discussion_r213670531
--- Diff:
metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/syslog/Syslog5424Parser.java
---
@@ -0,0 +1,75 @@
+/*
+ * 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.parsers.syslog;
+
+import com.github.palindromicity.syslog.NilPolicy;
+import com.github.palindromicity.syslog.SyslogParser;
+import com.github.palindromicity.syslog.SyslogParserBuilder;
+import com.github.palindromicity.syslog.dsl.SyslogFieldKeys;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import org.apache.metron.parsers.BasicParser;
+import org.json.simple.JSONObject;
+
+
+
+/**
+ * Parser for well structured RFC 5424 messages.
+ */
+public class Syslog5424Parser extends BasicParser {
+ public static final String NIL_POLICY_CONFIG = "nilPolicy";
+ private transient SyslogParser syslogParser;
+
+ @Override
+ public void configure(Map config) {
+// Default to OMIT policy for nil fields
+// this means they will not be in the returned field set
+String nilPolicyStr = (String)
config.getOrDefault(NIL_POLICY_CONFIG,NilPolicy.OMIT.name());
+NilPolicy nilPolicy = NilPolicy.valueOf(nilPolicyStr);
+syslogParser = new
SyslogParserBuilder().withNilPolicy(nilPolicy).build();
+ }
+
+ @Override
+ public void init() {
+ }
+
+ @Override
+ @SuppressWarnings("unchecked")
+ public List parse(byte[] rawMessage) {
+try {
+ if (rawMessage == null || rawMessage.length == 0) {
+return null;
+ }
+
+ String originalString = new String(rawMessage);
+ JSONObject jsonObject = new
JSONObject(syslogParser.parseLine(originalString));
+
+ // be sure to put in the original string, and the timestamp.
+ // we wil just copy over the timestamp from the syslog
+ jsonObject.put("original_string", originalString);
+ jsonObject.put("timestamp",
jsonObject.get(SyslogFieldKeys.HEADER_TIMESTAMP.getField()));
--- End diff --
Based on looking at the docs for the syslog library, it looks like it's
possible to not have a timestamp (or to not validly parse a timestamp). If we
have a nil for timestamp here, we probably want to default like we do
elsewhere, which is to current time. What do you think?
---
[GitHub] metron pull request #1175: METRON-1750 Metron Parser for valid RFC 5424 Sysl...
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1175#discussion_r213669448
--- Diff:
metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/syslog/Syslog5424Parser.java
---
@@ -0,0 +1,75 @@
+/*
+ * 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.parsers.syslog;
+
+import com.github.palindromicity.syslog.NilPolicy;
+import com.github.palindromicity.syslog.SyslogParser;
+import com.github.palindromicity.syslog.SyslogParserBuilder;
+import com.github.palindromicity.syslog.dsl.SyslogFieldKeys;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import org.apache.metron.parsers.BasicParser;
+import org.json.simple.JSONObject;
+
+
+
+/**
+ * Parser for well structured RFC 5424 messages.
+ */
+public class Syslog5424Parser extends BasicParser {
+ public static final String NIL_POLICY_CONFIG = "nilPolicy";
+ private transient SyslogParser syslogParser;
+
+ @Override
+ public void configure(Map config) {
+// Default to OMIT policy for nil fields
+// this means they will not be in the returned field set
+String nilPolicyStr = (String)
config.getOrDefault(NIL_POLICY_CONFIG,NilPolicy.OMIT.name());
+NilPolicy nilPolicy = NilPolicy.valueOf(nilPolicyStr);
+syslogParser = new
SyslogParserBuilder().withNilPolicy(nilPolicy).build();
+ }
+
+ @Override
+ public void init() {
+ }
+
+ @Override
+ @SuppressWarnings("unchecked")
+ public List parse(byte[] rawMessage) {
+try {
+ if (rawMessage == null || rawMessage.length == 0) {
+return null;
+ }
+
+ String originalString = new String(rawMessage);
+ JSONObject jsonObject = new
JSONObject(syslogParser.parseLine(originalString));
+
+ // be sure to put in the original string, and the timestamp.
+ // we wil just copy over the timestamp from the syslog
+ jsonObject.put("original_string", originalString);
+ jsonObject.put("timestamp",
jsonObject.get(SyslogFieldKeys.HEADER_TIMESTAMP.getField()));
--- End diff --
If we aren't able to parse the timestamp here, I presume there will be an
exception in the parser, right? I just want to make sure there's no way for
the parser to fail to return a timestamp.
---
[GitHub] metron issue #1091: METRON-1650: Cut size of packaging docker containers
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1091 This gets a +1 from me too, great job! ---
[GitHub] metron issue #1052: METRON-1604 : Add RHEL 7 power pc to OS family for the H...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1052 +1 by inspection ---
[GitHub] metron issue #1131: METRON-1694: Clean up Metron REST docs
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1131 +1 by inspection ---
[GitHub] metron issue #1130: METRON-1693: Fix Pcap CLI local FS finalizer
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1130 +1 by inspection. ---
[GitHub] metron issue #1128: METRON-1690: Add more context to PcapJob JobStatus
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1128 +1 by inspection ---
[GitHub] metron issue #1124: METRON-1560: Update MPack to support Pcap panel
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1124 +1 by inspection; this puts a nice bow on the backend. ---
[GitHub] metron issue #1125: METRON-1661: Create Pcap Query Filter endpoint
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1125 +1 by inspection ---
[GitHub] metron issue #1126: METRON-1687: Upgrade the rat plugin to 0.13-SNAPSHOT
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1126 Ok, I spun this up in full-dev and smoketested the UIs that these licenses touch. ---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1122 Due to a bug in apache rat, our license checking component, it's ignoring all license checks on typescript files. I have submitted a PR to up the rat plugin version and have fixed it in master #1126, let's make sure to add license headers here for new typescript files added in this PR though. ---
[GitHub] metron issue #1118: METRON-1662: Adding download button
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1118 Due to a bug in apache rat, our license checking component, it's ignoring all license checks on typescript files. I have submitted a PR to up the rat plugin version and have fixed it in master #1126, let's make sure to add license headers here for new typescript files added in this PR though. ---
[GitHub] metron issue #1121: METRON-1675: Add Pagination to PCAP
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1121 Due to a bug in apache rat, our license checking component, it's ignoring all license checks on typescript files. I have submitted a PR to up the rat plugin version and have fixed it in master #1126, let's make sure to add license headers here for new typescript files added in this PR though. ---
[GitHub] metron issue #1119: METRON-1676: Adding date range selector to PCAP filter b...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1119 Due to a bug in apache rat, our license checking component, it's ignoring all license checks on typescript files. I have submitted a PR to up the rat plugin version and have fixed it in master #1126, let's make sure to add license headers here for new typescript files added in this PR though. ---
[GitHub] metron pull request #1126: METRON-1687: Upgrade the rat plugin to 0.13-SNAPS...
GitHub user cestella opened a pull request: https://github.com/apache/metron/pull/1126 METRON-1687: Upgrade the rat plugin to 0.13-SNAPSHOT ## Contributor Comments Due to https://issues.apache.org/jira/browse/RAT-234, the rat plugin is not checking the typescript files for license headers. ## Pull Request Checklist Thank you for submitting a contribution to Apache Metron. Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions. Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) for complete smoke testing guides. In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following: ### For all changes: - [x] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel). - [x] Does your PR title start with METRON- where is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [x] Has your PR been rebased against the latest commit within the target branch (typically master)? ### For code changes: - [x] Have you included steps to reproduce the behavior or problem that is being changed or addressed? - [x] Have you included steps or a guide to how the change may be verified and tested manually? - [x] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via: ``` mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh ``` - [x] Have you written or updated unit tests and or integration tests to verify your changes? - [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [x] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent? Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. It is also recommended that [travis-ci](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cestella/incubator-metron upgrade_rat Alternatively you can review and apply these changes as the patch at: https://github.com/apache/metron/pull/1126.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 #1126 commit 6581ead93deb2a5c67f071a81db88512640f6abb Author: cstella Date: 2018-07-20T20:10:21Z METRON-1687: Upgrade the rat plugin to 0.13-SNAPSHOT ---
[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1103#discussion_r204137964
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-list/pcap-list.component.ts ---
@@ -0,0 +1,22 @@
+import { Component, OnInit, Input } from '@angular/core';
--- End diff --
we need a license header here
---
[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1103#discussion_r204138121
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-packet-line/pcap-packet-line.component.spec.ts
---
@@ -0,0 +1,1266 @@
+import { async, ComponentFixture, TestBed } from '@angular/core/testing';
--- End diff --
we need a license header here
---
[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1103#discussion_r204137924
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-list/pcap-list.component.spec.ts
---
@@ -0,0 +1,53 @@
+import { async, ComponentFixture, TestBed } from '@angular/core/testing';
--- End diff --
we need a license header here
---
[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1103#discussion_r204138341
--- Diff:
metron-interface/metron-alerts/src/app/pcap/service/pcap.service.spec.ts ---
@@ -0,0 +1,1735 @@
+import { TestBed, async, inject } from '@angular/core/testing';
--- End diff --
we need a license header here
---
[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1103#discussion_r204138476
--- Diff:
metron-interface/metron-alerts/src/app/pcap/service/pcap.service.ts ---
@@ -0,0 +1,10186 @@
+import {Injectable, NgZone} from '@angular/core';
--- End diff --
we need a license header here
---
[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1103#discussion_r204138196
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-packet/pcap-packet.component.ts
---
@@ -0,0 +1,22 @@
+import { Component, OnInit, Input } from '@angular/core';
--- End diff --
we need a license header here
---
[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1103#discussion_r204137821
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-filters/pcap-filters.component.spec.ts
---
@@ -0,0 +1,29 @@
+import { async, ComponentFixture, TestBed } from '@angular/core/testing';
--- End diff --
Some of these .ts files don't have license headers, how is this passing the
rat check? Do we have a rat exclusion that is too broad?
---
[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1103#discussion_r204138081
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-packet-line/pcap-packet-line.component.ts
---
@@ -0,0 +1,38 @@
+import { Component, OnInit, Input } from '@angular/core';
--- End diff --
we need a license header here
---
[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1103#discussion_r204138244
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.spec.ts
---
@@ -0,0 +1,51 @@
+import { async, ComponentFixture, TestBed } from '@angular/core/testing';
--- End diff --
we need a license header here
---
[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1103#discussion_r204138294
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts
---
@@ -0,0 +1,65 @@
+import { Component, OnInit, Input } from '@angular/core';
--- End diff --
we need a license header here
---
[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1103#discussion_r204137868
--- Diff:
metron-interface/metron-alerts/src/app/pcap/pcap-filters/pcap-filters.component.ts
---
@@ -0,0 +1,24 @@
+import { Component, OnInit, Input, Output, EventEmitter } from
'@angular/core';
--- End diff --
we need a license header here
---
[GitHub] metron issue #1054: METRON-1606 Add capability to wrap json message as entit...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1054 +1 by inspection ---
[GitHub] metron issue #1123: METRON-1685: Retrieve Pcap results in raw binary format
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1123 +1 by inspection, this looks very clean. ---
[GitHub] metron pull request #1120: METRON-1638: Retrieve Pcap results in pdml format
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1120#discussion_r203843748
--- Diff: metron-deployment/packaging/docker/rpm-docker/SPECS/metron.spec
---
@@ -419,6 +419,7 @@ This package installs the Metron Rest %{metron_home}
%dir %{metron_home}/lib
%{metron_home}/config/rest_application.yml
%{metron_home}/bin/metron-rest.sh
+%{metron_home}/bin/pcap_to_pdml.sh
--- End diff --
Is there anything to be done for the ubuntu debs?
---
[GitHub] metron issue #1120: METRON-1638: Retrieve Pcap results in pdml format
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1120 +1 by inspection, this looks good. ---
[GitHub] metron pull request #1120: METRON-1638: Retrieve Pcap results in pdml format
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1120#discussion_r203803381 --- Diff: metron-interface/pom.xml --- @@ -25,6 +25,9 @@ Interfaces for Metron https://metron.apache.org/ + +2.9.5 --- End diff -- Ok, do me a favor, when you test this out in full-dev, make sure that you can continue to use the REST endponts backing the management UI. We often have issues with classpath issues around newer versions of Jackson. Should we exclude the other jackson dependencies coming in transitively in metron-rest and metron-rest-client? ---
[GitHub] metron pull request #1120: METRON-1638: Retrieve Pcap results in pdml format
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1120#discussion_r203801015 --- Diff: metron-interface/metron-rest/src/main/scripts/pcap_to_pdml.sh --- @@ -0,0 +1,19 @@ +#!/bin/bash +# +# 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. +# +tshark -i - -T pdml --- End diff -- Can we make sure to add to the docs as part of this PR that we now rely on tshark being installed? ---
[GitHub] metron pull request #1120: METRON-1638: Retrieve Pcap results in pdml format
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1120#discussion_r203800738 --- Diff: metron-interface/pom.xml --- @@ -25,6 +25,9 @@ Interfaces for Metron https://metron.apache.org/ + +2.9.5 --- End diff -- What's this used for? ---
[GitHub] metron issue #1100: METRON-1659: The platform-info.sh should check for the v...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1100 Hey @JonZeolla can you close this one manually, for some reason the automatic close didn't work when I committed this. ---
[GitHub] metron issue #1106: METRON-1672: Add metron-alerts's UI unit tests to travis...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1106 Yeah, that makes sense. +1 by inspection, let's go to master. ---
[GitHub] metron pull request #1109: METRON-1674: Create REST endpoint for job status ...
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1109#discussion_r203502396
--- Diff:
metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/finalizer/PcapRestFinalizer.java
---
@@ -0,0 +1,46 @@
+/**
+ * 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.finalizer;
+
+import java.nio.file.Paths;
+import java.util.Map;
+import org.apache.hadoop.fs.Path;
+import org.apache.metron.job.Statusable;
+import org.apache.metron.pcap.config.PcapOptions;
+
+/**
+ * Write to HDFS.
+ */
+public class PcapRestFinalizer extends PcapFinalizer {
+
+ private String user;
+ private String jobType = Statusable.JobType.MAP_REDUCE.name();
+
+ public void setUser(String user) {
+this.user = user;
+ }
+
+ @Override
+ protected Path getOutputPath(Map config, int partition) {
+String jobId = PcapOptions.JOB_ID.get(config, String.class);
+String finalOutputPath = PcapOptions.FINAL_OUTPUT_PATH.get(config,
String.class);
--- End diff --
Why isn't the user passed into the config rather than being in a member
variable? This seems inconsistent. I'd expect a
PcapOptions.USERNAME.get(config, String.class) rather than a member variable.
---
[GitHub] metron issue #1110: METRON-1684: Fix Markdown problems in 3rdPartyParser.md
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1110 +1 by inspection ---
[GitHub] metron pull request #1108: METRON-1614: Create job status abstraction
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1108#discussion_r203383057
--- Diff:
metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/finalizer/PcapCliFinalizer.java
---
@@ -0,0 +1,37 @@
+/**
+ * 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.finalizer;
+
+import java.util.Map;
+import org.apache.hadoop.fs.Path;
+import org.apache.metron.pcap.config.PcapOptions;
+
+/**
+ * Write to local FS.
+ */
+public class PcapCliFinalizer extends PcapFinalizer {
+
+ @Override
+ protected String getOutputFileName(Map config, int
partition) {
+Path finalOutputPath = PcapOptions.FINAL_OUTPUT_PATH.get(config,
PcapOptions.STRING_TO_PATH, Path.class);
+String prefix = PcapOptions.FINAL_FILENAME_PREFIX.get(config,
String.class);
+return String.format("%s/pcap-data-%s+%04d.pcap", finalOutputPath,
prefix, partition);
--- End diff --
This constant string should be be a `static final String` with a comment
about the file format in English.
---
[GitHub] metron pull request #1108: METRON-1614: Create job status abstraction
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1108#discussion_r203383137
--- Diff:
metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/finalizer/PcapRestFinalizer.java
---
@@ -16,27 +16,21 @@
* limitations under the License.
*/
-package org.apache.metron.pcap;
+package org.apache.metron.pcap.finalizer;
-import java.util.List;
+import java.util.Map;
import org.apache.hadoop.fs.Path;
-import org.apache.metron.job.Pageable;
+import org.apache.metron.pcap.config.PcapOptions;
-public class PcapFiles implements Pageable {
-
- private List files;
-
- public PcapFiles(List files) {
-this.files = files;
- }
+/**
+ * Write to HDFS.
+ */
+public class PcapRestFinalizer extends PcapFinalizer {
@Override
- public Iterable asIterable() {
-return files;
+ protected String getOutputFileName(Map config, int
partition) {
+Path finalOutputPath =
PcapOptions.FINAL_OUTPUT_PATH.getTransformed(config, Path.class);
+return String.format("%s/page-%s", finalOutputPath, partition);
--- End diff --
This constant string should be be a `static final String` with a comment
about the file format in English.
---
[GitHub] metron pull request #1108: METRON-1614: Create job status abstraction
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1108#discussion_r203381697
--- Diff:
metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/finalizer/PcapFinalizerStrategies.java
---
@@ -0,0 +1,42 @@
+/**
+ * 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.finalizer;
+
+import java.util.Map;
+import org.apache.hadoop.fs.Path;
+import org.apache.metron.job.Finalizer;
+import org.apache.metron.job.JobException;
+import org.apache.metron.job.Pageable;
+
+public enum PcapFinalizerStrategies implements Finalizer {
--- End diff --
It'd be good to explain exactly what this does. I'm not super worried
about javadocs for the method, but general discussion of the purpose of this
would be helpful.
---
[GitHub] metron pull request #1108: METRON-1614: Create job status abstraction
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1108#discussion_r203382448 --- Diff: metron-platform/metron-pcap/src/main/java/org/apache/metron/pcap/finalizer/PcapFinalizer.java --- @@ -0,0 +1,122 @@ +/** + * 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.finalizer; + +import com.google.common.collect.Iterables; +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.LocatedFileStatus; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.RemoteIterator; +import org.apache.metron.common.hadoop.SequenceFileIterable; +import org.apache.metron.job.Finalizer; +import org.apache.metron.job.JobException; +import org.apache.metron.job.Pageable; +import org.apache.metron.pcap.PcapPages; +import org.apache.metron.pcap.config.PcapOptions; +import org.apache.metron.pcap.writer.PcapResultsWriter; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + --- End diff -- It'd be good to explain exactly what this does. I'm not super worried about javadocs for the method, but general discussion of the purpose of this would be helpful. ---
[GitHub] metron issue #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1099 Ok, so I want to capture the follow-on tasks: * Change Ambari to accept quoted parser groups * Decouple the ParserBolt from the Parse execution logic * Allow the option for intermediate kafka topics to be removed Did I miss anything? ---
[GitHub] metron pull request #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1099#discussion_r203115089 --- Diff: use-cases/parser_chaining/README.md --- @@ -233,3 +233,10 @@ cat ~/data.log | /usr/hdp/current/kafka-broker/bin/kafka-console-producer.sh --b ``` You should see indices created for the `cisco-5-304` and `cisco-6-302` data with appropriate fields created for each type. + +# Aggregated Parsers with Parser Chaining +Chained parsers can be run as aggregated parsers. These parsers continue to use the sensor specific Kafka topics, and do not do internal routing to the appropriate sensor. + --- End diff -- I think we should adjust, as a follow-on, ambari to accept proper CSV for parsers. So, if you want groups you just quote the groups. E.g. let's say you input the following: `bro,"snort,yaf"` That would create the following topologies: * `bro` * `snort` and `yaf` aggregated ---
[GitHub] metron pull request #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1099#discussion_r203052754 --- Diff: use-cases/parser_chaining/aggregated_parser_chaining_flow.xml --- @@ -0,0 +1 @@ +7Vtbb6M6EP41eewKMBDy2Nt2pXOr1COd3acVAYdYJRiB06b7648dbMAXEppCk27al+KxMXi+b8YzYzIB16vNXRHmy79wDNOJY8WbCbiZOI5tzyz6j0leKokf2JUgKVDMBzWCB/QLciG/L1mjGJbSQIJxSlAuCyOcZTAikiwsCvwsD1vgVH5qHiZQEzxEYapL/0MxWVbSwJk28m8QJUvxZMviLz4Po8ekwOuMP2/igMX2r+pehWIuPr5chjF+bonA7QRcFxiT6mq1uYYp061QW3Xf147e+r0LmJE+N4DqhqcwXfOl/42+Iv5u5EXo43mJCHzIw4i1nynmE3AVpijJaDOij4IFFTzBgiCqw0vesUJxzCa4WuCMPPDJLNouSYEfa8UCNgKl6TVOMZ3mJsMZu0lfCV8cewzctER8ZXcQryApXugQ3gu86g5OQsB1/twgKni6bIHpc1nIOZTU8zZ6pBdclR04uJpec7T5Wb6UKU5+Un4wjTk+ZYfNFPJAcLFiGs9pT0t+CjiU9GkoS6jAHggV1/2yFxgbTEdCRliqGZoSZiVm0KT0mVdzdpWwqz/CxWO49UI5ijRYqBXn7DJ6SVEWb0EwITVnfgHGf85Trs3GW/yzJvRWyOVpOIfpVd2pIGIAaUlWYs4KVNFPmWRt/4xwd0BnALgTTdv3JSxtR8fSNljZzB8CS93MNGxgTD07b3JtZfi2JXyt0uAGke/8Pnb9g1kTZTRt ZXQB37lxbRtNn6DIIoWbS7ZD1epnL7hb+XQ9eF1E9R7J1UnCIoFinGtGqQ2DZYChFhYwDQl6kt/EBA5/xj1G9B0boxaelNNgqphqtQB+U3s/Uudx5Xl8IM9TrVmbZ8uUetk9yeMfgTzMBRtHDkYPYCCHZU2Oyg7bkR2+6sj7sqMOm5SNYwR2CD+2ix0mH9/yxGGZV7HpAm1gvNet99qNu/gzgCevbU14cuHZWywBJpbUwjf58jqQ2KFwk5XALOaXN1EaliXdniUU2kpsovFdVtul48a9N97+h/D2fFPgXXxbsI2wHuj2XYNhVyzdadieATFvIKevWKNqjn3NOpDnqdsjmLUh4RHBHg10MxHuRaiM8IV3AViEUXXTqdsjPkPE/iGiZ0v4eo4h3h8tRnRMMWIX4j5F3PlE/K2IewFQUrx3xtzTMJct+qMm3m+zQzCTo7DgPfNuRw+3ZZs7S0y8qXVMTKa/Q8ylJ+KGpL2Vs+GCLHGCszBtZXKHRmVeYAjLnP3Z+Ihhmafk0K4an/eNy7SJxgzMgpOnogjxB6HbLmIPSkVwTCrW84gUYabUc3pn/kqOUMcTY1BxdipU7F8/Oiw3PZBoxhpTFXOdSCoa2MOkolN1mx2QZqBHhem8aWaqeFRh5NH82UwpZDpKutLbn6kTzUYkmh7lwaxA0XJF1V9+prW9g3UgdljhZUzB+mhJLdC3JUPRooHxjmqgWl7JDpstmk3l24RKGachTBVEZD0XsES/wnmdK+WMltu1eFcT74blXWuCy+ozDruVhqVwQQxJGGVWk0z9yxo3F+4OtzMAdq6SaE0DT8fOMWCnFjUP+xxAP3SWChLWCpZlmECDNZ4HPJ7yRYBnsCzXgI4aBh6Gjn70IFnW2aPjK2HRO8OjV/Muk6SASUhgTOWiYHQvPN0cp6QHMPLu0b3h9K8pmTbCotoIOfh7drvxIFT3LjvQT/eMe9cwEHYV/6Qjls69a6/V/WYnsVMZK+AGG laBASq1PHMYVHq0+Jow48ygcv1jQnUyNbNXZJA9vp6Sc0t7zPKtOCmSv6U6ao4J7JlyeOb7B2aZwNemspWpBswz3Y9YN+ss6h6Hjr6JjtNj0tENbJVDakjQ+zwBqFN56neCA9LR6/Fx6MnRsSGdLRfYvlh1+x4WiCqChaJ7zsFG46lr4ik4Kk+VCrBteQeyNFAnUo/PhuSontp8JI4Gr+aowc+OxtHpJ0cH4ejbcrczrzuqJ6DvW3f09AzB9Dukz5SuXSnxHQkxxzFUSixHR6wWvgIy2mx+7FdZZ/OLSnD7Pw== --- End diff -- You need to add the license header here. ---
[GitHub] metron pull request #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1099#discussion_r203052624 --- Diff: use-cases/parser_chaining/aggregated_parser_chaining_flow.svg --- @@ -0,0 +1,2 @@ +http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd;> --- End diff -- You need to add the license header here. ---
[GitHub] metron issue #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1099 @ottobackwards I agree, our bolts tend to do the same magic set of incantations to set up stellar. It'd be better to either try to infer that initialization where possible or to externalize that logic. Agreed that it's not this PR that should do that, but I would expect that we should probably consider a separate JIRA to the effect: * Decouple ParserBolt the parserbolt initialization and execution logic into a separate component from the Storm Bolt infrastructure (this will aid in NiFi or spark integration). Thoughts? If we agree, then I'll create the JIRA. ---
[GitHub] metron pull request #1108: METRON-1614: Create job status abstraction
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1108#discussion_r203049283
--- Diff:
metron-platform/metron-common/src/main/java/org/apache/metron/common/utils/HDFSUtils.java
---
@@ -29,6 +29,16 @@
public class HDFSUtils {
+ public static byte[] readBytes(String path) throws IOException {
+return readBytes(new Path(path));
+ }
+
+ public static byte[] readBytes(Path inPath) throws IOException {
+FileSystem fs = FileSystem.get(inPath.toUri(), new Configuration());
+FSDataInputStream inputStream = fs.open(inPath);
--- End diff --
I made this comment on @merrimanr 's PR, but it looks like it originated
here. This needs to be in a try-with-resources block otherwise we're leaking
InputStreams.
---
[GitHub] metron pull request #1109: METRON-1674: Create REST endpoint for job status ...
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1109#discussion_r203047956
--- Diff:
metron-platform/metron-common/src/main/java/org/apache/metron/common/utils/HDFSUtils.java
---
@@ -29,6 +29,16 @@
public class HDFSUtils {
+ public static byte[] readBytes(String path) throws IOException {
+return readBytes(new Path(path));
+ }
+
+ public static byte[] readBytes(Path inPath) throws IOException {
+FileSystem fs = FileSystem.get(inPath.toUri(), new Configuration());
+FSDataInputStream inputStream = fs.open(inPath);
--- End diff --
This should be in a try with resources block otherwise you're leaving
streams open.
---
[GitHub] metron pull request #1109: METRON-1674: Create REST endpoint for job status ...
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1109#discussion_r203047410
--- Diff:
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/config/PcapJobSupplier.java
---
@@ -0,0 +1,53 @@
+/**
+ * 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.rest.config;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.metron.job.JobException;
+import org.apache.metron.job.JobStatus;
+import org.apache.metron.job.Statusable;
+import org.apache.metron.pcap.finalizer.PcapFinalizerStrategies;
+import org.apache.metron.pcap.mr.PcapJob;
+import org.apache.metron.rest.model.pcap.PcapRequest;
+
+import java.util.function.Supplier;
+
+public class PcapJobSupplier implements Supplier> {
+
+ private PcapRequest pcapRequest;
+
+ @Override
+ public Statusable get() {
+try {
+ PcapJob pcapJob = createPcapJob();
+ return pcapJob.submit(PcapFinalizerStrategies.REST, pcapRequest);
+} catch (JobException e) {
+ return null;
+ //return new
JobStatus().withState(JobStatus.State.FAILED).withDescription(JobStatus.State.FAILED.toString());
--- End diff --
Is this intended? Seems like we should be passing back info about the
exception or at the very least throwing a runtime exception, right?
---
[GitHub] metron pull request #1109: METRON-1674: Create REST endpoint for job status ...
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1109#discussion_r203046824
--- Diff:
metron-interface/metron-rest-client/src/main/java/org/apache/metron/rest/model/pcap/PcapRequest.java
---
@@ -17,75 +17,65 @@
*/
package org.apache.metron.rest.model.pcap;
-public class PcapRequest {
+import org.apache.commons.collections4.map.AbstractMapDecorator;
+import org.apache.metron.pcap.config.PcapOptions;
- private String baseOutputPath;
- private String basePath;
- private Long startTime = 0L;
- private Long endTime = System.currentTimeMillis();
- private Integer numReducers = 1;
+import java.util.HashMap;
- public String getBaseOutputPath() {
-return baseOutputPath;
- }
+public class PcapRequest extends AbstractMapDecorator {
- public void setBaseOutputPath(String baseOutputPath) {
-this.baseOutputPath = baseOutputPath;
+ public PcapRequest() {
+super(new HashMap<>());
+setStartTimeMs(0L);
+setEndTimeMs(System.currentTimeMillis());
+setNumReducers(1);
--- End diff --
It might be worthwhile to pick the number of reducers > 1 just because this
is going to be slow by default. Maybe 10 would be a sensible middle ground?
---
[GitHub] metron pull request #1108: METRON-1614: Create job status abstraction
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1108#discussion_r203045196
--- Diff:
metron-platform/metron-job/src/main/java/org/apache/metron/job/Statusable.java
---
@@ -18,20 +18,45 @@
package org.apache.metron.job;
-import java.io.IOException;
import java.util.Map;
/**
* Abstraction for getting status on running jobs. Also provides options
for killing and validating.
*/
-public interface Statusable {
+public interface Statusable {
+
+ enum JobType {
+MAP_REDUCE,
+SPARK;
--- End diff --
No, just `SPARK`. The enum is spot on.
---
[GitHub] metron issue #1108: METRON-1614: Create job status abstraction
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1108 Just pointing out, I will code review but not vote on this because I contributed to this PR in an unattributed capacity. ---
[GitHub] metron pull request #1108: METRON-1614: Create job status abstraction
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1108#discussion_r203035286
--- Diff:
metron-platform/metron-job/src/main/java/org/apache/metron/job/Statusable.java
---
@@ -18,20 +18,45 @@
package org.apache.metron.job;
-import java.io.IOException;
import java.util.Map;
/**
* Abstraction for getting status on running jobs. Also provides options
for killing and validating.
*/
-public interface Statusable {
+public interface Statusable {
+
+ enum JobType {
+MAP_REDUCE,
+SPARK;
--- End diff --
Can we remove this until we have an actual spark job?
---
[GitHub] metron pull request #1108: METRON-1614: Create job status abstraction
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1108#discussion_r203035500 --- Diff: metron-platform/metron-job/src/test/java/org/apache/metron/job/manager/InMemoryJobManagerTest.java --- @@ -0,0 +1,192 @@ +package org.apache.metron.job.manager; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Supplier; +import org.apache.hadoop.fs.Path; +import org.apache.metron.job.Finalizer; +import org.apache.metron.job.JobException; +import org.apache.metron.job.JobStatus; +import org.apache.metron.job.JobStatus.State; +import org.apache.metron.job.Pageable; +import org.apache.metron.job.Statusable; +import org.apache.metron.job.Statusable.JobType; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +/** --- End diff -- Can we put the license header at the top of the file? ---
[GitHub] metron pull request #1108: METRON-1614: Create job status abstraction
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1108#discussion_r203037439
--- Diff:
metron-interface/metron-rest-client/src/main/java/org/apache/metron/rest/model/pcap/PcapRequest.java
---
@@ -17,52 +17,57 @@
*/
package org.apache.metron.rest.model.pcap;
-public class PcapRequest {
+// TODO reconcile with pcapmrjob
--- End diff --
Is this reconciliation part of this PR or part of the follow-on PR?
---
[GitHub] metron pull request #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1099#discussion_r202817242 --- Diff: metron-platform/metron-parsers/README.md --- @@ -82,6 +82,12 @@ topology in kafka. Errors are collected with the context of the error (e.g. stacktrace) and original message causing the error and sent to an `error` queue. Invalid messages as determined by global validation functions are also treated as errors and sent to an `error` queue. + +Multiple sensors can be aggregated into a single Storm topology. When this is done, there will be +multiple Kafka spouts, but only a single parser bolt which will handle delegating to the correct --- End diff -- @justinleet can you maybe create a data flow diagram or sequence diagram that shows a syslog record from the use-case flowing through this topology and add it to the use-case around parser chaining? It'd be something like, given a `cisco-6-302` record, it'll go: * From NiFi to the `pix_syslog_router` kafka topic * From the `pix_syslog_router` kafka topic to the `pix_syslog_router` spout in the aggregated storm topology * From the `pix_syslog_router` kafka spout to the parser bolt, which will run the `pix_syslog_router` Grok parser and write out to the `cisco-6-302` kafka topic * From the `cisco-6-302` kafka topic to the `cisco-6-302` spout in the aggregated storm topology * From the `cisco-6-302` kafka spout to the `cisco-6-302` Grok parser and write out to the `enrichments` kafka topic, where it's picked up by the enrichment topology. Eventually, we should consider taking out the writing to the `cisco-6-302` topic (optionally), but even eventually there may be value in those intermediate kafka topics due to how users may want to group sensors (e.g. grouping may be done via velocity or scalability requirements, rather than logical connection). ---
[GitHub] metron pull request #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1099#discussion_r202814185 --- Diff: metron-platform/metron-parsers/README.md --- @@ -82,6 +82,12 @@ topology in kafka. Errors are collected with the context of the error (e.g. stacktrace) and original message causing the error and sent to an `error` queue. Invalid messages as determined by global validation functions are also treated as errors and sent to an `error` queue. + +Multiple sensors can be aggregated into a single Storm topology. When this is done, there will be +multiple Kafka spouts, but only a single parser bolt which will handle delegating to the correct --- End diff -- You also might want 2 different parsers that source from the same kafka topic (think: 1 parser to send the data to enrichment and 1 parser to send to hbase as a streaming enrichment for authentication data) ---
[GitHub] metron pull request #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1099#discussion_r202813546 --- Diff: metron-platform/metron-parsers/README.md --- @@ -82,6 +82,12 @@ topology in kafka. Errors are collected with the context of the error (e.g. stacktrace) and original message causing the error and sent to an `error` queue. Invalid messages as determined by global validation functions are also treated as errors and sent to an `error` queue. + +Multiple sensors can be aggregated into a single Storm topology. When this is done, there will be +multiple Kafka spouts, but only a single parser bolt which will handle delegating to the correct --- End diff -- Well, parser chaining allows for DAGs of parsers, not just one level. Also, you might not want to group parsers based on chained units, but rather based on velocity or some other metric (e.g. I don't want to group a high velocity sensor with a bunch of low velcoity sensors in the syslog case). In that case, you would need the intermediate kafka topics. ---
[GitHub] metron pull request #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1099#discussion_r202805609 --- Diff: metron-platform/metron-parsers/README.md --- @@ -82,6 +82,12 @@ topology in kafka. Errors are collected with the context of the error (e.g. stacktrace) and original message causing the error and sent to an `error` queue. Invalid messages as determined by global validation functions are also treated as errors and sent to an `error` queue. + +Multiple sensors can be aggregated into a single Storm topology. When this is done, there will be +multiple Kafka spouts, but only a single parser bolt which will handle delegating to the correct --- End diff -- This PR gives us the ability to group the parsers into a single topology if we so desire. You would still write through to kafka. So, the topology in the example would have 3 kafka spouts: * One for monitoring `pix_syslog_router` (the syslog parser aka the routing parser) * One for monitoring `cisco-5-304` * One for monitoring `cisco-6-302` There would be one parser bolt, though, which would handle parsing all 3 sensor types. That is the contribution of this PR, the ability to determine the parser and filter and field transformations from the input kafka topic and use the appropriate one to parse the messages. There is not, however, any code here that would bypass the intermediate kafka write (e.g. from the router topology to the individual `cisco-5-304` or `cisco-6-302` topics). That's a current gap. ---
[GitHub] metron pull request #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1099#discussion_r202803869
--- Diff:
metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/bolt/ParserBolt.java
---
@@ -182,40 +185,61 @@ public void prepare(Map stormConf, TopologyContext
context, OutputCollector coll
super.prepare(stormConf, context, collector);
messageGetStrategy = MessageGetters.DEFAULT_BYTES_FROM_POSITION.get();
this.collector = collector;
-if(getSensorParserConfig() != null) {
- cache =
CachingStellarProcessor.createCache(getSensorParserConfig().getCacheConfig());
-}
-initializeStellar();
-if(getSensorParserConfig() != null && filter == null) {
-
getSensorParserConfig().getParserConfig().putIfAbsent("stellarContext",
stellarContext);
- if
(!StringUtils.isEmpty(getSensorParserConfig().getFilterClassName())) {
-filter = Filters.get(getSensorParserConfig().getFilterClassName()
-, getSensorParserConfig().getParserConfig()
-);
+
+// Build the Stellar cache
+Map cacheConfig = new HashMap<>();
+for (Map.Entry entry:
sensorToComponentMap.entrySet()) {
+ String sensor = entry.getKey();
+ SensorParserConfig config = getSensorParserConfig(sensor);
+
+ if (config != null) {
+cacheConfig.putAll(config.getCacheConfig());
}
}
+cache = CachingStellarProcessor.createCache(cacheConfig);
-parser.init();
+// Need to prep all sensors
+for (Map.Entry entry:
sensorToComponentMap.entrySet()) {
+ String sensor = entry.getKey();
+ MessageParser parser =
entry.getValue().getMessageParser();
--- End diff --
So, the consequences of this decision are as follows:
* You share an expression cache (i.e. the statement -> abstract syntax tree
cache; distinct from the expression -> evaluated return cache)
* You share an stellar value cache (expression -> evaluated return)
* You share the state in the Context (e.g. hbase connections, zookeeper
connections).
On the whole, anything shared in the context is intended to be shared
across users and sensors by virtue of Stellar being used in the enrichment
topology (where it's not sensor-by-sensor), so we shoudl be ok there. The real
question is whether users would prefer to have one knob per topology for
stellar cache sizing or whether they would prefer to have one knob per sensor.
I'd say that I'm ok with how this PR is doing it, because it's easier to reason
about resources, IMO, on a per-topology perspective.
---
[GitHub] metron pull request #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on a diff in the pull request: https://github.com/apache/metron/pull/1099#discussion_r202801374 --- Diff: metron-platform/metron-parsers/README.md --- @@ -82,6 +82,12 @@ topology in kafka. Errors are collected with the context of the error (e.g. stacktrace) and original message causing the error and sent to an `error` queue. Invalid messages as determined by global validation functions are also treated as errors and sent to an `error` queue. + +Multiple sensors can be aggregated into a single Storm topology. When this is done, there will be +multiple Kafka spouts, but only a single parser bolt which will handle delegating to the correct --- End diff -- @ottobackwards I think that's essentially the parser chaining stuff I added earlier, am I misunderstanding? The [use-case](https://github.com/apache/metron/tree/master/use-cases/parser_chaining) is using the "we get a ton of types of data in syslog" example. ---
[GitHub] metron issue #1107: METRON-1673: Fix Javadoc errors
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1107 +1 by inspection, thanks! ---
[GitHub] metron issue #1104: METRON-1670 Stellar WEEK_OF_YEAR test is locale sensitiv...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1104 +1 Great catch! ---
[GitHub] metron issue #1105: METRON-1236 Add start/stop/restart commands that execute...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1105 +1 by inspection; thanks! ---
[GitHub] metron issue #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1099 This looks good to me, I'm +1 by inspection, but I want to make sure enough time has passed so enough people can look at it. I'll hold my +1 until EOD monday. ---
[GitHub] metron issue #870: METRON-1364: Add an implementation of Robust PCA outlier ...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/870 Thanks @JonZeolla I am planning to try to get around to cleaning this up a bit this week. I have some concerns about its interactions with the sampling stellar functions. ---
[GitHub] metron issue #1097: METRON-1656 Create KAKFA_SEEK function
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1097 +1 by inspection ---
[GitHub] metron issue #1081: METRON-1641: Enable Pcap jobs to be submitted asynchrono...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1081 @mmiklavc can you merge and close this PR? ---
[GitHub] metron issue #1065: METRON-1620: Fixes for forensic clustering use case exam...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1065 @mmiklavc Can we get this merged? ---
[GitHub] metron issue #1102: METRON-1660: On Solr, sorting by threat score fails
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1102 +1 by inspection ---
[GitHub] metron issue #1101: METRON-1658: Upgrade bro to 2.5.4
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1101 +1 by inspection ---
[GitHub] metron pull request #1084: METRON-1644: Support parser chaining
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1084#discussion_r201448152
--- Diff:
metron-platform/metron-common/src/main/java/org/apache/metron/common/message/metadata/EnvelopedRawMessageStrategy.java
---
@@ -0,0 +1,146 @@
+/**
+ * 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.message.metadata;
+
+import org.apache.metron.common.Constants;
+import org.apache.metron.common.utils.JSONUtils;
+import org.json.simple.JSONObject;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * An alternative strategy whereby
+ *
+ * The raw data is presumed to be a JSON Map
+ * The data to be parsed is the contents of one of the fields.
+ * The non-data fields are considered metadata
+ *
+ *
+ * Additionally, the defaults around merging and reading metadata are
adjusted to be on by default.
+ * Note, this strategy allows for parser chaining and for a fully worked
example, check the parser chaining use-case.
+ */
+public class EnvelopedRawMessageStrategy implements RawMessageStrategy {
+ private static final Logger LOG =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ /**
+ * The field from the rawMessageStrategyConfig in the SensorParserConfig
that defines the field to use to
+ * define the data to be parsed.
+ */
+ public static final String MESSAGE_FIELD_CONFIG = "messageField";
+
+ /**
+ * Retrieve the raw message by parsing the JSON Map in the kafka value
and pulling the appropriate field.
+ * Also, augment the default metadata with the non-data fields in the
JSON Map.
+ *
+ * Note: The data field in the JSON Map is not considered metadata.
+ *
+ * @param rawMetadata The metadata read from kafka Key (e.g. the topic,
index, etc.)
+ * @param rawMessage The raw message from the kafka value
+ * @param readMetadata True if we want to read read the metadata
+ * @param config The config for the RawMessageStrategy (See the
rawMessageStrategyConfig in the SensorParserConfig)
+ * @return
+ */
+ @Override
+ public RawMessage get(Map rawMetadata, byte[]
rawMessage, boolean readMetadata, Map config) {
+String messageField = (String)config.get(MESSAGE_FIELD_CONFIG);
+if(messageField == null) {
+ throw new IllegalStateException("You must specify a message field in
the message supplier config. " +
+ "I expected to find a \"messageField\" field in the
config.");
--- End diff --
haha
---
[GitHub] metron issue #1094: METRON-1562: Enable Kerberos in REST for YARN and MR job...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1094 +1 by inspection ---
[GitHub] metron pull request #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1099#discussion_r201361910
--- Diff:
metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/topology/ParserTopologyBuilder.java
---
@@ -91,14 +101,14 @@ public Config getTopologyConfig() {
*/
public static ParserTopology build(String zookeeperUrl,
Optional brokerUrl,
- String sensorType,
- ValueSupplier
spoutParallelismSupplier,
- ValueSupplier
spoutNumTasksSupplier,
+ List sensorTypes,
+ ValueSupplier
spoutParallelismSupplier,
--- End diff --
Ah, I see. man, I miss C++ templates sometimes. I retract the nit; don't
worry about it.
---
[GitHub] metron pull request #1099: METRON-1657: Parser aggregation in storm
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1099#discussion_r201349285
--- Diff:
metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/topology/ParserTopologyBuilder.java
---
@@ -91,14 +101,14 @@ public Config getTopologyConfig() {
*/
public static ParserTopology build(String zookeeperUrl,
Optional brokerUrl,
- String sensorType,
- ValueSupplier
spoutParallelismSupplier,
- ValueSupplier
spoutNumTasksSupplier,
+ List sensorTypes,
+ ValueSupplier
spoutParallelismSupplier,
--- End diff --
This is a nit, but is there any reason why we couldn't make these
`ValueSupplier>` and `ValueSupplier>`?
---
[GitHub] metron issue #1084: METRON-1644: Support parser chaining
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1084 Intermittent test failure, travis sorted now. ---
[GitHub] metron issue #1081: METRON-1641: Enable Pcap jobs to be submitted asynchrono...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1081 +1, lgtm ---
[GitHub] metron pull request #1084: METRON-1644: Support parser chaining
GitHub user cestella reopened a pull request: https://github.com/apache/metron/pull/1084 METRON-1644: Support parser chaining ## Contributor Comments Currently we have only one layer of parsing prior to enrichment, but often real data is more complex. For instance, often data is wrapped in an envelope (e.g. syslog data which contains a field which needs to be parsed). This Jira allows us to support a DAG of parsers prior to enrichment by allowing us to provide a strategy for interpreting what is data and what is metadata in the parser bolt. This enables upstream parsers to pass in a JSON Blob which contains the metadata and have the parser bolt choose which field is the data to be parsed, the remaining fields would be considered metadata (and merged into the resulting data or not depending on our existing rules for handling metadata). To illustrate this better, I've provided a use-case with an example. Note, this PR depends on METRON-1643 and METRON-1642, so those should be reviewed prior to this. Manual Test script: * Go through the use-case doc. ## Pull Request Checklist Thank you for submitting a contribution to Apache Metron. Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions. Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) for complete smoke testing guides. In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following: ### For all changes: - [x] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel). - [x] Does your PR title start with METRON- where is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [x] Has your PR been rebased against the latest commit within the target branch (typically master)? ### For code changes: - [x] Have you included steps to reproduce the behavior or problem that is being changed or addressed? - [x] Have you included steps or a guide to how the change may be verified and tested manually? - [x] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via: ``` mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh ``` - [x] Have you written or updated unit tests and or integration tests to verify your changes? - [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [x] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent? ### For documentation related changes: - [x] Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via `site-book/target/site/index.html`: ``` cd site-book mvn site ``` Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. It is also recommended that [travis-ci](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cestella/incubator-metron enveloped_message_supplier Alternatively you can review and apply these changes as the patch at: https://github.com/apache/metron/pull/1084.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 #1084 commit 5030d00f9eb13ff662716f1c05a731938a66cb52 Author: cstella Date: 2018-06-05T19:05:59Z Regex based message router transformer. commit d1cb318961277da732650152c8526054769d830b Author: cstella Date: 2018-06-05T21:07:13Z Adding message supplier. Tests pending. commit ffad876b377f6cf94213b783a8c139e0973c2e00 Author: cstella Date: 2018-06-06T14:12:55Z Fixed tests commit b349bf0b8fa251846d77cabc8b6aa019154a6425 Author: cstella Date: 2018-06-06T14:22:53Z missing license commit 7c2723bc243996a2a0606e07def5e3b5c1736e59 Author: cstella Date: 2018-06-06T14:35:25Z updating test commit 71959c89816ddf4d84fef9
[GitHub] metron pull request #1084: METRON-1644: Support parser chaining
Github user cestella closed the pull request at: https://github.com/apache/metron/pull/1084 ---
[GitHub] metron issue #1084: METRON-1644: Support parser chaining
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1084 Ok, more javadocs is definitely fair. I went through the core abstractions and added javadocs. If I missed anything, let me know. ---
[GitHub] metron issue #1084: METRON-1644: Support parser chaining
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1084 @ottobackwards Ok, I attempted to do that [here](https://github.com/cestella/incubator-metron/blob/c4e4786e778d5b06cd16f7faa7d3522f620fc2ba/metron-platform/metron-parsers/ParserChaining.md). Can you help me understand where the gaps are. Is it one or more of the following: [ ] The document doesn't have enough detail about what was done before, just detail about the current architecture. [ ] The document does not link to relevant code examples to create custom raw message strategies. [ ] The document is too high level [ ] The document is too low level [ ] Other. Please specify details if this is checked. ---
[GitHub] metron issue #1083: METRON-1643: Create a REGEX_ROUTING field transformation
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1083 You got it [METRON-1655](https://issues.apache.org/jira/browse/METRON-1655) ---
[GitHub] metron issue #1084: METRON-1644: Support parser chaining
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1084 Ok, the prerequisite tickets are merged here and this is ready for review. ---
[GitHub] metron issue #1083: METRON-1643: Create a REGEX_ROUTING field transformation
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/1083
I think a decent UI that would make this a hell of a lot easier, but also
not embedding stellar in JSON would help a lot. Something like:
```
match {
REGEXP_MATCH(pix_type, '^6-302.*') => 'cisco-6-302',
REGEXP_MATCH(pix_type, '^5-304.*') => 'cisco-5-304',
default => NULL
}
```
is a lot easier to read and deal with. Also, enabling REGEXP_MATCH to take
multiple regexes via a list in the 2nd arg would make things easier.
---
[GitHub] metron pull request #1083: METRON-1643: Create a REGEX_ROUTING field transfo...
Github user cestella commented on a diff in the pull request:
https://github.com/apache/metron/pull/1083#discussion_r200675612
--- Diff: metron-platform/metron-parsers/README.md ---
@@ -337,6 +337,28 @@ The following config will rename the fields
`old_field` and `different_old_field
]
}
```
+* `REGEX_SELECT` : This transformation lets users set an output field to
one of a set of possibilities based on matching regexes. This transformation is
useful when the number or conditions are large enough to make a stellar
language match statement unwieldy.
+
+The following config will set the field `my_output` to one of the
+following, dependent upon the value of the `my_input` field:
+* `awesome` if `my_input` starts with `metron` or `mortron`
+* `boo` if `my_input` starts with `scary`
+```
+{
+...
+"fieldTransformations" : [
+ {
+"transformation" : "REGEX_SELECT"
+ , "input" : "my_input"
+ , "output" : "my_output"
+ , "config" : {
+"awesome" : [ "^metron.*", "^mortron.*" ],
--- End diff --
I changed the parsers docs to reflect the use-case that I wrote up for the
next PR. That should fold in well together and be a real use-case. Thanks for
the feedback.
---
[GitHub] metron issue #1083: METRON-1643: Create a REGEX_ROUTING field transformation
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/1083
Sure, so the difference in the parser chaining example would be between the
following
# Stellar
```
"fieldTransformations" : [
{
"transformation" : "STELLAR"
,"input" : "pix_type"
,"output" : "logical_source_type"
,"config" : {
"logical_source_type" : "match{REGEXP_MATCH(pix_type, '^6-302.*') =>
'cisco-6-302', REGEXP_MATCH(pix_type, '^5-304.*') => 'cisco-5-304', default =>
NULL}",
}
}
]
```
# `REGEX_SELECT`
```
"fieldTransformations" : [
{
"transformation" : "REGEX_SELECT"
,"input" : "pix_type"
,"output" : "logical_source_type"
,"config" : {
"cisco-6-302" : "^6-302.*",
"cisco-5-304" : "^5-304.*"
}
}
]
```
It bears mentioning that things get worse in stellar when:
1. You want to match on one of a set of regexs (e.g. `match {
REGEXP_MATCH(pix_type, 'regex1') or REGEXP_MATCH(pix_type, 'regex2') => 'foo',
...`)
2. The set of target kafka topics grows. In this example there are 2, but
if there were 5, they'd all have to fit in a long line of stellar.
I think it's not that stellar isn't equipped to handle it theoretically,
it's just that for these kinds of use-cases, it's more readable to create
something a bit more specific until we get multi-line Stellar available.
Ultimately, I consider this a stop-gap.
---
[GitHub] metron issue #1092: METRON-1652 Document X-Pack Common Problem
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1092 +1 by inspection ---
[GitHub] metron pull request #1093: METRON-1653: Stellar exceptions can be extremely ...
Github user cestella closed the pull request at: https://github.com/apache/metron/pull/1093 ---
[GitHub] metron issue #1093: METRON-1653: Stellar exceptions can be extremely nested ...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1093 Ah crap, I misread a stacktrace. It didn't fail to log the full one, I just failed to have reading comprehension. The main issue here was I saw a stack trace in the wild, which i'm not sure I can share, that appeared to cut off the trace AT the point of a NPE but without the full stack trace of the NPE, which made diagnosing it impossible. I see now that I just didn't get the full cut and paste. I'm closing this. ---
[GitHub] metron issue #1092: METRON-1652 Document X-Pack Common Problem
Github user cestella commented on the issue: https://github.com/apache/metron/pull/1092 I like this a lot. @mmiklavc anything missing here that we've seen in the wild? ---
[GitHub] metron pull request #1093: METRON-1653: Stellar exceptions can be extremely ...
GitHub user cestella opened a pull request: https://github.com/apache/metron/pull/1093 METRON-1653: Stellar exceptions can be extremely nested and we should make efforts to separate the nested exception better ## Contributor Comments Stellar exceptions can be extremely nested and we should make efforts to separate the nested exception better. What happens now is sometimes the exception gets cut off. This one should be able to be tested in the REPL. Just do something that causes an exception and ensure that an error is logged with the inner-most exception in addition to the ParseException that contains the innermost exception. ## Pull Request Checklist Thank you for submitting a contribution to Apache Metron. Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions. Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) for complete smoke testing guides. In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following: ### For all changes: - [x] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel). - [x] Does your PR title start with METRON- where is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [x] Has your PR been rebased against the latest commit within the target branch (typically master)? ### For code changes: - [x] Have you included steps to reproduce the behavior or problem that is being changed or addressed? - [x] Have you included steps or a guide to how the change may be verified and tested manually? - [x] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via: ``` mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh ``` Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. It is also recommended that [travis-ci](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request. You can merge this pull request into a Git repository by running: $ git pull https://github.com/cestella/incubator-metron stellar_exception_logging_part_2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/metron/pull/1093.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 #1093 commit a21ed3adf123abd29401119290048e59a051803c Author: cstella Date: 2018-07-03T14:10:43Z Better logging for nested exceptions. ---
