-
Notifications
You must be signed in to change notification settings - Fork 52
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
Upgrade metrics reporter to Metrics 3 #99
Comments
@ifesdjeen you seem to be on a roll recently, want to tackle this one? ;) |
@michaelklishin sure boss no problem. let me bring the query api to some borderline stable mergeable state and i'll do that one too :) |
We should have an issue for that, as I've completely forgotten you were working on that :) |
@michaelklishin: I was thinking about taking a swing at this. One question, though: Is there any reason this doesn't use metrics-clojure? |
@jwhitlark not sure. I think it's reasonable to use |
Ok, I'll take a look and see if I run into any problems. |
I can actually recommend using Dropwizard Metrics without any wrapper library. Their api is quite clean and I've found it easier to do it this way, mostly because the documentation of all the existing wrappers isn't as good as original. As far as I understand, driver is already providing all the "gathering" of data. We only need to read out the information for reporting. |
@jwhitlark and I maintain |
I'm sure you can figure out API, but I'm also a user of the library, and bringing in an extra dependency for something that's very simply done in Java isn't something I've tried to strip down all the extra dependencies for :) We've tried metrics-clojure in our setup, and it was difficult for us to use, so we switched to pure java implementation. It's also very easy to use. |
Ok, I'm glad to hear what you guys are thinking. Let me take a look and see what makes sense to me after I've read through the code. |
I don't think I'm going to get to this anytime soon. If someone else wants to take it on, feel free. |
@jwhitlark no worries, thanks for the update. @ifesdjeen do you want to look into it? |
We should investigate if upgrading it to Metrics 3 is feasible.
The text was updated successfully, but these errors were encountered: