Priority of ServerTracingFilter

[safetytrick]

The current priority of ServerTracingFilter is set at Priorities.HEADER_DECORATOR.

I don't think that is right for this library. I'm expecting this library to initialize tracing for a request and that should happen very early in the request. My expectation is that the entire request is instrumented including Authentication and Authorization filters.

Where this should fit is a little fuzzy, ContainerRequestFilter implementations that don't fit the mold in javax.ws.rs.Priorities were using Integer.MIN_VALUE. (I just looked at library implementations on my classpath), that included org.glassfish.jersey.logging.ServerLoggingFilter and another logging filter in a less common library.

Assuming no one will ever need to run before you is a little rude so I'd propose a priority of -1000. This is before most application code and all of javax.ws.rs.Priorities, I propose a number below 0 because application code I've seen commonly uses 0 to mean me first.

[pavolloffay]

The priority can be specified in the builder https://github.com/opentracing-contrib/java-jaxrs/blob/master/opentracing-jaxrs2/src/main/java/io/opentracing/contrib/jaxrs2/server/ServerTracingDynamicFeature.java#L160.

The question is whether we should change the default priority. There is some prior art when people requested using header decorator #15

[safetytrick]

Thank you, that sounds about right. I spent some time looking for other "tracing" filters in github and found a few opinions on this, Integer.MIN_VALUE (and +10), 0, or a custom constant with before authentication filter. Apache Geronimo was using HEADER_DECORATOR.

Brave looked like this: https://github.com/sapiokilias/t1/blob/101e8561f45d6308d8f9aaab47f3798a9fd54db6/instrumentation/jaxrs2/src/main/java/brave/jaxrs2/TracingContainerFilter.java#L33

This is the github query I used: https://github.com/search?q=%22import+javax.annotation.Priority%3B%22+tracing+ContainerRequestFilter&type=Code

I believe HEADER_DECORATOR should be used to set response headers for caching, csrf, security, etc. What I wasn't able to find was any opinionated documentation to that effect. Having that kind of an opinion as it pertains to tracing would be something I could imagine coming out of the opentracing spec.

I'm also picking at nats here 😄 but I think the default behavior would be improved with a default priority that runs before Priorities.AUTHENTICATION.

[pavolloffay]

@johnament was there any specific reason why tracing filter runs with priority HEADER_DECORATOR? Alternatively are there any concerns running it with the lowest priority Integer.NIM_VALUE +10?