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]>
