On Fri, Apr 11, 2014 at 1:37 AM, Steve Loughran <ste...@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 <t...@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.w...@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 <jayunit...@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 <
> ste...@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.
>
