Added Jakarta and Javax instrumentations

[marcingrzejszczak]

to have that working we've added additional convention and contexts around javax and jakarta classes. Added tests for RestEasy that use the instrumentations. Also added ObservedValve that is an instrumentation for Tomcat (we currently support Tomcat up till version 8 - no jakarta imports).

cc @bclozel

[sonatype-lift[bot]]

Sonatype Lift is retiring

Sonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console. We are extremely grateful and thank you for your support over the years.

📖 Read about the impacts and timeline

[bclozel]

Thanks Marcin for working on this!

I'm wondering about the instrumentation for the JAX RS client bits. This is using the ClientRequestFilter and ClientResponseFilter contracts for this. It seems that if the connection somehow fails, or if a filter fails, a javax.ws.rs.client.ResponseProcessingException is thrown directly and this exception does not go through the response filter. This means that observations would not be stopped in those cases and that the instrumentation would miss a bunch of cases: filter errors and I/O errors in general. This reminds me of the work we recently did on the HttpComponents client where we switched to a different approach. Is there a better way to instrument clients here. Maybe through the *Invoker contracts by wrapping? We would need to see some concrete usage with a client to see if that's feasible.

I'm also wondering about the same problems on the server side with the ContainerRequestFilter and ContainerResponseFilter contracts. There, it seems that any exception (thrown by a JAX-RS endpoint or a filter) will be handled by an ExceptionMapper. In this case we might not need a different instrumentation approach but we might need a consistent ordering of that filter in the chain. Should it be ordered so that all other filters had a chance to process the response so that we don't record the wrong information? How should that be enforced?

Now about the Tomcat Valve. I'm not super familiar with this contract, but I remember that the asyncSupported flag is off by default and that this can play a important part since requests might not be instrumented for the async I/O case. I think that for regular async support, you might not need to do something special for async dispatches (like in the Servlet filter case), but I'm not entirely sure and I think this should be checked.

In general, in the Spring Framework instrumentation I tried to provide a lot of flexibility with custom names for observations and custom conventions (by providing constructor variants), but I'm wondering if this doesn't clutter the API in the end. Maybe this would be better handled with other contracts like the observation filter? I think this really depends on the use case. Maybe here JAX RS implementations would like to have a choice to come up with their own custom name?

[marcingrzejszczak]

I'm wondering about the instrumentation for the JAX RS client bits. This is using the ClientRequestFilter and ClientResponseFilter contracts for this. It seems that if the connection somehow fails, or if a filter fails, a javax.ws.rs.client.ResponseProcessingException is thrown directly and this exception does not go through the response filter. This means that observations would not be stopped in those cases and that the instrumentation would miss a bunch of cases: filter errors and I/O errors in general. This reminds me of the work we recently did on the HttpComponents client where we switched to a different approach. Is there a better way to instrument clients here. Maybe through the *Invoker contracts by wrapping? We would need to see some concrete usage with a client to see if that's feasible. I'm also wondering about the same problems on the server side with the ContainerRequestFilter and ContainerResponseFilter contracts. There, it seems that any exception (thrown by a JAX-RS endpoint or a filter) will be handled by an ExceptionMapper. In this case we might not need a different instrumentation approach but we might need a consistent ordering of that filter in the chain. Should it be ordered so that all other filters had a chance to process the response so that we don't record the wrong information? How should that be enforced?

Yup I fully agree. I don't know if these are the right contracts to be used for proper wrapping. I think I'll need some help from people working with Jakarta API directly. I asked about it in the RestEASY tracker https://issues.redhat.com/browse/RESTEASY-3356?focusedId=22781362&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-22781362 and still am waiting for some feedback.

Now about the Tomcat Valve. I'm not super familiar with this contract, but I remember that the asyncSupported flag is off by default and that this can play a important part since requests might not be instrumented for the async I/O case. I think that for regular async support, you might not need to do something special for async dispatches (like in the Servlet filter case), but I'm not entirely sure and I think this should be checked.

We had it turned on for Sleuth for ages and it worked well :shrug:

In general, in the Spring Framework instrumentation I tried to provide a lot of flexibility with custom names for observations and custom conventions (by providing constructor variants), but I'm wondering if this doesn't clutter the API in the end. Maybe this would be better handled with other contracts like the observation filter? I think this really depends on the use case. Maybe here JAX RS implementations would like to have a choice to come up with their own custom name?

I see. I guess it would be good for someone using JAX RS to chime in and give us feedback. Do you know anyone we can mention here? :)

[bclozel]

Now about the Tomcat Valve. I'm not super familiar with this contract, but I remember that the asyncSupported flag is off by default and that this can play a important part since requests might not be instrumented for the async I/O case. I think that for regular async support, you might not need to do something special for async dispatches (like in the Servlet filter case), but I'm not entirely sure and I think this should be checked.

We had it turned on for Sleuth for ages and it worked well 🤷

My bad, I missed that it was calling setAsyncSupported(true) already in the constructor.

In general, in the Spring Framework instrumentation I tried to provide a lot of flexibility with custom names for observations and custom conventions (by providing constructor variants), but I'm wondering if this doesn't clutter the API in the end. Maybe this would be better handled with other contracts like the observation filter? I think this really depends on the use case. Maybe here JAX RS implementations would like to have a choice to come up with their own custom name?

I see. I guess it would be good for someone using JAX RS to chime in and give us feedback. Do you know anyone we can mention here? :)

Not really unfortunately. I think Helidon is integrating with tracing libraries directly. Maybe something there that we can take a look at?

[marcingrzejszczak]

So I think we're using the proper APIs here. We just need to add support for error cases https://github.com/helidon-io/helidon/blob/helidon-3.x/tracing/jersey/src/main/java/io/helidon/tracing/jersey/AbstractTracingFilter.java#L125C9-L139

[marcingrzejszczak]

However for the client side there's also the interceptor https://github.com/helidon-io/helidon/blob/helidon-3.x/tracing/jersey-client/src/main/java/io/helidon/tracing/jersey/client/ClientTracingInterceptor.java . I'll check it out

[mkjensen]

Hi,

What is the status of this?

It looks pretty interesting to us. We have a product using Spring Boot 3.2 consisting of both legacy code using RESTEasy (via RESTEasy Spring Boot Starter) and newer code using Spring Web MVC.

It would be nice if we could easily get the same metrics for both, specifically the http.server.requests metric.

Thanks.

[thomas-fra]

Hi,

What is the status of this?

It looks pretty interesting to us. We have a product using Spring Boot 3.2 consisting of both legacy code using RESTEasy (via RESTEasy Spring Boot Starter) and newer code using Spring Web MVC.

It would be nice if we could easily get the same metrics for both, specifically the http.server.requests metric.

Thanks.

+1 We are migrating from TomEE to Wildfly and so from cxf that come with metrics providers to RestEasy that have nothing :( @marcingrzejszczak ?