Rewrite Syslog single line using pyparsing.

.

[kiddi, 2014-03-05 21:30:18]

Sorry for the delay, but here is the first batch of comments.

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog.py
File plaso/parsers/syslog.py (right):

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog.py#newcode4
plaso/parsers/syslog.py:4: # Copyright 2014 The Plaso Project Authors.
don't change the year, this is an update to an already existing file (Although
the update is a more or less a complete rewrite)

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog.py#newcode45
plaso/parsers/syslog.py:45: timestamp: The timestamp time value, epoch.
copy the timestamp description from the previous parser

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog.py#newcode46
plaso/parsers/syslog.py:46: structure: Structure with the parse fields.
?

structure: What object is this (instance of)? .... contains the parsed
fields....

and if you are only using it to extract the computer name, why not just pass
that as a parameter????

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog.py#newcode54
plaso/parsers/syslog.py:54: self.timestamp = timestamp
no need to set the timestamp, that is done in the parent.

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog.py#newcode97
plaso/parsers/syslog.py:97: return True
I would like to see a bit more verification here... for instance is the
timestamp valid? Are necessary fields set with sane values?

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog_test.py
File plaso/parsers/syslog_test.py (right):

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog_test.py#n...
plaso/parsers/syslog_test.py:4: # Copyright 2014 The Plaso Project Authors.
don't change the year (Same reason as before)

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog_test.py#n...
plaso/parsers/syslog_test.py:23: # pylint:
disable-msg=pointless-string-statement
why is this? what is this?

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog_test.py#n...
plaso/parsers/syslog_test.py:64: self.assertEquals(event_objects[9].timestamp,
1364079678123000)
I would like to see at least one more message string test

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog_test.py#n...
plaso/parsers/syslog_test.py:66: 
I would also like to test the lines that fail....

https://codereview.appspot.com/57170053/diff/1/test_data/syslog
File test_data/syslog (right):

https://codereview.appspot.com/57170053/diff/1/test_data/syslog#newcode13
test_data/syslog:13: Dec 31 17:54:32 myhostname.myhost.com anacron[1234]:
Another one just like this (124 job run)
why not keep the multi-line in there and test to see if we don't fail parsing
that?

[Joaquin Moreno, 2014-03-13 12:26:39]

Code updated.

[Joaquin Moreno, 2014-03-13 12:29:20]

Please, apologies for the delay.  Round 2, fight!

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog.py
File plaso/parsers/syslog.py (right):

https://codereview.appspot.com/57170053/diff/1/plaso/parsers/syslog.py#newcode97
plaso/parsers/syslog.py:97: return True
ack

https://codereview.appspot.com/57170053/diff/1/test_data/syslog
File test_data/syslog (right):

https://codereview.appspot.com/57170053/diff/1/test_data/syslog#newcode13
test_data/syslog:13: Dec 31 17:54:32 myhostname.myhost.com anacron[1234]:
Another one just like this (124 job run)
Because when we spoke about multi line parser and simple line parser, we decide
to have a simpleline parser and multiline parser to try to avoid overheads in
logs that doesn't require multiline parsers.

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py
File plaso/parsers/syslog.py (right):

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py#ne...
plaso/parsers/syslog.py:112: 
Should I change structure for "parse_line"?, maybe is more understandable.

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py#ne...
plaso/parsers/syslog.py:147: pid = -1
I dont like and for sure you neither. May we discuss a better approach?

[kiddi, 2014-03-26 21:46:07]

Late reply but better late than never ;)

few more comments

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py
File plaso/parsers/syslog.py (right):

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py#ne...
plaso/parsers/syslog.py:109: logging.debug(u'It is not a timestamp Syslog
plaintext log file')
The timestamp is not correctly formatted for a syslog file?

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py#ne...
plaso/parsers/syslog.py:112: 
no that's fine here\

On 2014/03/13 12:29:21, Joaquin Moreno wrote:
> Should I change structure for "parse_line"?, maybe is more understandable.

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py#ne...
plaso/parsers/syslog.py:118: logging.warning(u'Unable to parse record, unknown
structure: %s' % key)
use .format()

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py#ne...
plaso/parsers/syslog.py:124: structure: log line of structure.
structure: A pyparsing structure object (instance of...)

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py#ne...
plaso/parsers/syslog.py:127: Return an object event.
event object

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py#ne...
plaso/parsers/syslog.py:147: pid = -1
do you need to set the pid if it is not set?

why not have it set to none??
On 2014/03/13 12:29:21, Joaquin Moreno wrote:
> I dont like and for sure you neither. May we discuss a better approach?

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py#ne...
plaso/parsers/syslog.py:149: reporter = structure.reporter.split()[0]
rather use partition

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog_test.py
File plaso/parsers/syslog_test.py (right):

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog_test....
plaso/parsers/syslog_test.py:59: # TODO: Error the host name is not ---, the
regular expression must be
let's fix this before submitting it in.

https://codereview.appspot.com/57170053/diff/20001/test_data/syslog
File test_data/syslog (right):

https://codereview.appspot.com/57170053/diff/20001/test_data/syslog#newcode15
test_data/syslog:15: many syslog parsers.
so yes, I agree with that, keep this simple, as in one single line and one multi
line

But my point was not about that, my point was to have a multi line entry here
and make sur ethe parser fails there, that is that the parser unsuccessfully
tries to parse it (as it should)

[Joaquin Moreno, 2014-07-14 10:20:19]

Hello come again ;)

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py
File plaso/parsers/syslog.py (right):

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog.py#ne...
plaso/parsers/syslog.py:109: logging.debug(u'It is not a timestamp Syslog
plaintext log file')
it is checking that it is a valid Syslog line. If it doesn't have a valid
timestamp return False, otherwise (after check all the fields)True.

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog_test.py
File plaso/parsers/syslog_test.py (right):

https://codereview.appspot.com/57170053/diff/20001/plaso/parsers/syslog_test....
plaso/parsers/syslog_test.py:59: # TODO: Error the host name is not ---, the
regular expression must be
Oki doki. Let me think about it :-)

https://codereview.appspot.com/57170053/diff/20001/test_data/syslog
File test_data/syslog (right):

https://codereview.appspot.com/57170053/diff/20001/test_data/syslog#newcode15
test_data/syslog:15: many syslog parsers.
Perfect. It works like that. It tried to parse the second line of the entry like
it was a new line:

$ tail -2 test_data/syslog
Nov 18 01:15:20 myhostname.myhost.com aprocess[10100]: This is a multi-line
message that screws up
many syslog parsers.
$ python plaso/parsers/syslog_test.py
...
WARNING:root:Unable to parse log line: many syslog parsers.
$

[kiddi, 2014-07-19 17:15:55]

I don't see any updates to the code, did you forget to push the new patch set?

[kiddi, 2014-11-05 17:25:40]

Hi

Any news?