search

elastic / apm-agent-java

https://www.elastic.co/guide/en/apm/agent/java/current/index.html
Apache License 2.0
566 stars 319 forks source link

Support for standalone JAX-RS #1752

Open tobiasstadler opened 3 years ago

tobiasstadler commented 3 years ago

Is your feature request related to a problem?

We are using Jersey without a servlet container and the JAX-RS instrumentation isn't working (No transactions are created)

Describe the solution you'd like

Maybe co.elastic.apm.agent.jaxrs.JaxRsTransactionNameInstrumentation#setTransactionName should create a transaction if it is null?

SylvainJuge commented 3 years ago

Hi @tobiasstadler ,

Thanks for filing an issue for this. As you have discovered we only handle naming of JAX-RS transactions if they are already created by another framework (here it's likely to be Servlet in most cases.

If I remember correctly, Jersey standalone relies on Jetty, do you know if there is another option as non-Servlet server ? (link to doc would be fine if you have such).

If it relies only on Jetty, then it would be probably easier to simply add plugin to support non-Servlet transactions on Jetty, I remember there is an intermediate API close to Servlets that is also available. In this case, the "hardest" part would be to ensure that the Servlet part would only customize naming and does not try to create another transaction when another one has been created by Jetty. The current instrumentation only deals with transaction naming, and the method we instrument might not always include the whole HTTP request processing, only the part that is executed within the resource method, which would not include serialization for example.

Also you might not be using the latest version of Jersey newer versions of Jersey have been moved to another namespace https://projects.eclipse.org/projects/ee4j.jersey and we don't support those newer versions due to the package name change.

SylvainJuge commented 3 years ago

Actually I found that there is quite a large list of possible non-Servlet implementations to support:

According to what we find on https://mvnrepository.com/artifact/org.glassfish.jersey.containers there are quite a few of them :

My guess is that the Jetty, Netty and Grizzly2 are likely the most commonly used in production.

tobiasstadler commented 3 years ago

Jersey standalone can be used with several http server/container (see https://eclipse-ee4j.github.io/jersey.github.io/documentation/latest/modules-and-dependencies.html#server-jdk). Jetty is one of them, but one can also use grizzly or even the jdk one.

tobiasstadler commented 3 years ago

Also there are other JAX-RS implementations which might without a servlet environment (e.g. RESTEasy). So in prefer a "generic" solution.

For the time being we can manually instrument our endpoints (the app is quite small).

SylvainJuge commented 3 years ago

Thanks for the links @tobiasstadler, in your case using the agent public API within a Jersey filter might allow to create the transaction with minimal code changes, without having to add annotations everywhere.

felixbarny commented 3 years ago

Also there are other JAX-RS implementations which might without a servlet environment (e.g. RESTEasy). So in prefer a "generic" solution.

The thing that makes a generic solution challenging is that there are no interfaces in the JAX-RS spec that dispatch an incoming request and expose an javax.ws.rs.core.Request that would allow us to read headers (to make distributed tracing work) and to fill the request context object. These dispatcher interfaces are implementation-specific, for example org.glassfish.jersey.server.spi.internal.ResourceMethodDispatcher. But once we have an implementation for one, such as jersey, it's probably not too complicated to adapt it to RESTEasy, as we'd mainly be relying on the JAX-RS defined types javax.ws.rs.core.Request and javax.ws.rs.core.Response.

Maybe co.elastic.apm.agent.jaxrs.JaxRsTransactionNameInstrumentation#setTransactionName should create a transaction if it is null?

While it may be better than not having a transaction at all, it may also be confusing as distributed tracing wouldn't work and the request context wouldn't be populated.

tobiasstadler commented 3 years ago

Yes, unfortunately It is hard to hook into JAX-RS using bytecode instrumentation.