Uploaded image for project: 'Flume'
  1. Flume
  2. FLUME-1520

Timestamp interceptor should support custom headers

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.8.0
    • Component/s: None
    • Labels:
      None
    1. FLUME-1520.patch
      4 kB
      Hari Shreedharan
    2. FLUME-1520-2.patch
      10 kB
      Tristan Stevens
    3. FLUME-1520-3.patch
      10 kB
      Attila Simon

      Activity

      Hide
      hudson Hudson added a comment -

      FAILURE: Integrated in Jenkins build Flume-trunk-hbase-1 #319 (See https://builds.apache.org/job/Flume-trunk-hbase-1/319/)
      FLUME-1520. Timestamp interceptor should support custom headers (denes: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=13771c905316052d3e94aeb3b4a0d49a27c0f852)

      • (edit) flume-ng-core/src/test/java/org/apache/flume/interceptor/TestTimestampInterceptor.java
      • (edit) flume-ng-core/src/main/java/org/apache/flume/interceptor/TimestampInterceptor.java
      • (edit) flume-ng-doc/sphinx/FlumeUserGuide.rst
      Show
      hudson Hudson added a comment - FAILURE: Integrated in Jenkins build Flume-trunk-hbase-1 #319 (See https://builds.apache.org/job/Flume-trunk-hbase-1/319/ ) FLUME-1520 . Timestamp interceptor should support custom headers (denes: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=13771c905316052d3e94aeb3b4a0d49a27c0f852 ) (edit) flume-ng-core/src/test/java/org/apache/flume/interceptor/TestTimestampInterceptor.java (edit) flume-ng-core/src/main/java/org/apache/flume/interceptor/TimestampInterceptor.java (edit) flume-ng-doc/sphinx/FlumeUserGuide.rst
      Hide
      denes Denes Arvay added a comment -

      Thanks Hari Shreedharan for filing the ticket and creating the initial patch, Tristan Stevens and Attila Simon for the reviews and the updated patches. I've committed the patch with a minor modification in the FlumeUserGuide.rst file as the interceptor's table was malformed.

      Show
      denes Denes Arvay added a comment - Thanks Hari Shreedharan for filing the ticket and creating the initial patch, Tristan Stevens and Attila Simon for the reviews and the updated patches. I've committed the patch with a minor modification in the FlumeUserGuide.rst file as the interceptor's table was malformed.
      Hide
      jira-bot ASF subversion and git services added a comment -

      Commit 13771c905316052d3e94aeb3b4a0d49a27c0f852 in flume's branch refs/heads/trunk from Denes Arvay
      [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=13771c9 ]

      FLUME-1520. Timestamp interceptor should support custom headers

      This change adds a configuration parameter to the TimestampInterceptor
      for the user to be able to define the name of the timestamp header.

      Reviewers: Tristan Stevens, Attila Simon

      (Hari Shreedharan, Tristan Stevens, Attila Simon via Denes Arvay)

      Show
      jira-bot ASF subversion and git services added a comment - Commit 13771c905316052d3e94aeb3b4a0d49a27c0f852 in flume's branch refs/heads/trunk from Denes Arvay [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=13771c9 ] FLUME-1520 . Timestamp interceptor should support custom headers This change adds a configuration parameter to the TimestampInterceptor for the user to be able to define the name of the timestamp header. Reviewers: Tristan Stevens, Attila Simon (Hari Shreedharan, Tristan Stevens, Attila Simon via Denes Arvay)
      Hide
      denes Denes Arvay added a comment -

      Thank you all for the contribution, I'll commit the latest patch if there are no concerns.

      Show
      denes Denes Arvay added a comment - Thank you all for the contribution, I'll commit the latest patch if there are no concerns.
      Hide
      sati Attila Simon added a comment -

      Hi Tristan Stevens,

      The 2nd patch contains an extra file. I guess it was added by mistake: flume-ng-core/src/main/java/org/apache/flume/interceptor/TimestampInterceptor.java.orig
      Unfortunately the patch fails maven build on checkstyle.

      Overall the doc, test and functionality looks good to me.
      I'm only nitpicking on the naming:

      public class TimestampInterceptor implements Interceptor {
          private final String header;
      ...
        public static class Constants {
          public static final String CONFIG_PRESERVE = "preserveExisting";
          public static final boolean DEFAULT_PRESERVE = false;
          public static final String CONFIG_HEADER_NAME = "headerName";
          public static final String DEFAULT_HEADER_NAME = "timestamp";
        }
      

      I have this renaming and using final for constants as well as removed some unused local variables from the tests. I upload a patch which compiles and passes junit as well. Please feel free to use it (completely or parts) if you like.

      Show
      sati Attila Simon added a comment - Hi Tristan Stevens , The 2nd patch contains an extra file. I guess it was added by mistake: flume-ng-core/src/main/java/org/apache/flume/interceptor/TimestampInterceptor.java.orig Unfortunately the patch fails maven build on checkstyle. Overall the doc, test and functionality looks good to me. I'm only nitpicking on the naming: public class TimestampInterceptor implements Interceptor { private final String header; ... public static class Constants { public static final String CONFIG_PRESERVE = "preserveExisting" ; public static final boolean DEFAULT_PRESERVE = false ; public static final String CONFIG_HEADER_NAME = "headerName" ; public static final String DEFAULT_HEADER_NAME = "timestamp" ; } I have this renaming and using final for constants as well as removed some unused local variables from the tests. I upload a patch which compiles and passes junit as well. Please feel free to use it (completely or parts) if you like.
      Hide
      tmgstev Tristan Stevens added a comment -

      Hari Shreedharan This patch still applies cleanly 5 years later!

      I've just added some documentation for it, but LGTM.

      Can we get this committed somebody?

      Show
      tmgstev Tristan Stevens added a comment - Hari Shreedharan This patch still applies cleanly 5 years later! I've just added some documentation for it, but LGTM. Can we get this committed somebody?
      Hide
      iekpo Israel Ekpo added a comment -

      This feels more like an improvement.

      Show
      iekpo Israel Ekpo added a comment - This feels more like an improvement.

        People

        • Assignee:
          hshreedharan Hari Shreedharan
          Reporter:
          hshreedharan Hari Shreedharan
        • Votes:
          0 Vote for this issue
          Watchers:
          7 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development