From 27751d700181d37a9e9ad91b5f26d0fb9edcb714 Mon Sep 17 00:00:00 2001 From: Rafael Winterhalter Date: Tue, 8 Aug 2023 01:23:29 +0200 Subject: [PATCH 1/2] Add error reporting API to tracer-api. --- .../apm/agent/impl/ElasticApmTracer.java | 12 ++++++++ .../apm/agent/impl/error/ErrorCapture.java | 4 ++- .../CaptureExceptionInstrumentation.java | 7 +++-- .../ElasticApmApiInstrumentation.java | 6 +++- .../Log4j2_7PlusLogCorrelationHelper.java | 2 +- .../AbstractLogCorrelationHelper.java | 4 +-- .../correlation/CorrelationIdMapAdapter.java | 4 +-- .../loginstr/error/LoggerErrorHelper.java | 5 ++-- .../apm/agent/tracer/AbstractSpan.java | 2 +- .../apm/agent/tracer/Activateable.java | 7 ----- .../apm/agent/tracer/ActivateableInScope.java | 29 +++++++++++++++++++ .../apm/agent/tracer/ElasticContext.java | 2 +- .../apm/agent/tracer/ErrorCapture.java | 26 +++++++++++++++++ .../apm/agent/tracer/GlobalTracer.java | 12 ++++++++ .../elastic/apm/agent/tracer/NoopTracer.java | 12 ++++++++ .../co/elastic/apm/agent/tracer/Tracer.java | 7 ++++- 16 files changed, 119 insertions(+), 22 deletions(-) create mode 100644 apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/ActivateableInScope.java create mode 100644 apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/ErrorCapture.java 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 f6e8d5edbf..e2f6c997d0 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 @@ -361,6 +361,12 @@ public Transaction currentTransaction() { return activeStack.get().currentTransaction(); } + @Nullable + @Override + public co.elastic.apm.agent.tracer.ErrorCapture getActiveError() { + return ErrorCapture.getActive(); + } + /** * Starts a span with a given parent context. *

@@ -434,6 +440,12 @@ public ErrorCapture captureException(@Nullable Throwable e, @Nullable AbstractSp return captureException(System.currentTimeMillis() * 1000, e, parent, initiatingClassLoader); } + @Nullable + @Override + public ErrorCapture captureException(@Nullable Throwable e, @Nullable ClassLoader initiatingClassLoader) { + return captureException(System.currentTimeMillis() * 1000, e, getActive(), initiatingClassLoader); + } + @Nullable private ErrorCapture captureException(long epochMicros, @Nullable Throwable e, @Nullable AbstractSpan parent, @Nullable ClassLoader initiatingClassLoader) { if (!isRunning() || e == null) { diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/error/ErrorCapture.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/error/ErrorCapture.java index 5f63869587..315bed3224 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/error/ErrorCapture.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/error/ErrorCapture.java @@ -37,7 +37,7 @@ /** * Data captured by an agent representing an event occurring in a monitored service */ -public class ErrorCapture implements Recyclable { +public class ErrorCapture implements Recyclable, co.elastic.apm.agent.tracer.ErrorCapture { private static final Logger logger = LoggerFactory.getLogger(ErrorCapture.class); @@ -199,11 +199,13 @@ private void setCulprit(StackTraceElement stackTraceElement) { culprit.append(')'); } + @Override public ErrorCapture activate() { activeError.set(this); return this; } + @Override public ErrorCapture deactivate() { activeError.remove(); return this; diff --git a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/CaptureExceptionInstrumentation.java b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/CaptureExceptionInstrumentation.java index 7d7f1f179d..4afe5ef561 100644 --- a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/CaptureExceptionInstrumentation.java +++ b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/CaptureExceptionInstrumentation.java @@ -18,8 +18,8 @@ */ package co.elastic.apm.agent.pluginapi; -import co.elastic.apm.agent.impl.Tracer; import co.elastic.apm.agent.sdk.internal.util.PrivilegedActionUtils; +import co.elastic.apm.agent.tracer.ErrorCapture; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; @@ -33,7 +33,10 @@ public class CaptureExceptionInstrumentation extends ApiInstrumentation { public static class AdviceClass { @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) public static void captureException(@Advice.Origin Class clazz, @Advice.Argument(0) Throwable t) { - tracer.require(Tracer.class).captureAndReportException(t, PrivilegedActionUtils.getClassLoader(clazz)); + ErrorCapture errorCapture = tracer.captureException(t, PrivilegedActionUtils.getClassLoader(clazz)); + if (errorCapture != null) { + errorCapture.end(); + } } } diff --git a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/ElasticApmApiInstrumentation.java b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/ElasticApmApiInstrumentation.java index 105d55a0c7..6be447f2f4 100644 --- a/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/ElasticApmApiInstrumentation.java +++ b/apm-agent-plugins/apm-api-plugin/src/main/java/co/elastic/apm/agent/pluginapi/ElasticApmApiInstrumentation.java @@ -21,6 +21,7 @@ import co.elastic.apm.agent.configuration.ServiceInfo; import co.elastic.apm.agent.impl.ElasticApmTracer; import co.elastic.apm.agent.impl.Tracer; +import co.elastic.apm.agent.tracer.ErrorCapture; import co.elastic.apm.agent.tracer.GlobalTracer; import co.elastic.apm.agent.tracer.Transaction; import co.elastic.apm.agent.sdk.internal.util.PrivilegedActionUtils; @@ -149,7 +150,10 @@ public CaptureExceptionInstrumentation() { public static class AdviceClass { @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) public static void captureException(@Advice.Origin Class clazz, @Advice.Argument(0) @Nullable Throwable e) { - tracer.require(Tracer.class).captureAndReportException(e, PrivilegedActionUtils.getClassLoader(clazz)); + ErrorCapture errorCapture = tracer.captureException(e, PrivilegedActionUtils.getClassLoader(clazz)); + if (errorCapture != null) { + errorCapture.end(); + } } } } diff --git a/apm-agent-plugins/apm-logging-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/correlation/Log4j2_7PlusLogCorrelationHelper.java b/apm-agent-plugins/apm-logging-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/correlation/Log4j2_7PlusLogCorrelationHelper.java index 65e4971f0c..0551af209a 100644 --- a/apm-agent-plugins/apm-logging-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/correlation/Log4j2_7PlusLogCorrelationHelper.java +++ b/apm-agent-plugins/apm-logging-plugin/apm-log4j2-plugin/src/main/java/co/elastic/apm/agent/log4j2/correlation/Log4j2_7PlusLogCorrelationHelper.java @@ -36,7 +36,7 @@ public class Log4j2_7PlusLogCorrelationHelper extends AbstractLogCorrelationHelp @Override protected boolean addToMdc() { - if (tracer.currentTransaction() == null && ErrorCapture.getActive() == null) { + if (tracer.currentTransaction() == null && tracer.getActiveError() == null) { return false; } ThreadContext.putAll(CorrelationIdMapAdapter.get()); diff --git a/apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/main/java/co/elastic/apm/agent/loginstr/correlation/AbstractLogCorrelationHelper.java b/apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/main/java/co/elastic/apm/agent/loginstr/correlation/AbstractLogCorrelationHelper.java index a939ca184f..09828a84d0 100644 --- a/apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/main/java/co/elastic/apm/agent/loginstr/correlation/AbstractLogCorrelationHelper.java +++ b/apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/main/java/co/elastic/apm/agent/loginstr/correlation/AbstractLogCorrelationHelper.java @@ -19,8 +19,8 @@ package co.elastic.apm.agent.loginstr.correlation; import co.elastic.apm.agent.tracer.AbstractSpan; +import co.elastic.apm.agent.tracer.ErrorCapture; import co.elastic.apm.agent.tracer.GlobalTracer; -import co.elastic.apm.agent.impl.error.ErrorCapture; import co.elastic.apm.agent.sdk.state.CallDepth; import co.elastic.apm.agent.sdk.state.GlobalState; import co.elastic.apm.agent.tracer.Tracer; @@ -80,7 +80,7 @@ protected boolean addToMdc() { addToMdc(TRANSACTION_ID_MDC_KEY, activeSpan.getTraceContext().getTransactionId().toString()); addedToMdc = true; } - ErrorCapture activeError = ErrorCapture.getActive(); + ErrorCapture activeError = tracer.getActiveError(); if (activeError != null) { addToMdc(ERROR_ID_MDC_KEY, activeError.getTraceContext().getId().toString()); addedToMdc = true; diff --git a/apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/main/java/co/elastic/apm/agent/loginstr/correlation/CorrelationIdMapAdapter.java b/apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/main/java/co/elastic/apm/agent/loginstr/correlation/CorrelationIdMapAdapter.java index 7262590c8e..7548453060 100644 --- a/apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/main/java/co/elastic/apm/agent/loginstr/correlation/CorrelationIdMapAdapter.java +++ b/apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/main/java/co/elastic/apm/agent/loginstr/correlation/CorrelationIdMapAdapter.java @@ -19,8 +19,8 @@ package co.elastic.apm.agent.loginstr.correlation; import co.elastic.apm.agent.tracer.AbstractSpan; +import co.elastic.apm.agent.tracer.ErrorCapture; import co.elastic.apm.agent.tracer.GlobalTracer; -import co.elastic.apm.agent.impl.error.ErrorCapture; import co.elastic.apm.agent.tracer.Tracer; import javax.annotation.Nullable; @@ -71,7 +71,7 @@ public String call() { @Override @Nullable public String call() { - ErrorCapture error = ErrorCapture.getActive(); + ErrorCapture error = tracer.getActiveError(); if (error == null) { return null; } diff --git a/apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/main/java/co/elastic/apm/agent/loginstr/error/LoggerErrorHelper.java b/apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/main/java/co/elastic/apm/agent/loginstr/error/LoggerErrorHelper.java index 521e241ccc..962c58c65e 100644 --- a/apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/main/java/co/elastic/apm/agent/loginstr/error/LoggerErrorHelper.java +++ b/apm-agent-plugins/apm-logging-plugin/apm-logging-plugin-common/src/main/java/co/elastic/apm/agent/loginstr/error/LoggerErrorHelper.java @@ -18,8 +18,8 @@ */ package co.elastic.apm.agent.loginstr.error; -import co.elastic.apm.agent.impl.error.ErrorCapture; import co.elastic.apm.agent.sdk.state.CallDepth; +import co.elastic.apm.agent.tracer.ErrorCapture; import co.elastic.apm.agent.tracer.Tracer; import co.elastic.apm.agent.sdk.internal.util.PrivilegedActionUtils; @@ -46,8 +46,7 @@ public LoggerErrorHelper(Class adviceClass, Tracer tracer) { public Object enter(@Nullable Throwable exception, Class originClass) { if (!callDepth.isNestedCallAndIncrement()) { if (exception != null) { - co.elastic.apm.agent.impl.Tracer required = tracer.require(co.elastic.apm.agent.impl.Tracer.class); - ErrorCapture error = required.captureException(exception, required.getActive(), PrivilegedActionUtils.getClassLoader(originClass)); + ErrorCapture error = tracer.captureException(exception, PrivilegedActionUtils.getClassLoader(originClass)); if (error != null) { error.activate(); } diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/AbstractSpan.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/AbstractSpan.java index 481ac69c1e..173b6dd9bc 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/AbstractSpan.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/AbstractSpan.java @@ -24,7 +24,7 @@ import javax.annotation.Nullable; -public interface AbstractSpan> extends Activateable, ReferenceCounted { +public interface AbstractSpan> extends ActivateableInScope, ReferenceCounted { int PRIORITY_DEFAULT = 0; int PRIORITY_LOW_LEVEL_FRAMEWORK = 10; diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Activateable.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Activateable.java index 737d5276b0..196a393a05 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Activateable.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/Activateable.java @@ -33,11 +33,4 @@ public interface Activateable> { * @return this */ T deactivate(); - - /** - * Activates context in a scope - * - * @return active scope that will deactivate context when closed - */ - Scope activateInScope(); } diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/ActivateableInScope.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/ActivateableInScope.java new file mode 100644 index 0000000000..4b9b4eeec8 --- /dev/null +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/ActivateableInScope.java @@ -0,0 +1,29 @@ +/* + * 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.tracer; + +public interface ActivateableInScope> extends Activateable { + + /** + * Activates context in a scope + * + * @return active scope that will deactivate context when closed + */ + Scope activateInScope(); +} diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/ElasticContext.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/ElasticContext.java index a36a74ec7b..0db6cc99eb 100644 --- a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/ElasticContext.java +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/ElasticContext.java @@ -25,7 +25,7 @@ import javax.annotation.Nullable; -public interface ElasticContext> extends ReferenceCounted, Activateable { +public interface ElasticContext> extends ActivateableInScope, ReferenceCounted { /** * @return the span/transaction that is associated to this context, {@literal null} if there is none diff --git a/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/ErrorCapture.java b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/ErrorCapture.java new file mode 100644 index 0000000000..2441b0e825 --- /dev/null +++ b/apm-agent-tracer/src/main/java/co/elastic/apm/agent/tracer/ErrorCapture.java @@ -0,0 +1,26 @@ +/* + * 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.tracer; + +public interface ErrorCapture extends Activateable { + + TraceContext getTraceContext(); + + void end(); +} 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 7dbb91ab95..85c34b4320 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 @@ -111,6 +111,12 @@ public Transaction currentTransaction() { return tracer.currentTransaction(); } + @Nullable + @Override + public ErrorCapture getActiveError() { + return tracer.getActiveError(); + } + @Nullable @Override public Transaction startRootTransaction(@Nullable ClassLoader initiatingClassLoader) { @@ -128,4 +134,10 @@ public Transaction startChildTransaction(@Nullable C headerCarrier, TextH public Transaction startChildTransaction(@Nullable C headerCarrier, BinaryHeaderGetter binaryHeadersGetter, @Nullable ClassLoader initiatingClassLoader) { return tracer.startChildTransaction(headerCarrier, binaryHeadersGetter, initiatingClassLoader); } + + @Nullable + @Override + public ErrorCapture captureException(@Nullable Throwable e, @Nullable ClassLoader initiatingClassLoader) { + return tracer.captureException(e, initiatingClassLoader); + } } 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 6b8490cbe4..e66281387a 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 @@ -88,6 +88,12 @@ public Transaction currentTransaction() { return null; } + @Nullable + @Override + public ErrorCapture getActiveError() { + return null; + } + @Nullable @Override public Transaction startRootTransaction(@Nullable ClassLoader initiatingClassLoader) { @@ -105,4 +111,10 @@ public Transaction startChildTransaction(@Nullable C headerCarrier, TextH public Transaction startChildTransaction(@Nullable C headerCarrier, BinaryHeaderGetter binaryHeadersGetter, @Nullable ClassLoader initiatingClassLoader) { return null; } + + @Nullable + @Override + public ErrorCapture captureException(@Nullable Throwable e, @Nullable ClassLoader initiatingClassLoader) { + return null; + } } 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 36fe7a695c..0bd0ceb1c7 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 @@ -45,7 +45,6 @@ public interface Tracer { Set getTraceHeaderNames(); - ElasticContext currentContext(); @Nullable @@ -54,6 +53,9 @@ public interface Tracer { @Nullable Transaction currentTransaction(); + @Nullable + ErrorCapture getActiveError(); + /** * Starts a trace-root transaction * @@ -91,4 +93,7 @@ public interface Tracer { */ @Nullable Transaction startChildTransaction(@Nullable C headerCarrier, BinaryHeaderGetter binaryHeadersGetter, @Nullable ClassLoader initiatingClassLoader); + + @Nullable + ErrorCapture captureException(@Nullable Throwable e, @Nullable ClassLoader initiatingClassLoader); } From 7906256b5ce0760e6ea3d7a90dde9d809d671b9f Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Fri, 8 Sep 2023 12:02:26 +0200 Subject: [PATCH 2/2] Added missing @Override annotations --- .../main/java/co/elastic/apm/agent/impl/error/ErrorCapture.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/error/ErrorCapture.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/error/ErrorCapture.java index 315bed3224..5be5431390 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/error/ErrorCapture.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/impl/error/ErrorCapture.java @@ -147,6 +147,7 @@ public ErrorCapture asChildOf(AbstractSpan parent) { return this; } + @Override public TraceContext getTraceContext() { return traceContext; } @@ -269,6 +270,7 @@ public void setTransactionType(@Nullable String type) { transactionInfo.type = type; } + @Override public void end() { tracer.endError(this); }