logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Remko Popma <remko.po...@gmail.com>
Subject Re: FlowMessageFactory
Date Fri, 19 Feb 2016 01:38:11 GMT
I would start with just a default FlowMessageFactory. Configurable with a
system property, so users can swap in their own if they want.

Only if the need arises to configure FlowMessageFactories on a per-logger
basis, we can consider adding the methods to LogManager to support that.

So no need for additional getLogger methods for now.

The default FlowMessageFactory implementation would be the logic that's in
AbstractMessageFactory now. Gary wrote it so I assume it meets his needs.

Gary, shall we deprecate MessageSupplier and remove entry/exitTrace methods
using them?


On Friday, 19 February 2016, Gary Gregory <garydgregory@gmail.com> wrote:

> On Thu, Feb 18, 2016 at 4:22 PM, Ralph Goers <ralph.goers@dslextreme.com
> <javascript:_e(%7B%7D,'cvml','ralph.goers@dslextreme.com');>> 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 <garydgregory@gmail.com
>> <javascript:_e(%7B%7D,'cvml','garydgregory@gmail.com');>> wrote:
>>
>> On Thu, Feb 18, 2016 at 2:13 PM, Remko Popma <remko.popma@gmail.com
>> <javascript:_e(%7B%7D,'cvml','remko.popma@gmail.com');>> 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 <garydgregory@gmail.com
>>> <javascript:_e(%7B%7D,'cvml','garydgregory@gmail.com');>> 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" <remko.popma@gmail.com
>>> <javascript:_e(%7B%7D,'cvml','remko.popma@gmail.com');>> 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: garydgregory@gmail.com
>> <javascript:_e(%7B%7D,'cvml','garydgregory@gmail.com');> |
>> ggregory@apache.org
>> <javascript:_e(%7B%7D,'cvml','ggregory@apache.org');>
>> 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: garydgregory@gmail.com
> <javascript:_e(%7B%7D,'cvml','garydgregory@gmail.com');> | ggregory@apache.org
> <javascript:_e(%7B%7D,'cvml','ggregory@apache.org');>
> 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
>

Mime
View raw message