Well, it would not have to be as fancy perhaps? Gary
On Sun, Feb 21, 2016 at 6:22 PM, Matt Sicker <[email protected]> wrote: > That's like a thousand extra lines. Yikes! > > On 21 February 2016 at 19:55, Gary Gregory <[email protected]> wrote: > >> Is it worth thinking about a standalone StatusLogger? >> >> Gary >> >> On Sun, Feb 21, 2016 at 5:50 PM, Remko Popma <[email protected]> >> wrote: >> >>> The recursive problem is that StatusLogger extends AbstractLogger... >>> >>> On Mon, Feb 22, 2016 at 10:40 AM, Gary Gregory <[email protected]> >>> wrote: >>> >>>> Is there a way StatusLogger could use System.err when it is not fully >>>> initialized? So that everything can still use StatusLogger? >>>> >>>> Gary >>>> >>>> On Sun, Feb 21, 2016 at 5:20 PM, Matt Sicker <[email protected]> wrote: >>>> >>>>> Considering every log message in PropertiesUtil is an error message, >>>>> maybe that class could just use System.err instead. >>>>> >>>>> On 21 February 2016 at 19:12, Ralph Goers <[email protected]> >>>>> wrote: >>>>> >>>>>> It might be. >>>>>> >>>>>> Ralph >>>>>> >>>>>> On Feb 21, 2016, at 4:19 PM, Matt Sicker <[email protected]> wrote: >>>>>> >>>>>> Maybe PropertiesUtil is too low-level to use StatusLogger? I remember >>>>>> fighting with a similar problem a while ago due to cyclic dependencies >>>>>> like >>>>>> that. >>>>>> >>>>>> On 21 February 2016 at 07:13, Remko Popma <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> I tried again to use >>>>>>> PropertiesUtil.getProperties().getStringProperty() instead of >>>>>>> System.getProperty, without success. >>>>>>> The reason is that this logic is executed to initialize a static >>>>>>> field in AbstractLogger; PropertiesUtil internally uses StatusLogger, >>>>>>> which >>>>>>> extends AbstractLogger. >>>>>>> >>>>>>> I see many log4j-api tests fail: it starts with >>>>>>> java.lang.NoClassDefFoundError: Could not initialize class >>>>>>> org.apache.logging.log4j.status.StatusLogger >>>>>>> and goes downhill from there... >>>>>>> >>>>>>> Perhaps there is some way around this but I don't see it... >>>>>>> >>>>>>> >>>>>>> On Sat, Feb 20, 2016 at 7:18 AM, Remko Popma <[email protected]> >>>>>>> wrote: >>>>>>> >>>>>>>> I documented the new properties in the Configuration manual page. >>>>>>>> Did I forget to commit that? >>>>>>>> >>>>>>>> >>>>>>>> On Saturday, 20 February 2016, Remko Popma <[email protected]> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> I initially used PropertiesUtil but this failed somehow. Since >>>>>>>>> this is used while initializing s class constant, the failure >>>>>>>>> resulted in a >>>>>>>>> NoClassDefError... >>>>>>>>> So I reverted to System.getProperties. >>>>>>>>> I can take another look, or if someone else has time, please feel >>>>>>>>> free to replace this with PropertiesUtil. >>>>>>>>> >>>>>>>>> On Saturday, 20 February 2016, Ralph Goers < >>>>>>>>> [email protected]> wrote: >>>>>>>>> >>>>>>>>>> I should have qualified this to say that the >>>>>>>>>> log4j2.component.properties file is managed by the PropertiesUtil >>>>>>>>>> class. >>>>>>>>>> Properties should be access through its methods. >>>>>>>>>> >>>>>>>>>> Ralph >>>>>>>>>> >>>>>>>>>> On Feb 19, 2016, at 1:09 PM, Ralph Goers < >>>>>>>>>> [email protected]> wrote: >>>>>>>>>> >>>>>>>>>> I see two new properties to allow users to override the default >>>>>>>>>> MessageFactory and FlowMessageFactory. It seems very unlikely they >>>>>>>>>> will >>>>>>>>>> ever get used, but they should NOT be calling System.getProperty() >>>>>>>>>> directly. >>>>>>>>>> >>>>>>>>>> Please remember that wherever adding something to the >>>>>>>>>> configuration won’t work you should access it through the >>>>>>>>>> log4j2.component.properties file. Values in that file can be >>>>>>>>>> overridden >>>>>>>>>> via system properties, but users can just create the properties file >>>>>>>>>> instead. In general, we should be leveraging that mechanism and not >>>>>>>>>> calling >>>>>>>>>> System.getProperty(). We also need to document each of these >>>>>>>>>> properties in >>>>>>>>>> a clear way. >>>>>>>>>> >>>>>>>>>> JAXB or Jackson isn’t going to make anything any easier. >>>>>>>>>> >>>>>>>>>> Ralph >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Feb 19, 2016, at 12:20 PM, Gary Gregory < >>>>>>>>>> [email protected]> wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Fri, Feb 19, 2016 at 9:24 AM, Remko Popma < >>>>>>>>>> [email protected]> wrote: >>>>>>>>>> > >>>>>>>>>> > FlowMessageFactory is now extracted. I'm quite happy with the >>>>>>>>>> result. >>>>>>>>>> > Please take a look at >>>>>>>>>> https://issues.apache.org/jira/browse/LOG4J2-1255 for further >>>>>>>>>> follow-up. >>>>>>>>>> >>>>>>>>>> OK, that seems fine. Thank you for doing the work. >>>>>>>>>> >>>>>>>>>> The only thing I am not happy about is the use of system >>>>>>>>>> properties instead of the config file. >>>>>>>>>> >>>>>>>>>> We are perpetuating a mess here. >>>>>>>>>> >>>>>>>>>> What is the role of properties files vs a configuration file? >>>>>>>>>> Which one overrides the other? Are they mutually exclusive? >>>>>>>>>> >>>>>>>>>> I could see sys props set on a command line used to override all >>>>>>>>>> config files. Or the other way around? >>>>>>>>>> >>>>>>>>>> In the long run, the use of sys props is bad. Some users >>>>>>>>>> configure only via files saved and moved around machines. You can't >>>>>>>>>> do that >>>>>>>>>> with sys props. >>>>>>>>>> >>>>>>>>>> Please, let's not make it worse by adding MORE sys props. >>>>>>>>>> >>>>>>>>>> Is the real issue that it is too much of a PITA to update our >>>>>>>>>> config code for XML, JSON, and YAML to support a new setting? >>>>>>>>>> >>>>>>>>>> This tells me we're doing it wrong. I know we do not want to many >>>>>>>>>> deps, our current scheme is too hard to maintain. We could use JAXB >>>>>>>>>> or >>>>>>>>>> Jackson instead. >>>>>>>>>> >>>>>>>>>> Gary >>>>>>>>>> >>>>>>>>>> > >>>>>>>>>> > On Fri, Feb 19, 2016 at 2:40 PM, Remko Popma < >>>>>>>>>> [email protected]> wrote: >>>>>>>>>> >> >>>>>>>>>> >> I see, so there actually is a use case to remove the need for >>>>>>>>>> the isTraceEnabled check with the Supplier param... >>>>>>>>>> >> >>>>>>>>>> >> Sent from my iPhone >>>>>>>>>> >> >>>>>>>>>> >> On 2016/02/19, at 14:10, Ralph Goers < >>>>>>>>>> [email protected]> wrote: >>>>>>>>>> >> >>>>>>>>>> >>> 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]> 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 >>>>>>>>>> >>> >>>>>>>>>> >>> >>>>>>>>>> > >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> 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 >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Matt Sicker <[email protected]> >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Matt Sicker <[email protected]> >>>>> >>>> >>>> >>>> >>>> -- >>>> 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 >>>> >>> >>> >> >> >> -- >> 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 >> > > > > -- > Matt Sicker <[email protected]> > -- 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
