diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 589b635329..b7bfd24d3a 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -31,6 +31,10 @@ Use subheadings with the "=====" level for adding notes for unreleased changes: === Unreleased +[float] +===== Features +* Added internal `safe_exceptions` config option to workaround JVM bugs related to touching exceptions - {pull}3528[#3528] + [float] ===== Bug fixes * Cleanup extra servlet request attribute used for Spring exception handler - {pull}3527[#3527] @@ -44,9 +48,6 @@ Use subheadings with the "=====" level for adding notes for unreleased changes: [float] ===== Features * Added a configuration option to use queues in names of spring-rabbit transactions - {pull}3424[#3424] - -[float] -===== Features * Add option to retry JMX metrics capture in case of exception - {pull}3511[#3511] [float] diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/CoreConfiguration.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/CoreConfiguration.java index f09ced8cfa..c185bbc314 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/CoreConfiguration.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/CoreConfiguration.java @@ -517,6 +517,14 @@ public String toString(Collection value) { .tags("added[1.44.0]") .buildWithDefault(false); + + private final ConfigurationOption safeExceptions = ConfigurationOption.integerOption() + .key("safe_exceptions") + .tags("internal") + .configurationCategory(CORE_CATEGORY) + .dynamic(true) + .buildWithDefault(0); + private final ConfigurationOption> classesExcludedFromInstrumentation = ConfigurationOption .builder(new ValueConverter>() { @@ -1163,6 +1171,15 @@ public boolean isContextPropagationOnly() { return contextPropagationOnly.get(); } + public boolean isRedactExceptions() { + return (safeExceptions.get() & 1) != 0; + } + + @Override + public boolean isUseServletAttributesForExceptionPropagation() { + return (safeExceptions.get() & 2) == 0; + } + public enum CloudProvider { AUTO, AWS, diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java index 6639e57be2..624bfc3f2f 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java @@ -26,6 +26,7 @@ import co.elastic.apm.agent.configuration.CoreConfiguration; import co.elastic.apm.agent.configuration.MetricsConfiguration; import co.elastic.apm.agent.configuration.ServerlessConfiguration; +import co.elastic.apm.agent.impl.error.RedactedException; import co.elastic.apm.agent.impl.metadata.ServiceFactory; import co.elastic.apm.agent.tracer.service.Service; import co.elastic.apm.agent.tracer.service.ServiceInfo; @@ -446,6 +447,8 @@ private ErrorCapture captureException(long epochMicros, @Nullable Throwable e, E return null; } + e = redactExceptionIfRequired(e); + while (e != null && WildcardMatcher.anyMatch(coreConfiguration.getUnnestExceptions(), e.getClass().getName()) != null) { e = e.getCause(); } @@ -995,4 +998,13 @@ public Service createService(String ephemeralId) { configurationRegistry.getConfig(ServerlessConfiguration.class).runsOnAwsLambda() ); } + + @Override + @Nullable + public Throwable redactExceptionIfRequired(@Nullable Throwable original) { + if (original != null && coreConfiguration.isRedactExceptions() && !(original instanceof RedactedException)) { + return new RedactedException(); + } + return original; + } } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/error/RedactedException.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/error/RedactedException.java new file mode 100644 index 0000000000..19ca4d2e31 --- /dev/null +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/error/RedactedException.java @@ -0,0 +1,28 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package co.elastic.apm.agent.impl.error; + +public class RedactedException extends Exception { + + public RedactedException() { + super("This exception is a placeholder for an exception which has occurred in the application." + + "The stacktrace corresponds to where elastic APM detected the exception."); + } + +} diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java index aabac69aed..aab38dddd2 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java @@ -104,6 +104,9 @@ public class Transaction extends AbstractSpan implements co.elastic @Nullable private String frameworkVersion; + @Nullable + private Throwable pendingException; + /** * Faas *

@@ -337,6 +340,7 @@ public void resetState() { frameworkVersion = null; faas.resetState(); wasActivated.set(false); + pendingException = null; // don't clear timerBySpanTypeAndSubtype map (see field-level javadoc) } @@ -535,4 +539,17 @@ private void trackMetrics() { phaser.readerUnlock(); } } + + + @Override + public void setPendingTransactionException(@Nullable Throwable exception) { + this.pendingException = exception; + } + + @Nullable + @Override + public Throwable getPendingTransactionException() { + return this.pendingException; + } + } diff --git a/apm-agent-core/src/test/java/org/example/stacktrace/ErrorCaptureTest.java b/apm-agent-core/src/test/java/org/example/stacktrace/ErrorCaptureTest.java index 91e2c267c8..5137144396 100644 --- a/apm-agent-core/src/test/java/org/example/stacktrace/ErrorCaptureTest.java +++ b/apm-agent-core/src/test/java/org/example/stacktrace/ErrorCaptureTest.java @@ -24,6 +24,7 @@ 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.error.RedactedException; import co.elastic.apm.agent.impl.stacktrace.StacktraceConfiguration; import co.elastic.apm.agent.impl.transaction.Transaction; import org.junit.jupiter.api.BeforeEach; @@ -107,6 +108,27 @@ void testNonConfiguredWrappingConfigured() { assertThat(errorCapture.getException()).isInstanceOf(WrapperException.class); } + @Test + void testExceptionRedaction() { + doReturn(true).when(coreConfiguration).isRedactExceptions(); + assertThat(tracer.redactExceptionIfRequired(null)).isNull(); + + Exception exception = new CustomException(); + Throwable redacted = tracer.redactExceptionIfRequired(exception); + assertThat(redacted).isInstanceOf(RedactedException.class); + + assertThat(tracer.redactExceptionIfRequired(redacted)).isSameAs(redacted); + + ErrorCapture errorCapture = tracer.captureException(exception, tracer.currentContext(), null); + assertThat(errorCapture).isNotNull(); + assertThat(errorCapture.getException()).isInstanceOf(RedactedException.class); + + ErrorCapture alreadyRedacted = tracer.captureException(redacted, tracer.currentContext(), null); + assertThat(alreadyRedacted).isNotNull(); + assertThat(alreadyRedacted.getException()).isSameAs(redacted); + } + + private static class NestedException extends Exception { public NestedException(Throwable cause) { super(cause); diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java index c52377be12..07b98092ac 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java @@ -253,6 +253,12 @@ public static { @Override public JakartaApmAsyncListener createInstance() { diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxApmAsyncListener.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxApmAsyncListener.java index 32fb3bac4e..2f18ed1944 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxApmAsyncListener.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxApmAsyncListener.java @@ -78,7 +78,7 @@ public void onComplete(AsyncEvent event) { @Override public void onTimeout(AsyncEvent event) { - throwable = event.getThrowable(); + throwable = asyncContextAdviceHelperImpl.getTracer().redactExceptionIfRequired(event.getThrowable()); if (isJBossEap6(event)) { endTransaction(event); } @@ -97,7 +97,7 @@ public void onTimeout(AsyncEvent event) { @Override public void onError(AsyncEvent event) { - throwable = event.getThrowable(); + throwable = asyncContextAdviceHelperImpl.getTracer().redactExceptionIfRequired(event.getThrowable()); if (isJBossEap6(event)) { endTransaction(event); } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxAsyncContextAdviceHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxAsyncContextAdviceHelper.java index 4809a726df..cc11be4565 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxAsyncContextAdviceHelper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JavaxAsyncContextAdviceHelper.java @@ -45,6 +45,10 @@ public JavaxAsyncContextAdviceHelper(Tracer tracer) { new JavaxAsyncContextAdviceHelper.ApmAsyncListenerAllocator()); } + public Tracer getTracer() { + return tracer; + } + private final class ApmAsyncListenerAllocator implements Allocator { @Override public JavaxApmAsyncListener createInstance() { diff --git a/apm-agent-plugins/apm-spring-webmvc/apm-spring-webmvc-spring5/src/main/java/co/elastic/apm/agent/springwebmvc/AbstractSpringExceptionHandlerInstrumentation.java b/apm-agent-plugins/apm-spring-webmvc/apm-spring-webmvc-spring5/src/main/java/co/elastic/apm/agent/springwebmvc/AbstractSpringExceptionHandlerInstrumentation.java index 331f8131a4..8d2faed51f 100644 --- a/apm-agent-plugins/apm-spring-webmvc/apm-spring-webmvc-spring5/src/main/java/co/elastic/apm/agent/springwebmvc/AbstractSpringExceptionHandlerInstrumentation.java +++ b/apm-agent-plugins/apm-spring-webmvc/apm-spring-webmvc-spring5/src/main/java/co/elastic/apm/agent/springwebmvc/AbstractSpringExceptionHandlerInstrumentation.java @@ -21,6 +21,10 @@ import co.elastic.apm.agent.sdk.ElasticApmInstrumentation; import co.elastic.apm.agent.servlet.Constants; import co.elastic.apm.agent.servlet.adapter.ServletRequestAdapter; +import co.elastic.apm.agent.tracer.GlobalTracer; +import co.elastic.apm.agent.tracer.Tracer; +import co.elastic.apm.agent.tracer.Transaction; +import co.elastic.apm.agent.tracer.configuration.CoreConfiguration; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -63,7 +67,17 @@ public static void captureRequestError(ServletRequestAdapte @Nullable HttpServletRequest request, @Nullable Exception e) { if (request != null && e != null) { - adapter.setAttribute(request, "co.elastic.apm.exception", e); + Tracer tracer = GlobalTracer.get(); + boolean useAttribs = tracer.getConfig(CoreConfiguration.class).isUseServletAttributesForExceptionPropagation(); + Throwable maybeRedacted = tracer.redactExceptionIfRequired(e); + if (useAttribs) { + adapter.setAttribute(request, "co.elastic.apm.exception", maybeRedacted); + } else { + Transaction transaction = tracer.currentContext().getTransaction(); + if(transaction != null) { + transaction.setPendingTransactionException(maybeRedacted); + } + } } } } diff --git a/apm-agent-plugins/apm-spring-webmvc/apm-spring-webmvc-spring5/src/test/java/co/elastic/apm/agent/springwebmvc/exception/AbstractExceptionHandlerInstrumentationWithExceptionResolverTest.java b/apm-agent-plugins/apm-spring-webmvc/apm-spring-webmvc-spring5/src/test/java/co/elastic/apm/agent/springwebmvc/exception/AbstractExceptionHandlerInstrumentationWithExceptionResolverTest.java index 0800056204..b3774107ed 100644 --- a/apm-agent-plugins/apm-spring-webmvc/apm-spring-webmvc-spring5/src/test/java/co/elastic/apm/agent/springwebmvc/exception/AbstractExceptionHandlerInstrumentationWithExceptionResolverTest.java +++ b/apm-agent-plugins/apm-spring-webmvc/apm-spring-webmvc-spring5/src/test/java/co/elastic/apm/agent/springwebmvc/exception/AbstractExceptionHandlerInstrumentationWithExceptionResolverTest.java @@ -18,9 +18,11 @@ */ package co.elastic.apm.agent.springwebmvc.exception; +import co.elastic.apm.agent.configuration.CoreConfiguration; import co.elastic.apm.agent.springwebmvc.exception.testapp.exception_resolver.ExceptionResolverController; import co.elastic.apm.agent.springwebmvc.exception.testapp.exception_resolver.ExceptionResolverRuntimeException; -import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.web.servlet.MvcResult; @@ -30,6 +32,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.doReturn; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; /** @@ -42,8 +45,12 @@ }) public abstract class AbstractExceptionHandlerInstrumentationWithExceptionResolverTest extends AbstractExceptionHandlerInstrumentationTest { - @Test - public void testCallApiWithExceptionThrown() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {true,false}) + public void testCallApiWithExceptionThrown(boolean useAttributeBasedPropagation) throws Exception { + CoreConfiguration coreConfig = config.getConfig(CoreConfiguration.class); + doReturn(useAttributeBasedPropagation).when(coreConfig).isUseServletAttributesForExceptionPropagation(); + ResultActions resultActions = this.mockMvc.perform(get("/exception-resolver/throw-exception")); MvcResult result = resultActions.andReturn(); MockHttpServletResponse response = result.getResponse(); diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/GlobalTracer.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/GlobalTracer.java index 1ad88cd129..449622326e 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/GlobalTracer.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/GlobalTracer.java @@ -150,4 +150,10 @@ public void reportLog(byte[] log) { public Service createService(String ephemeralId) { return tracer.createService(ephemeralId); } + + @Nullable + @Override + public Throwable redactExceptionIfRequired(@Nullable Throwable original) { + return tracer.redactExceptionIfRequired(original); + } } diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/NoopTracer.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/NoopTracer.java index 2c2727152d..5ee5d8a652 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/NoopTracer.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/NoopTracer.java @@ -125,4 +125,10 @@ public void reportLog(byte[] log) { public Service createService(String ephemeralId) { return null; } + + @Nullable + @Override + public Throwable redactExceptionIfRequired(@Nullable Throwable original) { + return original; + } } diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Tracer.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Tracer.java index b7f9042142..1cd0529de8 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Tracer.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Tracer.java @@ -88,4 +88,7 @@ public interface Tracer { @Nullable Service createService(String ephemeralId); + + @Nullable + Throwable redactExceptionIfRequired(@Nullable Throwable original); } diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Transaction.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Transaction.java index 1424da3766..1f6ade713c 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Transaction.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Transaction.java @@ -56,4 +56,9 @@ public interface Transaction> extends AbstractSpan { void setFrameworkName(@Nullable String frameworkName); void setFrameworkVersion(@Nullable String frameworkVersion); + + void setPendingTransactionException(Throwable exception); + + @Nullable + Throwable getPendingTransactionException(); } diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/CoreConfiguration.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/CoreConfiguration.java index 3d166ca595..28af7bd574 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/CoreConfiguration.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/configuration/CoreConfiguration.java @@ -50,6 +50,8 @@ public interface CoreConfiguration { TimeDuration getSpanMinDuration(); + boolean isUseServletAttributesForExceptionPropagation(); + enum EventType { /** * Request bodies will never be reported