Reduce allocations for formatting exception stack traces (#178)

elastic · Apr 26, 2022 · 23d5bd6 · 23d5bd6
1 parent 4a7a6b5
commit 23d5bd6
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 9 deletions.
diff --git a/ecs-logging-core/src/main/java/co/elastic/logging/EcsJsonSerializer.java b/ecs-logging-core/src/main/java/co/elastic/logging/EcsJsonSerializer.java
@@ -28,6 +28,7 @@
 import java.io.Writer;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 public class EcsJsonSerializer {
@@ -212,6 +213,10 @@ public static void serializeException(StringBuilder builder, Throwable thrown, b
     }
 
     public static void serializeException(StringBuilder builder, String exceptionClassName, String exceptionMessage, String stackTrace, boolean stackTraceAsArray) {
+        serializeException(builder, exceptionClassName, exceptionMessage, (CharSequence) stackTrace, stackTraceAsArray);
+    }
+
+    public static void serializeException(StringBuilder builder, String exceptionClassName, CharSequence exceptionMessage, CharSequence stackTrace, boolean stackTraceAsArray) {
         builder.append("\"error.type\":\"");
         JsonUtils.quoteAsString(exceptionClassName, builder);
         builder.append("\",");
@@ -258,16 +263,44 @@ public void println() {
         removeIfEndsWith(jsonBuilder, ",");
     }
 
-    private static void formatStackTraceAsArray(StringBuilder builder, String stackTrace) {
+    private static void formatStackTraceAsArray(StringBuilder builder, CharSequence stackTrace) {
         builder.append(NEW_LINE);
-        for (String line : NEW_LINE_PATTERN.split(stackTrace)) {
-            builder.append("\t\"");
-            JsonUtils.quoteAsString(line, builder);
-            builder.append("\",");
-            builder.append(NEW_LINE);
+
+        // splits the stackTrace by new lines
+        Matcher matcher = NEW_LINE_PATTERN.matcher(stackTrace);
+        if (matcher.find()) {
+            int index = 0;
+            do {
+                int start = matcher.start();
+                int end = matcher.end();
+                if (index == 0 && index == start && start == end) {
+                    // no empty leading substring included for zero-width match
+                    // at the beginning of the input char sequence.
+                    continue;
+                }
+
+                // append non-last line
+                appendStackTraceLine(builder, stackTrace, index, start);
+                builder.append(',');
+                builder.append(NEW_LINE);
+                index = end;
+            } while (matcher.find());
+
+            int length = stackTrace.length();
+            if (index < length) {
+                // append remaining line
+                appendStackTraceLine(builder, stackTrace, index, length);
+            }
+        } else {
+            // no newlines found, add entire stack trace as single element
+            appendStackTraceLine(builder, stackTrace, 0, stackTrace.length());
         }
-        removeIfEndsWith(builder, NEW_LINE);
-        removeIfEndsWith(builder, ",");
+    }
+
+    private static void appendStackTraceLine(StringBuilder builder, CharSequence stackTrace, int start, int end) {
+        builder.append("\t\"");
+        JsonUtils.quoteAsString(stackTrace, start, end, builder);
+        builder.append("\"");
     }
 
     public static void removeIfEndsWith(StringBuilder sb, String ending) {

diff --git a/ecs-logging-core/src/main/java/co/elastic/logging/JsonUtils.java b/ecs-logging-core/src/main/java/co/elastic/logging/JsonUtils.java
@@ -58,13 +58,21 @@ public final class JsonUtils {
     }
 
     public static void quoteAsString(CharSequence content, StringBuilder sb) {
+        if (content == null) {
+            sb.append("null");
+            return;
+        }
+        quoteAsString(content, 0, content.length(), sb);
+    }
+
+    public static void quoteAsString(CharSequence content, int start, int end, StringBuilder sb) {
         if (content == null) {
             sb.append("null");
             return;
         }
         final int[] escCodes = sOutputEscapes128;
         final int escLen = escCodes.length;
-        for (int i = 0, len = content.length(); i < len; ++i) {
+        for (int i = start; i < end; ++i) {
             char c = content.charAt(i);
             if (c >= escLen || escCodes[c] == 0) {
                 sb.append(c);

diff --git a/ecs-logging-core/src/test/java/co/elastic/logging/EcsJsonSerializerTest.java b/ecs-logging-core/src/test/java/co/elastic/logging/EcsJsonSerializerTest.java
@@ -148,13 +148,31 @@ void serializeExceptionWithStackTraceAsArray() throws JsonProcessingException {
         jsonBuilder.append('}');
 
         JsonNode jsonNode = objectMapper.readTree(jsonBuilder.toString());
+        System.out.println(jsonNode.toPrettyString());
         assertThat(jsonNode.get(ERROR_TYPE).textValue()).isEqualTo("className");
         assertThat(jsonNode.get(ERROR_STACK_TRACE).isArray()).isTrue();
+        assertThat(jsonNode.get(ERROR_STACK_TRACE).size()).isEqualTo(2);
         assertThat(jsonNode.get(ERROR_STACK_TRACE).get(0).textValue()).isEqualTo("stacktrace");
         assertThat(jsonNode.get(ERROR_STACK_TRACE).get(1).textValue()).isEqualTo("caused by error");
         assertThat(jsonNode.get(ERROR_MESSAGE).textValue()).isEqualTo("message");
     }
 
+    @Test
+    void serializeExceptionWithSingleLineStackTraceAsArray() throws JsonProcessingException {
+        StringBuilder jsonBuilder = new StringBuilder();
+        jsonBuilder.append('{');
+        EcsJsonSerializer.serializeException(jsonBuilder, "className", "message", "caused by error", true);
+        jsonBuilder.append('}');
+        System.out.println(jsonBuilder);
+        JsonNode jsonNode = objectMapper.readTree(jsonBuilder.toString());
+        System.out.println(jsonNode.toPrettyString());
+        assertThat(jsonNode.get(ERROR_TYPE).textValue()).isEqualTo("className");
+        assertThat(jsonNode.get(ERROR_STACK_TRACE).isArray()).isTrue();
+        assertThat(jsonNode.get(ERROR_STACK_TRACE).size()).isEqualTo(1);
+        assertThat(jsonNode.get(ERROR_STACK_TRACE).get(0).textValue()).isEqualTo("caused by error");
+        assertThat(jsonNode.get(ERROR_MESSAGE).textValue()).isEqualTo("message");
+    }
+
     @Test
     void serializeExceptionWithNullMessage() throws JsonProcessingException {
         StringBuilder jsonBuilder = new StringBuilder();