Instrument mp-rest-client

[pavolloffay]

Resolves #82

The current proposed solution:
All clients are by default traced. Tracing could be disabled via Traced(false) on the client interface or method.

Issues to address:

• How to disable, enable tracing? Tracing could be enabled by default and disabled if @Traced(false) is present on the interface or vice-versa. Make interface type available to providers microprofile-rest-client#121 Traced can be applied on interface or method
• Async integration probably would be leaking as AsyncInvocationInterceptor https://github.com/eclipse/microprofile-rest-client/blob/master/api/src/main/java/org/eclipse/microprofile/rest/client/ext/AsyncInvocationInterceptor.java#L40 does not have a callback to remove applied context. AsyncInvocationInterceptor does not have remove context callback microprofile-rest-client#122 done via AsyncInvocationInterceptor
• Test the client if it is directly injected - without manually interacting with RestClientBuilder (@Inject @RestClient ServiceInterface) - not needed the loading of components (interceptors) should be the same as for manually created client
• tests for disabling tracing on method and class.
• test when executor service is overridden

SmallRye implementation smallrye/smallrye-opentracing#28

Other links to blocking issues, mostly for testing (SmallRye):

• PartialResponse throws: method call not supported smallrye/smallrye-rest-client#11
• Update to MP REST Client 1.1 smallrye/smallrye-rest-client#6

[pavolloffay]

hi @andymc12 could you please look at the linked issues for mp-rest-client?

eclipse/microprofile-rest-client#122
eclipse/microprofile-rest-client#121

Alternative to these two could be a listener similar to onNewBuilder. But it would be beforeClientCreated(builder, interfaceClass).

[pavolloffay]

@fmhwong @objectiser could you please review?

The PR should be in a final form. Once this is merged we could split TCK and locate rest client tests in a separate module for vendors who do not implement rest-client, this however requires creating a shared tck package for some common classes and services e.g. tracer service.

I was able to successfully pass TCK with smallrye implementation, the PR to smallrye is linked in the first comment of this PR.

[objectiser]

Only minor comments.

[objectiser]

Looks like missing the async version of this.

[pavolloffay]

test testMultithreadedNestedSpansAsync should test it.

[fmhwong]

LGTM (just some minor comments)

[fachat]

In 19.0.0.2 the RestClientBuilder is still not automatically instrumented with opentracing, so the called service does not get the span id, thus cannot be correlated in e.g. zipkin.
Is there a target date for this? Is there a mitigation, because mp RestClient is so much more convenient to program.
Edit: openliberty 19.0.0.2, eclipse microprofile 2.1

[kenfinnigan]

@fachat that's a question for OpenLiberty vs the specification (as I believe that's what the version you mention refers to).

It is most definitely resolved in the specification, but it's likely OpenLiberty doesn't implement that version yet