Review Board 1.7.22

FLUME-1126: Support RFC 3164 and 5424 syslog format timestamp parsing

Review Request #4809 - Created April 19, 2012 and updated

Prasad Mujumdar
aprabhakar, mpercy
Support for timestamp and hostname parsing.
Updated syslog tests to cover timestamp and hostname handling.
Manually tested using syslog4j
Review request changed
Updated (April 27, 2012, 2:58 p.m.)
Refactored to use regex for syslog header format. Also additional config to specify add-hoc format 
Ship it!
Posted (April 27, 2012, 4:11 p.m.)

Changes look good Prasad. Some nits noted below. Once you are done with the revision, please update the patch on the Jira.
convention: rename to messageFormats instead of using underscore.
convention: please use class name with CamelCase (SyslogFormater)
nit: please rename it to initHeaderFormats() instead.
nit: please remove extraneous whitespace.
nit:trailing whitespace.
Posted (April 27, 2012, 4:27 p.m.)


Its not used anymore, removed.
Posted (April 27, 2012, 9:27 p.m.)
Interesting approach, Prasad. Looks pretty nice. I just have a couple of comments.
The RFC5424 version field should be a digit followed by a space. So I think this pattern should be: "(?:\\d\\s)?"; i.e. no [] character class around in. Worth adding a unit test w/ the example messages from RFC5424 to be sure it's compliant.
  1. Actually, I just checked and it works as-is. So, nevermind this comment :)
Why would the Scanner be closed? Shouldn't this exception be logged or re-thrown?
  1. Looks like it's not a closed Scanner but it will throw if match() is called when there are no matches. So, please disregard this too.