Remove micrometer metrics on close

[igouss]

No description provided.

[codecov]

Codecov Report

Merging #1700 (8b15e1c) into dev (8217f20) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##                dev    #1700   +/-   ##
=========================================
  Coverage     70.11%   70.11%           
  Complexity      561      561           
=========================================
  Files            26       26           
  Lines          2118     2118           
  Branches        296      296           
=========================================
  Hits           1485     1485           
  Misses          487      487           
  Partials        146      146           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8217f20...8b15e1c. Read the comment docs.

[brettwooldridge]

@checketts @shakuzen Hey, can I ask you guys to review this? Looks ok to me, but not being a Micrometers user I don't know about side-effects. Any? Issues with multiple pools in the same VM, etc?

[shakuzen]

This seems reasonable to me. I would just note that the remove method was added in Micrometer 1.1.0, which means this change would require at least that version, but that is probably okay given how old that version already is (we don't even support the 1.0.x or 1.1.x line anymore).

I suppose the motivation here is to no longer report metrics for HikariCP instances that are closed. If so, this seems like the right way to achieve it.

[shakuzen]

Any? Issues with multiple pools in the same VM, etc?

As long as each pool is using a different poolName, they will be tagged uniquely, creating unique metrics. If they had the same poolName, they would not make unique metrics.

[checketts]

I agree with @shakuzen . The only risk this adds is if there were 2 pools with the same name they would be sharing the meters, and the removal of the first would remove the meters even if the second is still in use.

I personally haven't used any of the remove methods so I'm not certain how the 2nd pool's usage of removed meters would behave. I'm guessing they would likely just be orphaned and stop reporting.

[brettwooldridge]

@checketts @shakuzen Thanks for the feedback guys. Yes, I understand that two pools with the same name could present a theoretical issue. In this case, it would be incumbent upon the user to ensure uniquely named pools. For explicitly unnamed pools, HikariCP itself will generate unique names via an incrementing integer suffix, so "default" configurations with multiple pools would not run into that edge case.