Details
-
Improvement
-
Status: Closed
-
Minor
-
Resolution: Fixed
-
5.2.1
-
None
Description
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, 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:
void connectSocket(
Socket sock,
Timeout connectTimeout,
HttpContext context)
throws IOException
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!
