On Mon, Feb 23, 2015 at 11:50 AM, Chamila De Alwis <[email protected]> wrote:
> Hi, > > +1 for all these changes. This might be crucial in the usability aspect of > the product. Another irregularity I've seen is the generic nature of a log > entry. If a debug/info entry is there indicating a certain operation is to > be done, it should indicate that the operation is *about* to be done, not > being done or done. If, say, "Git checkout" is the log entry before > actually making that decision, it can be ambiguous. > A good point Chamila, Talking about best practices, we should also try to log all the details we have at the time we log. Shouldn't leave any information that are related the information we give at the log. We can have entries for these facts as well, in the doc. Thanks. > > > > > Regards, > Chamila de Alwis > Software Engineer | WSO2 | +94772207163 > Blog: code.chamiladealwis.com > > > > On Mon, Feb 23, 2015 at 11:37 AM, Lahiru Sandaruwan <[email protected]> > wrote: > >> >> >> On Mon, Feb 23, 2015 at 11:32 AM, Isuru Perera <[email protected]> wrote: >> >>> Hi, >>> >>> I agree with every point except for skipping isInfoEnabled(). I believe >>> we must use these guard conditions. Some might decide to run Stratos with >>> only Error/Warn level logs. We might avoid guard conditions for error/fatal >>> level logs. >>> >> >> Agree. Let's enable that flexibility as well. So if we use that, we >> should use them all occasions we log info. >> >> Mari, >> >> Shall we go ahead and create a page? >> >> Thanks. >> >>> >>> And use String.format. Always avoid concatenation. String.format is much >>> better than concatenation and it helps string pooling as well. >>> >>> Thanks! >>> >>> On Mon, Feb 23, 2015 at 11:22 AM, Rajkumar Rajaratnam < >>> [email protected]> wrote: >>> >>>> +1 Lahiru. >>>> >>>> Also if you are logging an object, make sure you are overriding >>>> toString() method of the class. Otherwise the log is nothing but a garbage. >>>> Just got this log on master branch. >>>> >>>> *Rule executed for: >>>> org.apache.stratos.autoscaler.context.partition.ClusterLevelPartitionContext@42c0a4d8* >>>> >>>> ClusterLevelPartitionContext class doesn't override toString() method. >>>> >>>> Thanks. >>>> >>>> On Mon, Feb 23, 2015 at 7:27 AM, Lahiru Sandaruwan <[email protected]> >>>> wrote: >>>> >>>>> Hi all, >>>>> >>>>> There are lot of different approaches of logging in Stratos code. May >>>>> be we can follow same format so that code looks clean and understandable. >>>>> This is to decide which way we should follow. >>>>> >>>>> *1. Separation of objects* >>>>> >>>>> We are using square brackets in most of the places. I think that is a >>>>> good approach. May be we can use a comma after one object. We need to >>>>> decide. >>>>> >>>>> E.g. 1 >>>>> >>>>> [network-partition] network-partition-1 [cluster-instance] >>>>> single-cartridge-app-1 [cluster] php1.php.domain >>>>> >>>>> E.g. 2 >>>>> >>>>> [network-partition] network-partition-1, [cluster-instance] >>>>> single-cartridge-app-1, [cluster] php1.php.domain >>>>> >>>>> *2. Using same text for identifying a particular type* >>>>> E.g. We use [network-parition], [NW partition], [network partition] in >>>>> different places. We could stick to one. >>>>> >>>>> *3. Using the String.format or concatenation. * >>>>> >>>>> String.format brings clarity and readability while it reduces the >>>>> performance against concatenation. We need to decide what should be our >>>>> choice. I would vote for String.format considering performance gain is >>>>> negligible if we only have info level logs. >>>>> >>>>> *4. Use isDebugEnabled() and don't use isInfoEnabled()* >>>>> >>>>> Regarding this, we must use *isDebugEnabled* everywhere we do debug >>>>> logging. But i think we can skip *isInfoEnabled *assuming we will >>>>> have info logs in limited numbers. Wdyt? >>>>> >>>>> We can add a guidance at [2] for developers regarding all the matters >>>>> here, after deciding. >>>>> >>>>> Thanks. >>>>> >>>>> [1] >>>>> http://stackoverflow.com/questions/925423/is-it-better-practice-to-use-string-format-over-string-concatenation-in-java >>>>> [2] >>>>> https://cwiki.apache.org/confluence/display/STRATOS/Developer+Guide >>>>> -- >>>>> -- >>>>> Lahiru Sandaruwan >>>>> Committer and PMC member, Apache Stratos, >>>>> Senior Software Engineer, >>>>> WSO2 Inc., http://wso2.com >>>>> lean.enterprise.middleware >>>>> >>>>> email: [email protected] blog: http://lahiruwrites.blogspot.com/ >>>>> linked-in: http://lk.linkedin.com/pub/lahiru-sandaruwan/16/153/146 >>>>> >>>>> >>>> >>>> >>>> -- >>>> Rajkumar Rajaratnam >>>> Committer & PMC Member, Apache Stratos >>>> Software Engineer, WSO2 >>>> >>>> Mobile : +94777568639 >>>> Blog : rajkumarr.com >>>> >>>> -- >>>> <http://rajkumarr.com> >>>> <http://rajkumarr.com> >>>> Isuru Perera >>>> Senior Software Engineer | WSO2, Inc. | <http://rajkumarr.com> >>>> http://wso2.com/ >>>> Lean . Enterprise . Middleware >>>> >>>> about.me/chrishantha >>>> >>> >> >> >> -- >> -- >> Lahiru Sandaruwan >> Committer and PMC member, Apache Stratos, >> Senior Software Engineer, >> WSO2 Inc., http://wso2.com >> lean.enterprise.middleware >> >> email: [email protected] blog: http://lahiruwrites.blogspot.com/ >> linked-in: http://lk.linkedin.com/pub/lahiru-sandaruwan/16/153/146 >> >> > -- -- Lahiru Sandaruwan Committer and PMC member, Apache Stratos, Senior Software Engineer, WSO2 Inc., http://wso2.com lean.enterprise.middleware email: [email protected] blog: http://lahiruwrites.blogspot.com/ linked-in: http://lk.linkedin.com/pub/lahiru-sandaruwan/16/153/146
