Moved append operation inside init/1

in handlers' wrappers. Added comments,
fixed various small things.

5 files changed, 83 insertions, 46 deletions

diff --git a/src/rabbit.erl b/src/rabbit.erl
index c82e0a1360..dd1cec2fb6 100644
--- a/src/rabbit.erl
+++ b/src/rabbit.erl
@@ -27,7 +27,7 @@
 
 -behaviour(application).
 
--export([start/0, stop/0, stop_and_halt/0, status/0, rotate_logs/0, rotate_logs/1]).
+-export([start/0, stop/0, stop_and_halt/0, status/0, rotate_logs/1]).
 
 -export([start/2, stop/1]).
 
@@ -39,7 +39,6 @@
 
 -include("rabbit_framing.hrl").
 -include("rabbit.hrl").
--include_lib("kernel/include/file.hrl").
 
 -define(APPS, [os_mon, mnesia, rabbit]).
 
@@ -50,8 +49,7 @@
 -spec(start/0 :: () -> 'ok').
 -spec(stop/0 :: () -> 'ok').
 -spec(stop_and_halt/0 :: () -> 'ok').
--spec(rotate_logs/0 :: () -> 'ok').
--spec(rotate_logs/1 :: name() -> 'ok' | {'error', 'cannot_append_logfile'}).
+-spec(rotate_logs/1 :: name() -> 'ok' | {'error', any()}).
 -spec(status/0 :: () ->
              [{running_applications, [{atom(), string(), string()}]} |
               {nodes, [node()]} |
@@ -88,16 +86,11 @@ status() ->
     [{running_applications, application:which_applications()}] ++
         rabbit_mnesia:status().
 
-rotate_logs() ->
-    ok = rotate_logs(error_log_location(wrapper), [], main_log),
-    ok = rotate_logs(sasl_log_location(), [], sasl_log).
-
-rotate_logs(Suffix) ->
-    LSuffix = binary_to_list(Suffix),
-    case rotate_logs(error_log_location(wrapper), LSuffix, main_log) of
-        ok    -> rotate_logs(sasl_log_location(), LSuffix, sasl_log);
-        Error -> Error
-    end.
+rotate_logs(BinarySuffix) ->
+    Suffix = binary_to_list(BinarySuffix),
+    log_rotation_result(
+      rotate_logs(error_log_location(wrapper), Suffix, rabbit_error_logger_file_h),
+      rotate_logs(sasl_log_location(), Suffix, rabbit_sasl_report_file_h)).
 
 %%--------------------------------------------------------------------
 
@@ -177,10 +170,14 @@ start(normal, []) ->
                 ok = error_logger:add_report_handler(
                        rabbit_error_logger, [DefaultVHost]),
                 ok = start_builtin_amq_applications(),
-                ok = swap_handler(error_logger_file_h, rabbit_error_logger_file_h,
-                                  error_log_location(default)),
-                ok = swap_handler(sasl_report_file_h, rabbit_sasl_report_file_h,
-                                  sasl_log_location())
+                %% Swap default handlers with rabbit wrappers
+                %% to simplify swapping of log handlers later
+                ok = rotate_logs(error_log_location(default), "",
+                                 error_logger_file_h,
+                                 rabbit_error_logger_file_h),
+                ok = rotate_logs(sasl_log_location(), "",
+                                 sasl_report_file_h,
+                                 rabbit_sasl_report_file_h)
         end},
        {"TCP listeners",
         fun () ->
@@ -284,37 +281,25 @@ sasl_log_location() ->
         _                  -> undefined
     end.
 
-rotate_logs(File, Suffix, main_log) ->
-    rotate_logs(File, Suffix, rabbit_error_logger_file_h, rabbit_error_logger_file_h);
-rotate_logs(File, Suffix, sasl_log) ->
-    rotate_logs(File, Suffix, rabbit_sasl_report_file_h, rabbit_sasl_report_file_h).
+rotate_logs(File, Suffix, Handler) ->
+    rotate_logs(File, Suffix, Handler, Handler).
 
 rotate_logs(File, Suffix, OldHandler, NewHandler) ->
     case File of
         undefined -> ok;
         tty       -> ok;
-        _         -> case append_to_log_file(File, Suffix) of
-                         omit  -> swap_handler(OldHandler, NewHandler, File);
-                         ok    -> swap_handler(OldHandler, NewHandler, File);
-                         Error -> Error
-                     end
+        _         -> gen_event:swap_handler(
+                       error_logger,
+                       {OldHandler, swap},
+                       {NewHandler, {File, Suffix}})
     end.
 
-swap_handler(Old, New, File) ->
-    gen_event:swap_handler(error_logger, {Old, swap}, {New, File}).
-
-append_to_log_file(File, Suffix) ->
-    case file:read_file_info(File) of
-        {ok, FInfo} -> append_file(File, FInfo#file_info.size, Suffix);
-        {error, _}  -> ok
-    end.
-
-append_file(_, 0, _) ->
-    omit;
-append_file(_, _, []) ->
-    omit;
-append_file(File, _, Suffix) ->
-    case file:read_file(File) of
-        {ok, Data} -> file:write_file([File, Suffix], Data, [append]);
-        {error, _} -> {error, cannot_append_logfile}
-    end.
+log_rotation_result({error, MainLogError}, {error, SaslLogError}) ->
+    {error, {{cannot_rotate_main_logs, MainLogError},
+	     {cannot_rotate_sasl_logs, SaslLogError}}};
+log_rotation_result({error, MainLogError}, ok) ->
+    {error, {cannot_rotate_main_logs, MainLogError}};
+log_rotation_result(ok, {error, SaslLogError}) ->
+    {error, {cannot_rotate_sasl_logs, SaslLogError}};
+log_rotation_result(ok, ok) ->
+    ok.
diff --git a/src/rabbit_control.erl b/src/rabbit_control.erl
index 28761eeb3a..bc588279b4 100644
--- a/src/rabbit_control.erl
+++ b/src/rabbit_control.erl
@@ -132,7 +132,7 @@ action(status, Node, []) ->
 
 action(rotate_logs, Node, []) ->
     io:format("Reopening logs for node ~p ...", [Node]),
-    call(Node, {rabbit, rotate_logs, []});
+    call(Node, {rabbit, rotate_logs, [""]});
 action(rotate_logs, Node, Args = [Suffix]) ->
     io:format("Rotating logs to files with suffix ~p ...", [Suffix]),
     call(Node, {rabbit, rotate_logs, Args});
diff --git a/src/rabbit_error_logger_file_h.erl b/src/rabbit_error_logger_file_h.erl
index 4fadf46fda..20c6b77800 100644
--- a/src/rabbit_error_logger_file_h.erl
+++ b/src/rabbit_error_logger_file_h.erl
@@ -29,6 +29,20 @@
 
 -export([init/1, handle_event/2, handle_call/2, handle_info/2, terminate/2, code_change/3]).
 
+%% rabbit_error_logger_file_h is a wrapper around error_logger_file_h
+%% module because the original's init/1 does not match properly
+%% with the result of closing the old handler when swapping handlers.
+%% The first init/1 additionally allows for simple log rotation
+%% when suffix is not ""
+
+%% Used only when swapping handlers in log rotation
+init({{File, Suffix}, []}) ->
+    case rabbit_misc:append_file(File, Suffix) of
+        ok -> error_logger_file_h:init(File);
+        Error -> Error
+    end;
+%% Used only when swapping handlers without performing
+%% log rotation 
 init({File, []}) ->
     error_logger_file_h:init(File);
 init({_File, _Type} = FileInfo) ->
diff --git a/src/rabbit_misc.erl b/src/rabbit_misc.erl
index 11ab0caf9f..9aeb16216b 100644
--- a/src/rabbit_misc.erl
+++ b/src/rabbit_misc.erl
@@ -26,6 +26,7 @@
 -module(rabbit_misc).
 -include("rabbit.hrl").
 -include("rabbit_framing.hrl").
+-include_lib("kernel/include/file.hrl").
 
 -export([method_record_type/1, polite_pause/0, polite_pause/1]).
 -export([die/1, frame_error/2, protocol_error/3, protocol_error/4]).
@@ -41,6 +42,7 @@
 -export([intersperse/2, upmap/2, map_in_order/2]).
 -export([guid/0, string_guid/1, binstring_guid/1]).
 -export([dirty_read_all/1, dirty_foreach_key/2, dirty_dump_log/1]).
+-export([append_file/2]).
 
 -import(mnesia).
 -import(lists).
@@ -92,6 +94,7 @@
 -spec(dirty_foreach_key/2 :: (fun ((any()) -> any()), atom()) ->
              'ok' | 'aborted').
 -spec(dirty_dump_log/1 :: (string()) -> 'ok' | {'error', any()}).
+-spec(append_file/2 :: (string(), string()) -> 'ok' | {'error', any()}).
 
 -endif.
 
@@ -333,3 +336,21 @@ dirty_dump_log1(LH, {K, Terms}) ->
 dirty_dump_log1(LH, {K, Terms, BadBytes}) ->
     io:format("Bad Chunk, ~p: ~p~n", [BadBytes, Terms]),
     dirty_dump_log1(LH, disk_log:chunk(LH, K)).
+
+
+append_file(File, Suffix) ->
+    case catch file:read_file_info(File) of
+        {ok, FInfo}     -> append_file(File, FInfo#file_info.size, Suffix);
+        {error, enoent} -> ok;
+        {error, Error}  -> {error, {cannot_read_logfile, Error}}
+    end.
+
+append_file(_, 0, _) ->
+    ok;
+append_file(_, _, "") ->
+    ok;
+append_file(File, _, Suffix) ->
+    case catch file:read_file(File) of
+        {ok, Data}     -> file:write_file([File, Suffix], Data, [append]);
+        {error, Error} -> {error, {cannot_append_logfile, Error}}
+    end.
diff --git a/src/rabbit_sasl_report_file_h.erl b/src/rabbit_sasl_report_file_h.erl
index 7bfa2ca172..eb5bf09178 100644
--- a/src/rabbit_sasl_report_file_h.erl
+++ b/src/rabbit_sasl_report_file_h.erl
@@ -29,6 +29,21 @@
 
 -export([init/1, handle_event/2, handle_call/2, handle_info/2, terminate/2, code_change/3]).
 
+%% rabbit_sasl_report_file_h is a wrapper around sasl_report_file_h
+%% module because the original's init/1 does not match properly
+%% with the result of closing the old handler when swapping handlers.
+%% The first init/1 additionally allows for simple log rotation
+%% when suffix is not ""
+
+%% Used only when swapping handlers and performing
+%% log rotation
+init({{File, Suffix}, []}) ->
+    case rabbit_misc:append_file(File, Suffix) of
+        ok -> sasl_report_file_h:init({File, sasl_error_logger_type()});
+        Error -> Error
+    end;
+%% Used only when swapping handlers without
+%% doing any log rotation
 init({File, []}) ->
     sasl_report_file_h:init({File, sasl_error_logger_type()});
 init({_File, _Type} = FileInfo) ->
@@ -51,6 +66,8 @@ terminate(Reason, State) ->
 code_change(OldVsn, State, Extra) ->
     sasl_report_file_h:code_change(OldVsn, State, Extra).
 
+%%----------------------------------------------------------------------
+
 sasl_error_logger_type() ->
     case application:get_env(sasl, errlog_type) of
         {ok, error}    -> error;