From b6289e914c67bd786da829503b144c039303bfd3 Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Tue, 14 Aug 2018 11:43:18 +0200 Subject: [logger] Refactor some logger internals --- lib/kernel/src/logger.erl | 7 +--- lib/kernel/src/logger_backend.erl | 2 +- lib/kernel/src/logger_config.erl | 23 ++--------- lib/kernel/src/logger_disk_log_h.erl | 5 +-- lib/kernel/src/logger_server.erl | 74 +++++++++++++++++------------------- lib/kernel/src/logger_std_h.erl | 5 +-- lib/kernel/test/logger_SUITE.erl | 2 +- 7 files changed, 44 insertions(+), 74 deletions(-) diff --git a/lib/kernel/src/logger.erl b/lib/kernel/src/logger.erl index 0020fe220b..7b1377b8ca 100644 --- a/lib/kernel/src/logger.erl +++ b/lib/kernel/src/logger.erl @@ -434,12 +434,7 @@ get_primary_config() -> HandlerId :: handler_id(), Config :: handler_config(). get_handler_config(HandlerId) -> - case logger_config:get(?LOGGER_TABLE,HandlerId) of - {ok,{_,Config}} -> - {ok,Config}; - Error -> - Error - end. + logger_config:get(?LOGGER_TABLE,HandlerId). -spec get_handler_config() -> [Config] when Config :: handler_config(). diff --git a/lib/kernel/src/logger_backend.erl b/lib/kernel/src/logger_backend.erl index 4d7bd6b2a0..432c671afd 100644 --- a/lib/kernel/src/logger_backend.erl +++ b/lib/kernel/src/logger_backend.erl @@ -41,7 +41,7 @@ log_allowed(Log, Tid) -> call_handlers(#{level:=Level}=Log,[Id|Handlers],Tid) -> case logger_config:get(Tid,Id,Level) of - {ok,{Module,Config}} -> + {ok,#{module:=Module}=Config} -> Filters = maps:get(filters,Config,[]), case apply_filters(Id,Log,Filters,Config) of stop -> diff --git a/lib/kernel/src/logger_config.erl b/lib/kernel/src/logger_config.erl index 55427dce5a..6bfe658552 100644 --- a/lib/kernel/src/logger_config.erl +++ b/lib/kernel/src/logger_config.erl @@ -22,8 +22,8 @@ -export([new/1,delete/2, exist/2, allow/2,allow/3, - get/2, get/3, get/1, - create/3, create/4, set/3, + get/2, get/3, + create/3, set/3, set_module_level/3,unset_module_level/2, get_module_level/1,cache_module_level/2, level_to_int/1]). @@ -64,27 +64,19 @@ get(Tid,What) -> case ets:lookup(Tid,table_key(What)) of [{_,_,Config}] -> {ok,Config}; - [{_,_,Config,Module}] -> - {ok,{Module,Config}}; [] -> {error,{not_found,What}} end. get(Tid,What,Level) -> - MS = [{{table_key(What),'$1','$2'}, % primary config + MS = [{{table_key(What),'$1','$2'}, [{'>=','$1',level_to_int(Level)}], - ['$2']}, - {{table_key(What),'$1','$2','$3'}, % handler config - [{'>=','$1',level_to_int(Level)}], - [{{'$3','$2'}}]}], + ['$2']}], case ets:select(Tid,MS) of [] -> error; [Data] -> {ok,Data} end. -create(Tid,What,Module,Config) -> - LevelInt = level_to_int(maps:get(level,Config)), - ets:insert(Tid,{table_key(What),LevelInt,Config,Module}). create(Tid,What,Config) -> LevelInt = level_to_int(maps:get(level,Config)), ets:insert(Tid,{table_key(What),LevelInt,Config}). @@ -129,13 +121,6 @@ cache_module_level(Tid,Module) -> ets:insert_new(Tid,{Module,{GlobalLevelInt,cached}}), ok. -get(Tid) -> - {ok,Primary} = get(Tid,primary), - HMS = [{{table_key('$1'),'_','$2','$3'},[],[{{'$1','$3','$2'}}]}], - Handlers = ets:select(Tid,HMS), - Modules = get_module_level(Tid), - {Primary,Handlers,Modules}. - level_to_int(none) -> ?LOG_NONE; level_to_int(emergency) -> ?EMERGENCY; level_to_int(alert) -> ?ALERT; diff --git a/lib/kernel/src/logger_disk_log_h.erl b/lib/kernel/src/logger_disk_log_h.erl index e56531c3cb..3c12c9e68b 100644 --- a/lib/kernel/src/logger_disk_log_h.erl +++ b/lib/kernel/src/logger_disk_log_h.erl @@ -294,7 +294,7 @@ init([Name, %% initial start ok end, - enter_loop(Config1, State1) + gen_server:enter_loop(?MODULE, [], State1) catch _:Error -> unregister(RegName), @@ -307,9 +307,6 @@ init([Name, proc_lib:init_ack(Error) end. -enter_loop(_Config,State) -> - gen_server:enter_loop(?MODULE,[],State). - %% This is the synchronous log event. handle_call({log, Bin}, _From, State) -> {Result,State1} = do_log(Bin, call, State), diff --git a/lib/kernel/src/logger_server.erl b/lib/kernel/src/logger_server.erl index 644fdd5af2..01ac1b4b50 100644 --- a/lib/kernel/src/logger_server.erl +++ b/lib/kernel/src/logger_server.erl @@ -146,7 +146,7 @@ init([]) -> filters=>?DEFAULT_HANDLER_FILTERS}), %% If this fails, then the node should crash {ok,SimpleConfig} = logger_simple_h:adding_handler(SimpleConfig0), - logger_config:create(Tid,simple,logger_simple_h,SimpleConfig), + logger_config:create(Tid,simple,SimpleConfig), {ok, #state{tid=Tid, async_req_queue = queue:new()}}. handle_call({add_handler,Id,Module,HConfig}, From, #state{tid=Tid}=State) -> @@ -165,11 +165,11 @@ handle_call({add_handler,Id,Module,HConfig}, From, #state{tid=Tid}=State) -> %% to find out if this is a valid handler case erlang:function_exported(Module, log, 2) of true -> - logger_config:create(Tid,Id,Module,HConfig1), - {ok,Config} = do_get_config(Tid,primary), + logger_config:create(Tid,Id,HConfig1), + {ok,Config} = logger_config:get(Tid,primary), Handlers = maps:get(handlers,Config,[]), - do_set_config(Tid,primary, - Config#{handlers=>[Id|Handlers]}); + logger_config:set(Tid,primary, + Config#{handlers=>[Id|Handlers]}); false -> {error,{invalid_handler, {function_not_exported, @@ -181,8 +181,8 @@ handle_call({add_handler,Id,Module,HConfig}, From, #state{tid=Tid}=State) -> end; handle_call({remove_handler,HandlerId}, From, #state{tid=Tid}=State) -> case logger_config:get(Tid,HandlerId) of - {ok,{Module,HConfig}} -> - {ok,Config} = do_get_config(Tid,primary), + {ok,#{module:=Module}=HConfig} -> + {ok,Config} = logger_config:get(Tid,primary), Handlers0 = maps:get(handlers,Config,[]), Handlers = lists:delete(HandlerId,Handlers0), call_h_async( @@ -191,7 +191,7 @@ handle_call({remove_handler,HandlerId}, From, #state{tid=Tid}=State) -> call_h(Module,removing_handler,[HConfig],ok) end, fun(_Res) -> - do_set_config(Tid,primary,Config#{handlers=>Handlers}), + logger_config:set(Tid,primary,Config#{handlers=>Handlers}), logger_config:delete(Tid,HandlerId), ok end,From,State); @@ -204,25 +204,35 @@ handle_call({add_filter,Id,Filter}, _From,#state{tid=Tid}=State) -> handle_call({remove_filter,Id,FilterId}, _From, #state{tid=Tid}=State) -> Reply = do_remove_filter(Tid,Id,FilterId), {reply,Reply,State}; -handle_call({update_config,Id,NewConfig}, From, #state{tid=Tid}=State) -> - case logger_config:get(Tid,Id) of - {ok,{_Module,OldConfig}} -> - Config = maps:merge(OldConfig,NewConfig), - handle_call({set_config,Id,Config}, From, State); - {ok,OldConfig} -> +handle_call({update_config,primary,NewConfig}, _From, #state{tid=Tid}=State) -> + {ok,OldConfig} = logger_config:get(Tid,primary), + Config = maps:merge(OldConfig,NewConfig), + {reply,logger_config:set(Tid,primary,Config),State}; +handle_call({update_config,HandlerId,NewConfig}, From, #state{tid=Tid}=State) -> + case logger_config:get(Tid,HandlerId) of + {ok,#{module:=Module}=OldConfig} -> Config = maps:merge(OldConfig,NewConfig), - {reply,do_set_config(Tid,Id,Config),State}; + call_h_async( + fun() -> + call_h(Module,changing_config,[OldConfig,Config], + {ok,Config}) + end, + fun({ok,Config1}) -> + logger_config:set(Tid,HandlerId,Config1); + (Error) -> + Error + end,From,State); Error -> {reply,Error,State} end; handle_call({set_config,primary,Config0}, _From, #state{tid=Tid}=State) -> Config = maps:merge(default_config(primary),Config0), {ok,#{handlers:=Handlers}} = logger_config:get(Tid,primary), - Reply = do_set_config(Tid,primary,Config#{handlers=>Handlers}), + Reply = logger_config:set(Tid,primary,Config#{handlers=>Handlers}), {reply,Reply,State}; handle_call({set_config,HandlerId,Config0}, From, #state{tid=Tid}=State) -> case logger_config:get(Tid,HandlerId) of - {ok,{Module,OldConfig}} -> + {ok,#{module:=Module}=OldConfig} -> Config = maps:merge(default_config(HandlerId,Module),Config0), call_h_async( fun() -> @@ -230,7 +240,7 @@ handle_call({set_config,HandlerId,Config0}, From, #state{tid=Tid}=State) -> {ok,Config}) end, fun({ok,Config1}) -> - do_set_config(Tid,HandlerId,Config1); + logger_config:set(Tid,HandlerId,Config1); (Error) -> Error end,From,State); @@ -241,12 +251,12 @@ handle_call({update_formatter_config,HandlerId,NewFConfig},_From, #state{tid=Tid}=State) -> Reply = case logger_config:get(Tid,HandlerId) of - {ok,{_Mod,#{formatter:={FMod,OldFConfig}}=Config}} -> + {ok,#{formatter:={FMod,OldFConfig}}=Config} -> try FConfig = maps:merge(OldFConfig,NewFConfig), check_formatter({FMod,FConfig}), - do_set_config(Tid,HandlerId, - Config#{formatter=>{FMod,FConfig}}) + logger_config:set(Tid,HandlerId, + Config#{formatter=>{FMod,FConfig}}) catch throw:Reason -> {error,Reason} end; _ -> @@ -316,26 +326,26 @@ call(Request) -> end. do_add_filter(Tid,Id,{FId,_} = Filter) -> - case do_get_config(Tid,Id) of + case logger_config:get(Tid,Id) of {ok,Config} -> Filters = maps:get(filters,Config,[]), case lists:keymember(FId,1,Filters) of true -> {error,{already_exist,FId}}; false -> - do_set_config(Tid,Id,Config#{filters=>[Filter|Filters]}) + logger_config:set(Tid,Id,Config#{filters=>[Filter|Filters]}) end; Error -> Error end. do_remove_filter(Tid,Id,FilterId) -> - case do_get_config(Tid,Id) of + case logger_config:get(Tid,Id) of {ok,Config} -> Filters0 = maps:get(filters,Config,[]), case lists:keytake(FilterId,1,Filters0) of {value,_,Filters} -> - do_set_config(Tid,Id,Config#{filters=>Filters}); + logger_config:set(Tid,Id,Config#{filters=>Filters}); false -> {error,{not_found,FilterId}} end; @@ -343,20 +353,6 @@ do_remove_filter(Tid,Id,FilterId) -> Error end. -do_get_config(Tid,Id) -> - case logger_config:get(Tid,Id) of - {ok,{_,Config}} -> - {ok,Config}; - {ok,Config} -> - {ok,Config}; - Error -> - Error - end. - -do_set_config(Tid,Id,Config) -> - logger_config:set(Tid,Id,Config), - ok. - default_config(primary) -> #{level=>notice, filters=>[], diff --git a/lib/kernel/src/logger_std_h.erl b/lib/kernel/src/logger_std_h.erl index 9a2a1443b3..b60e7cf587 100644 --- a/lib/kernel/src/logger_std_h.erl +++ b/lib/kernel/src/logger_std_h.erl @@ -259,7 +259,7 @@ init([Name, Config = #{config := HConfig}, mode_tab => ModeTab}}, proc_lib:init_ack({ok,self(),Config1}), gen_server:cast(self(), repeated_filesync), - enter_loop(Config1, State1) + gen_server:enter_loop(?MODULE, [], State1) catch _:Error -> unregister(RegName), @@ -290,9 +290,6 @@ do_init(Name, Type) -> Error end. -enter_loop(_Config,State) -> - gen_server:enter_loop(?MODULE,[],State). - %% This is the synchronous log event. handle_call({log, Bin}, _From, State) -> {Result,State1} = do_log(Bin, call, State), diff --git a/lib/kernel/test/logger_SUITE.erl b/lib/kernel/test/logger_SUITE.erl index 6bd9b20c35..824c95d6df 100644 --- a/lib/kernel/test/logger_SUITE.erl +++ b/lib/kernel/test/logger_SUITE.erl @@ -259,7 +259,7 @@ change_config(_Config) -> logger:get_primary_config(), 3 = maps:size(PC1), %% Check that internal 'handlers' field has not been changed - MS = [{{{?HANDLER_KEY,'$1'},'_','_','_'},[],['$1']}], + MS = [{{{?HANDLER_KEY,'$1'},'_','_'},[],['$1']}], HIds1 = lists:sort(ets:select(?LOGGER_TABLE,MS)), % dirty, internal data HIds2 = lists:sort(logger:get_handler_ids()), HIds1 = HIds2, -- cgit v1.2.1