brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From geom...@apache.org
Subject [1/2] brooklyn-server git commit: better logging of REST exceptions
Date Fri, 28 Apr 2017 08:18:51 GMT
Repository: brooklyn-server
Updated Branches:
  refs/heads/master 01141a722 -> d05b9fd81


better logging of REST exceptions

include the trace on the first encounter of a new type or simply a new place where a type
is thrown from;
subsequent instances won't get the trace but handy when debugging if the first one does


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/5571b0de
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/5571b0de
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/5571b0de

Branch: refs/heads/master
Commit: 5571b0de2c62a81acfed95eabede3ec82e2e2caf
Parents: 547feec
Author: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Authored: Mon Apr 24 15:21:16 2017 +0100
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Committed: Mon Apr 24 15:26:55 2017 +0100

----------------------------------------------------------------------
 .../resources/AbstractBrooklynRestResource.java |  9 +++++
 .../rest/resources/CatalogResource.java         |  3 +-
 .../rest/util/DefaultExceptionMapper.java       | 39 +++++++++++++++++---
 3 files changed, 44 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/5571b0de/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java
----------------------------------------------------------------------
diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java
b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java
index 082a620..3f33643 100644
--- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java
+++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/AbstractBrooklynRestResource.java
@@ -20,6 +20,7 @@ package org.apache.brooklyn.rest.resources;
 
 import javax.annotation.Nullable;
 import javax.ws.rs.core.Context;
+import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriInfo;
 import javax.ws.rs.ext.ContextResolver;
 
@@ -27,7 +28,9 @@ import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.core.config.render.RendererHints;
 import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.rest.domain.ApiError;
 import org.apache.brooklyn.rest.util.BrooklynRestResourceUtils;
+import org.apache.brooklyn.rest.util.DefaultExceptionMapper;
 import org.apache.brooklyn.rest.util.ManagementContextProvider;
 import org.apache.brooklyn.rest.util.WebResourceUtils;
 import org.apache.brooklyn.rest.util.json.BrooklynJacksonJsonProvider;
@@ -72,6 +75,12 @@ public abstract class AbstractBrooklynRestResource {
         return brooklynRestResourceUtils;
     }
     
+    /** returns a bad request Response wrapping the given exception */
+    protected Response badRequest(Exception e) {
+        DefaultExceptionMapper.logExceptionDetailsForDebugging(e);
+        return ApiError.of(e).asBadRequestResponseJson();
+    }
+    
     protected ObjectMapper mapper() {
         return mapper(mgmt());
     }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/5571b0de/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
----------------------------------------------------------------------
diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
index c2e800a..8c04aef 100644
--- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
+++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java
@@ -64,6 +64,7 @@ import org.apache.brooklyn.rest.domain.CatalogLocationSummary;
 import org.apache.brooklyn.rest.domain.CatalogPolicySummary;
 import org.apache.brooklyn.rest.filter.HaHotStateRequired;
 import org.apache.brooklyn.rest.transform.CatalogTransformer;
+import org.apache.brooklyn.rest.util.DefaultExceptionMapper;
 import org.apache.brooklyn.rest.util.WebResourceUtils;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
@@ -156,7 +157,7 @@ public class CatalogResource extends AbstractBrooklynRestResource implements
Cat
             return buildCreateResponse(items);
         } catch (Exception e) {
             Exceptions.propagateIfFatal(e);
-            return ApiError.of(e).asBadRequestResponseJson();
+            return badRequest(e);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/5571b0de/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java
----------------------------------------------------------------------
diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java
b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java
index 010bcca..b2e640e 100644
--- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java
+++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/util/DefaultExceptionMapper.java
@@ -39,12 +39,15 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.yaml.snakeyaml.error.YAMLException;
 
+import com.google.common.annotations.Beta;
+
 @Provider
 public class DefaultExceptionMapper implements ExceptionMapper<Throwable> {
 
     private static final Logger LOG = LoggerFactory.getLogger(DefaultExceptionMapper.class);
 
-    static Set<Class<?>> warnedUnknownExceptions = MutableSet.of();
+    static Set<Class<?>> encounteredUnknownExceptions = MutableSet.of();
+    static Set<Object> encounteredExceptionRecords = MutableSet.of();
     
     /**
      * Maps a throwable to a response.
@@ -60,7 +63,7 @@ public class DefaultExceptionMapper implements ExceptionMapper<Throwable>
{
         // Don't depend on jetty, could be running in other environments as well.
         if (throwable1.getClass().getName().equals("org.eclipse.jetty.io.EofException"))
{
             if (LOG.isTraceEnabled()) {
-                LOG.trace("REST request running as {} threw: {}", Entitlements.getEntitlementContext(),

+                LOG.trace("REST request running as {} was disconnected, threw: {}", Entitlements.getEntitlementContext(),

                         Exceptions.collapse(throwable1));
             }
             return null;
@@ -74,9 +77,7 @@ public class DefaultExceptionMapper implements ExceptionMapper<Throwable>
{
             LOG.debug("REST request running as {} threw: {}", Entitlements.getEntitlementContext(),

                 Exceptions.collapse(throwable1));            
         }
-        if (LOG.isTraceEnabled()) {
-            LOG.trace("Full details of "+Entitlements.getEntitlementContext()+" "+throwable1,
throwable1);
-        }
+        logExceptionDetailsForDebugging(throwable1);
 
         // Some methods will throw this, which gets converted automatically
         if (throwable2 instanceof WebApplicationException) {
@@ -103,7 +104,7 @@ public class DefaultExceptionMapper implements ExceptionMapper<Throwable>
{
         }
 
         if (!Exceptions.isPrefixBoring(throwable2)) {
-            if ( warnedUnknownExceptions.add( throwable2.getClass() )) {
+            if ( encounteredUnknownExceptions.add( throwable2.getClass() )) {
                 LOG.warn("REST call generated exception type "+throwable2.getClass()+" unrecognized
in "+getClass()+" (subsequent occurrences will be logged debug only): " + throwable2, throwable2);
             }
         }
@@ -120,6 +121,32 @@ public class DefaultExceptionMapper implements ExceptionMapper<Throwable>
{
         return rb.build().asResponse(Status.INTERNAL_SERVER_ERROR, MediaType.APPLICATION_JSON_TYPE);
     }
     
+    /** Logs full details at trace, unless it is the first time we've seen this in which
case it is debug */
+    @Beta
+    public static void logExceptionDetailsForDebugging(Throwable t) {
+        if (LOG.isDebugEnabled()) {
+            if (firstEncounter(t)) {
+                // include debug trace for everything the first time we get one
+                LOG.debug("Full details of "+Entitlements.getEntitlementContext()+" "+t+"
(logging debug on first encounter; subsequent instances will be logged trace)", t);
+            } else if (LOG.isTraceEnabled()) {
+                LOG.trace("Full details of "+Entitlements.getEntitlementContext()+" "+t,
t);
+            }
+        }
+    }
+        
+    private static boolean firstEncounter(Throwable t) {
+        Set<Object> record = MutableSet.of();
+        while (true) {
+            record.add(t.getClass());
+            if (t.getStackTrace().length>0) {
+                if (record.add(t.getStackTrace()[0])) break;
+            }
+            t = t.getCause();
+            if (t==null) break;
+        }
+        return encounteredExceptionRecords.add(record);
+    }
+
     protected boolean isSevere(Throwable t) {
         // some things, like this, we want more prominent server notice of
         // (the list could be much larger but this is a start)


Mime
View raw message