You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by Steve Loughran <st...@hortonworks.com> on 2014/04/10 10:16:56 UTC

DISCUSS: use SLF4J APIs in new modules?

If we're thinking of future progress, here's a little low-level one: adopt
SLF4J as the API for logging


   1. its the new defacto standard of logging APIs
   2. its a lot better than commons-logging with on demand Inline string
   expansion of varags arguments.
   3. we already ship it, as jetty uses it
   4. we already depend on it, client-side and server-side in the
   hadoop-auth package
   5. it lets people log via logback if they want to. That's client-side,
   even if the server stays on log4j
   6. It's way faster than using String.format()


The best initial thing about SL4FJ is how it only expands its arguments
string values if needed

      LOG.debug("Initialized, principal [{}] from keytab [{}]", principal,
keytab);

not logging at debug? No need to test first. That alone saves code and
improves readability.

The slf4 expansion code handles null values as well as calling toString()
on non-null arguments. Oh and it does arrays too.

 int array = [1, 2, 3];
 String undef = null;

 LOG.info("a = {}, u = {}", array, undef)  -> "a = [1, 2, 3],  u = null"

Switching to SLF4J from commons-logging is as trivial as changing the type
of the logger created, but with one logger per class that does get
expensive in terms of change. Moving to SLF4J across the board would be a
big piece of work -but doable.

Rather than push for a dramatic change why not adopt a policy of demanding
it in new maven subprojects? hadoop-auth shows we permit it, so why not say
"you MUST"?

Once people have experience in using it, and are happy, then we could think
about switching to the new APIs in the core modules. The only troublespot
there is where code calls getLogger() on the commons log to get at the
log4j appender -there's ~3 places in production code that does this, 200+
in tests -tests that do it to turn back log levels. Those tests can stay
with commons-logging, same for the production uses. Mixing commons-logging
and slf4j isn't drastic -they both route to log4j or a.n.other back end.

-Stevve

-- 
CONFIDENTIALITY NOTICE
NOTICE: This message is intended for the use of the individual or entity to 
which it is addressed and may contain information that is confidential, 
privileged and exempt from disclosure under applicable law. If the reader 
of this message is not the intended recipient, you are hereby notified that 
any printing, copying, dissemination, distribution, disclosure or 
forwarding of this communication is strictly prohibited. If you have 
received this communication in error, please contact the sender immediately 
and delete it from your system. Thank You.

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Abdelrahman Shettia <as...@hortonworks.com>.
In addition, we need to consider limiting any printing messages to the
stdout in some cases. This can impact other running some apache products in
silent mode such as hive in 'hive -S' option.

Thanks
-Rahman


On Fri, Apr 11, 2014 at 10:06 AM, Karthik Kambatla <ka...@cloudera.com>wrote:

> On Fri, Apr 11, 2014 at 1:37 AM, Steve Loughran <stevel@hortonworks.com
> >wrote:
>
> > On 10 April 2014 16:38, Karthik Kambatla <ka...@cloudera.com> wrote:
> >
> > > +1 to use slf4j. I would actually vote for (1) new modules must-use,
> (2)
> > > new classes in existing modules are strongly recommended to use, (3)
> > > existing classes can switch to. That would take us closer to using
> slf4j
> > > everywhere faster.
> > >
> >
> >
> > #1  appeals to me.
> >
> > #2 & #3, we'd have a mixed phase but ultimately it'd be good. Maybe have
> a
> > policy for a class switch process of
> > a) switch the LOG declaration, change the imports
> > b) clean up all log statements, dropping the ifDebug and moving to {}
> > strings instead of "text"+ "value
> >
>
> I feel more the requirements we add to switching, the less likely people
> will make the time for it. I think it is reasonable to switch LOG
> declaration and drop ifDebug. Changing all log messages to use {} instead
> of " " +  " " could be really time-taking especially for classes with tons
> of log messages. On the other hand, asking people to do that if they are
> editing an existing log message anyway, it might fly.
>
>
> >
> > or do both at the same time, one class or package at time.
> >
> >
> > Having a consistent log scheme across all classes makes copying and
> moving
> > code easier, especially copy+paste
> >
> > I think there's some bits of code that takes a commons-log argument as a
> > parameter. If these are public they'd need to be retained, and we'd have
> to
> > add an SLF4J logger equivalent.
> >
> > -Steve
> >
> >
> > >
> > > On Thu, Apr 10, 2014 at 8:17 AM, Alejandro Abdelnur <tucu@cloudera.com
> > > >wrote:
> > >
> > > > +1 pn slf4j.
> > > >
> > > > one thing Jay, the issues with log4j will still be there as log4j
> will
> > > > still be under the hood.
> > > >
> > > > thx
> > > >
> > > > Alejandro
> > > > (phone typing)
> > > >
> > > > > On Apr 10, 2014, at 7:35, Andrew Wang <an...@cloudera.com>
> > > wrote:
> > > > >
> > > > > +1 from me, it'd be lovely to get rid of all those isDebugEnabled
> > > checks.
> > > > >
> > > > >
> > > > >> On Thu, Apr 10, 2014 at 4:13 AM, Jay Vyas <ja...@gmail.com>
> > > wrote:
> > > > >>
> > > > >> Slf4j is definetly a great step forward.  Log4j is restrictive for
> > > > complex
> > > > >> and multi tenant apps like hadoop.
> > > > >>
> > > > >> Also the fact that slf4j doesn't use any magic when binding to its
> > log
> > > > >> provider makes it way easier to swap out its implementation then
> > tools
> > > > of
> > > > >> the past.
> > > > >>
> > > > >>>> On Apr 10, 2014, at 2:16 AM, Steve Loughran <
> > stevel@hortonworks.com
> > > >
> > > > >>> wrote:
> > > > >>>
> > > > >>> If we're thinking of future progress, here's a little low-level
> > one:
> > > > >> adopt
> > > > >>> SLF4J as the API for logging
> > > > >>>
> > > > >>>
> > > > >>>  1. its the new defacto standard of logging APIs
> > > > >>>  2. its a lot better than commons-logging with on demand Inline
> > > string
> > > > >>>  expansion of varags arguments.
> > > > >>>  3. we already ship it, as jetty uses it
> > > > >>>  4. we already depend on it, client-side and server-side in the
> > > > >>>  hadoop-auth package
> > > > >>>  5. it lets people log via logback if they want to. That's
> > > client-side,
> > > > >>>  even if the server stays on log4j
> > > > >>>  6. It's way faster than using String.format()
> > > > >>>
> > > > >>>
> > > > >>> The best initial thing about SL4FJ is how it only expands its
> > > arguments
> > > > >>> string values if needed
> > > > >>>
> > > > >>>     LOG.debug("Initialized, principal [{}] from keytab [{}]",
> > > > principal,
> > > > >>> keytab);
> > > > >>>
> > > > >>> not logging at debug? No need to test first. That alone saves
> code
> > > and
> > > > >>> improves readability.
> > > > >>>
> > > > >>> The slf4 expansion code handles null values as well as calling
> > > > toString()
> > > > >>> on non-null arguments. Oh and it does arrays too.
> > > > >>>
> > > > >>> int array = [1, 2, 3];
> > > > >>> String undef = null;
> > > > >>>
> > > > >>> LOG.info("a = {}, u = {}", array, undef)  -> "a = [1, 2, 3],  u =
> > > null"
> > > > >>>
> > > > >>> Switching to SLF4J from commons-logging is as trivial as changing
> > the
> > > > >> type
> > > > >>> of the logger created, but with one logger per class that does
> get
> > > > >>> expensive in terms of change. Moving to SLF4J across the board
> > would
> > > > be a
> > > > >>> big piece of work -but doable.
> > > > >>>
> > > > >>> Rather than push for a dramatic change why not adopt a policy of
> > > > >> demanding
> > > > >>> it in new maven subprojects? hadoop-auth shows we permit it, so
> why
> > > not
> > > > >> say
> > > > >>> "you MUST"?
> > > > >>>
> > > > >>> Once people have experience in using it, and are happy, then we
> > could
> > > > >> think
> > > > >>> about switching to the new APIs in the core modules. The only
> > > > troublespot
> > > > >>> there is where code calls getLogger() on the commons log to get
> at
> > > the
> > > > >>> log4j appender -there's ~3 places in production code that does
> > this,
> > > > 200+
> > > > >>> in tests -tests that do it to turn back log levels. Those tests
> can
> > > > stay
> > > > >>> with commons-logging, same for the production uses. Mixing
> > > > >> commons-logging
> > > > >>> and slf4j isn't drastic -they both route to log4j or a.n.other
> back
> > > > end.
> > > > >>>
> > > > >>> -Stevve
> > > > >>>
> > > > >>> --
> > > > >>> CONFIDENTIALITY NOTICE
> > > > >>> NOTICE: This message is intended for the use of the individual or
> > > > entity
> > > > >> to
> > > > >>> which it is addressed and may contain information that is
> > > confidential,
> > > > >>> privileged and exempt from disclosure under applicable law. If
> the
> > > > reader
> > > > >>> of this message is not the intended recipient, you are hereby
> > > notified
> > > > >> that
> > > > >>> any printing, copying, dissemination, distribution, disclosure or
> > > > >>> forwarding of this communication is strictly prohibited. If you
> > have
> > > > >>> received this communication in error, please contact the sender
> > > > >> immediately
> > > > >>> and delete it from your system. Thank You.
> > > > >>
> > > >
> > >
> >
> > --
> > CONFIDENTIALITY NOTICE
> > NOTICE: This message is intended for the use of the individual or entity
> to
> > which it is addressed and may contain information that is confidential,
> > privileged and exempt from disclosure under applicable law. If the reader
> > of this message is not the intended recipient, you are hereby notified
> that
> > any printing, copying, dissemination, distribution, disclosure or
> > forwarding of this communication is strictly prohibited. If you have
> > received this communication in error, please contact the sender
> immediately
> > and delete it from your system. Thank You.
> >
>

-- 
CONFIDENTIALITY NOTICE
NOTICE: This message is intended for the use of the individual or entity to 
which it is addressed and may contain information that is confidential, 
privileged and exempt from disclosure under applicable law. If the reader 
of this message is not the intended recipient, you are hereby notified that 
any printing, copying, dissemination, distribution, disclosure or 
forwarding of this communication is strictly prohibited. If you have 
received this communication in error, please contact the sender immediately 
and delete it from your system. Thank You.

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Chris Nauroth <cn...@hortonworks.com>.
+1 for the proposal.  Notice also in Steve's examples that there is a lower
risk of a format conversion bug compared to printf style with String.format.

Chris Nauroth
Hortonworks
http://hortonworks.com/



On Fri, Apr 11, 2014 at 10:37 AM, Alejandro Abdelnur <tu...@cloudera.com>wrote:

> if you dont convert mgs to templates the dont remove the guards, else you
> create str mgs obj even when not logging.
>
> thx
>
> Alejandro
> (phone typing)
>
> > On Apr 11, 2014, at 10:06, Karthik Kambatla <ka...@cloudera.com> wrote:
> >
> > On Fri, Apr 11, 2014 at 1:37 AM, Steve Loughran <stevel@hortonworks.com
> >wrote:
> >
> >> On 10 April 2014 16:38, Karthik Kambatla <ka...@cloudera.com> wrote:
> >>
> >>> +1 to use slf4j. I would actually vote for (1) new modules must-use,
> (2)
> >>> new classes in existing modules are strongly recommended to use, (3)
> >>> existing classes can switch to. That would take us closer to using
> slf4j
> >>> everywhere faster.
> >>
> >>
> >> #1  appeals to me.
> >>
> >> #2 & #3, we'd have a mixed phase but ultimately it'd be good. Maybe
> have a
> >> policy for a class switch process of
> >> a) switch the LOG declaration, change the imports
> >> b) clean up all log statements, dropping the ifDebug and moving to {}
> >> strings instead of "text"+ "value
> >
> > I feel more the requirements we add to switching, the less likely people
> > will make the time for it. I think it is reasonable to switch LOG
> > declaration and drop ifDebug. Changing all log messages to use {} instead
> > of " " +  " " could be really time-taking especially for classes with
> tons
> > of log messages. On the other hand, asking people to do that if they are
> > editing an existing log message anyway, it might fly.
> >
> >
> >>
> >> or do both at the same time, one class or package at time.
> >>
> >>
> >> Having a consistent log scheme across all classes makes copying and
> moving
> >> code easier, especially copy+paste
> >>
> >> I think there's some bits of code that takes a commons-log argument as a
> >> parameter. If these are public they'd need to be retained, and we'd
> have to
> >> add an SLF4J logger equivalent.
> >>
> >> -Steve
> >>
> >>
> >>>
> >>> On Thu, Apr 10, 2014 at 8:17 AM, Alejandro Abdelnur <tucu@cloudera.com
> >>>> wrote:
> >>>
> >>>> +1 pn slf4j.
> >>>>
> >>>> one thing Jay, the issues with log4j will still be there as log4j will
> >>>> still be under the hood.
> >>>>
> >>>> thx
> >>>>
> >>>> Alejandro
> >>>> (phone typing)
> >>>>
> >>>>> On Apr 10, 2014, at 7:35, Andrew Wang <an...@cloudera.com>
> >>> wrote:
> >>>>>
> >>>>> +1 from me, it'd be lovely to get rid of all those isDebugEnabled
> >>> checks.
> >>>>>
> >>>>>
> >>>>>> On Thu, Apr 10, 2014 at 4:13 AM, Jay Vyas <ja...@gmail.com>
> >>> wrote:
> >>>>>>
> >>>>>> Slf4j is definetly a great step forward.  Log4j is restrictive for
> >>>> complex
> >>>>>> and multi tenant apps like hadoop.
> >>>>>>
> >>>>>> Also the fact that slf4j doesn't use any magic when binding to its
> >> log
> >>>>>> provider makes it way easier to swap out its implementation then
> >> tools
> >>>> of
> >>>>>> the past.
> >>>>>>
> >>>>>>>> On Apr 10, 2014, at 2:16 AM, Steve Loughran <
> >> stevel@hortonworks.com
> >>>>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>> If we're thinking of future progress, here's a little low-level
> >> one:
> >>>>>> adopt
> >>>>>>> SLF4J as the API for logging
> >>>>>>>
> >>>>>>>
> >>>>>>> 1. its the new defacto standard of logging APIs
> >>>>>>> 2. its a lot better than commons-logging with on demand Inline
> >>> string
> >>>>>>> expansion of varags arguments.
> >>>>>>> 3. we already ship it, as jetty uses it
> >>>>>>> 4. we already depend on it, client-side and server-side in the
> >>>>>>> hadoop-auth package
> >>>>>>> 5. it lets people log via logback if they want to. That's
> >>> client-side,
> >>>>>>> even if the server stays on log4j
> >>>>>>> 6. It's way faster than using String.format()
> >>>>>>>
> >>>>>>>
> >>>>>>> The best initial thing about SL4FJ is how it only expands its
> >>> arguments
> >>>>>>> string values if needed
> >>>>>>>
> >>>>>>>    LOG.debug("Initialized, principal [{}] from keytab [{}]",
> >>>> principal,
> >>>>>>> keytab);
> >>>>>>>
> >>>>>>> not logging at debug? No need to test first. That alone saves code
> >>> and
> >>>>>>> improves readability.
> >>>>>>>
> >>>>>>> The slf4 expansion code handles null values as well as calling
> >>>> toString()
> >>>>>>> on non-null arguments. Oh and it does arrays too.
> >>>>>>>
> >>>>>>> int array = [1, 2, 3];
> >>>>>>> String undef = null;
> >>>>>>>
> >>>>>>> LOG.info("a = {}, u = {}", array, undef)  -> "a = [1, 2, 3],  u =
> >>> null"
> >>>>>>>
> >>>>>>> Switching to SLF4J from commons-logging is as trivial as changing
> >> the
> >>>>>> type
> >>>>>>> of the logger created, but with one logger per class that does get
> >>>>>>> expensive in terms of change. Moving to SLF4J across the board
> >> would
> >>>> be a
> >>>>>>> big piece of work -but doable.
> >>>>>>>
> >>>>>>> Rather than push for a dramatic change why not adopt a policy of
> >>>>>> demanding
> >>>>>>> it in new maven subprojects? hadoop-auth shows we permit it, so why
> >>> not
> >>>>>> say
> >>>>>>> "you MUST"?
> >>>>>>>
> >>>>>>> Once people have experience in using it, and are happy, then we
> >> could
> >>>>>> think
> >>>>>>> about switching to the new APIs in the core modules. The only
> >>>> troublespot
> >>>>>>> there is where code calls getLogger() on the commons log to get at
> >>> the
> >>>>>>> log4j appender -there's ~3 places in production code that does
> >> this,
> >>>> 200+
> >>>>>>> in tests -tests that do it to turn back log levels. Those tests can
> >>>> stay
> >>>>>>> with commons-logging, same for the production uses. Mixing
> >>>>>> commons-logging
> >>>>>>> and slf4j isn't drastic -they both route to log4j or a.n.other back
> >>>> end.
> >>>>>>>
> >>>>>>> -Stevve
> >>>>>>>
> >>>>>>> --
> >>>>>>> CONFIDENTIALITY NOTICE
> >>>>>>> NOTICE: This message is intended for the use of the individual or
> >>>> entity
> >>>>>> to
> >>>>>>> which it is addressed and may contain information that is
> >>> confidential,
> >>>>>>> privileged and exempt from disclosure under applicable law. If the
> >>>> reader
> >>>>>>> of this message is not the intended recipient, you are hereby
> >>> notified
> >>>>>> that
> >>>>>>> any printing, copying, dissemination, distribution, disclosure or
> >>>>>>> forwarding of this communication is strictly prohibited. If you
> >> have
> >>>>>>> received this communication in error, please contact the sender
> >>>>>> immediately
> >>>>>>> and delete it from your system. Thank You.
> >>
> >> --
> >> CONFIDENTIALITY NOTICE
> >> NOTICE: This message is intended for the use of the individual or
> entity to
> >> which it is addressed and may contain information that is confidential,
> >> privileged and exempt from disclosure under applicable law. If the
> reader
> >> of this message is not the intended recipient, you are hereby notified
> that
> >> any printing, copying, dissemination, distribution, disclosure or
> >> forwarding of this communication is strictly prohibited. If you have
> >> received this communication in error, please contact the sender
> immediately
> >> and delete it from your system. Thank You.
> >>
>

-- 
CONFIDENTIALITY NOTICE
NOTICE: This message is intended for the use of the individual or entity to 
which it is addressed and may contain information that is confidential, 
privileged and exempt from disclosure under applicable law. If the reader 
of this message is not the intended recipient, you are hereby notified that 
any printing, copying, dissemination, distribution, disclosure or 
forwarding of this communication is strictly prohibited. If you have 
received this communication in error, please contact the sender immediately 
and delete it from your system. Thank You.

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Jay Vyas <ja...@gmail.com>.
Couple more questions:

- what is "source" vs. "modules" in steve's above outline?
- Should individual JIRAs be submitted to start doing this for segments of
the code, and if so at what granularity?

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Alejandro Abdelnur <tu...@cloudera.com>.
correct. 

thx

Alejandro
(phone typing)

> On Apr 22, 2014, at 15:02, Andrew Wang <an...@cloudera.com> wrote:
> 
> #7, if I understand Tucu's point correctly, was that removing
> isDebugEnabled guards also requires updating the logging to use templates
> rather than string construction. We don't need guards with the new-style
> templated logging.
> 
> +1 besides that though Steve, thanks for writing this up. It seems like the
> next step would be putting this on the wiki and linking it from places.
> 
> 
> On Fri, Apr 18, 2014 at 2:32 AM, Steve Loughran <st...@hortonworks.com>wrote:
> 
>> OK, should what policy should we recommend, using RFC2119 should/may/must
>> 
>> Source:
>> 
>> 
>>   1. switch to SLF4J in imports and use -except in situations where access
>>   to any underlying log4j appender is needed. In that case: retain
>>   commons-logging
>>   2. Methods that take a commons-logging argument SHOULD add a version
>>   which takes an SLF parameter -where it isn't too expensive to do this
>>   (i.e. code maintenance costs). Maybe these should stop taking logs as
>>   arguments and just use their local logs?
>>   3. info, warn, and error messages MAY be logged without wrappers
>>   4. arguments SHOULD be passed as trailing values, so that they can be
>>   on-demand stringified
>>   5. classes SHOULD provide low-cost toString operators with useful
>>   information. Propose: super.toString() plus, what? values with low cost
>>   eval and no failures if null?
>>   6. exceptions MUST be added as last value (this gets them picked up by
>>   both the text and the exception trace)
>>   7. debug messages in production code SHOULD still be protected by an
>>   ifDebugEnabled guard to reduce string construction costs?
>> 
>> #7 is to address Alejandro's point -I think we could not worry about tests,
>> because they aren't so important
>> 
>> Modules
>> 
>> 
>>   1. existing code MAY switch -ideally via a package-at-a-time basis, in
>>   patches that MUST do nothing but the import and binding.
>>   2. The same log path (as defined by classname or otherwise) MUST be used
>>   (to ensure that existing log4j settings still work)
>>   3. existing code MAY have their existing log operations moved to the new
>>   format -in patches that MUST do nothing but update the logging. (I'd
>>   recommend focusing on uses of stringifyException() first)
>>   4. new classes MUST use SLF4J from the outset -except in tests when
>>   access to underlying log appenders are needed
>>   5. new modules MUST use SLF4J  -except in tests when access to
>>   underlying log appenders are needed
>> 
>> What I'm trying to do there is have a low-cost migration that doesn't hit
>> every file at once, and lets people switch to it.
>> 
>> 
>> 
>> 
>> 
>>> On 16 April 2014 21:04, Eric Baldeschwieler <je...@gmail.com> wrote:
>>> 
>>> +1 * many.  I'd love to see us clean this up.  Getting to agreement on
>>> where we are going would be a huge step forward.
>>> 
>>> --
>>> Eric14 a.k.a. Eric Baldeschwieler
>>> 
>>> 
>>> On Mon, Apr 14, 2014 at 3:33 PM, Steve Loughran <stevel@hortonworks.com
>>>> wrote:
>>> 
>>>>> On 11 April 2014 18:37, Alejandro Abdelnur <tu...@cloudera.com> wrote:
>>>>> 
>>>>> if you dont convert mgs to templates the dont remove the guards, else
>>> you
>>>>> create str mgs obj even when not logging.
>>>> that is -we should have static constants? I like to do that in strings
>>>> anyway, because tests can stay in sync with messages that change,
>> though
>>>> once there's template expansion comparison suddenly gets harder
>>>> 
>>>> --
>>>> CONFIDENTIALITY NOTICE
>>>> NOTICE: This message is intended for the use of the individual or
>> entity
>>> to
>>>> which it is addressed and may contain information that is confidential,
>>>> privileged and exempt from disclosure under applicable law. If the
>> reader
>>>> of this message is not the intended recipient, you are hereby notified
>>> that
>>>> any printing, copying, dissemination, distribution, disclosure or
>>>> forwarding of this communication is strictly prohibited. If you have
>>>> received this communication in error, please contact the sender
>>> immediately
>>>> and delete it from your system. Thank You.
>> 
>> --
>> CONFIDENTIALITY NOTICE
>> NOTICE: This message is intended for the use of the individual or entity to
>> which it is addressed and may contain information that is confidential,
>> privileged and exempt from disclosure under applicable law. If the reader
>> of this message is not the intended recipient, you are hereby notified that
>> any printing, copying, dissemination, distribution, disclosure or
>> forwarding of this communication is strictly prohibited. If you have
>> received this communication in error, please contact the sender immediately
>> and delete it from your system. Thank You.
>> 

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Andrew Wang <an...@cloudera.com>.
#7, if I understand Tucu's point correctly, was that removing
isDebugEnabled guards also requires updating the logging to use templates
rather than string construction. We don't need guards with the new-style
templated logging.

+1 besides that though Steve, thanks for writing this up. It seems like the
next step would be putting this on the wiki and linking it from places.


On Fri, Apr 18, 2014 at 2:32 AM, Steve Loughran <st...@hortonworks.com>wrote:

> OK, should what policy should we recommend, using RFC2119 should/may/must
>
> Source:
>
>
>    1. switch to SLF4J in imports and use -except in situations where access
>    to any underlying log4j appender is needed. In that case: retain
>    commons-logging
>    2. Methods that take a commons-logging argument SHOULD add a version
>    which takes an SLF parameter -where it isn't too expensive to do this
>    (i.e. code maintenance costs). Maybe these should stop taking logs as
>    arguments and just use their local logs?
>    3. info, warn, and error messages MAY be logged without wrappers
>    4. arguments SHOULD be passed as trailing values, so that they can be
>    on-demand stringified
>    5. classes SHOULD provide low-cost toString operators with useful
>    information. Propose: super.toString() plus, what? values with low cost
>    eval and no failures if null?
>    6. exceptions MUST be added as last value (this gets them picked up by
>    both the text and the exception trace)
>    7. debug messages in production code SHOULD still be protected by an
>    ifDebugEnabled guard to reduce string construction costs?
>
> #7 is to address Alejandro's point -I think we could not worry about tests,
> because they aren't so important
>
> Modules
>
>
>    1. existing code MAY switch -ideally via a package-at-a-time basis, in
>    patches that MUST do nothing but the import and binding.
>    2. The same log path (as defined by classname or otherwise) MUST be used
>    (to ensure that existing log4j settings still work)
>    3. existing code MAY have their existing log operations moved to the new
>    format -in patches that MUST do nothing but update the logging. (I'd
>    recommend focusing on uses of stringifyException() first)
>    4. new classes MUST use SLF4J from the outset -except in tests when
>    access to underlying log appenders are needed
>    5. new modules MUST use SLF4J  -except in tests when access to
>    underlying log appenders are needed
>
> What I'm trying to do there is have a low-cost migration that doesn't hit
> every file at once, and lets people switch to it.
>
>
>
>
>
> On 16 April 2014 21:04, Eric Baldeschwieler <je...@gmail.com> wrote:
>
> > +1 * many.  I'd love to see us clean this up.  Getting to agreement on
> > where we are going would be a huge step forward.
> >
> > --
> > Eric14 a.k.a. Eric Baldeschwieler
> >
> >
> > On Mon, Apr 14, 2014 at 3:33 PM, Steve Loughran <stevel@hortonworks.com
> > >wrote:
> >
> > > On 11 April 2014 18:37, Alejandro Abdelnur <tu...@cloudera.com> wrote:
> > >
> > > > if you dont convert mgs to templates the dont remove the guards, else
> > you
> > > > create str mgs obj even when not logging.
> > > >
> > > >
> > > that is -we should have static constants? I like to do that in strings
> > > anyway, because tests can stay in sync with messages that change,
> though
> > > once there's template expansion comparison suddenly gets harder
> > >
> > > --
> > > CONFIDENTIALITY NOTICE
> > > NOTICE: This message is intended for the use of the individual or
> entity
> > to
> > > which it is addressed and may contain information that is confidential,
> > > privileged and exempt from disclosure under applicable law. If the
> reader
> > > of this message is not the intended recipient, you are hereby notified
> > that
> > > any printing, copying, dissemination, distribution, disclosure or
> > > forwarding of this communication is strictly prohibited. If you have
> > > received this communication in error, please contact the sender
> > immediately
> > > and delete it from your system. Thank You.
> > >
> >
>
> --
> CONFIDENTIALITY NOTICE
> NOTICE: This message is intended for the use of the individual or entity to
> which it is addressed and may contain information that is confidential,
> privileged and exempt from disclosure under applicable law. If the reader
> of this message is not the intended recipient, you are hereby notified that
> any printing, copying, dissemination, distribution, disclosure or
> forwarding of this communication is strictly prohibited. If you have
> received this communication in error, please contact the sender immediately
> and delete it from your system. Thank You.
>

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Steve Loughran <st...@hortonworks.com>.
OK, should what policy should we recommend, using RFC2119 should/may/must

Source:


   1. switch to SLF4J in imports and use -except in situations where access
   to any underlying log4j appender is needed. In that case: retain
   commons-logging
   2. Methods that take a commons-logging argument SHOULD add a version
   which takes an SLF parameter -where it isn't too expensive to do this
   (i.e. code maintenance costs). Maybe these should stop taking logs as
   arguments and just use their local logs?
   3. info, warn, and error messages MAY be logged without wrappers
   4. arguments SHOULD be passed as trailing values, so that they can be
   on-demand stringified
   5. classes SHOULD provide low-cost toString operators with useful
   information. Propose: super.toString() plus, what? values with low cost
   eval and no failures if null?
   6. exceptions MUST be added as last value (this gets them picked up by
   both the text and the exception trace)
   7. debug messages in production code SHOULD still be protected by an
   ifDebugEnabled guard to reduce string construction costs?

#7 is to address Alejandro's point -I think we could not worry about tests,
because they aren't so important

Modules


   1. existing code MAY switch -ideally via a package-at-a-time basis, in
   patches that MUST do nothing but the import and binding.
   2. The same log path (as defined by classname or otherwise) MUST be used
   (to ensure that existing log4j settings still work)
   3. existing code MAY have their existing log operations moved to the new
   format -in patches that MUST do nothing but update the logging. (I'd
   recommend focusing on uses of stringifyException() first)
   4. new classes MUST use SLF4J from the outset -except in tests when
   access to underlying log appenders are needed
   5. new modules MUST use SLF4J  -except in tests when access to
   underlying log appenders are needed

What I'm trying to do there is have a low-cost migration that doesn't hit
every file at once, and lets people switch to it.





On 16 April 2014 21:04, Eric Baldeschwieler <je...@gmail.com> wrote:

> +1 * many.  I'd love to see us clean this up.  Getting to agreement on
> where we are going would be a huge step forward.
>
> --
> Eric14 a.k.a. Eric Baldeschwieler
>
>
> On Mon, Apr 14, 2014 at 3:33 PM, Steve Loughran <stevel@hortonworks.com
> >wrote:
>
> > On 11 April 2014 18:37, Alejandro Abdelnur <tu...@cloudera.com> wrote:
> >
> > > if you dont convert mgs to templates the dont remove the guards, else
> you
> > > create str mgs obj even when not logging.
> > >
> > >
> > that is -we should have static constants? I like to do that in strings
> > anyway, because tests can stay in sync with messages that change, though
> > once there's template expansion comparison suddenly gets harder
> >
> > --
> > CONFIDENTIALITY NOTICE
> > NOTICE: This message is intended for the use of the individual or entity
> to
> > which it is addressed and may contain information that is confidential,
> > privileged and exempt from disclosure under applicable law. If the reader
> > of this message is not the intended recipient, you are hereby notified
> that
> > any printing, copying, dissemination, distribution, disclosure or
> > forwarding of this communication is strictly prohibited. If you have
> > received this communication in error, please contact the sender
> immediately
> > and delete it from your system. Thank You.
> >
>

-- 
CONFIDENTIALITY NOTICE
NOTICE: This message is intended for the use of the individual or entity to 
which it is addressed and may contain information that is confidential, 
privileged and exempt from disclosure under applicable law. If the reader 
of this message is not the intended recipient, you are hereby notified that 
any printing, copying, dissemination, distribution, disclosure or 
forwarding of this communication is strictly prohibited. If you have 
received this communication in error, please contact the sender immediately 
and delete it from your system. Thank You.

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Eric Baldeschwieler <je...@gmail.com>.
+1 * many.  I'd love to see us clean this up.  Getting to agreement on
where we are going would be a huge step forward.

--
Eric14 a.k.a. Eric Baldeschwieler


On Mon, Apr 14, 2014 at 3:33 PM, Steve Loughran <st...@hortonworks.com>wrote:

> On 11 April 2014 18:37, Alejandro Abdelnur <tu...@cloudera.com> wrote:
>
> > if you dont convert mgs to templates the dont remove the guards, else you
> > create str mgs obj even when not logging.
> >
> >
> that is -we should have static constants? I like to do that in strings
> anyway, because tests can stay in sync with messages that change, though
> once there's template expansion comparison suddenly gets harder
>
> --
> CONFIDENTIALITY NOTICE
> NOTICE: This message is intended for the use of the individual or entity to
> which it is addressed and may contain information that is confidential,
> privileged and exempt from disclosure under applicable law. If the reader
> of this message is not the intended recipient, you are hereby notified that
> any printing, copying, dissemination, distribution, disclosure or
> forwarding of this communication is strictly prohibited. If you have
> received this communication in error, please contact the sender immediately
> and delete it from your system. Thank You.
>

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Steve Loughran <st...@hortonworks.com>.
On 11 April 2014 18:37, Alejandro Abdelnur <tu...@cloudera.com> wrote:

> if you dont convert mgs to templates the dont remove the guards, else you
> create str mgs obj even when not logging.
>
>
that is -we should have static constants? I like to do that in strings
anyway, because tests can stay in sync with messages that change, though
once there's template expansion comparison suddenly gets harder

-- 
CONFIDENTIALITY NOTICE
NOTICE: This message is intended for the use of the individual or entity to 
which it is addressed and may contain information that is confidential, 
privileged and exempt from disclosure under applicable law. If the reader 
of this message is not the intended recipient, you are hereby notified that 
any printing, copying, dissemination, distribution, disclosure or 
forwarding of this communication is strictly prohibited. If you have 
received this communication in error, please contact the sender immediately 
and delete it from your system. Thank You.

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Alejandro Abdelnur <tu...@cloudera.com>.
if you dont convert mgs to templates the dont remove the guards, else you create str mgs obj even when not logging. 

thx

Alejandro
(phone typing)

> On Apr 11, 2014, at 10:06, Karthik Kambatla <ka...@cloudera.com> wrote:
> 
> On Fri, Apr 11, 2014 at 1:37 AM, Steve Loughran <st...@hortonworks.com>wrote:
> 
>> On 10 April 2014 16:38, Karthik Kambatla <ka...@cloudera.com> wrote:
>> 
>>> +1 to use slf4j. I would actually vote for (1) new modules must-use, (2)
>>> new classes in existing modules are strongly recommended to use, (3)
>>> existing classes can switch to. That would take us closer to using slf4j
>>> everywhere faster.
>> 
>> 
>> #1  appeals to me.
>> 
>> #2 & #3, we'd have a mixed phase but ultimately it'd be good. Maybe have a
>> policy for a class switch process of
>> a) switch the LOG declaration, change the imports
>> b) clean up all log statements, dropping the ifDebug and moving to {}
>> strings instead of "text"+ "value
> 
> I feel more the requirements we add to switching, the less likely people
> will make the time for it. I think it is reasonable to switch LOG
> declaration and drop ifDebug. Changing all log messages to use {} instead
> of " " +  " " could be really time-taking especially for classes with tons
> of log messages. On the other hand, asking people to do that if they are
> editing an existing log message anyway, it might fly.
> 
> 
>> 
>> or do both at the same time, one class or package at time.
>> 
>> 
>> Having a consistent log scheme across all classes makes copying and moving
>> code easier, especially copy+paste
>> 
>> I think there's some bits of code that takes a commons-log argument as a
>> parameter. If these are public they'd need to be retained, and we'd have to
>> add an SLF4J logger equivalent.
>> 
>> -Steve
>> 
>> 
>>> 
>>> On Thu, Apr 10, 2014 at 8:17 AM, Alejandro Abdelnur <tucu@cloudera.com
>>>> wrote:
>>> 
>>>> +1 pn slf4j.
>>>> 
>>>> one thing Jay, the issues with log4j will still be there as log4j will
>>>> still be under the hood.
>>>> 
>>>> thx
>>>> 
>>>> Alejandro
>>>> (phone typing)
>>>> 
>>>>> On Apr 10, 2014, at 7:35, Andrew Wang <an...@cloudera.com>
>>> wrote:
>>>>> 
>>>>> +1 from me, it'd be lovely to get rid of all those isDebugEnabled
>>> checks.
>>>>> 
>>>>> 
>>>>>> On Thu, Apr 10, 2014 at 4:13 AM, Jay Vyas <ja...@gmail.com>
>>> wrote:
>>>>>> 
>>>>>> Slf4j is definetly a great step forward.  Log4j is restrictive for
>>>> complex
>>>>>> and multi tenant apps like hadoop.
>>>>>> 
>>>>>> Also the fact that slf4j doesn't use any magic when binding to its
>> log
>>>>>> provider makes it way easier to swap out its implementation then
>> tools
>>>> of
>>>>>> the past.
>>>>>> 
>>>>>>>> On Apr 10, 2014, at 2:16 AM, Steve Loughran <
>> stevel@hortonworks.com
>>>> 
>>>>>>> wrote:
>>>>>>> 
>>>>>>> If we're thinking of future progress, here's a little low-level
>> one:
>>>>>> adopt
>>>>>>> SLF4J as the API for logging
>>>>>>> 
>>>>>>> 
>>>>>>> 1. its the new defacto standard of logging APIs
>>>>>>> 2. its a lot better than commons-logging with on demand Inline
>>> string
>>>>>>> expansion of varags arguments.
>>>>>>> 3. we already ship it, as jetty uses it
>>>>>>> 4. we already depend on it, client-side and server-side in the
>>>>>>> hadoop-auth package
>>>>>>> 5. it lets people log via logback if they want to. That's
>>> client-side,
>>>>>>> even if the server stays on log4j
>>>>>>> 6. It's way faster than using String.format()
>>>>>>> 
>>>>>>> 
>>>>>>> The best initial thing about SL4FJ is how it only expands its
>>> arguments
>>>>>>> string values if needed
>>>>>>> 
>>>>>>>    LOG.debug("Initialized, principal [{}] from keytab [{}]",
>>>> principal,
>>>>>>> keytab);
>>>>>>> 
>>>>>>> not logging at debug? No need to test first. That alone saves code
>>> and
>>>>>>> improves readability.
>>>>>>> 
>>>>>>> The slf4 expansion code handles null values as well as calling
>>>> toString()
>>>>>>> on non-null arguments. Oh and it does arrays too.
>>>>>>> 
>>>>>>> int array = [1, 2, 3];
>>>>>>> String undef = null;
>>>>>>> 
>>>>>>> LOG.info("a = {}, u = {}", array, undef)  -> "a = [1, 2, 3],  u =
>>> null"
>>>>>>> 
>>>>>>> Switching to SLF4J from commons-logging is as trivial as changing
>> the
>>>>>> type
>>>>>>> of the logger created, but with one logger per class that does get
>>>>>>> expensive in terms of change. Moving to SLF4J across the board
>> would
>>>> be a
>>>>>>> big piece of work -but doable.
>>>>>>> 
>>>>>>> Rather than push for a dramatic change why not adopt a policy of
>>>>>> demanding
>>>>>>> it in new maven subprojects? hadoop-auth shows we permit it, so why
>>> not
>>>>>> say
>>>>>>> "you MUST"?
>>>>>>> 
>>>>>>> Once people have experience in using it, and are happy, then we
>> could
>>>>>> think
>>>>>>> about switching to the new APIs in the core modules. The only
>>>> troublespot
>>>>>>> there is where code calls getLogger() on the commons log to get at
>>> the
>>>>>>> log4j appender -there's ~3 places in production code that does
>> this,
>>>> 200+
>>>>>>> in tests -tests that do it to turn back log levels. Those tests can
>>>> stay
>>>>>>> with commons-logging, same for the production uses. Mixing
>>>>>> commons-logging
>>>>>>> and slf4j isn't drastic -they both route to log4j or a.n.other back
>>>> end.
>>>>>>> 
>>>>>>> -Stevve
>>>>>>> 
>>>>>>> --
>>>>>>> CONFIDENTIALITY NOTICE
>>>>>>> NOTICE: This message is intended for the use of the individual or
>>>> entity
>>>>>> to
>>>>>>> which it is addressed and may contain information that is
>>> confidential,
>>>>>>> privileged and exempt from disclosure under applicable law. If the
>>>> reader
>>>>>>> of this message is not the intended recipient, you are hereby
>>> notified
>>>>>> that
>>>>>>> any printing, copying, dissemination, distribution, disclosure or
>>>>>>> forwarding of this communication is strictly prohibited. If you
>> have
>>>>>>> received this communication in error, please contact the sender
>>>>>> immediately
>>>>>>> and delete it from your system. Thank You.
>> 
>> --
>> CONFIDENTIALITY NOTICE
>> NOTICE: This message is intended for the use of the individual or entity to
>> which it is addressed and may contain information that is confidential,
>> privileged and exempt from disclosure under applicable law. If the reader
>> of this message is not the intended recipient, you are hereby notified that
>> any printing, copying, dissemination, distribution, disclosure or
>> forwarding of this communication is strictly prohibited. If you have
>> received this communication in error, please contact the sender immediately
>> and delete it from your system. Thank You.
>> 

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Karthik Kambatla <ka...@cloudera.com>.
On Fri, Apr 11, 2014 at 1:37 AM, Steve Loughran <st...@hortonworks.com>wrote:

> On 10 April 2014 16:38, Karthik Kambatla <ka...@cloudera.com> wrote:
>
> > +1 to use slf4j. I would actually vote for (1) new modules must-use, (2)
> > new classes in existing modules are strongly recommended to use, (3)
> > existing classes can switch to. That would take us closer to using slf4j
> > everywhere faster.
> >
>
>
> #1  appeals to me.
>
> #2 & #3, we'd have a mixed phase but ultimately it'd be good. Maybe have a
> policy for a class switch process of
> a) switch the LOG declaration, change the imports
> b) clean up all log statements, dropping the ifDebug and moving to {}
> strings instead of "text"+ "value
>

I feel more the requirements we add to switching, the less likely people
will make the time for it. I think it is reasonable to switch LOG
declaration and drop ifDebug. Changing all log messages to use {} instead
of " " +  " " could be really time-taking especially for classes with tons
of log messages. On the other hand, asking people to do that if they are
editing an existing log message anyway, it might fly.


>
> or do both at the same time, one class or package at time.
>
>
> Having a consistent log scheme across all classes makes copying and moving
> code easier, especially copy+paste
>
> I think there's some bits of code that takes a commons-log argument as a
> parameter. If these are public they'd need to be retained, and we'd have to
> add an SLF4J logger equivalent.
>
> -Steve
>
>
> >
> > On Thu, Apr 10, 2014 at 8:17 AM, Alejandro Abdelnur <tucu@cloudera.com
> > >wrote:
> >
> > > +1 pn slf4j.
> > >
> > > one thing Jay, the issues with log4j will still be there as log4j will
> > > still be under the hood.
> > >
> > > thx
> > >
> > > Alejandro
> > > (phone typing)
> > >
> > > > On Apr 10, 2014, at 7:35, Andrew Wang <an...@cloudera.com>
> > wrote:
> > > >
> > > > +1 from me, it'd be lovely to get rid of all those isDebugEnabled
> > checks.
> > > >
> > > >
> > > >> On Thu, Apr 10, 2014 at 4:13 AM, Jay Vyas <ja...@gmail.com>
> > wrote:
> > > >>
> > > >> Slf4j is definetly a great step forward.  Log4j is restrictive for
> > > complex
> > > >> and multi tenant apps like hadoop.
> > > >>
> > > >> Also the fact that slf4j doesn't use any magic when binding to its
> log
> > > >> provider makes it way easier to swap out its implementation then
> tools
> > > of
> > > >> the past.
> > > >>
> > > >>>> On Apr 10, 2014, at 2:16 AM, Steve Loughran <
> stevel@hortonworks.com
> > >
> > > >>> wrote:
> > > >>>
> > > >>> If we're thinking of future progress, here's a little low-level
> one:
> > > >> adopt
> > > >>> SLF4J as the API for logging
> > > >>>
> > > >>>
> > > >>>  1. its the new defacto standard of logging APIs
> > > >>>  2. its a lot better than commons-logging with on demand Inline
> > string
> > > >>>  expansion of varags arguments.
> > > >>>  3. we already ship it, as jetty uses it
> > > >>>  4. we already depend on it, client-side and server-side in the
> > > >>>  hadoop-auth package
> > > >>>  5. it lets people log via logback if they want to. That's
> > client-side,
> > > >>>  even if the server stays on log4j
> > > >>>  6. It's way faster than using String.format()
> > > >>>
> > > >>>
> > > >>> The best initial thing about SL4FJ is how it only expands its
> > arguments
> > > >>> string values if needed
> > > >>>
> > > >>>     LOG.debug("Initialized, principal [{}] from keytab [{}]",
> > > principal,
> > > >>> keytab);
> > > >>>
> > > >>> not logging at debug? No need to test first. That alone saves code
> > and
> > > >>> improves readability.
> > > >>>
> > > >>> The slf4 expansion code handles null values as well as calling
> > > toString()
> > > >>> on non-null arguments. Oh and it does arrays too.
> > > >>>
> > > >>> int array = [1, 2, 3];
> > > >>> String undef = null;
> > > >>>
> > > >>> LOG.info("a = {}, u = {}", array, undef)  -> "a = [1, 2, 3],  u =
> > null"
> > > >>>
> > > >>> Switching to SLF4J from commons-logging is as trivial as changing
> the
> > > >> type
> > > >>> of the logger created, but with one logger per class that does get
> > > >>> expensive in terms of change. Moving to SLF4J across the board
> would
> > > be a
> > > >>> big piece of work -but doable.
> > > >>>
> > > >>> Rather than push for a dramatic change why not adopt a policy of
> > > >> demanding
> > > >>> it in new maven subprojects? hadoop-auth shows we permit it, so why
> > not
> > > >> say
> > > >>> "you MUST"?
> > > >>>
> > > >>> Once people have experience in using it, and are happy, then we
> could
> > > >> think
> > > >>> about switching to the new APIs in the core modules. The only
> > > troublespot
> > > >>> there is where code calls getLogger() on the commons log to get at
> > the
> > > >>> log4j appender -there's ~3 places in production code that does
> this,
> > > 200+
> > > >>> in tests -tests that do it to turn back log levels. Those tests can
> > > stay
> > > >>> with commons-logging, same for the production uses. Mixing
> > > >> commons-logging
> > > >>> and slf4j isn't drastic -they both route to log4j or a.n.other back
> > > end.
> > > >>>
> > > >>> -Stevve
> > > >>>
> > > >>> --
> > > >>> CONFIDENTIALITY NOTICE
> > > >>> NOTICE: This message is intended for the use of the individual or
> > > entity
> > > >> to
> > > >>> which it is addressed and may contain information that is
> > confidential,
> > > >>> privileged and exempt from disclosure under applicable law. If the
> > > reader
> > > >>> of this message is not the intended recipient, you are hereby
> > notified
> > > >> that
> > > >>> any printing, copying, dissemination, distribution, disclosure or
> > > >>> forwarding of this communication is strictly prohibited. If you
> have
> > > >>> received this communication in error, please contact the sender
> > > >> immediately
> > > >>> and delete it from your system. Thank You.
> > > >>
> > >
> >
>
> --
> CONFIDENTIALITY NOTICE
> NOTICE: This message is intended for the use of the individual or entity to
> which it is addressed and may contain information that is confidential,
> privileged and exempt from disclosure under applicable law. If the reader
> of this message is not the intended recipient, you are hereby notified that
> any printing, copying, dissemination, distribution, disclosure or
> forwarding of this communication is strictly prohibited. If you have
> received this communication in error, please contact the sender immediately
> and delete it from your system. Thank You.
>

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Steve Loughran <st...@hortonworks.com>.
On 10 April 2014 16:38, Karthik Kambatla <ka...@cloudera.com> wrote:

> +1 to use slf4j. I would actually vote for (1) new modules must-use, (2)
> new classes in existing modules are strongly recommended to use, (3)
> existing classes can switch to. That would take us closer to using slf4j
> everywhere faster.
>


#1  appeals to me.

#2 & #3, we'd have a mixed phase but ultimately it'd be good. Maybe have a
policy for a class switch process of
a) switch the LOG declaration, change the imports
b) clean up all log statements, dropping the ifDebug and moving to {}
strings instead of "text"+ "value

or do both at the same time, one class or package at time.


Having a consistent log scheme across all classes makes copying and moving
code easier, especially copy+paste

I think there's some bits of code that takes a commons-log argument as a
parameter. If these are public they'd need to be retained, and we'd have to
add an SLF4J logger equivalent.

-Steve


>
> On Thu, Apr 10, 2014 at 8:17 AM, Alejandro Abdelnur <tucu@cloudera.com
> >wrote:
>
> > +1 pn slf4j.
> >
> > one thing Jay, the issues with log4j will still be there as log4j will
> > still be under the hood.
> >
> > thx
> >
> > Alejandro
> > (phone typing)
> >
> > > On Apr 10, 2014, at 7:35, Andrew Wang <an...@cloudera.com>
> wrote:
> > >
> > > +1 from me, it'd be lovely to get rid of all those isDebugEnabled
> checks.
> > >
> > >
> > >> On Thu, Apr 10, 2014 at 4:13 AM, Jay Vyas <ja...@gmail.com>
> wrote:
> > >>
> > >> Slf4j is definetly a great step forward.  Log4j is restrictive for
> > complex
> > >> and multi tenant apps like hadoop.
> > >>
> > >> Also the fact that slf4j doesn't use any magic when binding to its log
> > >> provider makes it way easier to swap out its implementation then tools
> > of
> > >> the past.
> > >>
> > >>>> On Apr 10, 2014, at 2:16 AM, Steve Loughran <stevel@hortonworks.com
> >
> > >>> wrote:
> > >>>
> > >>> If we're thinking of future progress, here's a little low-level one:
> > >> adopt
> > >>> SLF4J as the API for logging
> > >>>
> > >>>
> > >>>  1. its the new defacto standard of logging APIs
> > >>>  2. its a lot better than commons-logging with on demand Inline
> string
> > >>>  expansion of varags arguments.
> > >>>  3. we already ship it, as jetty uses it
> > >>>  4. we already depend on it, client-side and server-side in the
> > >>>  hadoop-auth package
> > >>>  5. it lets people log via logback if they want to. That's
> client-side,
> > >>>  even if the server stays on log4j
> > >>>  6. It's way faster than using String.format()
> > >>>
> > >>>
> > >>> The best initial thing about SL4FJ is how it only expands its
> arguments
> > >>> string values if needed
> > >>>
> > >>>     LOG.debug("Initialized, principal [{}] from keytab [{}]",
> > principal,
> > >>> keytab);
> > >>>
> > >>> not logging at debug? No need to test first. That alone saves code
> and
> > >>> improves readability.
> > >>>
> > >>> The slf4 expansion code handles null values as well as calling
> > toString()
> > >>> on non-null arguments. Oh and it does arrays too.
> > >>>
> > >>> int array = [1, 2, 3];
> > >>> String undef = null;
> > >>>
> > >>> LOG.info("a = {}, u = {}", array, undef)  -> "a = [1, 2, 3],  u =
> null"
> > >>>
> > >>> Switching to SLF4J from commons-logging is as trivial as changing the
> > >> type
> > >>> of the logger created, but with one logger per class that does get
> > >>> expensive in terms of change. Moving to SLF4J across the board would
> > be a
> > >>> big piece of work -but doable.
> > >>>
> > >>> Rather than push for a dramatic change why not adopt a policy of
> > >> demanding
> > >>> it in new maven subprojects? hadoop-auth shows we permit it, so why
> not
> > >> say
> > >>> "you MUST"?
> > >>>
> > >>> Once people have experience in using it, and are happy, then we could
> > >> think
> > >>> about switching to the new APIs in the core modules. The only
> > troublespot
> > >>> there is where code calls getLogger() on the commons log to get at
> the
> > >>> log4j appender -there's ~3 places in production code that does this,
> > 200+
> > >>> in tests -tests that do it to turn back log levels. Those tests can
> > stay
> > >>> with commons-logging, same for the production uses. Mixing
> > >> commons-logging
> > >>> and slf4j isn't drastic -they both route to log4j or a.n.other back
> > end.
> > >>>
> > >>> -Stevve
> > >>>
> > >>> --
> > >>> CONFIDENTIALITY NOTICE
> > >>> NOTICE: This message is intended for the use of the individual or
> > entity
> > >> to
> > >>> which it is addressed and may contain information that is
> confidential,
> > >>> privileged and exempt from disclosure under applicable law. If the
> > reader
> > >>> of this message is not the intended recipient, you are hereby
> notified
> > >> that
> > >>> any printing, copying, dissemination, distribution, disclosure or
> > >>> forwarding of this communication is strictly prohibited. If you have
> > >>> received this communication in error, please contact the sender
> > >> immediately
> > >>> and delete it from your system. Thank You.
> > >>
> >
>

-- 
CONFIDENTIALITY NOTICE
NOTICE: This message is intended for the use of the individual or entity to 
which it is addressed and may contain information that is confidential, 
privileged and exempt from disclosure under applicable law. If the reader 
of this message is not the intended recipient, you are hereby notified that 
any printing, copying, dissemination, distribution, disclosure or 
forwarding of this communication is strictly prohibited. If you have 
received this communication in error, please contact the sender immediately 
and delete it from your system. Thank You.

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Karthik Kambatla <ka...@cloudera.com>.
+1 to use slf4j. I would actually vote for (1) new modules must-use, (2)
new classes in existing modules are strongly recommended to use, (3)
existing classes can switch to. That would take us closer to using slf4j
everywhere faster.


On Thu, Apr 10, 2014 at 8:17 AM, Alejandro Abdelnur <tu...@cloudera.com>wrote:

> +1 pn slf4j.
>
> one thing Jay, the issues with log4j will still be there as log4j will
> still be under the hood.
>
> thx
>
> Alejandro
> (phone typing)
>
> > On Apr 10, 2014, at 7:35, Andrew Wang <an...@cloudera.com> wrote:
> >
> > +1 from me, it'd be lovely to get rid of all those isDebugEnabled checks.
> >
> >
> >> On Thu, Apr 10, 2014 at 4:13 AM, Jay Vyas <ja...@gmail.com> wrote:
> >>
> >> Slf4j is definetly a great step forward.  Log4j is restrictive for
> complex
> >> and multi tenant apps like hadoop.
> >>
> >> Also the fact that slf4j doesn't use any magic when binding to its log
> >> provider makes it way easier to swap out its implementation then tools
> of
> >> the past.
> >>
> >>>> On Apr 10, 2014, at 2:16 AM, Steve Loughran <st...@hortonworks.com>
> >>> wrote:
> >>>
> >>> If we're thinking of future progress, here's a little low-level one:
> >> adopt
> >>> SLF4J as the API for logging
> >>>
> >>>
> >>>  1. its the new defacto standard of logging APIs
> >>>  2. its a lot better than commons-logging with on demand Inline string
> >>>  expansion of varags arguments.
> >>>  3. we already ship it, as jetty uses it
> >>>  4. we already depend on it, client-side and server-side in the
> >>>  hadoop-auth package
> >>>  5. it lets people log via logback if they want to. That's client-side,
> >>>  even if the server stays on log4j
> >>>  6. It's way faster than using String.format()
> >>>
> >>>
> >>> The best initial thing about SL4FJ is how it only expands its arguments
> >>> string values if needed
> >>>
> >>>     LOG.debug("Initialized, principal [{}] from keytab [{}]",
> principal,
> >>> keytab);
> >>>
> >>> not logging at debug? No need to test first. That alone saves code and
> >>> improves readability.
> >>>
> >>> The slf4 expansion code handles null values as well as calling
> toString()
> >>> on non-null arguments. Oh and it does arrays too.
> >>>
> >>> int array = [1, 2, 3];
> >>> String undef = null;
> >>>
> >>> LOG.info("a = {}, u = {}", array, undef)  -> "a = [1, 2, 3],  u = null"
> >>>
> >>> Switching to SLF4J from commons-logging is as trivial as changing the
> >> type
> >>> of the logger created, but with one logger per class that does get
> >>> expensive in terms of change. Moving to SLF4J across the board would
> be a
> >>> big piece of work -but doable.
> >>>
> >>> Rather than push for a dramatic change why not adopt a policy of
> >> demanding
> >>> it in new maven subprojects? hadoop-auth shows we permit it, so why not
> >> say
> >>> "you MUST"?
> >>>
> >>> Once people have experience in using it, and are happy, then we could
> >> think
> >>> about switching to the new APIs in the core modules. The only
> troublespot
> >>> there is where code calls getLogger() on the commons log to get at the
> >>> log4j appender -there's ~3 places in production code that does this,
> 200+
> >>> in tests -tests that do it to turn back log levels. Those tests can
> stay
> >>> with commons-logging, same for the production uses. Mixing
> >> commons-logging
> >>> and slf4j isn't drastic -they both route to log4j or a.n.other back
> end.
> >>>
> >>> -Stevve
> >>>
> >>> --
> >>> CONFIDENTIALITY NOTICE
> >>> NOTICE: This message is intended for the use of the individual or
> entity
> >> to
> >>> which it is addressed and may contain information that is confidential,
> >>> privileged and exempt from disclosure under applicable law. If the
> reader
> >>> of this message is not the intended recipient, you are hereby notified
> >> that
> >>> any printing, copying, dissemination, distribution, disclosure or
> >>> forwarding of this communication is strictly prohibited. If you have
> >>> received this communication in error, please contact the sender
> >> immediately
> >>> and delete it from your system. Thank You.
> >>
>

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Steve Loughran <st...@hortonworks.com>.
On 10 April 2014 16:17, Alejandro Abdelnur <tu...@cloudera.com> wrote:

> +1 pn slf4j.
>
> one thing Jay, the issues with log4j will still be there as log4j will
> still be under the hood.
>
>

*may* still be under the hood. Even with commons-logging you can swap out
log4j, and indeed, I did exactly that for hadoop 0.17 at one point.

what we get with slf4j is easier coding

-- 
CONFIDENTIALITY NOTICE
NOTICE: This message is intended for the use of the individual or entity to 
which it is addressed and may contain information that is confidential, 
privileged and exempt from disclosure under applicable law. If the reader 
of this message is not the intended recipient, you are hereby notified that 
any printing, copying, dissemination, distribution, disclosure or 
forwarding of this communication is strictly prohibited. If you have 
received this communication in error, please contact the sender immediately 
and delete it from your system. Thank You.

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Alejandro Abdelnur <tu...@cloudera.com>.
+1 pn slf4j. 

one thing Jay, the issues with log4j will still be there as log4j will still be under the hood. 

thx

Alejandro
(phone typing)

> On Apr 10, 2014, at 7:35, Andrew Wang <an...@cloudera.com> wrote:
> 
> +1 from me, it'd be lovely to get rid of all those isDebugEnabled checks.
> 
> 
>> On Thu, Apr 10, 2014 at 4:13 AM, Jay Vyas <ja...@gmail.com> wrote:
>> 
>> Slf4j is definetly a great step forward.  Log4j is restrictive for complex
>> and multi tenant apps like hadoop.
>> 
>> Also the fact that slf4j doesn't use any magic when binding to its log
>> provider makes it way easier to swap out its implementation then tools of
>> the past.
>> 
>>>> On Apr 10, 2014, at 2:16 AM, Steve Loughran <st...@hortonworks.com>
>>> wrote:
>>> 
>>> If we're thinking of future progress, here's a little low-level one:
>> adopt
>>> SLF4J as the API for logging
>>> 
>>> 
>>>  1. its the new defacto standard of logging APIs
>>>  2. its a lot better than commons-logging with on demand Inline string
>>>  expansion of varags arguments.
>>>  3. we already ship it, as jetty uses it
>>>  4. we already depend on it, client-side and server-side in the
>>>  hadoop-auth package
>>>  5. it lets people log via logback if they want to. That's client-side,
>>>  even if the server stays on log4j
>>>  6. It's way faster than using String.format()
>>> 
>>> 
>>> The best initial thing about SL4FJ is how it only expands its arguments
>>> string values if needed
>>> 
>>>     LOG.debug("Initialized, principal [{}] from keytab [{}]", principal,
>>> keytab);
>>> 
>>> not logging at debug? No need to test first. That alone saves code and
>>> improves readability.
>>> 
>>> The slf4 expansion code handles null values as well as calling toString()
>>> on non-null arguments. Oh and it does arrays too.
>>> 
>>> int array = [1, 2, 3];
>>> String undef = null;
>>> 
>>> LOG.info("a = {}, u = {}", array, undef)  -> "a = [1, 2, 3],  u = null"
>>> 
>>> Switching to SLF4J from commons-logging is as trivial as changing the
>> type
>>> of the logger created, but with one logger per class that does get
>>> expensive in terms of change. Moving to SLF4J across the board would be a
>>> big piece of work -but doable.
>>> 
>>> Rather than push for a dramatic change why not adopt a policy of
>> demanding
>>> it in new maven subprojects? hadoop-auth shows we permit it, so why not
>> say
>>> "you MUST"?
>>> 
>>> Once people have experience in using it, and are happy, then we could
>> think
>>> about switching to the new APIs in the core modules. The only troublespot
>>> there is where code calls getLogger() on the commons log to get at the
>>> log4j appender -there's ~3 places in production code that does this, 200+
>>> in tests -tests that do it to turn back log levels. Those tests can stay
>>> with commons-logging, same for the production uses. Mixing
>> commons-logging
>>> and slf4j isn't drastic -they both route to log4j or a.n.other back end.
>>> 
>>> -Stevve
>>> 
>>> --
>>> CONFIDENTIALITY NOTICE
>>> NOTICE: This message is intended for the use of the individual or entity
>> to
>>> which it is addressed and may contain information that is confidential,
>>> privileged and exempt from disclosure under applicable law. If the reader
>>> of this message is not the intended recipient, you are hereby notified
>> that
>>> any printing, copying, dissemination, distribution, disclosure or
>>> forwarding of this communication is strictly prohibited. If you have
>>> received this communication in error, please contact the sender
>> immediately
>>> and delete it from your system. Thank You.
>> 

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Andrew Wang <an...@cloudera.com>.
+1 from me, it'd be lovely to get rid of all those isDebugEnabled checks.


On Thu, Apr 10, 2014 at 4:13 AM, Jay Vyas <ja...@gmail.com> wrote:

> Slf4j is definetly a great step forward.  Log4j is restrictive for complex
> and multi tenant apps like hadoop.
>
> Also the fact that slf4j doesn't use any magic when binding to its log
> provider makes it way easier to swap out its implementation then tools of
> the past.
>
> > On Apr 10, 2014, at 2:16 AM, Steve Loughran <st...@hortonworks.com>
> wrote:
> >
> > If we're thinking of future progress, here's a little low-level one:
> adopt
> > SLF4J as the API for logging
> >
> >
> >   1. its the new defacto standard of logging APIs
> >   2. its a lot better than commons-logging with on demand Inline string
> >   expansion of varags arguments.
> >   3. we already ship it, as jetty uses it
> >   4. we already depend on it, client-side and server-side in the
> >   hadoop-auth package
> >   5. it lets people log via logback if they want to. That's client-side,
> >   even if the server stays on log4j
> >   6. It's way faster than using String.format()
> >
> >
> > The best initial thing about SL4FJ is how it only expands its arguments
> > string values if needed
> >
> >      LOG.debug("Initialized, principal [{}] from keytab [{}]", principal,
> > keytab);
> >
> > not logging at debug? No need to test first. That alone saves code and
> > improves readability.
> >
> > The slf4 expansion code handles null values as well as calling toString()
> > on non-null arguments. Oh and it does arrays too.
> >
> > int array = [1, 2, 3];
> > String undef = null;
> >
> > LOG.info("a = {}, u = {}", array, undef)  -> "a = [1, 2, 3],  u = null"
> >
> > Switching to SLF4J from commons-logging is as trivial as changing the
> type
> > of the logger created, but with one logger per class that does get
> > expensive in terms of change. Moving to SLF4J across the board would be a
> > big piece of work -but doable.
> >
> > Rather than push for a dramatic change why not adopt a policy of
> demanding
> > it in new maven subprojects? hadoop-auth shows we permit it, so why not
> say
> > "you MUST"?
> >
> > Once people have experience in using it, and are happy, then we could
> think
> > about switching to the new APIs in the core modules. The only troublespot
> > there is where code calls getLogger() on the commons log to get at the
> > log4j appender -there's ~3 places in production code that does this, 200+
> > in tests -tests that do it to turn back log levels. Those tests can stay
> > with commons-logging, same for the production uses. Mixing
> commons-logging
> > and slf4j isn't drastic -they both route to log4j or a.n.other back end.
> >
> > -Stevve
> >
> > --
> > CONFIDENTIALITY NOTICE
> > NOTICE: This message is intended for the use of the individual or entity
> to
> > which it is addressed and may contain information that is confidential,
> > privileged and exempt from disclosure under applicable law. If the reader
> > of this message is not the intended recipient, you are hereby notified
> that
> > any printing, copying, dissemination, distribution, disclosure or
> > forwarding of this communication is strictly prohibited. If you have
> > received this communication in error, please contact the sender
> immediately
> > and delete it from your system. Thank You.
>

Re: DISCUSS: use SLF4J APIs in new modules?

Posted by Jay Vyas <ja...@gmail.com>.
Slf4j is definetly a great step forward.  Log4j is restrictive for complex and multi tenant apps like hadoop.

Also the fact that slf4j doesn't use any magic when binding to its log provider makes it way easier to swap out its implementation then tools of the past.

> On Apr 10, 2014, at 2:16 AM, Steve Loughran <st...@hortonworks.com> wrote:
> 
> If we're thinking of future progress, here's a little low-level one: adopt
> SLF4J as the API for logging
> 
> 
>   1. its the new defacto standard of logging APIs
>   2. its a lot better than commons-logging with on demand Inline string
>   expansion of varags arguments.
>   3. we already ship it, as jetty uses it
>   4. we already depend on it, client-side and server-side in the
>   hadoop-auth package
>   5. it lets people log via logback if they want to. That's client-side,
>   even if the server stays on log4j
>   6. It's way faster than using String.format()
> 
> 
> The best initial thing about SL4FJ is how it only expands its arguments
> string values if needed
> 
>      LOG.debug("Initialized, principal [{}] from keytab [{}]", principal,
> keytab);
> 
> not logging at debug? No need to test first. That alone saves code and
> improves readability.
> 
> The slf4 expansion code handles null values as well as calling toString()
> on non-null arguments. Oh and it does arrays too.
> 
> int array = [1, 2, 3];
> String undef = null;
> 
> LOG.info("a = {}, u = {}", array, undef)  -> "a = [1, 2, 3],  u = null"
> 
> Switching to SLF4J from commons-logging is as trivial as changing the type
> of the logger created, but with one logger per class that does get
> expensive in terms of change. Moving to SLF4J across the board would be a
> big piece of work -but doable.
> 
> Rather than push for a dramatic change why not adopt a policy of demanding
> it in new maven subprojects? hadoop-auth shows we permit it, so why not say
> "you MUST"?
> 
> Once people have experience in using it, and are happy, then we could think
> about switching to the new APIs in the core modules. The only troublespot
> there is where code calls getLogger() on the commons log to get at the
> log4j appender -there's ~3 places in production code that does this, 200+
> in tests -tests that do it to turn back log levels. Those tests can stay
> with commons-logging, same for the production uses. Mixing commons-logging
> and slf4j isn't drastic -they both route to log4j or a.n.other back end.
> 
> -Stevve
> 
> -- 
> CONFIDENTIALITY NOTICE
> NOTICE: This message is intended for the use of the individual or entity to 
> which it is addressed and may contain information that is confidential, 
> privileged and exempt from disclosure under applicable law. If the reader 
> of this message is not the intended recipient, you are hereby notified that 
> any printing, copying, dissemination, distribution, disclosure or 
> forwarding of this communication is strictly prohibited. If you have 
> received this communication in error, please contact the sender immediately 
> and delete it from your system. Thank You.