The use case I wanted to do this for is: LOGGER.entry(“Request: “, ()->gson.toJson(request)); . LOGGER.exit(response, ()->gson.toJson(response));
However this can be handled just as easily by LOGGER.entry(new JsonMessage(request)); . LOGGER.exit(response, new JsonMessage(response)); so I can live without the Supplier. I don’t think MessageSupplier actually makes any sense. I can’t see why I would want to do: LOGGER.entry(()->new JsonMessage(request)); since it is just creating one object instead of another. Ralph > On Feb 18, 2016, at 7:52 PM, Gary Gregory <[email protected]> wrote: > > On Feb 18, 2016 5:38 PM, "Remko Popma" <[email protected] > <mailto:[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] > > <mailto:[email protected]>> wrote: > >> > >> On Thu, Feb 18, 2016 at 4:22 PM, Ralph Goers <[email protected] > >> <mailto:[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] > >>>> <mailto:[email protected]>> wrote: > >>>> > >>>> On Thu, Feb 18, 2016 at 2:13 PM, Remko Popma <[email protected] > >>>> <mailto:[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] > >>>>> <mailto:[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] > >>>>>> <mailto:[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] <mailto:[email protected]> | > >>>> [email protected] <mailto:[email protected]> > >>>> Java Persistence with Hibernate, Second Edition > >>>> JUnit in Action, Second Edition > >>>> Spring Batch in Action > >>>> Blog: http://garygregory.wordpress.com > >>>> <http://garygregory.wordpress.com/> > >>>> Home: http://garygregory.com/ <http://garygregory.com/> > >>>> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory> > >>> > >>> > >> > >> > >> > >> -- > >> E-Mail: [email protected] <mailto:[email protected]> | > >> [email protected] <mailto:[email protected]> > >> Java Persistence with Hibernate, Second Edition > >> JUnit in Action, Second Edition > >> Spring Batch in Action > >> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/> > >> Home: http://garygregory.com/ <http://garygregory.com/> > >> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>
