Restclient @WithSpan throws in java.lang.ClassCastException

[ennishol]

Describe the bug

To be able to add a parameter to the trace, I added two annotations to rest client remote call @WithSpan and @SpanAttribute as described in the open telemetry documentation:

public interface RemoteRestClient {
    @GET
    @Path("/remote/{id}")
    @WithSpan
    Response getResponse(@SpanAttribute(value = "id") @PathParam("id") Long id) throws Exception;
}

This does not work however and the error is:

org.jboss.resteasy.spi.UnhandledException: java.lang.ClassCastException: class org.jboss.resteasy.microprofile.client.InvocationContextImpl cannot be cast to class io.quarkus.arc.ArcInvocationContext (org.jboss.resteasy.microprofile.client.InvocationContextImpl and io.quarkus.arc.ArcInvocationContext are in unnamed module of loader io.quarkus.bootstrap.classloading.QuarkusClassLoader @48524010)

The stacktrace below has been reversed to show the root cause first. [Click Here](http://localhost:8080/hello) to see the original stacktrace

java.lang.ClassCastException: class org.jboss.resteasy.microprofile.client.InvocationContextImpl cannot be cast to class io.quarkus.arc.ArcInvocationContext (org.jboss.resteasy.microprofile.client.InvocationContextImpl and io.quarkus.arc.ArcInvocationContext are in unnamed module of loader io.quarkus.bootstrap.classloading.QuarkusClassLoader @48524010)
	at io.quarkus.opentelemetry.runtime.tracing.cdi.WithSpanInterceptor_Bean.intercept(Unknown Source)
	at org.jboss.resteasy.microprofile.client.InvocationContextImpl$InterceptorInvocation.invoke(InvocationContextImpl.java:170)
	at org.jboss.resteasy.microprofile.client.InvocationContextImpl.invokeNext(InvocationContextImpl.java:69)
	at org.jboss.resteasy.microprofile.client.InvocationContextImpl.proceed(InvocationContextImpl.java:105)
	at io.quarkus.restclient.runtime.QuarkusProxyInvocationHandler.invoke(QuarkusProxyInvocationHandler.java:158)
	at jdk.proxy20/jdk.proxy20.$Proxy189.getResponse(Unknown Source)
	at org.acme.RemoteRestClient_5d475da7d2c3784fa411823307f5b6e5171ceacd_Synthetic_ClientProxy.getResponse(Unknown Source)

Reproducer with test
code-with-quarkus.tar.gz

or start with ./mvnw quarkus:dev and curl http://localhost:8080/hello

When I remove @WithSpan then it works but the parameter id is not propagated into the trace tags.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[quarkus-bot]

/cc @cescoffier (rest-client), @geoand (rest-client)

[manovotn]

The problem here is that rest client uses its own interceptor chain impl including custom invocation context (org.jboss.resteasy.microprofile.client.InvocationContextImpl. As far as Interceptors specification goes, this wouldn't matter but in Quarkus we also allow users to replace jakarta.interceptor.InvocationContext with io.quarkus.arc.ArcInvocationContext (which provides some extras, mainly access to interceptor bindings) which is where it fails as the custom impl has no idea about Arc's impl.
That's exactly the case with io.quarkus.opentelemetry.runtime.tracing.cdi.WithSpanInterceptor.

FTR, some aspects of rest client having its own interceptor impl were recently discussed in #35558.

We could change WithSpanInterceptor to only use jakarta.interceptor.InvocationContext and retrieve interceptor bindings differently.
However, rest-client will fail with any interceptor that makes use of ArcInvocationContext.

[manovotn]

Also of note is that the next interceptors specification version (used in CDI 4.1) will add the ability to retrieve interceptor bindings effectively removing the need for ArcInvocationContext.

[geoand]

@manovotn thanks for the analysis.

What is the best way to deal with this?

[manovotn]

What is the best way to deal with this?

I am not sure...
To enable this particular case, the easiest place a workaround into WithSpanInterceptor.
But, like I said, this will break any other interceptor using ArcInvocationContext...
The ultimate solution is not using two different interceptor chains but that would be a lot of work as you'd need to pre-generate code for rest client interfaces to register them as regular beans (similarly to what RR does).

[geoand]

Right. My vote is to address this specific use case now as it could quite prevalent and think about a general fix later.

@manovotn mind opening a PR with what you have in mind?

[manovotn]

@geoand so, I originally thought you could alter it like this but that's not helping here and you will get:

2023-09-25 08:20:53,569 SEVERE [io.qua.ope.run.exp.otl.VertxGrpcExporter] (vert.x-eventloop-thread-4) Failed to export spans. The request could not be executed. Full error message: Connection refused: localhost/127.0.0.1:4317

The custom InvocationContext impl has no idea about interceptor bindings and I didn't realize WithSpanInterceptor is heavily dependent on that (with no fallback). CC @radcortez

[manovotn]

Another approach I could think of is here - #36123
We can copy the custom context impl over to our codebase and make it implement ArcInvocationContext - that solves the general issue of rest client not working with any interceptor that uses ArcInvocationContext as its non-standard parameter.

I am not a fan of this copy-pasta but we already had to do the same other parts of rest-client so in this particular case it doesn't seem as harmful.

However, even with this solution I am still seeing the same Failed to export spans message even though the interceptor bindings are now visible. My knowledge of spans and telemetry is very much nonexistent though so I would appreciate if someone else took a glimpse :)

[geoand]

Thanks @manovotn. I do think #36123 can be the base for a solution.
I am not sure I'll have time to use your PR and debug the telementry issues soon. @brunobat any chance you can?

[brunobat]

Thanks @manovotn for the code. I'll debug what's going on.

[radcortez]

The custom InvocationContext impl has no idea about interceptor bindings and I didn't realize WithSpanInterceptor is heavily dependent on that (with no fallback). CC @radcortez

This is to retrieve the @WithSpan annotation from the intercept method because it may contain additional metadata to pass into the Span. I guess it could be done, by simple getting the method from the jakarta InvocationContext and checking for the annotation.

To be able to add a parameter to the trace, I added two annotations to rest client remote call @WithSpan and @SpanAttribute as described in the open telemetry documentation:

public interface RemoteRestClient {
    @GET
    @Path("/remote/{id}")
    @WithSpan
    Response getResponse(@SpanAttribute(value = "id") @PathParam("id") Long id) throws Exception;
}

On another note, what is the goal here? REST invocations (both server and client) are automatically traced and follow specific tracing rules for HTTP, so @WithSpan annotation is not required here.

[brunobat]

On another note, what is the goal here? REST invocations (both server and client) are automatically traced and follow specific tracing rules for HTTP, so @WithSpan annotation is not required here.

This is a really good point... Why do you need this @ennishol ? Only because of the @SpanAttribute(value = "id") ?

[brunobat]

@manovotn your PR works... Those warnings are just because the OTel exporter is trying to send traces to the OTel Collector and there's none. As soon as you have one, you can see the expected spans:

[brunobat]

@ennishol
You have the id that was called without the need of @WithSpan or @SpanAttribute(value = "id").
You can look at:

• the client GET has this attribute: http.url=http://localhost:8080/remote/666
• The remote server GET /remote/{id} will have: http.target=/remote/666

[manovotn]

@manovotn your PR works... Those warnings are just because the OTel exporter is trying to send traces to the OTel Collector and there's none. As soon as you have one, you can see the expected spans

Ah, good to know, thanks for double checking @brunobat!

[ennishol]

@brunobat
I need it as id=666 to be able to search for all spans with tag id. It should also work with @POST

public interface RemoteRestClient {
    @POST
    @Path("/remote")
    @WithSpan
    Response getResponsePost(@SpanAttribute(value = "id") @QueryParam("id") Long id) throws Exception;
}

[brunobat]

@ennishol you can search by */remote/* on http.url on the client's span. Another alternative could be doing this piece of code when you call the client, that would place that attribute in the current span:

// ...
Span current = Span.current();
Response response;
try(Scope scope = current.makeCurrent()){
    current.setAttribute("remote-id", 666L);
    response = client.getResponse(666L);
}
// ...

This is because you probably shouldn't add a new span just to extract the that id.

[ennishol]

@brunobat Thanks for suggestions. The thing is, our code sets the tag in all applications, so we want to have consistent tags everywhere. It is not just about searching this one trace, but possibly multiple ones across all applications without knowing URLs have been used in the requests

so @SpanAttribute won't work with RestClient? Maybe it is worth mentioning in the documentation then
Thanks!

[brunobat]

I was not saying that. It will work after the fix.
What you mention is fair and I've now identified a gap in our implementation. We should have support for this annotation: @AddingSpanAttributes. Created this: #36138
In the future you will be able to use this new annotation.

@manovotn PR is enough to fix the exception and later enable this new annotation to work.