Commit 90a221f5 authored by Guilherme Andrade's avatar Guilherme Andrade

Fix overly verbose `logger` messages on OTP 21+

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`:
* https://github.com/erlang-lager/lager/issues/492
* https://github.com/erlang-lager/lager/pull/488
parent d5b1a83e
......@@ -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
......
......@@ -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"}.
......
......@@ -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) ->
......
......@@ -31,6 +31,7 @@
%% API Function Exports
%% ------------------------------------------------------------------
-export([on_app_start/0]).
-export([set_loglevel/1]). -ignore_xref({set_loglevel,1}).
%% ------------------------------------------------------------------
......@@ -55,6 +56,13 @@
-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)).
......@@ -62,16 +70,30 @@
%% 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
......@@ -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;
......@@ -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).
......
......@@ -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, []).
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment