[GitHub] incubator-metron issue #451: METRON-157: Added CEF Parser

2017-02-21 Thread kylerichardson
Github user kylerichardson commented on the issue:

https://github.com/apache/incubator-metron/pull/451
  
Not from me. +1, great contribution.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron issue #451: METRON-157: Added CEF Parser

2017-02-21 Thread cestella
Github user cestella commented on the issue:

https://github.com/apache/incubator-metron/pull/451
  
+1 by inspection, this is great stuff.  Are there outstanding asks for this 
PR that would prevent it from being committed?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron issue #451: METRON-157: Added CEF Parser

2017-02-21 Thread simonellistonball
Github user simonellistonball commented on the issue:

https://github.com/apache/incubator-metron/pull/451
  
Kicking travis.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron issue #451: METRON-157: Added CEF Parser

2017-02-12 Thread trixpan
Github user trixpan commented on the issue:

https://github.com/apache/incubator-metron/pull/451
  
Seems ok to me.

The only last comment which certainly is not a blocker (and if I read the 
code correctly, is already addressed 
[here](https://github.com/apache/incubator-metron/pull/451/files#diff-5100b5d781b88b14f7335b604268905aR44)
 ) is to add escaped '='  to one of extension fields used in test as this is a 
quite common pattern and covered by specific escaping rules:

> If an equal sign (=) is used in the extensions, it has to be escaped with 
a backslash (\). Equal signs in
> the header need no escaping. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron issue #451: METRON-157: Added CEF Parser

2017-02-12 Thread simonellistonball
Github user simonellistonball commented on the issue:

https://github.com/apache/incubator-metron/pull/451
  
Agreed, let's pull the date discussion into a wider forum. Apart from this, 
is there anything else you see in this parser specifically to block merging?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron issue #451: METRON-157: Added CEF Parser

2017-02-12 Thread trixpan
Github user trixpan commented on the issue:

https://github.com/apache/incubator-metron/pull/451
  
Yep. I would say unless HPE clarifies Mmm being English only, providing the 
parsers with the ability to set locale would be ideal. 

And I didn't even mentioned that they use Zzz aka the timezone 
representation from hell... :-) 

One my favourite posts on the subject is:

http://stackoverflow.com/a/28554443/76

Which could be summarised by the question BST which of them? (also kindly 
illustrated here https://www.timeanddate.com/time/zones/) 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron issue #451: METRON-157: Added CEF Parser

2017-02-12 Thread simonellistonball
Github user simonellistonball commented on the issue:

https://github.com/apache/incubator-metron/pull/451
  
The joys of international date parsing, right? Seems like a the CEF 
standard is not the most well read among device vendors. A number of the 'from 
the wild' examples we've got in the tests already violate the rt standard set 
down by HPE, hence the DateUtils class separating the list of "according to the 
standard" and "found in the wild".

This feels like a much wider issue we should handle elsewhere, maybe in the 
new DateUtils class I introduced here. To do so properly we would have to have 
a way of feeding the source locale for the log feed into the parser. We should 
really open a discuss on the best way to do that in general, but maybe it's 
something bound to a general parser config (i.e. each parser can specify date 
locale) and this can be propagated on a general basis rather than parser to 
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron issue #451: METRON-157: Added CEF Parser

2017-02-12 Thread trixpan
Github user trixpan commented on the issue:

https://github.com/apache/incubator-metron/pull/451
  
No.  And under RFC 3164, Syslog's Mmm is English only but this certainty is 
not present in the CEF spec states MMM as SimpleDateFormat and makes no 
reference over locale. This in theory means it should be locale agnostic. 

 If they adopt the syslog approach, locale should not be an issue but being 
CEF God knows. :-) 

Regarding robustness, SDF should not be able to automatically recognise MMM 
in French on  metron cluster running under user.language=en. From the top of my 
head for this to occur, the code must invoke SDF specifying the locale used for 
parsing. 

This whole localised dates shouldn't be an issue for servers, as they 
frequently run without locale settings but its particularly complex within 
multinationals operating under multiple languages. Think about all those agents 
insisting in using local settings when crafting CEF messages...

Shouldn't happen but after witnessing vendors violating their own standards 
I lost faith :-) 

BTW, I am happy to forward you whatever I get back from HPE around this 
issue.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron issue #451: METRON-157: Added CEF Parser

2017-02-12 Thread simonellistonball
Github user simonellistonball commented on the issue:

https://github.com/apache/incubator-metron/pull/451
  
Syslog timestamp capture looks to be locale sensitive here, though all 
other date parsing is SimpleDateFormat based, so should be robust to locale. Do 
you see this issue on syslog headers?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron issue #451: METRON-157: Added CEF Parser

2017-02-11 Thread trixpan
Github user trixpan commented on the issue:

https://github.com/apache/incubator-metron/pull/451
  
@simonellistonball seems like the code will hit the same issue we hit in 
NIFI-3466? The CEF spec doesn mention but it seems like ArcSight behavior is to 
be able to MMM even when MMM is written with localised values for "Short Month 
Name"


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron issue #451: METRON-157: Added CEF Parser

2017-02-11 Thread simonellistonball
Github user simonellistonball commented on the issue:

https://github.com/apache/incubator-metron/pull/451
  
@kylerichardson no problem at all, would really appreciate it if you could 
review, and add anything from any work you have on this. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] incubator-metron issue #451: METRON-157: Added CEF Parser

2017-02-11 Thread kylerichardson
Github user kylerichardson commented on the issue:

https://github.com/apache/incubator-metron/pull/451
  
@simonellistonball, thanks for picking this one up! I have unassigned the 
JIRA from myself as I've clearly not had the time to work on it recently.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---