From 180e09fd0199a6bb66cf3a31bc6178b7f7b4a53e Mon Sep 17 00:00:00 2001 From: Andrew Thompson Date: Wed, 26 Sep 2012 14:02:32 -0400 Subject: [PATCH] Use an opaque type with a module for accessors rather than a record I didn't want the lager_log_message record being used across application boundaries, this will insulate other applications from any changes to the message type's internal structure. --- include/lager.hrl | 19 +++----- src/lager.erl | 7 +-- src/lager_console_backend.erl | 2 +- src/lager_default_formatter.erl | 82 +++++++++++++++++++-------------- src/lager_file_backend.erl | 2 +- src/lager_msg.erl | 51 ++++++++++++++++++++ src/lager_util.erl | 17 +++---- test/lager_test_backend.erl | 7 ++- 8 files changed, 122 insertions(+), 65 deletions(-) create mode 100644 src/lager_msg.erl diff --git a/include/lager.hrl b/include/lager.hrl index 50b74ba..509db51 100644 --- a/include/lager.hrl +++ b/include/lager.hrl @@ -14,13 +14,6 @@ %% specific language governing permissions and limitations %% under the License. --record(lager_log_message,{ - destinations, - metadata, - severity_as_int, - timestamp, - message - }). -define(DEFAULT_TRUNCATION, 4096). @@ -65,12 +58,12 @@ lager_util:level_to_num(Level) =< element(1, lager_mochiglobal:get(loglevel, {?LOG_NONE, []}))). -define(NOTIFY(Level, Pid, Format, Args), - gen_event:notify(lager_event,#lager_log_message{destinations=[], - message=io_lib:format(Format,Args), - metadata=[{pid,Pid},{line,?LINE},{file,?FILE},{module,?MODULE}], - timestamp=lager_util:format_time(), - severity_as_int=lager_util:level_to_num(Level) - })). + gen_event:notify(lager_event,lager_msg:new(io_lib:format(Format, Args), + lager_util:format_time(), + Level, + [{pid,Pid},{line,?LINE},{file,?FILE},{module,?MODULE}], + []) + )). %% FOR INTERNAL USE ONLY %% internal non-blocking logging call diff --git a/src/lager.erl b/src/lager.erl index 4bb4170..e6060e5 100644 --- a/src/lager.erl +++ b/src/lager.erl @@ -74,11 +74,8 @@ dispatch_log(Severity, Metadata, Format, Args, Size) when is_atom(Severity)-> A when is_list(A) ->safe_format_chop(Format,Args,Size); _ -> Format end, - gen_event:sync_notify(Pid, #lager_log_message{destinations=Destinations, - metadata=Metadata, - severity_as_int=SeverityAsInt, - timestamp=Timestamp, - message=Msg}); + gen_event:sync_notify(Pid, lager_msg:new(Msg, Timestamp, + Severity, Metadata, Destinations)); _ -> ok end diff --git a/src/lager_console_backend.erl b/src/lager_console_backend.erl index c55697a..76a8736 100644 --- a/src/lager_console_backend.erl +++ b/src/lager_console_backend.erl @@ -67,7 +67,7 @@ handle_call(_Request, State) -> {ok, ok, State}. %% @private -handle_event(#lager_log_message{}=Message, +handle_event(Message, #state{level=L,formatter=Formatter,format_config=FormatConfig} = State) -> case lager_util:is_loggable(Message, L, ?MODULE) of true -> diff --git a/src/lager_default_formatter.erl b/src/lager_default_formatter.erl index 73d0ba1..09b177e 100644 --- a/src/lager_default_formatter.erl +++ b/src/lager_default_formatter.erl @@ -53,10 +53,10 @@ %% %% `[{pid, ["My pid is ", pid], "Unknown Pid"}]' -> if pid is in the metada print "My pid is ?.?.?", otherwise print "Unknown Pid" %% @end --spec format(#lager_log_message{},list()) -> any(). -format(#lager_log_message{}=Msg,[]) -> +-spec format(lager_msg:lager_msg(),list()) -> any(). +format(Msg,[]) -> format(Msg, [{eol, "\n"}]); -format(#lager_log_message{}=Msg,[{eol, EOL}]) -> +format(Msg,[{eol, EOL}]) -> format(Msg, [date, " ", time, " [", severity, "] ", {pid, ""}, @@ -70,23 +70,30 @@ format(Message,Config) -> [ output(V,Message) || V <- Config ]. --spec output(term(),#lager_log_message{}) -> iolist(). -output(message,#lager_log_message{message=M}) -> M; -output(date,#lager_log_message{timestamp={D,_T}}) -> D; -output(time,#lager_log_message{timestamp={_D,T}}) -> T; -output(severity,#lager_log_message{severity_as_int=S}) -> - atom_to_list(lager_util:num_to_level(S)); -output(Prop,#lager_log_message{metadata=Metadata}) when is_atom(Prop) -> +-spec output(term(),lager_msg:lager_msg()) -> iolist(). +output(message,Msg) -> lager_msg:message(Msg); +output(date,Msg) -> + {D, _T} = lager_msg:timestamp(Msg), + D; +output(time,Msg) -> + {_D, T} = lager_msg:timestamp(Msg), + T; +output(severity,Msg) -> + atom_to_list(lager_msg:severity(Msg)); +output(Prop,Msg) when is_atom(Prop) -> + Metadata = lager_msg:metadata(Msg), make_printable(get_metadata(Prop,Metadata,<<"Undefined">>)); -output({Prop,Default},#lager_log_message{metadata=Metadata}=L) when is_atom(Prop) -> - make_printable(get_metadata(Prop,Metadata,output(Default,L))); -output({Prop, Present, Absent}, #lager_log_message{metadata=Metadata}=L) when is_atom(Prop) -> +output({Prop,Default},Msg) when is_atom(Prop) -> + Metadata = lager_msg:metadata(Msg), + make_printable(get_metadata(Prop,Metadata,output(Default,Msg))); +output({Prop, Present, Absent}, Msg) when is_atom(Prop) -> %% sort of like a poor man's ternary operator + Metadata = lager_msg:metadata(Msg), case get_metadata(Prop, Metadata) of undefined -> - [ output(V, L) || V <- Absent]; + [ output(V, Msg) || V <- Absent]; _ -> - [ output(V, L) || V <- Present] + [ output(V, Msg) || V <- Present] end; output(Other,_) -> make_printable(Other). @@ -111,44 +118,49 @@ get_metadata(Key, Metadata, Default) -> basic_test_() -> [{"Default formatting test", ?_assertEqual(iolist_to_binary([<<"Day Time [error] ">>, pid_to_list(self()), <<" Message\n">>]), - iolist_to_binary(format(#lager_log_message{timestamp={"Day","Time"}, - message="Message", - severity_as_int=lager_util:level_to_num(error), - metadata=[{pid,self()}]}, + iolist_to_binary(format(lager_msg:new("Message", + {"Day", "Time"}, + error, + [{pid, self()}], + []), []))) }, {"Basic Formatting", ?_assertEqual(<<"Simplist Format">>, - iolist_to_binary(format(#lager_log_message{timestamp={"Day","Time"}, - message="Message", - severity_as_int=lager_util:level_to_num(error), - metadata=[{pid,self()}]}, + iolist_to_binary(format(lager_msg:new("Message", + {"Day", "Time"}, + error, + [{pid, self()}], + []), ["Simplist Format"]))) }, {"Default equivalent formatting test", ?_assertEqual(iolist_to_binary([<<"Day Time [error] ">>, pid_to_list(self()), <<" Message\n">>]), - iolist_to_binary(format(#lager_log_message{timestamp={"Day","Time"}, - message="Message", - severity_as_int=lager_util:level_to_num(error), - metadata=[{pid,self()}]}, + iolist_to_binary(format(lager_msg:new("Message", + {"Day", "Time"}, + error, + [{pid, self()}], + []), [date, " ", time," [",severity,"] ",pid, " ", message, "\n"] ))) }, {"Non existant metadata can default to string", ?_assertEqual(iolist_to_binary([<<"Day Time [error] Fallback Message\n">>]), - iolist_to_binary(format(#lager_log_message{timestamp={"Day","Time"}, - message="Message", - severity_as_int=lager_util:level_to_num(error), - metadata=[]}, + iolist_to_binary(format(lager_msg:new("Message", + {"Day", "Time"}, + error, + [{pid, self()}], + []), [date, " ", time," [",severity,"] ",{does_not_exist,"Fallback"}, " ", message, "\n"] ))) }, {"Non existant metadata can default to other metadata", ?_assertEqual(iolist_to_binary([<<"Day Time [error] Fallback Message\n">>]), - iolist_to_binary(format(#lager_log_message{timestamp={"Day","Time"}, - message="Message", - severity_as_int=lager_util:level_to_num(error), - metadata=[{pid,"Fallback"}]}, + iolist_to_binary(format(lager_msg:new("Message", + {"Day", "Time"}, + error, + [{pid, "Fallback"}], + []), [date, " ", time," [",severity,"] ",{does_not_exist,pid}, " ", message, "\n"] ))) } diff --git a/src/lager_file_backend.erl b/src/lager_file_backend.erl index df56b20..d5107be 100644 --- a/src/lager_file_backend.erl +++ b/src/lager_file_backend.erl @@ -95,7 +95,7 @@ handle_event(Message, #state{name=Name, level=L,formatter=Formatter,formatter_config=FormatConfig} = State) -> case lager_util:is_loggable(Message,L,{lager_file_backend, Name}) of true -> - {ok,write(State, Message#lager_log_message.severity_as_int, Formatter:format(Message,FormatConfig)) }; + {ok,write(State, lager_msg:severity_as_int(Message), Formatter:format(Message,FormatConfig)) }; false -> {ok, State} end; diff --git a/src/lager_msg.erl b/src/lager_msg.erl new file mode 100644 index 0000000..437dd08 --- /dev/null +++ b/src/lager_msg.erl @@ -0,0 +1,51 @@ +-module(lager_msg). + +-export([new/5]). +-export([message/1]). +-export([timestamp/1]). +-export([severity/1]). +-export([severity_as_int/1]). +-export([metadata/1]). +-export([destinations/1]). + +-record(lager_msg,{ + destinations :: list(), + metadata :: [tuple()], + severity :: lager:log_level(), + timestamp :: {string(), string()}, + message :: list() + }). + +-opaque lager_msg() :: #lager_msg{}. +-export_type([lager_msg/0]). + +-spec new(list(), {string(), string()}, atom(), [tuple()], list()) -> lager_msg(). +new(Msg, Timestamp, Severity, Metadata, Destinations) -> + #lager_msg{message=Msg, timestamp=Timestamp, severity=Severity, + metadata=Metadata, destinations=Destinations}. + +-spec message(lager_msg()) -> list(). +message(Msg) -> + Msg#lager_msg.message. + +-spec timestamp(lager_msg()) -> {string(), string()}. +timestamp(Msg) -> + Msg#lager_msg.timestamp. + +-spec severity(lager_msg()) -> lager:log_level(). +severity(Msg) -> + Msg#lager_msg.severity. + +-spec severity_as_int(lager_msg()) -> lager:log_level_number(). +severity_as_int(Msg) -> + lager_util:level_to_num(Msg#lager_msg.severity). + +-spec metadata(lager_msg()) -> [tuple()]. +metadata(Msg) -> + Msg#lager_msg.metadata. + +-spec destinations(lager_msg()) -> list(). +destinations(Msg) -> + Msg#lager_msg.destinations. + + diff --git a/src/lager_util.erl b/src/lager_util.erl index 06fcfb3..a4747f3 100644 --- a/src/lager_util.erl +++ b/src/lager_util.erl @@ -329,9 +329,10 @@ check_trace_iter(Attrs, [{Key, Match}|T]) -> false end. --spec is_loggable(#lager_log_message{},integer(),term()) -> boolean(). -is_loggable(#lager_log_message{severity_as_int=Severity,destinations=Destinations},SeverityThreshold,MyName) -> - Severity =< SeverityThreshold orelse lists:member(MyName, Destinations). +-spec is_loggable(lager_msg:lager_msg(),integer(),term()) -> boolean(). +is_loggable(Msg ,SeverityThreshold,MyName) -> + lager_msg:severity_as_int(Msg) =< SeverityThreshold orelse + lists:member(MyName, lager_msg:destinations(Msg)). -ifdef(TEST). @@ -462,11 +463,11 @@ check_trace_test() -> is_loggable_test_() -> [ - {"Loggable by severity only", ?_assert(is_loggable(#lager_log_message{severity_as_int=1,destinations=[]},2,me))}, - {"Not loggable by severity only", ?_assertNot(is_loggable(#lager_log_message{severity_as_int=2,destinations=[]},1,me))}, - {"Loggable by severity with destination", ?_assert(is_loggable(#lager_log_message{severity_as_int=1,destinations=[you]},2,me))}, - {"Not loggable by severity with destination", ?_assertNot(is_loggable(#lager_log_message{severity_as_int=2,destinations=[you]},1,me))}, - {"Loggable by destination overriding severity", ?_assert(is_loggable(#lager_log_message{severity_as_int=2,destinations=[me]},1,me))} + {"Loggable by severity only", ?_assert(is_loggable(lager_msg:new("",{"",""}, alert, [], []),2,me))}, + {"Not loggable by severity only", ?_assertNot(is_loggable(lager_msg:new("",{"",""}, critical, [], []),1,me))}, + {"Loggable by severity with destination", ?_assert(is_loggable(lager_msg:new("",{"",""}, alert, [], [you]),2,me))}, + {"Not loggable by severity with destination", ?_assertNot(is_loggable(lager_msg:new("",{"",""}, critical, [], [you]),1,me))}, + {"Loggable by destination overriding severity", ?_assert(is_loggable(lager_msg:new("",{"",""}, critical, [], [me]),1,me))} ]. -endif. diff --git a/test/lager_test_backend.erl b/test/lager_test_backend.erl index 968186c..b317d81 100644 --- a/test/lager_test_backend.erl +++ b/test/lager_test_backend.erl @@ -54,10 +54,13 @@ handle_call({set_loglevel, Level}, State) -> handle_call(_Request, State) -> {ok, ok, State}. -handle_event(#lager_log_message{severity_as_int=Level,timestamp=Time, message=Message, metadata=Metadata}=Msg, +handle_event(Msg, #state{level=LogLevel,buffer=Buffer,ignored=Ignored} = State) -> case lager_util:is_loggable(Msg, LogLevel, ?MODULE) of - true -> {ok, State#state{buffer=Buffer ++ [{Level, Time, Message, Metadata}]}}; + true -> {ok, State#state{buffer=Buffer ++ + [{lager_msg:severity_as_int(Msg), + lager_msg:timestamp(Msg), + lager_msg:message(Msg), lager_msg:metadata(Msg)}]}}; _ -> {ok, State#state{ignored=Ignored ++ [ignored]}} end; handle_event(_Event, State) ->