rabbit_prelaunch_logging: $RABBITMQ_LOGS doesn't override log levelRABBITMQ_LOGS-envvar-does-not-override-configured-log-level

... if it is set in the configuration file.

Here is an example of that use case:
* The official Docker image sets RABBITMQ_LOGS=- in the environment
* A user of that image adds a configuration file with:
      log.console.level = debug

The initial implementation, introduced in rabbitmq/rabbitmq-server#2861,
considered that if the output is overriden in the environment (through
$RABBITMQ_LOGS), any output configuration in the configuration file is
ignored.

The problem is that the output-specific configuration could also set the
log level which is not changed by $RABBITMQ_LOGS. This patch fixes that
by keeping the log level from the configuration (if it is set obviously)
even if the output is overridden in the environment.

2 files changed, 117 insertions, 20 deletions

diff --git a/deps/rabbit/src/rabbit_prelaunch_logging.erl b/deps/rabbit/src/rabbit_prelaunch_logging.erl
index d8d561a7f4..b70c881582 100644
--- a/deps/rabbit/src/rabbit_prelaunch_logging.erl
+++ b/deps/rabbit/src/rabbit_prelaunch_logging.erl
@@ -680,7 +680,10 @@ handle_default_main_output(
     Overridden = rabbit_env:has_var_been_overridden(Context, main_log_file),
     Outputs1 = if
                    NoOutputsConfigured orelse Overridden ->
-                       log_file_var_to_outputs(MainLogFile);
+                       Output0 = log_file_var_to_output(MainLogFile),
+                       Output1 = keep_log_level_from_equivalent_output(
+                                   Output0, Outputs),
+                       [Output1];
                    true ->
                        [case Output of
                             #{module := Mod,
@@ -719,7 +722,10 @@ handle_default_upgrade_cat_output(
                    Context, upgrade_log_file),
     Outputs1 = if
                    NoOutputsConfigured orelse Overridden ->
-                       log_file_var_to_outputs(UpgLogFile);
+                       Output0 = log_file_var_to_output(UpgLogFile),
+                       Output1 = keep_log_level_from_equivalent_output(
+                                   Output0, Outputs),
+                       [Output1];
                    true ->
                       Outputs
                end,
@@ -731,25 +737,65 @@ handle_default_upgrade_cat_output(
                                                     outputs => Outputs1}}}
     end.
 
--spec log_file_var_to_outputs(file:filename() | string()) ->
-    [logger:handler_config()].
+-spec log_file_var_to_output(file:filename() | string()) ->
+    logger:handler_config().
+
+log_file_var_to_output("-") ->
+    #{module => rabbit_logger_std_h,
+      config => #{type => standard_io}};
+log_file_var_to_output("-stderr") ->
+    #{module => rabbit_logger_std_h,
+      config => #{type => standard_error}};
+log_file_var_to_output("exchange:" ++ _) ->
+    #{module => rabbit_logger_exchange_h,
+      config => #{}};
+log_file_var_to_output("syslog:" ++ _) ->
+    #{module => syslog_logger_h,
+      config => #{}};
+log_file_var_to_output(Filename) ->
+    #{module => rabbit_logger_std_h,
+      config => #{type => file,
+                  file => Filename}}.
+
+-spec keep_log_level_from_equivalent_output(
+        logger:handler_config(), [logger:handler_config()]) ->
+    logger:handler_config().
+%% @doc
+%% Keeps the log level from the equivalent output if found in the given list of
+%% outputs.
+%%
+%% If the output is overridden from the environment, or if no output is
+%% configured at all (and the default output is used), we should still keep the
+%% log level set in the configuration. The idea is that the $RABBITMQ_LOGS
+%% environment variable only overrides the output, not its log level (which
+%% would be set in $RABBITMQ_LOG).
+%%
+%% Here is an example of when it is used:
+%% * "$RABBITMQ_LOGS=-" is set in the environment
+%% * "log.console.level = debug" is set in the configuration file
+
+keep_log_level_from_equivalent_output(
+  #{module := Mod, config := #{type := Type}} = Output,
+  [#{module := Mod, config := #{type := Type}} = OverridenOutput | _])
+  when ?IS_STD_H_COMPAT(Mod) ->
+    keep_log_level_from_equivalent_output1(Output, OverridenOutput);
+keep_log_level_from_equivalent_output(
+  #{module := Mod} = Output,
+  [#{module := Mod} = OverridenOutput | _]) ->
+    keep_log_level_from_equivalent_output1(Output, OverridenOutput);
+keep_log_level_from_equivalent_output(Output, [_ | Rest]) ->
+    keep_log_level_from_equivalent_output(Output, Rest);
+keep_log_level_from_equivalent_output(Output, []) ->
+    Output.
+
+-spec keep_log_level_from_equivalent_output1(
+        logger:handler_config(), logger:handler_config()) ->
+    logger:handler_config().
 
-log_file_var_to_outputs("-") ->
-    [#{module => rabbit_logger_std_h,
-       config => #{type => standard_io}}];
-log_file_var_to_outputs("-stderr") ->
-    [#{module => rabbit_logger_std_h,
-       config => #{type => standard_error}}];
-log_file_var_to_outputs("exchange:" ++ _) ->
-    [#{module => rabbit_logger_exchange_h,
-       config => #{}}];
-log_file_var_to_outputs("syslog:" ++ _) ->
-    [#{module => syslog_logger_h,
-       config => #{}}];
-log_file_var_to_outputs(Filename) ->
-    [#{module => rabbit_logger_std_h,
-       config => #{type => file,
-                   file => Filename}}].
+keep_log_level_from_equivalent_output1(Output, #{level := Level}) ->
+    Output#{level => Level};
+keep_log_level_from_equivalent_output1(Output, _) ->
+    Output.
 
 -spec apply_log_levels_from_env(log_config(), rabbit_env:context()) ->
     log_config().
diff --git a/deps/rabbit/test/logging_SUITE.erl b/deps/rabbit/test/logging_SUITE.erl
index 5229d25c37..e5c865bed8 100644
--- a/deps/rabbit/test/logging_SUITE.erl
+++ b/deps/rabbit/test/logging_SUITE.erl
@@ -29,6 +29,7 @@
          logging_to_exchange_works/1,
          setting_log_levels_in_env_works/1,
          setting_log_levels_in_config_works/1,
+         setting_log_levels_in_config_with_output_overridden_in_env_works/1,
          format_messages_as_json_works/1]).
 
 all() ->
@@ -39,6 +40,7 @@ all() ->
      logging_to_exchange_works,
      setting_log_levels_in_env_works,
      setting_log_levels_in_config_works,
+     setting_log_levels_in_config_with_output_overridden_in_env_works,
      format_messages_as_json_works].
 
 init_per_suite(Config) ->
@@ -435,6 +437,55 @@ setting_log_levels_in_config_works(Config) ->
                         #{domain => ?RMQLOG_DOMAIN_GLOBAL})),
     ok.
 
+setting_log_levels_in_config_with_output_overridden_in_env_works(Config) ->
+    #{var_origins := Origins0} = Context0 = default_context(Config),
+    Context = Context0#{main_log_file => "-",
+                        var_origins => Origins0#{
+                                         main_log_file => environment}},
+    ok = application:set_env(
+           rabbit, log,
+           [{console, [{level, debug}]}],
+           [{persistent, true}]),
+    rabbit_prelaunch_logging:clear_config_run_number(),
+    rabbit_prelaunch_logging:setup(Context),
+
+    Handlers = logger:get_handler_config(),
+
+    StddevHandler = get_handler_by_id(Handlers, rmq_1_stdout),
+    ?assertNotEqual(undefined, StddevHandler),
+    ?assertMatch(
+       #{level := debug,
+         module := rabbit_logger_std_h,
+         filter_default := log,
+         filters := [{progress_reports, {_, log}},
+                     {rmqlog_filter, {_, #{global := debug,
+                                           upgrade := none}}}],
+         formatter := {rabbit_logger_text_fmt, _},
+         config := #{type := standard_io}},
+       StddevHandler),
+
+    UpgradeFileHandler = get_handler_by_id(Handlers, rmq_1_file_1),
+    UpgradeFile = upgrade_log_file_in_context(Context),
+    ?assertNotEqual(undefined, UpgradeFileHandler),
+    ?assertMatch(
+       #{level := info,
+         module := rabbit_logger_std_h,
+         filter_default := stop,
+         filters := [{rmqlog_filter, {_, #{upgrade := info}}}],
+         formatter := {rabbit_logger_text_fmt, _},
+         config := #{type := file,
+                     file := UpgradeFile}},
+       UpgradeFileHandler),
+
+    ?assert(ping_log(rmq_1_stdout, debug, Config)),
+    ?assert(ping_log(rmq_1_stdout, debug,
+                     #{domain => ?RMQLOG_DOMAIN_GLOBAL}, Config)),
+    ?assert(ping_log(rmq_1_stdout, debug,
+                     #{domain => ['3rd_party']}, Config)),
+    ?assertNot(ping_log(rmq_1_stdout, debug,
+                        #{domain => ?RMQLOG_DOMAIN_UPGRADE}, Config)),
+    ok.
+
 format_messages_as_json_works(Config) ->
     #{var_origins := Origins0} = Context0 = default_context(Config),
     Context = Context0#{log_levels => #{json => true},