Merge pull request #6352 from vpavic/gh-5590

* pr/6352:
  Polish SizeAndTimeBasedRollingPolicy changes
  Use SizeAndTimeBasedRollingPolicy file appender

Author: philwebb

spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc

@@ -41,6 +41,8 @@ content into your application; rather pick only the properties that you need.
 	logging.config= # Location of the logging configuration file. For instance `classpath:logback.xml` for Logback
 	logging.exception-conversion-word=%wEx # Conversion word used when logging exceptions.
 	logging.file= # Log file name. For instance `myapp.log`
+	logging.file.max-history= # Maximum of archive log files to keep. Only supported with the default logback setup.
+	logging.file.max-size= # Maximum log file size. Only supported with the default logback setup.
 	logging.level.*= # Log levels severity mapping. For instance `logging.level.org.springframework=DEBUG`
 	logging.path= # Location of the log file. For instance `/var/log`
 	logging.pattern.console= # Appender pattern for output to the console. Only supported with the default logback setup.

spring-boot-project/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc

@@ -1533,7 +1533,9 @@ relative to the current directory.
 |===
 
 Log files rotate when they reach 10 MB and, as with console output, `ERROR`-level,
-`WARN`-level, and `INFO`-level messages are logged by default.
+`WARN`-level, and `INFO`-level messages are logged by default. Size limits can be changed
+using the `logging.file.max-size` property. Previously rotated files are archived
+indefinitely unless the `logging.file.max-history` property has been set.
 
 NOTE: The logging system is initialized early in the application lifecycle. Consequently,
 logging properties are not found in property files loaded through `@PropertySource`
@@ -1620,6 +1622,16 @@ To help with the customization, some other properties are transferred from the S
 |`LOG_FILE`
 |If defined, it is used in the default log configuration.
 
+|`logging.file.max-size`
+|`LOG_FILE_MAX_SIZE`
+|Maximum log file size (if LOG_FILE enabled). (Only supported with the default logback
+ setup.)
+
+|`logging.file.max-history`
+|`LOG_FILE_MAX_HISTORY`
+|Maximum number of archive log files to keep (if LOG_FILE enabled). (Only supported with
+ the default logback setup.)
+
 |`logging.path`
 |`LOG_PATH`
 |If defined, it is used in the default log configuration.
@@ -1645,7 +1657,6 @@ To help with the customization, some other properties are transferred from the S
  environment variable).
 |===
 
-
 All the supported logging systems can consult System properties when parsing their
 configuration files.  See the default configurations in `spring-boot.jar` for examples:
 

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/LoggingSystemProperties.java

@@ -29,6 +29,7 @@
  * @author Andy Wilkinson
  * @author Phillip Webb
  * @author Madhura Bhave
+ * @author Vedran Pavic
  * @since 2.0.0
  */
 public class LoggingSystemProperties {
@@ -63,6 +64,16 @@ public class LoggingSystemProperties {
 	 */
 	public static final String FILE_LOG_PATTERN = "FILE_LOG_PATTERN";
 
+	/**
+	 * The name of the System property that contains the file log max history.
+	 */
+	public static final String FILE_MAX_HISTORY = "LOG_FILE_MAX_HISTORY";
+
+	/**
+	 * The name of the System property that contains the file log max size.
+	 */
+	public static final String FILE_MAX_SIZE = "LOG_FILE_MAX_SIZE";
+
 	/**
 	 * The name of the System property that contains the log level pattern.
 	 */
@@ -89,6 +100,8 @@ public void apply(LogFile logFile) {
 				"exception-conversion-word");
 		setSystemProperty(resolver, CONSOLE_LOG_PATTERN, "pattern.console");
 		setSystemProperty(resolver, FILE_LOG_PATTERN, "pattern.file");
+		setSystemProperty(resolver, FILE_MAX_HISTORY, "file.max-history");
+		setSystemProperty(resolver, FILE_MAX_SIZE, "file.max-size");
 		setSystemProperty(resolver, LOG_LEVEL_PATTERN, "pattern.level");
 		setSystemProperty(PID_KEY, new ApplicationPid().toString());
 		if (logFile != null) {

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java

@@ -24,9 +24,9 @@
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.core.Appender;
 import ch.qos.logback.core.ConsoleAppender;
-import ch.qos.logback.core.rolling.FixedWindowRollingPolicy;
+import ch.qos.logback.core.CoreConstants;
 import ch.qos.logback.core.rolling.RollingFileAppender;
-import ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy;
+import ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy;
 import ch.qos.logback.core.util.FileSize;
 import ch.qos.logback.core.util.OptionHelper;
 
@@ -45,6 +45,7 @@
  *
  * @author Phillip Webb
  * @author Madhura Bhave
+ * @author Vedran Pavic
  * @since 1.1.2
  */
 class DefaultLogbackConfiguration {
@@ -57,6 +58,8 @@ class DefaultLogbackConfiguration {
 	private static final String FILE_LOG_PATTERN = "%d{yyyy-MM-dd HH:mm:ss.SSS} "
 			+ "${LOG_LEVEL_PATTERN:-%5p} ${PID:- } --- [%t] %-40.40logger{39} : %m%n${LOG_EXCEPTION_CONVERSION_WORD:-%wEx}";
 
+	private static final String MAX_FILE_SIZE = "10MB";
+
 	private static final Charset UTF8 = Charset.forName("UTF-8");
 
 	private final PropertyResolver patterns;
@@ -140,34 +143,35 @@ private Appender<ILoggingEvent> fileAppender(LogbackConfigurator config,
 		config.start(encoder);
 		appender.setFile(logFile);
 		setRollingPolicy(appender, config, logFile);
-		setMaxFileSize(appender, config);
 		config.appender("FILE", appender);
 		return appender;
 	}
 
 	private void setRollingPolicy(RollingFileAppender<ILoggingEvent> appender,
 			LogbackConfigurator config, String logFile) {
-		FixedWindowRollingPolicy rollingPolicy = new FixedWindowRollingPolicy();
-		rollingPolicy.setFileNamePattern(logFile + ".%i");
+		SizeAndTimeBasedRollingPolicy<ILoggingEvent> rollingPolicy = new SizeAndTimeBasedRollingPolicy<>();
+		rollingPolicy.setFileNamePattern(logFile + ".%d{yyyy-MM-dd}.%i.gz");
+		setMaxFileSize(rollingPolicy,
+				this.patterns.getProperty("logging.file.max-size", MAX_FILE_SIZE));
+		rollingPolicy.setMaxHistory(this.patterns.getProperty("logging.file.max-history",
+				Integer.class, CoreConstants.UNBOUND_HISTORY));
 		appender.setRollingPolicy(rollingPolicy);
 		rollingPolicy.setParent(appender);
 		config.start(rollingPolicy);
 	}
 
-	private void setMaxFileSize(RollingFileAppender<ILoggingEvent> appender,
-			LogbackConfigurator config) {
-		SizeBasedTriggeringPolicy<ILoggingEvent> triggeringPolicy = new SizeBasedTriggeringPolicy<>();
+	private void setMaxFileSize(
+			SizeAndTimeBasedRollingPolicy<ILoggingEvent> rollingPolicy,
+			String maxFileSize) {
 		try {
-			triggeringPolicy.setMaxFileSize(FileSize.valueOf("10MB"));
+			rollingPolicy.setMaxFileSize(FileSize.valueOf(maxFileSize));
 		}
 		catch (NoSuchMethodError ex) {
 			// Logback < 1.1.8 used String configuration
-			Method method = ReflectionUtils.findMethod(SizeBasedTriggeringPolicy.class,
-					"setMaxFileSize", String.class);
-			ReflectionUtils.invokeMethod(method, triggeringPolicy, "10MB");
+			Method method = ReflectionUtils.findMethod(
+					SizeAndTimeBasedRollingPolicy.class, "setMaxFileSize", String.class);
+			ReflectionUtils.invokeMethod(method, rollingPolicy, maxFileSize);
 		}
-		appender.setTriggeringPolicy(triggeringPolicy);
-		config.start(triggeringPolicy);
 	}
 
 }

spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json

@@ -69,6 +69,20 @@
     "description": "Name of the log file. Names can be an exact location or relative to the current directory.",
     "sourceType": "org.springframework.boot.context.logging.LoggingApplicationListener"
   },
+  {
+    "name": "logging.file.max-size",
+    "type": "java.lang.String",
+    "description": "Maximum log file size. Only supported with the default logback setup.",
+    "sourceType": "org.springframework.boot.context.logging.LoggingApplicationListener",
+    "defaultValue": "10MB"
+  },
+  {
+    "name": "logging.file.max-history",
+    "type": "java.lang.Integer",
+    "description": "Maximum number of archive log files to keep. Only supported with the default logback setup.",
+    "sourceType": "org.springframework.boot.context.logging.LoggingApplicationListener",
+    "defaultValue": 0
+  },
   {
     "name": "logging.level",
     "type": "java.util.Map<java.lang.String,java.lang.String>",

spring-boot-project/spring-boot/src/main/resources/org/springframework/boot/logging/logback/file-appender.xml

@@ -12,12 +12,10 @@ initialization performed by Boot
 			<pattern>${FILE_LOG_PATTERN}</pattern>
 		</encoder>
 		<file>${LOG_FILE}</file>
-		<rollingPolicy class="ch.qos.logback.core.rolling.FixedWindowRollingPolicy">
-			<fileNamePattern>${LOG_FILE}.%i</fileNamePattern>
+		<rollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy">
+			<fileNamePattern>${LOG_FILE}.%d{yyyy-MM-dd}.%i.gz</fileNamePattern>
+			<maxFileSize>${MAX_FILE_SIZE:-10MB}</maxFileSize>
+			<maxHistory>${MAX_HISTORY:-0}</maxHistory>
 		</rollingPolicy>
-		<triggeringPolicy
-			class="ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy">
-			<MaxFileSize>10MB</MaxFileSize>
-		</triggeringPolicy>
 	</appender>
 </included>

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java

@@ -27,6 +27,10 @@
 import ch.qos.logback.classic.Logger;
 import ch.qos.logback.classic.LoggerContext;
 import ch.qos.logback.classic.spi.LoggerContextListener;
+import ch.qos.logback.core.ConsoleAppender;
+import ch.qos.logback.core.CoreConstants;
+import ch.qos.logback.core.rolling.RollingFileAppender;
+import ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.impl.SLF4JLogFactory;
 import org.hamcrest.Matcher;
@@ -48,6 +52,7 @@
 import org.springframework.boot.testsupport.assertj.Matched;
 import org.springframework.boot.testsupport.rule.OutputCapture;
 import org.springframework.mock.env.MockEnvironment;
+import org.springframework.test.util.ReflectionTestUtils;
 import org.springframework.util.FileCopyUtils;
 import org.springframework.util.StringUtils;
 
@@ -66,6 +71,7 @@
  * @author Andy Wilkinson
  * @author Ben Hale
  * @author Madhura Bhave
+ * @author Vedran Pavic
  */
 public class LogbackLoggingSystemTests extends AbstractLoggingSystemTests {
 
@@ -120,15 +126,16 @@ public void withFile() throws Exception {
 		assertThat(getLineWithText(output, "Hello world")).contains("INFO");
 		assertThat(file.exists()).isTrue();
 		assertThat(getLineWithText(file, "Hello world")).contains("INFO");
+		assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "maxFileSize")
+				.toString()).isEqualTo("10 MB");
+		assertThat(getRollingPolicy().getMaxHistory())
+				.isEqualTo(CoreConstants.UNBOUND_HISTORY);
 	}
 
 	@Test
 	public void testBasicConfigLocation() throws Exception {
 		this.loggingSystem.beforeInitialize();
-		ILoggerFactory factory = StaticLoggerBinder.getSingleton().getLoggerFactory();
-		LoggerContext context = (LoggerContext) factory;
-		Logger root = context.getLogger(org.slf4j.Logger.ROOT_LOGGER_NAME);
-		assertThat(root.getAppender("CONSOLE")).isNotNull();
+		assertThat(getConsoleAppender()).isNotNull();
 	}
 
 	@Test
@@ -339,6 +346,35 @@ public void testFilePatternProperty() throws Exception {
 		assertThat(getLineWithText(file, "Hello world")).doesNotContain("INFO");
 	}
 
+	@Test
+	public void testMaxFileSizeProperty() throws Exception {
+		MockEnvironment environment = new MockEnvironment();
+		environment.setProperty("logging.file.max-size", "100MB");
+		LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
+				environment);
+		File file = new File(tmpDir(), "logback-test.log");
+		LogFile logFile = getLogFile(file.getPath(), null);
+		this.loggingSystem.initialize(loggingInitializationContext, null, logFile);
+		this.logger.info("Hello world");
+		assertThat(getLineWithText(file, "Hello world")).contains("INFO");
+		assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "maxFileSize")
+				.toString()).isEqualTo("100 MB");
+	}
+
+	@Test
+	public void testMaxHistoryProperty() throws Exception {
+		MockEnvironment environment = new MockEnvironment();
+		environment.setProperty("logging.file.max-history", "30");
+		LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
+				environment);
+		File file = new File(tmpDir(), "logback-test.log");
+		LogFile logFile = getLogFile(file.getPath(), null);
+		this.loggingSystem.initialize(loggingInitializationContext, null, logFile);
+		this.logger.info("Hello world");
+		assertThat(getLineWithText(file, "Hello world")).contains("INFO");
+		assertThat(getRollingPolicy().getMaxHistory()).isEqualTo(30);
+	}
+
 	@Test
 	public void exceptionsIncludeClassPackaging() throws Exception {
 		this.loggingSystem.beforeInitialize();
@@ -404,6 +440,24 @@ public void initializationIsOnlyPerformedOnceUntilCleanedUp() throws Exception {
 		verify(listener, times(2)).onReset(loggerContext);
 	}
 
+	private static Logger getRootLogger() {
+		ILoggerFactory factory = StaticLoggerBinder.getSingleton().getLoggerFactory();
+		LoggerContext context = (LoggerContext) factory;
+		return context.getLogger(org.slf4j.Logger.ROOT_LOGGER_NAME);
+	}
+
+	private static ConsoleAppender getConsoleAppender() {
+		return (ConsoleAppender) getRootLogger().getAppender("CONSOLE");
+	}
+
+	private static RollingFileAppender getFileAppender() {
+		return (RollingFileAppender) getRootLogger().getAppender("FILE");
+	}
+
+	private static SizeAndTimeBasedRollingPolicy getRollingPolicy() {
+		return (SizeAndTimeBasedRollingPolicy) getFileAppender().getRollingPolicy();
+	}
+
 	private String getLineWithText(File file, String outputSearch) throws Exception {
 		return getLineWithText(FileCopyUtils.copyToString(new FileReader(file)),
 				outputSearch);