hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Abdelrahman Shettia <ashet...@hortonworks.com>
Subject Re: DISCUSS: use SLF4J APIs in new modules?
Date Fri, 11 Apr 2014 17:16:13 GMT
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 <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.
> >
>

-- 
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message