On Thu, Feb 18, 2016 at 4:22 PM, Ralph Goers <[email protected]>
wrote:
> Is it really necessary to have getLogger support FlowMessageFactory?
> These messages are really meant as wrappers for other messages. so I am not
> even sure what it would mean for getLogger() to support that. How would it
> know what Message it is wrapping?
>
> I am really getting sorry that I started this.
>
Well, hopefully, whatever happens, this is getting all of us into reviewing
existing and new code.
Another benefit of this conversation is that I fell that we have been
remarkably civil and respectful of each other, at least compared to other
outrageous behavior one can read about on the webs.
The use case I want most is in
org.apache.logging.log4j.LoggerTest.flowTracingString_ObjectArray2_ParameterizedMessageFactory()
Which can be summarized as:
Logger myLogger = LogManager.getLogger("Some.Logger", new
ParameterizedMessageFactory("Enter", "Exit"));
EntryMessage msg = myLogger.traceEntry("doFoo(a={}, b={})", 1, 2);
myLogger.traceExit(msg, 3);
If I cannot pass in my flow message factory or if there are now two
factories, I need to be able to set it somehow.
I do not like the idea of have a setFlowMessageFactory on a Logger because
I'd never want to change it.
Gary
> Ralph
>
> On Feb 18, 2016, at 3:31 PM, Gary Gregory <[email protected]> wrote:
>
> On Thu, Feb 18, 2016 at 2:13 PM, Remko Popma <[email protected]>
> wrote:
>
>> I think preserving binary compatibility on its own is a strong reason for
>> doing this, but it's more than that.
>>
>
> OK, since org.apache.logging.log4j.message.MessageFactory is in log4j-api
> that's important. I can buy that. BUT, we are also adding methods to Logger
> so that would break some things too. I guess less breakage is better than
> more in this case!
>
> Overall, I not convinced that this is the best approach but I can
> appreciate that you seem to feel about it stronger that I do.
>
>
>>
>> Having a separate factory for flow messages makes both factories more
>> cohesive (single responsibility principle). No need for one factory to
>> extend the other in my view.
>>
>
> The distinction is pretty subtle here IMO. We are still talking about
> creating messages, but I get your point. For me, the only reason for this
> is to minimize the risk of API breakage, a nobe goal for the log4j-api
> module, if not a requirement.
>
>
>>
>> The logger would have separate instances so users can configure them
>> separately: lower coupling.
>>
>
> OK. So now we have:
>
> org.apache.logging.log4j.LogManager.getLogger(Class<?>, MessageFactory)
> org.apache.logging.log4j.LogManager.getLogger(Object, MessageFactory)
> org.apache.logging.log4j.LogManager.getLogger(String, MessageFactory)
>
> We would add:
>
> org.apache.logging.log4j.LogManager.getLogger(Class<?>, MessageFactory,
> FlowMessageFactory)
> org.apache.logging.log4j.LogManager.getLogger(Object, MessageFactory,
> FlowMessageFactory)
> org.apache.logging.log4j.LogManager.getLogger(String, MessageFactory,
> FlowMessageFactory)
>
> Right? Any other places?
>
>
>>
>> These are both desirable properties so I believe it would improve the
>> design.
>>
>> Does this make sense?
>>
>
> Sure, even though I am less gun-ho about it than you are. I'd say go
> ahead, see how it looks and feels after you refactor. We can keep
> discussing it once your changes hits the repo if need be.
>
> Thank you for putting in the work!
> Gary
>
> Remko
>>
>> Sent from my iPhone
>>
>> On 2016/02/19, at 2:24, Gary Gregory <[email protected]> wrote:
>>
>> Is a flow message factory a kind of message factory or a different kind
>> of factory?
>>
>> Does a logger need instances of both or just the one?
>>
>> Since entry message extends message, should the factory do so as well?
>>
>> Gary, phone, typos.
>> On Feb 18, 2016 8:44 AM, "Remko Popma" <[email protected]> wrote:
>>
>>> Would anyone mind terribly if I factored out the FlowMessage creation
>>> methods from MessageFactory to a new interface FlowMessageFactory?
>>>
>>> Concretely, this interface would contain the methods introduced in
>>> LOG4J2-1255:
>>>
>>> EntryMessage newEntryMessage(Message message);
>>> ExitMessage newExitMessage(Object object, Message message);
>>> ExitMessage newExitMessage(EntryMessage message);
>>> ExitMessage newExitMessage(Object object, EntryMessage message);
>>>
>>> I think flow messages are different enough from normal Messages that a
>>> separate factory makes sense.
>>>
>>> It would also insulate users who created a custom MessageFactory from the
>>> changes we made in LOG4J2-1255.
>>>
>>> Thoughts?
>>>
>>> -Remko
>>>
>>>
>>>
>
>
> --
> E-Mail: [email protected] | [email protected]
> <[email protected]>
> Java Persistence with Hibernate, Second Edition
> <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>
>
>
--
E-Mail: [email protected] | [email protected]
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
