Added internal option to fully redact exceptions (#3528)

Author: JonasKunz

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]

apm-agent-core/src/main/java/co/elastic/apm/agent/configuration/CoreConfiguration.java

@@ -517,6 +517,14 @@ public String toString(Collection<String> value) {
         .tags("added[1.44.0]")
         .buildWithDefault(false);
 
+
+    private final ConfigurationOption<Integer> safeExceptions = ConfigurationOption.<Boolean>integerOption()
+        .key("safe_exceptions")
+        .tags("internal")
+        .configurationCategory(CORE_CATEGORY)
+        .dynamic(true)
+        .buildWithDefault(0);
+
     private final ConfigurationOption<List<WildcardMatcher>> classesExcludedFromInstrumentation = ConfigurationOption
         .builder(new ValueConverter<List<WildcardMatcher>>() {
 
@@ -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,

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;
+    }
 }

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.");
+    }
+
+}

apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java

@@ -104,6 +104,9 @@ public class Transaction extends AbstractSpan<Transaction> implements co.elastic
     @Nullable
     private String frameworkVersion;
 
+    @Nullable
+    private Throwable pendingException;
+
     /**
      * Faas
      * <p>
@@ -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;
+    }
+
 }

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);

apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java

@@ -253,6 +253,12 @@ public static <HttpServletRequest, HttpServletResponse, ServletContext, ServletC
                             break;
                         }
                     }
+                    if (t2 == null) {
+                        t2 = transaction.getPendingTransactionException();
+                        if(t2 != null) {
+                            overrideStatusCodeOnThrowable = false;
+                        }
+                    }
                 }
 
                 ServletContext servletContext = adapter.getServletContext(httpServletRequest);

apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JakartaApmAsyncListener.java

@@ -79,7 +79,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);
         }
@@ -98,7 +98,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);
         }

apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/JakartaAsyncContextAdviceHelper.java

@@ -44,6 +44,10 @@ public JakartaAsyncContextAdviceHelper(Tracer tracer) {
             new JakartaAsyncContextAdviceHelper.JakartaApmAsyncListenerAllocator());
     }
 
+    public Tracer getTracer() {
+        return tracer;
+    }
+
     private final class JakartaApmAsyncListenerAllocator implements Allocator<JakartaApmAsyncListener> {
         @Override
         public JakartaApmAsyncListener createInstance() {

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);
         }

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<JavaxApmAsyncListener> {
         @Override
         public JavaxApmAsyncListener createInstance() {

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 <HttpServletRequest> 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);
+                    }
+                }
             }
         }
     }

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();

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);
+    }
 }

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;
+    }
 }

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);
 }

apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Transaction.java

@@ -56,4 +56,9 @@ public interface Transaction<T extends Transaction<T>> extends AbstractSpan<T> {
     void setFrameworkName(@Nullable String frameworkName);
 
     void setFrameworkVersion(@Nullable String frameworkVersion);
+
+    void setPendingTransactionException(Throwable exception);
+
+    @Nullable
+    Throwable getPendingTransactionException();
 }

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