From 7f9281169683179caf9c37801a9b40e78fe02c04 Mon Sep 17 00:00:00 2001
From: Mark Allen <mrallen1@yahoo.com>
Date: Tue, 21 Jul 2015 09:05:42 -0500
Subject: [PATCH] Use '_lager_event' for sinks

During code review we decided that using Sink ++ '_lager_event'
would provide better isolation for gen_event names than
Sink ++ '_event' as we had been using up to this point.

For example a sink called 'audit' is now 'audit_lager_event'
than 'audit_event' which might clash with a user's predefined
own module or process name.
---
 README.md                   | 10 +++++-----
 src/lager_app.erl           | 11 +++++++++--
 src/lager_transform.erl     | 11 +++++++++--
 test/lager_test_backend.erl |  4 ++--
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/README.md b/README.md
index 535e938..645d3aa 100644
--- a/README.md
+++ b/README.md
@@ -114,19 +114,19 @@ need to:
 #### Names
 
 Each sink has two names: one atom to be used like a module name for
-sending messages, and that atom with `_event` appended for backend
+sending messages, and that atom with `_lager_event` appended for backend
 configuration.
 
 This reflects the legacy behavior: `lager:info` (or `critical`, or
 `debug`, etc) is a way of sending a message to a sink named
 `lager_event`. Now developers can invoke `audit:info` or
-`myCompanyName:debug` so long as the corresponding `audit_event` or
-`myCompanyName_event` sinks are configured.
+`myCompanyName:debug` so long as the corresponding `audit_lager_event` or
+`myCompanyName_lager_event` sinks are configured.
 
 #### rebar.config
 
 In `rebar.config` for the project that requires lager, include a list
-of sink names (without the `_event` suffix) in `erl_opts`:
+of sink names (without the `_lager_event` suffix) in `erl_opts`:
 
 `{lager_extra_sinks, [audit]}`
 
@@ -155,7 +155,7 @@ will be applied on that sink.
           %% Any other sinks
           {extra_sinks,
            [
-            {audit_event,
+            {audit_lager_event,
              [{handlers,
                [{lager_file_backend,
                  [{file, "sink1.log"},
diff --git a/src/lager_app.erl b/src/lager_app.erl
index d2d5451..bb05a2c 100644
--- a/src/lager_app.erl
+++ b/src/lager_app.erl
@@ -164,14 +164,21 @@ wrap_proplist_value(undefined) ->
 wrap_proplist_value(Value) ->
     {ok, Value}.
 
+%% The default sink has always been lager_event. Using 'lager_lager_event' is likely to break
+%% historical code and lots of tests.
+make_internal_sink_name(lager) -> ?DEFAULT_SINK;
+
+make_internal_sink_name(Sink) ->
+    list_to_atom(atom_to_list(Sink) ++ "_lager_event").
+
 configure_sink(Sink, SinkDef) ->
     lager_config:new_sink(Sink),
-    ChildId = list_to_atom(atom_to_list(Sink) ++ "_event"),
+    ChildId = make_internal_sink_name(Sink),
     _ = supervisor:start_child(lager_sup,
                                {ChildId,
                                 {gen_event, start_link,
                                  [{local, Sink}]},
-                                permanent, 5000, worker, [dynamic]}),
+                                permanent, 5000, worker, dynamic}),
     determine_async_behavior(Sink,
                              wrap_proplist_value(
                                proplists:get_value(async_threshold, SinkDef)),
diff --git a/src/lager_transform.erl b/src/lager_transform.erl
index ceddb22..ee1640e 100644
--- a/src/lager_transform.erl
+++ b/src/lager_transform.erl
@@ -79,6 +79,13 @@ walk_body(Acc, []) ->
 walk_body(Acc, [H|T]) ->
     walk_body([transform_statement(H, get(sinks))|Acc], T).
 
+%% 'lager' is special cased because its sink has *always* been named 'lager_event'
+make_internal_sink_name(lager) -> ?DEFAULT_SINK;
+
+%% Everything else should be Sink++_lager_event
+make_internal_sink_name(Sink) ->
+    list_to_atom(atom_to_list(Sink) ++ "_lager_event").
+
 transform_statement({call, Line, {remote, _Line1, {atom, _Line2, Module},
                                   {atom, _Line3, Function}}, Arguments0} = Stmt,
                     Sinks) ->
@@ -86,12 +93,12 @@ transform_statement({call, Line, {remote, _Line1, {atom, _Line2, Module},
         true ->
             case lists:member(Function, ?LEVELS) of
                 true ->
-                    SinkName = list_to_atom(atom_to_list(Module) ++ "_event"),
+                    SinkName = make_internal_sink_name(Module),
                     do_transform(Line, SinkName, Function, Arguments0);
                 false ->
                     case lists:keyfind(Function, 1, ?LEVELS_UNSAFE) of
                         {Function, Severity} ->
-                            SinkName = list_to_atom(atom_to_list(Module) ++ "_event"),
+                            SinkName = make_internal_sink_name(Module),
                             do_transform(Line, SinkName, Severity, Arguments0, unsafe);
                         false ->
                             Stmt
diff --git a/test/lager_test_backend.erl b/test/lager_test_backend.erl
index 51dd738..3b41ca5 100644
--- a/test/lager_test_backend.erl
+++ b/test/lager_test_backend.erl
@@ -27,8 +27,8 @@
 -export([init/1, handle_call/2, handle_event/2, handle_info/2, terminate/2,
         code_change/3]).
 
--define(TEST_SINK_NAME, '__lager_test_sink').        %% <-- used by parse transform
--define(TEST_SINK_EVENT, '__lager_test_sink_event'). %% <-- used by lager API calls and internals for gen_event
+-define(TEST_SINK_NAME, '__lager_test_sink').              %% <-- used by parse transform
+-define(TEST_SINK_EVENT, '__lager_test_sink_lager_event'). %% <-- used by lager API calls and internals for gen_event
 
 -record(state, {level :: list(), buffer :: list(), ignored :: term()}).
 -compile({parse_transform, lager_transform}).