Skip to content

Commit 1f15812

Browse files
Bennett-LynchBennett-Lynch
authored
Add "ConcurrencyAcquireDuration" metric for apache-client (#2912)
* Add "ConcurrencyAcquireDuration" metric for apache-client The time taken to acquire a new channel from a channel pool can be both non-trivial and highly variable, depending upon whether a new connection needs to be established, and depending upon the overhead of new connection establishment (including TLS handshakes). The same metric was recently added for netty-nio-client: #2903
1 parent 4cee2b0 commit 1f15812

File tree

7 files changed

+235
-96
lines changed

7 files changed

+235
-96
lines changed

‎.changes/next-release/feature-ApacheHTTPClient-2ecc813.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"category": "Apache HTTP Client",
3+
"contributor": "",
4+
"type": "feature",
5+
"description": "Add \"ConcurrencyAcquireDuration\" metric for apache-client"
6+
}

‎http-client-spi/src/main/java/software/amazon/awssdk/http/HttpMetric.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,15 +109,13 @@ public final class HttpMetric {
109109
* The time taken to acquire a channel from the connection pool.
110110
*
111111
* <p>For HTTP/1 operations, a channel is equivalent to a TCP connection. For HTTP/2 operations, a channel is equivalent to
112-
* an HTTP/2 stream channel. For both protocols, the time to acquire a new concurrency permit may include the following:
112+
* an HTTP/2 stream channel. For both protocols, the time to acquire a new channel may include the following:
113113
* <ol>
114114
* <li>Awaiting a concurrency permit, as restricted by the client's max concurrency configuration.</li>
115115
* <li>The time to establish a new connection, depending on whether an existing connection is available in the pool or
116116
* not.</li>
117117
* <li>The time taken to perform a TLS handshake/negotiation, if TLS is enabled.</li>
118118
* </ol>
119-
*
120-
* <p>Note: This metric is currently only supported in 'netty-nio-client'.
121119
*/
122120
public static final SdkMetric<Duration> CONCURRENCY_ACQUIRE_DURATION =
123121
metric("ConcurrencyAcquireDuration", Duration.class, MetricLevel.INFO);

‎http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/ApacheHttpClient.java

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import static software.amazon.awssdk.http.HttpMetric.LEASED_CONCURRENCY;
2424
import static software.amazon.awssdk.http.HttpMetric.MAX_CONCURRENCY;
2525
import static software.amazon.awssdk.http.HttpMetric.PENDING_CONCURRENCY_ACQUIRES;
26+
import static software.amazon.awssdk.http.apache.internal.conn.ClientConnectionRequestFactory.THREAD_LOCAL_REQUEST_METRIC_COLLECTOR;
2627
import static software.amazon.awssdk.utils.NumericUtils.saturatedCast;
2728

2829
import java.io.IOException;
@@ -230,7 +231,7 @@ public ExecutableHttpRequest prepareRequest(HttpExecuteRequest request) {
230231
return new ExecutableHttpRequest() {
231232
@Override
232233
public HttpExecuteResponse call() throws IOException {
233-
HttpExecuteResponse executeResponse = execute(apacheRequest);
234+
HttpExecuteResponse executeResponse = execute(apacheRequest, metricCollector);
234235
collectPoolMetric(metricCollector);
235236
return executeResponse;
236237
}
@@ -249,10 +250,15 @@ public void close() {
249250
cm.shutdown();
250251
}
251252

252-
private HttpExecuteResponse execute(HttpRequestBase apacheRequest) throws IOException {
253+
private HttpExecuteResponse execute(HttpRequestBase apacheRequest, MetricCollector metricCollector) throws IOException {
253254
HttpClientContext localRequestContext = ApacheUtils.newClientContext(requestConfig.proxyConfiguration());
254-
HttpResponse httpResponse = httpClient.execute(apacheRequest, localRequestContext);
255-
return createResponse(httpResponse, apacheRequest);
255+
THREAD_LOCAL_REQUEST_METRIC_COLLECTOR.set(metricCollector);
256+
try {
257+
HttpResponse httpResponse = httpClient.execute(apacheRequest, localRequestContext);
258+
return createResponse(httpResponse, apacheRequest);
259+
} finally {
260+
THREAD_LOCAL_REQUEST_METRIC_COLLECTOR.remove();
261+
}
256262
}
257263

258264
private HttpRequestBase toApacheRequest(HttpExecuteRequest request) {

‎http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/conn/ClientConnectionManagerFactory.java

Lines changed: 66 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,17 @@
1515

1616
package software.amazon.awssdk.http.apache.internal.conn;
1717

18-
import java.lang.reflect.InvocationHandler;
19-
import java.lang.reflect.InvocationTargetException;
20-
import java.lang.reflect.Method;
21-
import java.lang.reflect.Proxy;
18+
import java.io.IOException;
19+
import java.util.concurrent.TimeUnit;
20+
import org.apache.http.HttpClientConnection;
2221
import org.apache.http.conn.ConnectionRequest;
2322
import org.apache.http.conn.HttpClientConnectionManager;
24-
import org.apache.http.pool.ConnPoolControl;
25-
import org.slf4j.Logger;
26-
import org.slf4j.LoggerFactory;
23+
import org.apache.http.conn.routing.HttpRoute;
24+
import org.apache.http.protocol.HttpContext;
2725
import software.amazon.awssdk.annotations.SdkInternalApi;
2826

2927
@SdkInternalApi
3028
public final class ClientConnectionManagerFactory {
31-
private static final Logger log = LoggerFactory.getLogger(ClientConnectionManagerFactory.class);
3229

3330
private ClientConnectionManagerFactory() {
3431
}
@@ -40,52 +37,78 @@ private ClientConnectionManagerFactory() {
4037
* @param orig the target instance to be wrapped
4138
*/
4239
public static HttpClientConnectionManager wrap(HttpClientConnectionManager orig) {
43-
if (orig instanceof Wrapped) {
40+
if (orig instanceof DelegatingHttpClientConnectionManager) {
4441
throw new IllegalArgumentException();
4542
}
46-
Class<?>[] interfaces;
47-
if (orig instanceof ConnPoolControl) {
48-
interfaces = new Class<?>[]{
49-
HttpClientConnectionManager.class,
50-
ConnPoolControl.class,
51-
Wrapped.class};
52-
} else {
53-
interfaces = new Class<?>[]{
54-
HttpClientConnectionManager.class,
55-
Wrapped.class
56-
};
43+
return new InstrumentedHttpClientConnectionManager(orig);
44+
}
45+
46+
/**
47+
* Further wraps {@link ConnectionRequest} to capture performance metrics.
48+
*/
49+
private static class InstrumentedHttpClientConnectionManager extends DelegatingHttpClientConnectionManager {
50+
51+
private InstrumentedHttpClientConnectionManager(HttpClientConnectionManager delegate) {
52+
super(delegate);
53+
}
54+
55+
@Override
56+
public ConnectionRequest requestConnection(HttpRoute route, Object state) {
57+
ConnectionRequest connectionRequest = super.requestConnection(route, state);
58+
return ClientConnectionRequestFactory.wrap(connectionRequest);
5759
}
58-
return (HttpClientConnectionManager) Proxy.newProxyInstance(
59-
// https://github.com/aws/aws-sdk-java/pull/48#issuecomment-29454423
60-
ClientConnectionManagerFactory.class.getClassLoader(),
61-
interfaces,
62-
new Handler(orig));
6360
}
6461

6562
/**
66-
* The handler behind the dynamic proxy for {@link HttpClientConnectionManager}
67-
* so that the any returned instance of {@link ConnectionRequest} can
68-
* further wrapped for capturing performance metrics.
63+
* Delegates all methods to {@link HttpClientConnectionManager}. Subclasses can override select methods to change behavior.
6964
*/
70-
private static class Handler implements InvocationHandler {
71-
private final HttpClientConnectionManager orig;
65+
private static class DelegatingHttpClientConnectionManager implements HttpClientConnectionManager {
66+
67+
private final HttpClientConnectionManager delegate;
68+
69+
protected DelegatingHttpClientConnectionManager(HttpClientConnectionManager delegate) {
70+
this.delegate = delegate;
71+
}
72+
73+
@Override
74+
public ConnectionRequest requestConnection(HttpRoute route, Object state) {
75+
return delegate.requestConnection(route, state);
76+
}
77+
78+
@Override
79+
public void releaseConnection(HttpClientConnection conn, Object newState, long validDuration, TimeUnit timeUnit) {
80+
delegate.releaseConnection(conn, newState, validDuration, timeUnit);
81+
}
82+
83+
@Override
84+
public void connect(HttpClientConnection conn, HttpRoute route, int connectTimeout, HttpContext context)
85+
throws IOException {
86+
delegate.connect(conn, route, connectTimeout, context);
87+
}
88+
89+
@Override
90+
public void upgrade(HttpClientConnection conn, HttpRoute route, HttpContext context) throws IOException {
91+
delegate.upgrade(conn, route, context);
92+
}
7293

73-
Handler(HttpClientConnectionManager real) {
74-
this.orig = real;
94+
@Override
95+
public void routeComplete(HttpClientConnection conn, HttpRoute route, HttpContext context) throws IOException {
96+
delegate.routeComplete(conn, route, context);
97+
}
98+
99+
@Override
100+
public void closeIdleConnections(long idletime, TimeUnit timeUnit) {
101+
delegate.closeIdleConnections(idletime, timeUnit);
102+
}
103+
104+
@Override
105+
public void closeExpiredConnections() {
106+
delegate.closeExpiredConnections();
75107
}
76108

77109
@Override
78-
public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
79-
try {
80-
Object ret = method.invoke(orig, args);
81-
return ret instanceof ConnectionRequest
82-
? ClientConnectionRequestFactory.wrap((ConnectionRequest) ret)
83-
: ret
84-
;
85-
} catch (InvocationTargetException e) {
86-
log.debug("", e);
87-
throw e.getCause();
88-
}
110+
public void shutdown() {
111+
delegate.shutdown();
89112
}
90113
}
91114
}

‎http-clients/apache-client/src/main/java/software/amazon/awssdk/http/apache/internal/conn/ClientConnectionRequestFactory.java

Lines changed: 52 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,25 @@
1616

1717
package software.amazon.awssdk.http.apache.internal.conn;
1818

19-
import java.lang.reflect.InvocationHandler;
20-
import java.lang.reflect.InvocationTargetException;
21-
import java.lang.reflect.Method;
22-
import java.lang.reflect.Proxy;
19+
import java.time.Duration;
20+
import java.time.Instant;
21+
import java.util.concurrent.ExecutionException;
22+
import java.util.concurrent.TimeUnit;
23+
import org.apache.http.HttpClientConnection;
24+
import org.apache.http.conn.ConnectionPoolTimeoutException;
2325
import org.apache.http.conn.ConnectionRequest;
24-
import org.slf4j.Logger;
25-
import org.slf4j.LoggerFactory;
2626
import software.amazon.awssdk.annotations.SdkInternalApi;
27+
import software.amazon.awssdk.http.HttpMetric;
28+
import software.amazon.awssdk.http.apache.ApacheHttpClient;
29+
import software.amazon.awssdk.metrics.MetricCollector;
2730

2831
@SdkInternalApi
29-
final class ClientConnectionRequestFactory {
30-
private static final Logger log = LoggerFactory.getLogger(ClientConnectionRequestFactory.class);
31-
private static final Class<?>[] INTERFACES = {
32-
ConnectionRequest.class,
33-
Wrapped.class
34-
};
32+
public final class ClientConnectionRequestFactory {
33+
34+
/**
35+
* {@link ThreadLocal}, request-level {@link MetricCollector}, set and removed by {@link ApacheHttpClient}.
36+
*/
37+
public static final ThreadLocal<MetricCollector> THREAD_LOCAL_REQUEST_METRIC_COLLECTOR = new ThreadLocal<>();
3538

3639
private ClientConnectionRequestFactory() {
3740
}
@@ -43,48 +46,55 @@ private ClientConnectionRequestFactory() {
4346
* @param orig the target instance to be wrapped
4447
*/
4548
static ConnectionRequest wrap(ConnectionRequest orig) {
46-
if (orig instanceof Wrapped) {
49+
if (orig instanceof DelegatingConnectionRequest) {
4750
throw new IllegalArgumentException();
4851
}
49-
return (ConnectionRequest) Proxy.newProxyInstance(
50-
// https://github.com/aws/aws-sdk-java/pull/48#issuecomment-29454423
51-
ClientConnectionRequestFactory.class.getClassLoader(),
52-
INTERFACES,
53-
new Handler(orig));
52+
return new InstrumentedConnectionRequest(orig);
5453
}
5554

5655
/**
57-
* The handler behind the dynamic proxy for {@link ConnectionRequest}
58-
* so that the latency of the
59-
* {@link ConnectionRequest#get(long, java.util.concurrent.TimeUnit)}
60-
* can be captured.
56+
* Measures the latency of {@link ConnectionRequest#get(long, java.util.concurrent.TimeUnit)}.
6157
*/
62-
private static class Handler implements InvocationHandler {
63-
private final ConnectionRequest orig;
58+
private static class InstrumentedConnectionRequest extends DelegatingConnectionRequest {
6459

65-
Handler(ConnectionRequest orig) {
66-
this.orig = orig;
60+
private InstrumentedConnectionRequest(ConnectionRequest delegate) {
61+
super(delegate);
6762
}
6863

6964
@Override
70-
public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
65+
public HttpClientConnection get(long timeout, TimeUnit timeUnit) throws InterruptedException, ExecutionException,
66+
ConnectionPoolTimeoutException {
67+
Instant startTime = Instant.now();
7168
try {
72-
// TODO v2 service metrics
73-
// if ("get".equals(method.getName())) {
74-
// ServiceLatencyProvider latencyProvider = new ServiceLatencyProvider(
75-
// AWSServiceMetrics.HttpClientGetConnectionTime);
76-
// try {
77-
// return method.invoke(orig, args);
78-
// } finally {
79-
// AwsSdkMetrics.getServiceMetricCollector()
80-
// .collectLatency(latencyProvider.endTiming());
81-
// }
82-
// }
83-
return method.invoke(orig, args);
84-
} catch (InvocationTargetException e) {
85-
log.debug("", e);
86-
throw e.getCause();
69+
return super.get(timeout, timeUnit);
70+
} finally {
71+
Duration elapsed = Duration.between(startTime, Instant.now());
72+
MetricCollector metricCollector = THREAD_LOCAL_REQUEST_METRIC_COLLECTOR.get();
73+
metricCollector.reportMetric(HttpMetric.CONCURRENCY_ACQUIRE_DURATION, elapsed);
8774
}
8875
}
8976
}
77+
78+
/**
79+
* Delegates all methods to {@link ConnectionRequest}. Subclasses can override select methods to change behavior.
80+
*/
81+
private static class DelegatingConnectionRequest implements ConnectionRequest {
82+
83+
private final ConnectionRequest delegate;
84+
85+
private DelegatingConnectionRequest(ConnectionRequest delegate) {
86+
this.delegate = delegate;
87+
}
88+
89+
@Override
90+
public HttpClientConnection get(long timeout, TimeUnit timeUnit)
91+
throws InterruptedException, ExecutionException, ConnectionPoolTimeoutException {
92+
return delegate.get(timeout, timeUnit);
93+
}
94+
95+
@Override
96+
public boolean cancel() {
97+
return delegate.cancel();
98+
}
99+
}
90100
}

0 commit comments

Comments
 (0)