From 7b000157d033f8cde85ae163d75697af3c658fd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9F=90=E6=96=87?= Date: Fri, 5 May 2023 00:35:27 +0800 Subject: [PATCH] feat: refactor log configuration --- apps/emqx/src/config/emqx_config_logger.erl | 40 ++--- apps/emqx_conf/src/emqx_conf_schema.erl | 144 ++++++++---------- .../emqx_conf/test/emqx_conf_schema_tests.erl | 97 ++++++++++++ bin/emqx | 12 +- 4 files changed, 182 insertions(+), 111 deletions(-) diff --git a/apps/emqx/src/config/emqx_config_logger.erl b/apps/emqx/src/config/emqx_config_logger.erl index b0fc1ca67..901056c0f 100644 --- a/apps/emqx/src/config/emqx_config_logger.erl +++ b/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(Other) -> "log.file_handlers." ++ atom_to_list(Other). +id_for_log(console) -> "log.console"; +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 => - case conf_get("rotation.enable", SubConf) of - true -> wrap; - _ -> halt - end, - file => conf_get("file", SubConf), - max_no_files => conf_get("rotation.count", SubConf), - max_no_bytes => conf_get("max_size", SubConf) + type => wrap, + file => conf_get("sink_to", SubConf), + max_no_files => conf_get("rotation_count", SubConf), + max_no_bytes => conf_get("rotation_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, #{})), - lists:filter( - fun({_Name, Opts}) -> - B = conf_get("enable", Opts), - true = is_boolean(B), - B - end, - Handlers - ). + maps:to_list(conf_get("log.file", Conf, #{})). 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), - FileLevels = [ - conf_get("level", SubConf) - || {_, SubConf} <- - logger_file_handlers(Conf) - ], + ConsoleLevel = conf_get("log.console.level", Conf, undefined), + FileLevels = [conf_get("level", SubConf) || {_, SubConf} <- logger_file_handlers(Conf)], case FileLevels ++ [ConsoleLevel || ConsoleLevel =/= undefined] of %% warning is the default level we should use [] -> warning; diff --git a/apps/emqx_conf/src/emqx_conf_schema.erl b/apps/emqx_conf/src/emqx_conf_schema.erl index 94cbfb221..09f51a3b7 100644 --- a/apps/emqx_conf/src/emqx_conf_schema.erl +++ b/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"), - #{importance => ?IMPORTANCE_HIGH} - )}, - {"file_handlers", - sc( - map(name, ?R_REF("log_file_handler")), + ?UNION([ + ?R_REF("log_file_handler"), + ?MAP(handler_name, ?R_REF("log_file_handler")) + ]), #{ desc => ?DESC("log_file_handlers"), - %% because file_handlers is a map - %% so there has to be a default value in order to populate the raw configs - default => #{<<"default">> => #{<<"level">> => <<"warning">>}}, + converter => fun ensure_file_handlers/2, + 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", fun tr_logger_handlers/1}, + {"logger_level", fun emqx_config_logger:tr_level/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, - desc => ?DESC("common_handler_level") + default => text, + 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 - } - )}, - {"formatter", - sc( - hoconsc:enum([text, json]), - #{ - default => text, - desc => ?DESC("common_handler_formatter"), - importance => ?IMPORTANCE_MEDIUM + importance => ?IMPORTANCE_HIDDEN } )}, {"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"), #{})}, - {"burst_limit", sc(?R_REF("log_burst_limit"), #{})}, + {"overload_kill", sc(?R_REF("log_overload_kill"), #{importance => ?IMPORTANCE_HIDDEN})}, + {"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). diff --git a/apps/emqx_conf/test/emqx_conf_schema_tests.erl b/apps/emqx_conf/test/emqx_conf_schema_tests.erl index 667d1766f..0e88da318 100644 --- a/apps/emqx_conf/test/emqx_conf_schema_tests.erl +++ b/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 = <>, + {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, """ diff --git a/bin/emqx b/bin/emqx index 3b7212c99..07684af8e 100755 --- a/bin/emqx +++ b/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__FILE_HANDLERS__DEFAULT__ENABLE='false' + export EMQX_LOG__CONSOLE__ENABLE='true' + export EMQX_LOG__FILE__ENABLE='false' ;; file) - export EMQX_LOG__CONSOLE_HANDLER__ENABLE='false' - export EMQX_LOG__FILE_HANDLERS__DEFAULT__ENABLE='true' + export EMQX_LOG__CONSOLE__ENABLE='false' + export EMQX_LOG__FILE__ENABLE='true' ;; both) - export EMQX_LOG__CONSOLE_HANDLER__ENABLE='true' - export EMQX_LOG__FILE_HANDLERS__DEFAULT__ENABLE='true' + export EMQX_LOG__CONSOLE__ENABLE='true' + export EMQX_LOG__FILE__ENABLE='true' ;; default) # want to use config file defaults, do nothing