+ core: provide sum and percentile functions in Histogram.Snapshot, closes #85

3 files changed, 36 insertions, 38 deletions

diff --git a/kamon-core/src/main/scala/kamon/metric/instrument/Histogram.scala b/kamon-core/src/main/scala/kamon/metric/instrument/Histogram.scala
index 67db5d93..813a2a16 100644
--- a/kamon-core/src/main/scala/kamon/metric/instrument/Histogram.scala
+++ b/kamon-core/src/main/scala/kamon/metric/instrument/Histogram.scala
@@ -75,6 +75,8 @@ object Histogram {
     def numberOfMeasurements: Long
     def min: Long
     def max: Long
+    def sum: Long
+    def percentile(percentile: Double): Long
     def recordsIterator: Iterator[Record]
     def merge(that: Histogram.Snapshot, context: CollectionContext): Histogram.Snapshot
   }
@@ -83,6 +85,8 @@ object Histogram {
     def empty(targetScale: Scale) = new Snapshot {
       override def min: Long = 0L
       override def max: Long = 0L
+      override def sum: Long = 0L
+      override def percentile(percentile: Double): Long = 0L
       override def recordsIterator: Iterator[Record] = Iterator.empty
       override def merge(that: Snapshot, context: CollectionContext): Snapshot = that
       override def scale: Scale = targetScale
@@ -161,6 +165,22 @@ class CompactHdrSnapshot(val scale: Scale, val numberOfMeasurements: Long, compa
 
   def min: Long = if (compactRecords.length == 0) 0 else levelFromCompactRecord(compactRecords(0))
   def max: Long = if (compactRecords.length == 0) 0 else levelFromCompactRecord(compactRecords(compactRecords.length - 1))
+  def sum: Long = recordsIterator.foldLeft(0L)((a, r) ⇒ a + (r.count * r.level))
+
+  def percentile(p: Double): Long = {
+    val records = recordsIterator
+    val threshold = numberOfMeasurements * (p / 100D)
+    var countToCurrentLevel = 0L
+    var percentileLevel = 0L
+
+    while (countToCurrentLevel < threshold && records.hasNext) {
+      val record = records.next()
+      countToCurrentLevel += record.count
+      percentileLevel = record.level
+    }
+
+    percentileLevel
+  }
 
   def merge(that: Histogram.Snapshot, context: CollectionContext): Histogram.Snapshot = {
     if (that.isEmpty) this else if (this.isEmpty) that else {
diff --git a/kamon-core/src/test/scala/kamon/metric/instrument/HistogramSpec.scala b/kamon-core/src/test/scala/kamon/metric/instrument/HistogramSpec.scala
index cefdf0f4..c3060d4a 100644
--- a/kamon-core/src/test/scala/kamon/metric/instrument/HistogramSpec.scala
+++ b/kamon-core/src/test/scala/kamon/metric/instrument/HistogramSpec.scala
@@ -57,7 +57,7 @@ class HistogramSpec extends WordSpec with Matchers {
     }
 
     "produce a snapshot" which {
-      "supports min, max and numberOfMeasurements operations" in new HistogramFixture {
+      "supports min, max, percentile, sum and numberOfMeasurements operations" in new HistogramFixture {
         histogram.record(100)
         histogram.record(200, count = 200)
         histogram.record(300)
@@ -67,7 +67,12 @@ class HistogramSpec extends WordSpec with Matchers {
 
         snapshot.min should equal(100L +- 1L)
         snapshot.max should equal(900L +- 9L)
+        snapshot.percentile(50.0D) should be(200)
+        snapshot.percentile(99.5D) should be(300)
+        snapshot.percentile(99.9D) should be(900)
+        snapshot.sum should be(41300)
         snapshot.numberOfMeasurements should be(203)
+
       }
 
       "can be merged with another snapshot" in new MultipleHistogramFixture {
diff --git a/kamon-log-reporter/src/main/scala/kamon/logreporter/LogReporter.scala b/kamon-log-reporter/src/main/scala/kamon/logreporter/LogReporter.scala
index 307343de..05841c14 100644
--- a/kamon-log-reporter/src/main/scala/kamon/logreporter/LogReporter.scala
+++ b/kamon-log-reporter/src/main/scala/kamon/logreporter/LogReporter.scala
@@ -122,11 +122,11 @@ class LogReporterSubscriber extends Actor with ActorLogging {
           name,
           ams.processingTime.numberOfMeasurements, ams.timeInMailbox.numberOfMeasurements, ams.mailboxSize.min,
           ams.processingTime.min, ams.timeInMailbox.min, ams.mailboxSize.average,
-          ams.processingTime.percentile(0.50F), ams.timeInMailbox.percentile(0.50F), ams.mailboxSize.max,
-          ams.processingTime.percentile(0.90F), ams.timeInMailbox.percentile(0.90F),
-          ams.processingTime.percentile(0.95F), ams.timeInMailbox.percentile(0.95F),
-          ams.processingTime.percentile(0.99F), ams.timeInMailbox.percentile(0.99F), ams.errors.count,
-          ams.processingTime.percentile(0.999F), ams.timeInMailbox.percentile(0.999F),
+          ams.processingTime.percentile(50.0D), ams.timeInMailbox.percentile(50.0D), ams.mailboxSize.max,
+          ams.processingTime.percentile(90.0D), ams.timeInMailbox.percentile(90.0D),
+          ams.processingTime.percentile(95.0D), ams.timeInMailbox.percentile(95.0D),
+          ams.processingTime.percentile(99.0D), ams.timeInMailbox.percentile(99.0D), ams.errors.count,
+          ams.processingTime.percentile(99.9D), ams.timeInMailbox.percentile(99.9D),
           ams.processingTime.max, ams.timeInMailbox.max))
   }
 
@@ -169,7 +169,7 @@ class LogReporterSubscriber extends Actor with ActorLogging {
         ||                                                                                                  |
         |+--------------------------------------------------------------------------------------------------+"""
         .stripMargin.format(
-          rxBytes.min, txBytes.min, rxErrors.total, txErrors.total,
+          rxBytes.min, txBytes.min, rxErrors.sum, txErrors.sum,
           rxBytes.average, txBytes.average,
           rxBytes.max, txBytes.max))
   }
@@ -295,9 +295,9 @@ class LogReporterSubscriber extends Actor with ActorLogging {
     val sb = StringBuilder.newBuilder
 
     sb.append("|    Min: %-11s  50th Perc: %-12s   90th Perc: %-12s   95th Perc: %-12s |\n".format(
-      histogram.min, histogram.percentile(0.50F), histogram.percentile(0.90F), histogram.percentile(0.95F)))
+      histogram.min, histogram.percentile(50.0D), histogram.percentile(90.0D), histogram.percentile(95.0D)))
     sb.append("|                      99th Perc: %-12s 99.9th Perc: %-12s         Max: %-12s |".format(
-      histogram.percentile(0.99F), histogram.percentile(0.999F), histogram.max))
+      histogram.percentile(99.0D), histogram.percentile(99.9D), histogram.max))
 
     sb.toString()
   }
@@ -310,36 +310,9 @@ class LogReporterSubscriber extends Actor with ActorLogging {
 object LogReporterSubscriber {
 
   implicit class RichHistogramSnapshot(histogram: Histogram.Snapshot) {
-    def percentile(q: Float): Long = {
-      val records = histogram.recordsIterator
-      val qThreshold = histogram.numberOfMeasurements * q
-      var countToCurrentLevel = 0L
-      var qLevel = 0L
-
-      while (countToCurrentLevel < qThreshold && records.hasNext) {
-        val record = records.next()
-        countToCurrentLevel += record.count
-        qLevel = record.level
-      }
-
-      qLevel
-    }
-
     def average: Double = {
-      var acc = 0L
-      for (record ← histogram.recordsIterator) {
-        acc += record.count * record.level
-      }
-
-      return acc / histogram.numberOfMeasurements
-    }
-
-    def total: Long = {
-      histogram.recordsIterator.foldLeft(0L) { (acc, record) ⇒
-        {
-          acc + (record.count * record.level)
-        }
-      }
+      if(histogram.numberOfMeasurements == 0) 0D
+      else histogram.sum / histogram.numberOfMeasurements
     }
   }
 }
\ No newline at end of file