Introduce ApacheHttpClientMetricsBinder to support metering retries.

[cachescrubber]

This PR reinstates the ApacheHttpClientMetricsBinder which has been introduced in #3800 / #3801 and removed in the process of supporting cancellations in the async implementation.

Goals of the PR are

• ensure metering and observations of the classic and async variant of the http client produce the same observations.
• support the automatic retry feature of the HttpClient library (which is enabled by default) and allowing to choose wether to observe automatic retries individually, or as a single observation with the final outcome.

The builder feature of the ApacheHttpClientMetricsBinder class might be superseded by the ApacheHttpClientObservationConvention. The most important aspect of the class is to provide the actual instrumentation of the (Async)ExecChainHandler to the (Async)ClientBuilder in way that maintainers of this project chan change it any time without breaking public contracts to the user of this library.

The meterRetries feature toggle should probably move to the ApacheHttpClientObservationConvention.

[bclozel]

Thanks @cachescrubber for this contribution. I've revised and merged it with 80e8682, only keeping the wiremock integration tests.

We are not going to provide more options on the instrumentation, especially around retries. As stated in my previous comment, retry behavior is a bit inconsistent and we are not going to surface that as a first class option. I don't want to compensate that with a relative order of the interceptor in the chain. This behavior is not specified nor documented and we should not rely on implementation internals for observation behavior. One could argue that not recording observations for retry requests is misrepresenting the actual behavior in production. I wish we could add a KeyValue to signal retry requests, but we cannot do that consistently for both async and classic clients.

We are not going to provide a binder as a result, because without the meter retry option, this doesn't make sense anymore.

[cachescrubber]

Thanks @bclozel. Nice move to add the interceptor last! Could have come up with that by myself... This way we get correct measurements, and that is what matters.

.addExecInterceptorLast("micrometer", new ObservationExecChainHandler(observationRegistry))

One final note: The elapsed times measured during the observation do not include the imposed retry-delay. Meaning the caller would wait for maxRetries * retryIntervall + the actual request times. The Timer however would only account for the actual http transaction. Might be worth to document somewhere.

[bclozel]

Indeed. We're only measuring requests themselves and not the interceptor chain. Ideally we should and the observation interceptor should be first. It's just that in the current situation we have to choose between consistency and design on one side and accuracy on the other.

I'll amend the documentation to reflect that. I don't think the micrometer.io website has a section for this instrumentation. I'll look into this.

Thanks for your help @cachescrubber the situation is not ideal but at least we're in a much better place after this iteration.