Carter Kozak created HTTPCLIENT-2305:
----------------------------------------
Summary: Proposal: SSLConnectionSocketFactory refactor to allow
custom timer metrics on socket.connect
Key: HTTPCLIENT-2305
URL: https://issues.apache.org/jira/browse/HTTPCLIENT-2305
Project: HttpComponents HttpClient
Issue Type: Improvement
Components: HttpClient (classic)
Affects Versions: 5.2.1
Reporter: Carter Kozak
I'd like to add a timer metric around socket.connect in one of my classic
blocking clients.
Usually, I can implement such a thing by subclassing or introducing a
delagating implementation, adding a small piece of code to record metrics
before delegating to the original implementation -- this works incredibly well
for PoolingHttpClientConnectionManager
[here|https://github.com/palantir/dialogue/blob/cc36ec0494dc19db6d66be0cdbefbe34af2a3283/dialogue-apache-hc5-client/src/main/java/com/palantir/dialogue/hc5/InstrumentedPoolingHttpClientConnectionManager.java#L121-L141],
for example, which covers the full connect + handshake process across all
ipaddrs resolved for the provided hostname.
Unfortunately, it's quite a bit more challenging to implement similar metrics
over individual `Socket.connect` calls because they occur within a single large
and complex method in SSLConnectionSocketFactory, which must delegate to other
private methods:
https://github.com/apache/httpcomponents-client/blob/19f3922b37369431d379ce57fa187740b22436cf/httpclient5/src/main/java/org/apache/hc/client5/http/ssl/SSLConnectionSocketFactory.java#L223-L266
Proposal:
I would like to introduce a new method along these lines:
{code:java}
void connectSocket(
Socket sock,
Timeout connectTimeout,
HttpContext context)
throws IOException
{code}
This way I can override the method and delegate to the builtin implementation,
while timing only the socket.connect component of connection establishment.
Open questions:
1. Would you accept such a change?
2. The proposed example above uses the 'connectSocket' method name which would
be overloaded with existing methods on ConnectionSocketFactory. Perhaps a more
specific name like 'connectSocketTo' would be helpful here? Either way, I would
need to provide javadoc describing the guarantee that only 'socket.connect' is
handled.
3. Should such a new method be added as a default interface method on
ConnectionSocketFactory, or a new protected method on the
SSLConnectionSocketFactory implementation? Updating the implementation is the
simpler change, but providing the method on the interface may be more
extensible.
Feedback is always appreciated, thanks!
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org
