You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@samza.apache.org by Yan Fang <ya...@gmail.com> on 2014/06/21 00:19:27 UTC
Review Request 22845: SAMZA-215: Better logging for interactive command-line
tools
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22845/
-----------------------------------------------------------
Review request for samza.
Repository: samza
Description
-------
1. added an environment variable SAMZA_LOG4J_FILE in each script. and run-class.sh use that variable for log4j configuration instead of directly pointing to the position of log4j.xml.
2. added log4j-default.xml, which append the logs to console. run-job, kill-yarn-job and checkpoint scripts use this configuration.
3. changed the build.gradle to contain the new log4j-default.xml
Diffs
-----
build.gradle 1bbd5ca
samza-shell/src/main/bash/checkpoint-tool.sh 1a455df
samza-shell/src/main/bash/kill-yarn-job.sh 924aac6
samza-shell/src/main/bash/run-am.sh c202596
samza-shell/src/main/bash/run-class.sh cc341b7
samza-shell/src/main/bash/run-container.sh 72cee18
samza-shell/src/main/bash/run-job.sh 0605994
samza-shell/src/main/resources/log4j-default.xml PRE-CREATION
Diff: https://reviews.apache.org/r/22845/diff/
Testing
-------
Thanks,
Yan Fang
Re: Review Request 22845: SAMZA-215: Better logging for interactive
command-line tools
Posted by Yan Fang <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22845/
-----------------------------------------------------------
(Updated June 26, 2014, 4:52 p.m.)
Review request for samza.
Changes
-------
1. changed from SAMZA_LOG4J_FILE to SAMZA_LOG4J_CONFIG
2. changed the log4j-default.xml to log4j-console.xml
3. changed the dependencies in gradle.build for samza-shell and added log4j-console.xml as the log4j configuration for samza-shell. So now ./gradlew uses the same log4j configuration as the deploy/samza/bin/
Thank you.
Repository: samza
Description
-------
1. added an environment variable SAMZA_LOG4J_FILE in each script. and run-class.sh use that variable for log4j configuration instead of directly pointing to the position of log4j.xml.
2. added log4j-default.xml, which append the logs to console. run-job, kill-yarn-job and checkpoint scripts use this configuration.
3. changed the build.gradle to contain the new log4j-default.xml
Diffs (updated)
-----
build.gradle f728330
samza-shell/src/main/bash/checkpoint-tool.sh 1a455df
samza-shell/src/main/bash/kill-yarn-job.sh 924aac6
samza-shell/src/main/bash/run-am.sh c202596
samza-shell/src/main/bash/run-class.sh cc341b7
samza-shell/src/main/bash/run-container.sh 72cee18
samza-shell/src/main/bash/run-job.sh 0605994
samza-shell/src/main/resources/log4j-console.xml PRE-CREATION
Diff: https://reviews.apache.org/r/22845/diff/
Testing
-------
Thanks,
Yan Fang
Re: Review Request 22845: SAMZA-215: Better logging for interactive
command-line tools
Posted by Martin Kleppmann <mk...@linkedin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22845/#review46549
-----------------------------------------------------------
samza-shell/src/main/bash/run-class.sh
<https://reviews.apache.org/r/22845/#comment81991>
When I read the name "SAMZA_LOG4J_FILE", I first thought this was going to be the file that logs are written to, not the file containing the configuration. Could we call it SAMZA_LOG4J_CONFIG instead?
Also, just wanted to point out that we've lost the check for whether the config file exists. I don't think that's a problem (log4j will be unconfigured either way).
samza-shell/src/main/bash/run-job.sh
<https://reviews.apache.org/r/22845/#comment81993>
Perhaps call it log4j-console.xml? "default" is kinda generic and unspecific.
This log4j config gets used when a tool is invoked through the shell script, but not when invoked through Gradle (e.g. `./gradlew samza-shell:checkpointTool`). The gradle run classpath currently includes slf4j-simple, which also logs to the console, but with different formatting. Do you think you might be able to make the Gradle tasks use the same logging config as the shell scripts?
samza-shell/src/main/resources/log4j-default.xml
<https://reviews.apache.org/r/22845/#comment82023>
Irregular indentation (5 spaces)
- Martin Kleppmann
On June 20, 2014, 10:19 p.m., Yan Fang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22845/
> -----------------------------------------------------------
>
> (Updated June 20, 2014, 10:19 p.m.)
>
>
> Review request for samza.
>
>
> Repository: samza
>
>
> Description
> -------
>
> 1. added an environment variable SAMZA_LOG4J_FILE in each script. and run-class.sh use that variable for log4j configuration instead of directly pointing to the position of log4j.xml.
> 2. added log4j-default.xml, which append the logs to console. run-job, kill-yarn-job and checkpoint scripts use this configuration.
> 3. changed the build.gradle to contain the new log4j-default.xml
>
>
> Diffs
> -----
>
> build.gradle 1bbd5ca
> samza-shell/src/main/bash/checkpoint-tool.sh 1a455df
> samza-shell/src/main/bash/kill-yarn-job.sh 924aac6
> samza-shell/src/main/bash/run-am.sh c202596
> samza-shell/src/main/bash/run-class.sh cc341b7
> samza-shell/src/main/bash/run-container.sh 72cee18
> samza-shell/src/main/bash/run-job.sh 0605994
> samza-shell/src/main/resources/log4j-default.xml PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22845/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Yan Fang
>
>