HikariCP icon indicating copy to clipboard operation
HikariCP copied to clipboard

Adds support for Dropwizard 5 metrics.

Open pennello opened this issue 1 year ago • 5 comments

Hi @brettwooldridge ,

Here is a PR that adds support for Dropwizard 5 metrics alongside existing support for Codahale and Micrometer.

I tried to keep the code style consistent with the existing code, and with new tests, sought to keep common as many tests as possible for Codahale and Dropwizard 5, which are of course very similar, but also frustratingly different when shared in the same codebase. (I did this by having a test abstract base class and parameterizing the metric registry type.)

The existence of IMetricsTracker made adding Dropwizard 5 support relatively straightforward. But I didn't see a similar abstraction for health checking. Hence I didn't add support for that yet. I figured that this is still a strict improvement and would thus still be worth submitting to you.

I also made some library upgrades in order to get tests passing in a Java 18 environment.

Please let me know what you think, or if you think this might need any revisions before it's appropriate to get into dev.


For context, my company has been working on a Jetty 11 upgrade, which has necessitated a number of other nontrivial and sometimes backwards-incompatible upgrades, including Resteasy, Jakarta (over javax), and Dropwizard. Hence, to maintain the useful Hikari metrics, we wanted to add support here for Dropwizard 5.

pennello avatar Aug 08 '22 20:08 pennello

Some checks were not successful 1 failing and 1 successful checks Travis CI - Pull Request Failing after 1m — Build Failed

The command "mvn package -Dmaven.javadoc.skip=true -V -B" exited with 1.

Rats. When I run that locally on my branch, it works. I'm not sure what's wrong. I didn't see further logging from Travis. Am I missing something?

pennello avatar Aug 08 '22 20:08 pennello

Hi @brettwooldridge , just a friendly ping on this.

pennello avatar Sep 26 '22 21:09 pennello

@pennello, The travis-ci build for this is failing, any idea why?

lfbayer avatar Sep 27 '22 13:09 lfbayer

@lfbayer Thanks for the reply.

I checked the Travis logs and saw only the following:

The command "mvn package -Dmaven.javadoc.skip=true -V -B" exited with 1.

Is there a way to get more diagnostic information out of the CI build? When I run that locally on my branch, it works. I've got to be missing something simple.

pennello avatar Sep 27 '22 20:09 pennello

In the hopes of the Travis run succeeding on a re-try, I re-pushed as of a51f146a87ca5ac9702a87b0c79b716b3429c226. It passed, but I don't see Travis anymore. Only Snyk.

However, there were conflicts. I rebased, and also added a module declaration for Dropwizard 5 as of 3f377c0bda615f9abb2efacd2d67766997f3d2f8.

pennello avatar Jun 28 '23 22:06 pennello