Skip to content

Conversation

@laxmanchekka
Copy link
Contributor

@laxmanchekka laxmanchekka commented Dec 8, 2022

Description

  • Remove redundant dropwizard jvm/system metrics
  • Added JvmHeapPressureMetrics, FileDescriptorMetrics, JvmInfoMetrics
  • Add more quantiles for timer and distribution summary
  • Control the histogram buckets by supporting configuration of expected value range (min, max) for timers and distribution summaries. For histograms, if these min/max are not configured, then the algorithm will create huge number of buckets (~70 for timers, ~270 for distribution summaries). Each of these buckets translates to 1 time series in prometheus and that causes the prometheus to overflow/OOM/slowdown when we have many such meters. Hence, micrometer recommends to set these limits.

minimumExpectedValue/maximumExpectedValue: Controls the number of buckets shipped by publishPercentileHistogram and controls the accuracy and memory footprint of the underlying HdrHistogram structure.

More details about histograms here.
https://micrometer.io/docs/concepts#_histograms_and_percentiles

Testing

Tested manually.

Checklist:

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

Documentation

NA.

@github-actions

This comment has been minimized.

@laxmanchekka laxmanchekka changed the title metrics curation feat: metrics curation Dec 8, 2022
@codecov
Copy link

codecov bot commented Dec 8, 2022

Codecov Report

Merging #63 (2b855e9) into main (177d547) will decrease coverage by 0.27%.
The diff coverage is 92.30%.

@@             Coverage Diff              @@
##               main      #63      +/-   ##
============================================
- Coverage     69.50%   69.23%   -0.28%     
- Complexity      105      106       +1     
============================================
  Files            15       15              
  Lines           564      559       -5     
  Branches         33       32       -1     
============================================
- Hits            392      387       -5     
  Misses          153      153              
  Partials         19       19              
Flag Coverage Δ
unit 69.23% <92.30%> (-0.28%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...viceframework/metrics/PlatformMetricsRegistry.java 82.19% <92.30%> (-0.59%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

*
* See https://micrometer.io/docs/concepts#_timers for more details on the Timer.
*/
public static Timer registerTimer(String name, Map<String, String> tags, boolean histogram, Duration minExpectedValue, Duration maxExpectedValue) {
Copy link
Contributor

@avinashkolluru avinashkolluru Dec 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this minExpectedValue and maxExpectedValue and how will that help?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For histograms, if these min/max are not configured, then the algorithm will create huge number of buckets (~70 for timers, ~270 for distribution summaries). Each of these buckets translates to 1 time series in prometheus and that causes the prometheus to overflow/OOM/slowdown when we have many such meters.

Hence, micrometer recommends to set these limits.

minimumExpectedValue/maximumExpectedValue: Controls the number of buckets shipped by publishPercentileHistogram and controls the accuracy and memory footprint of the underlying HdrHistogram structure.

https://micrometer.io/docs/concepts#_histograms_and_percentiles

@laxmanchekka laxmanchekka merged commit b102518 into main Dec 8, 2022
@laxmanchekka laxmanchekka deleted the metrics-improvements branch December 8, 2022 11:58
@github-actions
Copy link

github-actions bot commented Dec 8, 2022

Unit Test Results

  9 files  ±0    9 suites  ±0   10s ⏱️ -1s
31 tests ±0  31 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b102518. ± Comparison against base commit 177d547.

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.

4 participants