Skip to content

Commit

Permalink
Migrate error_logger to logger
Browse files Browse the repository at this point in the history
It has been a very long time since OTP made `logger` available and
deprecated `error_logger`, so it's about time we migrate to it.

Note too that we're choosing to report structured logging, that is,
maps, as `worker_pool` is a library, it should allow application code to
choose how to filter and format log messages.
  • Loading branch information
NelsonVides committed Sep 21, 2024
1 parent b34a0db commit 9418fb0
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 24 deletions.
6 changes: 3 additions & 3 deletions src/wpool.erl
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
-type overrun_handler() :: {Module :: module(), Fun :: atom()}.
%% The module and function to call when a task is <i>overrun</i>
%%
%% The default value for this setting is `{error_logger, warning_report}'. The function must be of
%% The default value for this setting is `{logger, warning}'. The function must be of
%% arity 1, and it will be called as`Module:Fun(Args)' where `Args' is a proplist with the following
%% reported values:
%% <ul>
Expand Down Expand Up @@ -173,7 +173,7 @@
{worker_opt, [worker_opt()]} |
{strategy, supervisor_strategy()} |
{worker_shutdown, worker_shutdown()} |
{overrun_handler, overrun_handler()} |
{overrun_handler, overrun_handler() | [overrun_handler()]} |
{overrun_warning, overrun_warning()} |
{max_overrun_warnings, max_overrun_warnings()} |
{pool_sup_intensity, pool_sup_intensity()} |
Expand All @@ -192,7 +192,7 @@
worker_opt => [worker_opt()],
strategy => supervisor_strategy(),
worker_shutdown => worker_shutdown(),
overrun_handler => overrun_handler(),
overrun_handler => overrun_handler() | [overrun_handler()],
overrun_warning => overrun_warning(),
max_overrun_warnings => max_overrun_warnings(),
pool_sup_intensity => pool_sup_intensity(),
Expand Down
14 changes: 10 additions & 4 deletions src/wpool_pool.erl
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
%%% `t:wpool:pool_sup_intensity()' options respectively.
-module(wpool_pool).

-include_lib("kernel/include/logger.hrl").

-behaviour(supervisor).

%% API
Expand Down Expand Up @@ -329,7 +331,7 @@ time_checker_name(Name) ->
init({Name, Options}) ->
Size = maps:get(workers, Options, 100),
QueueType = maps:get(queue_type, Options),
OverrunHandler = maps:get(overrun_handler, Options, {error_logger, warning_report}),
OverrunHandler = maps:get(overrun_handler, Options, {logger, warning}),
SupShutdown = maps:get(pool_sup_shutdown, Options, brutal_kill),
TimeCheckerName = time_checker_name(Name),
QueueManagerName = queue_manager_name(Name),
Expand Down Expand Up @@ -522,15 +524,19 @@ find_wpool(Name) ->
%% @doc We use this function not to report an error if for some reason we've
%% lost the record on the persistent_term table. This SHOULDN'T be called too much.
build_wpool(Name) ->
error_logger:warning_msg("Building a #wpool record for ~p. Something must have failed.",
[Name]),
logger:warning(#{what => "Building a #wpool record. Something must have failed.",
pool => Name},
?LOCATION),
try supervisor:count_children(process_sup_name(Name)) of
Children ->
Size = proplists:get_value(active, Children, 0),
store_wpool(Name, Size, #{})
catch
_:Error ->
error_logger:warning_msg("Wpool ~p not found: ~p", [Name, Error]),
logger:warning(#{what => "Wpool not found",
pool => Name,
reason => Error},
?LOCATION),
undefined
end.

Expand Down
7 changes: 6 additions & 1 deletion src/wpool_process_callbacks.erl
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
-module(wpool_process_callbacks).

-include_lib("kernel/include/logger.hrl").

-behaviour(gen_event).

%% The callbacks are called in an extremely dynamic from call/3.
Expand Down Expand Up @@ -71,7 +73,10 @@ call(Module, Event, Args) ->
end
catch
E:R ->
error_logger:warning_msg("Could not call callback module, error:~p, reason:~p", [E, R])
logger:warning(#{what => "Could not call callback module",
error => E,
reason => R},
?LOCATION)
end.

ensure_loaded(Module) ->
Expand Down
8 changes: 6 additions & 2 deletions src/wpool_process_sup.erl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
%%% @doc This is the supervisor that supervises the `gen_server' workers specifically.
-module(wpool_process_sup).

-include_lib("kernel/include/logger.hrl").

-behaviour(supervisor).

%% API
Expand Down Expand Up @@ -62,6 +64,8 @@ add_initial_callback(EventManager, Module) ->
ok ->
ok;
Other ->
error_logger:warning_msg("The callback module:~p could not be loaded, reason:~p",
[Module, Other])
logger:warning(#{what => "The callback module could not be loaded",
module => Module,
reason => Other},
?LOCATION)
end.
7 changes: 6 additions & 1 deletion src/wpool_sup.erl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
%%% @private
-module(wpool_sup).

-include_lib("kernel/include/logger.hrl").

-behaviour(supervisor).

-export([start_link/0, init/1]).
Expand All @@ -37,7 +39,10 @@ start_pool(Name, Options) ->
stop_pool(Name) ->
case erlang:whereis(Name) of
undefined ->
error_logger:warning_msg("Couldn't stop ~p. It was not running", [Name]),
logger:warning(#{what => "Could not stop pool",
reason => "It was not running",
pool => Name},
?LOCATION),
ok;
Pid ->
ok = supervisor:terminate_child(?MODULE, Pid)
Expand Down
2 changes: 1 addition & 1 deletion src/wpool_utils.erl
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ add_defaults(Opts) when is_list(Opts) ->

defaults() ->
#{max_overrun_warnings => infinity,
overrun_handler => {error_logger, warning_report},
overrun_handler => {logger, warning},
overrun_warning => infinity,
queue_type => fifo,
worker_opt => [],
Expand Down
35 changes: 24 additions & 11 deletions src/wpool_worker.erl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
%%% It is a module that implements a very simple RPC-like interface.
-module(wpool_worker).

-include_lib("kernel/include/logger.hrl").

-behaviour(gen_server).

%% api
Expand Down Expand Up @@ -70,12 +72,15 @@ handle_cast({M, F, A}, State) ->
_ ->
{noreply, State, hibernate}
catch
_:Error:Stacktrace ->
log_error(M, F, A, Error, Stacktrace),
Class:Reason:Stacktrace ->
log_error(M, F, A, Class, Reason, Stacktrace),
{noreply, State, hibernate}
end;
handle_cast(Cast, State) ->
error_logger:error_msg("Invalid cast:~p", [Cast]),
logger:error(#{what => "Invalid cast",
cast => Cast,
worker => self()},
?LOCATION),
{noreply, State, hibernate}.

%% @private
Expand All @@ -86,17 +91,25 @@ handle_call({M, F, A}, _From, State) ->
R ->
{reply, {ok, R}, State, hibernate}
catch
_:Error:Stacktrace ->
log_error(M, F, A, Error, Stacktrace),
{reply, {error, Error}, State, hibernate}
Class:Reason:Stacktrace ->
log_error(M, F, A, Class, Reason, Stacktrace),
{reply, {error, Reason}, State, hibernate}
end;
handle_call(Call, _From, State) ->
error_logger:error_msg("Invalid call:~p", [Call]),
handle_call(Call, From, State) ->
logger:error(#{what => "Invalid call",
call => Call,
from => From,
worker => self()},
?LOCATION),
{reply, {error, invalid_request}, State, hibernate}.

%%%===================================================================
%%% not exported functions
%%%===================================================================
log_error(M, F, A, Error, Stacktrace) ->
error_logger:error_msg("Error on ~p:~p~p >> ~p Backtrace ~p",
[M, F, A, Error, Stacktrace]).
log_error(M, F, A, Class, Reason, Stacktrace) ->
logger:error(#{what => "Reason on ~p:~p~p >> ~p Backtrace ~p",
mfa => {M, F, A},
class => Class,
reason => Reason,
stacktrace => Stacktrace},
?LOCATION).
2 changes: 1 addition & 1 deletion test/wpool_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ stats(_Config) ->
PoolPid = Get(supervisor, InitStats),
Options = Get(options, InitStats),
infinity = Get(overrun_warning, Options),
{error_logger, warning_report} = Get(overrun_handler, Options),
{logger, warning} = Get(overrun_handler, Options),
10 = Get(workers, Options),
10 = Get(size, InitStats),
1 = Get(next_worker, InitStats),
Expand Down

0 comments on commit 9418fb0

Please sign in to comment.