You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Jake Maes <ja...@gmail.com> on 2016/07/28 20:25:07 UTC
Review Request 50583: SAMZA-954 Improve logging for Samza
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50583/
-----------------------------------------------------------
Review request for samza, Boris Shkolnik, Chris Pettitt, Fred Ji, Jake Maes, Navina Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure).
Bugs: SAMZA-954
https://issues.apache.org/jira/browse/SAMZA-954
Repository: samza
Description
-------
SAMZA-954 Improve logging for Samza
Diffs
-----
docs/learn/documentation/versioned/jobs/logging.md 0726d37affa06f85e20fbd3bc2395c28f30159a8
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala a37f3536c45fdcb6d5410328f031b0263b71a82d
samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala ad00ca00f918df4d71d1c920b77027401a55c80b
samza-core/src/main/scala/org/apache/samza/util/Logging.scala 250de1e2fa103be1a426d9da31187c12dbff8678
samza-test/src/main/resources/log4j.xml 29345f39ecef6f9ec769bf9d8eaab239f34e5d1e
Diff: https://reviews.apache.org/r/50583/diff/
Testing
-------
Tested using hello-samza with the following log4j.xml
```
<!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
<log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/">
<appender name="jmx" class="org.apache.samza.logging.log4j.JmxAppender" />
<appender name="RollingAppender" class="org.apache.log4j.RollingFileAppender">
<param name="File" value="${samza.log.dir}/${samza.container.name}.log" />
<param name="MaxFileSize" value="256MB" />
<param name="MaxBackupIndex" value="20" />
<layout class="org.apache.log4j.PatternLayout">
<param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
</layout>
</appender>
<appender name="StartupAppender" class="org.apache.log4j.RollingFileAppender">
<param name="File" value="${samza.log.dir}/${samza.container.name}-startup.log" />
<param name="MaxFileSize" value="256MB" />
<param name="MaxBackupIndex" value="1" />
<layout class="org.apache.log4j.PatternLayout">
<param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
</layout>
</appender>
<root>
<priority value="info" />
<appender-ref ref="RollingAppender"/>
<appender-ref ref="jmx" />
</root>
<logger name="STARTUP_LOGGER" additivity="false">
<level value="info" />
<appender-ref ref="StartupAppender"/>
</logger>
</log4j:configuration>
```
Thanks,
Jake Maes
Re: Review Request 50583: SAMZA-954 Improve logging for Samza
Posted by Navina Ramesh <nr...@linkedin.com>.
> On July 28, 2016, 8:37 p.m., Navina Ramesh wrote:
> > samza-core/src/main/scala/org/apache/samza/util/Logging.scala, line 30
> > <https://reviews.apache.org/r/50583/diff/1/?file=1456824#file1456824line30>
> >
> > what happens if a logger by name "startupLoggerName" is not defined?
>
> Jake Maes wrote:
> It still works, but the few startup messages are logged twice; once in each category. I tested this by accident when I didn't have the hello-samza log4j.xml configured.
Ok Cool :)
- Navina
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50583/#review143994
-----------------------------------------------------------
On July 28, 2016, 8:41 p.m., Jake Maes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50583/
> -----------------------------------------------------------
>
> (Updated July 28, 2016, 8:41 p.m.)
>
>
> Review request for samza, Boris Shkolnik, Chris Pettitt, Fred Ji, Jake Maes, Navina Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure).
>
>
> Bugs: SAMZA-954
> https://issues.apache.org/jira/browse/SAMZA-954
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-954 Improve logging for Samza
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/jobs/logging.md 0726d37affa06f85e20fbd3bc2395c28f30159a8
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala a37f3536c45fdcb6d5410328f031b0263b71a82d
> samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala ad00ca00f918df4d71d1c920b77027401a55c80b
> samza-core/src/main/scala/org/apache/samza/util/Logging.scala 250de1e2fa103be1a426d9da31187c12dbff8678
> samza-test/src/main/resources/log4j.xml 29345f39ecef6f9ec769bf9d8eaab239f34e5d1e
>
> Diff: https://reviews.apache.org/r/50583/diff/
>
>
> Testing
> -------
>
> Tested using hello-samza with the following log4j.xml
>
> ```
> <!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
> <log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/">
> <appender name="jmx" class="org.apache.samza.logging.log4j.JmxAppender" />
>
> <appender name="RollingAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="20" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <appender name="StartupAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}-startup.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="1" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <root>
> <priority value="info" />
> <appender-ref ref="RollingAppender"/>
> <appender-ref ref="jmx" />
> </root>
> <logger name="STARTUP_LOGGER" additivity="false">
> <level value="info" />
> <appender-ref ref="StartupAppender"/>
> </logger>
>
> </log4j:configuration>
>
> ```
>
>
> Thanks,
>
> Jake Maes
>
>
Re: Review Request 50583: SAMZA-954 Improve logging for Samza
Posted by Jake Maes <ja...@gmail.com>.
> On July 28, 2016, 8:37 p.m., Navina Ramesh wrote:
> > samza-core/src/main/scala/org/apache/samza/util/Logging.scala, line 30
> > <https://reviews.apache.org/r/50583/diff/1/?file=1456824#file1456824line30>
> >
> > what happens if a logger by name "startupLoggerName" is not defined?
It still works, but the few startup messages are logged twice; once in each category. I tested this by accident when I didn't have the hello-samza log4j.xml configured.
- Jake
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50583/#review143994
-----------------------------------------------------------
On July 28, 2016, 8:41 p.m., Jake Maes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50583/
> -----------------------------------------------------------
>
> (Updated July 28, 2016, 8:41 p.m.)
>
>
> Review request for samza, Boris Shkolnik, Chris Pettitt, Fred Ji, Jake Maes, Navina Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure).
>
>
> Bugs: SAMZA-954
> https://issues.apache.org/jira/browse/SAMZA-954
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-954 Improve logging for Samza
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/jobs/logging.md 0726d37affa06f85e20fbd3bc2395c28f30159a8
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala a37f3536c45fdcb6d5410328f031b0263b71a82d
> samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala ad00ca00f918df4d71d1c920b77027401a55c80b
> samza-core/src/main/scala/org/apache/samza/util/Logging.scala 250de1e2fa103be1a426d9da31187c12dbff8678
> samza-test/src/main/resources/log4j.xml 29345f39ecef6f9ec769bf9d8eaab239f34e5d1e
>
> Diff: https://reviews.apache.org/r/50583/diff/
>
>
> Testing
> -------
>
> Tested using hello-samza with the following log4j.xml
>
> ```
> <!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
> <log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/">
> <appender name="jmx" class="org.apache.samza.logging.log4j.JmxAppender" />
>
> <appender name="RollingAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="20" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <appender name="StartupAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}-startup.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="1" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <root>
> <priority value="info" />
> <appender-ref ref="RollingAppender"/>
> <appender-ref ref="jmx" />
> </root>
> <logger name="STARTUP_LOGGER" additivity="false">
> <level value="info" />
> <appender-ref ref="StartupAppender"/>
> </logger>
>
> </log4j:configuration>
>
> ```
>
>
> Thanks,
>
> Jake Maes
>
>
Re: Review Request 50583: SAMZA-954 Improve logging for Samza
Posted by Navina Ramesh <nr...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50583/#review143994
-----------------------------------------------------------
samza-core/src/main/scala/org/apache/samza/util/Logging.scala (line 29)
<https://reviews.apache.org/r/50583/#comment209928>
what happens if a logger by name "startupLoggerName" is not defined?
- Navina Ramesh
On July 28, 2016, 8:25 p.m., Jake Maes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50583/
> -----------------------------------------------------------
>
> (Updated July 28, 2016, 8:25 p.m.)
>
>
> Review request for samza, Boris Shkolnik, Chris Pettitt, Fred Ji, Jake Maes, Navina Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure).
>
>
> Bugs: SAMZA-954
> https://issues.apache.org/jira/browse/SAMZA-954
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-954 Improve logging for Samza
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/jobs/logging.md 0726d37affa06f85e20fbd3bc2395c28f30159a8
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala a37f3536c45fdcb6d5410328f031b0263b71a82d
> samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala ad00ca00f918df4d71d1c920b77027401a55c80b
> samza-core/src/main/scala/org/apache/samza/util/Logging.scala 250de1e2fa103be1a426d9da31187c12dbff8678
> samza-test/src/main/resources/log4j.xml 29345f39ecef6f9ec769bf9d8eaab239f34e5d1e
>
> Diff: https://reviews.apache.org/r/50583/diff/
>
>
> Testing
> -------
>
> Tested using hello-samza with the following log4j.xml
>
> ```
> <!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
> <log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/">
> <appender name="jmx" class="org.apache.samza.logging.log4j.JmxAppender" />
>
> <appender name="RollingAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="20" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <appender name="StartupAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}-startup.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="1" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <root>
> <priority value="info" />
> <appender-ref ref="RollingAppender"/>
> <appender-ref ref="jmx" />
> </root>
> <logger name="STARTUP_LOGGER" additivity="false">
> <level value="info" />
> <appender-ref ref="StartupAppender"/>
> </logger>
>
> </log4j:configuration>
>
> ```
>
>
> Thanks,
>
> Jake Maes
>
>
Re: Review Request 50583: SAMZA-954 Improve logging for Samza
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50583/#review144003
-----------------------------------------------------------
Ship it!
Ship It!
- Yi Pan (Data Infrastructure)
On July 28, 2016, 8:41 p.m., Jake Maes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50583/
> -----------------------------------------------------------
>
> (Updated July 28, 2016, 8:41 p.m.)
>
>
> Review request for samza, Boris Shkolnik, Chris Pettitt, Fred Ji, Jake Maes, Navina Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure).
>
>
> Bugs: SAMZA-954
> https://issues.apache.org/jira/browse/SAMZA-954
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-954 Improve logging for Samza
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/jobs/logging.md 0726d37affa06f85e20fbd3bc2395c28f30159a8
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala a37f3536c45fdcb6d5410328f031b0263b71a82d
> samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala ad00ca00f918df4d71d1c920b77027401a55c80b
> samza-core/src/main/scala/org/apache/samza/util/Logging.scala 250de1e2fa103be1a426d9da31187c12dbff8678
> samza-test/src/main/resources/log4j.xml 29345f39ecef6f9ec769bf9d8eaab239f34e5d1e
>
> Diff: https://reviews.apache.org/r/50583/diff/
>
>
> Testing
> -------
>
> Tested using hello-samza with the following log4j.xml
>
> ```
> <!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
> <log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/">
> <appender name="jmx" class="org.apache.samza.logging.log4j.JmxAppender" />
>
> <appender name="RollingAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="20" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <appender name="StartupAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}-startup.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="1" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <root>
> <priority value="info" />
> <appender-ref ref="RollingAppender"/>
> <appender-ref ref="jmx" />
> </root>
> <logger name="STARTUP_LOGGER" additivity="false">
> <level value="info" />
> <appender-ref ref="StartupAppender"/>
> </logger>
>
> </log4j:configuration>
>
> ```
>
>
> Thanks,
>
> Jake Maes
>
>
Re: Review Request 50583: SAMZA-954 Improve logging for Samza
Posted by Jake Maes <ja...@gmail.com>.
> On July 28, 2016, 8:42 p.m., Xinyu Liu wrote:
> > samza-core/src/main/scala/org/apache/samza/util/Logging.scala, line 32
> > <https://reviews.apache.org/r/50583/diff/1/?file=1456824#file1456824line32>
> >
> > Nit: this method does info log which is not very straghtforward from the name, and also the name "Log" is not consistent with the other methods. Do you think it's better to have:
> >
> > Logging.info((message, startupLog:boolean=false) => Any)
> >
> > That way we can use info log for both startup and non startup logging, and potentially adding it for debug and error in the future too.
I had similar feedback in the original review: https://reviews.apache.org/r/47992/
I decided not to change it because I couldn't forsee a case where startup logs would need to be other levels. e.g. You wouldn't log configuration at trace/error level. The only exception is WARN, but even then the startup log should be small enough to not warrant the separate level. So it didn't seem to be worth making the code more complicated.
- Jake
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50583/#review143998
-----------------------------------------------------------
On July 28, 2016, 8:41 p.m., Jake Maes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50583/
> -----------------------------------------------------------
>
> (Updated July 28, 2016, 8:41 p.m.)
>
>
> Review request for samza, Boris Shkolnik, Chris Pettitt, Fred Ji, Jake Maes, Navina Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure).
>
>
> Bugs: SAMZA-954
> https://issues.apache.org/jira/browse/SAMZA-954
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-954 Improve logging for Samza
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/jobs/logging.md 0726d37affa06f85e20fbd3bc2395c28f30159a8
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala a37f3536c45fdcb6d5410328f031b0263b71a82d
> samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala ad00ca00f918df4d71d1c920b77027401a55c80b
> samza-core/src/main/scala/org/apache/samza/util/Logging.scala 250de1e2fa103be1a426d9da31187c12dbff8678
> samza-test/src/main/resources/log4j.xml 29345f39ecef6f9ec769bf9d8eaab239f34e5d1e
>
> Diff: https://reviews.apache.org/r/50583/diff/
>
>
> Testing
> -------
>
> Tested using hello-samza with the following log4j.xml
>
> ```
> <!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
> <log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/">
> <appender name="jmx" class="org.apache.samza.logging.log4j.JmxAppender" />
>
> <appender name="RollingAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="20" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <appender name="StartupAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}-startup.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="1" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <root>
> <priority value="info" />
> <appender-ref ref="RollingAppender"/>
> <appender-ref ref="jmx" />
> </root>
> <logger name="STARTUP_LOGGER" additivity="false">
> <level value="info" />
> <appender-ref ref="StartupAppender"/>
> </logger>
>
> </log4j:configuration>
>
> ```
>
>
> Thanks,
>
> Jake Maes
>
>
Re: Review Request 50583: SAMZA-954 Improve logging for Samza
Posted by Xinyu Liu <xi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50583/#review143998
-----------------------------------------------------------
Ship it!
One minor suggestion below.
samza-core/src/main/scala/org/apache/samza/util/Logging.scala (line 31)
<https://reviews.apache.org/r/50583/#comment209931>
Nit: this method does info log which is not very straghtforward from the name, and also the name "Log" is not consistent with the other methods. Do you think it's better to have:
Logging.info((message, startupLog:boolean=false) => Any)
That way we can use info log for both startup and non startup logging, and potentially adding it for debug and error in the future too.
- Xinyu Liu
On July 28, 2016, 8:41 p.m., Jake Maes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50583/
> -----------------------------------------------------------
>
> (Updated July 28, 2016, 8:41 p.m.)
>
>
> Review request for samza, Boris Shkolnik, Chris Pettitt, Fred Ji, Jake Maes, Navina Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure).
>
>
> Bugs: SAMZA-954
> https://issues.apache.org/jira/browse/SAMZA-954
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-954 Improve logging for Samza
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/jobs/logging.md 0726d37affa06f85e20fbd3bc2395c28f30159a8
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala a37f3536c45fdcb6d5410328f031b0263b71a82d
> samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala ad00ca00f918df4d71d1c920b77027401a55c80b
> samza-core/src/main/scala/org/apache/samza/util/Logging.scala 250de1e2fa103be1a426d9da31187c12dbff8678
> samza-test/src/main/resources/log4j.xml 29345f39ecef6f9ec769bf9d8eaab239f34e5d1e
>
> Diff: https://reviews.apache.org/r/50583/diff/
>
>
> Testing
> -------
>
> Tested using hello-samza with the following log4j.xml
>
> ```
> <!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
> <log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/">
> <appender name="jmx" class="org.apache.samza.logging.log4j.JmxAppender" />
>
> <appender name="RollingAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="20" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <appender name="StartupAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}-startup.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="1" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <root>
> <priority value="info" />
> <appender-ref ref="RollingAppender"/>
> <appender-ref ref="jmx" />
> </root>
> <logger name="STARTUP_LOGGER" additivity="false">
> <level value="info" />
> <appender-ref ref="StartupAppender"/>
> </logger>
>
> </log4j:configuration>
>
> ```
>
>
> Thanks,
>
> Jake Maes
>
>
Re: Review Request 50583: SAMZA-954 Improve logging for Samza
Posted by "Yi Pan (Data Infrastructure)" <yi...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50583/#review144369
-----------------------------------------------------------
Ship it!
Ship It!
- Yi Pan (Data Infrastructure)
On July 28, 2016, 8:41 p.m., Jake Maes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50583/
> -----------------------------------------------------------
>
> (Updated July 28, 2016, 8:41 p.m.)
>
>
> Review request for samza, Boris Shkolnik, Chris Pettitt, Fred Ji, Jake Maes, Navina Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure).
>
>
> Bugs: SAMZA-954
> https://issues.apache.org/jira/browse/SAMZA-954
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-954 Improve logging for Samza
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/jobs/logging.md 0726d37affa06f85e20fbd3bc2395c28f30159a8
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala a37f3536c45fdcb6d5410328f031b0263b71a82d
> samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala ad00ca00f918df4d71d1c920b77027401a55c80b
> samza-core/src/main/scala/org/apache/samza/util/Logging.scala 250de1e2fa103be1a426d9da31187c12dbff8678
> samza-test/src/main/resources/log4j.xml 29345f39ecef6f9ec769bf9d8eaab239f34e5d1e
>
> Diff: https://reviews.apache.org/r/50583/diff/
>
>
> Testing
> -------
>
> Tested using hello-samza with the following log4j.xml
>
> ```
> <!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
> <log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/">
> <appender name="jmx" class="org.apache.samza.logging.log4j.JmxAppender" />
>
> <appender name="RollingAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="20" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <appender name="StartupAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}-startup.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="1" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <root>
> <priority value="info" />
> <appender-ref ref="RollingAppender"/>
> <appender-ref ref="jmx" />
> </root>
> <logger name="STARTUP_LOGGER" additivity="false">
> <level value="info" />
> <appender-ref ref="StartupAppender"/>
> </logger>
>
> </log4j:configuration>
>
> ```
>
>
> Thanks,
>
> Jake Maes
>
>
Re: Review Request 50583: SAMZA-954 Improve logging for Samza
Posted by Boris Shkolnik <bo...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50583/#review146007
-----------------------------------------------------------
Ship it!
samza-core/src/main/scala/org/apache/samza/util/Logging.scala (line 31)
<https://reviews.apache.org/r/50583/#comment212368>
Suggestion: what if we need a message at a WARN level. May be you can have another parameter 'level' with the default set to INFO.
- Boris Shkolnik
On July 28, 2016, 8:41 p.m., Jake Maes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50583/
> -----------------------------------------------------------
>
> (Updated July 28, 2016, 8:41 p.m.)
>
>
> Review request for samza, Boris Shkolnik, Chris Pettitt, Fred Ji, Jake Maes, Navina Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure).
>
>
> Bugs: SAMZA-954
> https://issues.apache.org/jira/browse/SAMZA-954
>
>
> Repository: samza
>
>
> Description
> -------
>
> SAMZA-954 Improve logging for Samza
>
>
> Diffs
> -----
>
> docs/learn/documentation/versioned/jobs/logging.md 0726d37affa06f85e20fbd3bc2395c28f30159a8
> samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala a37f3536c45fdcb6d5410328f031b0263b71a82d
> samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala ad00ca00f918df4d71d1c920b77027401a55c80b
> samza-core/src/main/scala/org/apache/samza/util/Logging.scala 250de1e2fa103be1a426d9da31187c12dbff8678
> samza-test/src/main/resources/log4j.xml 29345f39ecef6f9ec769bf9d8eaab239f34e5d1e
>
> Diff: https://reviews.apache.org/r/50583/diff/
>
>
> Testing
> -------
>
> Tested using hello-samza with the following log4j.xml
>
> ```
> <!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
> <log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/">
> <appender name="jmx" class="org.apache.samza.logging.log4j.JmxAppender" />
>
> <appender name="RollingAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="20" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <appender name="StartupAppender" class="org.apache.log4j.RollingFileAppender">
> <param name="File" value="${samza.log.dir}/${samza.container.name}-startup.log" />
> <param name="MaxFileSize" value="256MB" />
> <param name="MaxBackupIndex" value="1" />
> <layout class="org.apache.log4j.PatternLayout">
> <param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
> </layout>
> </appender>
> <root>
> <priority value="info" />
> <appender-ref ref="RollingAppender"/>
> <appender-ref ref="jmx" />
> </root>
> <logger name="STARTUP_LOGGER" additivity="false">
> <level value="info" />
> <appender-ref ref="StartupAppender"/>
> </logger>
>
> </log4j:configuration>
>
> ```
>
>
> Thanks,
>
> Jake Maes
>
>
Re: Review Request 50583: SAMZA-954 Improve logging for Samza
Posted by Jake Maes <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50583/
-----------------------------------------------------------
(Updated July 28, 2016, 8:41 p.m.)
Review request for samza, Boris Shkolnik, Chris Pettitt, Fred Ji, Jake Maes, Navina Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure).
Bugs: SAMZA-954
https://issues.apache.org/jira/browse/SAMZA-954
Repository: samza
Description
-------
SAMZA-954 Improve logging for Samza
Diffs (updated)
-----
docs/learn/documentation/versioned/jobs/logging.md 0726d37affa06f85e20fbd3bc2395c28f30159a8
samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala a37f3536c45fdcb6d5410328f031b0263b71a82d
samza-core/src/main/scala/org/apache/samza/metrics/JmxServer.scala ad00ca00f918df4d71d1c920b77027401a55c80b
samza-core/src/main/scala/org/apache/samza/util/Logging.scala 250de1e2fa103be1a426d9da31187c12dbff8678
samza-test/src/main/resources/log4j.xml 29345f39ecef6f9ec769bf9d8eaab239f34e5d1e
Diff: https://reviews.apache.org/r/50583/diff/
Testing
-------
Tested using hello-samza with the following log4j.xml
```
<!DOCTYPE log4j:configuration SYSTEM "log4j.dtd">
<log4j:configuration xmlns:log4j="http://jakarta.apache.org/log4j/">
<appender name="jmx" class="org.apache.samza.logging.log4j.JmxAppender" />
<appender name="RollingAppender" class="org.apache.log4j.RollingFileAppender">
<param name="File" value="${samza.log.dir}/${samza.container.name}.log" />
<param name="MaxFileSize" value="256MB" />
<param name="MaxBackupIndex" value="20" />
<layout class="org.apache.log4j.PatternLayout">
<param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
</layout>
</appender>
<appender name="StartupAppender" class="org.apache.log4j.RollingFileAppender">
<param name="File" value="${samza.log.dir}/${samza.container.name}-startup.log" />
<param name="MaxFileSize" value="256MB" />
<param name="MaxBackupIndex" value="1" />
<layout class="org.apache.log4j.PatternLayout">
<param name="ConversionPattern" value="%d{yyyy-MM-dd HH:mm:ss.SSS} [%t] %c{1} [%p] %m%n" />
</layout>
</appender>
<root>
<priority value="info" />
<appender-ref ref="RollingAppender"/>
<appender-ref ref="jmx" />
</root>
<logger name="STARTUP_LOGGER" additivity="false">
<level value="info" />
<appender-ref ref="StartupAppender"/>
</logger>
</log4j:configuration>
```
Thanks,
Jake Maes