Browse files

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.
  • Loading branch information...
1 parent 12a784b commit 180e09fd0199a6bb66cf3a31bc6178b7f7b4a53e @Vagabond Vagabond committed Sep 26, 2012
View
19 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
View
7 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
View
2 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 ->
View
82 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"]
)))
}
View
2 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;
View
51 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.
+
+
View
17 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.
View
7 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) ->

1 comment on commit 180e09f

@metadave

This approach certainly seems more readable and cleaner than the previous.

Please sign in to comment.