hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alejandro Abdelnur <t...@cloudera.com>
Subject Re: DISCUSS: use SLF4J APIs in new modules?
Date Fri, 11 Apr 2014 17:37:11 GMT
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 <kasha@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 <kasha@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 <andrew.wang@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 <jayunit100@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.
>> 

Mime
View raw message