spring-boot-1612: make console and file output configurable #1720
Conversation
1 disable the file output unless LOG_FILE is set 2 make the console optional fixes: spring-projectsgh-1612
|
This PR doesn't implement the changes described in #1612. With the changes in place the console is off by default. The intention was to have the console enabled and the file appender disabled by default. Furthermore, you've also introduced conditional logic to the Logback configuration, but haven't added a dependency on Janino (which is required for the conditional logic to work). If I add Janino to the classpath, setting logging.console to true doesn't enable the console. |
wilkinsona
added
the
status: waiting-for-feedback
|
make console output default true and add Janino to spring-boot |
|
Thanks for the code. However I'm not sure I like this change. I don't like having to pay for an extra dependency on this. I also kind of thought we could make it work for all logging systems (eg by using a different config file if LOG_FILE is set). |
|
@dsyer |
|
The current pull request has a janino dependency, so where is the new solution? By "system" I meant |
|
janino dependency is removed now. |
| Assert.notNull(configLocation, "ConfigLocation must not be null"); | ||
| String resolvedLocation = SystemPropertyUtils.resolvePlaceholders(configLocation); | ||
| try { | ||
| LogManager.getLogManager().readConfiguration( | ||
| ResourceUtils.getURL(resolvedLocation).openStream()); | ||
| if (fileOutput) { |
There was a problem hiding this comment.
Do we really need to use the Java API for the loggers here? I was hoping this would be in a config file. What happens if the user sets additional logger levels later, do they inherit from their parent or something? If so why do we need to iterate here (can't you just set the handler for the root logger).
There was a problem hiding this comment.
make java logging configured to root logger now
There was a problem hiding this comment.
I still don't like doing this in Java. It makes the log config files dependent on a very specific code path in the framework. It would be nice for users to be able to copy paste those files and use them as is.
There was a problem hiding this comment.
It would be nice for users to be able to copy paste those files and use them as is.
you mean the config files?
There was a problem hiding this comment.
do you have any idea how a properties file can reference variable defined in the same properties file?
like CONSOLE_LOG_LEVEL=OFF
java.util.logging.ConsoleHandler.level = ${CONSOLE_LOG_LEVEL}
|
remove comment and make java logging to root logger |
| LoggerContext context = (LoggerContext) factory; | ||
| context.stop(); | ||
| context.reset(); | ||
| try { | ||
| URL url = ResourceUtils.getURL(resolvedLocation); | ||
| new ContextInitializer(context).configureByResource(url); | ||
| if (consoleOutput) { |
There was a problem hiding this comment.
Maybe we could do the content of these two if() statements in separate <import/> files in logack.xml (instead of using the Java API, nice as it is)?
|
Get one of the four config files according to user properties files for fileoutput and consoleoutput |
|
I tried to merge this and got a conflict. I think it might be easier for you to rebase onto master and re-submit. Sorry this is taking so long. |
dsyer
removed
the
status: waiting-for-feedback
MockRestRequestMatchers was using org.junit.Assert.assertNotNull and thus could not be used in projects that use e.g. TestNG instead of JUnit 4.
1 disable the file output unless LOG_FILE is set
2 make the console optional
fixes: gh-1612