Merge pull request #6128 from DataDog/rgs/refactor-exception-profiling

instrument Exception and Error classes to avoid instrumenting generic throwables used for control flow

dd-java-agent/agent-bootstrap/src/main/java11/datadog/trace/bootstrap/instrumentation/jfr/exceptions/ExceptionProfiling.java

@@ -42,13 +42,13 @@ private ExceptionProfiling(final Config config) {
     this.recordExceptionMessage = recordExceptionMessage;
   }
 
-  public ExceptionSampleEvent process(final Throwable t, final int stackDepth) {
+  public ExceptionSampleEvent process(final Throwable t) {
     // always record the exception in histogram
     final boolean firstHit = histogram.record(t);
 
     final boolean sampled = sampler.sample();
     if (firstHit || sampled) {
-      return new ExceptionSampleEvent(t, stackDepth, sampled, firstHit);
+      return new ExceptionSampleEvent(t, sampled, firstHit);
     }
     return null;
   }

dd-java-agent/agent-bootstrap/src/main/java11/datadog/trace/bootstrap/instrumentation/jfr/exceptions/ExceptionSampleEvent.java

@@ -18,10 +18,6 @@ public class ExceptionSampleEvent extends Event implements ContextualEvent {
   @Label("Exception message")
   private final String message;
 
-  /** JFR may truncate the stack trace - so store original length as well. */
-  @Label("Exception stackdepth")
-  private final int stackDepth;
-
   @Label("Sampled")
   private final boolean sampled;
 
@@ -34,8 +30,7 @@ public class ExceptionSampleEvent extends Event implements ContextualEvent {
   @Label("Span Id")
   private long spanId;
 
-  public ExceptionSampleEvent(
-      Throwable e, final int stackDepth, boolean sampled, boolean firstOccurrence) {
+  public ExceptionSampleEvent(Throwable e, boolean sampled, boolean firstOccurrence) {
     /*
      * TODO: we should have some tests for this class.
      * Unfortunately at the moment this is not easily possible because we cannot build tests with groovy that
@@ -44,7 +39,6 @@ public ExceptionSampleEvent(
      */
     this.type = e.getClass().getName();
     this.message = getMessage(e);
-    this.stackDepth = stackDepth;
     this.sampled = sampled;
     this.firstOccurrence = firstOccurrence;
     captureContext();

dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie

@@ -42,7 +42,8 @@
 1 io.opentelemetry.javaagent.*
 1 java.*
 # allow exception profiling instrumentation
-0 java.lang.Throwable
+0 java.lang.Exception
+0 java.lang.Error
 # allow ProcessImpl instrumentation
 0 java.lang.ProcessImpl
 0 java.net.http.*

dd-java-agent/instrumentation/exception-profiling/src/main/java/datadog/exceptions/instrumentation/ThrowableInstrumentation.java

@@ -1,21 +1,15 @@
 package datadog.exceptions.instrumentation;
 
-import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.declaresField;
-import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
 
 import com.google.auto.service.AutoService;
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.api.Platform;
-import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.matcher.ElementMatcher;
 
-/** Provides instrumentation of {@linkplain Throwable} constructor. */
+/** Provides instrumentation of {@linkplain Exception} and {@linkplain Error} constructors. */
 @AutoService(Instrumenter.class)
 public final class ThrowableInstrumentation extends Instrumenter.Profiling
-    implements Instrumenter.ForBootstrap,
-        Instrumenter.ForSingleType,
-        Instrumenter.WithTypeStructure {
+    implements Instrumenter.ForBootstrap, Instrumenter.ForKnownTypes {
 
   public ThrowableInstrumentation() {
     super("throwables");
@@ -27,17 +21,14 @@ public boolean isEnabled() {
   }
 
   @Override
-  public String instrumentedType() {
-    return "java.lang.Throwable";
-  }
-
-  @Override
-  public ElementMatcher<TypeDescription> structureMatcher() {
-    return declaresField(named("stackTrace"));
+  public void adviceTransformations(AdviceTransformation transformation) {
+    transformation.applyAdvice(isConstructor(), packageName + ".ThrowableInstanceAdvice");
   }
 
   @Override
-  public void adviceTransformations(AdviceTransformation transformation) {
-    transformation.applyAdvice(isConstructor(), packageName + ".ThrowableInstanceAdvice");
+  public String[] knownMatchingTypes() {
+    return new String[] {
+      "java.lang.Exception", "java.lang.Error", "kotlin.Exception", "kotlin.Error"
+    };
   }
 }

dd-java-agent/instrumentation/exception-profiling/src/main/java11/datadog/exceptions/instrumentation/ThrowableInstanceAdvice.java

@@ -11,12 +11,7 @@
 
 public class ThrowableInstanceAdvice {
   @Advice.OnMethodExit(suppress = Throwable.class)
-  public static void onExit(
-      @Advice.This final Throwable t,
-      @Advice.FieldValue("stackTrace") StackTraceElement[] stackTrace) {
-    if (t.getClass().getName().endsWith(".ResourceLeakDetector$TraceRecord")) {
-      return;
-    }
+  public static void onExit(@Advice.This final Object t) {
     /*
      * This instrumentation handler is sensitive to any throwables thrown from its body -
      * it will go into infinite loop of trying to handle the new throwable instance and generating
@@ -47,8 +42,7 @@ public static void onExit(
        * JFR will assign the stacktrace depending on the place where the event is committed.
        * Therefore we need to commit the event here, right in the 'Exception' constructor
        */
-      final ExceptionSampleEvent event =
-          ExceptionProfiling.getInstance().process(t, stackTrace == null ? 0 : stackTrace.length);
+      final ExceptionSampleEvent event = ExceptionProfiling.getInstance().process((Throwable) t);
       if (event != null && event.shouldCommit()) {
         event.commit();
       }