migrate to indy apm-error-logging-plugin (#1362)

* migrate to indy apm-error-logging-plugin Co-authored-by: kananindzya <kananindzya@gmail.com>
elastic · Sep 14, 2020 · 554d5cb · 554d5cb
1 parent 7e5461a
commit 554d5cb
Show file tree

Hide file tree

Showing 9 changed files with 29 additions and 45 deletions.
diff --git a/...tLoggerErrorCapturingInstrumentation.java → ...tLoggerErrorCapturingInstrumentation.java b/...tLoggerErrorCapturingInstrumentation.java → ...tLoggerErrorCapturingInstrumentation.java
@@ -22,11 +22,11 @@
  * under the License.
  * #L%
  */
-package co.elastic.apm.agent.error.logging;
+package co.elastic.apm.agent.errorlogging;
 
 import co.elastic.apm.agent.bci.TracerAwareInstrumentation;
-import co.elastic.apm.agent.bci.VisibleForAdvice;
 import co.elastic.apm.agent.impl.error.ErrorCapture;
+import co.elastic.apm.agent.sdk.state.CallDepth;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.NamedElement;
 import net.bytebuddy.description.method.MethodDescription;
@@ -42,51 +42,36 @@
 
 public abstract class AbstractLoggerErrorCapturingInstrumentation extends TracerAwareInstrumentation {
 
-    @SuppressWarnings({"WeakerAccess"})
-    @VisibleForAdvice
-    public static final ThreadLocal<Boolean> nestedThreadLocal = new ThreadLocal<Boolean>() {
-        @Override
-        protected Boolean initialValue() {
-            return Boolean.FALSE;
-        }
-    };
-
     @Override
     public Class<?> getAdviceClass() {
         return LoggingAdvice.class;
     }
 
-    @Override
-    public boolean indyPlugin() {
-        return false;
-    }
-
     public static class LoggingAdvice {
 
-        @Advice.OnMethodEnter(suppress = Throwable.class)
-        public static void logEnter(@Advice.Argument(1) Throwable exception,
-                                    @Advice.Local("nested") boolean nested,
-                                    @Advice.Origin Class<?> clazz,
-                                    @Advice.Local("error") @Nullable ErrorCapture error) {
-            nested = nestedThreadLocal.get();
-            if (!nested) {
-                error = tracer.captureException(exception, tracer.getActive(), clazz.getClassLoader());
+        private static final CallDepth callDepth = CallDepth.get(LoggingAdvice.class);
+
+        @Nullable
+        @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
+        public static Object logEnter(@Advice.Argument(1) Throwable exception,
+                                        @Advice.Origin Class<?> clazz) {
+            if (!callDepth.isNestedCallAndIncrement()) {
+                ErrorCapture error = tracer.captureException(exception, tracer.getActive(), clazz.getClassLoader());
                 if (error != null) {
                     error.activate();
                 }
-                nestedThreadLocal.set(Boolean.TRUE);
+                return error;
             }
+            return null;
         }
 
-        @Advice.OnMethodExit(suppress = Throwable.class)
-        public static void logExit(@Advice.Local("nested") boolean nested,
-                                   @Advice.Local("error") @Nullable ErrorCapture error) {
-            if (error != null) {
+        @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false)
+        public static void logExit(@Advice.Enter @Nullable Object errorCaptureObj) {
+            callDepth.decrement();
+            if (errorCaptureObj instanceof ErrorCapture) {
+                ErrorCapture error = (ErrorCapture) errorCaptureObj;
                 error.deactivate().end();
             }
-            if (!nested) {
-                nestedThreadLocal.set(Boolean.FALSE);
-            }
         }
     }
 

diff --git a/...2LoggerErrorCapturingInstrumentation.java → ...2LoggerErrorCapturingInstrumentation.java b/...2LoggerErrorCapturingInstrumentation.java → ...2LoggerErrorCapturingInstrumentation.java
@@ -22,14 +22,14 @@
  * under the License.
  * #L%
  */
-package co.elastic.apm.agent.error.logging;
+package co.elastic.apm.agent.errorlogging;
 
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
 
 import java.util.Collection;
 
-import static co.elastic.apm.agent.error.logging.Slf4jLoggerErrorCapturingInstrumentation.SLF4J_LOGGER;
+import static co.elastic.apm.agent.errorlogging.Slf4jLoggerErrorCapturingInstrumentation.SLF4J_LOGGER;
 import static net.bytebuddy.matcher.ElementMatchers.hasSuperType;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.not;

diff --git a/...jLoggerErrorCapturingInstrumentation.java → ...jLoggerErrorCapturingInstrumentation.java b/...jLoggerErrorCapturingInstrumentation.java → ...jLoggerErrorCapturingInstrumentation.java
@@ -22,14 +22,13 @@
  * under the License.
  * #L%
  */
-package co.elastic.apm.agent.error.logging;
+package co.elastic.apm.agent.errorlogging;
 
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
 
 import java.util.Collection;
 
-import static co.elastic.apm.agent.error.logging.Log4j2LoggerErrorCapturingInstrumentation.LOG4J2_LOGGER;
 import static net.bytebuddy.matcher.ElementMatchers.hasSuperType;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.not;
@@ -42,7 +41,7 @@ public class Slf4jLoggerErrorCapturingInstrumentation extends AbstractLoggerErro
     @Override
     public ElementMatcher<? super TypeDescription> getTypeMatcher() {
         return hasSuperType(named(SLF4J_LOGGER)
-            .and(not(hasSuperType(named(LOG4J2_LOGGER)))));
+            .and(not(hasSuperType(named(Log4j2LoggerErrorCapturingInstrumentation.LOG4J2_LOGGER)))));
     }
 
     @Override

diff --git a/...apm/agent/error/logging/package-info.java → .../apm/agent/errorlogging/package-info.java b/...apm/agent/error/logging/package-info.java → .../apm/agent/errorlogging/package-info.java
@@ -23,6 +23,6 @@
  * #L%
  */
 @NonnullApi
-package co.elastic.apm.agent.error.logging;
+package co.elastic.apm.agent.errorlogging;
 
 import co.elastic.apm.agent.sdk.NonnullApi;
diff --git a/...n/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation b/...n/src/main/resources/META-INF/services/co.elastic.apm.agent.sdk.ElasticApmInstrumentation
@@ -1,2 +1,2 @@
-co.elastic.apm.agent.error.logging.Slf4jLoggerErrorCapturingInstrumentation
-co.elastic.apm.agent.error.logging.Log4j2LoggerErrorCapturingInstrumentation
+co.elastic.apm.agent.errorlogging.Slf4jLoggerErrorCapturingInstrumentation
+co.elastic.apm.agent.errorlogging.Log4j2LoggerErrorCapturingInstrumentation
diff --git a/...tractErrorLoggingInstrumentationTest.java → ...tractErrorLoggingInstrumentationTest.java b/...tractErrorLoggingInstrumentationTest.java → ...tractErrorLoggingInstrumentationTest.java
@@ -22,7 +22,7 @@
  * under the License.
  * #L%
  */
-package co.elastic.apm.agent.error.logging;
+package co.elastic.apm.agent.errorlogging;
 
 import co.elastic.apm.agent.AbstractInstrumentationTest;
 import co.elastic.apm.agent.impl.transaction.Transaction;

diff --git a/...gerErrorCapturingInstrumentationTest.java → ...gerErrorCapturingInstrumentationTest.java b/...gerErrorCapturingInstrumentationTest.java → ...gerErrorCapturingInstrumentationTest.java
@@ -22,7 +22,7 @@
  * under the License.
  * #L%
  */
-package co.elastic.apm.agent.error.logging;
+package co.elastic.apm.agent.errorlogging;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;

diff --git a/...gerErrorCapturingInstrumentationTest.java → ...gerErrorCapturingInstrumentationTest.java b/...gerErrorCapturingInstrumentationTest.java → ...gerErrorCapturingInstrumentationTest.java
@@ -22,7 +22,7 @@
  * under the License.
  * #L%
  */
-package co.elastic.apm.agent.error.logging;
+package co.elastic.apm.agent.errorlogging;
 
 import org.junit.jupiter.api.Test;
 import org.slf4j.Logger;

diff --git a/...og-correlation-plugin/src/test/java/co/elastic/apm/agent/mdc/MdcActivationListenerIT.java b/...og-correlation-plugin/src/test/java/co/elastic/apm/agent/mdc/MdcActivationListenerIT.java
@@ -27,8 +27,8 @@
 import co.elastic.apm.agent.MockReporter;
 import co.elastic.apm.agent.bci.ElasticApmAgent;
 import co.elastic.apm.agent.configuration.SpyConfiguration;
-import co.elastic.apm.agent.error.logging.Log4j2LoggerErrorCapturingInstrumentation;
-import co.elastic.apm.agent.error.logging.Slf4jLoggerErrorCapturingInstrumentation;
+import co.elastic.apm.agent.errorlogging.Log4j2LoggerErrorCapturingInstrumentation;
+import co.elastic.apm.agent.errorlogging.Slf4jLoggerErrorCapturingInstrumentation;
 import co.elastic.apm.agent.impl.ElasticApmTracerBuilder;
 import co.elastic.apm.agent.impl.ElasticApmTracer;
 import co.elastic.apm.agent.impl.transaction.Transaction;