Hi all!
I see that a lot of people are committing log4j.properties files in the compile scope of various projects. By now, when you start a flink application, you have multiple log4j.properties files in the classpath. ./flink-examples/flink-java-examples/src/main/resources/log4j.properties ./flink-runtime/src/main/resources/log4j.properties ./flink-staging/flink-tez/src/main/resources/log4j.properties That is certainly not how it should be. There should be none in there, it messes with all assumptions considering log configurations. For tests, there are even more log config files in the classpath ./flink-staging/flink-streaming/flink-streaming-core/src/test/resources/log4j.properties ./flink-staging/flink-hbase/src/test/resources/log4j.properties ./flink-staging/flink-tachyon/src/test/resources/log4j.properties Please remove these files and pay attention to not commit these files, they make it hard for anyone to actually want to debug based on log output to configure the logging. Greetings, Stephan |
I agree that we are currently in a bad situation with the log4j.properties
files. They should certainly not be in any JAR file we deploy to maven central. On the other hand, not having any log4j configuration files with default level set to INFO is costing me a lot of time and energy. Every time I want to run an example, some tests or start the Jobmanager out of the IDE, I have to copy a configuration file from somewhere or set the log level. Therefore, I propose to add a "log4j.properties" file to every maven module (in main, not test) with the log level set in INFO and exclude them using the maven-shade-plugin (we need to add the exclude only in the "flink-parent" and "flink-dist"). I would remove all the log4j-test.properties (it should not be an issue to log on INFO locally and for travis, we are using a different log4j configuration anyways). On Mon, Apr 6, 2015 at 9:01 PM, Stephan Ewen <[hidden email]> wrote: > Hi all! > > I see that a lot of people are committing log4j.properties files in the > compile scope of various projects. By now, when you start a flink > application, you have multiple log4j.properties files in the classpath. > > ./flink-examples/flink-java-examples/src/main/resources/log4j.properties > ./flink-runtime/src/main/resources/log4j.properties > ./flink-staging/flink-tez/src/main/resources/log4j.properties > > That is certainly not how it should be. There should be none in there, it > messes with all assumptions considering log configurations. > > For tests, there are even more log config files in the classpath > > > ./flink-staging/flink-streaming/flink-streaming-core/src/test/resources/log4j.properties > ./flink-staging/flink-hbase/src/test/resources/log4j.properties > ./flink-staging/flink-tachyon/src/test/resources/log4j.properties > > Please remove these files and pay attention to not commit these files, they > make it hard for anyone to actually want to debug based on log output to > configure the logging. > > Greetings, > Stephan > |
+1 for Robert's suggestion.
On Fri, Apr 10, 2015 at 2:49 PM, Robert Metzger <[hidden email]> wrote: > I agree that we are currently in a bad situation with the log4j.properties > files. > They should certainly not be in any JAR file we deploy to maven central. > > On the other hand, not having any log4j configuration files with default > level set to INFO is costing me a lot of time and energy. Every time I want > to run an example, some tests or start the Jobmanager out of the IDE, I > have to copy a configuration file from somewhere or set the log level. > > > Therefore, I propose to add a "log4j.properties" file to every maven module > (in main, not test) with the log level set in INFO and exclude them using > the maven-shade-plugin (we need to add the exclude only in the > "flink-parent" and "flink-dist"). I would remove all the > log4j-test.properties (it should not be an issue to log on INFO locally and > for travis, we are using a different log4j configuration anyways). > > On Mon, Apr 6, 2015 at 9:01 PM, Stephan Ewen <[hidden email]> wrote: > > > Hi all! > > > > I see that a lot of people are committing log4j.properties files in the > > compile scope of various projects. By now, when you start a flink > > application, you have multiple log4j.properties files in the classpath. > > > > ./flink-examples/flink-java-examples/src/main/resources/log4j.properties > > ./flink-runtime/src/main/resources/log4j.properties > > ./flink-staging/flink-tez/src/main/resources/log4j.properties > > > > That is certainly not how it should be. There should be none in there, it > > messes with all assumptions considering log configurations. > > > > For tests, there are even more log config files in the classpath > > > > > > > ./flink-staging/flink-streaming/flink-streaming-core/src/test/resources/log4j.properties > > ./flink-staging/flink-hbase/src/test/resources/log4j.properties > > ./flink-staging/flink-tachyon/src/test/resources/log4j.properties > > > > Please remove these files and pay attention to not commit these files, > they > > make it hard for anyone to actually want to debug based on log output to > > configure the logging. > > > > Greetings, > > Stephan > > > |
In reply to this post by Robert Metzger
I personally like not to have my build console swamped with log messages.
Otherwise the actually relevant stack traces disapprear almost instantly. I think that the current setup is not so bad, since all test pisk up the "src/test/resources/log4j-test.properties" anyways. If you want to add files to every project, why not add "template" files that you need to rename in order to enable log output? Stephan On Fri, Apr 10, 2015 at 2:49 PM, Robert Metzger <[hidden email]> wrote: > I agree that we are currently in a bad situation with the log4j.properties > files. > They should certainly not be in any JAR file we deploy to maven central. > > On the other hand, not having any log4j configuration files with default > level set to INFO is costing me a lot of time and energy. Every time I want > to run an example, some tests or start the Jobmanager out of the IDE, I > have to copy a configuration file from somewhere or set the log level. > > > Therefore, I propose to add a "log4j.properties" file to every maven module > (in main, not test) with the log level set in INFO and exclude them using > the maven-shade-plugin (we need to add the exclude only in the > "flink-parent" and "flink-dist"). I would remove all the > log4j-test.properties (it should not be an issue to log on INFO locally and > for travis, we are using a different log4j configuration anyways). > > On Mon, Apr 6, 2015 at 9:01 PM, Stephan Ewen <[hidden email]> wrote: > > > Hi all! > > > > I see that a lot of people are committing log4j.properties files in the > > compile scope of various projects. By now, when you start a flink > > application, you have multiple log4j.properties files in the classpath. > > > > ./flink-examples/flink-java-examples/src/main/resources/log4j.properties > > ./flink-runtime/src/main/resources/log4j.properties > > ./flink-staging/flink-tez/src/main/resources/log4j.properties > > > > That is certainly not how it should be. There should be none in there, it > > messes with all assumptions considering log configurations. > > > > For tests, there are even more log config files in the classpath > > > > > > > ./flink-staging/flink-streaming/flink-streaming-core/src/test/resources/log4j.properties > > ./flink-staging/flink-hbase/src/test/resources/log4j.properties > > ./flink-staging/flink-tachyon/src/test/resources/log4j.properties > > > > Please remove these files and pay attention to not commit these files, > they > > make it hard for anyone to actually want to debug based on log output to > > configure the logging. > > > > Greetings, > > Stephan > > > |
I created a pull request to exclude the log4j.properties files from all
jars: https://github.com/apache/flink/pull/586 Please give me +1's to merge it. Its actually a major issue that we have a log4j.properties file in our jars! (also for the milestone release). I committed the file to src/main of flink-runtime, thats why its ending up in the classpath. @Stephan: We can also keep the log4j-test.properties files to keep the console output clean. If you want to add files to every project, why not add "template" files > that you need to rename in order to enable log output? Because I still would have to rename the template when I want to test something. Also, it could still happen that somebody (like me) accidentally commits the renamed file. I don't see an issue with having a "log4j.properties" file with a reasonable default configuration for developer convenience. On Fri, Apr 10, 2015 at 3:12 PM, Stephan Ewen <[hidden email]> wrote: > I personally like not to have my build console swamped with log messages. > > Otherwise the actually relevant stack traces disapprear almost instantly. > > I think that the current setup is not so bad, since all test pisk up the > "src/test/resources/log4j-test.properties" anyways. > > If you want to add files to every project, why not add "template" files > that you need to rename in order to enable log output? > > > Stephan > > > On Fri, Apr 10, 2015 at 2:49 PM, Robert Metzger <[hidden email]> > wrote: > > > I agree that we are currently in a bad situation with the > log4j.properties > > files. > > They should certainly not be in any JAR file we deploy to maven central. > > > > On the other hand, not having any log4j configuration files with default > > level set to INFO is costing me a lot of time and energy. Every time I > want > > to run an example, some tests or start the Jobmanager out of the IDE, I > > have to copy a configuration file from somewhere or set the log level. > > > > > > Therefore, I propose to add a "log4j.properties" file to every maven > module > > (in main, not test) with the log level set in INFO and exclude them using > > the maven-shade-plugin (we need to add the exclude only in the > > "flink-parent" and "flink-dist"). I would remove all the > > log4j-test.properties (it should not be an issue to log on INFO locally > and > > for travis, we are using a different log4j configuration anyways). > > > > On Mon, Apr 6, 2015 at 9:01 PM, Stephan Ewen <[hidden email]> wrote: > > > > > Hi all! > > > > > > I see that a lot of people are committing log4j.properties files in the > > > compile scope of various projects. By now, when you start a flink > > > application, you have multiple log4j.properties files in the classpath. > > > > > > > ./flink-examples/flink-java-examples/src/main/resources/log4j.properties > > > ./flink-runtime/src/main/resources/log4j.properties > > > ./flink-staging/flink-tez/src/main/resources/log4j.properties > > > > > > That is certainly not how it should be. There should be none in there, > it > > > messes with all assumptions considering log configurations. > > > > > > For tests, there are even more log config files in the classpath > > > > > > > > > > > > ./flink-staging/flink-streaming/flink-streaming-core/src/test/resources/log4j.properties > > > ./flink-staging/flink-hbase/src/test/resources/log4j.properties > > > ./flink-staging/flink-tachyon/src/test/resources/log4j.properties > > > > > > Please remove these files and pay attention to not commit these files, > > they > > > make it hard for anyone to actually want to debug based on log output > to > > > configure the logging. > > > > > > Greetings, > > > Stephan > > > > > > |
I still think it is a problem if local builds spam the console. It makes
exception stack traces drop out of the history so fast that it becomes non-debuggable... On Fri, Apr 10, 2015 at 4:15 PM, Robert Metzger <[hidden email]> wrote: > I created a pull request to exclude the log4j.properties files from all > jars: https://github.com/apache/flink/pull/586 > > Please give me +1's to merge it. > > Its actually a major issue that we have a log4j.properties file in our > jars! (also for the milestone release). > I committed the file to src/main of flink-runtime, thats why its ending up > in the classpath. > > @Stephan: We can also keep the log4j-test.properties files to keep the > console output clean. > > If you want to add files to every project, why not add "template" files > > that you need to rename in order to enable log output? > > > Because I still would have to rename the template when I want to test > something. Also, it could still happen that somebody (like me) accidentally > commits the renamed file. > > I don't see an issue with having a "log4j.properties" file with a > reasonable default configuration for developer convenience. > > > On Fri, Apr 10, 2015 at 3:12 PM, Stephan Ewen <[hidden email]> wrote: > > > I personally like not to have my build console swamped with log messages. > > > > Otherwise the actually relevant stack traces disapprear almost instantly. > > > > I think that the current setup is not so bad, since all test pisk up the > > "src/test/resources/log4j-test.properties" anyways. > > > > If you want to add files to every project, why not add "template" files > > that you need to rename in order to enable log output? > > > > > > Stephan > > > > > > On Fri, Apr 10, 2015 at 2:49 PM, Robert Metzger <[hidden email]> > > wrote: > > > > > I agree that we are currently in a bad situation with the > > log4j.properties > > > files. > > > They should certainly not be in any JAR file we deploy to maven > central. > > > > > > On the other hand, not having any log4j configuration files with > default > > > level set to INFO is costing me a lot of time and energy. Every time I > > want > > > to run an example, some tests or start the Jobmanager out of the IDE, I > > > have to copy a configuration file from somewhere or set the log level. > > > > > > > > > Therefore, I propose to add a "log4j.properties" file to every maven > > module > > > (in main, not test) with the log level set in INFO and exclude them > using > > > the maven-shade-plugin (we need to add the exclude only in the > > > "flink-parent" and "flink-dist"). I would remove all the > > > log4j-test.properties (it should not be an issue to log on INFO locally > > and > > > for travis, we are using a different log4j configuration anyways). > > > > > > On Mon, Apr 6, 2015 at 9:01 PM, Stephan Ewen <[hidden email]> wrote: > > > > > > > Hi all! > > > > > > > > I see that a lot of people are committing log4j.properties files in > the > > > > compile scope of various projects. By now, when you start a flink > > > > application, you have multiple log4j.properties files in the > classpath. > > > > > > > > > > ./flink-examples/flink-java-examples/src/main/resources/log4j.properties > > > > ./flink-runtime/src/main/resources/log4j.properties > > > > ./flink-staging/flink-tez/src/main/resources/log4j.properties > > > > > > > > That is certainly not how it should be. There should be none in > there, > > it > > > > messes with all assumptions considering log configurations. > > > > > > > > For tests, there are even more log config files in the classpath > > > > > > > > > > > > > > > > > > ./flink-staging/flink-streaming/flink-streaming-core/src/test/resources/log4j.properties > > > > ./flink-staging/flink-hbase/src/test/resources/log4j.properties > > > > ./flink-staging/flink-tachyon/src/test/resources/log4j.properties > > > > > > > > Please remove these files and pay attention to not commit these > files, > > > they > > > > make it hard for anyone to actually want to debug based on log output > > to > > > > configure the logging. > > > > > > > > Greetings, > > > > Stephan > > > > > > > > > > |
Maybe there is a misunderstanding. "local builds" is executing "mvn verify"
locally for you? Having a log4j.properties file in src/main/resources with log level INFO and a log4j-test.properties file in src/test/resources with log level OFF will lead to "mvn verify" without any flink log messages. Running tests/examples/jobmanagers from the IDE will log with INFO. On Fri, Apr 10, 2015 at 5:34 PM, Stephan Ewen <[hidden email]> wrote: > I still think it is a problem if local builds spam the console. It makes > exception stack traces drop out of the history so fast that it becomes > non-debuggable... > > On Fri, Apr 10, 2015 at 4:15 PM, Robert Metzger <[hidden email]> > wrote: > > > I created a pull request to exclude the log4j.properties files from all > > jars: https://github.com/apache/flink/pull/586 > > > > Please give me +1's to merge it. > > > > Its actually a major issue that we have a log4j.properties file in our > > jars! (also for the milestone release). > > I committed the file to src/main of flink-runtime, thats why its ending > up > > in the classpath. > > > > @Stephan: We can also keep the log4j-test.properties files to keep the > > console output clean. > > > > If you want to add files to every project, why not add "template" files > > > that you need to rename in order to enable log output? > > > > > > Because I still would have to rename the template when I want to test > > something. Also, it could still happen that somebody (like me) > accidentally > > commits the renamed file. > > > > I don't see an issue with having a "log4j.properties" file with a > > reasonable default configuration for developer convenience. > > > > > > On Fri, Apr 10, 2015 at 3:12 PM, Stephan Ewen <[hidden email]> wrote: > > > > > I personally like not to have my build console swamped with log > messages. > > > > > > Otherwise the actually relevant stack traces disapprear almost > instantly. > > > > > > I think that the current setup is not so bad, since all test pisk up > the > > > "src/test/resources/log4j-test.properties" anyways. > > > > > > If you want to add files to every project, why not add "template" files > > > that you need to rename in order to enable log output? > > > > > > > > > Stephan > > > > > > > > > On Fri, Apr 10, 2015 at 2:49 PM, Robert Metzger <[hidden email]> > > > wrote: > > > > > > > I agree that we are currently in a bad situation with the > > > log4j.properties > > > > files. > > > > They should certainly not be in any JAR file we deploy to maven > > central. > > > > > > > > On the other hand, not having any log4j configuration files with > > default > > > > level set to INFO is costing me a lot of time and energy. Every time > I > > > want > > > > to run an example, some tests or start the Jobmanager out of the > IDE, I > > > > have to copy a configuration file from somewhere or set the log > level. > > > > > > > > > > > > Therefore, I propose to add a "log4j.properties" file to every maven > > > module > > > > (in main, not test) with the log level set in INFO and exclude them > > using > > > > the maven-shade-plugin (we need to add the exclude only in the > > > > "flink-parent" and "flink-dist"). I would remove all the > > > > log4j-test.properties (it should not be an issue to log on INFO > locally > > > and > > > > for travis, we are using a different log4j configuration anyways). > > > > > > > > On Mon, Apr 6, 2015 at 9:01 PM, Stephan Ewen <[hidden email]> > wrote: > > > > > > > > > Hi all! > > > > > > > > > > I see that a lot of people are committing log4j.properties files in > > the > > > > > compile scope of various projects. By now, when you start a flink > > > > > application, you have multiple log4j.properties files in the > > classpath. > > > > > > > > > > > > > > ./flink-examples/flink-java-examples/src/main/resources/log4j.properties > > > > > ./flink-runtime/src/main/resources/log4j.properties > > > > > ./flink-staging/flink-tez/src/main/resources/log4j.properties > > > > > > > > > > That is certainly not how it should be. There should be none in > > there, > > > it > > > > > messes with all assumptions considering log configurations. > > > > > > > > > > For tests, there are even more log config files in the classpath > > > > > > > > > > > > > > > > > > > > > > > > > ./flink-staging/flink-streaming/flink-streaming-core/src/test/resources/log4j.properties > > > > > ./flink-staging/flink-hbase/src/test/resources/log4j.properties > > > > > ./flink-staging/flink-tachyon/src/test/resources/log4j.properties > > > > > > > > > > Please remove these files and pay attention to not commit these > > files, > > > > they > > > > > make it hard for anyone to actually want to debug based on log > output > > > to > > > > > configure the logging. > > > > > > > > > > Greetings, > > > > > Stephan > > > > > > > > > > > > > > > |
Ah, okay. That seems good.
On Fri, Apr 10, 2015 at 5:38 PM, Robert Metzger <[hidden email]> wrote: > Maybe there is a misunderstanding. "local builds" is executing "mvn verify" > locally for you? > > Having a log4j.properties file in src/main/resources with log level INFO > and a log4j-test.properties file in src/test/resources with log level OFF > will lead to "mvn verify" without any flink log messages. Running > tests/examples/jobmanagers from the IDE will log with INFO. > > On Fri, Apr 10, 2015 at 5:34 PM, Stephan Ewen <[hidden email]> wrote: > > > I still think it is a problem if local builds spam the console. It makes > > exception stack traces drop out of the history so fast that it becomes > > non-debuggable... > > > > On Fri, Apr 10, 2015 at 4:15 PM, Robert Metzger <[hidden email]> > > wrote: > > > > > I created a pull request to exclude the log4j.properties files from all > > > jars: https://github.com/apache/flink/pull/586 > > > > > > Please give me +1's to merge it. > > > > > > Its actually a major issue that we have a log4j.properties file in our > > > jars! (also for the milestone release). > > > I committed the file to src/main of flink-runtime, thats why its ending > > up > > > in the classpath. > > > > > > @Stephan: We can also keep the log4j-test.properties files to keep the > > > console output clean. > > > > > > If you want to add files to every project, why not add "template" files > > > > that you need to rename in order to enable log output? > > > > > > > > > Because I still would have to rename the template when I want to test > > > something. Also, it could still happen that somebody (like me) > > accidentally > > > commits the renamed file. > > > > > > I don't see an issue with having a "log4j.properties" file with a > > > reasonable default configuration for developer convenience. > > > > > > > > > On Fri, Apr 10, 2015 at 3:12 PM, Stephan Ewen <[hidden email]> > wrote: > > > > > > > I personally like not to have my build console swamped with log > > messages. > > > > > > > > Otherwise the actually relevant stack traces disapprear almost > > instantly. > > > > > > > > I think that the current setup is not so bad, since all test pisk up > > the > > > > "src/test/resources/log4j-test.properties" anyways. > > > > > > > > If you want to add files to every project, why not add "template" > files > > > > that you need to rename in order to enable log output? > > > > > > > > > > > > Stephan > > > > > > > > > > > > On Fri, Apr 10, 2015 at 2:49 PM, Robert Metzger <[hidden email] > > > > > > wrote: > > > > > > > > > I agree that we are currently in a bad situation with the > > > > log4j.properties > > > > > files. > > > > > They should certainly not be in any JAR file we deploy to maven > > > central. > > > > > > > > > > On the other hand, not having any log4j configuration files with > > > default > > > > > level set to INFO is costing me a lot of time and energy. Every > time > > I > > > > want > > > > > to run an example, some tests or start the Jobmanager out of the > > IDE, I > > > > > have to copy a configuration file from somewhere or set the log > > level. > > > > > > > > > > > > > > > Therefore, I propose to add a "log4j.properties" file to every > maven > > > > module > > > > > (in main, not test) with the log level set in INFO and exclude them > > > using > > > > > the maven-shade-plugin (we need to add the exclude only in the > > > > > "flink-parent" and "flink-dist"). I would remove all the > > > > > log4j-test.properties (it should not be an issue to log on INFO > > locally > > > > and > > > > > for travis, we are using a different log4j configuration anyways). > > > > > > > > > > On Mon, Apr 6, 2015 at 9:01 PM, Stephan Ewen <[hidden email]> > > wrote: > > > > > > > > > > > Hi all! > > > > > > > > > > > > I see that a lot of people are committing log4j.properties files > in > > > the > > > > > > compile scope of various projects. By now, when you start a flink > > > > > > application, you have multiple log4j.properties files in the > > > classpath. > > > > > > > > > > > > > > > > > > ./flink-examples/flink-java-examples/src/main/resources/log4j.properties > > > > > > ./flink-runtime/src/main/resources/log4j.properties > > > > > > ./flink-staging/flink-tez/src/main/resources/log4j.properties > > > > > > > > > > > > That is certainly not how it should be. There should be none in > > > there, > > > > it > > > > > > messes with all assumptions considering log configurations. > > > > > > > > > > > > For tests, there are even more log config files in the classpath > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ./flink-staging/flink-streaming/flink-streaming-core/src/test/resources/log4j.properties > > > > > > ./flink-staging/flink-hbase/src/test/resources/log4j.properties > > > > > > ./flink-staging/flink-tachyon/src/test/resources/log4j.properties > > > > > > > > > > > > Please remove these files and pay attention to not commit these > > > files, > > > > > they > > > > > > make it hard for anyone to actually want to debug based on log > > output > > > > to > > > > > > configure the logging. > > > > > > > > > > > > Greetings, > > > > > > Stephan > > > > > > > > > > > > > > > > > > > > > |
Free forum by Nabble | Edit this page |