Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: v1.0.0
    • Fix Version/s: v1.1.0
    • Component/s: Sinks+Sources
    • Labels:
      None

      Description

      Logging appender for log4j framework, which uses AvroSource under the hood.

      1. FLUME-886.patch
        5 kB
        Jeff de Anda
      2. FLUME-886-1.patch
        13 kB
        Hari Shreedharan
      3. FLUME-886-3.patch
        14 kB
        Hari Shreedharan
      4. FLUME-886-4.patch
        18 kB
        Hari Shreedharan
      5. FLUME-886-5.patch
        21 kB
        Hari Shreedharan

        Activity

        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3243/
        -----------------------------------------------------------

        Review request for Flume and Eric Sammer.

        Summary
        -------

        Created log4j appender using avro source.

        This addresses bug flume-886.
        https://issues.apache.org/jira/browse/flume-886

        Diffs


        ./flume-ng-core/src/main/java/org/apache/flume/log4j/AvroAppender.java PRE-CREATION
        ./flume-ng-core/src/test/java/org/apache/flume/log4j/TestAvroAppender.java PRE-CREATION

        Diff: https://reviews.apache.org/r/3243/diff

        Testing
        -------

        Thanks,

        Jeff

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3243/ ----------------------------------------------------------- Review request for Flume and Eric Sammer. Summary ------- Created log4j appender using avro source. This addresses bug flume-886. https://issues.apache.org/jira/browse/flume-886 Diffs ./flume-ng-core/src/main/java/org/apache/flume/log4j/AvroAppender.java PRE-CREATION ./flume-ng-core/src/test/java/org/apache/flume/log4j/TestAvroAppender.java PRE-CREATION Diff: https://reviews.apache.org/r/3243/diff Testing ------- Thanks, Jeff
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3243/#review4707
        -----------------------------------------------------------

        Thanks for the patch Jeff, andy apologies for the delayed response. Some general comments about the change:

        1. Please use two-space indent and no tabs in your code. Also, please remove trailing whitespaces wherever they are present. Reviewboard highlights them with red color.

        2. Instead of adding it to the core module, I suggest creating an explicit client module that contains this as one of the submodules. The reason is that we do not want an application that wishes to use the log4j appender for flume to end up adding all of its dependencies in the classpath. Without an explicitly packaged system, it is not clear which dependencies are required and which are not.

        3. The current implementation is not capturing any logging event metadata at all. Looking at the LoggingEvent API from log4j 2.1.16, I feel that at least the logger name, the timestamp and the logging level must be captured as flume event headers.

        4. The event body is currently turned into a byte payload using the default encoding. However, I suggest using UTF-8 encoding as a standard and also adding a header in the event which identifies the character encoding used for extracting the bytes.

        5. What is the performance impact of requesting a specific client on every logging request? Would it make sense to have the client initialized once and used for repeated append calls? I am not sure of which is better, and suggest you looking into the cost of doing either approach in order to choose the final implementation.

        Thanks,
        Arvind

        • Arvind

        On 2011-12-16 22:25:40, Jeff de Anda wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3243/

        -----------------------------------------------------------

        (Updated 2011-12-16 22:25:40)

        Review request for Flume and Eric Sammer.

        Summary

        -------

        Created log4j appender using avro source.

        This addresses bug flume-886.

        https://issues.apache.org/jira/browse/flume-886

        Diffs

        -----

        ./flume-ng-core/src/main/java/org/apache/flume/log4j/AvroAppender.java PRE-CREATION

        ./flume-ng-core/src/test/java/org/apache/flume/log4j/TestAvroAppender.java PRE-CREATION

        Diff: https://reviews.apache.org/r/3243/diff

        Testing

        -------

        Thanks,

        Jeff

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3243/#review4707 ----------------------------------------------------------- Thanks for the patch Jeff, andy apologies for the delayed response. Some general comments about the change: 1. Please use two-space indent and no tabs in your code. Also, please remove trailing whitespaces wherever they are present. Reviewboard highlights them with red color. 2. Instead of adding it to the core module, I suggest creating an explicit client module that contains this as one of the submodules. The reason is that we do not want an application that wishes to use the log4j appender for flume to end up adding all of its dependencies in the classpath. Without an explicitly packaged system, it is not clear which dependencies are required and which are not. 3. The current implementation is not capturing any logging event metadata at all. Looking at the LoggingEvent API from log4j 2.1.16, I feel that at least the logger name, the timestamp and the logging level must be captured as flume event headers. 4. The event body is currently turned into a byte payload using the default encoding. However, I suggest using UTF-8 encoding as a standard and also adding a header in the event which identifies the character encoding used for extracting the bytes. 5. What is the performance impact of requesting a specific client on every logging request? Would it make sense to have the client initialized once and used for repeated append calls? I am not sure of which is better, and suggest you looking into the cost of doing either approach in order to choose the final implementation. Thanks, Arvind Arvind On 2011-12-16 22:25:40, Jeff de Anda wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3243/ ----------------------------------------------------------- (Updated 2011-12-16 22:25:40) Review request for Flume and Eric Sammer. Summary ------- Created log4j appender using avro source. This addresses bug flume-886. https://issues.apache.org/jira/browse/flume-886 Diffs ----- ./flume-ng-core/src/main/java/org/apache/flume/log4j/AvroAppender.java PRE-CREATION ./flume-ng-core/src/test/java/org/apache/flume/log4j/TestAvroAppender.java PRE-CREATION Diff: https://reviews.apache.org/r/3243/diff Testing ------- Thanks, Jeff
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/
        -----------------------------------------------------------

        Review request for Flume.

        Summary
        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.
        https://issues.apache.org/jira/browse/FLUME-886

        Diffs


        branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION
        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION
        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION
        branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION
        branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION
        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing
        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- Review request for Flume. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/#review5160
        -----------------------------------------------------------

        A few comments below, otherwise looks good!

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java
        <https://reviews.apache.org/r/3924/#comment11283>

        Perhaps we could have a better error message since these two messages will go up to users?

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java
        <https://reviews.apache.org/r/3924/#comment11284>

        Better error messages on all these errors?

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java
        <https://reviews.apache.org/r/3924/#comment11285>

        We don't want to shutdown the Transceiver here? Maybe not, I am not familiar with appenders.

        • Brock

        On 2012-02-16 07:42:40, Hari Shreedharan wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3924/

        -----------------------------------------------------------

        (Updated 2012-02-16 07:42:40)

        Review request for Flume.

        Summary

        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.

        https://issues.apache.org/jira/browse/FLUME-886

        Diffs

        -----

        branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION

        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing

        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/#review5160 ----------------------------------------------------------- A few comments below, otherwise looks good! branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java < https://reviews.apache.org/r/3924/#comment11283 > Perhaps we could have a better error message since these two messages will go up to users? branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java < https://reviews.apache.org/r/3924/#comment11284 > Better error messages on all these errors? branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java < https://reviews.apache.org/r/3924/#comment11285 > We don't want to shutdown the Transceiver here? Maybe not, I am not familiar with appenders. Brock On 2012-02-16 07:42:40, Hari Shreedharan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-16 07:42:40) Review request for Flume. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs ----- branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/
        -----------------------------------------------------------

        (Updated 2012-02-16 17:47:02.183272)

        Review request for Flume.

        Changes
        -------

        UPdated error messages. Kill client object if close function is called, forcing new client object if append is called.

        Summary
        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.
        https://issues.apache.org/jira/browse/FLUME-886

        Diffs (updated)


        branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION
        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION
        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION
        branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION
        branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION
        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing
        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-16 17:47:02.183272) Review request for Flume. Changes ------- UPdated error messages. Kill client object if close function is called, forcing new client object if append is called. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs (updated) branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/#review5164
        -----------------------------------------------------------

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java
        <https://reviews.apache.org/r/3924/#comment11312>

        Excellent error messages! The only comment I have is, should we pass the exception along so that people can see the stacktrace:

        throw new FlumeException(errormsg, e);

        • Brock

        On 2012-02-16 17:47:02, Hari Shreedharan wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3924/

        -----------------------------------------------------------

        (Updated 2012-02-16 17:47:02)

        Review request for Flume.

        Summary

        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.

        https://issues.apache.org/jira/browse/FLUME-886

        Diffs

        -----

        branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION

        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing

        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/#review5164 ----------------------------------------------------------- branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java < https://reviews.apache.org/r/3924/#comment11312 > Excellent error messages! The only comment I have is, should we pass the exception along so that people can see the stacktrace: throw new FlumeException(errormsg, e); Brock On 2012-02-16 17:47:02, Hari Shreedharan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-16 17:47:02) Review request for Flume. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs ----- branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/
        -----------------------------------------------------------

        (Updated 2012-02-16 18:50:02.817429)

        Review request for Flume.

        Changes
        -------

        Passing exception down.

        Summary
        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.
        https://issues.apache.org/jira/browse/FLUME-886

        Diffs (updated)


        branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION
        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION
        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION
        branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION
        branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION
        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing
        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-16 18:50:02.817429) Review request for Flume. Changes ------- Passing exception down. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs (updated) branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        Hari Shreedharan added a comment -
        • Cleaned up error messages.
        • Passing stack trace down to user.
        • Close function sets protocolClient to null.
        Show
        Hari Shreedharan added a comment - Cleaned up error messages. Passing stack trace down to user. Close function sets protocolClient to null.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/#review5165
        -----------------------------------------------------------

        Ship it!

        • Brock

        On 2012-02-16 18:50:02, Hari Shreedharan wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3924/

        -----------------------------------------------------------

        (Updated 2012-02-16 18:50:02)

        Review request for Flume.

        Summary

        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.

        https://issues.apache.org/jira/browse/FLUME-886

        Diffs

        -----

        branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION

        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing

        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/#review5165 ----------------------------------------------------------- Ship it! Brock On 2012-02-16 18:50:02, Hari Shreedharan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-16 18:50:02) Review request for Flume. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs ----- branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/#review5211
        -----------------------------------------------------------

        I have a suspicion that if you actually try to use this from an application via a configuration file you won't be able to. I don't see any setter methods for the host and port.

        • Ralph

        On 2012-02-16 18:50:02, Hari Shreedharan wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3924/

        -----------------------------------------------------------

        (Updated 2012-02-16 18:50:02)

        Review request for Flume.

        Summary

        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.

        https://issues.apache.org/jira/browse/FLUME-886

        Diffs

        -----

        branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION

        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing

        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/#review5211 ----------------------------------------------------------- I have a suspicion that if you actually try to use this from an application via a configuration file you won't be able to. I don't see any setter methods for the host and port. Ralph On 2012-02-16 18:50:02, Hari Shreedharan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-16 18:50:02) Review request for Flume. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs ----- branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-02-19 16:30:08, Ralph Goers wrote:

        > I have a suspicion that if you actually try to use this from an application via a configuration file you won't be able to. I don't see any setter methods for the host and port.

        The reason I didn't put setters for host and port are that the AvroClient object can be created only if know the hostname and port. So the constructor needs these arguments. If the hostname and port change because the config changes, the application can simply create a new object of the Log4jAppender(resetting the port and hostname using setters will have the same effect - since internally we would be doing the same thing, by replacing the protocolClient). For example this would be how the setter would look:

        public synchronized void setHostnameAndPort(String hostname, int port)

        { this.hostname = hostname; this.port = port protocolClient = null; //This basically to make sure a new AvroClient is created, considering the new setting. }
        • Hari

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/#review5211
        -----------------------------------------------------------

        On 2012-02-16 18:50:02, Hari Shreedharan wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3924/

        -----------------------------------------------------------

        (Updated 2012-02-16 18:50:02)

        Review request for Flume.

        Summary

        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.

        https://issues.apache.org/jira/browse/FLUME-886

        Diffs

        -----

        branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION

        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing

        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-02-19 16:30:08, Ralph Goers wrote: > I have a suspicion that if you actually try to use this from an application via a configuration file you won't be able to. I don't see any setter methods for the host and port. The reason I didn't put setters for host and port are that the AvroClient object can be created only if know the hostname and port. So the constructor needs these arguments. If the hostname and port change because the config changes, the application can simply create a new object of the Log4jAppender(resetting the port and hostname using setters will have the same effect - since internally we would be doing the same thing, by replacing the protocolClient). For example this would be how the setter would look: public synchronized void setHostnameAndPort(String hostname, int port) { this.hostname = hostname; this.port = port protocolClient = null; //This basically to make sure a new AvroClient is created, considering the new setting. } Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/#review5211 ----------------------------------------------------------- On 2012-02-16 18:50:02, Hari Shreedharan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-16 18:50:02) Review request for Flume. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs ----- branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/#review5213
        -----------------------------------------------------------

        Hari - thanks for the patch. Some feedback follows:

        • Please do not use tabs anywhere. Instead, please use two spaces for indent where you are otherwise using a tab.
        • Please remove any trailing white spaces from the sources. These are highlighted with red color in the review board diff view.
        • Since this is one of the many clients that we would like to build, I suggest that you move it under a top-level module called flume-ng-clients. That way the root directory will not end up containing a lot of client modules.

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java
        <https://reviews.apache.org/r/3924/#comment11406>

        This initialization should ideally happen in the overridden activateOptions() method of the appender. Also, as pointed out by Ralph, the configuration will need to be injected via setter methods, so please add them.

        One thing that I am not too sure about is how the NettyTransceiver is supposed to be used in a multi-threaded environment. It will be good to find that out before making the assumption that it is safe for such use.

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java
        <https://reviews.apache.org/r/3924/#comment11405>

        Since this appender is part of Flume distribution, the header names should be in flume's private namespace. For example - LOGGER_NAME should be "flume.client.log4j.logger.name".

        You can do this by having providing these standard names as part of the enum state. Also, it is customary to have the first enum be OTHER which ensures that the ordinal for unknown headers will always remain 0.

        • Arvind

        On 2012-02-16 18:50:02, Hari Shreedharan wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3924/

        -----------------------------------------------------------

        (Updated 2012-02-16 18:50:02)

        Review request for Flume.

        Summary

        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.

        https://issues.apache.org/jira/browse/FLUME-886

        Diffs

        -----

        branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION

        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing

        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/#review5213 ----------------------------------------------------------- Hari - thanks for the patch. Some feedback follows: Please do not use tabs anywhere. Instead, please use two spaces for indent where you are otherwise using a tab. Please remove any trailing white spaces from the sources. These are highlighted with red color in the review board diff view. Since this is one of the many clients that we would like to build, I suggest that you move it under a top-level module called flume-ng-clients. That way the root directory will not end up containing a lot of client modules. branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java < https://reviews.apache.org/r/3924/#comment11406 > This initialization should ideally happen in the overridden activateOptions() method of the appender. Also, as pointed out by Ralph, the configuration will need to be injected via setter methods, so please add them. One thing that I am not too sure about is how the NettyTransceiver is supposed to be used in a multi-threaded environment. It will be good to find that out before making the assumption that it is safe for such use. branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java < https://reviews.apache.org/r/3924/#comment11405 > Since this appender is part of Flume distribution, the header names should be in flume's private namespace. For example - LOGGER_NAME should be "flume.client.log4j.logger.name". You can do this by having providing these standard names as part of the enum state. Also, it is customary to have the first enum be OTHER which ensures that the ordinal for unknown headers will always remain 0. Arvind On 2012-02-16 18:50:02, Hari Shreedharan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-16 18:50:02) Review request for Flume. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs ----- branches/flume-728/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/main/java/org/apache/flume/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/java/org/apache/flume/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-log4jappender/src/test/resources/log4j.properties PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/
        -----------------------------------------------------------

        (Updated 2012-02-20 22:54:11.791741)

        Review request for Flume.

        Changes
        -------

        Incorporated feedback from Arvind and Ralph.

        Summary
        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.
        https://issues.apache.org/jira/browse/FLUME-886

        Diffs (updated)


        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION
        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION
        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION
        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION
        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION
        branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION
        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing
        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-20 22:54:11.791741) Review request for Flume. Changes ------- Incorporated feedback from Arvind and Ralph. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs (updated) branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        Hari Shreedharan added a comment -

        Incorporated feedback from Arvind and Ralph.

        Show
        Hari Shreedharan added a comment - Incorporated feedback from Arvind and Ralph.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/#review5226
        -----------------------------------------------------------

        Thanks for incorporating the feedback Hari. Change looks good, a couple of minor suggestions follow.

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java
        <https://reviews.apache.org/r/3924/#comment11424>

        A brief javadoc will be good that describes this implementation.

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java
        <https://reviews.apache.org/r/3924/#comment11423>

        Not sure if this is a valid assumption since the first line of the append() method raises an exception if the protocol client is null.

        Also, if we must dispose the protocol client, the underlying transceiver should be closed as well in order to ensure there are no resource leaks.

        • Arvind

        On 2012-02-20 22:54:11, Hari Shreedharan wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3924/

        -----------------------------------------------------------

        (Updated 2012-02-20 22:54:11)

        Review request for Flume.

        Summary

        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.

        https://issues.apache.org/jira/browse/FLUME-886

        Diffs

        -----

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION

        branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION

        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing

        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/#review5226 ----------------------------------------------------------- Thanks for incorporating the feedback Hari. Change looks good, a couple of minor suggestions follow. branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java < https://reviews.apache.org/r/3924/#comment11424 > A brief javadoc will be good that describes this implementation. branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java < https://reviews.apache.org/r/3924/#comment11423 > Not sure if this is a valid assumption since the first line of the append() method raises an exception if the protocol client is null. Also, if we must dispose the protocol client, the underlying transceiver should be closed as well in order to ensure there are no resource leaks. Arvind On 2012-02-20 22:54:11, Hari Shreedharan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-20 22:54:11) Review request for Flume. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs ----- branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-02-21 00:29:14, Arvind Prabhakar wrote:

        > branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java, lines 106-109

        > <https://reviews.apache.org/r/3924/diff/5/?file=76163#file76163line106>

        >

        > Not sure if this is a valid assumption since the first line of the append() method raises an exception if the protocol client is null.

        >

        > Also, if we must dispose the protocol client, the underlying transceiver should be closed as well in order to ensure there are no resource leaks.

        Hi Arvind,

        Thanks for the feedback. If close() is called on the appender, the protocolClient is set to null, which is what I meant to do. This is to ensure that a future append cannot happen on a closed appender. I think the comment was outdated, and a call to append no longer creates a new client, but throws an exception. Sorry about that. If append is called on the appender after close(), it throws an Exception which I think is a valid case. I think it is also valid to throw an exception, if append is called before the activeOptions() function is called(in which case the protocolClient is null). Both these cases exist only when the appender is used programatically, not through a config file.

        I will close the transceiver in the close function, to make sure the resources are released immediately once the close function is called

        Just a note: even without this the resources would be cleared off by Java garbage collector since the protocolClient reference is no longer valid, hence all internal members of the client will be cleared, since no references to them exist anywhere outside.

        On 2012-02-21 00:29:14, Arvind Prabhakar wrote:

        > branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java, line 40

        > <https://reviews.apache.org/r/3924/diff/5/?file=76163#file76163line40>

        >

        > A brief javadoc will be good that describes this implementation.

        Sure.

        • Hari

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/#review5226
        -----------------------------------------------------------

        On 2012-02-20 22:54:11, Hari Shreedharan wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3924/

        -----------------------------------------------------------

        (Updated 2012-02-20 22:54:11)

        Review request for Flume.

        Summary

        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.

        https://issues.apache.org/jira/browse/FLUME-886

        Diffs

        -----

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION

        branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION

        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing

        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-02-21 00:29:14, Arvind Prabhakar wrote: > branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java, lines 106-109 > < https://reviews.apache.org/r/3924/diff/5/?file=76163#file76163line106 > > > Not sure if this is a valid assumption since the first line of the append() method raises an exception if the protocol client is null. > > Also, if we must dispose the protocol client, the underlying transceiver should be closed as well in order to ensure there are no resource leaks. Hi Arvind, Thanks for the feedback. If close() is called on the appender, the protocolClient is set to null, which is what I meant to do. This is to ensure that a future append cannot happen on a closed appender. I think the comment was outdated, and a call to append no longer creates a new client, but throws an exception. Sorry about that. If append is called on the appender after close(), it throws an Exception which I think is a valid case. I think it is also valid to throw an exception, if append is called before the activeOptions() function is called(in which case the protocolClient is null). Both these cases exist only when the appender is used programatically, not through a config file. I will close the transceiver in the close function, to make sure the resources are released immediately once the close function is called Just a note: even without this the resources would be cleared off by Java garbage collector since the protocolClient reference is no longer valid, hence all internal members of the client will be cleared, since no references to them exist anywhere outside. On 2012-02-21 00:29:14, Arvind Prabhakar wrote: > branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java, line 40 > < https://reviews.apache.org/r/3924/diff/5/?file=76163#file76163line40 > > > A brief javadoc will be good that describes this implementation. Sure. Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/#review5226 ----------------------------------------------------------- On 2012-02-20 22:54:11, Hari Shreedharan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-20 22:54:11) Review request for Flume. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs ----- branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/
        -----------------------------------------------------------

        (Updated 2012-02-21 02:51:54.629181)

        Review request for Flume.

        Changes
        -------

        Javadocs added. Close off transceiver in close function.

        Summary
        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.
        https://issues.apache.org/jira/browse/FLUME-886

        Diffs (updated)


        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION
        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION
        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION
        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION
        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION
        branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION
        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing
        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-21 02:51:54.629181) Review request for Flume. Changes ------- Javadocs added. Close off transceiver in close function. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs (updated) branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/
        -----------------------------------------------------------

        (Updated 2012-02-21 02:53:41.556678)

        Review request for Flume.

        Changes
        -------

        Indentation correction.

        Summary
        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.
        https://issues.apache.org/jira/browse/FLUME-886

        Diffs (updated)


        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION
        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION
        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION
        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION
        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION
        branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION
        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing
        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-21 02:53:41.556678) Review request for Flume. Changes ------- Indentation correction. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs (updated) branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        Hari Shreedharan added a comment -

        Added javadocs, closed off transciever in the close().

        Show
        Hari Shreedharan added a comment - Added javadocs, closed off transciever in the close().
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/#review5231
        -----------------------------------------------------------

        Ship it!

        +1

        • Arvind

        On 2012-02-21 02:53:41, Hari Shreedharan wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3924/

        -----------------------------------------------------------

        (Updated 2012-02-21 02:53:41)

        Review request for Flume.

        Summary

        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.

        https://issues.apache.org/jira/browse/FLUME-886

        Diffs

        -----

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION

        branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION

        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing

        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/#review5231 ----------------------------------------------------------- Ship it! +1 Arvind On 2012-02-21 02:53:41, Hari Shreedharan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-21 02:53:41) Review request for Flume. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs ----- branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari
        Hide
        Arvind Prabhakar added a comment -

        Patch committed to flume-728 branch. Thanks Hari!

        Show
        Arvind Prabhakar added a comment - Patch committed to flume-728 branch. Thanks Hari!
        Hide
        Hudson added a comment -

        Integrated in flume-728 #113 (See https://builds.apache.org/job/flume-728/113/)
        FLUME-886. Create Log4j Appender.

        (Hari Shreedharan via Arvind Prabhakar) (Revision 1291585)

        Result = FAILURE
        arvind : http://svn.apache.org/viewvc/?view=rev&rev=1291585
        Files :

        • /incubator/flume/branches/flume-728/flume-ng-clients
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources
        • /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties
        • /incubator/flume/branches/flume-728/flume-ng-clients/pom.xml
        • /incubator/flume/branches/flume-728/pom.xml
        Show
        Hudson added a comment - Integrated in flume-728 #113 (See https://builds.apache.org/job/flume-728/113/ ) FLUME-886 . Create Log4j Appender. (Hari Shreedharan via Arvind Prabhakar) (Revision 1291585) Result = FAILURE arvind : http://svn.apache.org/viewvc/?view=rev&rev=1291585 Files : /incubator/flume/branches/flume-728/flume-ng-clients /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources /incubator/flume/branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties /incubator/flume/branches/flume-728/flume-ng-clients/pom.xml /incubator/flume/branches/flume-728/pom.xml
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3924/#review5233
        -----------------------------------------------------------

        Ship it!

        Looks good to me.

        • Ralph

        On 2012-02-21 02:53:41, Hari Shreedharan wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3924/

        -----------------------------------------------------------

        (Updated 2012-02-21 02:53:41)

        Review request for Flume.

        Summary

        -------

        A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port.

        This addresses bug FLUME-886.

        https://issues.apache.org/jira/browse/FLUME-886

        Diffs

        -----

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION

        branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION

        branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION

        branches/flume-728/pom.xml 1244858

        Diff: https://reviews.apache.org/r/3924/diff

        Testing

        -------

        Added unit tests.

        Thanks,

        Hari

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/#review5233 ----------------------------------------------------------- Ship it! Looks good to me. Ralph On 2012-02-21 02:53:41, Hari Shreedharan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3924/ ----------------------------------------------------------- (Updated 2012-02-21 02:53:41) Review request for Flume. Summary ------- A class which takes in a LoggingEvent and sends it to a predefined AvroSource at a given host and port. This addresses bug FLUME-886 . https://issues.apache.org/jira/browse/FLUME-886 Diffs ----- branches/flume-728/flume-ng-clients/flume-ng-log4jappender/pom.xml PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/main/java/org/apache/flume/clients/log4jappender/Log4jAvroHeaders.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java PRE-CREATION branches/flume-728/flume-ng-clients/flume-ng-log4jappender/src/test/resources/flume-log4jtest.properties PRE-CREATION branches/flume-728/flume-ng-clients/pom.xml PRE-CREATION branches/flume-728/pom.xml 1244858 Diff: https://reviews.apache.org/r/3924/diff Testing ------- Added unit tests. Thanks, Hari

          People

          • Assignee:
            Hari Shreedharan
            Reporter:
            Jeff de Anda
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development