Merge pull request #11682 from thalesmg/fix-file-logger-type-r53-20230925

fix(file_logger): change file logger type depending on rotation size

apps/emqx/src/config/emqx_config_logger.erl

@@ -151,13 +151,22 @@ tr_file_handlers(Conf) ->
     lists:map(fun tr_file_handler/1, Handlers).
 
 tr_file_handler({HandlerName, SubConf}) ->
+    FilePath = conf_get("path", SubConf),
+    RotationCount = conf_get("rotation_count", SubConf),
+    RotationSize = conf_get("rotation_size", SubConf),
+    Type =
+        case RotationSize of
+            infinity -> halt;
+            _ -> wrap
+        end,
+    HandlerConf = log_handler_conf(SubConf),
     {handler, atom(HandlerName), logger_disk_log_h, #{
         level => conf_get("level", SubConf),
-        config => (log_handler_conf(SubConf))#{
-            type => wrap,
-            file => conf_get("path", SubConf),
-            max_no_files => conf_get("rotation_count", SubConf),
-            max_no_bytes => conf_get("rotation_size", SubConf)
+        config => HandlerConf#{
+            type => Type,
+            file => FilePath,
+            max_no_files => RotationCount,
+            max_no_bytes => RotationSize
         },
         formatter => log_formatter(HandlerName, SubConf),
         filters => log_filter(HandlerName, SubConf),

apps/emqx_conf/test/emqx_conf_logger_SUITE.erl

@@ -19,45 +19,51 @@
 -compile(export_all).
 
 -include_lib("eunit/include/eunit.hrl").
+-include_lib("common_test/include/ct.hrl").
 -include_lib("snabbkaffe/include/snabbkaffe.hrl").
 
 %% erlfmt-ignore
 -define(BASE_CONF,
     """
-             node {
-                name = \"emqx1@127.0.0.1\"
-                cookie = \"emqxsecretcookie\"
-                data_dir = \"data\"
-             }
-             cluster {
-                name = emqxcl
-                discovery_strategy = static
-                static.seeds = \"emqx1@127.0.0.1\"
-                core_nodes = \"emqx1@127.0.0.1\"
-             }
-             log {
-                console {
-                enable = true
-                level = debug
-                }
-                file {
-                enable = true
-                level = info
-                path = \"log/emqx.log\"
-                }
-             }
+    log {
+       console {
+         enable = true
+         level = debug
+       }
+       file {
+         enable = true
+         level = info
+         path = \"log/emqx.log\"
+       }
+    }
     """).
 
 all() ->
     emqx_common_test_helpers:all(?MODULE).
 
 init_per_suite(Config) ->
-    emqx_common_test_helpers:load_config(emqx_conf_schema, iolist_to_binary(?BASE_CONF)),
-    emqx_mgmt_api_test_util:init_suite([emqx_conf]),
-    Config.
+    Apps = emqx_cth_suite:start(
+        [
+            emqx,
+            {emqx_conf, ?BASE_CONF}
+        ],
+        #{work_dir => emqx_cth_suite:work_dir(Config)}
+    ),
+    [{apps, Apps} | Config].
+
+end_per_suite(Config) ->
+    Apps = ?config(apps, Config),
+    ok = emqx_cth_suite:stop(Apps),
+    ok.
 
-end_per_suite(_Config) ->
-    emqx_mgmt_api_test_util:end_suite([emqx_conf]).
+init_per_testcase(_TestCase, Config) ->
+    LogConfRaw = emqx_conf:get_raw([<<"log">>]),
+    [{log_conf_raw, LogConfRaw} | Config].
+
+end_per_testcase(_TestCase, Config) ->
+    LogConfRaw = ?config(log_conf_raw, Config),
+    {ok, _} = emqx_conf:update([<<"log">>], LogConfRaw, #{}),
+    ok.
 
 t_log_conf(_Conf) ->
     FileExpect = #{
@@ -109,3 +115,23 @@ t_log_conf(_Conf) ->
     ?assertMatch({error, {not_found, default}}, logger:get_handler_config(default)),
     ?assertMatch({error, {not_found, console}}, logger:get_handler_config(console)),
     ok.
+
+t_file_logger_infinity_rotation(_Config) ->
+    ConfPath = [<<"log">>],
+    FileConfPath = [<<"file">>, <<"default">>],
+    ConfRaw = emqx_conf:get_raw(ConfPath),
+    FileConfRaw = emqx_utils_maps:deep_get(FileConfPath, ConfRaw),
+    %% inconsistent config: infinity rotation size, but finite rotation count
+    BadFileConfRaw = maps:merge(
+        FileConfRaw,
+        #{
+            <<"rotation_size">> => <<"infinity">>,
+            <<"rotation_count">> => 10
+        }
+    ),
+    BadConfRaw = emqx_utils_maps:deep_put(FileConfPath, ConfRaw, BadFileConfRaw),
+    ?assertMatch({ok, _}, emqx_conf:update(ConfPath, BadConfRaw, #{})),
+    HandlerIds = logger:get_handler_ids(),
+    %% ensure that the handler is correctly added
+    ?assert(lists:member(default, HandlerIds), #{handler_ids => HandlerIds}),
+    ok.

apps/emqx_conf/test/emqx_conf_schema_tests.erl

@@ -194,6 +194,59 @@ validate_log(Conf) ->
         ConsoleHandler
     ).
 
+%% erlfmt-ignore
+-define(FILE_LOG_BASE_CONF,
+    """
+    log.file.default {
+        enable = true
+        file = \"log/xx-emqx.log\"
+        formatter = text
+        level = debug
+        rotation_count = ~s
+        rotation_size = ~s
+        time_offset = \"+01:00\"
+      }
+    """
+).
+
+file_log_infinity_rotation_size_test_() ->
+    ensure_acl_conf(),
+    BaseConf = to_bin(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"]),
+    Gen = fun(#{count := Count, size := Size}) ->
+        Conf0 = to_bin(?FILE_LOG_BASE_CONF, [Count, Size]),
+        Conf1 = [BaseConf, Conf0],
+        {ok, Conf} = hocon:binary(Conf1, #{format => richmap}),
+        ConfList = hocon_tconf:generate(emqx_conf_schema, Conf),
+        Kernel = proplists:get_value(kernel, ConfList),
+        Loggers = proplists:get_value(logger, Kernel),
+        FileHandlers = lists:filter(fun(L) -> element(3, L) =:= logger_disk_log_h end, Loggers),
+        lists:keyfind(default, 2, FileHandlers)
+    end,
+    [
+        {"base conf: finite log (type = wrap)",
+            ?_assertMatch(
+                {handler, default, logger_disk_log_h, #{
+                    config := #{
+                        type := wrap,
+                        max_no_bytes := 1073741824,
+                        max_no_files := 20
+                    }
+                }},
+                Gen(#{count => "20", size => "\"1024MB\""})
+            )},
+        {"rotation size = infinity (type = halt)",
+            ?_assertMatch(
+                {handler, default, logger_disk_log_h, #{
+                    config := #{
+                        type := halt,
+                        max_no_bytes := infinity,
+                        max_no_files := 9
+                    }
+                }},
+                Gen(#{count => "9", size => "\"infinity\""})
+            )}
+    ].
+
 %% erlfmt-ignore
 -define(KERNEL_LOG_CONF,
     """

changes/ce/fix-1111682.en.md

@@ -0,0 +1 @@
+Fixed an issue where logging would stop if "Rotation Size" would be set to `infinity` on file log handlers.