From 5bfde9daf93a54980a3ae3057bfa49a5a8b02f46 Mon Sep 17 00:00:00 2001
From: Brian Harrington <brharrington@gmail.com>
Date: Wed, 14 Dec 2022 06:45:41 -0600
Subject: [PATCH] use stat placeholders for normalize

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.
---
 .../com/netflix/atlas/webapi/ExprApi.scala    | 24 ++++++++++++++++++-
 .../netflix/atlas/webapi/ExprApiSuite.scala   | 18 ++++++++++++++
 2 files changed, 41 insertions(+), 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
index c8299c3f0..065643ff5 100644
--- 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
index bd32f3ad5..5afd687e9 100644
--- 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"