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
