#859 fix inherited timed annotation

[mtakaki]

Suggestion for fixing the the issue #859 that prevented having resources inheriting annotated methods.
Also fixing the support for absolute metric names that apparently was not working properly.

[rferreira]

hey folks what's the good word on this PR?

[mtakaki]

I haven't heard anything regarding my proposed fix for this issue, @rferreira. My suggestion at #746 is still valid. You can import this fix into your project at any time. To make it easier, just add this file: https://raw.githubusercontent.com/mtakaki/metrics/mtakaki_746_fix_inherited_timed_annotation/metrics-jersey2/src/main/java/com/codahale/metrics/jersey2/InstrumentedResourceMethodApplicationListener.java

[edwardsb]

@mtakaki I did as you suggested, and got

java.lang.NoSuchMethodException: Could not find a suitable constructor in com.mywebservice.InstrumentedResourceMethodApplicationListener class.

[mtakaki]

@edwardsb which version of dropwizard are you using? I noticed it didn't work as soon as I migrated from 0.8.x to 0.9.x. So I had to do some changes to it: https://gist.github.com/mtakaki/746dba30edfefc0ffbd5

To be honest, I completely gave up about having my fix merged. So this has been my current approach to get around this issue.

[edwardsb]

I am using 0.9.x.

I copied your gist in and its no longer throwing the suitable constructor error.

Though I am now getting null pointers on exception metered methods of sub-resources. For some reason my top level resources that have @timed, @metered, and @ExeptionMetered are getting registered on startup, but sub-resource still don't look like they are in the maps of timers, meters, and exception meters.

Ultimately this line is called:
final ExceptionMeterMetric metric = this.exceptionMeters.get(chooseName(annotation.name(), annotation.absolute(), handlerClass, definitionMethod));

and since there is nothing in the map with that key, metric is null and then we call a method on it.

Do you register your sub-resources with the Jersey environment? I tried that too and it wasn't working.

[ryantenney]

@mtakaki I appreciate the fix, and it will be merged, but not into 3.1. It'd be perfect for 3.2, but I'm trying to focus on v4, and you can expect the fix to be included there.

[mtakaki]

@edwardsb I'll take a look tonight. So you have two resources, a parent and a child, and both are being used as resources? I'll try this later today, but I don't see why it would not work. And yes, you need to register all the concrete resource classes.

@ryantenney sorry, I didn't know you were too busy to look at my PR. Thanks for working on the version 4 :) Really appreciate the good work you're doing with the metrics project.

[edwardsb]

@mtakaki Yes here is a gist of what I am working with: https://gist.github.com/edwardsb/0a8d232c66bf54f0b75a

Thanks for the help. Do you mind if I message you on twitter?

[mtakaki]

@edwardsb not at all. Feel free to message me on twitter.

[leeavital]

This is something we would find useful as well. I'd be glad to help in any way :)

[rkalicinski]

Is it possible to merge those commits?

[github-actions]

This pull request is stale because it has been open 180 days with no activity. Remove the "stale" label or comment or this will be closed in 14 days.