diff --git a/dd-java-agent/src/main/java/datadog/trace/bootstrap/BootstrapInitializationTelemetry.java b/dd-java-agent/src/main/java/datadog/trace/bootstrap/BootstrapInitializationTelemetry.java index 391436c549d..007e926d089 100644 --- a/dd-java-agent/src/main/java/datadog/trace/bootstrap/BootstrapInitializationTelemetry.java +++ b/dd-java-agent/src/main/java/datadog/trace/bootstrap/BootstrapInitializationTelemetry.java @@ -1,12 +1,14 @@ package datadog.trace.bootstrap; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; + import datadog.json.JsonWriter; import datadog.trace.bootstrap.environment.EnvironmentVariables; import de.thetaphi.forbiddenapis.SuppressForbidden; import java.io.Closeable; import java.io.OutputStream; import java.util.ArrayList; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -51,14 +53,14 @@ public static BootstrapInitializationTelemetry createFromForwarderPath(String fo */ public abstract void onError(Throwable t); + public abstract void onError(String reasonCode); + /** * Indicates an exception that occurred during the bootstrapping process that left initialization * incomplete. Equivalent to calling {@link #onError(Throwable)} and {@link #markIncomplete()} */ public abstract void onFatalError(Throwable t); - public abstract void onError(String reasonCode); - public abstract void markIncomplete(); public abstract void finish(); @@ -78,10 +80,10 @@ public void onAbort(String reasonCode) {} public void onError(String reasonCode) {} @Override - public void onFatalError(Throwable t) {} + public void onError(Throwable t) {} @Override - public void onError(Throwable t) {} + public void onFatalError(Throwable t) {} @Override public void markIncomplete() {} @@ -93,38 +95,31 @@ public void finish() {} public static final class JsonBased extends BootstrapInitializationTelemetry { private final JsonSender sender; - private final List meta; - private final Map> points; + private final Telemetry telemetry; // one way false to true private volatile boolean incomplete = false; - private volatile boolean error = false; JsonBased(JsonSender sender) { this.sender = sender; - this.meta = new ArrayList<>(); - this.points = new LinkedHashMap<>(); + this.telemetry = new Telemetry(); } @Override public void initMetaInfo(String attr, String value) { - synchronized (this.meta) { - this.meta.add(attr); - this.meta.add(value); - } + telemetry.setMetadata(attr, value); } @Override public void onAbort(String reasonCode) { - onPoint("library_entrypoint.abort", "reason:" + reasonCode); + onPoint("library_entrypoint.abort", singletonList("reason:" + reasonCode)); markIncomplete(); - setMetaInfo("abort", mapResultClass(reasonCode), reasonCode); + setResultMeta("abort", mapResultClass(reasonCode), reasonCode); } @Override public void onError(Throwable t) { - error = true; - setMetaInfo("error", "internal_error", t.getMessage()); + setResultMeta("error", "internal_error", t.getMessage()); List causes = new ArrayList<>(); @@ -145,6 +140,12 @@ public void onError(Throwable t) { onPoint("library_entrypoint.error", causes); } + @Override + public void onError(String reasonCode) { + onPoint("library_entrypoint.error", singletonList("error_type:" + reasonCode)); + setResultMeta("error", mapResultClass(reasonCode), reasonCode); + } + private int maxTags() { String maxTags = EnvironmentVariables.get("DD_TELEMETRY_FORWARDER_MAX_TAGS"); @@ -165,14 +166,7 @@ public void onFatalError(Throwable t) { markIncomplete(); } - @Override - public void onError(String reasonCode) { - error = true; - onPoint("library_entrypoint.error", "error_type:" + reasonCode); - setMetaInfo("error", mapResultClass(reasonCode), reasonCode); - } - - private void setMetaInfo(String result, String resultClass, String resultReason) { + private void setResultMeta(String result, String resultClass, String resultReason) { initMetaInfo("result", result); initMetaInfo("result_class", resultClass); initMetaInfo("result_reason", resultReason); @@ -195,14 +189,8 @@ private String mapResultClass(String reasonCode) { } } - private void onPoint(String name, String tag) { - onPoint(name, Collections.singletonList(tag)); - } - private void onPoint(String name, List tags) { - synchronized (this.points) { - this.points.put(name, tags); - } + telemetry.addPoint(name, tags); } @Override @@ -212,51 +200,91 @@ public void markIncomplete() { @Override public void finish() { - if (!this.incomplete && !this.error) { - setMetaInfo("success", "success", "Successfully configured ddtrace package"); + if (!this.incomplete) { + onPoint("library_entrypoint.complete", emptyList()); + } + + this.sender.send(telemetry); + } + } + + public static class Telemetry { + private final Map metadata; + private final Map> points; + + public Telemetry() { + metadata = new LinkedHashMap<>(); + points = new LinkedHashMap<>(); + + setResults("success", "success", "Successfully configured ddtrace package"); + } + + public void setMetadata(String name, String value) { + synchronized (metadata) { + metadata.put(name, value); } + } + public void setResults(String result, String resultClass, String resultReason) { + synchronized (metadata) { + metadata.put("result", result); + metadata.put("result_class", resultClass); + metadata.put("result_reason", resultReason); + } + } + + public void addPoint(String name, List tags) { + synchronized (points) { + points.put(name, tags); + } + } + + @Override + public String toString() { try (JsonWriter writer = new JsonWriter()) { writer.beginObject(); writer.name("metadata").beginObject(); - synchronized (this.meta) { - for (int i = 0; i + 1 < this.meta.size(); i = i + 2) { - writer.name(this.meta.get(i)); - writer.value(this.meta.get(i + 1)); + synchronized (metadata) { + for (Map.Entry entry : metadata.entrySet()) { + writer.name(entry.getKey()); + writer.value(entry.getValue()); } + + metadata.clear(); } writer.endObject(); writer.name("points").beginArray(); - synchronized (this.points) { + synchronized (points) { for (Map.Entry> entry : points.entrySet()) { writer.beginObject(); writer.name("name").value(entry.getKey()); - writer.name("tags").beginArray(); - for (String tag : entry.getValue()) { - writer.value(tag); + if (!entry.getValue().isEmpty()) { + writer.name("tags").beginArray(); + for (String tag : entry.getValue()) { + writer.value(tag); + } + writer.endArray(); } - writer.endArray(); writer.endObject(); } - this.points.clear(); - } - if (!this.incomplete) { - writer.beginObject().name("name").value("library_entrypoint.complete").endObject(); + + points.clear(); } writer.endArray(); writer.endObject(); - this.sender.send(writer.toByteArray()); - } catch (Throwable t) { - // Since this is the reporting mechanism, there's little recourse here - // Decided to simply ignore - arguably might want to write to stderr + return writer.toString(); } } } + /** + * Declare telemetry as {@code Object} to avoid issue with double class loading from different + * classloaders. + */ public interface JsonSender { - void send(byte[] payload); + void send(Object telemetry); } public static final class ForwarderJsonSender implements JsonSender { @@ -267,8 +295,8 @@ public static final class ForwarderJsonSender implements JsonSender { } @Override - public void send(byte[] payload) { - ForwarderJsonSenderThread t = new ForwarderJsonSenderThread(forwarderPath, payload); + public void send(Object telemetry) { + ForwarderJsonSenderThread t = new ForwarderJsonSenderThread(forwarderPath, telemetry); t.setDaemon(true); t.start(); } @@ -276,12 +304,12 @@ public void send(byte[] payload) { public static final class ForwarderJsonSenderThread extends Thread { private final String forwarderPath; - private final byte[] payload; + private final Object telemetry; - public ForwarderJsonSenderThread(String forwarderPath, byte[] payload) { + public ForwarderJsonSenderThread(String forwarderPath, Object telemetry) { super("dd-forwarder-json-sender"); this.forwarderPath = forwarderPath; - this.payload = payload; + this.telemetry = telemetry; } @SuppressForbidden @@ -291,6 +319,8 @@ public void run() { // Run forwarder and mute tracing for subprocesses executed in by dd-java-agent. try (final Closeable ignored = muteTracing()) { + byte[] payload = telemetry.toString().getBytes(); + Process process = builder.start(); try (OutputStream out = process.getOutputStream()) { out.write(payload); diff --git a/dd-java-agent/src/test/groovy/datadog/trace/bootstrap/BootstrapInitializationTelemetryTest.groovy b/dd-java-agent/src/test/groovy/datadog/trace/bootstrap/BootstrapInitializationTelemetryTest.groovy index ac62d4c7ff1..8efa3c014c5 100644 --- a/dd-java-agent/src/test/groovy/datadog/trace/bootstrap/BootstrapInitializationTelemetryTest.groovy +++ b/dd-java-agent/src/test/groovy/datadog/trace/bootstrap/BootstrapInitializationTelemetryTest.groovy @@ -3,51 +3,55 @@ package datadog.trace.bootstrap import groovy.json.JsonBuilder import spock.lang.Specification -import static java.nio.charset.StandardCharsets.UTF_8 - class BootstrapInitializationTelemetryTest extends Specification { - def initTelemetry, capture + Capture capture + def initTelemetry def setup() { - def capture = new Capture() - def initTelemetry = new BootstrapInitializationTelemetry.JsonBased(capture) + capture = new Capture() // There's an annoying interaction between our bootstrap injection // and the GroovyClassLoader class resolution. Groovy resolves the import // against the application ClassLoader, but when a method invocation // happens it resolves the invocation against the bootstrap classloader. - + // // To side step this problem, put a Groovy Proxy around the object under test - // codeNarc was incorrectly flagging "import groovy.util.Proxy" as unnecessary, // since groovy.util is imported implicitly. However, java.util is also // implicitly imported and also contains a Proxy class, so need to use the // full name inline to disambiguate and pass codeNarc. def initTelemetryProxy = new groovy.util.Proxy() - initTelemetryProxy.setAdaptee(initTelemetry) - + initTelemetryProxy.setAdaptee(new BootstrapInitializationTelemetry.JsonBased(capture)) this.initTelemetry = initTelemetryProxy - this.initTelemetry.initMetaInfo("runtime_name", "java") - this.initTelemetry.initMetaInfo("runtime_version", "1.8.0_382") - this.capture = capture } - def "test success"() { + def "test happy path"() { + when: + initTelemetry.finish() + + then: + assertJson("success", "success", "Successfully configured ddtrace package", [[name: "library_entrypoint.complete"]]) + } + + def "test non fatal error as text"() { when: + initTelemetry.onError("some reason") initTelemetry.finish() then: - capture.json() == json("success", "success", "Successfully configured ddtrace package", - [[name: "library_entrypoint.complete"]]) + assertJson("error", "unknown", "some reason", [ + [name: "library_entrypoint.error", tags: ["error_type:some reason"]], + [name: "library_entrypoint.complete"] + ]) } - def "real example"() { + def "test non fatal error as exception"() { when: - initTelemetry.onError(new Exception("foo")) + initTelemetry.onError(new Exception("non fatal error")) initTelemetry.finish() then: - capture.json() == json("error", "internal_error", "foo", [ + assertJson("error", "internal_error", "non fatal error", [ [name: "library_entrypoint.error", tags: ["error_type:java.lang.Exception"]], [name: "library_entrypoint.complete"] ]) @@ -59,8 +63,7 @@ class BootstrapInitializationTelemetryTest extends Specification { initTelemetry.finish() then: - capture.json() == json("abort", resultClass, reasonCode, - [[name: "library_entrypoint.abort", tags: ["reason:${reasonCode}"]]]) + assertJson("abort", resultClass, reasonCode, [[name: "library_entrypoint.abort", tags: ["reason:${reasonCode}"]]]) where: reasonCode | resultClass @@ -70,68 +73,45 @@ class BootstrapInitializationTelemetryTest extends Specification { "foo" | "unknown" } - def "trivial completion check"() { - when: - initTelemetry.finish() - - then: - capture.json().contains("library_entrypoint.complete") - } - - def "trivial incomplete check"() { - when: - initTelemetry.markIncomplete() - initTelemetry.finish() - - then: - !capture.json().contains("library_entrypoint.complete") - } - - def "incomplete on fatal error"() { + def "test fatal error"() { when: - initTelemetry.onFatalError(new Exception("foo")) + initTelemetry.onFatalError(new Exception("fatal error")) initTelemetry.finish() then: - !capture.json().contains("library_entrypoint.complete") - capture.json() == json("error", "internal_error", "foo", - [[name: "library_entrypoint.error", tags: ["error_type:java.lang.Exception"]]]) + assertJson("error", "internal_error", "fatal error", [[name: "library_entrypoint.error", tags: ["error_type:java.lang.Exception"]]]) } - def "incomplete on abort"() { - when: - initTelemetry.onAbort("reason") - initTelemetry.finish() - - then: - !capture.json().contains("library_entrypoint.complete") - } - - def "unwind root cause"() { + def "test unwind root cause"() { when: initTelemetry.onError(new Exception("top cause", new FileNotFoundException("root cause"))) initTelemetry.finish() then: - capture.json() == json("error", "internal_error", "top cause", [ + assertJson("error", "internal_error", "top cause", [ [name: "library_entrypoint.error", tags: ["error_type:java.io.FileNotFoundException", "error_type:java.lang.Exception"]], [name: "library_entrypoint.complete"] ]) } - private static String json(String result, String resultClass, String resultReason, List points) { - return """{"metadata":{"runtime_name":"java","runtime_version":"1.8.0_382","result":"${result}","result_class":"${resultClass}","result_reason":"${resultReason}"},"points":${new JsonBuilder(points)}}""" + private boolean assertJson(String result, String resultClass, String resultReason, List points) { + def expectedJson = """{"metadata":{"result":"${result}","result_class":"${resultClass}","result_reason":"${resultReason}"},"points":${new JsonBuilder(points)}}""" + def actualJson = capture.json() + + assert expectedJson == actualJson + + return true } static class Capture implements BootstrapInitializationTelemetry.JsonSender { - String json + Object telemetry - void send(byte[] payload) { - this.json = new String(payload, UTF_8) + void send(Object telemetry) { + this.telemetry = telemetry } String json() { - return this.json + return telemetry.toString() } } }