Too many log4j.properties files

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Too many log4j.properties files

Stephan Ewen
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
Reply | Threaded
Open this post in threaded view
|

Re: Too many log4j.properties files

Robert Metzger
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
>
Reply | Threaded
Open this post in threaded view
|

Re: Too many log4j.properties files

Márton Balassi
+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
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Too many log4j.properties files

Stephan Ewen
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
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Too many log4j.properties files

Robert Metzger
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
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Too many log4j.properties files

Stephan Ewen
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
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Too many log4j.properties files

Robert Metzger
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
> > > > >
> > > >
> > >
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: Too many log4j.properties files

Stephan Ewen
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
> > > > > >
> > > > >
> > > >
> > >
> >
>