> On April 11, 2013, 10:23 a.m., Israel Ekpo wrote: > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java, > > line 150 > > <https://reviews.apache.org/r/10379/diff/1/?file=279376#file279376line150> > > > > I think you should > > > > 1. Add a null check for the event headers before attempting to retrieve > > the timestamp header. > > > > 2. Provide a strategy for still returning an index using the current > > time in cases where no valid timestamp header in present in the event.
Hi Mr Ekpo, The ensureHasTimestamp method already adds the current time to a new timestamp header if it's not there. However, you are correct that it did not handle having no headers at all. I have remedied that. - Edward ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10379/#review18997 ----------------------------------------------------------- On April 9, 2013, 5:59 p.m., Edward Sargisson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10379/ > ----------------------------------------------------------- > > (Updated April 9, 2013, 5:59 p.m.) > > > Review request for Flume and Hari Shreedharan. > > > Description > ------- > > This change gets the timestamp from the event and uses it (in UTC) to > determine the name of the index to write to. This is required to match the > behaviour of Logstash so that Kibana can find the log events. > The previous code would use the current time and would do it in the timezone > of the Flume agent's host. > > > This addresses bug FLUME-1782. > https://issues.apache.org/jira/browse/FLUME-1782 > > > Diffs > ----- > > flume-ng-doc/sphinx/FlumeUserGuide.rst 693c0d7 > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/main/java/org/apache/flume/sink/elasticsearch/ElasticSearchSink.java > 1b3db14 > > flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java > 94b95b1 > > Diff: https://reviews.apache.org/r/10379/diff/ > > > Testing > ------- > > All unit tests and integration tests pass. A snapshot using commit > 5b9d31f1ad228 and the patch for flume-1972 has passed our internal > integration tests using customisations. > > > Thanks, > > Edward Sargisson > >
