From 9c4d99da73f16b5b25b1f3778541dc8d02100915 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 29 Feb 2024 16:37:34 +0100 Subject: [PATCH 1/6] remove unused captured exceptions --- .../java/co/elastic/apm/agent/bci/InstrumentationTest.java | 2 +- .../apm/agent/cassandra3/Cassandra3Instrumentation.java | 2 +- .../v5_6/ElasticsearchClientAsyncInstrumentation.java | 3 +-- .../v6_4/ElasticsearchClientAsyncInstrumentation.java | 3 +-- .../RunnableCallableForkJoinTaskInstrumentation.java | 3 +-- .../elastic/apm/agent/vertx/v3/TaskQueueInstrumentation.java | 2 +- .../elastic/apm/agent/vertx/v4/TaskQueueInstrumentation.java | 2 +- 7 files changed, 7 insertions(+), 10 deletions(-) diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/InstrumentationTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/InstrumentationTest.java index 0f3471a712..53e2954672 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/InstrumentationTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/InstrumentationTest.java @@ -603,7 +603,7 @@ public static String onMethodEnter() { @Advice.AssignReturned.ToReturned @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false) - public static String onMethodExit(@Advice.Thrown Throwable throwable) { + public static String onMethodExit() { throw new RuntimeException("This exception should be suppressed"); } } diff --git a/apm-agent-plugins/apm-cassandra/apm-cassandra3-plugin/src/main/java/co/elastic/apm/agent/cassandra3/Cassandra3Instrumentation.java b/apm-agent-plugins/apm-cassandra/apm-cassandra3-plugin/src/main/java/co/elastic/apm/agent/cassandra3/Cassandra3Instrumentation.java index 5564915e39..02aaf49a32 100644 --- a/apm-agent-plugins/apm-cassandra/apm-cassandra3-plugin/src/main/java/co/elastic/apm/agent/cassandra3/Cassandra3Instrumentation.java +++ b/apm-agent-plugins/apm-cassandra/apm-cassandra3-plugin/src/main/java/co/elastic/apm/agent/cassandra3/Cassandra3Instrumentation.java @@ -106,7 +106,7 @@ public static void onEnter() { } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false) - public static void onExit(@Advice.Thrown @Nullable Throwable thrown, @Advice.Return @Nullable Object returnValue) { + public static void onExit() { CassandraHelper.inSyncExecute(false); } } diff --git a/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-5_6/src/main/java/co/elastic/apm/agent/esrestclient/v5_6/ElasticsearchClientAsyncInstrumentation.java b/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-5_6/src/main/java/co/elastic/apm/agent/esrestclient/v5_6/ElasticsearchClientAsyncInstrumentation.java index 4bc3fa6c8c..10b79b1767 100644 --- a/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-5_6/src/main/java/co/elastic/apm/agent/esrestclient/v5_6/ElasticsearchClientAsyncInstrumentation.java +++ b/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-5_6/src/main/java/co/elastic/apm/agent/esrestclient/v5_6/ElasticsearchClientAsyncInstrumentation.java @@ -86,8 +86,7 @@ public static Object[] onBeforeExecute(@Advice.Argument(0) String method, } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false) - public static void onAfterExecute(@Advice.Thrown @Nullable Throwable t, - @Advice.Enter @Nullable Object[] entryArgs) { + public static void onAfterExecute(@Advice.Enter @Nullable Object[] entryArgs) { if (entryArgs != null) { final Span span = (Span) entryArgs[0]; if (span != null) { diff --git a/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-6_4/src/main/java/co/elastic/apm/agent/esrestclient/v6_4/ElasticsearchClientAsyncInstrumentation.java b/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-6_4/src/main/java/co/elastic/apm/agent/esrestclient/v6_4/ElasticsearchClientAsyncInstrumentation.java index d8d737609e..dc518f15c5 100644 --- a/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-6_4/src/main/java/co/elastic/apm/agent/esrestclient/v6_4/ElasticsearchClientAsyncInstrumentation.java +++ b/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-6_4/src/main/java/co/elastic/apm/agent/esrestclient/v6_4/ElasticsearchClientAsyncInstrumentation.java @@ -77,8 +77,7 @@ public static Object[] onBeforeExecute(@Advice.Argument(0) Request request, } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false) - public static void onAfterExecute(@Advice.Thrown @Nullable Throwable t, - @Advice.Enter @Nullable Object[] entryArgs) { + public static void onAfterExecute(@Advice.Enter @Nullable Object[] entryArgs) { if (entryArgs != null) { final Span span = (Span) entryArgs[0]; if (span != null) { diff --git a/apm-agent-plugins/apm-java-concurrent-plugin/src/main/java/co/elastic/apm/agent/concurrent/RunnableCallableForkJoinTaskInstrumentation.java b/apm-agent-plugins/apm-java-concurrent-plugin/src/main/java/co/elastic/apm/agent/concurrent/RunnableCallableForkJoinTaskInstrumentation.java index 12b8b96723..5ef881945c 100644 --- a/apm-agent-plugins/apm-java-concurrent-plugin/src/main/java/co/elastic/apm/agent/concurrent/RunnableCallableForkJoinTaskInstrumentation.java +++ b/apm-agent-plugins/apm-java-concurrent-plugin/src/main/java/co/elastic/apm/agent/concurrent/RunnableCallableForkJoinTaskInstrumentation.java @@ -79,8 +79,7 @@ public static Object onEnter(@Advice.This Object thiz) { } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false) - public static void onExit(@Advice.Thrown Throwable thrown, - @Nullable @Advice.Enter Object context) { + public static void onExit(@Nullable @Advice.Enter Object context) { if (context != null) { ((ElasticContext) context).deactivate(); } diff --git a/apm-agent-plugins/apm-vertx/apm-vertx3-plugin/src/main/java/co/elastic/apm/agent/vertx/v3/TaskQueueInstrumentation.java b/apm-agent-plugins/apm-vertx/apm-vertx3-plugin/src/main/java/co/elastic/apm/agent/vertx/v3/TaskQueueInstrumentation.java index caf730a326..f704b02395 100644 --- a/apm-agent-plugins/apm-vertx/apm-vertx3-plugin/src/main/java/co/elastic/apm/agent/vertx/v3/TaskQueueInstrumentation.java +++ b/apm-agent-plugins/apm-vertx/apm-vertx3-plugin/src/main/java/co/elastic/apm/agent/vertx/v3/TaskQueueInstrumentation.java @@ -58,7 +58,7 @@ public static void onEnter() { } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false) - public static void onExit(@Advice.Thrown @Nullable Throwable thrown) { + public static void onExit() { JavaConcurrent.allowContextPropagationOnCurrentThread(); } diff --git a/apm-agent-plugins/apm-vertx/apm-vertx4-plugin/src/main/java/co/elastic/apm/agent/vertx/v4/TaskQueueInstrumentation.java b/apm-agent-plugins/apm-vertx/apm-vertx4-plugin/src/main/java/co/elastic/apm/agent/vertx/v4/TaskQueueInstrumentation.java index bae0962152..85e905bf19 100644 --- a/apm-agent-plugins/apm-vertx/apm-vertx4-plugin/src/main/java/co/elastic/apm/agent/vertx/v4/TaskQueueInstrumentation.java +++ b/apm-agent-plugins/apm-vertx/apm-vertx4-plugin/src/main/java/co/elastic/apm/agent/vertx/v4/TaskQueueInstrumentation.java @@ -60,7 +60,7 @@ public static void onEnter() { } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false) - public static void onExit(@Advice.Thrown @Nullable Throwable thrown) { + public static void onExit() { JavaConcurrent.allowContextPropagationOnCurrentThread(); } From ea5372527a6f930f095be5734e25473664d3d968 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 29 Feb 2024 17:11:36 +0100 Subject: [PATCH 2/6] add list of TODOs --- .../apm/agent/httpclient/v3/HttpClient3Instrumentation.java | 2 +- .../httpclient/v4/LegacyApacheHttpClientInstrumentation.java | 2 +- .../httpclient/v4/helper/ApacheHttpClient4ApiAdapter.java | 2 +- .../httpclient/v5/helper/ApacheHttpClient5ApiAdapter.java | 2 +- .../ElasticsearchRestClientInstrumentationHelper.java | 4 ++-- .../src/main/java/co/elastic/apm/agent/grpc/GrpcHelper.java | 2 +- .../apm/agent/kafka/KafkaProducerHeadersInstrumentation.java | 2 +- .../apm/agent/okhttp/OkHttp3ClientAsyncInstrumentation.java | 4 ++-- .../apm/agent/okhttp/OkHttpClientAsyncInstrumentation.java | 2 +- .../elastic/apm/agent/process/ProcessExitInstrumentation.java | 2 +- .../java/co/elastic/apm/agent/servlet/ServletApiAdvice.java | 2 +- 11 files changed, 13 insertions(+), 13 deletions(-) diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient3-plugin/src/main/java/co/elastic/apm/agent/httpclient/v3/HttpClient3Instrumentation.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient3-plugin/src/main/java/co/elastic/apm/agent/httpclient/v3/HttpClient3Instrumentation.java index 30972b9675..7ab14d80d4 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient3-plugin/src/main/java/co/elastic/apm/agent/httpclient/v3/HttpClient3Instrumentation.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient3-plugin/src/main/java/co/elastic/apm/agent/httpclient/v3/HttpClient3Instrumentation.java @@ -146,7 +146,7 @@ public static void onExit(@Advice.Thrown @Nullable Throwable thrown, span.getContext().getHttp().withStatusCode(statusLine.getStatusCode()); } - if (thrown instanceof CircularRedirectException) { + if (thrown instanceof CircularRedirectException) { // TODO span.withOutcome(Outcome.FAILURE); } diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java index e186e43d27..1517af8c5b 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java @@ -125,7 +125,7 @@ public static void onAfterExecute(@Advice.Return @Nullable HttpResponse response } finally { // in case of circular redirect, we get an exception but status code won't be available without response // thus we have to deal with span outcome directly - if (t instanceof CircularRedirectException) { + if (t instanceof CircularRedirectException) { // TODO span.withOutcome(Outcome.FAILURE); } diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/ApacheHttpClient4ApiAdapter.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/ApacheHttpClient4ApiAdapter.java index 13bf22af4e..11a7e7892e 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/ApacheHttpClient4ApiAdapter.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/ApacheHttpClient4ApiAdapter.java @@ -65,7 +65,7 @@ public int getResponseCode(CloseableHttpResponse closeableHttpResponse) { @Override public boolean isCircularRedirectException(Throwable t) { - return t instanceof CircularRedirectException; + return t instanceof CircularRedirectException; // TODO } @Override diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/ApacheHttpClient5ApiAdapter.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/ApacheHttpClient5ApiAdapter.java index e4ce612dd1..4185c8db7a 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/ApacheHttpClient5ApiAdapter.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/ApacheHttpClient5ApiAdapter.java @@ -78,7 +78,7 @@ public int getResponseCode(CloseableHttpResponse closeableHttpResponse) { @Override public boolean isCircularRedirectException(Throwable t) { - return t instanceof CircularRedirectException; + return t instanceof CircularRedirectException; // TODO } @Override diff --git a/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-common/src/main/java/co/elastic/apm/agent/esrestclient/ElasticsearchRestClientInstrumentationHelper.java b/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-common/src/main/java/co/elastic/apm/agent/esrestclient/ElasticsearchRestClientInstrumentationHelper.java index 759cb061d0..90db7c9d2a 100644 --- a/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-common/src/main/java/co/elastic/apm/agent/esrestclient/ElasticsearchRestClientInstrumentationHelper.java +++ b/apm-agent-plugins/apm-es-restclient-plugin/apm-es-restclient-plugin-common/src/main/java/co/elastic/apm/agent/esrestclient/ElasticsearchRestClientInstrumentationHelper.java @@ -154,14 +154,14 @@ public void finishClientSpan(@Nullable Response response, Span span, @Nullabl cluster = response.getHeader("x-found-handling-cluster"); } else if (t != null) { - if (t instanceof ResponseException) { + if (t instanceof ResponseException) { // TODO ResponseException esre = (ResponseException) t; HttpHost host = esre.getResponse().getHost(); address = host.getHostName(); port = host.getPort(); url = host.toURI(); statusCode = esre.getResponse().getStatusLine().getStatusCode(); - } else if (t instanceof CancellationException) { + } else if (t instanceof CancellationException) { // TODO // We can't tell whether a cancelled search is related to a failure or not span.withOutcome(Outcome.UNKNOWN); } diff --git a/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/main/java/co/elastic/apm/agent/grpc/GrpcHelper.java b/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/main/java/co/elastic/apm/agent/grpc/GrpcHelper.java index 012ec45dbe..3b60b473d6 100644 --- a/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/main/java/co/elastic/apm/agent/grpc/GrpcHelper.java +++ b/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/main/java/co/elastic/apm/agent/grpc/GrpcHelper.java @@ -257,7 +257,7 @@ public void exitServerListenerMethod(@Nullable Throwable thrown, if (null != thrown) { // when there is a runtime exception thrown in one of the listener methods the calling code will catch it // and make this the last listener method called - terminateStatus = Status.fromThrowable(thrown); + terminateStatus = Status.fromThrowable(thrown); // TODO setTerminateStatus = true; } else if (transaction.getOutcome() == Outcome.UNKNOWN) { diff --git a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/main/java/co/elastic/apm/agent/kafka/KafkaProducerHeadersInstrumentation.java b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/main/java/co/elastic/apm/agent/kafka/KafkaProducerHeadersInstrumentation.java index c51890a0a5..a8821bfa75 100644 --- a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/main/java/co/elastic/apm/agent/kafka/KafkaProducerHeadersInstrumentation.java +++ b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/main/java/co/elastic/apm/agent/kafka/KafkaProducerHeadersInstrumentation.java @@ -112,7 +112,7 @@ public static Object[] afterSend(@Advice.Enter @Nullable Object[] enter, return null; } Object[] overrideThrowable = null; - if (throwable != null && throwable.getMessage().contains("Magic v1 does not support record headers")) { + if (throwable != null && throwable.getMessage().contains("Magic v1 does not support record headers")) { // TODO // Probably our fault - ignore span and retry. May happen when using a new client with an old (< 0.11.0) // broker. In such cases we DO check the version, but the first version check may be not yet up to date. logger.info("Adding header to Kafka record is not allowed with the used broker, attempting to resend record"); diff --git a/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttp3ClientAsyncInstrumentation.java b/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttp3ClientAsyncInstrumentation.java index 6e192c571e..d14c90983b 100644 --- a/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttp3ClientAsyncInstrumentation.java +++ b/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttp3ClientAsyncInstrumentation.java @@ -123,7 +123,7 @@ public void onFailure(Call call, IOException e) { try { span.captureException(e).withOutcome(Outcome.FAILURE).end(); } catch (Throwable t) { - logger.error(t.getMessage(), t); + logger.error(t.getMessage(), t); // TODO } finally { delegate.onFailure(call, e); } @@ -135,7 +135,7 @@ public void onResponse(Call call, Response response) throws IOException { span.getContext().getHttp().withStatusCode(response.code()); span.end(); } catch (Throwable t) { - logger.error(t.getMessage(), t); + logger.error(t.getMessage(), t); // TODO } finally { delegate.onResponse(call, response); } diff --git a/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttpClientAsyncInstrumentation.java b/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttpClientAsyncInstrumentation.java index aedf7110b8..8c30a76f83 100644 --- a/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttpClientAsyncInstrumentation.java +++ b/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttpClientAsyncInstrumentation.java @@ -124,7 +124,7 @@ public void onFailure(Request req, IOException e) { .withOutcome(Outcome.FAILURE) .end(); } catch (Throwable t) { - logger.error(t.getMessage(), t); + logger.error(t.getMessage(), t); // TODO } finally { delegate.onFailure(req, e); } diff --git a/apm-agent-plugins/apm-process-plugin/src/main/java/co/elastic/apm/agent/process/ProcessExitInstrumentation.java b/apm-agent-plugins/apm-process-plugin/src/main/java/co/elastic/apm/agent/process/ProcessExitInstrumentation.java index 3a33315a3c..e21131aca0 100644 --- a/apm-agent-plugins/apm-process-plugin/src/main/java/co/elastic/apm/agent/process/ProcessExitInstrumentation.java +++ b/apm-agent-plugins/apm-process-plugin/src/main/java/co/elastic/apm/agent/process/ProcessExitInstrumentation.java @@ -145,7 +145,7 @@ public static void onExit(@Advice.This Process process, @Advice.Thrown Throwable return; } - if (thrown instanceof IllegalThreadStateException) { + if (thrown instanceof IllegalThreadStateException) { // TODO ? // this call to exitValue was invoked before the process had terminated return; } 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 07b98092ac..8039da604e 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 @@ -239,7 +239,7 @@ public static Date: Thu, 29 Feb 2024 17:19:57 +0100 Subject: [PATCH 3/6] remove false positive --- .../apm/agent/okhttp/OkHttp3ClientAsyncInstrumentation.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttp3ClientAsyncInstrumentation.java b/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttp3ClientAsyncInstrumentation.java index d14c90983b..6e192c571e 100644 --- a/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttp3ClientAsyncInstrumentation.java +++ b/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttp3ClientAsyncInstrumentation.java @@ -123,7 +123,7 @@ public void onFailure(Call call, IOException e) { try { span.captureException(e).withOutcome(Outcome.FAILURE).end(); } catch (Throwable t) { - logger.error(t.getMessage(), t); // TODO + logger.error(t.getMessage(), t); } finally { delegate.onFailure(call, e); } @@ -135,7 +135,7 @@ public void onResponse(Call call, Response response) throws IOException { span.getContext().getHttp().withStatusCode(response.code()); span.end(); } catch (Throwable t) { - logger.error(t.getMessage(), t); // TODO + logger.error(t.getMessage(), t); } finally { delegate.onResponse(call, response); } From c73c8486eaf3812236d32e4c51d82293e2d03503 Mon Sep 17 00:00:00 2001 From: Sylvain Juge <763082+SylvainJuge@users.noreply.github.com> Date: Thu, 29 Feb 2024 17:24:05 +0100 Subject: [PATCH 4/6] remove another false positive --- .../apm/agent/okhttp/OkHttpClientAsyncInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttpClientAsyncInstrumentation.java b/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttpClientAsyncInstrumentation.java index 8c30a76f83..aedf7110b8 100644 --- a/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttpClientAsyncInstrumentation.java +++ b/apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttpClientAsyncInstrumentation.java @@ -124,7 +124,7 @@ public void onFailure(Request req, IOException e) { .withOutcome(Outcome.FAILURE) .end(); } catch (Throwable t) { - logger.error(t.getMessage(), t); // TODO + logger.error(t.getMessage(), t); } finally { delegate.onFailure(req, e); } From 6cd0f9be28f3e8cc64bb691fd94ec4f308032603 Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Mon, 4 Mar 2024 13:52:05 +0100 Subject: [PATCH 5/6] Prevent touching exceptions --- .../configuration/CoreConfiguration.java | 5 ++++ .../v3/HttpClient3Instrumentation.java | 7 ++++-- ...LegacyApacheHttpClientInstrumentation.java | 12 ++++++---- .../helper/ApacheHttpClient4ApiAdapter.java | 10 +++++++- .../helper/ApacheHttpClient5ApiAdapter.java | 10 +++++++- ...searchRestClientInstrumentationHelper.java | 23 +++++++++++-------- .../co/elastic/apm/agent/grpc/GrpcHelper.java | 7 +++++- .../KafkaProducerHeadersInstrumentation.java | 5 +++- .../process/ProcessExitInstrumentation.java | 4 +++- .../apm/agent/servlet/ServletApiAdvice.java | 6 ++++- .../configuration/CoreConfiguration.java | 2 ++ 11 files changed, 69 insertions(+), 22 deletions(-) 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 c185bbc314..36d8c42b85 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 @@ -1175,6 +1175,11 @@ public boolean isRedactExceptions() { return (safeExceptions.get() & 1) != 0; } + @Override + public boolean isAvoidTouchingExceptions() { + return isRedactExceptions() || !captureExceptionDetails(); + } + @Override public boolean isUseServletAttributesForExceptionPropagation() { return (safeExceptions.get() & 2) == 0; diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient3-plugin/src/main/java/co/elastic/apm/agent/httpclient/v3/HttpClient3Instrumentation.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient3-plugin/src/main/java/co/elastic/apm/agent/httpclient/v3/HttpClient3Instrumentation.java index 7ab14d80d4..4f178a6df1 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient3-plugin/src/main/java/co/elastic/apm/agent/httpclient/v3/HttpClient3Instrumentation.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient3-plugin/src/main/java/co/elastic/apm/agent/httpclient/v3/HttpClient3Instrumentation.java @@ -28,6 +28,7 @@ import co.elastic.apm.agent.tracer.Outcome; import co.elastic.apm.agent.tracer.Span; import co.elastic.apm.agent.tracer.Tracer; +import co.elastic.apm.agent.tracer.configuration.CoreConfiguration; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; @@ -146,8 +147,10 @@ public static void onExit(@Advice.Thrown @Nullable Throwable thrown, span.getContext().getHttp().withStatusCode(statusLine.getStatusCode()); } - if (thrown instanceof CircularRedirectException) { // TODO - span.withOutcome(Outcome.FAILURE); + if(thrown != null && !tracer.getConfig(CoreConfiguration.class).isAvoidTouchingExceptions()) { + if (thrown instanceof CircularRedirectException) { + span.withOutcome(Outcome.FAILURE); + } } span.captureException(thrown) diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java index 1517af8c5b..5125477133 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/LegacyApacheHttpClientInstrumentation.java @@ -23,6 +23,7 @@ import co.elastic.apm.agent.tracer.ElasticContext; import co.elastic.apm.agent.tracer.Outcome; import co.elastic.apm.agent.tracer.Span; +import co.elastic.apm.agent.tracer.configuration.CoreConfiguration; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; @@ -123,10 +124,13 @@ public static void onAfterExecute(@Advice.Return @Nullable HttpResponse response } span.captureException(t); } finally { - // in case of circular redirect, we get an exception but status code won't be available without response - // thus we have to deal with span outcome directly - if (t instanceof CircularRedirectException) { // TODO - span.withOutcome(Outcome.FAILURE); + + if(t != null && !tracer.getConfig(CoreConfiguration.class).isAvoidTouchingExceptions()) { + // in case of circular redirect, we get an exception but status code won't be available without response + // thus we have to deal with span outcome directly + if (t instanceof CircularRedirectException) { + span.withOutcome(Outcome.FAILURE); + } } span.deactivate().end(); diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/ApacheHttpClient4ApiAdapter.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/ApacheHttpClient4ApiAdapter.java index 11a7e7892e..fd763f23d5 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/ApacheHttpClient4ApiAdapter.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient4-plugin/src/main/java/co/elastic/apm/agent/httpclient/v4/helper/ApacheHttpClient4ApiAdapter.java @@ -20,6 +20,9 @@ import co.elastic.apm.agent.httpclient.common.ApacheHttpClientApiAdapter; +import co.elastic.apm.agent.tracer.GlobalTracer; +import co.elastic.apm.agent.tracer.Tracer; +import co.elastic.apm.agent.tracer.configuration.CoreConfiguration; import org.apache.http.HttpHost; import org.apache.http.HttpRequest; import org.apache.http.StatusLine; @@ -32,6 +35,8 @@ public class ApacheHttpClient4ApiAdapter implements ApacheHttpClientApiAdapter { private static final ApacheHttpClient4ApiAdapter INSTANCE = new ApacheHttpClient4ApiAdapter(); + private final Tracer tracer = GlobalTracer.get(); + private ApacheHttpClient4ApiAdapter() { } @@ -65,7 +70,10 @@ public int getResponseCode(CloseableHttpResponse closeableHttpResponse) { @Override public boolean isCircularRedirectException(Throwable t) { - return t instanceof CircularRedirectException; // TODO + if (t == null || tracer.getConfig(CoreConfiguration.class).isAvoidTouchingExceptions()) { + return false; + } + return t instanceof CircularRedirectException; } @Override diff --git a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/ApacheHttpClient5ApiAdapter.java b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/ApacheHttpClient5ApiAdapter.java index 4185c8db7a..233a92188a 100644 --- a/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/ApacheHttpClient5ApiAdapter.java +++ b/apm-agent-plugins/apm-apache-httpclient/apm-apache-httpclient5-plugin/src/main/java/co/elastic/apm/agent/httpclient/v5/helper/ApacheHttpClient5ApiAdapter.java @@ -21,6 +21,9 @@ import co.elastic.apm.agent.httpclient.common.ApacheHttpClientApiAdapter; import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; +import co.elastic.apm.agent.tracer.GlobalTracer; +import co.elastic.apm.agent.tracer.Tracer; +import co.elastic.apm.agent.tracer.configuration.CoreConfiguration; import org.apache.hc.client5.http.CircularRedirectException; import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; import org.apache.hc.client5.http.routing.RoutingSupport; @@ -38,6 +41,8 @@ public class ApacheHttpClient5ApiAdapter implements ApacheHttpClientApiAdapter span, @Nullabl cluster = response.getHeader("x-found-handling-cluster"); } else if (t != null) { - if (t instanceof ResponseException) { // TODO - ResponseException esre = (ResponseException) t; - HttpHost host = esre.getResponse().getHost(); - address = host.getHostName(); - port = host.getPort(); - url = host.toURI(); - statusCode = esre.getResponse().getStatusLine().getStatusCode(); - } else if (t instanceof CancellationException) { // TODO - // We can't tell whether a cancelled search is related to a failure or not - span.withOutcome(Outcome.UNKNOWN); + if (!tracer.getConfig(CoreConfiguration.class).isAvoidTouchingExceptions()) { + if (t instanceof ResponseException) { + ResponseException esre = (ResponseException) t; + HttpHost host = esre.getResponse().getHost(); + address = host.getHostName(); + port = host.getPort(); + url = host.toURI(); + statusCode = esre.getResponse().getStatusLine().getStatusCode(); + } else if (t instanceof CancellationException) { + // We can't tell whether a cancelled search is related to a failure or not + span.withOutcome(Outcome.UNKNOWN); + } } span.captureException(t); } diff --git a/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/main/java/co/elastic/apm/agent/grpc/GrpcHelper.java b/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/main/java/co/elastic/apm/agent/grpc/GrpcHelper.java index 3b60b473d6..495a02eb2b 100644 --- a/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/main/java/co/elastic/apm/agent/grpc/GrpcHelper.java +++ b/apm-agent-plugins/apm-grpc/apm-grpc-plugin/src/main/java/co/elastic/apm/agent/grpc/GrpcHelper.java @@ -27,6 +27,7 @@ import co.elastic.apm.agent.tracer.Span; import co.elastic.apm.agent.tracer.Tracer; import co.elastic.apm.agent.tracer.Transaction; +import co.elastic.apm.agent.tracer.configuration.CoreConfiguration; import co.elastic.apm.agent.tracer.dispatch.AbstractHeaderGetter; import co.elastic.apm.agent.tracer.dispatch.TextHeaderGetter; import co.elastic.apm.agent.tracer.dispatch.TextHeaderSetter; @@ -257,7 +258,11 @@ public void exitServerListenerMethod(@Nullable Throwable thrown, if (null != thrown) { // when there is a runtime exception thrown in one of the listener methods the calling code will catch it // and make this the last listener method called - terminateStatus = Status.fromThrowable(thrown); // TODO + if (tracer.getConfig(CoreConfiguration.class).isAvoidTouchingExceptions()) { + terminateStatus = Status.UNKNOWN; + } else { + terminateStatus = Status.fromThrowable(thrown); + } setTerminateStatus = true; } else if (transaction.getOutcome() == Outcome.UNKNOWN) { diff --git a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/main/java/co/elastic/apm/agent/kafka/KafkaProducerHeadersInstrumentation.java b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/main/java/co/elastic/apm/agent/kafka/KafkaProducerHeadersInstrumentation.java index a8821bfa75..f034815c46 100644 --- a/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/main/java/co/elastic/apm/agent/kafka/KafkaProducerHeadersInstrumentation.java +++ b/apm-agent-plugins/apm-kafka-plugin/apm-kafka-headers-plugin/src/main/java/co/elastic/apm/agent/kafka/KafkaProducerHeadersInstrumentation.java @@ -23,6 +23,7 @@ import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; import co.elastic.apm.agent.tracer.Span; +import co.elastic.apm.agent.tracer.configuration.CoreConfiguration; import net.bytebuddy.asm.Advice; import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument; import net.bytebuddy.description.method.MethodDescription; @@ -112,7 +113,9 @@ public static Object[] afterSend(@Advice.Enter @Nullable Object[] enter, return null; } Object[] overrideThrowable = null; - if (throwable != null && throwable.getMessage().contains("Magic v1 does not support record headers")) { // TODO + if (throwable != null + && !tracer.getConfig(CoreConfiguration.class).isAvoidTouchingExceptions() + && throwable.getMessage().contains("Magic v1 does not support record headers")) { // Probably our fault - ignore span and retry. May happen when using a new client with an old (< 0.11.0) // broker. In such cases we DO check the version, but the first version check may be not yet up to date. logger.info("Adding header to Kafka record is not allowed with the used broker, attempting to resend record"); diff --git a/apm-agent-plugins/apm-process-plugin/src/main/java/co/elastic/apm/agent/process/ProcessExitInstrumentation.java b/apm-agent-plugins/apm-process-plugin/src/main/java/co/elastic/apm/agent/process/ProcessExitInstrumentation.java index e21131aca0..b6c38cc6fd 100644 --- a/apm-agent-plugins/apm-process-plugin/src/main/java/co/elastic/apm/agent/process/ProcessExitInstrumentation.java +++ b/apm-agent-plugins/apm-process-plugin/src/main/java/co/elastic/apm/agent/process/ProcessExitInstrumentation.java @@ -18,6 +18,7 @@ */ package co.elastic.apm.agent.process; +import co.elastic.apm.agent.tracer.configuration.CoreConfiguration; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; @@ -145,7 +146,8 @@ public static void onExit(@Advice.This Process process, @Advice.Thrown Throwable return; } - if (thrown instanceof IllegalThreadStateException) { // TODO ? + if (!tracer.getConfig(CoreConfiguration.class).isAvoidTouchingExceptions() + && thrown instanceof IllegalThreadStateException) { // this call to exitValue was invoked before the process had terminated return; } 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 8039da604e..65695644e1 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 @@ -239,7 +239,11 @@ public static Date: Mon, 4 Mar 2024 13:57:09 +0100 Subject: [PATCH 6/6] Added changelog --- CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index a7d1d31ffb..c6d4ae1b6a 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -39,6 +39,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes: [float] ===== Bug fixes * Added missing support for TracerBuilder in OpenTelemetry bridge - {pull}3535[#3535] +* Fixed some locations to not touch exceptions when `safe_exception` is configured - {pull}3543[#3543] [float] ===== Potentially breaking changes