Merge branch 'master' into dougqh/collection-benchmarks

Author: dougqh

dd-java-agent/agent-bootstrap/src/jmh/java/datadog/trace/bootstrap/ExceptionLoggerBenchmark.java

@@ -0,0 +1,28 @@
+package datadog.trace.bootstrap;
+
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Threads;
+import org.openjdk.jmh.annotations.Warmup;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Benchmark showing impact of ExceptionLogger
+ *
+ * <p>NOTE: This benchmark exists to check the efficiency of retrieving the ExceptionLogger.
+ * Previously, this caused significant allocation.
+ */
+@Fork(2)
+@Warmup(iterations = 2)
+@Measurement(iterations = 5)
+@Threads(8)
+public class ExceptionLoggerBenchmark {
+  @Benchmark
+  public Logger getExceptionLogger() {
+    // This matches what happens in the bytecode weaving that defends against
+    // exception leaking out of instrumentation.
+    return LoggerFactory.getLogger(ExceptionLogger.class);
+  }
+}

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/ExceptionLogger.java

@@ -8,6 +8,8 @@
  *
  * <p>See datadog.trace.agent.tooling.ExceptionHandlers
  */
-public class ExceptionLogger {
+public final class ExceptionLogger {
   public static final Logger LOGGER = LoggerFactory.getLogger(ExceptionLogger.class);
+
+  private ExceptionLogger() {}
 }

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java

@@ -206,33 +206,40 @@ private List<Class<?>> detectMethodParameters(
     }
     List<Class<?>> result = new ArrayList<>();
     for (Class<?> changedClass : changedClasses) {
-      Method[] declaredMethods = changedClass.getDeclaredMethods();
       boolean addClass = true;
-      // capping scanning of methods to 100 to avoid generated class with thousand of methods
-      // assuming that in those first 100 methods there is at least one with at least one parameter
-      for (int methodIdx = 0; methodIdx < declaredMethods.length && methodIdx < 100; methodIdx++) {
-        Method method = declaredMethods[methodIdx];
-        Parameter[] parameters = method.getParameters();
-        if (parameters.length == 0) {
-          continue;
-        }
-        if (parameters[0].isNamePresent()) {
-          if (!SpringHelper.isSpringUsingOnlyMethodParameters(instrumentation)) {
-            return changedClasses;
+      try {
+        Method[] declaredMethods = changedClass.getDeclaredMethods();
+        // capping scanning of methods to 100 to avoid generated class with thousand of methods
+        // assuming that in those first 100 methods there is at least one with at least one
+        // parameter
+        for (int methodIdx = 0;
+            methodIdx < declaredMethods.length && methodIdx < 100;
+            methodIdx++) {
+          Method method = declaredMethods[methodIdx];
+          Parameter[] parameters = method.getParameters();
+          if (parameters.length == 0) {
+            continue;
+          }
+          if (parameters[0].isNamePresent()) {
+            if (!SpringHelper.isSpringUsingOnlyMethodParameters(instrumentation)) {
+              return changedClasses;
+            }
+            LOGGER.debug(
+                "Detecting method parameter: method={} param={}, Skipping retransforming this class",
+                method.getName(),
+                parameters[0].getName());
+            // skip the class: compiled with -parameters
+            reportError(
+                changes,
+                "Method Parameters detected, instrumentation not supported for "
+                    + changedClass.getTypeName());
+            addClass = false;
           }
-          LOGGER.debug(
-              "Detecting method parameter: method={} param={}, Skipping retransforming this class",
-              method.getName(),
-              parameters[0].getName());
-          // skip the class: compiled with -parameters
-          reportError(
-              changes,
-              "Method Parameters detected, instrumentation not supported for "
-                  + changedClass.getTypeName());
-          addClass = false;
+          // we found at leat a method with one parameter if name is not present we can stop there
+          break;
         }
-        // we found at leat a method with one parameter if name is not present we can stop there
-        break;
+      } catch (Exception e) {
+        LOGGER.debug("Exception scanning method parameters", e);
       }
       if (addClass) {
         result.add(changedClass);

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumenter.java

@@ -119,7 +119,15 @@ private boolean addLineCaptures(ClassFileLines classFileLines) {
       int till = sourceLine.getTill();
 
       boolean isSingleLine = from == till;
-
+      if (isSingleLine) {
+        List<MethodNode> methods = classFileLines.getMethodsByLine(from);
+        if (methods.size() == 1 && "<init>".equals(methods.get(0).name)) {
+          if (from == classFileLines.getMethodStart(methodNode)) {
+            reportError("Cannot instrument the first line of a constructor");
+            return false;
+          }
+        }
+      }
       LabelNode beforeLabel = classFileLines.getLineLabel(from);
       // single line N capture translates to line range (N, N+1)
       LabelNode afterLabel = classFileLines.getLineLabel(till + (isSingleLine ? 1 : 0));

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/symbol/SymbolExtractor.java

@@ -57,8 +57,12 @@ private static Scope extractScopes(ClassNode classNode, String jarName) {
       int classStartLine = Integer.MAX_VALUE;
       int classEndLine = 0;
       for (Scope scope : methodScopes) {
-        classStartLine = Math.min(classStartLine, scope.getStartLine());
-        classEndLine = Math.max(classEndLine, scope.getEndLine());
+        if (scope.getStartLine() > 0) {
+          classStartLine = Math.min(classStartLine, scope.getStartLine());
+        }
+        if (scope.getEndLine() > 0) {
+          classEndLine = Math.max(classEndLine, scope.getEndLine());
+        }
       }
       List<Symbol> fields = extractFields(classNode);
       LanguageSpecifics classSpecifics =
@@ -505,7 +509,18 @@ private static MethodLineInfo extractMethodLineInfo(
       node = node.getNext();
     }
     lineNo.sort(Integer::compareTo);
-    int startLine = lineNo.isEmpty() ? 0 : lineNo.get(0);
+    if (methodNode.name.equals("<init>") && !lineNo.isEmpty()) {
+      // for constructors we are dropping the first line because it is not instrumentable:
+      // it needs to call the super or this before
+      lineNo.remove(0);
+    }
+    int startLine;
+    if (lineNo.isEmpty()) {
+      startLine = 0;
+      maxLine = 0;
+    } else {
+      startLine = lineNo.get(0);
+    }
     List<Scope.LineRange> ranges = buildRanges(lineNo);
     return new MethodLineInfo(startLine, maxLine, map, ranges);
   }

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java

@@ -256,6 +256,22 @@ public void constructor() throws IOException, URISyntaxException {
     assertOneSnapshot(listener);
   }
 
+  @Test
+  public void constructorFirstLine() throws IOException, URISyntaxException {
+    final String CLASS_NAME = "CapturedSnapshot02";
+    int line = getLineForLineProbe(CLASS_NAME, LINE_PROBE_ID2);
+    TestSnapshotListener listener = installLineProbe(LINE_PROBE_ID2, CLASS_NAME, line);
+    Class<?> testClass = compileAndLoadClass(CLASS_NAME);
+    int result = Reflect.onClass(testClass).call("main", "f").get();
+    assertEquals(42, result);
+    ArgumentCaptor<ProbeId> probeIdCaptor = ArgumentCaptor.forClass(ProbeId.class);
+    ArgumentCaptor<String> strCaptor = ArgumentCaptor.forClass(String.class);
+    verify(probeStatusSink).addError(probeIdCaptor.capture(), strCaptor.capture());
+    assertEquals(LINE_PROBE_ID2.getId(), probeIdCaptor.getAllValues().get(0).getId());
+    assertEquals(
+        "Cannot instrument the first line of a constructor", strCaptor.getAllValues().get(0));
+  }
+
   @Test
   public void overloadedConstructor() throws IOException, URISyntaxException {
     final String CLASS_NAME = "CapturedSnapshot02";