Skip to content

Commit

Permalink
core: fix percentile approximation (#1715)
Browse files Browse the repository at this point in the history
In #1704 the approximation was updated to use doubles
instead of longs. As part of that non-finite values were
ignored. Before they would be treated as 0. By ignoring
them a bad value could potentially be carried forward if
it only occurred for a single interval.
  • Loading branch information
brharrington authored Oct 29, 2024
1 parent 062ac71 commit 80de795
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -980,8 +980,7 @@ object MathExpr {
var j = 0
while (j < usedCounts.length) {
val v = bounded(j).data(i)
if (v.isFinite)
counts(usedCounts(j)) = v
counts(usedCounts(j)) = if (v.isFinite) v else 0.0
j += 1
}
PercentileBuckets.percentiles(counts, pcts, results)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ class PercentilesSuite extends FunSuite {
} toList
}

private val inputNaN100 = {
(0 until 100).map { i =>
val bucket = f"D${PercentileBuckets.indexOf(i)}%04X"
val v = 1.0 / 60.0
ts(bucket, v, Double.NaN)
} toList
}

private val inputBad100 = {
// simulates bad client that incorrectly encodes the percentile tag
(0 until 100).map { i =>
Expand Down Expand Up @@ -137,6 +145,24 @@ class PercentilesSuite extends FunSuite {
assertEquals(e.getMessage, "requirement failed: invalid percentile encoding: [D000A,D000a]")
}

test("distribution summary non-finite data") {
val data = eval("name,test,:eq,(,9,25,50,90,100,),:percentiles", inputNaN100)

assertEquals(data.size, 5)
List(9.0, 25.0, 50.0, 90.0).zip(data).foreach {
case (p, t) =>
assertEquals(t.tags, Map("name" -> "test", "percentile" -> f"$p%5.1f"))
assertEquals(t.label, f"percentile(name=test, $p%5.1f)")

val estimate1 = t.data(0L)
assertEqualsDouble(p, estimate1, 2.0)

val estimate2 = t.data(step)
assert(estimate2.isNaN)
}
assertEquals(data.last.label, f"percentile(name=test, 100.0)")
}

test("timer :sum") {
val data = eval("name,test,:eq,(,25,50,90,),:percentiles", inputTimer100)

Expand Down

0 comments on commit 80de795

Please sign in to comment.