From 49d3163471aeb61c7015bd06c22c7af78823ab77 Mon Sep 17 00:00:00 2001
From: "Zaiming (Stone) Shi" <zmstone@gmail.com>
Date: Tue, 8 Feb 2022 18:52:51 +0100
Subject: [PATCH] fix(emqx_cluster_rpc): do not log throw stacktrace

also not logging the call arguments as it may containe sensitive info
---
 apps/emqx_conf/src/emqx_cluster_rpc.erl | 34 +++++++++++++++----------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/apps/emqx_conf/src/emqx_cluster_rpc.erl b/apps/emqx_conf/src/emqx_cluster_rpc.erl
index e7e922397..2c8b14393 100644
--- a/apps/emqx_conf/src/emqx_cluster_rpc.erl
+++ b/apps/emqx_conf/src/emqx_cluster_rpc.erl
@@ -228,7 +228,7 @@ catch_up(#{node := Node, retry_interval := RetryMs} = State, SkipResult) ->
                         {atomic, ok} -> catch_up(State, false);
                         Error ->
                             ?SLOG(error, #{
-                                msg => "failed to commit applied call",
+                                msg => "failed_to_commit_applied_call",
                                 applied_id => NextId,
                                 error => Error}),
                             RetryMs
@@ -359,28 +359,34 @@ apply_mfa(TnxId, {M, F, A}) ->
     Res =
         try erlang:apply(M, F, A)
         catch
-            Class:Reason:Stacktrace ->
+            throw : Reason ->
+                {error, #{reason => Reason}};
+            Class : Reason : Stacktrace ->
                 {error, #{exception => Class, reason => Reason, stacktrace => Stacktrace}}
         end,
-    Meta = #{tnx_id => TnxId, module => M, function => F, args => ?TO_BIN(A)},
+    %% Do not log args as it might be sensitive information
+    Meta = #{tnx_id => TnxId, entrypoint => format_mfa(M, F, length(A))},
     IsSuccess = is_success(Res),
-    log_and_alarm(IsSuccess, Res, Meta, TnxId),
+    log_and_alarm(IsSuccess, Res, Meta),
     {IsSuccess, Res}.
 
+format_mfa(M, F, A) ->
+    iolist_to_binary([atom_to_list(M), ":", atom_to_list(F), "/", integer_to_list(A)]).
+
 is_success(ok) -> true;
 is_success({ok, _}) -> true;
 is_success(_) -> false.
 
-log_and_alarm(true, Res, Meta, TnxId) ->
-    OkMeta = Meta#{msg => <<"succeeded to apply MFA">>, result => Res},
-    ?SLOG(debug, OkMeta),
-    Message = ["cluster_rpc_apply_failed:", integer_to_binary(TnxId)],
-    emqx_alarm:deactivate(cluster_rpc_apply_failed, OkMeta#{result => ?TO_BIN(Res)}, Message);
-log_and_alarm(false, Res, Meta, TnxId) ->
-    NotOkMeta = Meta#{msg => <<"failed to apply MFA">>, result => Res},
-    ?SLOG(error, NotOkMeta),
-    Message = ["cluster_rpc_apply_failed:", integer_to_binary(TnxId)],
-    emqx_alarm:activate(cluster_rpc_apply_failed, NotOkMeta#{result => ?TO_BIN(Res)}, Message).
+log_and_alarm(true, Res, Meta) ->
+    ?SLOG(debug, Meta#{msg => "cluster_rpc_apply_ok", result => Res}),
+    do_alarm(deactivate, Res, Meta);
+log_and_alarm(false, Res, Meta) ->
+    ?SLOG(error, Meta#{msg => "cluster_rpc_apply_failed", result => Res}),
+    do_alarm(activate, Res, Meta).
+
+do_alarm(Fun, Res, #{tnx_id := Id} = Meta) ->
+    AlarmMsg = ["cluster_rpc_apply_failed=", integer_to_list(Id)],
+    emqx_alarm:Fun(cluster_rpc_apply_failed, Meta#{result => ?TO_BIN(Res)}, AlarmMsg).
 
 wait_for_all_nodes_commit(TnxId, Delay, Remain) ->
     case lagging_node(TnxId) of