use stat placeholders for normalize (#1503)

When normalizing an expression that uses a filter, use one of the `:stat-$(aggr)` placeholders if possible. This avoids duplication of the input expression making them easier to read. It also helps if comparing expressions after normalization to see if they are equivalent.
Netflix · Dec 14, 2022 · 9a59fca · 9a59fca
1 parent 3fe64d9
commit 9a59fca
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 1 deletion.
diff --git a/atlas-webapi/src/main/scala/com/netflix/atlas/webapi/ExprApi.scala b/atlas-webapi/src/main/scala/com/netflix/atlas/webapi/ExprApi.scala
@@ -28,6 +28,7 @@ import com.netflix.atlas.core.model.FilterExpr
 import com.netflix.atlas.core.model.ModelExtractors
 import com.netflix.atlas.core.model.Query
 import com.netflix.atlas.core.model.StyleExpr
+import com.netflix.atlas.core.model.TimeSeriesExpr
 import com.netflix.atlas.core.stacklang.Context
 import com.netflix.atlas.core.stacklang.Interpreter
 import com.netflix.atlas.core.stacklang.Word
@@ -295,10 +296,31 @@ object ExprApi {
     }
   }
 
+  private def normalizeStat(expr: StyleExpr): StyleExpr = {
+    expr
+      .rewrite {
+        case FilterExpr.Filter(ts1, ts2) =>
+          val updated = ts2.rewrite {
+            case FilterExpr.Stat(ts, s, None) if ts == ts1 =>
+              s match {
+                case "avg"   => FilterExpr.StatAvg
+                case "min"   => FilterExpr.StatMin
+                case "max"   => FilterExpr.StatMax
+                case "last"  => FilterExpr.StatLast
+                case "total" => FilterExpr.StatTotal
+                case "count" => FilterExpr.StatCount
+                case _       => FilterExpr.Stat(ts, s, None)
+              }
+          }
+          FilterExpr.Filter(ts1, updated.asInstanceOf[TimeSeriesExpr])
+      }
+      .asInstanceOf[StyleExpr]
+  }
+
   private def exprStrings(exprs: List[StyleExpr]): List[String] = {
     // Rewrite the expressions and convert to a normalized strings
     exprs.map { expr =>
-      val rewritten = expr.rewrite {
+      val rewritten = normalizeStat(expr).rewrite {
         case q: Query => sort(q)
       }
       // Remove explicit :const, it can be determined from implicit conversion

diff --git a/atlas-webapi/src/test/scala/com/netflix/atlas/webapi/ExprApiSuite.scala b/atlas-webapi/src/test/scala/com/netflix/atlas/webapi/ExprApiSuite.scala
@@ -392,6 +392,24 @@ class ExprApiSuite extends MUnitRouteSuite {
     assertEquals(normalize(avg), List(avg))
   }
 
+  test("normalize :stat-aggr with no condition filters") {
+    val expr = "app,foo,:eq,name,cpuUser,:eq,:and,:sum,(,nf.cluster,),:by,:stat-max,:filter"
+    assertEquals(normalize(expr), List(expr))
+  }
+
+  test("normalize :stat to :stat-aggr if possible") {
+    val expr = "name,sps,:eq,(,nf.cluster,),:by,:dup,max,:stat,5,:gt,:filter"
+    val expected = "name,sps,:eq,:sum,(,nf.cluster,),:by,:stat-max,5.0,:gt,:filter"
+    assertEquals(normalize(expr), List(expected))
+  }
+
+  test("normalize :stat to :stat-aggr nested") {
+    val expr =
+      "name,sps,:eq,(,nf.cluster,),:by,:dup,:dup,max,:stat,:swap,avg,:stat,:sub,5,:gt,:filter"
+    val expected = "name,sps,:eq,:sum,(,nf.cluster,),:by,:stat-max,:stat-avg,:sub,5.0,:gt,:filter"
+    assertEquals(normalize(expr), List(expected))
+  }
+
   test("normalize simplify query") {
     val input = "app,foo,:eq,name,cpuUser,:eq,:and,:true,:and,:sum"
     val expected = "app,foo,:eq,name,cpuUser,:eq,:and,:sum"