tumblr / colossus

I/O and Microservice library for Scala
Apache License 2.0
1.14k stars 96 forks source link

fixing histogram values to not be higher than max #503

Closed amotamed closed 7 years ago

amotamed commented 7 years ago

@benblack86 @DanSimon @aliyakamercan

Since our histogram values are approximations, we occasionally run into times when the approximation calculation is greater than the max value. Since there's no issue in the underlying approximation calculation (tradeoff between accuracy and data size to keep) I'm setting a hard limit on max values.

codecov-io commented 7 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (develop@d19e11a). Click here to learn what that means. The diff coverage is 100%.

@@            Coverage Diff             @@
##             develop     #503   +/-   ##
==========================================
  Coverage           ?   84.89%           
==========================================
  Files              ?       98           
  Lines              ?     4264           
  Branches           ?      346           
==========================================
  Hits               ?     3620           
  Misses             ?      644           
  Partials           ?        0
Impacted Files Coverage Δ
.../scala/colossus/metrics/collectors/Histogram.scala 91.79% <100%> (ø)

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 d19e11a...aa02047. Read the comment docs.

benblack86 commented 7 years ago

👍

DanSimon commented 7 years ago

I don't know what the deal is with codecov, but all tests pass and I think this change makes a lot of sense so a big 👍 from me.