From 17adc0b168fa09df71b882d46773ff6f642a70be Mon Sep 17 00:00:00 2001
From: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
Date: Fri, 3 Jun 2016 09:13:11 +0100
Subject: [PATCH] prevent OOME on big streams and better logging for that case

---
 .../brooklyn/core/mgmt/BrooklynTaskTags.java   |  9 ++++++++-
 .../brooklyn/logback-logger-excludes.xml       |  8 +++++++-
 .../rest/util/DefaultExceptionMapper.java      | 18 +++++++++++++++---
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
index 0ce9a2ec09..58018b92b7 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/BrooklynTaskTags.java
@@ -190,9 +190,16 @@ protected WrappedStream(String streamType, ByteArrayOutputStream stream) {
         public Integer getStreamSize() {
             return streamSize.get();
         }
-        // there is a stream api so don't return everything unless explicitly requested!
+        // there is a stream call on Activity REST api which accesses streamContent.get() directly;
+        // so when serializing the tag, abbreviate things
         @JsonProperty("streamContents")
         public String getStreamContentsAbbreviated() {
+            // TODO would be nice to just get the first 80 chars but that's a refactoring
+            // which might affect persistence.  if stream is very large (100MB+) then we sometimes
+            // get OOME without it, so let's abbreviate
+            if (streamSize.get()>8192) {
+                return "<contents-too-large>";
+            }
             return Strings.maxlenWithEllipsis(streamContents.get(), 80);
         }
         @Override
diff --git a/logging/logback-includes/src/main/resources/brooklyn/logback-logger-excludes.xml b/logging/logback-includes/src/main/resources/brooklyn/logback-logger-excludes.xml
index 0b3824cf21..66fff3c882 100644
--- a/logging/logback-includes/src/main/resources/brooklyn/logback-logger-excludes.xml
+++ b/logging/logback-includes/src/main/resources/brooklyn/logback-logger-excludes.xml
@@ -61,6 +61,12 @@
         <appender-ref ref="FILE" />
     </logger>
 
-    <logger name="org.apache.cxf" level="WARN"/>
+    <!-- CXF logging is extremely verbose, including 
+         debug logging for every REST in and out,
+         and warns and errors which we catch and handle;
+         it might be nice to keep the latter, but they show up in info log (no way to say debug file only?),
+         so simplest is to disable all but severe -->
+    <logger name="org.apache.cxf" level="SEVERE"/>
+    
     <logger name="io.cloudsoft.winrm4j.winrm.WinRmTool" level="DEBUG"/>
 </included>
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 6797a64578..dffc143f1e 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
@@ -66,13 +66,18 @@ public Response toResponse(Throwable throwable1) {
             return null;
         }
 
-        LOG.debug("REST request running as {} threw: {}", Entitlements.getEntitlementContext(), 
-            Exceptions.collapse(throwable1));
+        Throwable throwable2 = Exceptions.getFirstInteresting(throwable1);
+        if (isSevere(throwable2)) {
+            LOG.warn("REST request running as {} threw: {}", Entitlements.getEntitlementContext(), 
+                Exceptions.collapse(throwable1));
+        } else {
+            LOG.debug("REST request running as {} threw: {}", Entitlements.getEntitlementContext(), 
+                Exceptions.collapse(throwable1));            
+        }
         if (LOG.isTraceEnabled()) {
             LOG.trace("Full details of "+Entitlements.getEntitlementContext()+" "+throwable1, throwable1);
         }
 
-        Throwable throwable2 = Exceptions.getFirstInteresting(throwable1);
         // Some methods will throw this, which gets converted automatically
         if (throwable2 instanceof WebApplicationException) {
             WebApplicationException wae = (WebApplicationException) throwable2;
@@ -114,4 +119,11 @@ public Response toResponse(Throwable throwable1) {
             rb.message("Internal error. Contact server administrator to consult logs for more details.");
         return rb.build().asResponse(Status.INTERNAL_SERVER_ERROR, MediaType.APPLICATION_JSON_TYPE);
     }
+    
+    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)
+        if (t instanceof OutOfMemoryError) return true;
+        return false;
+    }
 }