From 5fbbcd3f274b8a730ea6005a5461fb2462d7a031 Mon Sep 17 00:00:00 2001 From: seoyoung-park Date: Mon, 16 Oct 2023 15:47:51 +0900 Subject: [PATCH] [#9631] Limit length of errorMessage for better stability --- .../resources/profiles/local/pinpoint.config | 1 + .../profiles/release/pinpoint.config | 5 ++- .../exception/model/ExceptionWrapper.java | 31 ++++++++++---- .../model/ExceptionWrapperFactory.java | 6 ++- .../config/DefaultExceptionTraceConfig.java | 8 ++++ .../monitor/config/ExceptionTraceConfig.java | 2 + .../ExceptionWrapperFactoryProvider.java | 5 ++- .../model/ExceptionWrapperFactoryTest.java | 42 +++++++++++++++++++ .../DefaultExceptionRecordingServiceTest.java | 2 +- 9 files changed, 89 insertions(+), 13 deletions(-) create mode 100644 profiler/src/test/java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapperFactoryTest.java diff --git a/agent/src/main/resources/profiles/local/pinpoint.config b/agent/src/main/resources/profiles/local/pinpoint.config index b815670afe3e0..7e4e0ca133ac9 100644 --- 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 index ea6bc4ecf05f0..27e67eef16dc0 100644 --- 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/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapper.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapper.java index cc30a797cc514..51ee66c5fd11d 100644 --- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapper.java +++ b/profiler/src/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/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapperFactory.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapperFactory.java index da3a7871055ea..455d162984a14 100644 --- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapperFactory.java +++ b/profiler/src/main/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 newExceptionWrappers(ExceptionContext context) { @@ -44,7 +46,7 @@ public List 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/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/monitor/config/DefaultExceptionTraceConfig.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/monitor/config/DefaultExceptionTraceConfig.java index 919db07cef8d4..12831eddc5062 100644 --- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/monitor/config/DefaultExceptionTraceConfig.java +++ b/profiler/src/main/java/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/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/monitor/config/ExceptionTraceConfig.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/monitor/config/ExceptionTraceConfig.java index d2465d14ba028..d7f26e6807409 100644 --- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/monitor/config/ExceptionTraceConfig.java +++ b/profiler/src/main/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/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/provider/exception/ExceptionWrapperFactoryProvider.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/provider/exception/ExceptionWrapperFactoryProvider.java index e8150e991fd65..58335a3b89bea 100644 --- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/context/provider/exception/ExceptionWrapperFactoryProvider.java +++ b/profiler/src/main/java/com/navercorp/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/profiler/src/test/java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapperFactoryTest.java b/profiler/src/test/java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapperFactoryTest.java new file mode 100644 index 0000000000000..03dc5989c7eb3 --- /dev/null +++ b/profiler/src/test/java/com/navercorp/pinpoint/profiler/context/exception/model/ExceptionWrapperFactoryTest.java @@ -0,0 +1,42 @@ +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 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 wrappers = factory.newExceptionWrappers(th, ANY_INT, ANY_INT); + Assertions.assertEquals(MAX_DEPTH, wrappers.size()); + } +} \ No newline at end of file diff --git a/profiler/src/test/java/com/navercorp/pinpoint/profiler/metadata/DefaultExceptionRecordingServiceTest.java b/profiler/src/test/java/com/navercorp/pinpoint/profiler/metadata/DefaultExceptionRecordingServiceTest.java index 5b1b27438822c..3095024dcd69a 100644 --- a/profiler/src/test/java/com/navercorp/pinpoint/profiler/metadata/DefaultExceptionRecordingServiceTest.java +++ b/profiler/src/test/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 );