Skip to content

Commit

Permalink
Polish, report active connections rather than total (fixes #2066, fixes
Browse files Browse the repository at this point in the history 
#1875)
  • Loading branch information
Jon Schneider committed May 8, 2020
1 parent a1b04f5 commit 56ae455
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 63 deletions.
83 changes: 66 additions & 17 deletions ...c/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpConnectionPoolMetrics.java
View file
Original file line number Diff line number Diff line change
@@ -1,23 +1,43 @@
/**
* Copyright 2020 VMware, Inc.
* <p>
* 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
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <p>
* 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;
import io.micrometer.core.instrument.MeterRegistry;
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}.
*
* <p>
* Example usage:
* <pre>
* return new ConnectionPool(connectionPoolSize, connectionPoolKeepAliveMs, TimeUnit.MILLISECONDS);
* new OkHttpConnectionPoolMetrics(connectionPool, "okhttp.pool", Tags.of());
* </pre>
*
* @author Ben Hubert
* @since 1.6.0
*/
public class OkHttpConnectionPoolMetrics implements MeterBinder {

Expand All @@ -27,6 +47,7 @@ public class OkHttpConnectionPoolMetrics implements MeterBinder {
private final String name;
private final Iterable<Tag> tags;
private final Double maxIdleConnectionCount;
private ConnectionPoolConnectionStats connectionStats = new ConnectionPoolConnectionStats();

/**
* Creates a meter binder for the given connection pool.
Expand All @@ -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.
Expand Down Expand Up @@ -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<Tag> tags, Integer maxIdleConnections) {
Expand All @@ -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;
Expand All @@ -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);
}
}
}
}
94 changes: 48 additions & 46 deletions ...st/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpConnectionPoolMetricsTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
/**
* Copyright 2020 VMware, Inc.
* <p>
* 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
* <p>
* https://www.apache.org/licenses/LICENSE-2.0
* <p>
* 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;
Expand All @@ -7,59 +22,36 @@
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;
import static org.junit.jupiter.api.Assertions.assertThrows;
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
Expand All @@ -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
}
Expand All @@ -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);
}

Expand All @@ -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());
}

}

0 comments on commit 56ae455

Please sign in to comment.