opentracing-contrib / java-jaxrs

OpenTracing Java JAX-RS instrumentation
Apache License 2.0
37 stars 33 forks source link

Provide ContainerRequestContext to decorateResponse #135

Open hypnoce opened 4 years ago

hypnoce commented 4 years ago

There are scenarios where information held in the ContainerRequestContext need to be accessed in the decorateResponse method (properties, uriInfo...). Do you see any value into adding the ContainerRequestContext in the decorateResponse response arguments ? Thanks

pavolloffay commented 4 years ago

Can you please describe your use-case/ what information is needed to be accessed from ContainerRequestContext?

hypnoce commented 4 years ago

I'm trying to get the path parameters through the URI info as well as some properties that were set using requestContext.setProperty(). I could create another filter and make sure it's priority comes just after the ServerTracingFilter priority. But using the span decorator is a more robust method.

pavolloffay commented 4 years ago

Why decorateRequest does not work

void decorateRequest(ContainerRequestContext requestContext, Span span);

it is passing in the ContainerRequestContext

hypnoce commented 4 years ago

There are some information injected in the request via setProperty that I need to access after the query is processed. Also I need to know the status code as well as the uriinfo at the same time.

hypnoce commented 4 years ago

Hey @pavolloffay any news on this ? Thanks.

pavolloffay commented 4 years ago

This is pity because we have just released 1.0 and this is breaking change.

We will have to overload the response method and deprecate the old one.

hypnoce commented 4 years ago

Indeed. If we could use default method in interface, we could have made the transition smoother without breaking the API. Is it intended to be java 1.6 ? Thanks

pavolloffay commented 4 years ago

It's already released. We build this against java 1.7.

hypnoce commented 4 years ago

A sorry yes I meant java8.

pavolloffay commented 4 years ago

That should be fine, it's backward compatible.