From 8f1486f6d398e5d9b7e549aacf6f78320e722b86 Mon Sep 17 00:00:00 2001
From: Kjell Winblad <kjellwinblad@gmail.com>
Date: Thu, 18 Apr 2024 13:50:22 +0200
Subject: [PATCH] fix: clean up trace messages to make it easier to interpret

This commit removes some redundant trace messages and renames some to
make it easier to interpret what is happening for the user.
---
 .../test/emqx_bridge_http_SUITE.erl               |  3 +--
 apps/emqx_rule_engine/src/emqx_rule_runtime.erl   | 15 ++++++---------
 .../emqx_rule_engine_api_rule_apply_SUITE.erl     |  3 +--
 3 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/apps/emqx_bridge_http/test/emqx_bridge_http_SUITE.erl b/apps/emqx_bridge_http/test/emqx_bridge_http_SUITE.erl
index 73f6359ab..9d215d815 100644
--- a/apps/emqx_bridge_http/test/emqx_bridge_http_SUITE.erl
+++ b/apps/emqx_bridge_http/test/emqx_bridge_http_SUITE.erl
@@ -418,9 +418,8 @@ t_send_get_trace_messages(Config) ->
         begin
             Bin = read_rule_trace_file(TraceName, Now),
             ?assertNotEqual(nomatch, binary:match(Bin, [<<"rule_activated">>])),
-            ?assertNotEqual(nomatch, binary:match(Bin, [<<"SELECT_yielded_result">>])),
+            ?assertNotEqual(nomatch, binary:match(Bin, [<<"SQL_yielded_result">>])),
             ?assertNotEqual(nomatch, binary:match(Bin, [<<"bridge_action">>])),
-            ?assertNotEqual(nomatch, binary:match(Bin, [<<"action_activated">>])),
             ?assertNotEqual(nomatch, binary:match(Bin, [<<"action_template_rendered">>])),
             ?assertNotEqual(nomatch, binary:match(Bin, [<<"QUERY_ASYNC">>]))
         end
diff --git a/apps/emqx_rule_engine/src/emqx_rule_runtime.erl b/apps/emqx_rule_engine/src/emqx_rule_runtime.erl
index 3dfc5f6c8..3872fb973 100644
--- a/apps/emqx_rule_engine/src/emqx_rule_runtime.erl
+++ b/apps/emqx_rule_engine/src/emqx_rule_runtime.erl
@@ -132,7 +132,7 @@ apply_rule(Rule = #{id := RuleID}, Columns, Envs) ->
                     reason => Error,
                     stacktrace => StkTrace
                 },
-                warning
+                error
             ),
             {error, {Error, StkTrace}}
     after
@@ -176,18 +176,18 @@ do_apply_rule(
         {ok, ColumnsAndSelected, FinalCollection} ->
             case FinalCollection of
                 [] ->
-                    trace_rule_sql("FOREACH_yielded_no_result"),
+                    trace_rule_sql("SQL_yielded_no_result"),
                     ok = emqx_metrics_worker:inc(rule_metrics, RuleId, 'failed.no_result');
                 _ ->
                     trace_rule_sql(
-                        "FOREACH_yielded_result", #{result => FinalCollection}, debug
+                        "SQL_yielded_result", #{result => FinalCollection}, debug
                     ),
                     ok = emqx_metrics_worker:inc(rule_metrics, RuleId, 'passed')
             end,
             NewEnvs = maps:merge(ColumnsAndSelected, Envs),
             {ok, [handle_action_list(RuleId, Actions, Coll, NewEnvs) || Coll <- FinalCollection]};
         false ->
-            trace_rule_sql("FOREACH_yielded_no_result_no_match"),
+            trace_rule_sql("SQL_yielded_no_result"),
             ok = emqx_metrics_worker:inc(rule_metrics, RuleId, 'failed.no_result'),
             {error, nomatch}
     end;
@@ -204,11 +204,11 @@ do_apply_rule(
 ) ->
     case evaluate_select(Fields, Columns, Conditions) of
         {ok, Selected} ->
-            trace_rule_sql("SELECT_yielded_result", #{result => Selected}, debug),
+            trace_rule_sql("SQL_yielded_result", #{result => Selected}, debug),
             ok = emqx_metrics_worker:inc(rule_metrics, RuleId, 'passed'),
             {ok, handle_action_list(RuleId, Actions, Selected, maps:merge(Columns, Envs))};
         false ->
-            trace_rule_sql("SELECT_yielded_no_result_no_match"),
+            trace_rule_sql("SQL_yielded_no_result"),
             ok = emqx_metrics_worker:inc(rule_metrics, RuleId, 'failed.no_result'),
             {error, nomatch}
     end.
@@ -392,10 +392,8 @@ handle_action_list(RuleId, Actions, Selected, Envs) ->
 
 handle_action(RuleId, ActId, Selected, Envs) ->
     ok = emqx_metrics_worker:inc(rule_metrics, RuleId, 'actions.total'),
-    trace_action(ActId, "activating_action"),
     try
         Result = do_handle_action(RuleId, ActId, Selected, Envs),
-        trace_action(ActId, "action_activated", #{result => Result}),
         Result
     catch
         throw:out_of_service ->
@@ -467,7 +465,6 @@ do_handle_action(RuleId, #{mod := Mod, func := Func} = Action, Selected, Envs) -
     Result = Mod:Func(Selected, Envs, Args),
     {_, IncCtx} = do_handle_action_get_trace_inc_metrics_context(RuleId, Action),
     inc_action_metrics(IncCtx, Result),
-    trace_action(Action, "call_action_function_result", #{result => Result}, debug),
     Result.
 
 do_handle_action_get_trace_inc_metrics_context(RuleID, Action) ->
diff --git a/apps/emqx_rule_engine/test/emqx_rule_engine_api_rule_apply_SUITE.erl b/apps/emqx_rule_engine/test/emqx_rule_engine_api_rule_apply_SUITE.erl
index c875617ce..837fc3274 100644
--- a/apps/emqx_rule_engine/test/emqx_rule_engine_api_rule_apply_SUITE.erl
+++ b/apps/emqx_rule_engine/test/emqx_rule_engine_api_rule_apply_SUITE.erl
@@ -128,9 +128,8 @@ basic_apply_rule_test_helper(Config, TraceType, StopAfterRender) ->
             Bin = read_rule_trace_file(TraceName, TraceType, Now),
             io:format("THELOG:~n~s", [Bin]),
             ?assertNotEqual(nomatch, binary:match(Bin, [<<"rule_activated">>])),
-            ?assertNotEqual(nomatch, binary:match(Bin, [<<"SELECT_yielded_result">>])),
+            ?assertNotEqual(nomatch, binary:match(Bin, [<<"SQL_yielded_result">>])),
             ?assertNotEqual(nomatch, binary:match(Bin, [<<"bridge_action">>])),
-            ?assertNotEqual(nomatch, binary:match(Bin, [<<"action_activated">>])),
             ?assertNotEqual(nomatch, binary:match(Bin, [<<"action_template_rendered">>])),
             ?assertNotEqual(nomatch, binary:match(Bin, [<<"QUERY_ASYNC">>]))
         end