-
Notifications
You must be signed in to change notification settings - Fork 884
JAVA-3114 Include dropwizard metrics into the shaded dependencies list for 3.x driver #1685
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
Conversation
Thanks for the PR @Mmuzaf! Have you signed the Contributor License Agreement for contributions to DataStax open source projects? If not you can find it at https://cla.datastax.com/. Thanks! |
@absurdfarce Thank you for your reply, I'll check the CLA right away. Do we need to create a JIRA issue for this case? |
@absurdfarce I have signed the CLA |
@absurdfarce In a nutshell, I propose to shade the dropwizard metrics into the java-driver itself and make the next minor release 3.11.4. This will allow us to untangle the cassandra library upgrade from the driver usage. Please, let me know if this works for you or any kind of help is required :-) |
Thanks for the PR @Mmuzaf! I spent a bit of time looking at this this morning and while I'm not opposed to shading the metrics lib I don't love it either. We currently only shade Netty and that's largely to avoid any performance inconsistencies that can crop up between versions. In this case we'd really be shading the metrics Java for exactly one consumer (Cassandra itself), which we can do... but it did get me wondering if we wouldn't be better off just upgrading the metrics lib directly. 3.2.2 is very old at this point so an upgrade doesn't seem like an unreasonable change. The problem seems to be that moving to 4.x means moving to Java8 as a minimum while the 3.x Java driver (currently) has to support Java6 and above. We're kicking this idea around within DataStax, trying to determine what action we want to take here. What about on your side? Do you foresee any issue if we just explicitly upgrade the dependency on Dropwizard metrics to 4.x and release that as, say, 3.11.5? Any reason that wouldn't work for you? |
@absurdfarce I was thinking about upgrading the metrics version in both the driver and Cassandra at the same time and I had the following concerns:
So, the shading looks like a small evil that will help us a lot, but if you still think the metrics upgrade is better, I can also prepare a new patch for the driver. It is worth mentioning that I've also checked the amount of changes with the driver upgrade to 4.x in the Cassandra itself to find the easiest solution for the metrics upgrade, and stuck with rewriting the code as the driver's API 3.x -> 4.x has many valuable changes I'm not yet familiar with :-) |
Hey @Mmuzaf, after pondering this for a bit I think I'm going to go with the following approach:
I completely agree with you that the long-term plan cannot be released as a patch release; these changes would have to be released as 3.12.0. Finally, I think we'd be okay with Java clients using 3.12.0 (as described above) to connect to Cassandra 3.x or 4.x. The version of dropwizard-metrics in play should on affect local operations for the Java driver; it shouldn't have any impact on communication with Cassandra. We'll test to make absolutely sure but my strong suspicion is that we won't have a problem there. Does that all sound about right for you? |
@absurdfarce I agree. |
I've created JAVA-3114 to track this work (and the 3.11.5 release it will headline). Thanks for the offer @Mmuzaf , I'll definitely hit you up to help out if something else presents itself. My next steps are to get this PR merged and then see what else we had that was aimed for the next 3.11.x release... I don't think there was much (if anything) in that queue but I want to double-check. |
The patch attempts to fix a backwards compatibility problem that occurs when upgrading the Dwopwizard Metrics library from 3.x to 4.x. The problem with simply upgrading the Dropwizard version is that the
JmxPerporter
class has moved from thecom.codahale.metrics
package in the 3.x version to thecom.codahale.metrics.jmx
package in the 4.x version.The `cassandra-driver-core' itself doesn't provide the necessary classes for metrics, and relies entirely on Cassandra's classpath. This, in turn, makes the library upgrade in the Cassandra project a bit difficult as it clashes with the library version used in the driver.
The following solution may be possible:
withoutMetrics()
to avoid metrics initialisation, the drawback of this solution - metrics won't be accessible;The same problem could exist for the 4.x version and the additional investigation is required.
Reference JIRA:
https://issues.apache.org/jira/browse/CASSANDRA-14667