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 >
