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] > <mailto:[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] > <mailto:[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] > <mailto:[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 >>> > <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 >>> >>>> >>>> <http://garygregory.wordpress.com/> >>> >>>> >>>> Home: http://garygregory.com/ <http://garygregory.com/> >>> >>>> >>>> Tweet! http://twitter.com/GaryGregory >>> >>>> >>>> <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 >>> >>>> >> <http://garygregory.wordpress.com/> >>> >>>> >> Home: http://garygregory.com/ <http://garygregory.com/> >>> >>>> >> Tweet! http://twitter.com/GaryGregory >>> >>>> >> <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 <http://garygregory.wordpress.com/> >>> Home: http://garygregory.com/ <http://garygregory.com/> >>> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory> > > > > > > -- > Matt Sicker <[email protected] <mailto:[email protected]>>
