From 4d993c67c222f303a290d042e6eda679e77e70cb Mon Sep 17 00:00:00 2001 From: brharrington Date: Thu, 14 Mar 2024 07:58:43 -0500 Subject: [PATCH] fix NPE for missing tag with group by (#1122) The change in #1120 can result in a NullPointerException if a group by is used and one of the required dimensions is missing. --- .../com/netflix/spectator/atlas/impl/Evaluator.java | 2 +- .../netflix/spectator/atlas/impl/EvaluatorTest.java | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/spectator-reg-atlas/src/main/java/com/netflix/spectator/atlas/impl/Evaluator.java b/spectator-reg-atlas/src/main/java/com/netflix/spectator/atlas/impl/Evaluator.java index 9ed0e0e7c..252b1c10f 100644 --- a/spectator-reg-atlas/src/main/java/com/netflix/spectator/atlas/impl/Evaluator.java +++ b/spectator-reg-atlas/src/main/java/com/netflix/spectator/atlas/impl/Evaluator.java @@ -197,7 +197,7 @@ public EvalPayload eval(long timestamp) { double acc = expr.isCount() ? 1.0 : v; metrics.add(new EvalPayload.Metric(subId, tags, acc)); } - } else { + } else if (tags != null) { TagsValuePair p = new TagsValuePair(tags, v); aggregator.update(p); LOGGER.trace("aggregating: {}: {}", timestamp, p); 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 index 003978aae..cbd21d1b6 100644 --- 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 @@ -123,6 +123,19 @@ public void sumAndMaxForGroup() { Assertions.assertEquals(expected, sort(payload)); } + @Test + public void missingTagGroupBy() { + List subs = new ArrayList<>(); + subs.add(newSubscription("sum", ":true,:sum,(,app,),:by")); + + Evaluator evaluator = newEvaluator(); + evaluator.sync(subs); + EvalPayload payload = evaluator.eval(0L, data("foo", 1.0, 2.0, 3.0)); + + EvalPayload expected = new EvalPayload(0L, Collections.emptyList()); + Assertions.assertEquals(expected, sort(payload)); + } + @Test public void updateSub() {