Skip to content

Commit

Permalink
core: fix expr string for :percentiles (#1716)
Browse files Browse the repository at this point in the history
The changes in #1706 have a bug for percentiles used after
a group by where the query clause would get duplicated.
  • Loading branch information
brharrington authored Nov 1, 2024
1 parent 80de795 commit 2dac822
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -872,9 +872,10 @@ object MathExpr {

override def append(builder: java.lang.StringBuilder): Unit = {
// Base expr
Interpreter.append(builder, expr.af.query)
if (evalGroupKeys.nonEmpty)
Interpreter.append(builder, expr.af.query, evalGroupKeys, ":by")
else
Interpreter.append(builder, expr.af.query)

// Percentiles
builder.append(",(,")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,25 @@ class PercentilesSuite extends FunSuite {
private val step = 60000L
private val context = EvalContext(start, start + step * 2, step)

def ts(bucket: String, values: Double*): TimeSeries = {
private def ts(bucket: String, values: Double*): TimeSeries = {
val seq = new ArrayTimeSeq(DsType.Gauge, start, step, values.toArray)
val mode = if (Integer.parseInt(bucket.substring(1), 16) % 2 == 0) "even" else "odd"
TimeSeries(Map("name" -> "test", "mode" -> mode, "percentile" -> bucket), seq)
}

def eval(str: String, input: List[TimeSeries]): List[TimeSeries] = {
val expr = interpreter.execute(str).stack match {
private def parseExpr(str: String): TimeSeriesExpr = {
interpreter.execute(str).stack match {
case (v: TimeSeriesExpr) :: _ => v
case _ => throw new IllegalArgumentException("invalid expr")
}
}

private def eval(str: String, input: List[TimeSeries]): List[TimeSeries] = {
val expr = parseExpr(str)
// Verify we can reparse the string representation and get an identical expression.
val e2 = parseExpr(expr.toString)
val e3 = parseExpr(e2.toString)
assertEquals(e2, e3)
expr.eval(context, input).data
}

Expand Down Expand Up @@ -308,6 +316,19 @@ class PercentilesSuite extends FunSuite {
}
}

test("group by with math") {
val data = eval("name,test,:eq,(,mode,),:by,(,90,),:percentiles,1000,:mul", input100)
.sortWith(_.tags("mode") < _.tags("mode"))

assertEquals(data.size, 2)
List("even", "odd").zip(data).foreach {
case (m, t) =>
val estimate = t.data(0L)
assertEquals(t.tags, Map("name" -> "test", "mode" -> m, "percentile" -> " 90.0"))
assertEquals(t.label, f"(percentile((mode=$m), 90.0) * 1000.0)")
}
}

test("distribution summary empty") {
val data = eval(":false,(,25,50,90,),:percentiles", input100)
assertEquals(data.size, 0)
Expand Down

0 comments on commit 2dac822

Please sign in to comment.