atlas: aggr function with empty tags (#1097)

Uses a placeholder name of `unknown` if the query for an aggregate function has no exact tags. This matches the behavior of the backend.
Netflix · Dec 2, 2023 · 278395b · 278395b
1 parent 3fdaece
commit 278395b
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 13 deletions.
diff --git a/spectator-reg-atlas/src/main/java/com/netflix/spectator/atlas/impl/DataExpr.java b/spectator-reg-atlas/src/main/java/com/netflix/spectator/atlas/impl/DataExpr.java
@@ -176,6 +176,16 @@ final class All implements DataExpr {
 
   /** Base type for simple aggregate functions. */
   interface AggregateFunction extends DataExpr {
+
+    /** Return the exact matches from the query clause. */
+    Map<String, String> queryTags();
+
+    @Override default Map<String, String> resultTags(Map<String, String> tags) {
+      Map<String, String> ts = queryTags();
+      return ts.isEmpty()
+          ? Collections.singletonMap("name", "unknown")
+          : ts;
+    }
   }
 
   /**
@@ -202,7 +212,7 @@ final class Sum implements AggregateFunction {
       return true;
     }
 
-    @Override public Map<String, String> resultTags(Map<String, String> tags) {
+    @Override public Map<String, String> queryTags() {
       return queryTags;
     }
 
@@ -269,7 +279,7 @@ final class Min implements AggregateFunction {
       return false;
     }
 
-    @Override public Map<String, String> resultTags(Map<String, String> tags) {
+    @Override public Map<String, String> queryTags() {
       return queryTags;
     }
 
@@ -336,7 +346,7 @@ final class Max implements AggregateFunction {
       return false;
     }
 
-    @Override public Map<String, String> resultTags(Map<String, String> tags) {
+    @Override public Map<String, String> queryTags() {
       return queryTags;
     }
 
@@ -407,7 +417,7 @@ final class Count implements AggregateFunction {
       return true;
     }
 
-    @Override public Map<String, String> resultTags(Map<String, String> tags) {
+    @Override public Map<String, String> queryTags() {
       return queryTags;
     }
 
@@ -496,7 +506,7 @@ private Map<String, String> keyTags(Map<String, String> tags) {
       if (resultTags == null) {
         return null;
       } else {
-        resultTags.putAll(af.resultTags(tags));
+        resultTags.putAll(af.queryTags());
         return resultTags;
       }
     }

diff --git a/spectator-reg-atlas/src/test/java/com/netflix/spectator/atlas/impl/DataExprTest.java b/spectator-reg-atlas/src/test/java/com/netflix/spectator/atlas/impl/DataExprTest.java
@@ -313,6 +313,20 @@ public void mismatchedClosingParen() {
         () -> Parser.parseDataExpr("key,(,key,),),:in,:sum"));
   }
 
+  @Test
+  public void orClauseTags() {
+    DataExpr expr = parse("name,foo,:eq,name,bar,:eq,:or,:sum");
+    Map<String, String> tags = expr.resultTags(Collections.singletonMap("name", "foo"));
+    Assertions.assertEquals(Collections.singletonMap("name", "unknown"), tags);
+  }
+
+  @Test
+  public void orClauseTagsGroupBy() {
+    DataExpr expr = parse("name,foo,:eq,name,bar,:eq,:or,:sum,(,name,),:by");
+    Map<String, String> tags = expr.resultTags(Collections.singletonMap("name", "foo"));
+    Assertions.assertEquals(Collections.singletonMap("name", "foo"), tags);
+  }
+
   @Test
   public void allEqualsContract() {
     EqualsVerifier

diff --git a/spectator-reg-atlas/src/test/java/com/netflix/spectator/atlas/impl/EvaluatorTest.java b/spectator-reg-atlas/src/test/java/com/netflix/spectator/atlas/impl/EvaluatorTest.java
@@ -117,8 +117,8 @@ public void sumAndMaxForGroup() {
     EvalPayload payload = evaluator.eval(0L, data("foo", 1.0, 2.0, 3.0));
 
     List<EvalPayload.Metric> metrics = new ArrayList<>();
-    metrics.add(new EvalPayload.Metric("max", Collections.emptyMap(), 3.0));
-    metrics.add(new EvalPayload.Metric("sum", Collections.emptyMap(), 6.0));
+    metrics.add(new EvalPayload.Metric("max", tags("name", "unknown"), 3.0));
+    metrics.add(new EvalPayload.Metric("sum", tags("name", "unknown"), 6.0));
     EvalPayload expected = new EvalPayload(0L, metrics);
     Assertions.assertEquals(expected, sort(payload));
   }
@@ -133,7 +133,7 @@ public void updateSub() {
     evaluator.sync(sumSub);
     EvalPayload payload = evaluator.eval(0L, data("foo", 1.0, 2.0, 3.0));
     List<EvalPayload.Metric> metrics = new ArrayList<>();
-    metrics.add(new EvalPayload.Metric("sum", Collections.emptyMap(), 6.0));
+    metrics.add(new EvalPayload.Metric("sum", tags("name", "unknown"), 6.0));
     EvalPayload expected = new EvalPayload(0L, metrics);
     Assertions.assertEquals(expected, payload);
 
@@ -143,7 +143,7 @@ public void updateSub() {
     evaluator.sync(maxSub);
     payload = evaluator.eval(0L, data("foo", 1.0, 2.0, 3.0));
     metrics = new ArrayList<>();
-    metrics.add(new EvalPayload.Metric("sum", Collections.emptyMap(), 3.0));
+    metrics.add(new EvalPayload.Metric("sum", tags("name", "unknown"), 3.0));
     expected = new EvalPayload(0L, metrics);
     Assertions.assertEquals(expected, payload);
   }
@@ -212,7 +212,7 @@ public void delayAggrCounterSum() {
 
     Assertions.assertEquals(1, payload.getMetrics().size());
     Assertions.assertEquals(
-        new EvalPayload.Metric("sum", tags(), 6.0),
+        new EvalPayload.Metric("sum", tags("name", "unknown"), 6.0),
         payload.getMetrics().get(0)
     );
   }
@@ -229,7 +229,7 @@ public void delayAggrGaugeSum() {
     Assertions.assertEquals(3, payload.getMetrics().size());
     for (EvalPayload.Metric m : payload.getMetrics()) {
       Map<String, String> tags = m.getTags();
-      Assertions.assertEquals(1, tags.size());
+      Assertions.assertEquals(2, tags.size());
       Assertions.assertTrue(tags.containsKey("atlas.aggr"));
     }
   }
@@ -247,7 +247,7 @@ public void delayAggrGaugeCount() {
     Assertions.assertEquals(3, payload.getMetrics().size());
     for (EvalPayload.Metric m : payload.getMetrics()) {
       Map<String, String> tags = m.getTags();
-      Assertions.assertEquals(1, tags.size());
+      Assertions.assertEquals(2, tags.size());
       Assertions.assertTrue(tags.containsKey("atlas.aggr"));
       Assertions.assertEquals(1.0, m.getValue());
     }
@@ -294,7 +294,7 @@ public void delayAggrGaugeMax() {
 
     Assertions.assertEquals(1, payload.getMetrics().size());
     Assertions.assertEquals(
-        new EvalPayload.Metric("max", tags(), 3.0),
+        new EvalPayload.Metric("max", tags("name", "unknown"), 3.0),
         payload.getMetrics().get(0)
     );
   }