diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpConnectionPoolMetrics.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpConnectionPoolMetrics.java index c43d36d39a..3ef0dbea25 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpConnectionPoolMetrics.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpConnectionPoolMetrics.java @@ -1,3 +1,18 @@ +/** + * Copyright 2020 VMware, Inc. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package io.micrometer.core.instrument.binder.okhttp3; import io.micrometer.core.instrument.Gauge; @@ -5,19 +20,24 @@ import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tags; import io.micrometer.core.instrument.binder.MeterBinder; +import io.micrometer.core.lang.NonNull; import okhttp3.ConnectionPool; import java.util.Collections; import java.util.Optional; +import java.util.concurrent.CountDownLatch; /** * MeterBinder for collecting metrics of a given OkHttp {@link ConnectionPool}. - * + *

* Example usage: *

  *     return new ConnectionPool(connectionPoolSize, connectionPoolKeepAliveMs, TimeUnit.MILLISECONDS);
  *     new OkHttpConnectionPoolMetrics(connectionPool, "okhttp.pool", Tags.of());
  * 
+ * + * @author Ben Hubert + * @since 1.6.0 */ public class OkHttpConnectionPoolMetrics implements MeterBinder { @@ -27,6 +47,7 @@ public class OkHttpConnectionPoolMetrics implements MeterBinder { private final String name; private final Iterable tags; private final Double maxIdleConnectionCount; + private ConnectionPoolConnectionStats connectionStats = new ConnectionPoolConnectionStats(); /** * Creates a meter binder for the given connection pool. @@ -38,16 +59,6 @@ public OkHttpConnectionPoolMetrics(ConnectionPool connectionPool) { this(connectionPool, DEFAULT_NAME, Collections.emptyList(), null); } - /** - * Creates a meter binder for the given connection pool. - * - * @param connectionPool The connection pool to monitor. Must not be null. - * @param name The desired name for the exposed metrics. Must not be null. - */ - public OkHttpConnectionPoolMetrics(ConnectionPool connectionPool, String name) { - this(connectionPool, name, Collections.emptyList(), null); - } - /** * Creates a meter binder for the given connection pool. * Metrics will be exposed using {@link #DEFAULT_NAME} as name. @@ -78,7 +89,7 @@ public OkHttpConnectionPoolMetrics(ConnectionPool connectionPool, String name, I * @param tags A list of tags which will be passed for all meters. Must not be null. * @param maxIdleConnections The maximum number of idle connections this pool will hold. This * value is passed to the {@link ConnectionPool} constructor but is - * not exposed by this instance. Therefore this meter allows to pass + * not exposed by this instance. Therefore this binder allows to pass * it, to be able to monitor it. */ public OkHttpConnectionPoolMetrics(ConnectionPool connectionPool, String name, Iterable tags, Integer maxIdleConnections) { @@ -91,6 +102,7 @@ public OkHttpConnectionPoolMetrics(ConnectionPool connectionPool, String name, I if (tags == null) { throw new IllegalArgumentException("Given list of tags must not be null."); } + this.connectionPool = connectionPool; this.name = name; this.tags = tags; @@ -100,18 +112,55 @@ public OkHttpConnectionPoolMetrics(ConnectionPool connectionPool, String name, I } @Override - public void bindTo(MeterRegistry registry) { - Gauge.builder(name + ".connection.count", () -> Integer.valueOf(connectionPool.connectionCount()).doubleValue()) - .tags(Tags.of(tags).and("state", "total")) + public void bindTo(@NonNull MeterRegistry registry) { + Gauge.builder(name + ".connection.count", connectionStats, ConnectionPoolConnectionStats::getActiveCount) + .baseUnit("connections") + .description("The state of connections in the OkHttp connection pool") + .tags(Tags.of(tags).and("state", "active")) .register(registry); - Gauge.builder(name + ".connection.count", () -> Integer.valueOf(connectionPool.idleConnectionCount()).doubleValue()) + + Gauge.builder(name + ".connection.count", connectionStats, ConnectionPoolConnectionStats::getIdleConnectionCount) + .baseUnit("connections") + .description("The state of connections in the OkHttp connection pool") .tags(Tags.of(tags).and("state", "idle")) .register(registry); + if (this.maxIdleConnectionCount != null) { Gauge.builder(name + ".connection.limit", () -> this.maxIdleConnectionCount) - .tags(Tags.of(tags).and("state", "idle")) + .baseUnit("connections") + .description("The maximum idle connection count in an OkHttp connection pool.") + .tags(Tags.concat(tags, "state", "idle")) .register(registry); } } + /** + * Allow us to coordinate between active and idle, making sure they always sum to the total available connections. + * Since we're calculating active from total-idle, we want to synchronize on idle to make sure the sum is accurate. + */ + private final class ConnectionPoolConnectionStats { + private volatile CountDownLatch uses = new CountDownLatch(0); + private int idle; + private int total; + + public int getActiveCount() { + snapshotStatsIfNecessary(); + uses.countDown(); + return total - idle; + } + + public int getIdleConnectionCount() { + snapshotStatsIfNecessary(); + uses.countDown(); + return idle; + } + + private synchronized void snapshotStatsIfNecessary() { + if (uses.getCount() == 0) { + idle = connectionPool.idleConnectionCount(); + total = connectionPool.connectionCount(); + uses = new CountDownLatch(2); + } + } + } } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpConnectionPoolMetricsTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpConnectionPoolMetricsTest.java index 8e65a08e44..2aa52227bb 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpConnectionPoolMetricsTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpConnectionPoolMetricsTest.java @@ -1,3 +1,18 @@ +/** + * Copyright 2020 VMware, Inc. + *

+ * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package io.micrometer.core.instrument.binder.okhttp3; import io.micrometer.core.instrument.MeterRegistry; @@ -7,7 +22,6 @@ import io.micrometer.core.instrument.simple.SimpleConfig; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import okhttp3.ConnectionPool; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -15,51 +29,29 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +/** + * @author Ben Hubert + */ class OkHttpConnectionPoolMetricsTest { - - private MeterRegistry registry; - private ConnectionPool connectionPool; - - @BeforeEach - void setup() { - registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock()); - connectionPool = mock(ConnectionPool.class); - } + private final MeterRegistry registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock()); + private final ConnectionPool connectionPool = mock(ConnectionPool.class); @Test void creationWithNullConnectionPoolThrowsException() { - assertThrows(IllegalArgumentException.class, () -> { - new OkHttpConnectionPoolMetrics(null); - }); - assertThrows(IllegalArgumentException.class, () -> { - new OkHttpConnectionPoolMetrics(null, "irrelevant"); - }); - assertThrows(IllegalArgumentException.class, () -> { - new OkHttpConnectionPoolMetrics(null, Tags.empty()); - }); - assertThrows(IllegalArgumentException.class, () -> { - new OkHttpConnectionPoolMetrics(null, "irrelevant", Tags.empty()); - }); + assertThrows(IllegalArgumentException.class, () -> new OkHttpConnectionPoolMetrics(null)); + assertThrows(IllegalArgumentException.class, () -> new OkHttpConnectionPoolMetrics(null, Tags.empty())); + assertThrows(IllegalArgumentException.class, () -> new OkHttpConnectionPoolMetrics(null, "irrelevant", Tags.empty())); } @Test void creationWithNullNameThrowsException() { - assertThrows(IllegalArgumentException.class, () -> { - new OkHttpConnectionPoolMetrics(connectionPool, (String) null); - }); - assertThrows(IllegalArgumentException.class, () -> { - new OkHttpConnectionPoolMetrics(connectionPool, null, Tags.empty()); - }); + assertThrows(IllegalArgumentException.class, () -> new OkHttpConnectionPoolMetrics(connectionPool, null, Tags.empty())); } @Test void creationWithNullTagsThrowsException() { - assertThrows(IllegalArgumentException.class, () -> { - new OkHttpConnectionPoolMetrics(connectionPool, (Tags) null); - }); - assertThrows(IllegalArgumentException.class, () -> { - new OkHttpConnectionPoolMetrics(connectionPool, "irrelevant.name", null); - }); + assertThrows(IllegalArgumentException.class, () -> new OkHttpConnectionPoolMetrics(connectionPool, null)); + assertThrows(IllegalArgumentException.class, () -> new OkHttpConnectionPoolMetrics(connectionPool, "irrelevant.name", null)); } @Test @@ -71,14 +63,14 @@ void instanceUsesDefaultName() { @Test void instanceUsesDefaultNameAndGivenTag() { - OkHttpConnectionPoolMetrics instance = new OkHttpConnectionPoolMetrics(connectionPool, Tags.of("foo", "bar")); + OkHttpConnectionPoolMetrics instance = new OkHttpConnectionPoolMetrics(connectionPool, Tags.of("foo", "bar")); instance.bindTo(registry); registry.get("okhttp.pool.connection.count").tags("foo", "bar"); // does not throw MeterNotFoundException } @Test void instanceUsesGivenName() { - OkHttpConnectionPoolMetrics instance = new OkHttpConnectionPoolMetrics(connectionPool, "some.meter"); + OkHttpConnectionPoolMetrics instance = new OkHttpConnectionPoolMetrics(connectionPool, "some.meter", Tags.empty()); instance.bindTo(registry); registry.get("some.meter.connection.count"); // does not throw MeterNotFoundException } @@ -91,22 +83,35 @@ void instanceUsesGivenNameAndTag() { } @Test - void total() { + void active() { OkHttpConnectionPoolMetrics instance = new OkHttpConnectionPoolMetrics(connectionPool, Tags.of("foo", "bar")); instance.bindTo(registry); when(connectionPool.connectionCount()).thenReturn(17); + when(connectionPool.idleConnectionCount()).thenReturn(10, 9); + + assertThat(registry.get("okhttp.pool.connection.count") + .tags(Tags.of("foo", "bar", "state", "active")) + .gauge().value()).isEqualTo(7.0); assertThat(registry.get("okhttp.pool.connection.count") - .tags(Tags.of("foo", "bar").and("state", "total")) - .gauge().value()).isEqualTo(17.0); + .tags(Tags.of("foo", "bar", "state", "idle")) + .gauge().value()).isEqualTo(10.0); + + assertThat(registry.get("okhttp.pool.connection.count") + .tags(Tags.of("foo", "bar", "state", "idle")) + .gauge().value()).isEqualTo(9.0); + assertThat(registry.get("okhttp.pool.connection.count") + .tags(Tags.of("foo", "bar", "state", "active")) + .gauge().value()).isEqualTo(8.0); } @Test void idle() { OkHttpConnectionPoolMetrics instance = new OkHttpConnectionPoolMetrics(connectionPool, Tags.of("foo", "bar")); instance.bindTo(registry); + when(connectionPool.connectionCount()).thenReturn(20); when(connectionPool.idleConnectionCount()).thenReturn(13); assertThat(registry.get("okhttp.pool.connection.count") - .tags(Tags.of("foo", "bar").and("state", "idle")) + .tags(Tags.of("foo", "bar", "state", "idle")) .gauge().value()).isEqualTo(13.0); } @@ -123,11 +128,8 @@ void maxIfGiven() { void maxIfNotGiven() { OkHttpConnectionPoolMetrics instance = new OkHttpConnectionPoolMetrics(connectionPool, "huge.pool", Tags.of("foo", "bar"), null); instance.bindTo(registry); - assertThrows(MeterNotFoundException.class, () -> { - registry.get("huge.pool.connection.limit") - .tags(Tags.of("foo", "bar")) - .gauge(); - }); + assertThrows(MeterNotFoundException.class, () -> registry.get("huge.pool.connection.limit") + .tags(Tags.of("foo", "bar")) + .gauge()); } - }