[#9631] Limit length of errorMessage for better stability

pinpoint-apm · Oct 16, 2023 · 30fec4e · 30fec4e
1 parent 2b2672d
commit 30fec4e
Show file tree

Hide file tree

Showing 9 changed files with 101 additions and 13 deletions.
diff --git a/agent/src/main/resources/profiles/local/pinpoint.config b/agent/src/main/resources/profiles/local/pinpoint.config
@@ -329,6 +329,7 @@ profiler.uri.stat.tomcat.useuserinput=false
 profiler.exceptiontrace.enable=true
 # Permits per second
 profiler.exceptiontrace.new.throughput=1000
+profiler.exceptiontrace.errormessage.max=2048
 # Permits depth of exception. if max depth is 0, it is unlimited.
 profiler.exceptiontrace.max.depth=0
 profiler.exceptiontrace.io.buffering.buffersize=20

diff --git a/agent/src/main/resources/profiles/release/pinpoint.config b/agent/src/main/resources/profiles/release/pinpoint.config
@@ -326,11 +326,12 @@ profiler.uri.stat.tomcat.useuserinput=false
 ###########################################################
 # Exception Trace
 ###########################################################
-profiler.exceptiontrace.enable=false
+profiler.exceptiontrace.enable=true
 # Permits per second
 profiler.exceptiontrace.new.throughput=1000
+profiler.exceptiontrace.errormessage.max=2048
 # Permits depth of exception. if max depth is 0, it is unlimited.
-profiler.exceptiontrace.max.depth=0
+profiler.exceptiontrace.max.depth=5
 profiler.exceptiontrace.io.buffering.buffersize=20
 
 ###########################################################

diff --git a/...c/main/java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapper.java b/...c/main/java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapper.java
@@ -33,21 +33,38 @@ public class ExceptionWrapper {
     private final long exceptionId;
     private final int exceptionDepth;
 
-    private ExceptionWrapper(Throwable throwable, long startTime, long exceptionId, int exceptionDepth) {
-        Objects.requireNonNull(throwable);
-        this.exceptionClassName = StringUtils.defaultIfEmpty(throwable.getClass().getSimpleName(), EMPTY_STRING);
-        this.exceptionMessage = StringUtils.defaultIfEmpty(throwable.getMessage(), EMPTY_STRING);
-        this.stackTraceElements = throwable.getStackTrace();
+    public ExceptionWrapper(
+            String exceptionClassName,
+            String exceptionMessage,
+            StackTraceElement[] stackTraceElements,
+            long startTime,
+            long exceptionId,
+            int exceptionDepth
+    ) {
+        this.exceptionClassName = Objects.requireNonNull(exceptionClassName, "exceptionClassName");
+        this.exceptionMessage = Objects.requireNonNull(exceptionMessage, "exceptionMessage");
+        this.stackTraceElements = Objects.requireNonNull(stackTraceElements, "stackTraceElements");
         this.startTime = startTime;
         this.exceptionId = exceptionId;
         this.exceptionDepth = exceptionDepth;
     }
 
-    public static ExceptionWrapper newException(Throwable throwable, long startTime, long exceptionId, int exceptionDepth) {
+    public static ExceptionWrapper newException(
+            Throwable throwable,
+            long startTime, long exceptionId, int exceptionDepth,
+            int maxErrorMessageLength
+    ) {
         if (throwable == null) {
             return null;
         }
-        return new ExceptionWrapper(throwable, startTime, exceptionId, exceptionDepth);
+        return new ExceptionWrapper(
+                StringUtils.defaultIfEmpty(throwable.getClass().getSimpleName(), EMPTY_STRING),
+                StringUtils.abbreviate(throwable.getMessage(), maxErrorMessageLength),
+                throwable.getStackTrace(),
+                startTime,
+                exceptionId,
+                exceptionDepth
+        );
     }
 
     public String getExceptionClassName() {

diff --git a/...java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapperFactory.java b/...java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapperFactory.java
@@ -24,9 +24,11 @@
  */
 public class ExceptionWrapperFactory {
     private final int maxDepth;
+    private final int maxErrorMessageLength;
 
-    public ExceptionWrapperFactory(int maxDepth) {
+    public ExceptionWrapperFactory(int maxDepth, int maxErrorMessageLength) {
         this.maxDepth = maxDepth;
+        this.maxErrorMessageLength = maxErrorMessageLength;
     }
 
     public List<ExceptionWrapper> newExceptionWrappers(ExceptionContext context) {
@@ -44,7 +46,7 @@ public List<ExceptionWrapper> newExceptionWrappers(Throwable throwable, long sta
         Throwable curr = throwable;
         int depth = 0;
         while (curr != null && (maxDepth == 0 || depth < maxDepth)) {
-            exceptionWrappers.add(ExceptionWrapper.newException(curr, startTime, exceptionId, depth));
+            exceptionWrappers.add(ExceptionWrapper.newException(curr, startTime, exceptionId, depth, maxErrorMessageLength));
             curr = curr.getCause();
             depth++;
         }

diff --git a/...a/com/navercorp/pinpoint/profiler/context/monitor/config/DefaultExceptionTraceConfig.java b/...a/com/navercorp/pinpoint/profiler/context/monitor/config/DefaultExceptionTraceConfig.java
@@ -7,6 +7,8 @@ public class DefaultExceptionTraceConfig implements ExceptionTraceConfig {
     private boolean exceptionTraceEnable = false;
     @Value("${profiler.exceptiontrace.new.throughput}")
     private double exceptionTraceNewThroughPut = 1000;
+    @Value("${profiler.exceptiontrace.errormessage.max}")
+    private int errorMessageMaxLength = 2048;
     @Value("${profiler.exceptiontrace.max.depth}")
     private int exceptionTraceMaxDepth = 0;
     @Value("${profiler.exceptiontrace.io.buffering.buffersize}")
@@ -22,6 +24,11 @@ public double getExceptionTraceNewThroughput() {
         return exceptionTraceNewThroughPut;
     }
 
+    @Override
+    public int getErrorMessageMaxLength() {
+        return errorMessageMaxLength;
+    }
+
     @Override
     public int getExceptionTraceMaxDepth() {
         return exceptionTraceMaxDepth;
@@ -37,6 +44,7 @@ public String toString() {
         return "DefaultExceptionTraceConfig{" +
                 "exceptionTraceEnable=" + exceptionTraceEnable +
                 ", exceptionTraceNewThroughPut=" + exceptionTraceNewThroughPut +
+                ", errorMessageMaxLength=" + errorMessageMaxLength +
                 ", exceptionTraceMaxDepth=" + exceptionTraceMaxDepth +
                 ", ioBufferingBufferSize=" + ioBufferingBufferSize +
                 '}';

diff --git a/...ain/java/com/navercorp/pinpoint/profiler/context/monitor/config/ExceptionTraceConfig.java b/...ain/java/com/navercorp/pinpoint/profiler/context/monitor/config/ExceptionTraceConfig.java
@@ -3,6 +3,8 @@
 public interface ExceptionTraceConfig {
     boolean isExceptionTraceEnable();
 
+    int getErrorMessageMaxLength();
+
     double getExceptionTraceNewThroughput();
 
     int getExceptionTraceMaxDepth();

diff --git a/...vercorp/pinpoint/profiler/context/provider/exception/ExceptionWrapperFactoryProvider.java b/...vercorp/pinpoint/profiler/context/provider/exception/ExceptionWrapperFactoryProvider.java
@@ -35,6 +35,9 @@ public ExceptionWrapperFactoryProvider(ExceptionTraceConfig exceptionTraceConfig
 
     @Override
     public ExceptionWrapperFactory get() {
-        return new ExceptionWrapperFactory(exceptionTraceConfig.getExceptionTraceMaxDepth());
+        return new ExceptionWrapperFactory(
+                exceptionTraceConfig.getExceptionTraceMaxDepth(),
+                exceptionTraceConfig.getErrorMessageMaxLength()
+        );
     }
 }
diff --git a/.../com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapperFactoryTest.java b/.../com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapperFactoryTest.java
@@ -0,0 +1,54 @@
+package com.navercorp.pinpoint.profiler.context.exception.model;
+
+
+import com.navercorp.pinpoint.common.util.StringUtils;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+/**
+ * @author intr3p1d
+ */
+class ExceptionWrapperFactoryTest {
+
+    private final static int MAX_DEPTH = 5;
+    private final static int MAX_LENGTH = 10;
+    private final static int ANY_INT = 1;
+    ExceptionWrapperFactory factory = new ExceptionWrapperFactory(MAX_DEPTH, MAX_LENGTH);
+
+    @Test
+    public void testErrorMessageLimitWorks() {
+        String errorMessage = "Message that exceed 10 characters";
+        Throwable th = new RuntimeException(errorMessage);
+
+        List<ExceptionWrapper> wrappers = factory.newExceptionWrappers(th, ANY_INT, ANY_INT);
+
+        String abbreviated = StringUtils.abbreviate(errorMessage, MAX_LENGTH);
+        Assertions.assertEquals(abbreviated, wrappers.get(0).getExceptionMessage());
+        Assertions.assertEquals(abbreviated.length(), wrappers.get(0).getExceptionMessage().length());
+    }
+
+    @Test
+    public void testErrorMessageLimitWorks2() {
+        String errorMessage = "Message";
+        Throwable th = new RuntimeException(errorMessage);
+
+        List<ExceptionWrapper> wrappers = factory.newExceptionWrappers(th, ANY_INT, ANY_INT);
+
+        String abbreviated = StringUtils.abbreviate(errorMessage, MAX_LENGTH);
+        Assertions.assertEquals(abbreviated, wrappers.get(0).getExceptionMessage());
+        Assertions.assertEquals(abbreviated.length(), wrappers.get(0).getExceptionMessage().length());
+    }
+
+    @Test
+    public void testExceptionMaxDepthWorks(){
+        Throwable th = new RuntimeException("inital throwable");
+        for (int i = 0; i < 10; i++) {
+            th = new RuntimeException(th);
+        }
+
+        List<ExceptionWrapper> wrappers = factory.newExceptionWrappers(th, ANY_INT, ANY_INT);
+        Assertions.assertEquals(MAX_DEPTH, wrappers.size());
+    }
+}
diff --git a/...t/java/com/navercorp/pinpoint/profiler/metadata/DefaultExceptionRecordingServiceTest.java b/...t/java/com/navercorp/pinpoint/profiler/metadata/DefaultExceptionRecordingServiceTest.java
@@ -39,7 +39,7 @@ public class DefaultExceptionRecordingServiceTest {
     private final static Logger logger = LogManager.getLogger(DefaultExceptionRecordingServiceTest.class);
 
     ExceptionTraceSampler exceptionTraceSampler = new ExceptionTraceSampler(1000);
-    ExceptionWrapperFactory exceptionWrapperFactory = new ExceptionWrapperFactory(10);
+    ExceptionWrapperFactory exceptionWrapperFactory = new ExceptionWrapperFactory(10, 1048);
     DefaultExceptionRecordingService exceptionRecordingService = new DefaultExceptionRecordingService(
             exceptionTraceSampler, exceptionWrapperFactory
     );