From b54db11a28737ca51d5a479ab8b4118a15d3abf2 Mon Sep 17 00:00:00 2001 From: Nick Allen Date: Tue, 29 Aug 2017 10:38:23 -0400 Subject: [PATCH 1/2] METRON-1138 Improve Error Message with Bad Profile Expression --- .../profiler/DefaultProfileBuilder.java | 53 +++++++++++++------ .../profiler/DefaultProfileBuilderTest.java | 2 +- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/metron-analytics/metron-profiler-common/src/main/java/org/apache/metron/profiler/DefaultProfileBuilder.java b/metron-analytics/metron-profiler-common/src/main/java/org/apache/metron/profiler/DefaultProfileBuilder.java index 1f352d0e0c..2e3416007b 100644 --- a/metron-analytics/metron-profiler-common/src/main/java/org/apache/metron/profiler/DefaultProfileBuilder.java +++ b/metron-analytics/metron-profiler-common/src/main/java/org/apache/metron/profiler/DefaultProfileBuilder.java @@ -27,9 +27,11 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import org.apache.commons.collections4.ListUtils; @@ -245,17 +247,28 @@ private Object execute(String expression, String expressionType) { * @param expressionType The type of expression; init, update, result. Provides additional context if expression execution fails. */ private void assign(Map expressions, Map transientState, String expressionType) { - try { - // execute each of the 'update' expressions - MapUtils.emptyIfNull(expressions) - .forEach((var, expr) -> executor.assign(var, expr, transientState)); + // for each expression... + for(Map.Entry entry : MapUtils.emptyIfNull(expressions).entrySet()) { + String var = entry.getKey(); + String expr = entry.getValue(); + + try { + // assign the result of the expression to the variable + executor.assign(var, expr, transientState); - } catch(ParseException e) { + } catch (Throwable e) { - // make it brilliantly clear that one of the 'update' expressions is bad - String msg = format("Bad '%s' expression: error=%s, profile=%s, entity=%s", expressionType, e.getMessage(), profileName, entity); - throw new ParseException(msg, e); + // in-scope variables = persistent state maintained by the profiler + the transient state + Set variablesInScope = new HashSet<>(); + variablesInScope.addAll(transientState.keySet()); + variablesInScope.addAll(executor.getState().keySet()); + + String msg = format("Bad '%s' expression: error='%s', expr='%s', profile='%s', entity='%s', variables-available='%s'", + expressionType, e.getMessage(), expr, profileName, entity, variablesInScope); + LOG.error(msg, e); + throw new ParseException(msg, e); + } } } @@ -269,14 +282,24 @@ private void assign(Map expressions, Map transie private List execute(List expressions, Map transientState, String expressionType) { List results = new ArrayList<>(); - try { - ListUtils.emptyIfNull(expressions) - .forEach((expr) -> results.add(executor.execute(expr, transientState, Object.class))); + for(String expr: ListUtils.emptyIfNull(expressions)) { + try { + // execute an expression + Object result = executor.execute(expr, transientState, Object.class); + results.add(result); + + } catch (Throwable e) { - } catch (Throwable e) { - String msg = format("Bad '%s' expression: error=%s, profile=%s, entity=%s", expressionType, e.getMessage(), profileName, entity); - LOG.error(msg, e); - throw new ParseException(msg, e); + // in-scope variables = persistent state maintained by the profiler + the transient state + Set variablesInScope = new HashSet<>(); + variablesInScope.addAll(transientState.keySet()); + variablesInScope.addAll(executor.getState().keySet()); + + String msg = format("Bad '%s' expression: error='%s', expr='%s', profile='%s', entity='%s', variables-available='%s'", + expressionType, e.getMessage(), expr, profileName, entity, variablesInScope); + LOG.error(msg, e); + throw new ParseException(msg, e); + } } return results; diff --git a/metron-analytics/metron-profiler-common/src/test/java/org/apache/metron/profiler/DefaultProfileBuilderTest.java b/metron-analytics/metron-profiler-common/src/test/java/org/apache/metron/profiler/DefaultProfileBuilderTest.java index 71ef9827b8..d25b7ff13a 100644 --- a/metron-analytics/metron-profiler-common/src/test/java/org/apache/metron/profiler/DefaultProfileBuilderTest.java +++ b/metron-analytics/metron-profiler-common/src/test/java/org/apache/metron/profiler/DefaultProfileBuilderTest.java @@ -600,7 +600,7 @@ public void testBadResultExpression() throws Exception { * "init": { "x": "0" }, * "update": { "x": "x + 1" }, * "result": "x", - * "groupBy": ["2 / 0"] + * "groupBy": ["nonexistant"] * } */ @Multiline From 70b59347bc8b31d3a27400a16e245e770b1352ba Mon Sep 17 00:00:00 2001 From: Nick Allen Date: Tue, 29 Aug 2017 11:01:01 -0400 Subject: [PATCH 2/2] Fixed docs for groupBy expression --- metron-analytics/metron-profiler/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/metron-analytics/metron-profiler/README.md b/metron-analytics/metron-profiler/README.md index f27af59a51..f2842169f7 100644 --- a/metron-analytics/metron-profiler/README.md +++ b/metron-analytics/metron-profiler/README.md @@ -200,6 +200,7 @@ A common use case would be grouping by day of week. This allows a contiguous sc ``` The expression can reference any of these variables. +* Any variable defined by the profile in its `init` or `update` expressions. * `profile` The name of the profile. * `entity` The name of the entity being profiled. * `start` The start time of the profile period in epoch milliseconds.