Add probe diagnostics (#153)

Co-authored-by: Jean-Philippe Bempel <jean-philippe.bempel@datadoghq.com>

Author: jbachorik

dd-java-agent/agent-debugging/debugging-bootstrap/src/main/java/datadog/trace/bootstrap/debugging/DebuggerContext.java

@@ -0,0 +1,48 @@
+package datadog.trace.bootstrap.debugging;
+
+import java.util.List;
+import lombok.extern.slf4j.Slf4j;
+
+@Slf4j
+public class DebuggerContext {
+
+  public interface Sink {
+    void addSnapshot(Snapshot snapshot);
+
+    void addDiagnostics(String probeId, List<DiagnosticMessage> messages);
+
+    default void skipSnapshot(Snapshot snapshot, long threshold, long duration) {}
+  }
+
+  private static volatile Sink sink;
+
+  public static void init(Sink sink) {
+    DebuggerContext.sink = sink;
+  }
+
+  public static void skipSnapshot(Snapshot snapshot, long threshold, long duration) {
+    Sink localSink = sink;
+    if (localSink == null) {
+      return;
+    }
+    log.info("SnapshotHelper.skip");
+    localSink.skipSnapshot(snapshot, threshold, duration);
+  }
+
+  public static void addSnapshot(Snapshot snapshot) {
+    Sink localSink = sink;
+    if (localSink == null) {
+      return;
+    }
+    log.info("SnapshotHelper.add");
+    localSink.addSnapshot(snapshot);
+  }
+
+  public static void reportDiagnostics(String probeId, List<DiagnosticMessage> messages) {
+    Sink localSink = sink;
+    if (localSink == null) {
+      return;
+    }
+    localSink.addDiagnostics(probeId, messages);
+  }
+}

dd-java-agent/agent-debugging/debugging-bootstrap/src/main/java/datadog/trace/bootstrap/debugging/DiagnosticMessage.java

@@ -0,0 +1,37 @@
+package datadog.trace.bootstrap.debugging;
+
+import java.time.Instant;
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
+
+@ToString
+@EqualsAndHashCode
+public final class DiagnosticMessage {
+  public enum Kind {
+    INFO,
+    WARN,
+    ERROR;
+  }
+
+  private final long timestamp;
+  private final Kind kind;
+  private final String message;
+
+  public DiagnosticMessage(Kind kind, String message) {
+    this.timestamp = Instant.now().toEpochMilli();
+    this.kind = kind;
+    this.message = message;
+  }
+
+  public Kind getKind() {
+    return kind;
+  }
+
+  public String getMessage() {
+    return message;
+  }
+
+  public long getTimestamp() {
+    return timestamp;
+  }
+}

dd-java-agent/agent-debugging/debugging-bootstrap/src/main/java/datadog/trace/bootstrap/debugging/ProbeDiagnostic.java

@@ -0,0 +1,25 @@
+package datadog.trace.bootstrap.debugging;
+
+import java.util.List;
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
+
+@ToString
+@EqualsAndHashCode
+public final class ProbeDiagnostic {
+  private final String probeId;
+  private final List<DiagnosticMessage> messages;
+
+  public ProbeDiagnostic(String probeId, List<DiagnosticMessage> messages) {
+    this.probeId = probeId;
+    this.messages = messages;
+  }
+
+  public String getProbeId() {
+    return probeId;
+  }
+
+  public List<DiagnosticMessage> getMessages() {
+    return messages;
+  }
+}

dd-java-agent/agent-debugging/debugging-bootstrap/src/main/java/datadog/trace/bootstrap/debugging/Snapshot.java

@@ -15,12 +15,6 @@ public final class Snapshot {
   private static final String LANGUAGE = "java";
   private static final int VERSION = 0;
 
-  public interface Listener {
-    void onSkip(Snapshot snapshot, long threshold, long duration);
-
-    void onCommit(Snapshot snapshot);
-  }
-
   @ToString
   @EqualsAndHashCode
   public static final class Capture {
@@ -272,12 +266,12 @@ public void commit(long threshold, boolean lineScope) {
     duration = System.nanoTime() - startTs;
     if (threshold > -1 && !lineScope) {
       if (duration < threshold) {
-        SnapshotHelper.skip(this, threshold, duration);
+        DebuggerContext.skipSnapshot(this, threshold, duration);
         log.warn("Skipping snapshot");
       }
     }
     log.info("Committing snapshot: {}", this);
-    SnapshotHelper.add(this);
+    DebuggerContext.addSnapshot(this);
   }
 
   public long getStartTs() {

dd-java-agent/agent-debugging/debugging-bootstrap/src/main/java/datadog/trace/bootstrap/debugging/SnapshotHelper.java

@@ -1,8 +1,10 @@
 package com.datadog.debugging.agent;
 
+import com.datadog.debugging.instrumentation.InstrumentationResult;
 import com.datadog.debugging.instrumentation.MethodProbeInstrumentor;
 import datadog.trace.agent.tooling.AgentTooling;
 import datadog.trace.api.Config;
+import datadog.trace.bootstrap.debugging.DebuggerContext;
 import java.io.IOException;
 import java.lang.instrument.ClassFileTransformer;
 import java.nio.file.Files;
@@ -63,8 +65,9 @@ public byte[] transform(
           if (where.isMethodMatching(methodNode.name)
               && where.isSignatureMatching(methodNode.desc)) {
             log.debug("Instrumenting method: {}.{}{}", typeName, methodNode.name, methodNode.desc);
-            MethodProbeInstrumentor.instrument(probe, methodNode);
-            transformed = true;
+            InstrumentationResult result = MethodProbeInstrumentor.instrument(probe, methodNode);
+            transformed = result.isSuccess();
+            DebuggerContext.reportDiagnostics(probe.getProbeId(), result.getDiagnostics());
           }
         }
         if (transformed) {

dd-java-agent/agent-debugging/src/main/java/com/datadog/debugging/agent/DebuggerTransformer.java

@@ -3,7 +3,7 @@
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import datadog.trace.api.Config;
-import datadog.trace.bootstrap.debugging.SnapshotHelper;
+import datadog.trace.bootstrap.debugging.DebuggerContext;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -17,7 +17,7 @@ public static synchronized void run(Instrumentation instrumentation) {
     if (Config.get().isDebuggingEnabled()) {
       log.info("starting debugging agent");
       SnapshotSink sink = new SnapshotSink(Config.get());
-      SnapshotHelper.init(sink);
+      DebuggerContext.init(sink);
       String configFileName = System.getProperty("dd.debugger.config-file", "debugger-config.json");
       try {
         DebuggerProbe[] probes = null;

dd-java-agent/agent-debugging/src/main/java/com/datadog/debugging/agent/DebuggingAgent.java

@@ -6,13 +6,21 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import datadog.trace.api.Config;
-import datadog.trace.bootstrap.debugging.SnapshotHelper;
+import datadog.trace.bootstrap.debugging.DebuggerContext;
+import datadog.trace.bootstrap.debugging.DiagnosticMessage;
+import datadog.trace.bootstrap.debugging.ProbeDiagnostic;
+import java.util.List;
 import lombok.extern.slf4j.Slf4j;
 
 @Slf4j
-public class SnapshotSink implements SnapshotHelper.Sink {
+public class SnapshotSink implements DebuggerContext.Sink {
   private final SnapshotUploader snapshotUploader;
   private final boolean isJsonFormat;
+  private static final ObjectMapper MAPPER = new ObjectMapper();
+
+  static {
+    MAPPER.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);
+  }
 
   public SnapshotSink(Config config) {
     this(config, new SnapshotUploader(config));
@@ -24,19 +32,36 @@ public SnapshotSink(Config config) {
   }
 
   @Override
-  public void add(datadog.trace.bootstrap.debugging.Snapshot snapshot) {
-    // JSON or binary
+  public void addSnapshot(datadog.trace.bootstrap.debugging.Snapshot snapshot) {
+    if (isJsonFormat) {
+      try {
+        String jsonStr = MAPPER.writeValueAsString(snapshot);
+        log.info("JsonPaylod: {}", jsonStr);
+        snapshotUploader.upload(jsonStr, SnapshotUploader.Kind.SNAPSHOT);
+      } catch (JsonProcessingException ex) {
+        log.warn("Error during snapshot flush: ", ex);
+      }
+    } else {
+      log.warn("Unsupported serialization format");
+      // ByteBuffer buffer = ByteBuffer.allocate(4096); // FIXME
+      // SnapshotWriter.write(snapshot, buffer);
+      // snapshotUploader.upload(buffer);
+    }
+  }
+
+  @Override
+  public void addDiagnostics(String probeId, List<DiagnosticMessage> messages) {
+    ProbeDiagnostic probeDiagnostic = new ProbeDiagnostic(probeId, messages);
     if (isJsonFormat) {
       try {
-        ObjectMapper mapper = new ObjectMapper();
-        mapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);
-        String jsonStr = mapper.writeValueAsString(snapshot);
+        String jsonStr = MAPPER.writeValueAsString(probeDiagnostic);
         log.info("JsonPaylod: {}", jsonStr);
-        snapshotUploader.upload(jsonStr);
+        snapshotUploader.upload(jsonStr, SnapshotUploader.Kind.DIAGNOSTIC);
       } catch (JsonProcessingException ex) {
-        log.warn("Error during flush: ", ex);
+        log.warn("Error during diagnostic flush: ", ex);
       }
     } else {
+      log.warn("Unsupported serialization format");
       // ByteBuffer buffer = ByteBuffer.allocate(4096); // FIXME
       // SnapshotWriter.write(snapshot, buffer);
       // snapshotUploader.upload(buffer);

dd-java-agent/agent-debugging/src/main/java/com/datadog/debugging/agent/SnapshotSink.java

@@ -0,0 +1,22 @@
+package com.datadog.debugging.instrumentation;
+
+import datadog.trace.bootstrap.debugging.DiagnosticMessage;
+import java.util.List;
+
+public class InstrumentationResult {
+  private final boolean success;
+  private final List<DiagnosticMessage> diagnostics;
+
+  public InstrumentationResult(boolean success, List<DiagnosticMessage> diagnostics) {
+    this.success = success;
+    this.diagnostics = diagnostics;
+  }
+
+  public boolean isSuccess() {
+    return success;
+  }
+
+  public List<DiagnosticMessage> getDiagnostics() {
+    return diagnostics;
+  }
+}

dd-java-agent/agent-debugging/src/main/java/com/datadog/debugging/instrumentation/InstrumentationResult.java

@@ -9,6 +9,7 @@
 import static com.datadog.debugging.instrumentation.Types.THROWABLE_TYPE;
 
 import com.datadog.debugging.agent.DebuggerProbe;
+import datadog.trace.bootstrap.debugging.DiagnosticMessage;
 import datadog.trace.bootstrap.debugging.Snapshot;
 import java.util.ArrayList;
 import java.util.List;
@@ -56,20 +57,33 @@ LabelNode getLineLabel(int line) {
 
   private final DebuggerProbe probe;
   private final MethodNode methodNode;
-  private int snapshotVar = -1;
-  private LabelNode returnHandlerLabel = null;
-  private LabelNode snapshotInitLabel = new LabelNode();
+  private final LabelNode snapshotInitLabel = new LabelNode();
 
   private final LineMap lineMap = new LineMap();
   private final boolean isStatic;
-  private int offset = 0;
   private final String[] argumentNames;
 
-  public static void instrument(DebuggerProbe probe, MethodNode methodNode) {
-    new MethodProbeInstrumentor(probe, methodNode).instrument();
+  private int offset = 0;
+  private int snapshotVar = -1;
+  private LabelNode returnHandlerLabel = null;
+
+  private final List<DiagnosticMessage> diagnostics;
+
+  public static InstrumentationResult instrument(DebuggerProbe probe, MethodNode methodNode) {
+    List<DiagnosticMessage> diagnostics = new ArrayList<>();
+    boolean success = true;
+    try {
+      new MethodProbeInstrumentor(probe, methodNode, diagnostics).instrument();
+    } catch (Throwable t) {
+      success = false;
+      diagnostics.add(new DiagnosticMessage(DiagnosticMessage.Kind.ERROR, t.toString()));
+    }
+    return new InstrumentationResult(success, diagnostics);
   }
 
-  private MethodProbeInstrumentor(DebuggerProbe probe, MethodNode methodNode) {
+  private MethodProbeInstrumentor(
+      DebuggerProbe probe, MethodNode methodNode, List<DiagnosticMessage> diagnostics) {
+    this.diagnostics = diagnostics;
     this.probe = probe;
     this.methodNode = methodNode;
 
@@ -139,6 +153,8 @@ private void processInstruction(AbstractInsnNode node) {
   private void addLineCaptures(LineMap lineMap) {
     DebuggerProbe.SourceLine[] targetLines = probe.getWhere().getSourceLines();
     if (targetLines == null) {
+      diagnostics.add(
+          new DiagnosticMessage(DiagnosticMessage.Kind.WARN, "Missing line debug information"));
       return;
     }
     for (DebuggerProbe.SourceLine sourceLine : targetLines) {
@@ -338,6 +354,8 @@ private void tryBox(Type type, InsnList insnList) {
   private void collectLocalVariables(AbstractInsnNode location, InsnList insnList) {
     // expected stack top: [capture]
     if (location == null) {
+      diagnostics.add(
+          new DiagnosticMessage(DiagnosticMessage.Kind.WARN, "Missing local variable debug info"));
       // method capture, local variables are not initialized - bail out
       return;
     }