Switch from lager to built in logger #99
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,21 @@ | ||
| %% -*-erlang-*- | ||
|
|
||
| [ | ||
| {kernel, [ | ||
| {logger_level, warning}, | ||
| {logger, | ||
| [{handler, default, logger_std_h, | ||
| #{formatter => {amoc_lager_logger_formatter, | ||
| #{report_cb => fun amoc_lager_logger_formatter:report_cb/1}}} | ||
| }, | ||
| {handler, logfile, logger_std_h, | ||
| #{config => #{file => "log/console.log"}, | ||
| formatter => {amoc_lager_logger_formatter, | ||
| #{report_cb => fun amoc_lager_logger_formatter:report_cb/1}}} | ||
| } | ||
| ]}]}, | ||
| {lager, [ | ||
| {lager_use_logger, true} | ||
| ]}, | ||
| {ssl, [{session_cb, amoc_always_null_ssl_session_cache}]} | ||
| ]. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| %% This module is copied from: https://github.com/erlang-lager/lager/blob/adt/lager_use_logger-option/src/lager_logger_formatter.erl | ||
| %% with amendments. Original comments left intact. | ||
| %% Fixed timestamp formatting, added catch all clause for report_cb. | ||
|
|
||
| -module(amoc_lager_logger_formatter). | ||
|
|
||
| %% convert logger formatter calls into lager formatter ones | ||
|
|
||
| -export([report_cb/1, format/2]).%, check_config/1]). | ||
|
|
||
| -spec report_cb(lager:report()) -> {io:format(),[term()]}. | ||
| report_cb(#{label := {gen_server, terminate}, name := Name, reason := Reason}) -> | ||
| Formatted = error_logger_lager_h:format_reason(Reason), | ||
| {"gen_server ~w terminated with reason: ~s", [Name, Formatted]}; | ||
| report_cb(#{label := {gen_fsm, terminate}, name := Name, state_name := StateName, reason := Reason}) -> | ||
| Formatted = error_logger_lager_h:format_reason(Reason), | ||
| {"gen_fsm ~w in state ~w terminated with reason: ~s", [Name, StateName, Formatted]}; | ||
| report_cb(#{label := {gen_event, terminate}, name := Name, handler := Handler, reason := Reason}) -> | ||
| Formatted = error_logger_lager_h:format_reason(Reason), | ||
| {"gen_event ~w installed in ~w terminated with reason: ~s", [Handler, Name, Formatted]}; | ||
| report_cb(#{label := {gen_statem, terminate}, name := Name, reason := Reason}) -> | ||
| Formatted = error_logger_lager_h:format_reason(Reason), | ||
| %% XXX I can't find the FSM statename in the error report, maybe it should be added | ||
| {"gen_statem ~w terminated with reason: ~s", [Name, Formatted]}; | ||
| report_cb(#{msg := {report, #{label := {Behaviour, no_handle_info}, mod := Mod, msg := Msg}}}) -> | ||
| {"undefined handle_info for ~p in ~s ~p", [Msg, Behaviour, Mod]}; | ||
| report_cb(#{label := {supervisor, progress}, report := Report}) -> | ||
| case application:get_env(lager, suppress_supervisor_start_stop, false) of | ||
| true -> | ||
| {"", []}; | ||
| false -> | ||
| {supervisor, Name} = lists:keyfind(supervisor, 1, Report), | ||
| {started, Started} = lists:keyfind(started, 1, Report), | ||
| case lists:keyfind(id, 1, Started) of | ||
| false -> | ||
| %% supervisor itself starting | ||
| {mfa, {Module, Function, Args}} = lists:keyfind(mfa, 1, Started), | ||
| {pid, Pid} = lists:keyfind(pid, 1, Started), | ||
| {"Supervisor ~w started as ~p at pid ~w", [Name, error_logger_lager_h:format_mfa({Module, Function, Args}), Pid]}; | ||
| {id, ChildID} -> | ||
| case lists:keyfind(pid, 1, Started) of | ||
| {pid, Pid} -> | ||
| {"Supervisor ~w started child ~p at pid ~w", [Name, ChildID, Pid]}; | ||
| false -> | ||
| %% children is a list of pids for some reason? and we only get the count | ||
| {nb_children, ChildCount} = lists:keyfind(nb_children, 1, Started), | ||
| {"Supervisor ~w started ~b children ~p", [Name, ChildCount, ChildID]} | ||
| end | ||
| end | ||
| end; | ||
| report_cb(#{label := {supervisor, _Error}, report := Report}) -> | ||
| {supervisor, Name} = lists:keyfind(supervisor, 1, Report), | ||
| {reason, Reason} = lists:keyfind(reason, 1, Report), | ||
| Formatted = error_logger_lager_h:format_reason(Reason), | ||
| {errorContext, ErrorContext} = lists:keyfind(errorContext, 1, Report), | ||
| {offender, Offender} = lists:keyfind(offender, 1, Report), | ||
| case lists:keyfind(mod, 1, Offender) of | ||
| {mod, _Mod} -> | ||
| {pid, Pid} = lists:keyfind(pid, 1, Offender), | ||
| %% this comes from supervisor_bridge | ||
| {"Supervisor ~w had ~p ~p with reason ~s", [Name, Pid, ErrorContext, Formatted]}; | ||
| false -> | ||
| {id, ChildID} = lists:keyfind(id, 1, Offender), | ||
| case lists:keyfind(pid, 1, Offender) of | ||
| {pid, Pid} -> | ||
| {"Supervisor ~w had ~p ~p ~p with reason ~s", [Name, ChildID, Pid, ErrorContext, Formatted]}; | ||
| false -> | ||
| {"Supervisor ~w had ~p ~p with reason ~s", [Name, ChildID, ErrorContext, Formatted]} | ||
| end | ||
| end; | ||
| report_cb(#{label := {application_controller, progress}, report := Report}) -> | ||
| case application:get_env(lager, suppress_application_start_stop, false) of | ||
| true -> {"", []}; | ||
| false -> | ||
| {application, Name} = lists:keyfind(application, 1, Report), | ||
| {started_at, Node} = lists:keyfind(started_at, 1, Report), | ||
| {"Application ~w started on node ~w", [Name, Node]} | ||
| end; | ||
| report_cb(#{label := {application_controller, exit}, report := Report}) -> | ||
| {exited, Reason} = lists:keyfind(exited, 1, Report), | ||
| case application:get_env(lager, suppress_application_start_stop) of | ||
| {ok, true} when Reason == stopped -> | ||
| {"", []}; | ||
| _ -> | ||
| {application, Name} = lists:keyfind(application, 1, Report), | ||
| Formatted = error_logger_lager_h:format_reason(Reason), | ||
| {"Application ~w exited with reason: ~s", [Name, Formatted]} | ||
| end; | ||
| report_cb(Other) -> | ||
| {"Unexpected logger report_cb: ~w", [Other]}. | ||
|
There was a problem hiding this comment. As far as I know this is our addition to the formater, right? I think it makes unexpected logs unreadable in some case. For instance I have this error (it's my fault in Erlang and docsh configuration) printed by pure logger: Which is readable and I know what to do about it. When the lager/logger integration is enabled the above log looks like the following: This is very cryptic and it's close to impossible to figure out what's wrong. Current master branch of Amoc produces the same output as my first example. I'm not sure if trying to unify lager and logger logs makes sense, at least for Amoc. |
||
| %% TODO handle proc_lib crash | ||
|
|
||
| -spec format(LogEvent, Config) -> unicode:chardata() when | ||
| LogEvent :: logger:log_event(), | ||
| Config :: term(). %% see: lib/kernel/src/logger_formatter.erl for config definition | ||
| format(#{msg := {report, _Report}, meta := Metadata} = Event, #{report_cb := Fun} = Config) when is_function(Fun, 1); is_function(Fun, 2) -> | ||
| format(Event#{meta => Metadata#{report_cb => Fun}}, maps:remove(report_cb, Config)); | ||
| format(#{level := _Level, msg := {report, Report}, meta := #{report_cb := Fun}} = Event, Config) when is_function(Fun, 1) -> | ||
| case Fun(Report) of | ||
| {"", []} -> ""; | ||
| {Format, Args} when is_list(Format), is_list(Args) -> | ||
| format(Event#{msg => {Format, Args}}, Config) | ||
| end; | ||
| format(#{level := Level, msg := {string, String}, meta := Metadata}, Config) -> | ||
| do_format(Level, String, Metadata, Config); | ||
| format(#{level := Level, msg := {FmtStr, FmtArgs}, meta := Metadata}, Config) -> | ||
| Msg = lager_format:format(FmtStr, FmtArgs, maps:get(max_size, Config, 1024)), | ||
| do_format(Level, Msg, Metadata, Config). | ||
|
|
||
| do_format(Level, Msg, Metadata, Config) -> | ||
| FormatModule = maps:get(formatter, Config, lager_default_formatter), | ||
| Timestamp = maps:get(time, Metadata), | ||
| MegaSecs = Timestamp div 1000000000000, | ||
| Secs = (Timestamp rem 1000000000000) div 1000000, | ||
| MicroSecs = (Timestamp rem 1000000000000) rem 1000000, | ||
| {Colors, End} = case maps:get(colors, Config, false) of | ||
| true -> | ||
| {application:get_env(lager, colors, []), "\e[0m"}; | ||
| false -> | ||
| {[], ""} | ||
| end, | ||
| [FormatModule:format(lager_msg:new(Msg, {MegaSecs, Secs, MicroSecs}, Level, convert_metadata(Metadata), []), maps:get(formatter_config, Config, []), Colors), End]. | ||
|
|
||
| convert_metadata(Metadata) -> | ||
| maps:fold(fun(mfa, {Module, Function, Arity}, Acc) -> | ||
| [{module, Module}, {function, Function}, {arity, Arity}|Acc]; | ||
| (K, V, Acc) -> | ||
| [{K, V}|Acc] | ||
| end, [], Metadata). | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we contribute back the timestamp fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or all the changes? I think we'll need this formatter in other places like
amoc-arsenal-xmppand possiblyMongooseIM. It's better to use one file instead of coping it to every repo.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole thing is quite controversial. Maybe I focused on the wrong thing which is the seamless combination of lager and logger.
Unfortunately the integration of the two by the lager author Andrew Thompson is left not quite finished. It seems to work, but it is in a feature branch of lager, not promoted to a release (as the author announced at his presentation this spring in SF - https://codesync.global/media/erlang-logging-for-the-21st-otp-andrew-thompson/).
Given the state of affairs I wouldn't recommend including lager_logger integration in Mongoose yet, let's wait for the next release of Lager).
Should we include it in Amoc at all? Well perhaps as a way of testing it, for example performance and be prepared for the final version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on this issue erlang-lager/lager#519 I think the author may need some external help to get this integration moving. I think we can try contributing the fixes and see if they get accepted.