You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by ni...@apache.org on 2012/03/16 15:38:18 UTC

svn commit: r1301549 - in /cxf/branches/2.5.x-fixes: ./ common/common/src/main/java/org/apache/cxf/common/logging/LogUtils.java

Author: ningjiang
Date: Fri Mar 16 14:38:18 2012
New Revision: 1301549

URL: http://svn.apache.org/viewvc?rev=1301549&view=rev
Log:
Merged revisions 1301539 via svnmerge from 
https://svn.apache.org/repos/asf/cxf/trunk

........
  r1301539 | ningjiang | 2012-03-16 22:18:01 +0800 (Fri, 16 Mar 2012) | 1 line
  
  CXF-4180 LogUtils should default to CXF's Slf4jLogger if it can't find Log4J or JCL driver
........

Modified:
    cxf/branches/2.5.x-fixes/   (props changed)
    cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/common/logging/LogUtils.java

Propchange: cxf/branches/2.5.x-fixes/
------------------------------------------------------------------------------
    svn:mergeinfo = /cxf/trunk:1301539

Propchange: cxf/branches/2.5.x-fixes/
------------------------------------------------------------------------------
Binary property 'svnmerge-integrated' - no diff available.

Modified: cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/common/logging/LogUtils.java
URL: http://svn.apache.org/viewvc/cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/common/logging/LogUtils.java?rev=1301549&r1=1301548&r2=1301549&view=diff
==============================================================================
--- cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/common/logging/LogUtils.java (original)
+++ cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/common/logging/LogUtils.java Fri Mar 16 14:38:18 2012
@@ -97,17 +97,8 @@ public final class LogUtils {
             }
             if (StringUtils.isEmpty(cname)) {
                 Class.forName("org.slf4j.impl.StaticLoggerBinder");
-                Class<?> cls = Class.forName("org.slf4j.LoggerFactory");
-                Class<?> fcls = cls.getMethod("getILoggerFactory").invoke(null).getClass();
-                if (fcls.getName().contains("Log4j")) {
-                    cname = "org.apache.cxf.common.logging.Log4jLogger";
-                } else if (fcls.getName().contains("JCL")) {
-                    cls = Class.forName("org.apache.commons.logging.LogFactory");
-                    fcls = cls.getMethod("getFactory").invoke(null).getClass();
-                    if (fcls.getName().contains("Log4j")) {
-                        cname = "org.apache.cxf.common.logging.Log4jLogger";
-                    }
-                }
+                // using the Slf4jLogger directly
+                cname = "org.apache.cxf.common.logging.Slf4jLogger";
             }
             if (!StringUtils.isEmpty(cname)) {
                 try {



Re: svn commit: r1301549 - in /cxf/branches/2.5.x-fixes: ./ common/common/src/main/java/org/apache/cxf/common/logging/LogUtils.java

Posted by Christian Schneider <ch...@die-schneider.net>.
Btw.

Class.forName("org.slf4j.impl.StaticLoggerBinder");

does not work. Jetty also does that to check if slf4j is available. The problem is that pax logging does not even export this package.

So my proposal would be to completely switch to slf4j and not test for anything in the setup. If people want j.u.l. then they can use the slf4j adapter for it. That would bring our code in line with camel and karaf.

Another question is of course when to do such a change. I am not sure if we should switch right now.

Christian



Am 16.03.2012 15:48, schrieb Daniel Kulp:
>
> I'm -1 for this change for 2 reasons:
>
> 1) This breaks existing logging configuration and thus cannot be pulled back
> onto the other branches.   Users may already have j.u.l configs that would
> no longer work.
>
> 2) Performance issues - the Slf4jLogger does a LOT more work than the j.u.l
> loggers and thus we should only use this as a last resort, not the default
> which this change pretty much entails.
>
> Dan
>
>
> On Friday, March 16, 2012 02:38:18 PM ningjiang@apache.org wrote:
>> Author: ningjiang
>> Date: Fri Mar 16 14:38:18 2012
>> New Revision: 1301549
>>
>> URL: http://svn.apache.org/viewvc?rev=1301549&view=rev
>> Log:
>> Merged revisions 1301539 via svnmerge from
>> https://svn.apache.org/repos/asf/cxf/trunk
>>
>> ........
>>    r1301539 | ningjiang | 2012-03-16 22:18:01 +0800 (Fri, 16 Mar 2012) | 1
>> line
>>
>>    CXF-4180 LogUtils should default to CXF's Slf4jLogger if it can't find
>> Log4J or JCL driver ........
>>
>> Modified:
>>      cxf/branches/2.5.x-fixes/   (props changed)
>>
>> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/commo
>> n/logging/LogUtils.java
>>
>> Propchange: cxf/branches/2.5.x-fixes/
>> --------------------------------------------------------------------------
>> ---- svn:mergeinfo = /cxf/trunk:1301539
>>
>> Propchange: cxf/branches/2.5.x-fixes/
>> --------------------------------------------------------------------------
>> ---- Binary property 'svnmerge-integrated' - no diff available.
>>
>> Modified:
>> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/commo
>> n/logging/LogUtils.java URL:
>> http://svn.apache.org/viewvc/cxf/branches/2.5.x-fixes/common/common/src/m
>> ain/java/org/apache/cxf/common/logging/LogUtils.java?rev=1301549&r1=130154
>> 8&r2=1301549&view=diff
>> =========================================================================
>> ===== ---
>> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/commo
>> n/logging/LogUtils.java (original) +++
>> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/commo
>> n/logging/LogUtils.java Fri Mar 16 14:38:18 2012 @@ -97,17 +97,8 @@ public
>> final class LogUtils {
>>               }
>>               if (StringUtils.isEmpty(cname)) {
>>                   Class.forName("org.slf4j.impl.StaticLoggerBinder");
>> -                Class<?>  cls = Class.forName("org.slf4j.LoggerFactory");
>> -                Class<?>  fcls =
>> cls.getMethod("getILoggerFactory").invoke(null).getClass(); -
>>     if (fcls.getName().contains("Log4j")) {
>> -                    cname = "org.apache.cxf.common.logging.Log4jLogger";
>> -                } else if (fcls.getName().contains("JCL")) {
>> -                    cls =
>> Class.forName("org.apache.commons.logging.LogFactory"); -
>>     fcls = cls.getMethod("getFactory").invoke(null).getClass(); -
>>            if (fcls.getName().contains("Log4j")) {
>> -                        cname =
>> "org.apache.cxf.common.logging.Log4jLogger"; -                    }
>> -                }
>> +                // using the Slf4jLogger directly
>> +                cname = "org.apache.cxf.common.logging.Slf4jLogger";
>>               }
>>               if (!StringUtils.isEmpty(cname)) {
>>                   try {


-- 
Christian Schneider
http://www.liquid-reality.de

Open Source Architect
Talend Application Integration Division http://www.talend.com


Re: svn commit: r1301549 - in /cxf/branches/2.5.x-fixes: ./ common/common/src/main/java/org/apache/cxf/common/logging/LogUtils.java

Posted by Daniel Kulp <dk...@apache.org>.
On Friday, March 16, 2012 11:59:02 PM Willem Jiang wrote:
> Sorry, I didn't realized the change has so many side effects.
> I will revert the back port patches and find a better solutioni on the
> trunk.

Can you try the patch below?
That should keep the detection in place but adds the j.u.l detection, but 
fall back to slf4j if detection fails.   On the back ported branch, remove 
the final "else".

This works in OSGi as well as it removes the "impl" forName and checks for 
the pax.logging stuff.


Dan



diff --git a/api/src/main/java/org/apache/cxf/common/logging/LogUtils.java 
b/api/src/main/java/org/apache/cxf/common/logging/LogUtils.java
index 1842ad4..b04b857 100644
--- a/api/src/main/java/org/apache/cxf/common/logging/LogUtils.java
+++ b/api/src/main/java/org/apache/cxf/common/logging/LogUtils.java
@@ -35,7 +35,6 @@ import java.util.logging.Logger;
 
 import org.apache.cxf.common.i18n.BundleUtils;
 import org.apache.cxf.common.util.StringUtils;
-import org.apache.log4j.spi.LoggerFactory;
 
 
 /**
@@ -97,9 +96,31 @@ public final class LogUtils {
                 }
             }
             if (StringUtils.isEmpty(cname)) {
-                Class.forName("org.slf4j.impl.StaticLoggerBinder");
-                // using the Slf4jLogger directly
-                cname = "org.apache.cxf.common.logging.Slf4jLogger";
+                try {
+                    Class<?> cls = 
Class.forName("org.slf4j.LoggerFactory");
+                    Class<?> fcls = 
cls.getMethod("getILoggerFactory").invoke(null).getClass();
+                    String clsName = fcls.getName();
+                    if (clsName.contains("Log4j")) {
+                        cname = 
"org.apache.cxf.common.logging.Log4jLogger";
+                    } else if (clsName.contains("JCL")) {
+                        cls = 
Class.forName("org.apache.commons.logging.LogFactory");
+                        fcls = 
cls.getMethod("getFactory").invoke(null).getClass();
+                        if (fcls.getName().contains("Log4j")) {
+                            cname = 
"org.apache.cxf.common.logging.Log4jLogger";
+                        }
+                    } else if (clsName.contains("JDK14")
+                        || clsName.contains("pax.logging")) {
+                        //both of these we can use the appropriate j.u.l 
API's 
+                        //directly and have it work properly
+                        cname = null;
+                    } else {
+                        // Cannot really detect where it's logging so we'll
+                        // go ahead and use the Slf4jLogger directly
+                        cname = 
"org.apache.cxf.common.logging.Slf4jLogger";
+                    }
+                } catch (Throwable t) {
+                    //ignore - Slf4j not available
+                }
             }
             if (!StringUtils.isEmpty(cname)) {
                 try {



> 
> On Fri Mar 16 22:48:26 2012, Daniel Kulp wrote:
> > I'm -1 for this change for 2 reasons:
> > 
> > 1) This breaks existing logging configuration and thus cannot be pulled
> > back onto the other branches.   Users may already have j.u.l configs
> > that would no longer work.
> > 
> > 2) Performance issues - the Slf4jLogger does a LOT more work than the
> > j.u.l loggers and thus we should only use this as a last resort, not
> > the default which this change pretty much entails.
> > 
> > Dan
> > 
> > On Friday, March 16, 2012 02:38:18 PM ningjiang@apache.org wrote:
> >> Author: ningjiang
> >> Date: Fri Mar 16 14:38:18 2012
> >> New Revision: 1301549
> >> 
> >> URL: http://svn.apache.org/viewvc?rev=1301549&view=rev
> >> Log:
> >> Merged revisions 1301539 via svnmerge from
> >> https://svn.apache.org/repos/asf/cxf/trunk
> >> 
> >> ........
> >> 
> >>    r1301539 | ningjiang | 2012-03-16 22:18:01 +0800 (Fri, 16 Mar 2012)
> >>    | 1
> >> 
> >> line
> >> 
> >>    CXF-4180 LogUtils should default to CXF's Slf4jLogger if it can't
> >>    find
> >> 
> >> Log4J or JCL driver ........
> >> 
> >> Modified:
> >>      cxf/branches/2.5.x-fixes/   (props changed)
> >> 
> >> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/com
> >> mo
> >> n/logging/LogUtils.java
> >> 
> >> Propchange: cxf/branches/2.5.x-fixes/
> >> -----------------------------------------------------------------------
> >> --- ---- svn:mergeinfo = /cxf/trunk:1301539
> >> 
> >> Propchange: cxf/branches/2.5.x-fixes/
> >> -----------------------------------------------------------------------
> >> --- ---- Binary property 'svnmerge-integrated' - no diff available.
> >> 
> >> Modified:
> >> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/com
> >> mo
> >> n/logging/LogUtils.java URL:
> >> http://svn.apache.org/viewvc/cxf/branches/2.5.x-fixes/common/common/src
> >> /m
> >> ain/java/org/apache/cxf/common/logging/LogUtils.java?rev=1301549&r1=130
> >> 154 8&r2=1301549&view=diff
> >> =======================================================================
> >> ==
> >> ===== ---
> >> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/com
> >> mo
> >> n/logging/LogUtils.java (original) +++
> >> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/com
> >> mo
> >> n/logging/LogUtils.java Fri Mar 16 14:38:18 2012 @@ -97,17 +97,8 @@
> >> public final class LogUtils {
> >> 
> >>               }
> >>               if (StringUtils.isEmpty(cname)) {
> >>               
> >>                   Class.forName("org.slf4j.impl.StaticLoggerBinder");
> >> 
> >> -                Class<?>  cls =
> >> Class.forName("org.slf4j.LoggerFactory"); -                Class<?> 
> >> fcls =
> >> cls.getMethod("getILoggerFactory").invoke(null).getClass(); -
> >> 
> >>     if (fcls.getName().contains("Log4j")) {
> >> 
> >> -                    cname =
> >> "org.apache.cxf.common.logging.Log4jLogger";
> >> -                } else if (fcls.getName().contains("JCL")) {
> >> -                    cls =
> >> Class.forName("org.apache.commons.logging.LogFactory"); -
> >> 
> >>     fcls = cls.getMethod("getFactory").invoke(null).getClass(); -
> >>     
> >>            if (fcls.getName().contains("Log4j")) {
> >> 
> >> -                        cname =
> >> "org.apache.cxf.common.logging.Log4jLogger"; -                    }
> >> -                }
> >> +                // using the Slf4jLogger directly
> >> +                cname = "org.apache.cxf.common.logging.Slf4jLogger";
> >> 
> >>               }
> >>               if (!StringUtils.isEmpty(cname)) {
> >>               
> >>                   try {
-- 
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com


Re: svn commit: r1301549 - in /cxf/branches/2.5.x-fixes: ./ common/common/src/main/java/org/apache/cxf/common/logging/LogUtils.java

Posted by Willem Jiang <wi...@gmail.com>.
Sorry, I didn't realized the change has so many side effects.
I will revert the back port patches and find a better solutioni on the 
trunk.

On Fri Mar 16 22:48:26 2012, Daniel Kulp wrote:
>
>
> I'm -1 for this change for 2 reasons:
>
> 1) This breaks existing logging configuration and thus cannot be pulled back
> onto the other branches.   Users may already have j.u.l configs that would
> no longer work.
>
> 2) Performance issues - the Slf4jLogger does a LOT more work than the j.u.l
> loggers and thus we should only use this as a last resort, not the default
> which this change pretty much entails.
>
> Dan
>
>
> On Friday, March 16, 2012 02:38:18 PM ningjiang@apache.org wrote:
>> Author: ningjiang
>> Date: Fri Mar 16 14:38:18 2012
>> New Revision: 1301549
>>
>> URL: http://svn.apache.org/viewvc?rev=1301549&view=rev
>> Log:
>> Merged revisions 1301539 via svnmerge from
>> https://svn.apache.org/repos/asf/cxf/trunk
>>
>> ........
>>    r1301539 | ningjiang | 2012-03-16 22:18:01 +0800 (Fri, 16 Mar 2012) | 1
>> line
>>
>>    CXF-4180 LogUtils should default to CXF's Slf4jLogger if it can't find
>> Log4J or JCL driver ........
>>
>> Modified:
>>      cxf/branches/2.5.x-fixes/   (props changed)
>>
>> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/commo
>> n/logging/LogUtils.java
>>
>> Propchange: cxf/branches/2.5.x-fixes/
>> --------------------------------------------------------------------------
>> ---- svn:mergeinfo = /cxf/trunk:1301539
>>
>> Propchange: cxf/branches/2.5.x-fixes/
>> --------------------------------------------------------------------------
>> ---- Binary property 'svnmerge-integrated' - no diff available.
>>
>> Modified:
>> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/commo
>> n/logging/LogUtils.java URL:
>> http://svn.apache.org/viewvc/cxf/branches/2.5.x-fixes/common/common/src/m
>> ain/java/org/apache/cxf/common/logging/LogUtils.java?rev=1301549&r1=130154
>> 8&r2=1301549&view=diff
>> =========================================================================
>> ===== ---
>> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/commo
>> n/logging/LogUtils.java (original) +++
>> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/commo
>> n/logging/LogUtils.java Fri Mar 16 14:38:18 2012 @@ -97,17 +97,8 @@ public
>> final class LogUtils {
>>               }
>>               if (StringUtils.isEmpty(cname)) {
>>                   Class.forName("org.slf4j.impl.StaticLoggerBinder");
>> -                Class<?>  cls = Class.forName("org.slf4j.LoggerFactory");
>> -                Class<?>  fcls =
>> cls.getMethod("getILoggerFactory").invoke(null).getClass(); -
>>     if (fcls.getName().contains("Log4j")) {
>> -                    cname = "org.apache.cxf.common.logging.Log4jLogger";
>> -                } else if (fcls.getName().contains("JCL")) {
>> -                    cls =
>> Class.forName("org.apache.commons.logging.LogFactory"); -
>>     fcls = cls.getMethod("getFactory").invoke(null).getClass(); -
>>            if (fcls.getName().contains("Log4j")) {
>> -                        cname =
>> "org.apache.cxf.common.logging.Log4jLogger"; -                    }
>> -                }
>> +                // using the Slf4jLogger directly
>> +                cname = "org.apache.cxf.common.logging.Slf4jLogger";
>>               }
>>               if (!StringUtils.isEmpty(cname)) {
>>                   try {



-- 
Willem
----------------------------------
FuseSource
Web: http://www.fusesource.com
Blog:    http://willemjiang.blogspot.com (English)
         http://jnn.javaeye.com (Chinese)
Twitter: willemjiang 
Weibo: willemjiang 


Re: svn commit: r1301549 - in /cxf/branches/2.5.x-fixes: ./ common/common/src/main/java/org/apache/cxf/common/logging/LogUtils.java

Posted by Daniel Kulp <dk...@apache.org>.

I'm -1 for this change for 2 reasons:

1) This breaks existing logging configuration and thus cannot be pulled back 
onto the other branches.   Users may already have j.u.l configs that would 
no longer work.

2) Performance issues - the Slf4jLogger does a LOT more work than the j.u.l 
loggers and thus we should only use this as a last resort, not the default 
which this change pretty much entails.   

Dan


On Friday, March 16, 2012 02:38:18 PM ningjiang@apache.org wrote:
> Author: ningjiang
> Date: Fri Mar 16 14:38:18 2012
> New Revision: 1301549
> 
> URL: http://svn.apache.org/viewvc?rev=1301549&view=rev
> Log:
> Merged revisions 1301539 via svnmerge from
> https://svn.apache.org/repos/asf/cxf/trunk
> 
> ........
>   r1301539 | ningjiang | 2012-03-16 22:18:01 +0800 (Fri, 16 Mar 2012) | 1
> line
> 
>   CXF-4180 LogUtils should default to CXF's Slf4jLogger if it can't find
> Log4J or JCL driver ........
> 
> Modified:
>     cxf/branches/2.5.x-fixes/   (props changed)
>    
> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/commo
> n/logging/LogUtils.java
> 
> Propchange: cxf/branches/2.5.x-fixes/
> --------------------------------------------------------------------------
> ---- svn:mergeinfo = /cxf/trunk:1301539
> 
> Propchange: cxf/branches/2.5.x-fixes/
> --------------------------------------------------------------------------
> ---- Binary property 'svnmerge-integrated' - no diff available.
> 
> Modified:
> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/commo
> n/logging/LogUtils.java URL:
> http://svn.apache.org/viewvc/cxf/branches/2.5.x-fixes/common/common/src/m
> ain/java/org/apache/cxf/common/logging/LogUtils.java?rev=1301549&r1=130154
> 8&r2=1301549&view=diff
> =========================================================================
> ===== ---
> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/commo
> n/logging/LogUtils.java (original) +++
> cxf/branches/2.5.x-fixes/common/common/src/main/java/org/apache/cxf/commo
> n/logging/LogUtils.java Fri Mar 16 14:38:18 2012 @@ -97,17 +97,8 @@ public
> final class LogUtils {
>              }
>              if (StringUtils.isEmpty(cname)) {
>                  Class.forName("org.slf4j.impl.StaticLoggerBinder");
> -                Class<?> cls = Class.forName("org.slf4j.LoggerFactory");
> -                Class<?> fcls =
> cls.getMethod("getILoggerFactory").invoke(null).getClass(); -            
>    if (fcls.getName().contains("Log4j")) {
> -                    cname = "org.apache.cxf.common.logging.Log4jLogger";
> -                } else if (fcls.getName().contains("JCL")) {
> -                    cls =
> Class.forName("org.apache.commons.logging.LogFactory"); -                
>    fcls = cls.getMethod("getFactory").invoke(null).getClass(); -         
>           if (fcls.getName().contains("Log4j")) {
> -                        cname =
> "org.apache.cxf.common.logging.Log4jLogger"; -                    }
> -                }
> +                // using the Slf4jLogger directly
> +                cname = "org.apache.cxf.common.logging.Slf4jLogger";
>              }
>              if (!StringUtils.isEmpty(cname)) {
>                  try {
-- 
Daniel Kulp
dkulp@apache.org - http://dankulp.com/blog
Talend Community Coder - http://coders.talend.com