On Feb 18, 2016 5:38 PM, "Remko Popma" <[email protected]> wrote: > > 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?
That's fine with me. Gary > > > On Friday, 19 February 2016, Gary Gregory <[email protected]> wrote: >> >> 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] >>>> Java Persistence with Hibernate, Second Edition >>>> JUnit in Action, Second Edition >>>> Spring Batch in Action >>>> 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 >> JUnit in Action, Second Edition >> Spring Batch in Action >> Blog: http://garygregory.wordpress.com >> Home: http://garygregory.com/ >> Tweet! http://twitter.com/GaryGregory
