Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unnest exceptions before filtering #3025

Merged
merged 5 commits into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ communication - {pull}2996[#2996]
* Prevent potential connection leak on network failure - {pull}2869[#2869]
* Fix for inferred spans where the parent id was also a child id - {pull}2686[#2686]
* Fix context propagation for async 7.x and 8.x Elasticsearch clients - {pull}3015[#3015]
* Fix exceptions filtering based on <<config-ignore-exceptions>> when those are <<config-unnest-exceptions, nested>> - {pull}3025[#3025]

[[release-notes-1.x]]
=== Java Agent version 1.x
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,14 @@ public ErrorCapture captureException(@Nullable Throwable e, @Nullable AbstractSp

@Nullable
private ErrorCapture captureException(long epochMicros, @Nullable Throwable e, @Nullable AbstractSpan<?> parent, @Nullable ClassLoader initiatingClassLoader) {
if (!isRunning()) {
if (!isRunning() || e == null) {
return null;
}

while (e != null && WildcardMatcher.anyMatch(coreConfiguration.getUnnestExceptions(), e.getClass().getName()) != null) {
jackshirazi marked this conversation as resolved.
Show resolved Hide resolved
e = e.getCause();
}

// note: if we add inheritance support for exception filtering, caching would be required for performance
if (e != null && !WildcardMatcher.isAnyMatch(coreConfiguration.getIgnoreExceptions(), e.getClass().getName())) {
ErrorCapture error = errorPool.createInstance();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@
*/
package co.elastic.apm.agent.impl.error;

import co.elastic.apm.agent.configuration.CoreConfiguration;
import co.elastic.apm.agent.impl.ElasticApmTracer;
import co.elastic.apm.agent.impl.context.TransactionContext;
import co.elastic.apm.agent.impl.stacktrace.StacktraceConfiguration;
import co.elastic.apm.agent.impl.transaction.AbstractSpan;
import co.elastic.apm.agent.impl.transaction.Span;
import co.elastic.apm.agent.impl.transaction.TraceContext;
import co.elastic.apm.agent.impl.transaction.Transaction;
import co.elastic.apm.agent.common.util.WildcardMatcher;
import co.elastic.apm.agent.objectpool.Recyclable;
import co.elastic.apm.agent.sdk.logging.Logger;
import co.elastic.apm.agent.sdk.logging.LoggerFactory;
Expand Down Expand Up @@ -153,11 +151,7 @@ public TraceContext getTraceContext() {
}

public void setException(Throwable e) {
if (WildcardMatcher.anyMatch(tracer.getConfig(CoreConfiguration.class).getUnnestExceptions(), e.getClass().getName()) != null) {
this.exception = e.getCause();
} else {
this.exception = e;
}
exception = e;
}

public StringBuilder getCulprit() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@
package org.example.stacktrace;

import co.elastic.apm.agent.MockTracer;
import co.elastic.apm.agent.configuration.SpyConfiguration;
import co.elastic.apm.agent.common.util.WildcardMatcher;
import co.elastic.apm.agent.configuration.CoreConfiguration;
import co.elastic.apm.agent.impl.ElasticApmTracer;
import co.elastic.apm.agent.impl.context.Request;
import co.elastic.apm.agent.impl.error.ErrorCapture;
import co.elastic.apm.agent.impl.stacktrace.StacktraceConfiguration;
import co.elastic.apm.agent.impl.transaction.Transaction;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.stagemonitor.configuration.ConfigurationRegistry;

import java.util.List;

Expand All @@ -36,14 +36,15 @@

class ErrorCaptureTest {

private StacktraceConfiguration stacktraceConfiguration;
private ElasticApmTracer tracer;
private StacktraceConfiguration stacktraceConfiguration;
private CoreConfiguration coreConfiguration;

@BeforeEach
void setUp() {
final ConfigurationRegistry registry = SpyConfiguration.createSpyConfig();
tracer = MockTracer.create(registry);
stacktraceConfiguration = registry.getConfig(StacktraceConfiguration.class);
tracer = MockTracer.createRealTracer();
stacktraceConfiguration = tracer.getConfig(StacktraceConfiguration.class);
coreConfiguration = tracer.getConfig(CoreConfiguration.class);
}

@Test
Expand All @@ -65,11 +66,45 @@ void testCulprit() {
}

@Test
void testUnnestNestedExceptions() {
final ErrorCapture errorCapture = new ErrorCapture(tracer);
final NestedException nestedException = new NestedException(new Exception());
errorCapture.setException(nestedException);
void testUnnestNestedException() {
final NestedException nestedException = new NestedException(new CustomException());
ErrorCapture errorCapture = tracer.captureException(nestedException, null, null);
assertThat(errorCapture).isNotNull();
assertThat(errorCapture.getException()).isNotInstanceOf(NestedException.class);
assertThat(errorCapture.getException()).isInstanceOf(CustomException.class);
}

@Test
void testUnnestDoublyNestedException() {
final NestedException nestedException = new NestedException(new NestedException(new CustomException()));
ErrorCapture errorCapture = tracer.captureException(nestedException, null, null);
assertThat(errorCapture).isNotNull();
assertThat(errorCapture.getException()).isNotInstanceOf(NestedException.class);
assertThat(errorCapture.getException()).isInstanceOf(CustomException.class);
}

@Test
void testIgnoredNestedException() {
eyalkoren marked this conversation as resolved.
Show resolved Hide resolved
doReturn(List.of(WildcardMatcher.valueOf("*CustomException"))).when(coreConfiguration).getIgnoreExceptions();
final NestedException nestedException = new NestedException(new CustomException());
ErrorCapture errorCapture = tracer.captureException(nestedException, null, null);
assertThat(errorCapture).isNull();
}

@Test
void testNonConfiguredNestingException() {
final WrapperException wrapperException = new WrapperException(new CustomException());
ErrorCapture errorCapture = tracer.captureException(wrapperException, null, null);
assertThat(errorCapture).isNotNull();
assertThat(errorCapture.getException()).isInstanceOf(WrapperException.class);
}

@Test
void testNonConfiguredWrappingConfigured() {
final NestedException nestedException = new NestedException(new WrapperException(new NestedException(new Exception())));
ErrorCapture errorCapture = tracer.captureException(nestedException, null, null);
assertThat(errorCapture).isNotNull();
assertThat(errorCapture.getException()).isInstanceOf(WrapperException.class);
}

private static class NestedException extends Exception {
Expand All @@ -78,6 +113,14 @@ public NestedException(Throwable cause) {
}
}

private static class WrapperException extends Exception {
public WrapperException(Throwable cause) {
super(cause);
}
}

private static class CustomException extends Exception {}

@Test
void testTransactionContextTransfer() {
final Transaction transaction = new Transaction(tracer);
Expand Down