Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apache HTTP client instrumentation #533

Closed
shakuzen opened this issue Apr 3, 2018 · 9 comments
Closed

Apache HTTP client instrumentation #533

shakuzen opened this issue Apr 3, 2018 · 9 comments
Labels
enhancement A general enhancement
Milestone

Comments

@shakuzen
Copy link
Member

shakuzen commented Apr 3, 2018

It looks like there is already OkHttp instrumentation (which isn't documented as far as I can tell). It would be nice if there were also instrumentation for the Apache HTTP client. This would help achieve HTTP client instrumentation in cases where people are using the HTTP client directly (instead of through RestTemplate in a Spring application) or for things like the Elasticsearch Java Low Level REST.

@jkschneider jkschneider added this to the 1.1.0 milestone Apr 11, 2018
@jkschneider jkschneider added the spring-boot change Change is needed in Spring Boot for this issue label Sep 20, 2018
@jkschneider jkschneider modified the milestones: 1.1.0-rc.1, 1.1.0-rc.2, 1.1.0 Oct 14, 2018
@jkschneider jkschneider modified the milestones: 1.1.0, 1.x Oct 26, 2018
@jkschneider
Copy link
Contributor

jkschneider commented Dec 12, 2018

Add metrics for:

  1. DNS resolution time (inject custom DnsResolver, delegating to the user-provided resolver, and timing).
  2. SSL handshake time (inject SSLConnectionSocketFactory, delegating).
  3. TCP connect time (also in custom SSLConnectionSocketFactory).

Apply these on custom PoolingHttpClientConnectionManager.

Feedback suggests that DNS resolution/SSL handshake can go up dramatically in noisy neighbor situations.

@benhubert
Copy link
Contributor

Would be great to also have metrics for monitoring the connection pools of
the PoolingHttpClientConnectionManager.

When enabling DEBUG output for org.apache.http.impl.conn.PoolingHttpClientConnectionManager, you can see numbers such as this line in your log file: [total kept alive: 2; route allocated: 2 of 10; total allocated: 2 of 50]. It would be very helpful, if we could monitor these numbes using Micrometer/Prometheus.

@benhubert
Copy link
Contributor

Just want to inform you, that we have a solution for monitoring the connection pools here at @willhaben. I'll contribute it as soon as we tested it with one of our applications.

@shakuzen shakuzen added the enhancement A general enhancement label Jan 22, 2019
@ravin-singh
Copy link

@benhubert How it's going with the testing of pool monitoring? I would like to use for one of our app in case it can be made available.

@benhubert
Copy link
Contributor

@ravin-singh Sorry. We are working on some other big changes at the moment, which are causing delays for this particular task. But it looks like we are able to continue with this next week, so I should be able to prepare the pull request soon.

@ravin-singh
Copy link

Thanks @benhubert. For mean time I have created a workaround solution that exposed pool stats to micrometer.
https://gist.github.com/ravin-singh/8654b654163c508cdb227a621fe078f0

shakuzen pushed a commit to benhubert/micrometer that referenced this issue Oct 1, 2019
shakuzen pushed a commit to benhubert/micrometer that referenced this issue Oct 1, 2019
…crometer-metrics#533

To avoid problems with the thread pool (i.e. if `@PreDestroy` is not called at
shutdown) I removed the thread pool.
shakuzen pushed a commit that referenced this issue Oct 1, 2019
)

Provides metrics for the underlying connection pool total stats.

See #533
@shakuzen
Copy link
Member Author

shakuzen commented Oct 1, 2019

With the merged pull requests, I'm considering this enhancement request complete. There is now MicrometerHttpRequestExecutor which can be used to time requests made from an Apache HttpClient, and there is PoolingHttpClientConnectionManagerMetricsBinder which provides metrics on the connection pool for a PoolingHttpClientConnectionManager.

I've opened up follow-up issues to track other ideas for metrics for Apache HttpClient. See #1616 and #1617.

@j-gurda
Copy link

j-gurda commented Feb 7, 2020

Thank you for your contribution!

I just wonder how I may use PoolingHttpClientConnectionManagerMetricsBinder with HTTP Client constructed with HttpClientBuilder. The problem is that HttpClientBuilder internally instantiates PoolingHttpClientConnectionManager baseing on a bunch of configuration properties (see HttpClientBuilder::build. Maybe I miss something but I believe it's impossible to access such PoolingHttpClientConnectionManager in a "civilized way" and I need to use reflection:

try {
    Class<?> internalHttpClient = Class.forName("org.apache.http.impl.client.InternalHttpClient"); // HttpClientBuilder returns InternalHttpClient which is an internal implementation of CloseableHttpClient
    Field connManagerField = internalHttpClient.getDeclaredField("connManager");
    connManagerField.setAccessible(true);
    PoolingHttpClientConnectionManager connManager = (PoolingHttpClientConnectionManager) connManagerField.get(client);
    new PoolingHttpClientConnectionManagerMetricsBinder(connManager, "proxyHttpClient").bindTo(meterRegistry);
} catch (ClassNotFoundException | NoSuchFieldException | IllegalAccessException e) {
     // some error handling here
}

@shakuzen
Copy link
Member Author

@j-gurda usage questions like this can easily be lost on closed issues (especially old, closed issues). StackOverflow or our Slack channel are a better place for such questions. You can make your own PoolingHttpClientConnectionManager and pass it to both the builder and the meter binder like:

PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager();
HttpClient httpClient = HttpClientBuilder.create().setConnectionManager(connectionManager).build();
new PoolingHttpClientConnectionManagerMetricsBinder(connectionManager, "my-pool").bindTo(registry);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants