Merge pull request #6878 from zmstone/slog-ftw

refactor: delete deprecated logging macros

.gitignore

@@ -62,3 +62,4 @@ erlang_ls.config
 .envrc
 # elixir
 mix.lock
+apps/emqx/test/emqx_static_checks_data/

apps/emqx/include/logger.hrl

@@ -17,46 +17,6 @@
 -ifndef(EMQX_LOGGER_HRL).
 -define(EMQX_LOGGER_HRL, true).
 
-%% debug | info | notice | warning | error | critical | alert | emergency
--define(DEBUG(Format), ?LOG(debug, Format, [])).
--define(DEBUG(Format, Args), ?LOG(debug, Format, Args)).
-
--define(INFO(Format), ?LOG(info, Format, [])).
--define(INFO(Format, Args), ?LOG(info, Format, Args)).
-
--define(NOTICE(Format), ?LOG(notice, Format, [])).
--define(NOTICE(Format, Args), ?LOG(notice, Format, Args)).
-
--define(WARN(Format), ?LOG(warning, Format, [])).
--define(WARN(Format, Args), ?LOG(warning, Format, Args)).
-
--define(ERROR(Format), ?LOG(error, Format, [])).
--define(ERROR(Format, Args), ?LOG(error, Format, Args)).
-
--define(CRITICAL(Format), ?LOG(critical, Format, [])).
--define(CRITICAL(Format, Args), ?LOG(critical, Format, Args)).
-
--define(ALERT(Format), ?LOG(alert, Format, [])).
--define(ALERT(Format, Args), ?LOG(alert, Format, Args)).
-
--define(LOG(Level, Format), ?LOG(Level, Format, [])).
-
-%% deprecated
--define(LOG(Level, Format, Args, Meta),
-        %% check 'allow' here so we do not have to pass an anonymous function
-        %% down to logger which may cause `badfun` exception during upgrade
-        case logger:allow(Level, ?MODULE) of
-            true ->
-                logger:log(Level, (Format), (Args),
-                           (Meta)#{ mfa => {?MODULE, ?FUNCTION_NAME, ?FUNCTION_ARITY}
-                                  , line => ?LINE
-                                  });
-            false ->
-                ok
-        end).
-
--define(LOG(Level, Format, Args), ?LOG(Level, Format, Args, #{})).
-
 %% structured logging
 -define(SLOG(Level, Data),
         ?SLOG(Level, Data, #{})).

apps/emqx/src/emqx_ctl.erl

@@ -105,7 +105,10 @@ run_command(Cmd, Args) when is_atom(Cmd) ->
                 _ -> ok
             catch
                 _:Reason:Stacktrace ->
-                    ?ERROR("CMD Error:~0p, Stacktrace:~0p", [Reason, Stacktrace]),
+                    ?SLOG(error, #{msg => "ctl_command_crashed",
+                                   stacktrace => Stacktrace,
+                                   reason => Reason
+                                  }),
                     {error, Reason}
             end;
         [] ->

apps/emqx/src/emqx_hocon.erl

@@ -0,0 +1,30 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2022 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+
+%% @doc HOCON schema help module
+-module(emqx_hocon).
+
+-export([format_path/1]).
+
+format_path([]) -> "";
+format_path([Name]) -> iol(Name);
+format_path([Name | Rest]) ->
+    [iol(Name) , "." | format_path(Rest)].
+
+%% Ensure iolist()
+iol(B) when is_binary(B) -> B;
+iol(A) when is_atom(A) -> atom_to_binary(A, utf8);
+iol(L) when is_list(L) -> L.

apps/emqx/test/emqx_bpapi_static_checks.erl

@@ -18,8 +18,6 @@
 
 -export([run/0, dump/1, dump/0, check_compat/1, versions_file/0]).
 
--include_lib("emqx/include/logger.hrl").
-
 %% Using an undocumented API here :(
 -include_lib("dialyzer/src/dialyzer.hrl").
 
@@ -75,14 +73,14 @@ run() ->
             Dumps = filelib:wildcard(dumps_dir() ++ "/*.bpapi"),
             case Dumps of
                 [] ->
-                    ?ERROR("No BPAPI dumps are found in ~s, abort", [dumps_dir()]),
+                    logger:error("No BPAPI dumps are found in ~s, abort", [dumps_dir()]),
                     false;
                 _ ->
-                    ?NOTICE("Running API compatibility checks for ~p", [Dumps]),
+                    logger:notice("Running API compatibility checks for ~p", [Dumps]),
                     check_compat(Dumps)
             end;
         false ->
-            ?CRITICAL("Backplane API violations found on the current branch."),
+            logger:critical("Backplane API violations found on the current branch.", []),
             false
     end.
 
@@ -116,11 +114,11 @@ check_api_immutability(#{release := Rel1, api := APIs1}, #{release := Rel2, api
                           ok;
                       undefined ->
                           setnok(),
-                          ?ERROR("API ~p v~p was removed in release ~p without being deprecated.",
+                          logger:error("API ~p v~p was removed in release ~p without being deprecated.",
                                        [API, Version, Rel2]);
                       _Val ->
                           setnok(),
-                          ?ERROR("API ~p v~p was changed between ~p and ~p. Backplane API should be immutable.",
+                          logger:error("API ~p v~p was changed between ~p and ~p. Backplane API should be immutable.",
                                        [API, Version, Rel1, Rel2])
                   end
           end,
@@ -145,7 +143,8 @@ typecheck_apis( #{release := CallerRelease, api := CallerAPIs, signatures := Cal
                                   ok;
                               TypeErrors ->
                                   setnok(),
-                                  [?ERROR("Incompatible RPC call: "
+                                  [logger:error(
+                                         "Incompatible RPC call: "
                                           "type of the parameter ~p of RPC call ~s on release ~p "
                                           "is not a subtype of the target function ~s on release ~p.~n"
                                           "Caller type: ~s~nCallee type: ~s~n",
@@ -211,21 +210,21 @@ dump(Opts) ->
     erase(bpapi_ok).
 
 prepare(#{reldir := RelDir, plt := PLT}) ->
-    ?INFO("Starting xref...", []),
+    logger:info("Starting xref...", []),
     xref:start(?XREF),
     filelib:wildcard(RelDir ++ "/*/ebin/") =:= [] andalso
         error("No applications found in the release directory. Wrong directory?"),
     xref:set_default(?XREF, [{warnings, false}]),
     xref:add_release(?XREF, RelDir),
     %% Now to the dialyzer stuff:
-    ?INFO("Loading PLT...", []),
+    logger:info("Loading PLT...", []),
     dialyzer_plt:from_file(PLT).
 
 find_remote_calls(_Opts) ->
     Query = "XC | (A - [" ?IGNORED_APPS "]:App - [" ?IGNORED_MODULES "]:Mod - [" ?EXEMPTIONS "])
                || (([" ?RPC_MODULES "] : Mod + [" ?RPC_FUNCTIONS "]) - [" ?IGNORED_RPC_CALLS "])",
     {ok, Calls} = xref:q(?XREF, Query),
-    ?INFO("Calls to RPC modules ~p", [Calls]),
+    logger:info("Calls to RPC modules ~p", [Calls]),
     {Callers, _Callees} = lists:unzip(Calls),
     Callers.
 
@@ -235,10 +234,9 @@ warn_nonbpapi_rpcs([]) ->
 warn_nonbpapi_rpcs(L) ->
     setnok(),
     lists:foreach(fun({M, F, A}) ->
-                          ?ERROR("~p:~p/~p does a remote call outside of a dedicated "
+                          logger:error("~p:~p/~p does a remote call outside of a dedicated "
                                        "backplane API module. "
-                                 "It may break during rolling cluster upgrade",
+                                       "It may break during rolling cluster upgrade", [M, F, A])
-                                 [M, F, A])
                   end,
                   L).
 
@@ -258,7 +256,7 @@ dump_api(Term = #{api := _, signatures := _, release := Release}) ->
 -spec dump_versions(api_dump()) -> ok.
 dump_versions(APIs) ->
     Filename = versions_file(),
-    ?NOTICE("Dumping API versions to ~p", [Filename]),
+    logger:notice("Dumping API versions to ~p", [Filename]),
     ok = filelib:ensure_dir(Filename),
     {ok, FD} = file:open(Filename, [write]),
     lists:foreach(fun(API) ->
@@ -308,7 +306,8 @@ enrich({From0, To0}, {Acc0, PLT}) ->
             {Acc, PLT};
         {{value, _}, none} ->
             setnok(),
-            ?CRITICAL("Backplane API function ~s calls a missing remote function ~s",
+            logger:critical(
+                  "Backplane API function ~s calls a missing remote function ~s",
                   [format_call(From0), format_call(To0)]),
             error(missing_target)
      end.

apps/emqx/test/emqx_static_checks.erl

@@ -19,7 +19,6 @@
 -compile(export_all).
 -compile(nowarn_export_all).
 
--include_lib("emqx/include/logger.hrl").
 -include_lib("common_test/include/ct.hrl").
 -include_lib("stdlib/include/assert.hrl").
 
@@ -29,7 +28,8 @@ init_per_suite(Config) ->
     Config.
 
 end_per_suite(_Config) ->
-    ?NOTICE("If this test suite failed, and you are unsure why, read this:~n"
+    logger:notice(
+          "If this test suite failed, and you are unsure why, read this:~n"
           "https://github.com/emqx/emqx/blob/master/apps/emqx/src/bpapi/README.md", []).
 
 t_run_check(_) ->
@@ -39,13 +39,14 @@ t_run_check(_) ->
         {ok, NewData} = file:consult(emqx_bpapi_static_checks:versions_file()),
         OldData =:= NewData orelse
             begin
-                ?CRITICAL("BPAPI versions were changed, but not committed to the repo.\n"
+                logger:critical(
+                      "BPAPI versions were changed, but not committed to the repo.\n"
                       "Run 'make && make static_checks' and then add the changed "
-                          "'bpapi.versions' files to the commit.", []),
+                      "'bpapi.versions' files to the commit."),
                 error(version_mismatch)
             end
     catch
         EC:Err:Stack ->
-            ?CRITICAL("Test suite failed: ~p:~p~nStack:~p", [EC, Err, Stack]),
+            logger:critical("Test suite failed: ~p:~p~nStack:~p", [EC, Err, Stack]),
             error(tc_failed)
     end.

apps/emqx_exhook/src/emqx_exhook_mgr.erl

@@ -331,8 +331,10 @@ handle_info({timeout, _Ref, {reload, Name}}, State) ->
         {error, not_found} ->
             {noreply, NState};
         {error, Reason} ->
-            ?LOG(warning, "Failed to reload exhook callback server \"~ts\", "
+            ?SLOG(warning,
-                 "Reason: ~0p", [Name, Reason]),
+                  #{msg => "failed_to_reload_exhook_callback_server",
+                    reason => Reason,
+                    name => Name}),
             {noreply, ensure_reload_timer(NState)}
     end;
 
@@ -382,8 +384,8 @@ do_load_server(Name, State = #{orders := Orders}) ->
                     {ok, ServerState} ->
                         save(Name, ServerState),
                         update_order(Orders),
-                        ?LOG(info, "Load exhook callback server "
+                        ?SLOG(info, #{msg => "load_exhook_callback_server_ok",
-                             "\"~ts\" successfully!", [Name]),
+                                      name => Name}),
                     {ok, State3#{running := maps:put(Name, Options, Running)}};
                 {error, Reason} ->
                     {{error, Reason}, State};

apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl

@@ -245,7 +245,8 @@ handle_timeout(_TRef, force_close, Channel = #channel{closed_reason = Reason}) -
     {shutdown, {error, {force_close, Reason}}, Channel};
 
 handle_timeout(_TRef, Msg, Channel) ->
-    ?WARN("Unexpected timeout: ~p", [Msg]),
+    ?SLOG(warning, #{msg => "unexpected_timeout_signal",
+                     signal => Msg}),
     {ok, Channel}.
 
 -spec handle_call(Req :: any(), From :: any(), channel())

apps/emqx_retainer/src/emqx_retainer.erl

@@ -60,6 +60,7 @@
 
 -define(DEF_MAX_PAYLOAD_SIZE, (1024 * 1024)).
 -define(DEF_EXPIRY_INTERVAL, 0).
+-define(MAX_PAYLOAD_SIZE_CONFIG_PATH, [retainer, max_payload_size]).
 
 -define(CAST(Msg), gen_server:cast(?MODULE, Msg)).
 
@@ -293,9 +294,9 @@ new_state() ->
 new_context(Id) ->
     #{context_id => Id}.
 
-is_too_big(Size) ->
+
-    Limit = emqx_conf:get([retainer, max_payload_size], ?DEF_MAX_PAYLOAD_SIZE),
+payload_size_limit() ->
-    Limit > 0 andalso (Size > Limit).
+    emqx_conf:get(?MAX_PAYLOAD_SIZE_CONFIG_PATH, ?DEF_MAX_PAYLOAD_SIZE).
 
 %% @private
 dispatch(Context, Topic) ->
@@ -309,13 +310,17 @@ delete_message(Context, Topic) ->
 
 -spec store_retained(context(), message()) -> ok.
 store_retained(Context, #message{topic = Topic, payload = Payload} = Msg) ->
-    case is_too_big(erlang:byte_size(Payload)) of
+    Size = iolist_size(Payload),
-        false ->
+    case payload_size_limit() of
-            Mod = get_backend_module(),
+        Limit when Limit > 0 andalso Limit < Size ->
-            Mod:store_retained(Context, Msg);
+            ?SLOG(error, #{msg => "retain_failed_for_payload_size_exceeded_limit",
+                           topic => Topic,
+                           config => emqx_hocon:format_path(?MAX_PAYLOAD_SIZE_CONFIG_PATH),
+                           size => Size,
+                           limit => Limit});
         _ ->
-            ?ERROR("Cannot retain message(topic=~ts, payload_size=~p) for payload is too big!",
+            Mod = get_backend_module(),
-                   [Topic, iolist_size(Payload)])
+            Mod:store_retained(Context, Msg)
     end.
 
 -spec clean(context()) -> ok.