search

micrometer-metrics / micrometer

An application observability facade for the most popular observability tools. Think SLF4J, but for observability.
https://micrometer.io
Apache License 2.0
4.47k stars 990 forks source link

Upgrade to Apache HttpComponents Client 5.3.0 #4089

Closed cachescrubber closed 3 weeks ago

cachescrubber commented 1 year ago

Apache HttpComponents follow-up.

This is a follow-up to the changes made to the HttpComponents instrumentation in #3800 and #3941, specially to the latter. To solve the issues left I filed HTTPCLIENT-2291 and asked for clarification of the documentation in regard to the exec chain behavior in case of retries. The javadoc for (Async)ExecChainHandler has been updated in this pr. Both changes are scheduled with the release 5.3.0 of httpclient5.

With the changes in 5.3.0 the ObservationExecChainHandler could be safely positioned before or after the retry exec chain element. Async or classic exec chain behavior would be identical. In micrometer, only the javadoc of ObservationExecChainHandler had to be adjusted accordingly, the current implementation would be just fine.

As an opinionated version of the instrumentation I would recommend to document the following:

meter request as a whole, without individual retries

addExecInterceptorFirst("micrometer", new ObservationExecChainHandler(observationRegistry));

meter retries individually

addExecInterceptorAfter(ChainElement.RETRY.name(), "micrometer", new ObservationExecChainHandler(observationRegistry));

I don't know exactly when httpclient5 5.3.0 will be released, but IMO it will be rather short term. So with a bit of luck it might be picked up by micrometer 1.12.0.

@bclozel FYI

cachescrubber commented 11 months ago

I renamed the issue to Upgrade to Apache HttpComponents Client 5.3.0. The release is scheduled for the coming weeks, obviously too late for 1.12.0.

marcingrzejszczak commented 10 months ago

Hey @cachescrubber, are you willing to file a PR?

cachescrubber commented 9 months ago

Hi @marcingrzejszczak I can file a PR.

Ideally I could create a Section on Apache HttpComponents instrumentation in the Reference Instrumentations section of the documentation. Would that be feasible?

Target version would be 1.13, right?

marcingrzejszczak commented 9 months ago

Correct on both questions. Target 1.13, and reference instrumentations section :)

cachescrubber commented 9 months ago

Ok, fine. One more question - I there a timeline for removal of the deprecated http-client 4 supports and the initial Interceptor / requestExecutor? The deprecations are not marked forRemoval and no further information is given in the javadoc.

marcingrzejszczak commented 9 months ago

@shakuzen @jonatan-ivanov I don't remember what was the decision about the deprecation time line, do you?

jonatan-ivanov commented 9 months ago

fyi: We have 5.3.x in main: https://github.com/micrometer-metrics/micrometer/pull/4470. I'm not sure we do have such a deprecation timeline. I would say we should keep the 4.x support as long as 4.x is supported by Apache (assuming Micrometer users use it). The last release of 4.x was back in 2022 November though from their status page, it seems the version is still supported: https://hc.apache.org/status.html

cachescrubber commented 7 months ago

I finally managed to create the discussed documentation section.

shakuzen commented 3 weeks ago

As pointed out by @jonatan-ivanov, we've already upgraded to Apache HC 5.3 in Micrometer 1.13. I'm closing this. Let me know if I've missed something and there is more to do.

cachescrubber commented 3 weeks ago

All good, issue should have been closed with #4860