Skip to content

Commit

Permalink
Fix overly verbose logger messages on OTP 21+
Browse files Browse the repository at this point in the history
The way `error_logger` messages were forwarded to `logger` raised their level
for undetermined reasons (e.g. an info message became a notice.)

In order to fix this, invoke `logger` functions directly when they're available,
*unless* `lager` is also present and has sabotaged the default `logger` handler,
which would condemn our own `logger` messages into oblivion; in such cases, keep
using `error_logger` as `lager` will display those messages properly in any case.

Some information on the conflict between `logger` and `lager`:
* erlang-lager/lager#492
* erlang-lager/lager#488
  • Loading branch information
g-andrade committed Aug 11, 2019
1 parent d5b1a83 commit 90a221f
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- half-baked and unwarranted support for `file://`-prefixed URLs
### Fixed
- case-sensitive search of `.mmdb` file within tarball
- overly verbose `logger` messages on OTP 21+

## [1.6.2] - 2019-03-16
### Fixed
Expand Down
4 changes: 3 additions & 1 deletion rebar.config
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
{platform_define, "^[2-9]", 'POST_OTP_18'},
{platform_define, "^1[89]", 'SSL_OLD_CLIENT_OPTIONS'},
{platform_define, "^20", 'SSL_OLD_CLIENT_OPTIONS'},
{platform_define, "^21.[0-2]", 'SSL_OLD_CLIENT_OPTIONS'}
{platform_define, "^21.[0-2]", 'SSL_OLD_CLIENT_OPTIONS'},
{platform_define, "^1", 'NO_LOGGER'},
{platform_define, "^20", 'NO_LOGGER'}
]}.

{minimum_otp_vsn, "18"}.
Expand Down
10 changes: 8 additions & 2 deletions src/locus_app.erl
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,15 @@
%% application Function Definitions
%% ------------------------------------------------------------------

-spec start(term(), list()) -> {ok, pid()}.
-spec start(term(), list()) -> {ok, pid()} | {error, term()}.
start(_StartType, _StartArgs) ->
locus_sup:start_link().
case locus_sup:start_link() of
{ok, Pid} ->
locus_logger:on_app_start(),
{ok, Pid};
{error, Reason} ->
{error, Reason}
end.

-spec stop(term()) -> ok.
stop(_State) ->
Expand Down
90 changes: 77 additions & 13 deletions src/locus_logger.erl
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
%% API Function Exports
%% ------------------------------------------------------------------

-export([on_app_start/0]).
-export([set_loglevel/1]). -ignore_xref({set_loglevel,1}).

%% ------------------------------------------------------------------
Expand All @@ -55,23 +56,44 @@
-define(warning, 2).
-define(error, 3).

-define(is_loglevel(V),
((V) =:= debug orelse
(V) =:= info orelse
(V) =:= warning orelse
(V) =:= error orelse
(V) =:= none)).

-define(case_match(Value, Pattern, Then, OrElse),
(case (Value) of (Pattern) -> (Then); _ -> (OrElse) end)).

%% ------------------------------------------------------------------
%% API Function Definitions
%% ------------------------------------------------------------------

-spec on_app_start() -> ok.
%% @private
-ifdef(NO_LOGGER).
on_app_start() -> ok.
-else.
on_app_start() ->
CurrentLevel = application:get_env(locus, log_level, undefined),
_ = logger:set_application_level(locus, CurrentLevel),
ok.
-endif.

%% @doc Changes the logging verbosity in runtime
%%
%% `Level' must be either `debug', `info', `warning', `error' or `none'.
-spec set_loglevel(debug | info | warning | error | none) -> ok.
set_loglevel(Level) when Level =:= debug;
Level =:= info;
Level =:= warning;
Level =:= error;
Level =:= none ->
-ifdef(NO_LOGGER).
set_loglevel(Level) when ?is_loglevel(Level) ->
application:set_env(locus, log_level, Level).
-else.
set_loglevel(Level) when ?is_loglevel(Level) ->
application:set_env(locus, log_level, Level),
_ = logger:set_application_level(locus, Level),
ok.
-endif.

%% ------------------------------------------------------------------
%% locus_event_subscriber Function Definitions
Expand Down Expand Up @@ -159,11 +181,11 @@ report(MinWeight, DatabaseId, {load_attempt_finished, Source, {ok, Version}}) ->
ok
end;
report(MinWeight, DatabaseId, {load_attempt_finished, Source, {error, Error}}) ->
LogFun = ?case_match(Source, {cache,_}, warning_msg, error_msg),
LogFun = ?case_match(Source, {cache,_}, fun log_warning/2, fun log_error/2),
if MinWeight =< ?debug ->
log(LogFun, "~p database failed to load from ~p: ~p", [DatabaseId, Source, Error]);
LogFun("~p database failed to load from ~p: ~p", [DatabaseId, Source, Error]);
MinWeight =< ?error ->
log(LogFun, "~p database failed to load (~p): ~p", [DatabaseId, resumed_source(Source), Error]);
LogFun("~p database failed to load (~p): ~p", [DatabaseId, resumed_source(Source), Error]);
true ->
ok
end;
Expand All @@ -184,16 +206,58 @@ report(MinWeight, DatabaseId, {cache_attempt_finished, Filename, {error, Error}}
ok
end.

-ifdef(NO_LOGGER).
log_info(Fmt, Args) ->
log(info_msg, Fmt, Args).
log_to_error_logger(info_msg, Fmt, Args).

%log_warning(Fmt, Args) ->
% log(warning_msg, Fmt, Args).
log_warning(Fmt, Args) ->
log_to_error_logger(warning_msg, Fmt, Args).

log_error(Fmt, Args) ->
log(error_msg, Fmt, Args).
log_to_error_logger(error_msg, Fmt, Args).

-else.
log_info(Fmt, Args) ->
case use_error_logger() of
true -> log_to_error_logger(info_msg, Fmt, Args);
false -> log_to_logger(info, Fmt, Args)
end.

log_warning(Fmt, Args) ->
case use_error_logger() of
true -> log_to_error_logger(warning_msg, Fmt, Args);
false -> log_to_logger(warning, Fmt, Args)
end.

log_error(Fmt, Args) ->
case use_error_logger() of
true -> log_to_error_logger(error_msg, Fmt, Args);
false -> log_to_logger(error, Fmt, Args)
end.

log_to_logger(Fun, Fmt, Args) ->
FullFmt = "[locus] " ++ Fmt,
logger:Fun(FullFmt, Args).

% `lager' and `logger' don`t play nice with each other (as of Jun 2019)
% * https://github.com/erlang-lager/lager/issues/492
% * https://github.com/erlang-lager/lager/pull/488
use_error_logger() ->
has_lager() andalso not has_usable_logger().

% Taken from: https://github.com/ferd/cth_readable/pull/23
has_lager() ->
% Module is present
erlang:function_exported(logger, module_info, 0).

% Taken from: https://github.com/ferd/cth_readable/pull/23
has_usable_logger() ->
%% The config is set (lager didn't remove it)
erlang:function_exported(logger, get_handler_config, 1) andalso
logger:get_handler_config(default) =/= {error, {not_found, default}}.
-endif.

log(Fun, Fmt, Args) ->
log_to_error_logger(Fun, Fmt, Args) ->
FullFmt = "[locus] " ++ Fmt ++ "~n",
error_logger:(Fun)(FullFmt, Args).

Expand Down
2 changes: 1 addition & 1 deletion src/locus_sup.erl
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
%% API Function Definitions
%% ------------------------------------------------------------------

-spec start_link() -> {ok, pid()}.
-spec start_link() -> {ok, pid()} | {error, term()}.
start_link() ->
supervisor:start_link({local, ?SERVER}, ?MODULE, []).

Expand Down

0 comments on commit 90a221f

Please sign in to comment.