+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. >