From a192bd9e919b70e1e3aa06f6ce4f52870b28a79a Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Wed, 14 Nov 2012 14:44:02 -0500 Subject: [PATCH] Replace mochiglobal with a public ETS table This also removes the dependency on the syntax_tools and compiler applications, which interfered with lager's startup/shutdown order. --- Makefile | 2 +- include/lager.hrl | 2 +- src/lager.app.src | 4 +- src/lager.erl | 29 +++++---- src/lager_app.erl | 6 +- src/lager_config.erl | 61 +++++++++++++++++++ src/lager_console_backend.erl | 10 ++-- src/lager_crash_log.erl | 2 - src/lager_file_backend.erl | 10 ++-- src/lager_handler_watcher.erl | 4 -- src/lager_mochiglobal.erl | 107 ---------------------------------- src/lager_sup.erl | 2 + test/lager_test_backend.erl | 20 +++---- 13 files changed, 98 insertions(+), 161 deletions(-) create mode 100644 src/lager_config.erl delete mode 100644 src/lager_mochiglobal.erl diff --git a/Makefile b/Makefile index 17e14f5..80231c5 100644 --- a/Makefile +++ b/Makefile @@ -24,7 +24,7 @@ docs: ./rebar doc APPS = kernel stdlib sasl erts ssl tools os_mon runtime_tools crypto inets \ - xmerl webtool snmp public_key mnesia eunit syntax_tools compiler + xmerl webtool snmp public_key mnesia eunit COMBO_PLT = $(HOME)/.riak_combo_dialyzer_plt check_plt: compile diff --git a/include/lager.hrl b/include/lager.hrl index 8b9bcc7..a8e5946 100644 --- a/include/lager.hrl +++ b/include/lager.hrl @@ -55,7 +55,7 @@ end). -define(SHOULD_LOG(Level), - lager_util:level_to_num(Level) =< element(1, lager_mochiglobal:get(loglevel, {?LOG_NONE, []}))). + lager_util:level_to_num(Level) =< element(1, lager_config:get(loglevel, {?LOG_NONE, []}))). -define(NOTIFY(Level, Pid, Format, Args), gen_event:notify(lager_event, {log, lager_msg:new(io_lib:format(Format, Args), diff --git a/src/lager.app.src b/src/lager.app.src index 63798f9..08ce465 100644 --- a/src/lager.app.src +++ b/src/lager.app.src @@ -7,9 +7,7 @@ {modules, []}, {applications, [ kernel, - stdlib, - compiler, - syntax_tools + stdlib ]}, {registered, []}, {mod, {lager_app, []}}, diff --git a/src/lager.erl b/src/lager.erl index 01cb5bc..d20bdb2 100644 --- a/src/lager.erl +++ b/src/lager.erl @@ -19,7 +19,6 @@ -module(lager). -include("lager.hrl"). --include_lib("eunit/include/eunit.hrl"). %% API -export([start/0, @@ -60,7 +59,7 @@ dispatch_log(Severity, Metadata, Format, Args, Size) when is_atom(Severity)-> %% lager isn't running {error, lager_not_running}; Pid -> - {LevelThreshold,TraceFilters} = lager_mochiglobal:get(loglevel,{?LOG_NONE,[]}), + {LevelThreshold,TraceFilters} = lager_config:get(loglevel,{?LOG_NONE,[]}), SeverityAsInt=lager_util:level_to_num(Severity), Destinations = case TraceFilters of [] -> []; @@ -115,10 +114,10 @@ trace_file(File, Filter, Level) -> case Res of {ok, _} -> %% install the trace. - {MinLevel, Traces} = lager_mochiglobal:get(loglevel), + {MinLevel, Traces} = lager_config:get(loglevel), case lists:member(Trace, Traces) of false -> - lager_mochiglobal:put(loglevel, {MinLevel, [Trace|Traces]}); + lager_config:set(loglevel, {MinLevel, [Trace|Traces]}); _ -> ok end, @@ -137,10 +136,10 @@ trace_console(Filter, Level) -> Trace0 = {Filter, Level, lager_console_backend}, case lager_util:validate_trace(Trace0) of {ok, Trace} -> - {MinLevel, Traces} = lager_mochiglobal:get(loglevel), + {MinLevel, Traces} = lager_config:get(loglevel), case lists:member(Trace, Traces) of false -> - lager_mochiglobal:put(loglevel, {MinLevel, [Trace|Traces]}); + lager_config:set(loglevel, {MinLevel, [Trace|Traces]}); _ -> ok end, {ok, Trace}; @@ -149,9 +148,9 @@ trace_console(Filter, Level) -> end. stop_trace({_Filter, _Level, Target} = Trace) -> - {MinLevel, Traces} = lager_mochiglobal:get(loglevel), + {MinLevel, Traces} = lager_config:get(loglevel), NewTraces = lists:delete(Trace, Traces), - lager_mochiglobal:put(loglevel, {MinLevel, NewTraces}), + lager_config:set(loglevel, {MinLevel, NewTraces}), case get_loglevel(Target) of none -> %% check no other traces point here @@ -167,8 +166,8 @@ stop_trace({_Filter, _Level, Target} = Trace) -> ok. clear_all_traces() -> - {MinLevel, _Traces} = lager_mochiglobal:get(loglevel), - lager_mochiglobal:put(loglevel, {MinLevel, []}), + {MinLevel, _Traces} = lager_config:get(loglevel), + lager_config:set(loglevel, {MinLevel, []}), lists:foreach(fun(Handler) -> case get_loglevel(Handler) of none -> @@ -196,7 +195,7 @@ status() -> [begin io_lib:format("Tracing messages matching ~p at level ~p to ~p\n", [Filter, lager_util:num_to_level(Level), Destination]) - end || {Filter, Level, Destination} <- element(2, lager_mochiglobal:get(loglevel))]], + end || {Filter, Level, Destination} <- element(2, lager_config:get(loglevel))]], io:put_chars(Status). %% @doc Set the loglevel for a particular backend. @@ -204,8 +203,8 @@ set_loglevel(Handler, Level) when is_atom(Level) -> Reply = gen_event:call(lager_event, Handler, {set_loglevel, Level}, infinity), %% recalculate min log level MinLog = minimum_loglevel(get_loglevels()), - {_, Traces} = lager_mochiglobal:get(loglevel), - lager_mochiglobal:put(loglevel, {MinLog, Traces}), + {_, Traces} = lager_config:get(loglevel), + lager_config:set(loglevel, {MinLog, Traces}), Reply. %% @doc Set the loglevel for a particular backend that has multiple identifiers @@ -215,8 +214,8 @@ set_loglevel(Handler, Ident, Level) when is_atom(Level) -> Reply = gen_event:call(lager_event, {Handler, Ident}, {set_loglevel, Level}, infinity), %% recalculate min log level MinLog = minimum_loglevel(get_loglevels()), - {_, Traces} = lager_mochiglobal:get(loglevel), - lager_mochiglobal:put(loglevel, {MinLog, Traces}), + {_, Traces} = lager_config:get(loglevel), + lager_config:set(loglevel, {MinLog, Traces}), Reply. %% @doc Get the loglevel for a particular backend. In the case that the backend diff --git a/src/lager_app.erl b/src/lager_app.erl index 932a213..559c72d 100644 --- a/src/lager_app.erl +++ b/src/lager_app.erl @@ -33,8 +33,6 @@ start() -> application:start(lager). start(_StartType, _StartArgs) -> - %% until lager is completely started, allow all messages to go through - lager_mochiglobal:put(loglevel, {?DEBUG, []}), {ok, Pid} = lager_sup:start_link(), Handlers = case application:get_env(lager, handlers) of undefined -> @@ -51,8 +49,8 @@ start(_StartType, _StartArgs) -> %% mask the messages we have no use for MinLog = lager:minimum_loglevel(lager:get_loglevels()), - {_, Traces} = lager_mochiglobal:get(loglevel), - lager_mochiglobal:put(loglevel, {MinLog, Traces}), + {_, Traces} = lager_config:get(loglevel), + lager_config:set(loglevel, {MinLog, Traces}), SavedHandlers = case application:get_env(lager, error_logger_redirect) of {ok, false} -> diff --git a/src/lager_config.erl b/src/lager_config.erl new file mode 100644 index 0000000..6a8e42a --- /dev/null +++ b/src/lager_config.erl @@ -0,0 +1,61 @@ +%% Copyright (c) 2011-2012 Basho Technologies, Inc. All Rights Reserved. +%% +%% This file is provided to you under the Apache License, +%% Version 2.0 (the "License"); you may not use this file +%% except in compliance with the License. You may obtain +%% a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, +%% software distributed under the License is distributed on an +%% "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +%% KIND, either express or implied. See the License for the +%% specific language governing permissions and limitations +%% under the License. + +%% @doc Helper functions for working with lager's runtime config + +-module(lager_config). + +-include("lager.hrl"). + +-export([new/0, get/1, get/2, set/2]). + +-define(TBL, lager_config). + +new() -> + %% set up the ETS configuration table + try ets:new(?TBL, [named_table, public, set, {keypos, 1}, {read_concurrency, true}]) of + _Result -> + ok + catch + error:badarg -> + ?INT_LOG(warning, "Table ~p already exists", [?TBL]) + end, + %% use insert_new here so that if we're in an appup we don't mess anything up + %% + %% until lager is completely started, allow all messages to go through + ets:insert_new(?TBL, {loglevel, {?DEBUG, []}}), + ok. + + +get(Key) -> + case ets:lookup(?TBL, Key) of + [] -> + undefined; + [{Key, Res}] -> + Res + end. + +get(Key, Default) -> + case ?MODULE:get(Key) of + undefined -> + Default; + Res -> + Res + end. + +set(Key, Value) -> + ets:insert(?TBL, {Key, Value}). + diff --git a/src/lager_console_backend.erl b/src/lager_console_backend.erl index f0765eb..9c94136 100644 --- a/src/lager_console_backend.erl +++ b/src/lager_console_backend.erl @@ -116,8 +116,6 @@ console_log_test_() -> application:load(lager), application:set_env(lager, handlers, []), application:set_env(lager, error_logger_redirect, false), - application:start(compiler), - application:start(syntax_tools), application:start(lager), whereis(user) end, @@ -135,7 +133,7 @@ console_log_test_() -> unregister(user), register(user, Pid), erlang:group_leader(Pid, whereis(lager_event)), - lager_mochiglobal:put(loglevel, {?INFO, []}), + lager_config:set(loglevel, {?INFO, []}), lager:log(info, self(), "Test message"), receive {io_request, From, ReplyAs, {put_chars, unicode, Msg}} -> @@ -154,7 +152,7 @@ console_log_test_() -> register(user, Pid), erlang:group_leader(Pid, whereis(lager_event)), gen_event:add_handler(lager_event, lager_console_backend, [info, true]), - lager_mochiglobal:put(loglevel, {?INFO, []}), + lager_config:set(loglevel, {?INFO, []}), lager:info("Test message"), lager:info("Test message"), PidStr = pid_to_list(self()), @@ -174,7 +172,7 @@ console_log_test_() -> register(user, Pid), gen_event:add_handler(lager_event, lager_console_backend, info), erlang:group_leader(Pid, whereis(lager_event)), - lager_mochiglobal:put(loglevel, {?INFO, []}), + lager_config:set(loglevel, {?INFO, []}), lager:debug("Test message"), receive {io_request, From, ReplyAs, {put_chars, unicode, _Msg}} -> @@ -202,7 +200,7 @@ console_log_test_() -> unregister(user), register(user, Pid), gen_event:add_handler(lager_event, lager_console_backend, info), - lager_mochiglobal:put(loglevel, {?INFO, []}), + lager_config:set(loglevel, {?INFO, []}), erlang:group_leader(Pid, whereis(lager_event)), lager:debug("Test message"), receive diff --git a/src/lager_crash_log.erl b/src/lager_crash_log.erl index 59b3b20..7c2f90e 100644 --- a/src/lager_crash_log.erl +++ b/src/lager_crash_log.erl @@ -241,8 +241,6 @@ filesystem_test_() -> application:set_env(lager, handlers, [{lager_test_backend, info}]), application:set_env(lager, error_logger_redirect, true), application:unset_env(lager, crash_log), - application:start(compiler), - application:start(syntax_tools), application:start(lager), timer:sleep(100), lager_test_backend:flush() diff --git a/src/lager_file_backend.erl b/src/lager_file_backend.erl index 6a5048f..b3436b6 100644 --- a/src/lager_file_backend.erl +++ b/src/lager_file_backend.erl @@ -241,8 +241,6 @@ filesystem_test_() -> application:load(lager), application:set_env(lager, handlers, [{lager_test_backend, info}]), application:set_env(lager, error_logger_redirect, false), - application:start(compiler), - application:start(syntax_tools), application:start(lager) end, fun(_) -> @@ -352,8 +350,8 @@ filesystem_test_() -> {"test.log", critical}), lager:error("Test message"), ?assertEqual({ok, <<>>}, file:read_file("test.log")), - {Level, _} = lager_mochiglobal:get(loglevel), - lager_mochiglobal:put(loglevel, {Level, [{[{module, + {Level, _} = lager_config:get(loglevel), + lager_config:set(loglevel, {Level, [{[{module, ?MODULE}], ?DEBUG, {lager_file_backend, "test.log"}}]}), lager:error("Test message"), @@ -370,8 +368,8 @@ filesystem_test_() -> {ok, Bin1} = file:read_file("test.log"), ?assertMatch([_, _, "[critical]", _, "Test message\n"], re:split(Bin1, " ", [{return, list}, {parts, 5}])), ok = file:delete("test.log"), - {Level, _} = lager_mochiglobal:get(loglevel), - lager_mochiglobal:put(loglevel, {Level, [{[{module, + {Level, _} = lager_config:get(loglevel), + lager_config:set(loglevel, {Level, [{[{module, ?MODULE}], ?DEBUG, {lager_file_backend, "test.log"}}]}), lager:critical("Test message"), diff --git a/src/lager_handler_watcher.erl b/src/lager_handler_watcher.erl index 4838b0a..a330a7d 100644 --- a/src/lager_handler_watcher.erl +++ b/src/lager_handler_watcher.erl @@ -112,8 +112,6 @@ reinstall_on_initial_failure_test_() -> application:set_env(lager, handlers, [{lager_test_backend, info}, {lager_crash_backend, [from_now(2), undefined]}]), application:set_env(lager, error_logger_redirect, false), application:unset_env(lager, crash_log), - application:start(compiler), - application:start(syntax_tools), application:start(lager), try ?assertEqual(1, lager_test_backend:count()), @@ -140,8 +138,6 @@ reinstall_on_runtime_failure_test_() -> application:set_env(lager, handlers, [{lager_test_backend, info}, {lager_crash_backend, [undefined, from_now(5)]}]), application:set_env(lager, error_logger_redirect, false), application:unset_env(lager, crash_log), - application:start(compiler), - application:start(syntax_tools), application:start(lager), try ?assertEqual(0, lager_test_backend:count()), diff --git a/src/lager_mochiglobal.erl b/src/lager_mochiglobal.erl deleted file mode 100644 index 5a92ce8..0000000 --- a/src/lager_mochiglobal.erl +++ /dev/null @@ -1,107 +0,0 @@ -%% @author Bob Ippolito -%% @copyright 2010 Mochi Media, Inc. -%% @doc Abuse module constant pools as a "read-only shared heap" (since erts 5.6) -%% [1]. --module(lager_mochiglobal). --author("Bob Ippolito "). --export([get/1, get/2, put/2, delete/1]). - --spec get(atom()) -> any() | undefined. -%% @equiv get(K, undefined) -get(K) -> - get(K, undefined). - --spec get(atom(), T) -> any() | T. -%% @doc Get the term for K or return Default. -get(K, Default) -> - get(K, Default, key_to_module(K)). - -get(_K, Default, Mod) -> - try Mod:term() - catch error:undef -> - Default - end. - --spec put(atom(), any()) -> ok. -%% @doc Store term V at K, replaces an existing term if present. -put(K, V) -> - put(K, V, key_to_module(K)). - -put(_K, V, Mod) -> - Bin = compile(Mod, V), - code:purge(Mod), - {module, Mod} = code:load_binary(Mod, atom_to_list(Mod) ++ ".erl", Bin), - ok. - --spec delete(atom()) -> boolean(). -%% @doc Delete term stored at K, no-op if non-existent. -delete(K) -> - delete(K, key_to_module(K)). - -delete(_K, Mod) -> - code:purge(Mod), - code:delete(Mod). - --spec key_to_module(atom()) -> atom(). -key_to_module(K) -> - list_to_atom("lager_mochiglobal:" ++ atom_to_list(K)). - --spec compile(atom(), any()) -> binary(). -compile(Module, T) -> - {ok, Module, Bin} = compile:forms(forms(Module, T), - [verbose, report_errors]), - Bin. - --spec forms(atom(), any()) -> [erl_syntax:syntaxTree()]. -forms(Module, T) -> - [erl_syntax:revert(X) || X <- term_to_abstract(Module, term, T)]. - --spec term_to_abstract(atom(), atom(), any()) -> [erl_syntax:syntaxTree()]. -term_to_abstract(Module, Getter, T) -> - [%% -module(Module). - erl_syntax:attribute( - erl_syntax:atom(module), - [erl_syntax:atom(Module)]), - %% -export([Getter/0]). - erl_syntax:attribute( - erl_syntax:atom(export), - [erl_syntax:list( - [erl_syntax:arity_qualifier( - erl_syntax:atom(Getter), - erl_syntax:integer(0))])]), - %% Getter() -> T. - erl_syntax:function( - erl_syntax:atom(Getter), - [erl_syntax:clause([], none, [erl_syntax:abstract(T)])])]. - -%% -%% Tests -%% --ifdef(TEST). --include_lib("eunit/include/eunit.hrl"). -get_put_delete_test() -> - K = '$$test$$mochiglobal', - delete(K), - ?assertEqual( - bar, - get(K, bar)), - try - ?MODULE:put(K, baz), - ?assertEqual( - baz, - get(K, bar)), - ?MODULE:put(K, wibble), - ?assertEqual( - wibble, - ?MODULE:get(K)) - after - delete(K) - end, - ?assertEqual( - bar, - get(K, bar)), - ?assertEqual( - undefined, - ?MODULE:get(K)), - ok. --endif. diff --git a/src/lager_sup.erl b/src/lager_sup.erl index 40496ae..51238ae 100644 --- a/src/lager_sup.erl +++ b/src/lager_sup.erl @@ -32,6 +32,8 @@ start_link() -> supervisor:start_link({local, ?MODULE}, ?MODULE, []). init([]) -> + %% set up the config, is safe even during relups + lager_config:new(), Children = [ {lager, {gen_event, start_link, [{local, lager_event}]}, permanent, 5000, worker, [dynamic]}, diff --git a/test/lager_test_backend.erl b/test/lager_test_backend.erl index c761b4b..4c612f1 100644 --- a/test/lager_test_backend.erl +++ b/test/lager_test_backend.erl @@ -170,7 +170,7 @@ lager_test_() -> lager:debug("this message will be ignored"), ?assertEqual(0, count()), ?assertEqual(0, count_ignored()), - lager_mochiglobal:put(loglevel, {?DEBUG, []}), + lager_config:set(loglevel, {?DEBUG, []}), lager:debug("this message should be ignored"), ?assertEqual(0, count()), ?assertEqual(1, count_ignored()), @@ -184,10 +184,10 @@ lager_test_() -> }, {"tracing works", fun() -> - lager_mochiglobal:put(loglevel, {?ERROR, []}), + lager_config:set(loglevel, {?ERROR, []}), ok = lager:info("hello world"), ?assertEqual(0, count()), - lager_mochiglobal:put(loglevel, {?ERROR, [{[{module, + lager_config:set(loglevel, {?ERROR, [{[{module, ?MODULE}], ?DEBUG, ?MODULE}]}), ok = lager:info("hello world"), ?assertEqual(1, count()), @@ -196,14 +196,14 @@ lager_test_() -> }, {"tracing works with custom attributes", fun() -> - lager_mochiglobal:put(loglevel, {?ERROR, []}), + lager_config:set(loglevel, {?ERROR, []}), lager:info([{requestid, 6}], "hello world"), ?assertEqual(0, count()), - lager_mochiglobal:put(loglevel, {?ERROR, + lager_config:set(loglevel, {?ERROR, [{[{requestid, 6}], ?DEBUG, ?MODULE}]}), lager:info([{requestid, 6}, {foo, bar}], "hello world"), ?assertEqual(1, count()), - lager_mochiglobal:put(loglevel, {?ERROR, + lager_config:set(loglevel, {?ERROR, [{[{requestid, '*'}], ?DEBUG, ?MODULE}]}), lager:info([{requestid, 6}], "hello world"), ?assertEqual(2, count()), @@ -212,7 +212,7 @@ lager_test_() -> }, {"tracing honors loglevel", fun() -> - lager_mochiglobal:put(loglevel, {?ERROR, [{[{module, + lager_config:set(loglevel, {?ERROR, [{[{module, ?MODULE}], ?NOTICE, ?MODULE}]}), ok = lager:info("hello world"), ?assertEqual(0, count()), @@ -229,8 +229,6 @@ setup() -> application:load(lager), application:set_env(lager, handlers, [{?MODULE, info}]), application:set_env(lager, error_logger_redirect, false), - application:start(compiler), - application:start(syntax_tools), application:start(lager), gen_event:call(lager_event, ?MODULE, flush). @@ -284,8 +282,6 @@ error_logger_redirect_crash_test_() -> application:load(lager), application:set_env(lager, error_logger_redirect, true), application:set_env(lager, handlers, [{?MODULE, error}]), - application:start(compiler), - application:start(syntax_tools), application:start(lager), crash:start() end, @@ -714,7 +710,7 @@ error_logger_redirect_test_() -> ?assertEqual(1, count()), ?assertEqual(0, count_ignored()), lager:set_loglevel(?MODULE, error), - lager_mochiglobal:put(loglevel, {?DEBUG, []}), + lager_config:set(loglevel, {?DEBUG, []}), sync_error_logger:info_report([hello, world]), _ = gen_event:which_handlers(error_logger), ?assertEqual(1, count()),