Mailing-List: contact common-dev-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-dev@hadoop.apache.org
Received-SPF: pass (athena.apache.org: domain of tucu@cloudera.com designates
 209.85.160.53 as permitted sender)
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable
Subject: Re: DISCUSS: use SLF4J APIs in new modules?
References: 
 <CA+4kjVv_9cMmTDQZCgZy-CHSLyB1WkGdUNXS7wrhesLwbuHJjQ@mail.gmail.com>
 <F66C0E69-4820-4B14-8072-127E5EE56F0B@gmail.com>
 <CAGB5D2YanB2NjFH8CHNm=X5P2_c1omF5ADcSiW2FrTLpLZFEfA@mail.gmail.com>
 <D9ED7860-6B1A-4503-82ED-4642E6C0302A@cloudera.com>
 <CALwhT96orv_zjPXhxtr4_V8FaUyrJHa1_X_hnLwf=nDudQGwBQ@mail.gmail.com>
 <CA+4kjVvnu6=Wv6iTmAyxdnAY8=E4rRzeCKt+sjY1SS4bHYeAMg@mail.gmail.com>
 <CALwhT94GPN4mEKJwDYK9f5LZ3bHs+PiyGS78_Ax0vLM0SoTz5g@mail.gmail.com>
From: Alejandro Abdelnur <tucu@cloudera.com>
In-Reply-To: 
 <CALwhT94GPN4mEKJwDYK9f5LZ3bHs+PiyGS78_Ax0vLM0SoTz5g@mail.gmail.com>
Message-Id: <85445587-F151-4DBD-B6E6-2B507BCA18E3@cloudera.com>
Date: Fri, 11 Apr 2014 10:37:11 -0700
To: "common-dev@hadoop.apache.org" <common-dev@hadoop.apache.org>
Mime-Version: 1.0 (1.0)

if you dont convert mgs to templates the dont remove the guards, else you cr=
eate str mgs obj even when not logging.=20

thx

Alejandro
(phone typing)

> On Apr 11, 2014, at 10:06, Karthik Kambatla <kasha@cloudera.com> wrote:
>=20
> On Fri, Apr 11, 2014 at 1:37 AM, Steve Loughran <stevel@hortonworks.com>wr=
ote:
>=20
>> On 10 April 2014 16:38, Karthik Kambatla <kasha@cloudera.com> wrote:
>>=20
>>> +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.
>>=20
>>=20
>> #1  appeals to me.
>>=20
>> #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
>=20
> 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.
>=20
>=20
>>=20
>> or do both at the same time, one class or package at time.
>>=20
>>=20
>> Having a consistent log scheme across all classes makes copying and movin=
g
>> code easier, especially copy+paste
>>=20
>> 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 t=
o
>> add an SLF4J logger equivalent.
>>=20
>> -Steve
>>=20
>>=20
>>>=20
>>> On Thu, Apr 10, 2014 at 8:17 AM, Alejandro Abdelnur <tucu@cloudera.com
>>>> wrote:
>>>=20
>>>> +1 pn slf4j.
>>>>=20
>>>> one thing Jay, the issues with log4j will still be there as log4j will
>>>> still be under the hood.
>>>>=20
>>>> thx
>>>>=20
>>>> Alejandro
>>>> (phone typing)
>>>>=20
>>>>> On Apr 10, 2014, at 7:35, Andrew Wang <andrew.wang@cloudera.com>
>>> wrote:
>>>>>=20
>>>>> +1 from me, it'd be lovely to get rid of all those isDebugEnabled
>>> checks.
>>>>>=20
>>>>>=20
>>>>>> On Thu, Apr 10, 2014 at 4:13 AM, Jay Vyas <jayunit100@gmail.com>
>>> wrote:
>>>>>>=20
>>>>>> Slf4j is definetly a great step forward.  Log4j is restrictive for
>>>> complex
>>>>>> and multi tenant apps like hadoop.
>>>>>>=20
>>>>>> 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.
>>>>>>=20
>>>>>>>> On Apr 10, 2014, at 2:16 AM, Steve Loughran <
>> stevel@hortonworks.com
>>>>=20
>>>>>>> wrote:
>>>>>>>=20
>>>>>>> If we're thinking of future progress, here's a little low-level
>> one:
>>>>>> adopt
>>>>>>> SLF4J as the API for logging
>>>>>>>=20
>>>>>>>=20
>>>>>>> 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()
>>>>>>>=20
>>>>>>>=20
>>>>>>> The best initial thing about SL4FJ is how it only expands its
>>> arguments
>>>>>>> string values if needed
>>>>>>>=20
>>>>>>>    LOG.debug("Initialized, principal [{}] from keytab [{}]",
>>>> principal,
>>>>>>> keytab);
>>>>>>>=20
>>>>>>> not logging at debug? No need to test first. That alone saves code
>>> and
>>>>>>> improves readability.
>>>>>>>=20
>>>>>>> The slf4 expansion code handles null values as well as calling
>>>> toString()
>>>>>>> on non-null arguments. Oh and it does arrays too.
>>>>>>>=20
>>>>>>> int array =3D [1, 2, 3];
>>>>>>> String undef =3D null;
>>>>>>>=20
>>>>>>> LOG.info("a =3D {}, u =3D {}", array, undef)  -> "a =3D [1, 2, 3],  u=
 =3D
>>> null"
>>>>>>>=20
>>>>>>> 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.
>>>>>>>=20
>>>>>>> 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"?
>>>>>>>=20
>>>>>>> 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.
>>>>>>>=20
>>>>>>> -Stevve
>>>>>>>=20
>>>>>>> --
>>>>>>> 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.
>>=20
>> --
>> CONFIDENTIALITY NOTICE
>> NOTICE: This message is intended for the use of the individual or entity t=
o
>> 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 th=
at
>> 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 immediate=
ly
>> and delete it from your system. Thank You.
>>=20