Skip to content

Switching from FileSize to DataSize for logback size-based properties. #15957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Switching from FileSize to DataSize for logback size-based properties. #15957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 38 additions & 8 deletions ...t/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
package org.springframework.boot.logging.logback;

import java.lang.reflect.Method;
import java.util.Objects;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.encoder.PatternLayoutEncoder;
Expand All @@ -35,7 +38,10 @@
import org.springframework.core.env.Environment;
import org.springframework.core.env.PropertyResolver;
import org.springframework.core.env.PropertySourcesPropertyResolver;
import org.springframework.util.Assert;
import org.springframework.util.ReflectionUtils;
import org.springframework.util.StringUtils;
import org.springframework.util.unit.DataSize;

/**
* Default logback configuration used by Spring Boot. Uses {@link LogbackConfigurator} to
Expand All @@ -58,6 +64,9 @@ class DefaultLogbackConfiguration {
private static final String FILE_LOG_PATTERN = "%d{${LOG_DATEFORMAT_PATTERN:-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 Pattern SIZE_PATTERN = Pattern
.compile("^([+\\-]?\\d+)\\s*([A-Z]{0,2})S?$", Pattern.CASE_INSENSITIVE);

private static final String MAX_FILE_SIZE = "10MB";

private final PropertyResolver patterns;
Expand Down Expand Up @@ -144,30 +153,51 @@ private void setRollingPolicy(RollingFileAppender<ILoggingEvent> appender,
rollingPolicy.setCleanHistoryOnStart(this.patterns.getProperty(
"logging.file.clean-history-on-start", Boolean.class, false));
rollingPolicy.setFileNamePattern(logFile + ".%d{yyyy-MM-dd}.%i.gz");
setMaxFileSize(rollingPolicy,
this.patterns.getProperty("logging.file.max-size", MAX_FILE_SIZE));
setMaxFileSize(rollingPolicy, getDataSize(
this.patterns.getProperty("logging.file.max-size", MAX_FILE_SIZE)));
setTotalSizeCap(rollingPolicy,
getDataSize(this.patterns.getProperty("logging.file.total-size-cap",
String.valueOf(CoreConstants.UNBOUNDED_TOTAL_SIZE_CAP))));
rollingPolicy.setMaxHistory(this.patterns.getProperty("logging.file.max-history",
Integer.class, CoreConstants.UNBOUND_HISTORY));
rollingPolicy.setTotalSizeCap(
FileSize.valueOf(this.patterns.getProperty("logging.file.total-size-cap",
String.valueOf(CoreConstants.UNBOUNDED_TOTAL_SIZE_CAP))));
appender.setRollingPolicy(rollingPolicy);
rollingPolicy.setParent(appender);
config.start(rollingPolicy);
}

private void setTotalSizeCap(
SizeAndTimeBasedRollingPolicy<ILoggingEvent> rollingPolicy,
DataSize totalSizeCap) {
Assert.isTrue(!totalSizeCap.isNegative(), "TotalSizeCap must not be a negative");
rollingPolicy.setTotalSizeCap(new FileSize(totalSizeCap.toBytes()));
}

private void setMaxFileSize(
SizeAndTimeBasedRollingPolicy<ILoggingEvent> rollingPolicy,
String maxFileSize) {
DataSize maxFileSize) {
Assert.isTrue(!maxFileSize.isNegative(), "MaxFileSize must not be a negative");
try {
rollingPolicy.setMaxFileSize(FileSize.valueOf(maxFileSize));
rollingPolicy.setMaxFileSize(new FileSize(maxFileSize.toBytes()));
}
catch (NoSuchMethodError ex) {
// Logback < 1.1.8 used String configuration
Method method = ReflectionUtils.findMethod(
SizeAndTimeBasedRollingPolicy.class, "setMaxFileSize", String.class);
ReflectionUtils.invokeMethod(method, rollingPolicy, maxFileSize);
ReflectionUtils.invokeMethod(method, rollingPolicy,
Objects.toString(maxFileSize.toBytes()));
}
}

private static DataSize getDataSize(String value) {
Matcher matcher = SIZE_PATTERN.matcher(value);
if (matcher.matches()) {
String size = matcher.group(1);
String unit = matcher.group(2);
return StringUtils.hasText(unit) ? DataSize.parse(size + unit.toUpperCase())
: DataSize.parse(size);
}
throw new IllegalArgumentException(
"'" + value + "' does not match data size pattern");
}

}
4 changes: 2 additions & 2 deletions ...ect/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@
},
{
"name": "logging.file.max-size",
"type": "java.lang.String",
"type": "org.springframework.util.unit.DataSize",
"description": "Maximum log file size. Only supported with the default logback setup.",
"sourceType": "org.springframework.boot.context.logging.LoggingApplicationListener",
"defaultValue": "10MB"
Expand All @@ -111,7 +111,7 @@
},
{
"name": "logging.file.total-size-cap",
"type": "java.lang.String",
"type": "org.springframework.util.unit.DataSize",
"description": "Total size of log backups to be kept. Only supported with the default logback setup.",
"sourceType": "org.springframework.boot.context.logging.LoggingApplicationListener",
"defaultValue": "0"
Expand Down
133 changes: 124 additions & 9 deletions ...oot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import org.springframework.util.StringUtils;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.contentOf;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
Expand Down Expand Up @@ -381,10 +382,53 @@ public void testCleanHistoryOnStartPropertyWithXmlConfiguration() {
assertThat(getRollingPolicy().isCleanHistoryOnStart()).isTrue();
}

@Test
public void testMaxFileSizePropertyNegativeValue() {
assertThatThrownBy(() -> {
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);
}).isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("MaxFileSize must not be a negative");
}

@Test
public void testMaxFileSizePropertyInvalidFormat() {
assertThatThrownBy(() -> {
MockEnvironment environment = new MockEnvironment();
environment.setProperty("logging.file.max-size", "100 :)");
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);
}).isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("does not match data size pattern");
}

@Test
public void testMaxFileSizePropertyLogbackFormat() {
MockEnvironment environment = new MockEnvironment();
environment.setProperty("logging.file.max-size", "100 kbs");
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 KB");
}

@Test
public void testMaxFileSizeProperty() {
MockEnvironment environment = new MockEnvironment();
environment.setProperty("logging.file.max-size", "100MB");
environment.setProperty("logging.file.max-size", "+10TB");
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
environment);
File file = new File(tmpDir(), "logback-test.log");
Expand All @@ -393,7 +437,22 @@ public void testMaxFileSizeProperty() {
this.logger.info("Hello world");
assertThat(getLineWithText(file, "Hello world")).contains("INFO");
assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "maxFileSize")
.toString()).isEqualTo("100 MB");
.toString()).isEqualTo("10240 GB");
}

@Test
public void testMaxFileSizePropertyRawBytes() {
MockEnvironment environment = new MockEnvironment();
environment.setProperty("logging.file.max-size", "1024");
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("1 KB");
}

@Test
Expand Down Expand Up @@ -427,7 +486,7 @@ public void testMaxHistoryProperty() {
}

@Test
public void testMaxHistoryPropertyWithXmlConfiguration() throws Exception {
public void testMaxHistoryPropertyWithXmlConfiguration() {
MockEnvironment environment = new MockEnvironment();
environment.setProperty("logging.file.max-history", "30");
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
Expand All @@ -441,11 +500,53 @@ public void testMaxHistoryPropertyWithXmlConfiguration() throws Exception {
assertThat(getRollingPolicy().getMaxHistory()).isEqualTo(30);
}

@Test
public void testTotalSizeCapPropertyNegativeValue() {
assertThatThrownBy(() -> {
MockEnvironment environment = new MockEnvironment();
environment.setProperty("logging.file.total-size-cap", "-101 Mb");
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);
}).isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("TotalSizeCap must not be a negative");
}

@Test
public void testTotalSizeCapPropertyInvalidFormat() {
assertThatThrownBy(() -> {
MockEnvironment environment = new MockEnvironment();
environment.setProperty("logging.file.total-size-cap", "101 :)");
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);
}).isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("does not match data size pattern");
}

@Test
public void testTotalSizeCapPropertyLogbackFormat() {
MockEnvironment environment = new MockEnvironment();
environment.setProperty("logging.file.total-size-cap", "101 mbs");
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(), "totalSizeCap")
.toString()).isEqualTo("101 MB");
}

@Test
public void testTotalSizeCapProperty() {
String expectedSize = "101 MB";
MockEnvironment environment = new MockEnvironment();
environment.setProperty("logging.file.total-size-cap", expectedSize);
environment.setProperty("logging.file.total-size-cap", "101MB");
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
environment);
File file = new File(tmpDir(), "logback-test.log");
Expand All @@ -454,14 +555,28 @@ public void testTotalSizeCapProperty() {
this.logger.info("Hello world");
assertThat(getLineWithText(file, "Hello world")).contains("INFO");
assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "totalSizeCap")
.toString()).isEqualTo(expectedSize);
.toString()).isEqualTo("101 MB");
}

@Test
public void testTotalSizeCapPropertyRawBytes() {
MockEnvironment environment = new MockEnvironment();
environment.setProperty("logging.file.total-size-cap", "10485760");
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(), "totalSizeCap")
.toString()).isEqualTo("10 MB");
}

@Test
public void testTotalSizeCapPropertyWithXmlConfiguration() {
String expectedSize = "101 MB";
MockEnvironment environment = new MockEnvironment();
environment.setProperty("logging.file.total-size-cap", expectedSize);
environment.setProperty("logging.file.total-size-cap", "101MB");
LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext(
environment);
File file = new File(tmpDir(), "logback-test.log");
Expand All @@ -471,7 +586,7 @@ public void testTotalSizeCapPropertyWithXmlConfiguration() {
this.logger.info("Hello world");
assertThat(getLineWithText(file, "Hello world")).contains("INFO");
assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "totalSizeCap")
.toString()).isEqualTo(expectedSize);
.toString()).isEqualTo("101 MB");
}

@Test
Expand Down