Skip to content

Commit

Permalink
Merge branch 'main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
kadirtaskiran authored Feb 15, 2024
2 parents 92fe561 + 770334b commit b7fd987
Show file tree
Hide file tree
Showing 18 changed files with 162 additions and 11 deletions.
4 changes: 1 addition & 3 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes:
[float]
===== Features
* Added internal `safe_exceptions` config option to workaround JVM bugs related to touching exceptions - {pull}3528[#3528]
* Bumped base alpine docker image version - {pull}3524[#3524]
[float]
Expand All @@ -48,9 +49,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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>>() {

Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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.");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ public class Transaction extends AbstractSpan<Transaction> implements co.elastic
@Nullable
private String frameworkVersion;

@Nullable
private Throwable pendingException;

/**
* Faas
* <p>
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,7 @@ public interface Tracer {

@Nullable
Service createService(String ephemeralId);

@Nullable
Throwable redactExceptionIfRequired(@Nullable Throwable original);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public interface CoreConfiguration {

TimeDuration getSpanMinDuration();

boolean isUseServletAttributesForExceptionPropagation();

enum EventType {
/**
* Request bodies will never be reported
Expand Down

0 comments on commit b7fd987

Please sign in to comment.