Skip to content

Commit

Permalink
[#9631] Limit length of errorMessage for better stability
Browse files Browse the repository at this point in the history
  • Loading branch information
intr3p1d committed Oct 16, 2023
1 parent 333212d commit 5fbbcd3
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 13 deletions.
1 change: 1 addition & 0 deletions agent/src/main/resources/profiles/local/pinpoint.config
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions agent/src/main/resources/profiles/release/pinpoint.config
Original file line number Diff line number Diff line change
Expand Up @@ -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

###########################################################
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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++;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand All @@ -22,6 +24,11 @@ public double getExceptionTraceNewThroughput() {
return exceptionTraceNewThroughPut;
}

@Override
public int getErrorMessageMaxLength() {
return errorMessageMaxLength;

Check warning on line 29 in profiler/src/main/java/com/navercorp/pinpoint/profiler/context/monitor/config/DefaultExceptionTraceConfig.java

View check run for this annotation

Codecov / codecov/patch

profiler/src/main/java/com/navercorp/pinpoint/profiler/context/monitor/config/DefaultExceptionTraceConfig.java#L29

Added line #L29 was not covered by tests
}

@Override
public int getExceptionTraceMaxDepth() {
return exceptionTraceMaxDepth;
Expand All @@ -37,6 +44,7 @@ public String toString() {
return "DefaultExceptionTraceConfig{" +
"exceptionTraceEnable=" + exceptionTraceEnable +
", exceptionTraceNewThroughPut=" + exceptionTraceNewThroughPut +
", errorMessageMaxLength=" + errorMessageMaxLength +
", exceptionTraceMaxDepth=" + exceptionTraceMaxDepth +
", ioBufferingBufferSize=" + ioBufferingBufferSize +
'}';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
public interface ExceptionTraceConfig {
boolean isExceptionTraceEnable();

int getErrorMessageMaxLength();

double getExceptionTraceNewThroughput();

int getExceptionTraceMaxDepth();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ public ExceptionWrapperFactoryProvider(ExceptionTraceConfig exceptionTraceConfig

@Override
public ExceptionWrapperFactory get() {
return new ExceptionWrapperFactory(exceptionTraceConfig.getExceptionTraceMaxDepth());
return new ExceptionWrapperFactory(
exceptionTraceConfig.getExceptionTraceMaxDepth(),
exceptionTraceConfig.getErrorMessageMaxLength()

Check warning on line 40 in profiler/src/main/java/com/navercorp/pinpoint/profiler/context/provider/exception/ExceptionWrapperFactoryProvider.java

View check run for this annotation

Codecov / codecov/patch

profiler/src/main/java/com/navercorp/pinpoint/profiler/context/provider/exception/ExceptionWrapperFactoryProvider.java#L38-L40

Added lines #L38 - L40 were not covered by tests
);
}
}
Original file line number Diff line number Diff line change
@@ -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<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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down

0 comments on commit 5fbbcd3

Please sign in to comment.