JAX-RS filters not taken into account

[pilhuhn]

When I have a Jax-RS ContainerRequestFilter (whichis a pre-match filter), it may be executed before the incoming call hits my JAX-RS endpoint.
So basically my "outer span" is the one of the JAX-RS endpoint method, where in reality the filter(s) should be taken into account as well, as they may already burn a lot of time of an incoming request.

Likewise outgoing response filters need to be taken into account as well.

[NicoNes]

I agree with @pilhuhn .

The MP OpenTracing specification actually states that:

The MicroProfile implementation must provide a mechanism to automatically start a Span for any incoming JAX-RS request, and finish the Span when the request completes.

This looks good at first sight.
But reading the spec, it appears that this Span must be started once the incoming request has been matched with a JAX-RS resource method so that we can get resource method name and class to build the Span operation name.

So it raises following questions:

• why is the incoming request Span started so late ? (Before matching phase there is a valid JAX-RS pre-matching phase that should be traced)
• what to do for incoming request that does not match no JAX-RS resource method (and return 404) ? (They are valid JAX-RS processed by the JAX-RS application, so they should be traced)
• what to do for incoming request aborted in a pre-matching filter ? (They are valid JAX-RS requests processed by the JAX-RS application, so they should be traced)

My proposal to fix this little "gap" would be to define an "outer Request Span" as the parent of the "JAX-RS resource method Span" already defined by the spec.

This "outer Request Span" starts once the application receives the incoming request and finishes when the request completes.
The "JAX-RS resource method Span" starts once the matched JAX-RS resource method is invoked and finishes when the JAX-RS resource method exits.

It would look like:

Request Span started
   JAX-RS resource method Span started
   JAX-RS resource method Span finished
Request Span finished

The "outer Request Span" operation name could simply be <HTTP METHOD>.
It must defines the same tags as the "inner JAX-RS resource method Span" by default.

At first sight this could be weird to define same tags on those two Spans but for example HTTP_URL read when
"outer Request Span" starts can be latter changed in a JAX-RS prematching filter so that HTTP_URL read when "inner JAX-RS resource method Span" starts is a not the same. This example also works for HTTP_METHOD and also for HTTP_STATUS.

All this tracing could be disabled using existing config mp.opentracing.server.skip-pattern.
To only disable tracing for the "inner JAX-RS resource method Span" the existing Traced(false) could be used on the JAX-RS resource method.

WDYT ?

[pavolloffay]

thanks for looping in @NicoNes.

Is my understanding correct that the prematch filters are just normal Conteiner{Request/Response}Filter with @PreMatching annotation?

Another possible solution is to start the span in the prematch and override the name/attributes in the inner layer.

[NicoNes]

Is my understanding correct that the prematch filters are just normal Conteiner{Request/Response}Filter with @PreMatching annotation?

Yes that's right

Another possible solution is to start the span in the prematch and override the name/attributes in the inner layer.

Actually your solution is better than mine and works fine.
In my proposal, I introduced the "outer Span" level because I forgot that Span operation name can be changed after the Span starts. 👎

So the only things that need to be changed in the spec are:

• the initial Span operation name. It could be <HTTP METHOD>.
• the behavior of @Trace(false) to say that it will only disable resource method invocation tracing not the whole tracing for the given URI. To disable the whole tracing for the given URI, config mp.opentracing.server.skip-pattern should be used.

Thanks

[NicoNes]

Hey @pavolloffay ,

Any chance to get the next version of the spec updated to take this into account ?

Thanks

[Emily-Jiang]

@NicoNes we may not do any active development on this spec as the future is on OpenTelemetry, which superceded Open Tracing.

[NicoNes]

@Emily-Jiang it makes sense.
I hope the new MP-OpenTelemetry spec will handle the problem spotted in this issue into account.

Thanks !

[Emily-Jiang]

@NicoNes if you would like to get involved in the new initive of OpenTelemetry, please let me know. We are in the process of getting a prototype.

[NicoNes]

@Emily-Jiang Yes it would be great.

Thanks

[Emily-Jiang]

@NicoNes This tweet contains all of the meeting details. Hope you can join the call to discuss further!