Skip error.stack_trace if empty

elastic · Jul 11, 2024 · a81f132 · a81f132
1 parent 584bd10
commit a81f132
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 28 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
@@ -208,22 +208,21 @@ public static void serializeException(StringBuilder builder, Throwable thrown, b
         if (thrown != null) {
             builder.append("\"error.type\":\"");
             JsonUtils.quoteAsString(thrown.getClass().getName(), builder);
-            builder.append("\",");
+            builder.append('\"');
 
             String message = thrown.getMessage();
             if (message != null) {
-                builder.append("\"error.message\":\"");
+                builder.append(",\"error.message\":\"");
                 JsonUtils.quoteAsString(message, builder);
-                builder.append("\",");
+                builder.append('\"');
             }
-            if (stackTraceAsArray) {
-                builder.append("\"error.stack_trace\":[").append(NEW_LINE);
-                formatThrowableAsArray(builder, thrown);
-                builder.append("]");
+
+            int prevLength = builder.length();
+            builder.append(",\"error.stack_trace\":").append(stackTraceAsArray ? '[' : "\"");
+            if (formatThrowable(builder, thrown, stackTraceAsArray)) {
+                builder.append(stackTraceAsArray ? ']' : '\"');
             } else {
-                builder.append("\"error.stack_trace\":\"");
-                JsonUtils.quoteAsString(formatThrowable(thrown), builder);
-                builder.append("\"");
+                builder.setLength(prevLength); // reset if no stacktrace was written
             }
         }
     }
@@ -249,30 +248,38 @@ public static void serializeException(StringBuilder builder, String exceptionCla
         }
     }
 
-    private static CharSequence formatThrowable(final Throwable throwable) {
-        StringBuilder buffer = getMessageStringBuilder();
-        final PrintWriter pw = new PrintWriter(new StringBuilderWriter(buffer));
-        throwable.printStackTrace(pw);
-        pw.flush();
-        return buffer;
-    }
-
-    private static void formatThrowableAsArray(final StringBuilder jsonBuilder, final Throwable throwable) {
+    private static boolean formatThrowable(final StringBuilder jsonBuilder, final Throwable throwable, boolean stackTraceAsArray) {
         final StringBuilder buffer = getMessageStringBuilder();
-        final PrintWriter pw = new PrintWriter(new StringBuilderWriter(buffer), true) {
+        final int initialLength = jsonBuilder.length();
+        try (PrintWriter pw = new PrintWriter(new StringBuilderWriter(buffer), true) {
+            private int lines = 0;
+
             @Override
             public void println() {
                 flush();
-                jsonBuilder.append("\t\"");
-                JsonUtils.quoteAsString(buffer, jsonBuilder);
-                jsonBuilder.append("\",");
-                jsonBuilder.append(NEW_LINE);
+                if (stackTraceAsArray) {
+                    if (lines > 0) jsonBuilder.append(',');
+                    jsonBuilder.append(NEW_LINE).append("\t\"");
+                    JsonUtils.quoteAsString(buffer, jsonBuilder);
+                    jsonBuilder.append('\"');
+                } else {
+                    JsonUtils.quoteAsString(buffer, jsonBuilder);
+                    JsonUtils.quoteAsString(NEW_LINE, jsonBuilder);
+                }
                 buffer.setLength(0);
+                lines++;
+            }
+
+            @Override
+            public void close() {
+                if (lines <= 1) {
+                    jsonBuilder.setLength(initialLength); // skip the first line (message) if no stacktrace follows
+                }
             }
-        };
-        throwable.printStackTrace(pw);
-        removeIfEndsWith(jsonBuilder, NEW_LINE);
-        removeIfEndsWith(jsonBuilder, ",");
+        }) {
+            throwable.printStackTrace(pw);
+        }
+        return jsonBuilder.length() > initialLength;
     }
 
     private static void formatStackTraceAsArray(StringBuilder builder, CharSequence stackTrace) {

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
@@ -61,6 +61,25 @@ void serializeExceptionAsString() throws IOException {
         assertThat(jsonNode.get(ERROR_STACK_TRACE).textValue()).isEqualTo(stringWriter.toString());
     }
 
+    @Test
+    void serializeExceptionWithoutStacktraceAsString() throws IOException {
+        Exception exception = new Exception("no stacktrace"){
+            @Override
+            public synchronized Throwable fillInStackTrace() {
+                return this;
+            }
+        };
+        StringBuilder jsonBuilder = new StringBuilder();
+        jsonBuilder.append('{');
+        EcsJsonSerializer.serializeException(jsonBuilder, exception, false);
+        jsonBuilder.append('}');
+        JsonNode jsonNode = objectMapper.readTree(jsonBuilder.toString());
+
+        assertThat(jsonNode.get(ERROR_TYPE).textValue()).isEqualTo(exception.getClass().getName());
+        assertThat(jsonNode.get(ERROR_MESSAGE).textValue()).isEqualTo("no stacktrace");
+        assertThat(jsonNode.get(ERROR_STACK_TRACE)).isNull();
+    }
+
     @Test
     void testEscaping() throws IOException {
         String loggerName = "logger\"";
@@ -125,6 +144,26 @@ void serializeExceptionAsArray() throws IOException {
                 .isEqualTo(stringWriter.toString());
     }
 
+    @Test
+    void serializeExceptionWithoutStacktraceAsArray() throws IOException {
+        Exception exception = new Exception("no stacktrace"){
+            @Override
+            public synchronized Throwable fillInStackTrace() {
+                return this;
+            }
+        };
+        StringBuilder jsonBuilder = new StringBuilder();
+        jsonBuilder.append('{');
+        EcsJsonSerializer.serializeException(jsonBuilder, exception, true);
+        jsonBuilder.append('}');
+        System.out.println(jsonBuilder);
+        JsonNode jsonNode = objectMapper.readTree(jsonBuilder.toString());
+
+        assertThat(jsonNode.get(ERROR_TYPE).textValue()).isEqualTo(exception.getClass().getName());
+        assertThat(jsonNode.get(ERROR_MESSAGE).textValue()).isEqualTo("no stacktrace");
+        assertThat(jsonNode.get(ERROR_STACK_TRACE)).isNull();
+    }
+
     @Test
     void testRemoveIfEndsWith() {
         assertRemoveIfEndsWith("", "foo", "");