feat: refactor log configuration

apps/emqx/src/config/emqx_config_logger.erl

@@ -112,8 +112,8 @@ update_log_handler({Action, {handler, Id, Mod, Conf}}) ->
     end,
     ok.
 
-id_for_log(console) -> "log.console_handler";
+id_for_log(console) -> "log.console";
-id_for_log(Other) -> "log.file_handlers." ++ atom_to_list(Other).
+id_for_log(Other) -> "log.file." ++ atom_to_list(Other).
 
 atom(Id) when is_binary(Id) -> binary_to_atom(Id, utf8);
 atom(Id) when is_atom(Id) -> Id.
@@ -126,12 +126,12 @@ tr_handlers(Conf) ->
 
 %% For the default logger that outputs to console
 tr_console_handler(Conf) ->
-    case conf_get("log.console_handler.enable", Conf) of
+    case conf_get("log.console.enable", Conf) of
         true ->
-            ConsoleConf = conf_get("log.console_handler", Conf),
+            ConsoleConf = conf_get("log.console", Conf),
             [
                 {handler, console, logger_std_h, #{
-                    level => conf_get("log.console_handler.level", Conf),
+                    level => conf_get("log.console.level", Conf),
                     config => (log_handler_conf(ConsoleConf))#{type => standard_io},
                     formatter => log_formatter(ConsoleConf),
                     filters => log_filter(ConsoleConf)
@@ -150,14 +150,10 @@ tr_file_handler({HandlerName, SubConf}) ->
     {handler, atom(HandlerName), logger_disk_log_h, #{
         level => conf_get("level", SubConf),
         config => (log_handler_conf(SubConf))#{
-            type =>
+            type => wrap,
-                case conf_get("rotation.enable", SubConf) of
+            file => conf_get("sink_to", SubConf),
-                    true -> wrap;
+            max_no_files => conf_get("rotation_count", SubConf),
-                    _ -> halt
+            max_no_bytes => conf_get("rotation_size", SubConf)
-                end,
-            file => conf_get("file", SubConf),
-            max_no_files => conf_get("rotation.count", SubConf),
-            max_no_bytes => conf_get("max_size", SubConf)
         },
         formatter => log_formatter(SubConf),
         filters => log_filter(SubConf),
@@ -165,15 +161,7 @@ tr_file_handler({HandlerName, SubConf}) ->
     }}.
 
 logger_file_handlers(Conf) ->
-    Handlers = maps:to_list(conf_get("log.file_handlers", Conf, #{})),
+    maps:to_list(conf_get("log.file", Conf, #{})).
-    lists:filter(
-        fun({_Name, Opts}) ->
-            B = conf_get("enable", Opts),
-            true = is_boolean(B),
-            B
-        end,
-        Handlers
-    ).
 
 conf_get(Key, Conf) -> emqx_schema:conf_get(Key, Conf).
 conf_get(Key, Conf, Default) -> emqx_schema:conf_get(Key, Conf, Default).
@@ -237,12 +225,8 @@ log_filter(Conf) ->
     end.
 
 tr_level(Conf) ->
-    ConsoleLevel = conf_get("log.console_handler.level", Conf, undefined),
+    ConsoleLevel = conf_get("log.console.level", Conf, undefined),
-    FileLevels = [
+    FileLevels = [conf_get("level", SubConf) || {_, SubConf} <- logger_file_handlers(Conf)],
-        conf_get("level", SubConf)
-     || {_, SubConf} <-
-            logger_file_handlers(Conf)
-    ],
     case FileLevels ++ [ConsoleLevel || ConsoleLevel =/= undefined] of
         %% warning is the default level we should use
         [] -> warning;

apps/emqx_conf/src/emqx_conf_schema.erl

@@ -687,11 +687,12 @@ fields("rpc") ->
                     desc => ?DESC(rpc_mode)
                 }
             )},
-        {"driver",
+        {"protocol",
             sc(
                 hoconsc:enum([tcp, ssl]),
                 #{
                     mapping => "gen_rpc.driver",
+                    aliases => [driver],
                     default => tcp,
                     desc => ?DESC(rpc_driver)
                 }
@@ -866,19 +867,22 @@ fields("rpc") ->
     ];
 fields("log") ->
     [
-        {"console_handler",
+        {"console",
+            sc(?R_REF("console_handler"), #{
+                aliases => [console_handler],
+                importance => ?IMPORTANCE_HIGH
+            })},
+        {"file",
             sc(
-                ?R_REF("console_handler"),
+                ?UNION([
-                #{importance => ?IMPORTANCE_HIGH}
+                    ?R_REF("log_file_handler"),
-            )},
+                    ?MAP(handler_name, ?R_REF("log_file_handler"))
-        {"file_handlers",
+                ]),
-            sc(
-                map(name, ?R_REF("log_file_handler")),
                 #{
                     desc => ?DESC("log_file_handlers"),
-                    %% because file_handlers is a map
+                    converter => fun ensure_file_handlers/2,
-                    %% so there has to be a default value in order to populate the raw configs
+                    default => #{<<"level">> => <<"warning">>},
-                    default => #{<<"default">> => #{<<"level">> => <<"warning">>}},
+                    aliases => [file_handlers],
                     importance => ?IMPORTANCE_HIGH
                 }
             )}
@@ -887,50 +891,39 @@ fields("console_handler") ->
     log_handler_common_confs(console);
 fields("log_file_handler") ->
     [
-        {"file",
+        {"sink_to",
             sc(
                 file(),
                 #{
                     desc => ?DESC("log_file_handler_file"),
                     default => <<"${EMQX_LOG_DIR}/emqx.log">>,
                     converter => fun emqx_schema:naive_env_interpolation/1,
-                    validator => fun validate_file_location/1
+                    validator => fun validate_file_location/1,
+                    aliases => [file],
+                    importance => ?IMPORTANCE_HIGH
                 }
             )},
-        {"rotation",
+        {"rotation_count",
             sc(
-                ?R_REF("log_rotation"),
+                range(1, 128),
-                #{}
+                #{
+                    aliases => [rotation],
+                    default => 10,
+                    desc => ?DESC("log_rotation_count"),
+                    importance => ?IMPORTANCE_MEDIUM
+                }
             )},
-        {"max_size",
+        {"rotation_size",
             sc(
                 hoconsc:union([infinity, emqx_schema:bytesize()]),
                 #{
                     default => <<"50MB">>,
                     desc => ?DESC("log_file_handler_max_size"),
+                    aliases => [max_size],
                     importance => ?IMPORTANCE_MEDIUM
                 }
             )}
     ] ++ log_handler_common_confs(file);
-fields("log_rotation") ->
-    [
-        {"enable",
-            sc(
-                boolean(),
-                #{
-                    default => true,
-                    desc => ?DESC("log_rotation_enable")
-                }
-            )},
-        {"count",
-            sc(
-                range(1, 2048),
-                #{
-                    default => 10,
-                    desc => ?DESC("log_rotation_count")
-                }
-            )}
-    ];
 fields("log_overload_kill") ->
     [
         {"enable",
@@ -1038,8 +1031,8 @@ translation("ekka") ->
     [{"cluster_discovery", fun tr_cluster_discovery/1}];
 translation("kernel") ->
     [
-        {"logger_level", fun tr_logger_level/1},
+        {"logger_level", fun emqx_config_logger:tr_level/1},
-        {"logger", fun tr_logger_handlers/1},
+        {"logger", fun emqx_config_logger:tr_handlers/1},
         {"error_logger", fun(_) -> silent end}
     ];
 translation("emqx") ->
@@ -1113,24 +1106,9 @@ tr_cluster_discovery(Conf) ->
     Strategy = conf_get("cluster.discovery_strategy", Conf),
     {Strategy, filter(cluster_options(Strategy, Conf))}.
 
--spec tr_logger_level(hocon:config()) -> logger:level().
-tr_logger_level(Conf) ->
-    emqx_config_logger:tr_level(Conf).
-
-tr_logger_handlers(Conf) ->
-    emqx_config_logger:tr_handlers(Conf).
-
 log_handler_common_confs(Handler) ->
-    lists:map(
-        fun
-            ({_Name, #{importance := _}} = F) -> F;
-            ({Name, Sc}) -> {Name, Sc#{importance => ?IMPORTANCE_LOW}}
-        end,
-        do_log_handler_common_confs(Handler)
-    ).
-do_log_handler_common_confs(Handler) ->
     %% we rarely support dynamic defaults like this
-    %% for this one, we have build-time defualut the same as runtime default
+    %% for this one, we have build-time default the same as runtime default
     %% so it's less tricky
     EnableValues =
         case Handler of
@@ -1140,21 +1118,31 @@ do_log_handler_common_confs(Handler) ->
     EnvValue = os:getenv("EMQX_DEFAULT_LOG_HANDLER"),
     Enable = lists:member(EnvValue, EnableValues),
     [
+        {"level",
+            sc(
+                log_level(),
+                #{
+                    default => warning,
+                    desc => ?DESC("common_handler_level"),
+                    importance => ?IMPORTANCE_HIGH
+                }
+            )},
         {"enable",
             sc(
                 boolean(),
                 #{
                     default => Enable,
                     desc => ?DESC("common_handler_enable"),
-                    importance => ?IMPORTANCE_LOW
+                    importance => ?IMPORTANCE_MEDIUM
                 }
             )},
-        {"level",
+        {"formatter",
             sc(
-                log_level(),
+                hoconsc:enum([text, json]),
                 #{
-                    default => warning,
+                    default => text,
-                    desc => ?DESC("common_handler_level")
+                    desc => ?DESC("common_handler_formatter"),
+                    importance => ?IMPORTANCE_MEDIUM
                 }
             )},
         {"time_offset",
@@ -1173,16 +1161,7 @@ do_log_handler_common_confs(Handler) ->
                 #{
                     default => unlimited,
                     desc => ?DESC("common_handler_chars_limit"),
-                    importance => ?IMPORTANCE_LOW
+                    importance => ?IMPORTANCE_HIDDEN
-                }
-            )},
-        {"formatter",
-            sc(
-                hoconsc:enum([text, json]),
-                #{
-                    default => text,
-                    desc => ?DESC("common_handler_formatter"),
-                    importance => ?IMPORTANCE_MEDIUM
                 }
             )},
         {"single_line",
@@ -1191,7 +1170,7 @@ do_log_handler_common_confs(Handler) ->
                 #{
                     default => true,
                     desc => ?DESC("common_handler_single_line"),
-                    importance => ?IMPORTANCE_LOW
+                    importance => ?IMPORTANCE_HIDDEN
                 }
             )},
         {"sync_mode_qlen",
@@ -1199,7 +1178,8 @@ do_log_handler_common_confs(Handler) ->
                 non_neg_integer(),
                 #{
                     default => 100,
-                    desc => ?DESC("common_handler_sync_mode_qlen")
+                    desc => ?DESC("common_handler_sync_mode_qlen"),
+                    importance => ?IMPORTANCE_HIDDEN
                 }
             )},
         {"drop_mode_qlen",
@@ -1207,7 +1187,8 @@ do_log_handler_common_confs(Handler) ->
                 pos_integer(),
                 #{
                     default => 3000,
-                    desc => ?DESC("common_handler_drop_mode_qlen")
+                    desc => ?DESC("common_handler_drop_mode_qlen"),
+                    importance => ?IMPORTANCE_HIDDEN
                 }
             )},
         {"flush_qlen",
@@ -1215,17 +1196,19 @@ do_log_handler_common_confs(Handler) ->
                 pos_integer(),
                 #{
                     default => 8000,
-                    desc => ?DESC("common_handler_flush_qlen")
+                    desc => ?DESC("common_handler_flush_qlen"),
+                    importance => ?IMPORTANCE_HIDDEN
                 }
             )},
-        {"overload_kill", sc(?R_REF("log_overload_kill"), #{})},
+        {"overload_kill", sc(?R_REF("log_overload_kill"), #{importance => ?IMPORTANCE_HIDDEN})},
-        {"burst_limit", sc(?R_REF("log_burst_limit"), #{})},
+        {"burst_limit", sc(?R_REF("log_burst_limit"), #{importance => ?IMPORTANCE_HIDDEN})},
         {"supervisor_reports",
             sc(
                 hoconsc:enum([error, progress]),
                 #{
                     default => error,
-                    desc => ?DESC("common_handler_supervisor_reports")
+                    desc => ?DESC("common_handler_supervisor_reports"),
+                    importance => ?IMPORTANCE_HIDDEN
                 }
             )},
         {"max_depth",
@@ -1233,7 +1216,8 @@ do_log_handler_common_confs(Handler) ->
                 hoconsc:union([unlimited, non_neg_integer()]),
                 #{
                     default => 100,
-                    desc => ?DESC("common_handler_max_depth")
+                    desc => ?DESC("common_handler_max_depth"),
+                    importance => ?IMPORTANCE_HIDDEN
                 }
             )}
     ].
@@ -1355,3 +1339,9 @@ validator_string_re(Val, RE, Error) ->
 
 node_array() ->
     hoconsc:union([emqx_schema:comma_separated_atoms(), hoconsc:array(atom())]).
+
+ensure_file_handlers(Conf, _Opts) ->
+    FileFields = lists:map(fun({F, _}) -> list_to_binary(F) end, fields("log_file_handler")),
+    HandlersWithoutName = maps:with(FileFields, Conf),
+    HandlersWithName = maps:without(FileFields, Conf),
+    emqx_utils_maps:deep_merge(#{<<"default">> => HandlersWithoutName}, HandlersWithName).

apps/emqx_conf/test/emqx_conf_schema_tests.erl

@@ -47,6 +47,103 @@ array_nodes_test() ->
     ),
     ok.
 
+%% erlfmt-ignore
+-define(OUTDATED_LOG_CONF,
+    """
+console_handler {
+  burst_limit {
+    enable = true
+    max_count = 10000
+    window_time = 1000
+  }
+  chars_limit = unlimited
+  drop_mode_qlen = 3000
+  enable = true
+  flush_qlen = 8000
+  formatter = text
+  level = warning
+  max_depth = 100
+  overload_kill {
+    enable = true
+    mem_size = 31457280
+    qlen = 20000
+    restart_after = 5000
+  }
+  single_line = true
+  supervisor_reports = error
+  sync_mode_qlen = 100
+  time_offset = system
+}
+file_handlers {
+  default {
+    burst_limit {
+      enable = true
+      max_count = 10000
+      window_time = 1000
+    }
+    chars_limit = unlimited
+    drop_mode_qlen = 3000
+    enable = false
+    file = \"log/emqx.log\"
+    flush_qlen = 8000
+    formatter = text
+    level = warning
+    max_depth = 100
+    max_size = 52428800
+    overload_kill {
+      enable = true
+      mem_size = 31457280
+      qlen = 20000
+      restart_after = 5000
+    }
+    rotation {count = 10, enable = true}
+    single_line = true
+    supervisor_reports = error
+    sync_mode_qlen = 100
+    time_offset = \"+01:00\"
+  }
+}
+    """
+).
+
+outdated_log_test() ->
+    BaseConf = to_bin(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"]),
+    Conf0 = <<BaseConf/binary, ?OUTDATED_LOG_CONF>>,
+    {ok, ConfMap0} = hocon:binary(Conf0, #{format => richmap}),
+    ConfList = hocon_tconf:generate(emqx_conf_schema, ConfMap0),
+    ct:pal("fff:~p", [ConfList]),
+    Log = proplists:get_value(log, ConfList),
+    Console = proplists:get_value(console, Log),
+    File = proplists:get_value(file, Log),
+    ?assertEqual(1, Console, {Console, File}),
+    ok.
+
+-define(NEW_LOG_CONF,
+    ""
+    "\n"
+    "console {\n"
+    "  enable = true\n"
+    "  formatter = text\n"
+    "  level = warning\n"
+    "  time_offset = system\n"
+    "}\n"
+    "file {\n"
+    "    enable = true\n"
+    "    file = \"log/emqx.log\"\n"
+    "    formatter = text\n"
+    "    level = warning\n"
+    "    rotation {count = 10, enable = true}\n"
+    "    time_offset = \"+01:00\"\n"
+    "  }\n"
+    "file_handlers.default {\n"
+    "    enable = false,\n"
+    "    file = \"log/file_handlres_emqx.log\"\n"
+    "  }\n"
+    "}\n"
+    "    "
+    ""
+).
+
 %% erlfmt-ignore
 -define(BASE_AUTHN_ARRAY,
     """

bin/emqx

@@ -875,16 +875,16 @@ tr_log_to_env() {
     unset EMQX_LOG__TO
     case "${log_to}" in
         console)
-            export EMQX_LOG__CONSOLE_HANDLER__ENABLE='true'
+            export EMQX_LOG__CONSOLE__ENABLE='true'
-            export EMQX_LOG__FILE_HANDLERS__DEFAULT__ENABLE='false'
+            export EMQX_LOG__FILE__ENABLE='false'
             ;;
         file)
-            export EMQX_LOG__CONSOLE_HANDLER__ENABLE='false'
+            export EMQX_LOG__CONSOLE__ENABLE='false'
-            export EMQX_LOG__FILE_HANDLERS__DEFAULT__ENABLE='true'
+            export EMQX_LOG__FILE__ENABLE='true'
             ;;
         both)
-            export EMQX_LOG__CONSOLE_HANDLER__ENABLE='true'
+            export EMQX_LOG__CONSOLE__ENABLE='true'
-            export EMQX_LOG__FILE_HANDLERS__DEFAULT__ENABLE='true'
+            export EMQX_LOG__FILE__ENABLE='true'
             ;;
         default)
             # want to use config file defaults, do nothing