Add @Priority annotations to container request/response #139
Conversation
…onse is executed last
|
What about the client filters ? Same here surely ? |
|
@janolaveide I wasn't sure if you could set them on the client filters. I'll add them anyway. |
| @@ -37,6 +37,11 @@ | |||
| <version>1</version> | |||
| </dependency> | |||
| <dependency> | |||
| <groupId>javax.annotation</groupId> | |||
| <artifactId>javax.annotation-api</artifactId> | |||
| <version>1.2</version> | |||
There was a problem hiding this comment.
just curious.. this matches servlet 2.5, right? If so, maybe make a comment that it does (so that someone changing below doesn't invalidate this)
There was a problem hiding this comment.
or if it has nothing to do with any of the dependencies here, ignore my comment :P
There was a problem hiding this comment.
I don't think it's related. I had to add this dependency to get access to the @Priority annotation as it wasn't available in any of the other dependencies.
|
is there any way to test this? (ex show it prevents a bug) |
|
I'm trying to think how we could test this. We'd need to create a set of filters that would inspect the zipkin information show how to ensure its being included in the request I'd imagine. |
|
Not sure what you want to test, testing that it actually executes first ? I haven't looked at the actual test cases you have in brave, but in my manual testing I add a time-consuming filter with priority > 1 and verifies that brave timing reflects this. |
|
sounds sufficiently tricky to automatically test at the moment. Thanks for manual testing. |
Add @priority annotations to container request/response
|
This change is released as part of brave 3.5.0 |
Ensure the container request is executed first (Priority 0) and the container response is executed last (also Priority 0)
See https://jersey.java.net/documentation/latest/filters-and-interceptors.html#d0e10117 for more details
Resolves #134