Polish "Add metrics support for idle jdbc connections"

See gh-17504

Author: snicoll

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/jdbc/DataSourcePoolMetrics.java

@@ -38,7 +38,6 @@
  *
  * @author Jon Schneider
  * @author Phillip Webb
- * @author Artsiom Yudovin
  * @since 2.0.0
  */
 public class DataSourcePoolMetrics implements MeterBinder {
@@ -67,9 +66,9 @@ public DataSourcePoolMetrics(DataSource dataSource, DataSourcePoolMetadataProvid
 	public void bindTo(MeterRegistry registry) {
 		if (this.metadataProvider.getDataSourcePoolMetadata(this.dataSource) != null) {
 			bindPoolMetadata(registry, "active", DataSourcePoolMetadata::getActive);
+			bindPoolMetadata(registry, "idle", DataSourcePoolMetadata::getIdle);
 			bindPoolMetadata(registry, "max", DataSourcePoolMetadata::getMax);
 			bindPoolMetadata(registry, "min", DataSourcePoolMetadata::getMin);
-			bindPoolMetadata(registry, "idle", DataSourcePoolMetadata::getIdle);
 		}
 	}
 

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

@@ -2023,8 +2023,8 @@ is required. A `CacheMetricsRegistrar` bean is made available to make that proce
 ==== DataSource Metrics
 Auto-configuration enables the instrumentation of all available `DataSource` objects with
 a metric named `jdbc`. Data source instrumentation results in gauges representing the
-currently active, maximum allowed, and minimum allowed connections in the pool. Each of
-these gauges has a name that is prefixed by `jdbc`.
+currently active, idle, maximum allowed, and minimum allowed connections in the pool. Each
+of these gauges has a name that is prefixed by `jdbc`.
 
 Metrics are also tagged by the name of the `DataSource` computed based on the bean name.
 

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/CommonsDbcp2DataSourcePoolMetadata.java

@@ -24,7 +24,6 @@
  * {@link DataSourcePoolMetadata} for an Apache Commons DBCP2 {@link DataSource}.
  *
  * @author Stephane Nicoll
- * @author Artsiom Yudovin
  * @since 2.0.0
  */
 public class CommonsDbcp2DataSourcePoolMetadata extends AbstractDataSourcePoolMetadata<BasicDataSource> {
@@ -38,6 +37,11 @@ public Integer getActive() {
 		return getDataSource().getNumActive();
 	}
 
+	@Override
+	public Integer getIdle() {
+		return getDataSource().getNumIdle();
+	}
+
 	@Override
 	public Integer getMax() {
 		return getDataSource().getMaxTotal();
@@ -58,9 +62,4 @@ public Boolean getDefaultAutoCommit() {
 		return getDataSource().getDefaultAutoCommit();
 	}
 
-	@Override
-	public Integer getIdle() {
-		return getDataSource().getNumIdle();
-	}
-
 }

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/DataSourcePoolMetadata.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -55,6 +55,7 @@ public interface DataSourcePoolMetadata {
 	 * if that information is not available.
 	 * @return the number of established but idle connections or {@code null}
 	 * @since 2.2.0
+	 * @see #getActive()
 	 */
 	default Integer getIdle() {
 		return null;

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/HikariDataSourcePoolMetadata.java

@@ -16,8 +16,6 @@
 
 package org.springframework.boot.jdbc.metadata;
 
-import java.util.Objects;
-
 import javax.sql.DataSource;
 
 import com.zaxxer.hikari.HikariDataSource;
@@ -29,7 +27,6 @@
  * {@link DataSourcePoolMetadata} for a Hikari {@link DataSource}.
  *
  * @author Stephane Nicoll
- * @author Artsiom Yudovin
  * @since 2.0.0
  */
 public class HikariDataSourcePoolMetadata extends AbstractDataSourcePoolMetadata<HikariDataSource> {
@@ -50,12 +47,12 @@ public Integer getActive() {
 
 	@Override
 	public Integer getIdle() {
-		HikariPool pool = getHikariPool();
-		if (Objects.nonNull(pool)) {
-			return pool.getIdleConnections();
+		try {
+			return getHikariPool().getIdleConnections();
+		}
+		catch (Exception ex) {
+			return null;
 		}
-
-		return null;
 	}
 
 	private HikariPool getHikariPool() {

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/jdbc/metadata/TomcatDataSourcePoolMetadata.java

@@ -23,7 +23,6 @@
  * {@link DataSourcePoolMetadata} for a Tomcat DataSource.
  *
  * @author Stephane Nicoll
- * @author Artsiom Yudovin
  * @since 2.0.0
  */
 public class TomcatDataSourcePoolMetadata extends AbstractDataSourcePoolMetadata<DataSource> {
@@ -38,6 +37,11 @@ public Integer getActive() {
 		return (pool != null) ? pool.getActive() : 0;
 	}
 
+	@Override
+	public Integer getIdle() {
+		return getDataSource().getNumIdle();
+	}
+
 	@Override
 	public Integer getMax() {
 		return getDataSource().getMaxActive();
@@ -58,9 +62,4 @@ public Boolean getDefaultAutoCommit() {
 		return getDataSource().isDefaultAutoCommit();
 	}
 
-	@Override
-	public Integer getIdle() {
-		return getDataSource().getNumIdle();
-	}
-
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/AbstractDataSourcePoolMetadataTests.java

@@ -34,7 +34,8 @@
 abstract class AbstractDataSourcePoolMetadataTests<D extends AbstractDataSourcePoolMetadata<?>> {
 
 	/**
-	 * Return a data source metadata instance with a min size of 0 and max size of 2.
+	 * Return a data source metadata instance with a min size of 0 and max size of 2. Idle
+	 * connections are not reclaimed immediately.
 	 * @return the data source metadata
 	 */
 	protected abstract D getDataSourceMetadata();
@@ -70,6 +71,8 @@ void getPoolSizeOneConnection() {
 
 	@Test
 	void getIdle() {
+		JdbcTemplate jdbcTemplate = new JdbcTemplate(getDataSourceMetadata().getDataSource());
+		jdbcTemplate.execute((ConnectionCallback<Void>) (connection) -> null);
 		assertThat(getDataSourceMetadata().getIdle()).isEqualTo(Integer.valueOf(1));
 	}
 

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/CommonsDbcp2DataSourcePoolMetadataTests.java

@@ -19,9 +19,6 @@
 import org.apache.commons.dbcp2.BasicDataSource;
 import org.junit.jupiter.api.Test;
 
-import org.springframework.jdbc.core.ConnectionCallback;
-import org.springframework.jdbc.core.JdbcTemplate;
-
 import static org.assertj.core.api.Assertions.assertThat;
 
 /**
@@ -86,19 +83,12 @@ private CommonsDbcp2DataSourcePoolMetadata createDataSourceMetadata(int minSize,
 		BasicDataSource dataSource = createDataSource();
 		dataSource.setMinIdle(minSize);
 		dataSource.setMaxTotal(maxSize);
-
-		this.initPool(dataSource);
-
+		dataSource.setMinEvictableIdleTimeMillis(5000);
 		return new CommonsDbcp2DataSourcePoolMetadata(dataSource);
 	}
 
 	private BasicDataSource createDataSource() {
 		return initializeBuilder().type(BasicDataSource.class).build();
 	}
 
-	private void initPool(BasicDataSource dataSource) {
-		JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource);
-		jdbcTemplate.execute((ConnectionCallback<Void>) (connection) -> null);
-	}
-
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/HikariDataSourcePoolMetadataTests.java

@@ -18,16 +18,12 @@
 
 import com.zaxxer.hikari.HikariDataSource;
 
-import org.springframework.jdbc.core.ConnectionCallback;
-import org.springframework.jdbc.core.JdbcTemplate;
-
 import static org.assertj.core.api.Assertions.assertThat;
 
 /**
  * Tests for {@link HikariDataSourcePoolMetadata}.
  *
  * @author Stephane Nicoll
- * @author Artsiom Yudovin
  */
 public class HikariDataSourcePoolMetadataTests
 		extends AbstractDataSourcePoolMetadataTests<HikariDataSourcePoolMetadata> {
@@ -58,15 +54,8 @@ private HikariDataSource createDataSource(int minSize, int maxSize) {
 		HikariDataSource dataSource = initializeBuilder().type(HikariDataSource.class).build();
 		dataSource.setMinimumIdle(minSize);
 		dataSource.setMaximumPoolSize(maxSize);
-
-		this.initPool(dataSource);
-
+		dataSource.setIdleTimeout(5000);
 		return dataSource;
 	}
 
-	private void initPool(HikariDataSource dataSource) {
-		JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource);
-		jdbcTemplate.execute((ConnectionCallback<Void>) (connection) -> null);
-	}
-
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/jdbc/metadata/TomcatDataSourcePoolMetadataTests.java

@@ -18,9 +18,6 @@
 
 import org.apache.tomcat.jdbc.pool.DataSource;
 
-import org.springframework.jdbc.core.ConnectionCallback;
-import org.springframework.jdbc.core.JdbcTemplate;
-
 import static org.assertj.core.api.Assertions.assertThat;
 
 /**
@@ -57,19 +54,12 @@ private DataSource createDataSource(int minSize, int maxSize) {
 		DataSource dataSource = initializeBuilder().type(DataSource.class).build();
 		dataSource.setMinIdle(minSize);
 		dataSource.setMaxActive(maxSize);
+		dataSource.setMinEvictableIdleTimeMillis(5000);
 
 		// Avoid warnings
 		dataSource.setInitialSize(minSize);
 		dataSource.setMaxIdle(maxSize);
-
-		this.initPool(dataSource);
-
 		return dataSource;
 	}
 
-	private void initPool(DataSource dataSource) {
-		JdbcTemplate jdbcTemplate = new JdbcTemplate(dataSource);
-		jdbcTemplate.execute((ConnectionCallback<Void>) (connection) -> null);
-	}
-
 }