Skip to content

Commit 42e725d

Browse files
committed
Merge pull request #50957 from GollapudiSrikanth
Closes gh-50957 * srikanthgollapudi-configurable-timeout-jmshandler: Polish "Add support for configuring start timeout for JMS health checks" Add support for configuring start timeout for JMS health checks
2 parents 9d472e8 + db12d4d commit 42e725d

5 files changed

Lines changed: 153 additions & 8 deletions

File tree

module/spring-boot-jms/src/main/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthContributorAutoConfiguration.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
2525
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
2626
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
27+
import org.springframework.boot.context.properties.EnableConfigurationProperties;
2728
import org.springframework.boot.health.autoconfigure.contributor.CompositeHealthContributorConfiguration;
2829
import org.springframework.boot.health.autoconfigure.contributor.ConditionalOnEnabledHealthIndicator;
2930
import org.springframework.boot.health.contributor.HealthContributor;
@@ -35,17 +36,19 @@
3536
* {@link EnableAutoConfiguration Auto-configuration} for {@link JmsHealthIndicator}.
3637
*
3738
* @author Stephane Nicoll
39+
* @author Venkata Naga Sai Srikanth Gollapudi
3840
* @since 4.0.0
3941
*/
4042
@AutoConfiguration(after = JmsAutoConfiguration.class)
4143
@ConditionalOnClass({ ConnectionFactory.class, JmsHealthIndicator.class })
4244
@ConditionalOnBean(ConnectionFactory.class)
4345
@ConditionalOnEnabledHealthIndicator("jms")
46+
@EnableConfigurationProperties(JmsHealthIndicatorProperties.class)
4447
public final class JmsHealthContributorAutoConfiguration
4548
extends CompositeHealthContributorConfiguration<JmsHealthIndicator, ConnectionFactory> {
4649

47-
JmsHealthContributorAutoConfiguration() {
48-
super(JmsHealthIndicator::new);
50+
JmsHealthContributorAutoConfiguration(JmsHealthIndicatorProperties properties) {
51+
super((connectionFactory) -> new JmsHealthIndicator(connectionFactory, properties.getStartTimeout()));
4952
}
5053

5154
@Bean
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Copyright 2012-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.jms.autoconfigure.health;
18+
19+
import java.time.Duration;
20+
21+
import org.springframework.boot.context.properties.ConfigurationProperties;
22+
import org.springframework.boot.jms.health.JmsHealthIndicator;
23+
24+
/**
25+
* Configuration properties for {@link JmsHealthIndicator}.
26+
*
27+
* @author Venkata Naga Sai Srikanth Gollapudi
28+
* @author Stephane Nicoll
29+
* @since 4.2.0
30+
*/
31+
@ConfigurationProperties("management.health.jms")
32+
public class JmsHealthIndicatorProperties {
33+
34+
/**
35+
* Timeout to use when starting a connection for the health check.
36+
*/
37+
private Duration startTimeout = JmsHealthIndicator.DEFAULT_START_TIMEOUT;
38+
39+
public Duration getStartTimeout() {
40+
return this.startTimeout;
41+
}
42+
43+
public void setStartTimeout(Duration startTimeout) {
44+
this.startTimeout = startTimeout;
45+
}
46+
47+
}

module/spring-boot-jms/src/main/java/org/springframework/boot/jms/health/JmsHealthIndicator.java

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

1717
package org.springframework.boot.jms.health;
1818

19+
import java.time.Duration;
1920
import java.util.concurrent.CountDownLatch;
2021
import java.util.concurrent.TimeUnit;
2122

@@ -25,25 +26,55 @@
2526
import org.apache.commons.logging.Log;
2627
import org.apache.commons.logging.LogFactory;
2728

29+
import org.springframework.boot.convert.DurationStyle;
2830
import org.springframework.boot.health.contributor.AbstractHealthIndicator;
2931
import org.springframework.boot.health.contributor.Health;
3032
import org.springframework.boot.health.contributor.HealthIndicator;
33+
import org.springframework.core.log.LogMessage;
34+
import org.springframework.util.Assert;
3135

3236
/**
3337
* {@link HealthIndicator} for a JMS {@link ConnectionFactory}.
3438
*
3539
* @author Stephane Nicoll
40+
* @author Venkata Naga Sai Srikanth Gollapudi
3641
* @since 4.0.0
3742
*/
3843
public class JmsHealthIndicator extends AbstractHealthIndicator {
3944

45+
/**
46+
* Default timeout to use when starting a connection for the health check.
47+
*/
48+
public static final Duration DEFAULT_START_TIMEOUT = Duration.ofSeconds(5);
49+
4050
private final Log logger = LogFactory.getLog(JmsHealthIndicator.class);
4151

4252
private final ConnectionFactory connectionFactory;
4353

54+
private final Duration startTimeout;
55+
56+
/**
57+
* Create a new {@link JmsHealthIndicator} instance with a
58+
* {@linkplain #DEFAULT_START_TIMEOUT default} start timeout.
59+
* @param connectionFactory the connection factory to use
60+
*/
4461
public JmsHealthIndicator(ConnectionFactory connectionFactory) {
62+
this(connectionFactory, DEFAULT_START_TIMEOUT);
63+
}
64+
65+
/**
66+
* Create a new {@link JmsHealthIndicator} instance with the given
67+
* {@code startTimeout}.
68+
* @param connectionFactory the connection factory to use
69+
* @param startTimeout timeout to use when starting a connection for the health check
70+
* @since 4.2.0
71+
*/
72+
public JmsHealthIndicator(ConnectionFactory connectionFactory, Duration startTimeout) {
4573
super("JMS health check failed");
74+
Assert.notNull(startTimeout, "'startTimeout' must not be null");
75+
Assert.isTrue(startTimeout.compareTo(Duration.ZERO) > 0, "'startTimeout' must be greater than 0");
4676
this.connectionFactory = connectionFactory;
77+
this.startTimeout = startTimeout;
4778
}
4879

4980
@Override
@@ -67,9 +98,11 @@ private final class MonitoredConnection {
6798
void start() throws JMSException {
6899
new Thread(() -> {
69100
try {
70-
if (!this.latch.await(5, TimeUnit.SECONDS)) {
101+
Duration startTimeout1 = JmsHealthIndicator.this.startTimeout;
102+
if (!this.latch.await(startTimeout1.toNanos(), TimeUnit.NANOSECONDS)) {
71103
JmsHealthIndicator.this.logger
72-
.warn("Connection failed to start within 5 seconds and will be closed.");
104+
.warn(LogMessage.format("Connection failed to start within %s and will be closed.",
105+
DurationStyle.SIMPLE.print(startTimeout1)));
73106
closeConnection();
74107
}
75108
}

module/spring-boot-jms/src/test/java/org/springframework/boot/jms/autoconfigure/health/JmsHealthContributorAutoConfigurationTests.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.boot.jms.autoconfigure.health;
1818

19+
import java.time.Duration;
20+
1921
import jakarta.jms.ConnectionFactory;
2022
import org.junit.jupiter.api.Test;
2123

@@ -31,6 +33,7 @@
3133
* Tests for {@link JmsHealthContributorAutoConfiguration}.
3234
*
3335
* @author Phillip Webb
36+
* @author Venkata Naga Sai Srikanth Gollapudi
3437
*/
3538
class JmsHealthContributorAutoConfigurationTests {
3639

@@ -44,6 +47,27 @@ void runShouldCreateIndicator() {
4447
this.contextRunner.run((context) -> assertThat(context).hasSingleBean(JmsHealthIndicator.class));
4548
}
4649

50+
@Test
51+
void runWhenStartTimeoutIsConfiguredShouldCreateIndicatorWithConfiguredStartTimeout() {
52+
this.contextRunner.withPropertyValues("management.health.jms.start-timeout=10ms").run((context) -> {
53+
assertThat(context).hasSingleBean(JmsHealthIndicator.class);
54+
assertThat(context).hasSingleBean(JmsHealthIndicatorProperties.class);
55+
assertThat(context.getBean(JmsHealthIndicatorProperties.class).getStartTimeout())
56+
.isEqualTo(Duration.ofMillis(10));
57+
assertThat(context.getBean(JmsHealthIndicator.class)).hasFieldOrPropertyWithValue("startTimeout",
58+
Duration.ofMillis(10));
59+
});
60+
}
61+
62+
@Test
63+
void runWhenStartTimeoutIsZeroShouldFail() {
64+
this.contextRunner.withPropertyValues("management.health.jms.start-timeout=0ms")
65+
.run((context) -> assertThat(context).hasFailed()
66+
.getFailure()
67+
.rootCause()
68+
.hasMessage("'startTimeout' must be greater than 0"));
69+
}
70+
4771
@Test
4872
void runWhenDisabledShouldNotCreateIndicator() {
4973
this.contextRunner.withPropertyValues("management.health.jms.enabled:false")

module/spring-boot-jms/src/test/java/org/springframework/boot/jms/health/JmsHealthIndicatorTests.java

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

1717
package org.springframework.boot.jms.health;
1818

19+
import java.time.Duration;
20+
1921
import jakarta.jms.Connection;
2022
import jakarta.jms.ConnectionFactory;
2123
import jakarta.jms.ConnectionMetaData;
@@ -28,6 +30,7 @@
2830
import org.springframework.boot.health.contributor.Status;
2931

3032
import static org.assertj.core.api.Assertions.assertThat;
33+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
3134
import static org.mockito.BDDMockito.given;
3235
import static org.mockito.BDDMockito.then;
3336
import static org.mockito.BDDMockito.willAnswer;
@@ -38,9 +41,33 @@
3841
* Tests for {@link JmsHealthIndicator}.
3942
*
4043
* @author Stephane Nicoll
44+
* @author Venkata Naga Sai Srikanth Gollapudi
4145
*/
4246
class JmsHealthIndicatorTests {
4347

48+
@Test
49+
@SuppressWarnings("NullAway") // Test null check
50+
void createWhenStartTimeoutIsNullThrowsException() {
51+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
52+
assertThatIllegalArgumentException().isThrownBy(() -> new JmsHealthIndicator(connectionFactory, null))
53+
.withMessage("'startTimeout' must not be null");
54+
}
55+
56+
@Test
57+
void createWhenStartTimeoutIsZeroThrowsException() {
58+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
59+
assertThatIllegalArgumentException().isThrownBy(() -> new JmsHealthIndicator(connectionFactory, Duration.ZERO))
60+
.withMessage("'startTimeout' must be greater than 0");
61+
}
62+
63+
@Test
64+
void createWhenStartTimeoutIsNegativeThrowsException() {
65+
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
66+
assertThatIllegalArgumentException()
67+
.isThrownBy(() -> new JmsHealthIndicator(connectionFactory, Duration.ofMillis(-1)))
68+
.withMessage("'startTimeout' must be greater than 0");
69+
}
70+
4471
@Test
4572
void jmsBrokerIsUp() throws JMSException {
4673
ConnectionMetaData connectionMetaData = mock(ConnectionMetaData.class);
@@ -98,6 +125,19 @@ void jmsBrokerUsesFailover() throws JMSException {
98125

99126
@Test
100127
void whenConnectionStartIsUnresponsiveStatusIsDown() throws JMSException {
128+
Health health = healthWhenConnectionStartIsUnresponsive(Duration.ofSeconds(5));
129+
assertThat(health.getStatus()).isEqualTo(Status.DOWN);
130+
assertThat((String) health.getDetails().get("error")).contains("Connection closed");
131+
}
132+
133+
@Test
134+
void whenConnectionStartIsUnresponsiveUsesConfiguredStartTimeout() throws JMSException {
135+
Health health = healthWhenConnectionStartIsUnresponsive(Duration.ofMillis(10));
136+
assertThat(health.getStatus()).isEqualTo(Status.DOWN);
137+
assertThat((String) health.getDetails().get("error")).contains("Connection closed");
138+
}
139+
140+
private Health healthWhenConnectionStartIsUnresponsive(Duration startTimeout) throws JMSException {
101141
ConnectionMetaData connectionMetaData = mock(ConnectionMetaData.class);
102142
given(connectionMetaData.getJMSProviderName()).willReturn("JMS test provider");
103143
Connection connection = mock(Connection.class);
@@ -109,10 +149,8 @@ void whenConnectionStartIsUnresponsiveStatusIsDown() throws JMSException {
109149
}).given(connection).close();
110150
ConnectionFactory connectionFactory = mock(ConnectionFactory.class);
111151
given(connectionFactory.createConnection()).willReturn(connection);
112-
JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory);
113-
Health health = indicator.health();
114-
assertThat(health.getStatus()).isEqualTo(Status.DOWN);
115-
assertThat((String) health.getDetails().get("error")).contains("Connection closed");
152+
JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory, startTimeout);
153+
return indicator.health();
116154
}
117155

118156
private static final class UnresponsiveStartAnswer implements Answer<Void> {

0 commit comments

Comments
 (0)