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 <[email protected]> wrote: > On Thu, Feb 18, 2016 at 4:22 PM, Ralph Goers <[email protected] > <javascript:_e(%7B%7D,'cvml','[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] >> <javascript:_e(%7B%7D,'cvml','[email protected]');>> wrote: >> >> On Thu, Feb 18, 2016 at 2:13 PM, Remko Popma <[email protected] >> <javascript:_e(%7B%7D,'cvml','[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] >>> <javascript:_e(%7B%7D,'cvml','[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] >>> <javascript:_e(%7B%7D,'cvml','[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] >> <javascript:_e(%7B%7D,'cvml','[email protected]');> | >> [email protected] >> <javascript:_e(%7B%7D,'cvml','[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] > <javascript:_e(%7B%7D,'cvml','[email protected]');> | [email protected] > <javascript:_e(%7B%7D,'cvml','[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 >
