Skip to content

Commit 773dda3

Browse files
mbhavembhave
committed
Log file location should be evaluated just once
Using a random value for the logfile name caused the logfile endpoint to return a 404 as the name was resolved from the environment on every request. This commit registers a bean for LogFile which is then used by the logfile endpoint. Fixes spring-projectsgh-17434
1 parent 7854876 commit 773dda3

File tree

8 files changed

+55
-48
lines changed

8 files changed

+55
-48
lines changed

‎spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/logging/LogFileWebEndpointAutoConfiguration.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.boot.actuate.autoconfigure.logging;
1818

19+
import org.springframework.beans.factory.ObjectProvider;
1920
import org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnEnabledEndpoint;
2021
import org.springframework.boot.actuate.logging.LogFileWebEndpoint;
2122
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
@@ -24,6 +25,7 @@
2425
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
2526
import org.springframework.boot.autoconfigure.condition.SpringBootCondition;
2627
import org.springframework.boot.context.properties.EnableConfigurationProperties;
28+
import org.springframework.boot.logging.LogFile;
2729
import org.springframework.context.annotation.Bean;
2830
import org.springframework.context.annotation.ConditionContext;
2931
import org.springframework.context.annotation.Conditional;
@@ -52,8 +54,8 @@ public LogFileWebEndpointAutoConfiguration(LogFileWebEndpointProperties properti
5254
@Bean
5355
@ConditionalOnMissingBean
5456
@Conditional(LogFileCondition.class)
55-
public LogFileWebEndpoint logFileWebEndpoint(Environment environment) {
56-
return new LogFileWebEndpoint(environment, this.properties.getExternalFile());
57+
public LogFileWebEndpoint logFileWebEndpoint(ObjectProvider<LogFile> logFile) {
58+
return new LogFileWebEndpoint(logFile.getIfAvailable(), this.properties.getExternalFile());
5759
}
5860

5961
private static class LogFileCondition extends SpringBootCondition {

‎spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/documentation/LogFileWebEndpointDocumentationTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.junit.Test;
2020

2121
import org.springframework.boot.actuate.logging.LogFileWebEndpoint;
22+
import org.springframework.boot.logging.LogFile;
2223
import org.springframework.context.annotation.Bean;
2324
import org.springframework.context.annotation.Configuration;
2425
import org.springframework.context.annotation.Import;
@@ -56,7 +57,7 @@ static class TestConfiguration {
5657

5758
@Bean
5859
public LogFileWebEndpoint endpoint(Environment environment) {
59-
return new LogFileWebEndpoint(environment);
60+
return new LogFileWebEndpoint(LogFile.get(environment), null);
6061
}
6162

6263
}

‎spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/logging/LogFileWebEndpoint.java

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.springframework.boot.actuate.endpoint.annotation.ReadOperation;
2626
import org.springframework.boot.actuate.endpoint.web.annotation.WebEndpoint;
2727
import org.springframework.boot.logging.LogFile;
28-
import org.springframework.core.env.Environment;
2928
import org.springframework.core.io.FileSystemResource;
3029
import org.springframework.core.io.Resource;
3130

@@ -42,17 +41,13 @@ public class LogFileWebEndpoint {
4241

4342
private static final Log logger = LogFactory.getLog(LogFileWebEndpoint.class);
4443

45-
private final Environment environment;
46-
4744
private File externalFile;
4845

49-
public LogFileWebEndpoint(Environment environment, File externalFile) {
50-
this.environment = environment;
51-
this.externalFile = externalFile;
52-
}
46+
private final LogFile logFile;
5347

54-
public LogFileWebEndpoint(Environment environment) {
55-
this(environment, null);
48+
public LogFileWebEndpoint(LogFile logFile, File externalFile) {
49+
this.externalFile = externalFile;
50+
this.logFile = logFile;
5651
}
5752

5853
@ReadOperation(produces = "text/plain; charset=UTF-8")
@@ -68,12 +63,11 @@ private Resource getLogFileResource() {
6863
if (this.externalFile != null) {
6964
return new FileSystemResource(this.externalFile);
7065
}
71-
LogFile logFile = LogFile.get(this.environment);
72-
if (logFile == null) {
66+
if (this.logFile == null) {
7367
logger.debug("Missing 'logging.file' or 'logging.path' properties");
7468
return null;
7569
}
76-
return new FileSystemResource(logFile.toString());
70+
return new FileSystemResource(this.logFile.toString());
7771
}
7872

7973
}

‎spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/logging/LogFileWebEndpointTests.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.junit.Test;
2626
import org.junit.rules.TemporaryFolder;
2727

28+
import org.springframework.boot.logging.LogFile;
2829
import org.springframework.core.io.Resource;
2930
import org.springframework.mock.env.MockEnvironment;
3031
import org.springframework.util.FileCopyUtils;
@@ -46,8 +47,6 @@ public class LogFileWebEndpointTests {
4647

4748
private final MockEnvironment environment = new MockEnvironment();
4849

49-
private final LogFileWebEndpoint endpoint = new LogFileWebEndpoint(this.environment);
50-
5150
private File logFile;
5251

5352
@Before
@@ -58,26 +57,29 @@ public void before() throws IOException {
5857

5958
@Test
6059
public void nullResponseWithoutLogFile() {
61-
assertThat(this.endpoint.logFile()).isNull();
60+
LogFileWebEndpoint endpoint = new LogFileWebEndpoint(null, null);
61+
assertThat(endpoint.logFile()).isNull();
6262
}
6363

6464
@Test
6565
public void nullResponseWithMissingLogFile() {
6666
this.environment.setProperty("logging.file", "no_test.log");
67-
assertThat(this.endpoint.logFile()).isNull();
67+
LogFileWebEndpoint endpoint = new LogFileWebEndpoint(LogFile.get(this.environment), null);
68+
assertThat(endpoint.logFile()).isNull();
6869
}
6970

7071
@Test
7172
public void resourceResponseWithLogFile() throws Exception {
7273
this.environment.setProperty("logging.file", this.logFile.getAbsolutePath());
73-
Resource resource = this.endpoint.logFile();
74+
LogFileWebEndpoint endpoint = new LogFileWebEndpoint(LogFile.get(this.environment), null);
75+
Resource resource = endpoint.logFile();
7476
assertThat(resource).isNotNull();
7577
assertThat(StreamUtils.copyToString(resource.getInputStream(), StandardCharsets.UTF_8)).isEqualTo("--TEST--");
7678
}
7779

7880
@Test
7981
public void resourceResponseWithExternalLogFile() throws Exception {
80-
LogFileWebEndpoint endpoint = new LogFileWebEndpoint(this.environment, this.logFile);
82+
LogFileWebEndpoint endpoint = new LogFileWebEndpoint(null, this.logFile);
8183
Resource resource = endpoint.logFile();
8284
assertThat(resource).isNotNull();
8385
assertThat(StreamUtils.copyToString(resource.getInputStream(), StandardCharsets.UTF_8)).isEqualTo("--TEST--");

‎spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/logging/LogFileWebEndpointWebIntegrationTests.java

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,19 @@
1717
package org.springframework.boot.actuate.logging;
1818

1919
import java.io.File;
20-
import java.io.IOException;
2120

22-
import org.junit.Before;
23-
import org.junit.Rule;
21+
import org.junit.ClassRule;
2422
import org.junit.Test;
2523
import org.junit.rules.TemporaryFolder;
2624
import org.junit.runner.RunWith;
2725

2826
import org.springframework.boot.actuate.endpoint.web.test.WebEndpointRunners;
29-
import org.springframework.boot.test.util.TestPropertyValues;
27+
import org.springframework.boot.logging.LogFile;
3028
import org.springframework.context.ConfigurableApplicationContext;
3129
import org.springframework.context.annotation.Bean;
3230
import org.springframework.context.annotation.Configuration;
33-
import org.springframework.core.env.Environment;
3431
import org.springframework.http.MediaType;
32+
import org.springframework.mock.env.MockEnvironment;
3533
import org.springframework.test.web.reactive.server.WebTestClient;
3634
import org.springframework.util.FileCopyUtils;
3735

@@ -48,32 +46,19 @@ public class LogFileWebEndpointWebIntegrationTests {
4846

4947
private static WebTestClient client;
5048

51-
@Rule
52-
public final TemporaryFolder temp = new TemporaryFolder();
49+
@ClassRule
50+
public static final TemporaryFolder temp = new TemporaryFolder();
5351

54-
private File logFile;
55-
56-
@Before
57-
public void setUp() throws IOException {
58-
this.logFile = this.temp.newFile();
59-
FileCopyUtils.copy("--TEST--".getBytes(), this.logFile);
60-
}
61-
62-
@Test
63-
public void getRequestProduces404ResponseWhenLogFileNotFound() {
64-
client.get().uri("/actuator/logfile").exchange().expectStatus().isNotFound();
65-
}
52+
private static File logFile;
6653

6754
@Test
6855
public void getRequestProducesResponseWithLogFile() {
69-
TestPropertyValues.of("logging.file:" + this.logFile.getAbsolutePath()).applyTo(context);
7056
client.get().uri("/actuator/logfile").exchange().expectStatus().isOk().expectHeader()
7157
.contentType("text/plain; charset=UTF-8").expectBody(String.class).isEqualTo("--TEST--");
7258
}
7359

7460
@Test
7561
public void getRequestThatAcceptsTextPlainProducesResponseWithLogFile() {
76-
TestPropertyValues.of("logging.file:" + this.logFile.getAbsolutePath()).applyTo(context);
7762
client.get().uri("/actuator/logfile").accept(MediaType.TEXT_PLAIN).exchange().expectStatus().isOk()
7863
.expectHeader().contentType("text/plain; charset=UTF-8").expectBody(String.class).isEqualTo("--TEST--");
7964
}
@@ -82,8 +67,12 @@ public void getRequestThatAcceptsTextPlainProducesResponseWithLogFile() {
8267
static class TestConfiguration {
8368

8469
@Bean
85-
public LogFileWebEndpoint logFileEndpoint(Environment environment) {
86-
return new LogFileWebEndpoint(environment);
70+
public LogFileWebEndpoint logFileEndpoint() throws Exception {
71+
logFile = temp.newFile();
72+
FileCopyUtils.copy("--TEST--".getBytes(), logFile);
73+
MockEnvironment environment = new MockEnvironment();
74+
environment.setProperty("logging.file", logFile.getAbsolutePath());
75+
return new LogFileWebEndpoint(LogFile.get(environment), null);
8776
}
8877

8978
}

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/logging/LoggingApplicationListener.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,11 @@ public class LoggingApplicationListener implements GenericApplicationListener {
121121
*/
122122
public static final String LOGGING_SYSTEM_BEAN_NAME = "springBootLoggingSystem";
123123

124+
/**
125+
* The name of the {@link LogFile} bean.
126+
*/
127+
public static final String LOGFILE_BEAN_NAME = "springBootLogFile";
128+
124129
private static final Map<String, List<String>> DEFAULT_GROUP_LOGGERS;
125130
static {
126131
MultiValueMap<String, String> loggers = new LinkedMultiValueMap<>();
@@ -160,6 +165,8 @@ public class LoggingApplicationListener implements GenericApplicationListener {
160165

161166
private LoggingSystem loggingSystem;
162167

168+
private LogFile logFile;
169+
163170
private int order = DEFAULT_ORDER;
164171

165172
private boolean parseArgs = true;
@@ -224,6 +231,9 @@ private void onApplicationPreparedEvent(ApplicationPreparedEvent event) {
224231
if (!beanFactory.containsBean(LOGGING_SYSTEM_BEAN_NAME)) {
225232
beanFactory.registerSingleton(LOGGING_SYSTEM_BEAN_NAME, this.loggingSystem);
226233
}
234+
if (this.logFile != null && !beanFactory.containsBean(LOGFILE_BEAN_NAME)) {
235+
beanFactory.registerSingleton(LOGFILE_BEAN_NAME, this.logFile);
236+
}
227237
}
228238

229239
private void onContextClosedEvent() {
@@ -246,12 +256,12 @@ private void onApplicationFailedEvent() {
246256
*/
247257
protected void initialize(ConfigurableEnvironment environment, ClassLoader classLoader) {
248258
new LoggingSystemProperties(environment).apply();
249-
LogFile logFile = LogFile.get(environment);
250-
if (logFile != null) {
251-
logFile.applyToSystemProperties();
259+
this.logFile = LogFile.get(environment);
260+
if (this.logFile != null) {
261+
this.logFile.applyToSystemProperties();
252262
}
253263
initializeEarlyLoggingLevel(environment);
254-
initializeSystem(environment, this.loggingSystem, logFile);
264+
initializeSystem(environment, this.loggingSystem, this.logFile);
255265
initializeFinalLoggingLevels(environment, this.loggingSystem);
256266
registerShutdownHookIfNecessary(environment, this.loggingSystem);
257267
}

‎spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/EndpointsPropertiesSampleActuatorApplicationTests.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,13 @@ public void testCustomContextPath() {
6666
assertThat(entity.getBody()).contains("\"hello\":\"world\"");
6767
}
6868

69+
@Test
70+
public void logfileWithRandomName() {
71+
ResponseEntity<String> entity = this.restTemplate.withBasicAuth("user", getPassword())
72+
.getForEntity("/admin/logfile", String.class);
73+
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK);
74+
}
75+
6976
private String getPassword() {
7077
return "password";
7178
}

‎spring-boot-samples/spring-boot-sample-actuator/src/test/resources/application-endpoints.properties

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
server.error.path: /oops
22
management.endpoint.health.show-details: always
33
management.endpoints.web.base-path: /admin
4+
logging.file=./target/${spring.application.instance_id}.log
5+
spring.application.instance_id=${random.value}

0 commit comments

Comments
 (0)