You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by md...@apache.org on 2013/06/13 10:23:02 UTC

svn commit: r1492561 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java

Author: mduerig
Date: Thu Jun 13 08:23:02 2013
New Revision: 1492561

URL: http://svn.apache.org/r1492561
Log:
OAK-144 Implement Observation
log stack trace from where an offending listener was registered

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java?rev=1492561&r1=1492560&r2=1492561&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java Thu Jun 13 08:23:02 2013
@@ -18,6 +18,8 @@
  */
 package org.apache.jackrabbit.oak.plugins.observation;
 
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
@@ -60,6 +62,8 @@ class ChangeProcessor implements Runnabl
     private final AtomicReference<EventFilter> filterRef;
     private final AtomicReference<String> userDataRef = new AtomicReference<String>(null);
 
+    private final String initStacktrace;
+
     private volatile boolean running;
     private volatile boolean stopping;
     private ScheduledFuture<?> future;
@@ -75,6 +79,14 @@ class ChangeProcessor implements Runnabl
         this.namePathMapper = observationManager.getNamePathMapper();
         this.listener = listener;
         filterRef = new AtomicReference<EventFilter>(filter);
+        initStacktrace = log.isWarnEnabled(DEPRECATED) ? getStackTrace() : null;
+    }
+
+    private static String getStackTrace() {
+        StringWriter sw = new StringWriter();
+        PrintWriter pw = new PrintWriter(sw);
+        new Exception().printStackTrace(pw);
+        return sw.toString();
     }
 
     public void setFilter(EventFilter filter) {
@@ -152,8 +164,9 @@ class ChangeProcessor implements Runnabl
         if (!userInfoAccessedWithoutExternalsCheck) {
             log.warn(DEPRECATED,
                     "Event listener " + listener + " is trying to access"
-                    + " event user information without checking for whether"
-                    + " the event is external on " + event);
+                    + " event user information on event " + event
+                    + " without checking whether the event is external."
+                    + " The event listener was registered here: " + initStacktrace);
             userInfoAccessedWithoutExternalsCheck = true;
         }
     }
@@ -162,7 +175,8 @@ class ChangeProcessor implements Runnabl
         if (!userInfoAccessedFromExternalEvent) {
             log.warn(DEPRECATED,
                     "Event listener " + listener + " is trying to access"
-                    + " event user information from an external event on " + event);
+                    + " event user information for external event " + event + '.'
+                    + " The event listener was registered here: " + initStacktrace);
             userInfoAccessedFromExternalEvent = true;
         }
     }
@@ -171,8 +185,9 @@ class ChangeProcessor implements Runnabl
         if (!dateAccessedWithoutExternalsCheck) {
             log.warn(DEPRECATED,
                     "Event listener " + listener + " is trying to access"
-                    + " event date information without checking for whether"
-                    + " the event is external on " + event);
+                    + " event date information on event " + event
+                    + " without checking whether the event is external."
+                    + " The event listener was registered here: " + initStacktrace);
             dateAccessedWithoutExternalsCheck = true;
         }
     }
@@ -181,7 +196,8 @@ class ChangeProcessor implements Runnabl
         if (!dateAccessedFromExternalEvent) {
             log.warn(DEPRECATED,
                     "Event listener " + listener + " is trying to access"
-                    + " event date information from an external event on " + event);
+                    + " event date information for external event " + event + '.'
+                    + " The event listener was registered here: " + initStacktrace);
             dateAccessedFromExternalEvent = true;
         }
     }



Re: svn commit: r1492561 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java

Posted by Michael Dürig <md...@apache.org>.

On 13.6.13 11:27, Michael Dürig wrote:
>
>
> Ok agreed. We make it the responsibility of the deployment to have a
> "sensible" logging configuration then. I'll change this later on.

See http://svn.apache.org/r1492631

Michael

Re: svn commit: r1492561 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java

Posted by Michael Dürig <md...@apache.org>.

On 13.6.13 11:19, Jukka Zitting wrote:
> Hi,
>
> On Thu, Jun 13, 2013 at 1:13 PM, Michael Dürig <md...@apache.org> wrote:
>> Right. I guess we need to weight not having the stack trace at all since the
>> default logging configuration doesn't includes it against being able to
>> include additional information through configuration.
>
> Which default configuration are you referring to? Can we change it?
>
> A logging configuration that doesn't include the exception attached
> with a warning log message seems potentially troublesome in any case.

Ok agreed. We make it the responsibility of the deployment to have a 
"sensible" logging configuration then. I'll change this later on.

Michael

Re: svn commit: r1492561 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On Thu, Jun 13, 2013 at 1:13 PM, Michael Dürig <md...@apache.org> wrote:
> Right. I guess we need to weight not having the stack trace at all since the
> default logging configuration doesn't includes it against being able to
> include additional information through configuration.

Which default configuration are you referring to? Can we change it?

A logging configuration that doesn't include the exception attached
with a warning log message seems potentially troublesome in any case.

BR,

Jukka Zitting

Re: svn commit: r1492561 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java

Posted by Michael Dürig <md...@apache.org>.

On 13.6.13 11:07, Jukka Zitting wrote:
> Hi,
>
> On Thu, Jun 13, 2013 at 11:42 AM, Michael Dürig <md...@apache.org> wrote:
>> I considered that but wanted to have the stack trace in the log message
>> regardless of the logger message format configuration.
>
> OK. Note thought that this approach takes away the ability of logging
> frameworks to provide additional context information that the default
> JVM stack trace formatting doesn't provide. For example, see
> http://logback.qos.ch/reasonsToSwitch.html#packagingData.

Right. I guess we need to weight not having the stack trace at all since 
the default logging configuration doesn't includes it against being able 
to include additional information through configuration.

Michael

Re: svn commit: r1492561 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On Thu, Jun 13, 2013 at 11:42 AM, Michael Dürig <md...@apache.org> wrote:
> I considered that but wanted to have the stack trace in the log message
> regardless of the logger message format configuration.

OK. Note thought that this approach takes away the ability of logging
frameworks to provide additional context information that the default
JVM stack trace formatting doesn't provide. For example, see
http://logback.qos.ch/reasonsToSwitch.html#packagingData.

BR,

Jukka Zitting

Re: svn commit: r1492561 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java

Posted by Michael Dürig <md...@apache.org>.
On 13.6.13 9:35, Jukka Zitting wrote:
> A better approach might be to keep an Exception instance for this
> purpose. Logging tools are typically better prepared to handle and
> format a stack trace from an exception instance instead of a long
> multi-line string as the log message.

I considered that but wanted to have the stack trace in the log message 
regardless of the logger message format configuration.

Michael

Re: svn commit: r1492561 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/observation/ChangeProcessor.java

Posted by Jukka Zitting <ju...@gmail.com>.
Hi,

On Thu, Jun 13, 2013 at 11:23 AM,  <md...@apache.org> wrote:
> +    private final String initStacktrace;

A better approach might be to keep an Exception instance for this
purpose. Logging tools are typically better prepared to handle and
format a stack trace from an exception instance instead of a long
multi-line string as the log message.

BR,

Jukka Zitting