From 38fa5bca9e77c2e8b64c10483a84d61685427d00 Mon Sep 17 00:00:00 2001 From: Tommy Ludwig <8924140+shakuzen@users.noreply.github.com> Date: Mon, 30 Sep 2019 23:48:17 -0400 Subject: [PATCH] Polish #1223 --- ...pClientConnectionManagerMetricsBinder.java | 56 +++++-------------- ...entConnectionManagerMetricsBinderTest.java | 12 +--- 2 files changed, 16 insertions(+), 52 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/PoolingHttpClientConnectionManagerMetricsBinder.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/PoolingHttpClientConnectionManagerMetricsBinder.java index d1fb293051..df2bf88f77 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/PoolingHttpClientConnectionManagerMetricsBinder.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/httpcomponents/PoolingHttpClientConnectionManagerMetricsBinder.java @@ -18,7 +18,6 @@ import io.micrometer.core.instrument.*; import io.micrometer.core.instrument.binder.MeterBinder; import io.micrometer.core.lang.NonNull; -import org.apache.http.conn.HttpClientConnectionManager; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; /** @@ -31,28 +30,12 @@ */ public class PoolingHttpClientConnectionManagerMetricsBinder implements MeterBinder { - private static final String METER_TOTAL_MAX_DESC = "The configured maximum number of allowed persistent connections for all routes."; - private static final String METER_TOTAL_MAX = "httpcomponents.httpclient.pool.total.max"; - private static final String METER_TOTAL_CONNECTIONS_DESC = "The number of persistent and leased connections for all routes."; - private static final String METER_TOTAL_CONNECTIONS = "httpcomponents.httpclient.pool.total.connections"; - private static final String METER_TOTAL_PENDING_DESC = "The number of connection requests being blocked awaiting a free connection for all routes."; - private static final String METER_TOTAL_PENDING = "httpcomponents.httpclient.pool.total.pending"; - private static final String METER_DEFAULT_MAX_PER_ROUTE_DESC = "The configured default maximum number of allowed persistent connections per route."; - private static final String METER_DEFAULT_MAX_PER_ROUTE = "httpcomponents.httpclient.pool.route.max.default"; - private static final String TAG_CONNECTIONS_STATE = "state"; - private final PoolingHttpClientConnectionManager connectionManager; private final Iterable tags; /** * Creates a metrics binder for the given pooling connection manager. * - * For convenience this constructor will take care of casting the given - * {@link HttpClientConnectionManager} to the required {@link - * PoolingHttpClientConnectionManager}. An {@link IllegalArgumentException} - * is thrown, if the given {@code connectionManager} is not an instance of - * {@link PoolingHttpClientConnectionManager}. - * * @param connectionManager The connection manager to monitor. * @param name Name of the connection manager. Will be added as tag with the * key "httpclient". @@ -60,30 +43,21 @@ public class PoolingHttpClientConnectionManagerMetricsBinder implements MeterBin * of arguments representing key/value pairs of tags. */ @SuppressWarnings("WeakerAccess") - public PoolingHttpClientConnectionManagerMetricsBinder(HttpClientConnectionManager connectionManager, String name, String... tags) { + public PoolingHttpClientConnectionManagerMetricsBinder(PoolingHttpClientConnectionManager connectionManager, String name, String... tags) { this(connectionManager, name, Tags.of(tags)); } /** * Creates a metrics binder for the given pooling connection manager. * - * For convenience this constructor will take care of casting the given - * {@link HttpClientConnectionManager} to the required {@link - * PoolingHttpClientConnectionManager}. An {@link IllegalArgumentException} - * is thrown, if the given {@code connectionManager} is not an instance of - * {@link PoolingHttpClientConnectionManager}. - * * @param connectionManager The connection manager to monitor. * @param name Name of the connection manager. Will be added as tag with the * key "httpclient". * @param tags Tags to apply to all recorded metrics. */ @SuppressWarnings("WeakerAccess") - public PoolingHttpClientConnectionManagerMetricsBinder(HttpClientConnectionManager connectionManager, String name, Iterable tags) { - if (!(connectionManager instanceof PoolingHttpClientConnectionManager)) { - throw new IllegalArgumentException("The given connectionManager is not an instance of PoolingHttpClientConnectionManager."); - } - this.connectionManager = (PoolingHttpClientConnectionManager) connectionManager; + public PoolingHttpClientConnectionManagerMetricsBinder(PoolingHttpClientConnectionManager connectionManager, String name, Iterable tags) { + this.connectionManager = connectionManager; this.tags = Tags.concat(tags, "httpclient", name); } @@ -93,34 +67,34 @@ public void bindTo(@NonNull MeterRegistry registry) { } private void registerTotalMetrics(MeterRegistry registry) { - Gauge.builder(METER_TOTAL_MAX, + Gauge.builder("httpcomponents.httpclient.pool.total.max", connectionManager, (connectionManager) -> connectionManager.getTotalStats().getMax()) - .description(METER_TOTAL_MAX_DESC) + .description("The configured maximum number of allowed persistent connections for all routes.") .tags(tags) .register(registry); - Gauge.builder(METER_TOTAL_CONNECTIONS, + Gauge.builder("httpcomponents.httpclient.pool.total.connections", connectionManager, (connectionManager) -> connectionManager.getTotalStats().getAvailable()) - .description(METER_TOTAL_CONNECTIONS_DESC) - .tags(tags).tag(TAG_CONNECTIONS_STATE, "available") + .description("The number of persistent and leased connections for all routes.") + .tags(tags).tag("state", "available") .register(registry); - Gauge.builder(METER_TOTAL_CONNECTIONS, + Gauge.builder("httpcomponents.httpclient.pool.total.connections", connectionManager, (connectionManager) -> connectionManager.getTotalStats().getLeased()) - .description(METER_TOTAL_CONNECTIONS_DESC) - .tags(tags).tag(TAG_CONNECTIONS_STATE, "leased") + .description("The number of persistent and leased connections for all routes.") + .tags(tags).tag("state", "leased") .register(registry); - Gauge.builder(METER_TOTAL_PENDING, + Gauge.builder("httpcomponents.httpclient.pool.total.pending", connectionManager, (connectionManager) -> connectionManager.getTotalStats().getPending()) - .description(METER_TOTAL_PENDING_DESC) + .description("The number of connection requests being blocked awaiting a free connection for all routes.") .tags(tags) .register(registry); - Gauge.builder(METER_DEFAULT_MAX_PER_ROUTE, + Gauge.builder("httpcomponents.httpclient.pool.route.max.default", connectionManager, PoolingHttpClientConnectionManager::getDefaultMaxPerRoute) - .description(METER_DEFAULT_MAX_PER_ROUTE_DESC) + .description("The configured default maximum number of allowed persistent connections per route.") .tags(tags) .register(registry); } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/httpcomponents/PoolingHttpClientConnectionManagerMetricsBinderTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/httpcomponents/PoolingHttpClientConnectionManagerMetricsBinderTest.java index 94f33537b8..1ea691f5f7 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/httpcomponents/PoolingHttpClientConnectionManagerMetricsBinderTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/httpcomponents/PoolingHttpClientConnectionManagerMetricsBinderTest.java @@ -19,14 +19,12 @@ import io.micrometer.core.instrument.MockClock; import io.micrometer.core.instrument.simple.SimpleConfig; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; -import org.apache.http.conn.HttpClientConnectionManager; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.http.pool.PoolStats; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -48,17 +46,9 @@ void setup() { binder.bindTo(registry); } - @Test - void creationWithNonPoolingHttpClientThrowsException() { - assertThrows(IllegalArgumentException.class, () -> { - HttpClientConnectionManager connectionManager = mock(HttpClientConnectionManager.class); - new PoolingHttpClientConnectionManagerMetricsBinder(connectionManager, "test"); - }); - } - @Test void creationWithPoolingHttpClientIsOk() { - HttpClientConnectionManager connectionManager = mock(PoolingHttpClientConnectionManager.class); + PoolingHttpClientConnectionManager connectionManager = mock(PoolingHttpClientConnectionManager.class); new PoolingHttpClientConnectionManagerMetricsBinder(connectionManager, "test"); }