Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/filter metrics v3.2 #1118

Merged

Conversation

reftel
Copy link
Contributor

@reftel reftel commented Mar 29, 2017

This adds support for measuring time for the entire request processing, and the request and response filtering steps, in addition to the current measurement of the time used by the request method itself. In addition, support is added for letting users specify the clock to be used.

No extra metrics are added for methods with @timed annotations that specify an explicit name, as it's not really clear what those extra metrics should be named, and it seems wrong to add attributes to the @timed annotation for jersey-specific concepts. Ideas for how to add the extra metrics for explicitly named methods welcome!

Corresponding PR for master: #1119

@reftel reftel force-pushed the feature/filter_metrics-v3.2 branch from 52503d8 to 90f5816 Compare March 29, 2017 18:59
@reftel reftel mentioned this pull request Mar 29, 2017
@arteam
Copy link
Member

arteam commented Apr 8, 2017

This looks like an interesting addition, could you explain your use case? Do you have several filters which take a lot of time to process? Or do you want to measure Jersey's overhead along with the method?

@reftel
Copy link
Contributor Author

reftel commented Apr 8, 2017

Sure! I'm working on some services related to handling of case folders for the Norwegian Tax Authority. This involves some rather complex data structures that can contain highly sensitive information in several places.
One of the cases where filter metrics is valuable is in an application that handles modification requests for cases. If we get a simple operation (e.g. adding a comment) on a complicated case, a filter we have for preprocessing the case to find all information that may be relevant for an authorisation decision so that we in a later stage can make an informed decision, can easily take more time than the operation itself. We also have other filters that make HTTP calls to a third-party service, so including or not including the request filter stage in the metric for the endpoint makes a big difference - our consumers care about the response time for the request as a whole, and monitoring just the time spent by the request method isn't enough.

@reftel
Copy link
Contributor Author

reftel commented Aug 21, 2017

Was that explanation enough? Is there anything else that we can do to progress on this feature?

@arteam
Copy link
Member

arteam commented Aug 24, 2017

I will look at it more one more time tomorrow and if it's OK will make a new 3.2.5 release.

@arteam arteam added the feature label Sep 8, 2017
@arteam arteam added this to the 3.2.5 milestone Sep 8, 2017
@arteam arteam merged commit 84fa2db into dropwizard:3.2-development Sep 8, 2017
@arteam
Copy link
Member

arteam commented Sep 8, 2017

Thank you very much for the contribution! The feature looks very useful and the implementation seems backward compatible and don't create a big performance penalty.

@arteam arteam mentioned this pull request Sep 15, 2017
arteam pushed a commit that referenced this pull request Sep 15, 2017
* Refactor

* Let user specify clock for @timed

* Time request and response filtering
@arteam arteam modified the milestones: 3.2.5, 4.0.0 Sep 15, 2017
@arteam
Copy link
Member

arteam commented Sep 15, 2017

I've decided to go with this change only for 4.0.0 and keep the 3.2 branch only for bugfixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants