Skip to content
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

Prevent PrometheusHistogramMetricsTracker from registering metrics twice in the same CollectorRegistry #1467

Closed
wants to merge 7 commits into from

Conversation

edysli
Copy link

@edysli edysli commented Oct 11, 2019

Fixes #1465 and formats code related to PrometheusHistogramMetricsTracker according to this project's Eclipse formatter profile.

Reformat `PrometheusHistogramMetricsTrackerFactoryTest` using the
project's Eclipse formatter profile like was done for
`PrometheusMetricsTrackerFactoryTest` in
086bf18. This makes similiarities
between the two files readily apparent.

Also use class `StubPoolStats` instead of local implementation.

Issue-Id: #1465
Autoformat `PrometheusHistogramMetricsTrackerFactory` using the
project's Eclipse formatter profile.

Issue-Id: #1465
Copy tests `testMultiplePoolNameWithOneCollectorRegistry` and
`testMultiplePoolNameWithDifferentCollectorRegistries` from
`PrometheusMetricsTrackerTest` to
`PrometheusHistogramMetricsTrackerTest` so that we test the same
behaviour with regard to `CollectorRegistry` across both
implementations.

Also port refactorings from 086bf18
to `PrometheusHistogramMetricsTrackerTest` and reformat.

Issue-Id: #1465
Port improvements from #1331 (086bf18)
to `PrometheusHistogramMetricsTrackerFactory`: add mechanism to avoid
registering a `io.prometheus.client.Collector` more than once in the
same `io.prometheus.client.CollectorRegistry`.

Also reformat `PrometheusHistogramMetricsTrackerFactory` according to
this project's Eclipse formatter profile.

Issue-Id: #1465
Port improvements from #1331 (086bf18)

to `PrometheusHistogramMetricsTracker`: add mechanism to avoid
registering a `io.prometheus.client.Collector` more than once in the
same `io.prometheus.client.CollectorRegistry`. Rename method
`registerHistogram` to `createHistogram` because it doesn't actually
register anything.

Issue-Id: #1465
@edysli edysli changed the title [WIP] Prevent PrometheusHistogramMetricsTracker from registering metrics twice in the same CollectorRegistry Prevent PrometheusHistogramMetricsTracker from registering metrics twice in the same CollectorRegistry Oct 14, 2019
@codecov
Copy link

codecov bot commented Oct 14, 2019

Codecov Report

Merging #1467 into dev will increase coverage by 0.29%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #1467      +/-   ##
============================================
+ Coverage     71.38%   71.68%   +0.29%     
- Complexity      548      550       +2     
============================================
  Files            25       25              
  Lines          2034     2034              
  Branches        263      264       +1     
============================================
+ Hits           1452     1458       +6     
+ Misses          439      432       -7     
- Partials        143      144       +1
Impacted Files Coverage Δ Complexity Δ
src/main/java/com/zaxxer/hikari/pool/PoolBase.java 70.67% <0%> (+2.25%) 54% <0%> (+2%) ⬆️

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 396896b...7aa70ec. Read the comment docs.

@edysli
Copy link
Author

edysli commented Oct 14, 2019

@apodkutin @vanDonselaar since you guys worked on this area of the code before, would you please review this PR?

@rabindrarakshit
Copy link

@edysli I think we can close/reject this PR since #1692 is already merged.

@edysli
Copy link
Author

edysli commented Jan 16, 2021

@edysli I think we can close/reject this PR since #1692 is already merged.

Absolutely! :D

@edysli edysli closed this Jan 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PrometheusHistogramMetricsTracker should not register metrics twice
2 participants