----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4809/#review7093 -----------------------------------------------------------
Nice work Prasad, looks good overall. I have a couple of nits / questions inline. Also, we should file another JIRA to support the RFC5424 timestamp format that supports fractional seconds, i.e. 2003-10-11T22:14:15.003Z and 2003-08-24T05:14:15.000003-07:00. While we would need to round microsecond times to the nearest millisecond since our base format is Java milliseconds since the epoch, today we treat these example timestamp strings as invalid. flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java <https://reviews.apache.org/r/4809/#comment15702> This variable is not ever read. flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java <https://reviews.apache.org/r/4809/#comment15703> Not sure what's going on here. Why are we checking for a dash? And what does startedTimeStamp mean? I don't think it means we have started parsing the timestamp. - Mike On 2012-04-20 03:19:50, Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4809/ > ----------------------------------------------------------- > > (Updated 2012-04-20 03:19:50) > > > Review request for Flume, Arvind Prabhakar and Mike Percy. > > > Summary > ------- > > Support for timestamp and hostname parsing. > > > This addresses bug FLUME-1126. > https://issues.apache.org/jira/browse/FLUME-1126 > > > Diffs > ----- > > flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java > b0485b1 > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java > 732cce5 > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java > 653f5eb > > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUdpSource.java > 3a7c486 > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUtils.java > 8b1f7c5 > > Diff: https://reviews.apache.org/r/4809/diff > > > Testing > ------- > > Updated syslog tests to cover timestamp and hostname handling. > Manually tested using syslog4j > > > Thanks, > > Prasad > >
