From 6e57041a6a3cd75bf651ccdcbf486fba1617c8c5 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 20 Dec 2019 12:09:14 -0800 Subject: [PATCH 01/27] Extract Monitor classes from DDAgentWriter --- .../trace/common/writer/DDAgentWriter.java | 237 +----------------- .../datadog/trace/common/writer/Writer.java | 9 +- .../trace/common/writer/ddagent/Monitor.java | 232 +++++++++++++++++ .../groovy/datadog/trace/DDTracerTest.groovy | 5 +- .../trace/api/writer/DDAgentWriterTest.groovy | 15 +- 5 files changed, 254 insertions(+), 244 deletions(-) create mode 100644 dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/Monitor.java diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java index a98baabf56..4a8e230a3f 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java @@ -13,11 +13,9 @@ import com.lmax.disruptor.EventTranslatorOneArg; import com.lmax.disruptor.SleepingWaitStrategy; import com.lmax.disruptor.dsl.Disruptor; import com.lmax.disruptor.dsl.ProducerType; -import com.timgroup.statsd.NonBlockingStatsDClient; -import com.timgroup.statsd.StatsDClient; import datadog.opentracing.DDSpan; -import datadog.opentracing.DDTraceOTInfo; import datadog.trace.common.util.DaemonThreadFactory; +import datadog.trace.common.writer.ddagent.Monitor; import java.util.ArrayList; import java.util.List; import java.util.concurrent.Executors; @@ -84,7 +82,7 @@ public class DDAgentWriter implements Writer { public DDAgentWriter() { this( new DDApi(DEFAULT_AGENT_HOST, DEFAULT_TRACE_AGENT_PORT, DEFAULT_AGENT_UNIX_DOMAIN_SOCKET), - new NoopMonitor()); + new Monitor.Noop()); } public DDAgentWriter(final DDApi api, final Monitor monitor) { @@ -93,7 +91,7 @@ public class DDAgentWriter implements Writer { /** Old signature (pre-Monitor) used in tests */ private DDAgentWriter(final DDApi api) { - this(api, new NoopMonitor()); + this(api, new Monitor.Noop()); } /** @@ -108,7 +106,7 @@ public class DDAgentWriter implements Writer { final int disruptorSize, final int senderQueueSize, final int flushFrequencySeconds) { - this(api, new NoopMonitor(), disruptorSize, senderQueueSize, flushFrequencySeconds); + this(api, new Monitor.Noop(), disruptorSize, senderQueueSize, flushFrequencySeconds); } // DQH - TODO - Update the tests & remove this @@ -122,7 +120,7 @@ public class DDAgentWriter implements Writer { // DQH - TODO - Update the tests & remove this private DDAgentWriter(final DDApi api, final int disruptorSize, final int flushFrequencySeconds) { - this(api, new NoopMonitor(), disruptorSize, SENDER_QUEUE_SIZE, flushFrequencySeconds); + this(api, new Monitor.Noop(), disruptorSize, SENDER_QUEUE_SIZE, flushFrequencySeconds); } private DDAgentWriter( @@ -257,7 +255,7 @@ public class DDAgentWriter implements Writer { // if something is *probably* the NoopMonitor. String str = "DDAgentWriter { api=" + api; - if (!(monitor instanceof NoopMonitor)) { + if (!(monitor instanceof Monitor.Noop)) { str += ", monitor=" + monitor; } str += " }"; @@ -426,227 +424,4 @@ public class DDAgentWriter implements Writer { return new Event<>(); } } - - /** - * Callback interface for monitoring the health of the DDAgentWriter. Provides hooks for major - * lifecycle events... - * - * - */ - public interface Monitor { - void onStart(final DDAgentWriter agentWriter); - - void onShutdown(final DDAgentWriter agentWriter, final boolean flushSuccess); - - void onPublish(final DDAgentWriter agentWriter, final List trace); - - void onFailedPublish(final DDAgentWriter agentWriter, final List trace); - - void onFlush(final DDAgentWriter agentWriter, final boolean early); - - void onScheduleFlush(final DDAgentWriter agentWriter, final boolean previousIncomplete); - - void onSerialize( - final DDAgentWriter agentWriter, final List trace, final byte[] serializedTrace); - - void onFailedSerialize( - final DDAgentWriter agentWriter, final List trace, final Throwable optionalCause); - - void onSend( - final DDAgentWriter agentWriter, - final int representativeCount, - final int sizeInBytes, - final DDApi.Response response); - - void onFailedSend( - final DDAgentWriter agentWriter, - final int representativeCount, - final int sizeInBytes, - final DDApi.Response response); - } - - public static final class NoopMonitor implements Monitor { - @Override - public void onStart(final DDAgentWriter agentWriter) {} - - @Override - public void onShutdown(final DDAgentWriter agentWriter, final boolean flushSuccess) {} - - @Override - public void onPublish(final DDAgentWriter agentWriter, final List trace) {} - - @Override - public void onFailedPublish(final DDAgentWriter agentWriter, final List trace) {} - - @Override - public void onFlush(final DDAgentWriter agentWriter, final boolean early) {} - - @Override - public void onScheduleFlush( - final DDAgentWriter agentWriter, final boolean previousIncomplete) {} - - @Override - public void onSerialize( - final DDAgentWriter agentWriter, final List trace, final byte[] serializedTrace) {} - - @Override - public void onFailedSerialize( - final DDAgentWriter agentWriter, final List trace, final Throwable optionalCause) {} - - @Override - public void onSend( - final DDAgentWriter agentWriter, - final int representativeCount, - final int sizeInBytes, - final DDApi.Response response) {} - - @Override - public void onFailedSend( - final DDAgentWriter agentWriter, - final int representativeCount, - final int sizeInBytes, - final DDApi.Response response) {} - - @Override - public String toString() { - return "NoOp"; - } - } - - public static final class StatsDMonitor implements Monitor { - public static final String PREFIX = "datadog.tracer"; - - public static final String LANG_TAG = "lang"; - public static final String LANG_VERSION_TAG = "lang_version"; - public static final String LANG_INTERPRETER_TAG = "lang_interpreter"; - public static final String LANG_INTERPRETER_VENDOR_TAG = "lang_interpreter_vendor"; - public static final String TRACER_VERSION_TAG = "tracer_version"; - - private final String hostInfo; - private final StatsDClient statsd; - - // DQH - Made a conscious choice to not take a Config object here. - // Letting the creating of the Monitor take the Config, - // so it can decide which Monitor variant to create. - public StatsDMonitor(final String host, final int port) { - hostInfo = host + ":" + port; - statsd = new NonBlockingStatsDClient(PREFIX, host, port, getDefaultTags()); - } - - // Currently, intended for testing - private StatsDMonitor(final StatsDClient statsd) { - hostInfo = null; - this.statsd = statsd; - } - - protected static final String[] getDefaultTags() { - return new String[] { - tag(LANG_TAG, "java"), - tag(LANG_VERSION_TAG, DDTraceOTInfo.JAVA_VERSION), - tag(LANG_INTERPRETER_TAG, DDTraceOTInfo.JAVA_VM_NAME), - tag(LANG_INTERPRETER_VENDOR_TAG, DDTraceOTInfo.JAVA_VM_VENDOR), - tag(TRACER_VERSION_TAG, DDTraceOTInfo.VERSION) - }; - } - - private static String tag(final String tagPrefix, final String tagValue) { - return tagPrefix + ":" + tagValue; - } - - @Override - public void onStart(final DDAgentWriter agentWriter) { - statsd.recordGaugeValue("queue.max_length", agentWriter.getDisruptorCapacity()); - } - - @Override - public void onShutdown(final DDAgentWriter agentWriter, final boolean flushSuccess) {} - - @Override - public void onPublish(final DDAgentWriter agentWriter, final List trace) { - statsd.incrementCounter("queue.accepted"); - statsd.count("queue.accepted_lengths", trace.size()); - } - - @Override - public void onFailedPublish(final DDAgentWriter agentWriter, final List trace) { - statsd.incrementCounter("queue.dropped"); - } - - @Override - public void onScheduleFlush(final DDAgentWriter agentWriter, final boolean previousIncomplete) { - // not recorded - } - - @Override - public void onFlush(final DDAgentWriter agentWriter, final boolean early) {} - - @Override - public void onSerialize( - final DDAgentWriter agentWriter, final List trace, final byte[] serializedTrace) { - // DQH - Because of Java tracer's 2 phase acceptance and serialization scheme, this doesn't - // map precisely - statsd.count("queue.accepted_size", serializedTrace.length); - } - - @Override - public void onFailedSerialize( - final DDAgentWriter agentWriter, final List trace, final Throwable optionalCause) { - // TODO - DQH - make a new stat for serialization failure -- or maybe count this towards - // api.errors??? - } - - @Override - public void onSend( - final DDAgentWriter agentWriter, - final int representativeCount, - final int sizeInBytes, - final DDApi.Response response) { - onSendAttempt(agentWriter, representativeCount, sizeInBytes, response); - } - - @Override - public void onFailedSend( - final DDAgentWriter agentWriter, - final int representativeCount, - final int sizeInBytes, - final DDApi.Response response) { - onSendAttempt(agentWriter, representativeCount, sizeInBytes, response); - } - - private void onSendAttempt( - final DDAgentWriter agentWriter, - final int representativeCount, - final int sizeInBytes, - final DDApi.Response response) { - statsd.incrementCounter("api.requests"); - statsd.recordGaugeValue("queue.length", representativeCount); - // TODO: missing queue.spans (# of spans being sent) - statsd.recordGaugeValue("queue.size", sizeInBytes); - - if (response.exception() != null) { - // covers communication errors -- both not receiving a response or - // receiving malformed response (even when otherwise successful) - statsd.incrementCounter("api.errors"); - } - - if (response.status() != null) { - statsd.incrementCounter("api.responses", "status: " + response.status()); - } - } - - @Override - public String toString() { - if (hostInfo == null) { - return "StatsD"; - } else { - return "StatsD { host=" + hostInfo + " }"; - } - } - } } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java index a6d99ee749..669bcfd9fc 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java @@ -2,6 +2,7 @@ package datadog.trace.common.writer; import datadog.opentracing.DDSpan; import datadog.trace.api.Config; +import datadog.trace.common.writer.ddagent.Monitor; import java.io.Closeable; import java.util.List; import java.util.Properties; @@ -65,14 +66,14 @@ public interface Writer extends Closeable { return new DDAgentWriter(createApi(config), createMonitor(config)); } - private static final DDApi createApi(final Config config) { + private static DDApi createApi(final Config config) { return new DDApi( config.getAgentHost(), config.getAgentPort(), config.getAgentUnixDomainSocket()); } - private static final DDAgentWriter.Monitor createMonitor(final Config config) { + private static Monitor createMonitor(final Config config) { if (!config.isHealthMetricsEnabled()) { - return new DDAgentWriter.NoopMonitor(); + return new Monitor.Noop(); } else { String host = config.getHealthMetricsStatsdHost(); if (host == null) { @@ -87,7 +88,7 @@ public interface Writer extends Closeable { port = config.getJmxFetchStatsdPort(); } - return new DDAgentWriter.StatsDMonitor(host, port); + return new Monitor.StatsD(host, port); } } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/Monitor.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/Monitor.java new file mode 100644 index 0000000000..12afd9906c --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/Monitor.java @@ -0,0 +1,232 @@ +package datadog.trace.common.writer.ddagent; + +import com.timgroup.statsd.NonBlockingStatsDClient; +import com.timgroup.statsd.StatsDClient; +import datadog.opentracing.DDSpan; +import datadog.opentracing.DDTraceOTInfo; +import datadog.trace.common.writer.DDAgentWriter; +import datadog.trace.common.writer.DDApi; +import java.util.List; + +/** + * Callback interface for monitoring the health of the DDAgentWriter. Provides hooks for major + * lifecycle events... + * + *
    + *
  • start + *
  • shutdown + *
  • publishing to disruptor + *
  • serializing + *
  • sending to agent + *
+ */ +public interface Monitor { + void onStart(final DDAgentWriter agentWriter); + + void onShutdown(final DDAgentWriter agentWriter, final boolean flushSuccess); + + void onPublish(final DDAgentWriter agentWriter, final List trace); + + void onFailedPublish(final DDAgentWriter agentWriter, final List trace); + + void onFlush(final DDAgentWriter agentWriter, final boolean early); + + void onScheduleFlush(final DDAgentWriter agentWriter, final boolean previousIncomplete); + + void onSerialize( + final DDAgentWriter agentWriter, final List trace, final byte[] serializedTrace); + + void onFailedSerialize( + final DDAgentWriter agentWriter, final List trace, final Throwable optionalCause); + + void onSend( + final DDAgentWriter agentWriter, + final int representativeCount, + final int sizeInBytes, + final DDApi.Response response); + + void onFailedSend( + final DDAgentWriter agentWriter, + final int representativeCount, + final int sizeInBytes, + final DDApi.Response response); + + final class StatsD implements Monitor { + public static final String PREFIX = "datadog.tracer"; + + public static final String LANG_TAG = "lang"; + public static final String LANG_VERSION_TAG = "lang_version"; + public static final String LANG_INTERPRETER_TAG = "lang_interpreter"; + public static final String LANG_INTERPRETER_VENDOR_TAG = "lang_interpreter_vendor"; + public static final String TRACER_VERSION_TAG = "tracer_version"; + + private final String hostInfo; + private final StatsDClient statsd; + + // DQH - Made a conscious choice to not take a Config object here. + // Letting the creating of the Monitor take the Config, + // so it can decide which Monitor variant to create. + public StatsD(final String host, final int port) { + hostInfo = host + ":" + port; + statsd = new NonBlockingStatsDClient(PREFIX, host, port, getDefaultTags()); + } + + // Currently, intended for testing + private StatsD(final StatsDClient statsd) { + hostInfo = null; + this.statsd = statsd; + } + + protected static final String[] getDefaultTags() { + return new String[] { + tag(LANG_TAG, "java"), + tag(LANG_VERSION_TAG, DDTraceOTInfo.JAVA_VERSION), + tag(LANG_INTERPRETER_TAG, DDTraceOTInfo.JAVA_VM_NAME), + tag(LANG_INTERPRETER_VENDOR_TAG, DDTraceOTInfo.JAVA_VM_VENDOR), + tag(TRACER_VERSION_TAG, DDTraceOTInfo.VERSION) + }; + } + + private static String tag(final String tagPrefix, final String tagValue) { + return tagPrefix + ":" + tagValue; + } + + @Override + public void onStart(final DDAgentWriter agentWriter) { + statsd.recordGaugeValue("queue.max_length", agentWriter.getDisruptorCapacity()); + } + + @Override + public void onShutdown(final DDAgentWriter agentWriter, final boolean flushSuccess) {} + + @Override + public void onPublish(final DDAgentWriter agentWriter, final List trace) { + statsd.incrementCounter("queue.accepted"); + statsd.count("queue.accepted_lengths", trace.size()); + } + + @Override + public void onFailedPublish(final DDAgentWriter agentWriter, final List trace) { + statsd.incrementCounter("queue.dropped"); + } + + @Override + public void onScheduleFlush(final DDAgentWriter agentWriter, final boolean previousIncomplete) { + // not recorded + } + + @Override + public void onFlush(final DDAgentWriter agentWriter, final boolean early) {} + + @Override + public void onSerialize( + final DDAgentWriter agentWriter, final List trace, final byte[] serializedTrace) { + // DQH - Because of Java tracer's 2 phase acceptance and serialization scheme, this doesn't + // map precisely + statsd.count("queue.accepted_size", serializedTrace.length); + } + + @Override + public void onFailedSerialize( + final DDAgentWriter agentWriter, final List trace, final Throwable optionalCause) { + // TODO - DQH - make a new stat for serialization failure -- or maybe count this towards + // api.errors??? + } + + @Override + public void onSend( + final DDAgentWriter agentWriter, + final int representativeCount, + final int sizeInBytes, + final DDApi.Response response) { + onSendAttempt(agentWriter, representativeCount, sizeInBytes, response); + } + + @Override + public void onFailedSend( + final DDAgentWriter agentWriter, + final int representativeCount, + final int sizeInBytes, + final DDApi.Response response) { + onSendAttempt(agentWriter, representativeCount, sizeInBytes, response); + } + + private void onSendAttempt( + final DDAgentWriter agentWriter, + final int representativeCount, + final int sizeInBytes, + final DDApi.Response response) { + statsd.incrementCounter("api.requests"); + statsd.recordGaugeValue("queue.length", representativeCount); + // TODO: missing queue.spans (# of spans being sent) + statsd.recordGaugeValue("queue.size", sizeInBytes); + + if (response.exception() != null) { + // covers communication errors -- both not receiving a response or + // receiving malformed response (even when otherwise successful) + statsd.incrementCounter("api.errors"); + } + + if (response.status() != null) { + statsd.incrementCounter("api.responses", "status: " + response.status()); + } + } + + @Override + public String toString() { + if (hostInfo == null) { + return "StatsD"; + } else { + return "StatsD { host=" + hostInfo + " }"; + } + } + } + + final class Noop implements Monitor { + @Override + public void onStart(final DDAgentWriter agentWriter) {} + + @Override + public void onShutdown(final DDAgentWriter agentWriter, final boolean flushSuccess) {} + + @Override + public void onPublish(final DDAgentWriter agentWriter, final List trace) {} + + @Override + public void onFailedPublish(final DDAgentWriter agentWriter, final List trace) {} + + @Override + public void onFlush(final DDAgentWriter agentWriter, final boolean early) {} + + @Override + public void onScheduleFlush( + final DDAgentWriter agentWriter, final boolean previousIncomplete) {} + + @Override + public void onSerialize( + final DDAgentWriter agentWriter, final List trace, final byte[] serializedTrace) {} + + @Override + public void onFailedSerialize( + final DDAgentWriter agentWriter, final List trace, final Throwable optionalCause) {} + + @Override + public void onSend( + final DDAgentWriter agentWriter, + final int representativeCount, + final int sizeInBytes, + final DDApi.Response response) {} + + @Override + public void onFailedSend( + final DDAgentWriter agentWriter, + final int representativeCount, + final int sizeInBytes, + final DDApi.Response response) {} + + @Override + public String toString() { + return "NoOp"; + } + } +} diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/DDTracerTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/DDTracerTest.groovy index 65ab4d4305..5a7ac705c5 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/DDTracerTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/DDTracerTest.groovy @@ -14,6 +14,7 @@ import datadog.trace.common.sampling.Sampler import datadog.trace.common.writer.DDAgentWriter import datadog.trace.common.writer.ListWriter import datadog.trace.common.writer.LoggingWriter +import datadog.trace.common.writer.ddagent.Monitor import datadog.trace.util.test.DDSpecification import io.opentracing.propagation.TextMapInject import org.junit.Rule @@ -49,7 +50,7 @@ class DDTracerTest extends DDSpecification { ((DDAgentWriter) tracer.writer).api.tracesUrl.port() == 8126 ((DDAgentWriter) tracer.writer).api.tracesUrl.encodedPath() == "/v0.3/traces" || ((DDAgentWriter) tracer.writer).api.tracesUrl.encodedPath() == "/v0.4/traces" - tracer.writer.monitor instanceof DDAgentWriter.NoopMonitor + tracer.writer.monitor instanceof Monitor.Noop tracer.spanContextDecorators.size() == 15 @@ -65,7 +66,7 @@ class DDTracerTest extends DDSpecification { def tracer = new DDTracer(new Config()) then: - tracer.writer.monitor instanceof DDAgentWriter.StatsDMonitor + tracer.writer.monitor instanceof Monitor.StatsD tracer.writer.monitor.hostInfo == "localhost:8125" } diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy index 0868e56ba5..f61ffb6641 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy @@ -8,6 +8,7 @@ import datadog.opentracing.PendingTrace import datadog.trace.api.sampling.PrioritySampling import datadog.trace.common.writer.DDAgentWriter import datadog.trace.common.writer.DDApi +import datadog.trace.common.writer.ddagent.Monitor import datadog.trace.util.test.DDSpecification import spock.lang.Timeout @@ -252,7 +253,7 @@ class DDAgentWriterTest extends DDSpecification { } } def api = new DDApi("localhost", agent.address.port, null) - def monitor = Mock(DDAgentWriter.Monitor) + def monitor = Mock(Monitor) def writer = new DDAgentWriter(api, monitor) when: @@ -301,7 +302,7 @@ class DDAgentWriterTest extends DDSpecification { } } def api = new DDApi("localhost", agent.address.port, null) - def monitor = Mock(DDAgentWriter.Monitor) + def monitor = Mock(Monitor) def writer = new DDAgentWriter(api, monitor) when: @@ -343,7 +344,7 @@ class DDAgentWriterTest extends DDSpecification { return DDApi.Response.failed(new IOException("comm error")) } } - def monitor = Mock(DDAgentWriter.Monitor) + def monitor = Mock(Monitor) def writer = new DDAgentWriter(api, monitor) when: @@ -401,7 +402,7 @@ class DDAgentWriterTest extends DDSpecification { def api = new DDApi("localhost", agent.address.port, null) // This test focuses just on failed publish, so not verifying every callback - def monitor = Stub(DDAgentWriter.Monitor) + def monitor = Stub(Monitor) monitor.onPublish(_, _) >> { numPublished.incrementAndGet() } @@ -506,7 +507,7 @@ class DDAgentWriterTest extends DDSpecification { def api = new DDApi("localhost", agent.address.port, null) // This test focuses just on failed publish, so not verifying every callback - def monitor = Stub(DDAgentWriter.Monitor) + def monitor = Stub(Monitor) monitor.onPublish(_, _) >> { numPublished.incrementAndGet() } @@ -577,7 +578,7 @@ class DDAgentWriterTest extends DDSpecification { numResponses += 1 } - def monitor = new DDAgentWriter.StatsDMonitor(statsd) + def monitor = new Monitor.StatsD(statsd) def writer = new DDAgentWriter(api, monitor) writer.start() @@ -625,7 +626,7 @@ class DDAgentWriterTest extends DDSpecification { numErrors += 1 } - def monitor = new DDAgentWriter.StatsDMonitor(statsd) + def monitor = new Monitor.StatsD(statsd) def writer = new DDAgentWriter(api, monitor) writer.start() From f181fa721f24b7f8d44299c0c2855b3922769590 Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Fri, 20 Dec 2019 16:17:08 -0500 Subject: [PATCH 02/27] Add classloader matcher to muzzle scan --- .../tooling/muzzle/MuzzleVersionScanPlugin.java | 12 ++++++++++-- .../jax-rs-annotations-1/jax-rs-annotations-1.gradle | 6 +++++- .../instrumentation/jedis-1.4/jedis-1.4.gradle | 2 +- .../servlet/request-2/request-2.gradle | 8 ++++++-- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleVersionScanPlugin.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleVersionScanPlugin.java index 1443db72ed..46ce074d80 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleVersionScanPlugin.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleVersionScanPlugin.java @@ -53,8 +53,11 @@ public class MuzzleVersionScanPlugin { final ReferenceMatcher muzzle = (ReferenceMatcher) m.invoke(instrumenter); final List mismatches = muzzle.getMismatchedReferenceSources(userClassLoader); - final boolean passed = mismatches.size() == 0; - if (mismatches.size() > 0) {} + + final boolean classLoaderMatch = + ((Instrumenter.Default) instrumenter).classLoaderMatcher().matches(userClassLoader); + final boolean passed = mismatches.isEmpty() && classLoaderMatch; + if (passed && !assertPass) { System.err.println( "MUZZLE PASSED " @@ -64,6 +67,11 @@ public class MuzzleVersionScanPlugin { } else if (!passed && assertPass) { System.err.println( "FAILED MUZZLE VALIDATION: " + instrumenter.getClass().getName() + " mismatches:"); + + if (!classLoaderMatch) { + System.err.println("-- classloader mismatch"); + } + for (final Reference.Mismatch mismatch : mismatches) { System.err.println("-- " + mismatch); } diff --git a/dd-java-agent/instrumentation/jax-rs-annotations-1/jax-rs-annotations-1.gradle b/dd-java-agent/instrumentation/jax-rs-annotations-1/jax-rs-annotations-1.gradle index 320d759b31..da43f865e3 100644 --- a/dd-java-agent/instrumentation/jax-rs-annotations-1/jax-rs-annotations-1.gradle +++ b/dd-java-agent/instrumentation/jax-rs-annotations-1/jax-rs-annotations-1.gradle @@ -4,7 +4,11 @@ muzzle { module = "jsr311-api" versions = "[0.5,)" } - // Muzzle doesn't detect the classLoaderMatcher, so we can't assert fail for v2 api. + fail { + group = "javax.ws.rs" + module = "javax.ws.rs-api" + versions = "[,]" + } } apply from: "${rootDir}/gradle/java.gradle" diff --git a/dd-java-agent/instrumentation/jedis-1.4/jedis-1.4.gradle b/dd-java-agent/instrumentation/jedis-1.4/jedis-1.4.gradle index 459a688e75..a3f451170e 100644 --- a/dd-java-agent/instrumentation/jedis-1.4/jedis-1.4.gradle +++ b/dd-java-agent/instrumentation/jedis-1.4/jedis-1.4.gradle @@ -3,8 +3,8 @@ muzzle { group = "redis.clients" module = "jedis" versions = "[1.4.0,3.0.0)" + assertInverse = true } - // Muzzle doesn't detect the classLoaderMatcher, so we can't assert fail for 3.0+ } apply from: "${rootDir}/gradle/java.gradle" diff --git a/dd-java-agent/instrumentation/servlet/request-2/request-2.gradle b/dd-java-agent/instrumentation/servlet/request-2/request-2.gradle index 2a5e4a90d5..926907ab91 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/request-2.gradle +++ b/dd-java-agent/instrumentation/servlet/request-2/request-2.gradle @@ -5,8 +5,12 @@ muzzle { versions = "[2.3, 3.0)" assertInverse = true } - // can't add a fail block for servlet 3, because servlet 3 is backward compatible with servlet 2.3+, - // meaning that for every class that exists in servlet 2, it also exists in servlet 3 + + fail { + group = "javax.servlet" + module = 'javax.servlet-api' + versions = "[3.0,)" + } } apply from: "${rootDir}/gradle/java.gradle" From 2ea76494f8bac76976ee05e9b078c8f3edd91acb Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 20 Dec 2019 12:19:00 -0800 Subject: [PATCH 03/27] Extract DisruptorEvent and EventTranslator classes from DDAgentWriter --- .../trace/common/writer/DDAgentWriter.java | 43 ++++--------------- .../common/writer/ddagent/DisruptorEvent.java | 35 +++++++++++++++ 2 files changed, 44 insertions(+), 34 deletions(-) create mode 100644 dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DisruptorEvent.java diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java index 4a8e230a3f..fc0191fb09 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java @@ -6,15 +6,13 @@ import static datadog.trace.api.Config.DEFAULT_TRACE_AGENT_PORT; import static java.util.concurrent.TimeUnit.SECONDS; import com.fasterxml.jackson.core.JsonProcessingException; -import com.lmax.disruptor.EventFactory; import com.lmax.disruptor.EventHandler; -import com.lmax.disruptor.EventTranslator; -import com.lmax.disruptor.EventTranslatorOneArg; import com.lmax.disruptor.SleepingWaitStrategy; import com.lmax.disruptor.dsl.Disruptor; import com.lmax.disruptor.dsl.ProducerType; import datadog.opentracing.DDSpan; import datadog.trace.common.util.DaemonThreadFactory; +import datadog.trace.common.writer.ddagent.DisruptorEvent; import datadog.trace.common.writer.ddagent.Monitor; import java.util.ArrayList; import java.util.List; @@ -44,21 +42,10 @@ public class DDAgentWriter implements Writer { private static final int FLUSH_PAYLOAD_BYTES = 5_000_000; // 5 MB private static final int FLUSH_PAYLOAD_DELAY = 1; // 1/second - private static final EventTranslatorOneArg>, List> TRANSLATOR = - new EventTranslatorOneArg>, List>() { - @Override - public void translateTo( - final Event> event, final long sequence, final List trace) { - event.data = trace; - } - }; - private static final EventTranslator>> FLUSH_TRANSLATOR = - new EventTranslator>>() { - @Override - public void translateTo(final Event> event, final long sequence) { - event.shouldFlush = true; - } - }; + private static final DisruptorEvent.TraceTranslator TRANSLATOR = + new DisruptorEvent.TraceTranslator(); + private static final DisruptorEvent.FlushTranslator FLUSH_TRANSLATOR = + new DisruptorEvent.FlushTranslator(); private static final ThreadFactory DISRUPTOR_THREAD_FACTORY = new DaemonThreadFactory("dd-trace-disruptor"); @@ -68,7 +55,7 @@ public class DDAgentWriter implements Writer { private final Runnable flushTask = new FlushTask(); private final DDApi api; private final int flushFrequencySeconds; - private final Disruptor>> disruptor; + private final Disruptor>> disruptor; private final Semaphore senderSemaphore; private final ScheduledExecutorService scheduledWriterExecutor; @@ -134,7 +121,7 @@ public class DDAgentWriter implements Writer { disruptor = new Disruptor<>( - new DisruptorEventFactory>(), + new DisruptorEvent.Factory>(), Math.max(2, Integer.highestOneBit(disruptorSize - 1) << 1), // Next power of 2 DISRUPTOR_THREAD_FACTORY, ProducerType.MULTI, @@ -287,13 +274,13 @@ public class DDAgentWriter implements Writer { } /** This class is intentionally not threadsafe. */ - private class TraceConsumer implements EventHandler>> { + private class TraceConsumer implements EventHandler>> { private List serializedTraces = new ArrayList<>(); private int payloadSize = 0; @Override public void onEvent( - final Event> event, final long sequence, final boolean endOfBatch) { + final DisruptorEvent> event, final long sequence, final boolean endOfBatch) { final List trace = event.data; event.data = null; // clear the event for reuse. if (trace != null) { @@ -412,16 +399,4 @@ public class DDAgentWriter implements Writer { } } } - - private static class Event { - private volatile boolean shouldFlush = false; - private volatile T data = null; - } - - private static class DisruptorEventFactory implements EventFactory> { - @Override - public Event newInstance() { - return new Event<>(); - } - } } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DisruptorEvent.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DisruptorEvent.java new file mode 100644 index 0000000000..30db86f21a --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DisruptorEvent.java @@ -0,0 +1,35 @@ +package datadog.trace.common.writer.ddagent; + +import com.lmax.disruptor.EventFactory; +import com.lmax.disruptor.EventTranslator; +import com.lmax.disruptor.EventTranslatorOneArg; +import datadog.opentracing.DDSpan; +import java.util.List; + +public class DisruptorEvent { + public volatile boolean shouldFlush = false; + public volatile T data = null; + + public static class Factory implements EventFactory> { + @Override + public DisruptorEvent newInstance() { + return new DisruptorEvent<>(); + } + } + + public static class TraceTranslator + implements EventTranslatorOneArg>, List> { + @Override + public void translateTo( + final DisruptorEvent> event, final long sequence, final List trace) { + event.data = trace; + } + } + + public static class FlushTranslator implements EventTranslator>> { + @Override + public void translateTo(final DisruptorEvent> event, final long sequence) { + event.shouldFlush = true; + } + } +} From 8fdd30d3edc22a35cf26821f2ad5167e2c8ee2c9 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 20 Dec 2019 12:54:40 -0800 Subject: [PATCH 04/27] Make TraceConsumer a static class --- .../trace/common/writer/DDAgentWriter.java | 71 ++++++++++--------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java index fc0191fb09..70851bd433 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java @@ -57,7 +57,6 @@ public class DDAgentWriter implements Writer { private final int flushFrequencySeconds; private final Disruptor>> disruptor; - private final Semaphore senderSemaphore; private final ScheduledExecutorService scheduledWriterExecutor; private final AtomicInteger traceCount = new AtomicInteger(0); private final AtomicReference> flushSchedule = new AtomicReference<>(); @@ -126,10 +125,9 @@ public class DDAgentWriter implements Writer { DISRUPTOR_THREAD_FACTORY, ProducerType.MULTI, new SleepingWaitStrategy(0, TimeUnit.MILLISECONDS.toNanos(5))); - disruptor.handleEventsWith(new TraceConsumer()); + disruptor.handleEventsWith(new TraceConsumer(traceCount, senderQueueSize, this)); this.flushFrequencySeconds = flushFrequencySeconds; - senderSemaphore = new Semaphore(senderQueueSize); scheduledWriterExecutor = Executors.newScheduledThreadPool(1, SCHEDULED_FLUSH_THREAD_FACTORY); apiPhaser = new Phaser(); // Ensure API calls are completed when flushing @@ -274,10 +272,21 @@ public class DDAgentWriter implements Writer { } /** This class is intentionally not threadsafe. */ - private class TraceConsumer implements EventHandler>> { + private static class TraceConsumer implements EventHandler>> { + private final AtomicInteger traceCount; + private final Semaphore senderSemaphore; + private final DDAgentWriter writer; + private List serializedTraces = new ArrayList<>(); private int payloadSize = 0; + private TraceConsumer( + final AtomicInteger traceCount, final int senderQueueSize, final DDAgentWriter writer) { + this.traceCount = traceCount; + senderSemaphore = new Semaphore(senderQueueSize); + this.writer = writer; + } + @Override public void onEvent( final DisruptorEvent> event, final long sequence, final boolean endOfBatch) { @@ -286,19 +295,19 @@ public class DDAgentWriter implements Writer { if (trace != null) { traceCount.incrementAndGet(); try { - final byte[] serializedTrace = api.serializeTrace(trace); + final byte[] serializedTrace = writer.api.serializeTrace(trace); payloadSize += serializedTrace.length; serializedTraces.add(serializedTrace); - monitor.onSerialize(DDAgentWriter.this, trace, serializedTrace); + writer.monitor.onSerialize(writer, trace, serializedTrace); } catch (final JsonProcessingException e) { log.warn("Error serializing trace", e); - monitor.onFailedSerialize(DDAgentWriter.this, trace, e); + writer.monitor.onFailedSerialize(writer, trace, e); } catch (final Throwable e) { log.debug("Error while serializing trace", e); - monitor.onFailedSerialize(DDAgentWriter.this, trace, e); + writer.monitor.onFailedSerialize(writer, trace, e); } } @@ -313,16 +322,16 @@ public class DDAgentWriter implements Writer { private void reportTraces(final boolean early) { try { if (serializedTraces.isEmpty()) { - monitor.onFlush(DDAgentWriter.this, early); + writer.monitor.onFlush(writer, early); - apiPhaser.arrive(); // Allow flush to return + writer.apiPhaser.arrive(); // Allow flush to return return; // scheduleFlush called in finally block. } - if (scheduledWriterExecutor.isShutdown()) { - monitor.onFailedSend( - DDAgentWriter.this, traceCount.get(), payloadSize, DDApi.Response.failed(-1)); - apiPhaser.arrive(); // Allow flush to return + if (writer.scheduledWriterExecutor.isShutdown()) { + writer.monitor.onFailedSend( + writer, traceCount.get(), payloadSize, DDApi.Response.failed(-1)); + writer.apiPhaser.arrive(); // Allow flush to return return; } final List toSend = serializedTraces; @@ -333,20 +342,20 @@ public class DDAgentWriter implements Writer { final int sizeInBytes = payloadSize; try { - monitor.onFlush(DDAgentWriter.this, early); + writer.monitor.onFlush(writer, early); // Run the actual IO task on a different thread to avoid blocking the consumer. try { senderSemaphore.acquire(); } catch (final InterruptedException e) { - monitor.onFailedSend( - DDAgentWriter.this, representativeCount, sizeInBytes, DDApi.Response.failed(e)); + writer.monitor.onFailedSend( + writer, representativeCount, sizeInBytes, DDApi.Response.failed(e)); // Finally, we'll schedule another flush // Any threads awaiting the flush will continue to wait return; } - scheduledWriterExecutor.execute( + writer.scheduledWriterExecutor.execute( new Runnable() { @Override public void run() { @@ -354,13 +363,12 @@ public class DDAgentWriter implements Writer { try { final DDApi.Response response = - api.sendSerializedTraces(representativeCount, sizeInBytes, toSend); + writer.api.sendSerializedTraces(representativeCount, sizeInBytes, toSend); if (response.success()) { log.debug("Successfully sent {} traces to the API", toSend.size()); - monitor.onSend( - DDAgentWriter.this, representativeCount, sizeInBytes, response); + writer.monitor.onSend(writer, representativeCount, sizeInBytes, response); } else { log.debug( "Failed to send {} traces (representing {}) of size {} bytes to the API", @@ -368,8 +376,8 @@ public class DDAgentWriter implements Writer { representativeCount, sizeInBytes); - monitor.onFailedSend( - DDAgentWriter.this, representativeCount, sizeInBytes, response); + writer.monitor.onFailedSend( + writer, representativeCount, sizeInBytes, response); } } catch (final Throwable e) { log.debug("Failed to send traces to the API: {}", e.getMessage()); @@ -378,24 +386,21 @@ public class DDAgentWriter implements Writer { // shouldn't occur. // However, just to be safe to start, create a failed Response to handle any // spurious Throwable-s. - monitor.onFailedSend( - DDAgentWriter.this, - representativeCount, - sizeInBytes, - DDApi.Response.failed(e)); + writer.monitor.onFailedSend( + writer, representativeCount, sizeInBytes, DDApi.Response.failed(e)); } finally { - apiPhaser.arrive(); // Flush completed. + writer.apiPhaser.arrive(); // Flush completed. } } }); } catch (final RejectedExecutionException ex) { - monitor.onFailedSend( - DDAgentWriter.this, representativeCount, sizeInBytes, DDApi.Response.failed(ex)); - apiPhaser.arrive(); // Allow flush to return + writer.monitor.onFailedSend( + writer, representativeCount, sizeInBytes, DDApi.Response.failed(ex)); + writer.apiPhaser.arrive(); // Allow flush to return } } finally { payloadSize = 0; - scheduleFlush(); + writer.scheduleFlush(); } } } From 97ed587547c201f785a3e24f75ce8af5f4ae4943 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 20 Dec 2019 13:01:14 -0800 Subject: [PATCH 05/27] Extract TraceConsumer from DDAgentWriter Unfortunately this required making some things public that were previously private or package visible. I expect this to be temporary. --- .../trace/common/writer/DDAgentWriter.java | 149 +---------------- .../datadog/trace/common/writer/DDApi.java | 13 +- .../common/writer/ddagent/TraceConsumer.java | 151 ++++++++++++++++++ .../trace/api/writer/DDAgentWriterTest.groovy | 3 +- 4 files changed, 164 insertions(+), 152 deletions(-) create mode 100644 dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceConsumer.java diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java index 70851bd433..1010b1043f 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java @@ -5,8 +5,6 @@ import static datadog.trace.api.Config.DEFAULT_AGENT_UNIX_DOMAIN_SOCKET; import static datadog.trace.api.Config.DEFAULT_TRACE_AGENT_PORT; import static java.util.concurrent.TimeUnit.SECONDS; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.lmax.disruptor.EventHandler; import com.lmax.disruptor.SleepingWaitStrategy; import com.lmax.disruptor.dsl.Disruptor; import com.lmax.disruptor.dsl.ProducerType; @@ -14,14 +12,12 @@ import datadog.opentracing.DDSpan; import datadog.trace.common.util.DaemonThreadFactory; import datadog.trace.common.writer.ddagent.DisruptorEvent; import datadog.trace.common.writer.ddagent.Monitor; -import java.util.ArrayList; +import datadog.trace.common.writer.ddagent.TraceConsumer; import java.util.List; import java.util.concurrent.Executors; import java.util.concurrent.Phaser; -import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; -import java.util.concurrent.Semaphore; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -39,7 +35,6 @@ import lombok.extern.slf4j.Slf4j; public class DDAgentWriter implements Writer { private static final int DISRUPTOR_BUFFER_SIZE = 1024; private static final int SENDER_QUEUE_SIZE = 16; - private static final int FLUSH_PAYLOAD_BYTES = 5_000_000; // 5 MB private static final int FLUSH_PAYLOAD_DELAY = 1; // 1/second private static final DisruptorEvent.TraceTranslator TRANSLATOR = @@ -57,13 +52,13 @@ public class DDAgentWriter implements Writer { private final int flushFrequencySeconds; private final Disruptor>> disruptor; - private final ScheduledExecutorService scheduledWriterExecutor; + public final ScheduledExecutorService scheduledWriterExecutor; private final AtomicInteger traceCount = new AtomicInteger(0); private final AtomicReference> flushSchedule = new AtomicReference<>(); - private final Phaser apiPhaser; + public final Phaser apiPhaser; private volatile boolean running = false; - private final Monitor monitor; + public final Monitor monitor; public DDAgentWriter() { this( @@ -248,7 +243,7 @@ public class DDAgentWriter implements Writer { return str; } - private void scheduleFlush() { + public void scheduleFlush() { if (flushFrequencySeconds > 0 && !scheduledWriterExecutor.isShutdown()) { final ScheduledFuture previous = flushSchedule.getAndSet( @@ -270,138 +265,4 @@ public class DDAgentWriter implements Writer { disruptor.publishEvent(FLUSH_TRANSLATOR); } } - - /** This class is intentionally not threadsafe. */ - private static class TraceConsumer implements EventHandler>> { - private final AtomicInteger traceCount; - private final Semaphore senderSemaphore; - private final DDAgentWriter writer; - - private List serializedTraces = new ArrayList<>(); - private int payloadSize = 0; - - private TraceConsumer( - final AtomicInteger traceCount, final int senderQueueSize, final DDAgentWriter writer) { - this.traceCount = traceCount; - senderSemaphore = new Semaphore(senderQueueSize); - this.writer = writer; - } - - @Override - public void onEvent( - final DisruptorEvent> event, final long sequence, final boolean endOfBatch) { - final List trace = event.data; - event.data = null; // clear the event for reuse. - if (trace != null) { - traceCount.incrementAndGet(); - try { - final byte[] serializedTrace = writer.api.serializeTrace(trace); - payloadSize += serializedTrace.length; - serializedTraces.add(serializedTrace); - - writer.monitor.onSerialize(writer, trace, serializedTrace); - } catch (final JsonProcessingException e) { - log.warn("Error serializing trace", e); - - writer.monitor.onFailedSerialize(writer, trace, e); - } catch (final Throwable e) { - log.debug("Error while serializing trace", e); - - writer.monitor.onFailedSerialize(writer, trace, e); - } - } - - if (event.shouldFlush || payloadSize >= FLUSH_PAYLOAD_BYTES) { - final boolean early = (payloadSize >= FLUSH_PAYLOAD_BYTES); - - reportTraces(early); - event.shouldFlush = false; - } - } - - private void reportTraces(final boolean early) { - try { - if (serializedTraces.isEmpty()) { - writer.monitor.onFlush(writer, early); - - writer.apiPhaser.arrive(); // Allow flush to return - return; - // scheduleFlush called in finally block. - } - if (writer.scheduledWriterExecutor.isShutdown()) { - writer.monitor.onFailedSend( - writer, traceCount.get(), payloadSize, DDApi.Response.failed(-1)); - writer.apiPhaser.arrive(); // Allow flush to return - return; - } - final List toSend = serializedTraces; - serializedTraces = new ArrayList<>(toSend.size()); - // ^ Initialize with similar size to reduce arraycopy churn. - - final int representativeCount = traceCount.getAndSet(0); - final int sizeInBytes = payloadSize; - - try { - writer.monitor.onFlush(writer, early); - - // Run the actual IO task on a different thread to avoid blocking the consumer. - try { - senderSemaphore.acquire(); - } catch (final InterruptedException e) { - writer.monitor.onFailedSend( - writer, representativeCount, sizeInBytes, DDApi.Response.failed(e)); - - // Finally, we'll schedule another flush - // Any threads awaiting the flush will continue to wait - return; - } - writer.scheduledWriterExecutor.execute( - new Runnable() { - @Override - public void run() { - senderSemaphore.release(); - - try { - final DDApi.Response response = - writer.api.sendSerializedTraces(representativeCount, sizeInBytes, toSend); - - if (response.success()) { - log.debug("Successfully sent {} traces to the API", toSend.size()); - - writer.monitor.onSend(writer, representativeCount, sizeInBytes, response); - } else { - log.debug( - "Failed to send {} traces (representing {}) of size {} bytes to the API", - toSend.size(), - representativeCount, - sizeInBytes); - - writer.monitor.onFailedSend( - writer, representativeCount, sizeInBytes, response); - } - } catch (final Throwable e) { - log.debug("Failed to send traces to the API: {}", e.getMessage()); - - // DQH - 10/2019 - DDApi should wrap most exceptions itself, so this really - // shouldn't occur. - // However, just to be safe to start, create a failed Response to handle any - // spurious Throwable-s. - writer.monitor.onFailedSend( - writer, representativeCount, sizeInBytes, DDApi.Response.failed(e)); - } finally { - writer.apiPhaser.arrive(); // Flush completed. - } - } - }); - } catch (final RejectedExecutionException ex) { - writer.monitor.onFailedSend( - writer, representativeCount, sizeInBytes, DDApi.Response.failed(ex)); - writer.apiPhaser.arrive(); // Allow flush to return - } - } finally { - payloadSize = 0; - writer.scheduleFlush(); - } - } - } } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java index 164089f3c5..2e42f3194f 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java @@ -21,7 +21,6 @@ import okhttp3.MediaType; import okhttp3.OkHttpClient; import okhttp3.Request; import okhttp3.RequestBody; -import okhttp3.Response; import okio.BufferedSink; import org.msgpack.core.MessagePack; import org.msgpack.core.MessagePacker; @@ -108,11 +107,11 @@ public class DDApi { return sendSerializedTraces(serializedTraces.size(), sizeInBytes, serializedTraces); } - byte[] serializeTrace(final List trace) throws JsonProcessingException { + public byte[] serializeTrace(final List trace) throws JsonProcessingException { return OBJECT_MAPPER.writeValueAsBytes(trace); } - Response sendSerializedTraces( + public Response sendSerializedTraces( final int representativeCount, final Integer sizeInBytes, final List traces) { try { final RequestBody body = @@ -348,21 +347,21 @@ public class DDApi { } public final boolean success() { - return this.success; + return success; } // TODO: DQH - In Java 8, switch to OptionalInteger public final Integer status() { - return this.status; + return status; } public final JsonNode json() { - return this.json; + return json; } // TODO: DQH - In Java 8, switch to Optional? public final Throwable exception() { - return this.exception; + return exception; } } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceConsumer.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceConsumer.java new file mode 100644 index 0000000000..fb63d1f93b --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceConsumer.java @@ -0,0 +1,151 @@ +package datadog.trace.common.writer.ddagent; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.lmax.disruptor.EventHandler; +import datadog.opentracing.DDSpan; +import datadog.trace.common.writer.DDAgentWriter; +import datadog.trace.common.writer.DDApi; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.Semaphore; +import java.util.concurrent.atomic.AtomicInteger; +import lombok.extern.slf4j.Slf4j; + +/** This class is intentionally not threadsafe. */ +@Slf4j +public class TraceConsumer implements EventHandler>> { + private static final int FLUSH_PAYLOAD_BYTES = 5_000_000; // 5 MB + + private final AtomicInteger traceCount; + private final Semaphore senderSemaphore; + private final DDAgentWriter writer; + + private List serializedTraces = new ArrayList<>(); + private int payloadSize = 0; + + public TraceConsumer( + final AtomicInteger traceCount, final int senderQueueSize, final DDAgentWriter writer) { + this.traceCount = traceCount; + senderSemaphore = new Semaphore(senderQueueSize); + this.writer = writer; + } + + @Override + public void onEvent( + final DisruptorEvent> event, final long sequence, final boolean endOfBatch) { + final List trace = event.data; + event.data = null; // clear the event for reuse. + if (trace != null) { + traceCount.incrementAndGet(); + try { + final byte[] serializedTrace = writer.getApi().serializeTrace(trace); + payloadSize += serializedTrace.length; + serializedTraces.add(serializedTrace); + + writer.monitor.onSerialize(writer, trace, serializedTrace); + } catch (final JsonProcessingException e) { + log.warn("Error serializing trace", e); + + writer.monitor.onFailedSerialize(writer, trace, e); + } catch (final Throwable e) { + log.debug("Error while serializing trace", e); + + writer.monitor.onFailedSerialize(writer, trace, e); + } + } + + if (event.shouldFlush || payloadSize >= FLUSH_PAYLOAD_BYTES) { + final boolean early = (payloadSize >= FLUSH_PAYLOAD_BYTES); + + reportTraces(early); + event.shouldFlush = false; + } + } + + private void reportTraces(final boolean early) { + try { + if (serializedTraces.isEmpty()) { + writer.monitor.onFlush(writer, early); + + writer.apiPhaser.arrive(); // Allow flush to return + return; + // scheduleFlush called in finally block. + } + if (writer.scheduledWriterExecutor.isShutdown()) { + writer.monitor.onFailedSend( + writer, traceCount.get(), payloadSize, DDApi.Response.failed(-1)); + writer.apiPhaser.arrive(); // Allow flush to return + return; + } + final List toSend = serializedTraces; + serializedTraces = new ArrayList<>(toSend.size()); + // ^ Initialize with similar size to reduce arraycopy churn. + + final int representativeCount = traceCount.getAndSet(0); + final int sizeInBytes = payloadSize; + + try { + writer.monitor.onFlush(writer, early); + + // Run the actual IO task on a different thread to avoid blocking the consumer. + try { + senderSemaphore.acquire(); + } catch (final InterruptedException e) { + writer.monitor.onFailedSend( + writer, representativeCount, sizeInBytes, DDApi.Response.failed(e)); + + // Finally, we'll schedule another flush + // Any threads awaiting the flush will continue to wait + return; + } + writer.scheduledWriterExecutor.execute( + new Runnable() { + @Override + public void run() { + senderSemaphore.release(); + + try { + final DDApi.Response response = + writer + .getApi() + .sendSerializedTraces(representativeCount, sizeInBytes, toSend); + + if (response.success()) { + log.debug("Successfully sent {} traces to the API", toSend.size()); + + writer.monitor.onSend(writer, representativeCount, sizeInBytes, response); + } else { + log.debug( + "Failed to send {} traces (representing {}) of size {} bytes to the API", + toSend.size(), + representativeCount, + sizeInBytes); + + writer.monitor.onFailedSend(writer, representativeCount, sizeInBytes, response); + } + } catch (final Throwable e) { + log.debug("Failed to send traces to the API: {}", e.getMessage()); + + // DQH - 10/2019 - DDApi should wrap most exceptions itself, so this really + // shouldn't occur. + // However, just to be safe to start, create a failed Response to handle any + // spurious Throwable-s. + writer.monitor.onFailedSend( + writer, representativeCount, sizeInBytes, DDApi.Response.failed(e)); + } finally { + writer.apiPhaser.arrive(); // Flush completed. + } + } + }); + } catch (final RejectedExecutionException ex) { + writer.monitor.onFailedSend( + writer, representativeCount, sizeInBytes, DDApi.Response.failed(ex)); + writer.apiPhaser.arrive(); // Allow flush to return + } + } finally { + payloadSize = 0; + writer.scheduleFlush(); + } + } +} diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy index f61ffb6641..07835abe5d 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy @@ -9,6 +9,7 @@ import datadog.trace.api.sampling.PrioritySampling import datadog.trace.common.writer.DDAgentWriter import datadog.trace.common.writer.DDApi import datadog.trace.common.writer.ddagent.Monitor +import datadog.trace.common.writer.ddagent.TraceConsumer import datadog.trace.util.test.DDSpecification import spock.lang.Timeout @@ -180,7 +181,7 @@ class DDAgentWriterTest extends DDSpecification { minimalSpan = new DDSpan(0, minimalContext) minimalTrace = [minimalSpan] traceSize = DDApi.OBJECT_MAPPER.writeValueAsBytes(minimalTrace).length - maxedPayloadTraceCount = ((int) (DDAgentWriter.FLUSH_PAYLOAD_BYTES / traceSize)) + 1 + maxedPayloadTraceCount = ((int) (TraceConsumer.FLUSH_PAYLOAD_BYTES / traceSize)) + 1 } def "check that are no interactions after close"() { From 84f9d80258e457624f126e16a30b0716007167e8 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 20 Dec 2019 13:36:16 -0800 Subject: [PATCH 06/27] Extract TraceSerializingDisruptor class from DDAgentWriter --- .../trace/common/writer/DDAgentWriter.java | 95 ++------------ .../common/writer/ddagent/TraceConsumer.java | 2 +- .../ddagent/TraceSerializingDisruptor.java | 119 ++++++++++++++++++ .../trace/api/writer/DDAgentWriterTest.groovy | 28 ++--- 4 files changed, 147 insertions(+), 97 deletions(-) create mode 100644 dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceSerializingDisruptor.java diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java index 1010b1043f..a822c540ae 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java @@ -5,23 +5,17 @@ import static datadog.trace.api.Config.DEFAULT_AGENT_UNIX_DOMAIN_SOCKET; import static datadog.trace.api.Config.DEFAULT_TRACE_AGENT_PORT; import static java.util.concurrent.TimeUnit.SECONDS; -import com.lmax.disruptor.SleepingWaitStrategy; -import com.lmax.disruptor.dsl.Disruptor; -import com.lmax.disruptor.dsl.ProducerType; import datadog.opentracing.DDSpan; import datadog.trace.common.util.DaemonThreadFactory; -import datadog.trace.common.writer.ddagent.DisruptorEvent; import datadog.trace.common.writer.ddagent.Monitor; import datadog.trace.common.writer.ddagent.TraceConsumer; +import datadog.trace.common.writer.ddagent.TraceSerializingDisruptor; import java.util.List; import java.util.concurrent.Executors; import java.util.concurrent.Phaser; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; import lombok.extern.slf4j.Slf4j; /** @@ -37,26 +31,16 @@ public class DDAgentWriter implements Writer { private static final int SENDER_QUEUE_SIZE = 16; private static final int FLUSH_PAYLOAD_DELAY = 1; // 1/second - private static final DisruptorEvent.TraceTranslator TRANSLATOR = - new DisruptorEvent.TraceTranslator(); - private static final DisruptorEvent.FlushTranslator FLUSH_TRANSLATOR = - new DisruptorEvent.FlushTranslator(); - - private static final ThreadFactory DISRUPTOR_THREAD_FACTORY = - new DaemonThreadFactory("dd-trace-disruptor"); private static final ThreadFactory SCHEDULED_FLUSH_THREAD_FACTORY = new DaemonThreadFactory("dd-trace-writer"); - private final Runnable flushTask = new FlushTask(); private final DDApi api; - private final int flushFrequencySeconds; - private final Disruptor>> disruptor; + public final int flushFrequencySeconds; + public final TraceSerializingDisruptor disruptor; public final ScheduledExecutorService scheduledWriterExecutor; private final AtomicInteger traceCount = new AtomicInteger(0); - private final AtomicReference> flushSchedule = new AtomicReference<>(); - public final Phaser apiPhaser; - private volatile boolean running = false; + public final Phaser apiPhaser = new Phaser(); // Ensure API calls are completed when flushing; public final Monitor monitor; @@ -114,24 +98,18 @@ public class DDAgentWriter implements Writer { this.monitor = monitor; disruptor = - new Disruptor<>( - new DisruptorEvent.Factory>(), - Math.max(2, Integer.highestOneBit(disruptorSize - 1) << 1), // Next power of 2 - DISRUPTOR_THREAD_FACTORY, - ProducerType.MULTI, - new SleepingWaitStrategy(0, TimeUnit.MILLISECONDS.toNanos(5))); - disruptor.handleEventsWith(new TraceConsumer(traceCount, senderQueueSize, this)); + new TraceSerializingDisruptor( + disruptorSize, this, new TraceConsumer(traceCount, senderQueueSize, this)); this.flushFrequencySeconds = flushFrequencySeconds; scheduledWriterExecutor = Executors.newScheduledThreadPool(1, SCHEDULED_FLUSH_THREAD_FACTORY); - apiPhaser = new Phaser(); // Ensure API calls are completed when flushing apiPhaser.register(); // Register on behalf of the scheduled executor thread. } // Exposing some statistics for consumption by monitors public final long getDisruptorCapacity() { - return disruptor.getRingBuffer().getBufferSize(); + return disruptor.getDisruptorCapacity(); } public final long getDisruptorUtilizedCapacity() { @@ -139,14 +117,14 @@ public class DDAgentWriter implements Writer { } public final long getDisruptorRemainingCapacity() { - return disruptor.getRingBuffer().remainingCapacity(); + return disruptor.getDisruptorRemainingCapacity(); } @Override public void write(final List trace) { // We can't add events after shutdown otherwise it will never complete shutting down. - if (running) { - final boolean published = disruptor.getRingBuffer().tryPublishEvent(TRANSLATOR, trace); + if (disruptor.running) { + final boolean published = disruptor.tryPublish(trace); if (published) { monitor.onPublish(DDAgentWriter.this, trace); @@ -176,15 +154,12 @@ public class DDAgentWriter implements Writer { @Override public void start() { disruptor.start(); - running = true; - scheduleFlush(); monitor.onStart(this); } @Override public void close() { - running = false; boolean flushSuccess = true; @@ -199,34 +174,13 @@ public class DDAgentWriter implements Writer { flushSuccess = false; } - flushSuccess |= flush(); - disruptor.shutdown(); + flushSuccess |= disruptor.flush(); + + disruptor.close(); monitor.onShutdown(this, flushSuccess); } - /** This method will block until the flush is complete. */ - public boolean flush() { - if (running) { - log.info("Flushing any remaining traces."); - // Register with the phaser so we can block until the flush completion. - apiPhaser.register(); - disruptor.publishEvent(FLUSH_TRANSLATOR); - try { - // Allow thread to be interrupted. - apiPhaser.awaitAdvanceInterruptibly(apiPhaser.arriveAndDeregister()); - - return true; - } catch (final InterruptedException e) { - log.warn("Waiting for flush interrupted.", e); - - return false; - } - } else { - return false; - } - } - @Override public String toString() { // DQH - I don't particularly like the instanceof check, @@ -242,27 +196,4 @@ public class DDAgentWriter implements Writer { return str; } - - public void scheduleFlush() { - if (flushFrequencySeconds > 0 && !scheduledWriterExecutor.isShutdown()) { - final ScheduledFuture previous = - flushSchedule.getAndSet( - scheduledWriterExecutor.schedule(flushTask, flushFrequencySeconds, SECONDS)); - - final boolean previousIncomplete = (previous != null); - if (previousIncomplete) { - previous.cancel(true); - } - - monitor.onScheduleFlush(this, previousIncomplete); - } - } - - private class FlushTask implements Runnable { - @Override - public void run() { - // Don't call flush() because it would block the thread also used for sending the traces. - disruptor.publishEvent(FLUSH_TRANSLATOR); - } - } } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceConsumer.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceConsumer.java index fb63d1f93b..921961eb02 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceConsumer.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceConsumer.java @@ -145,7 +145,7 @@ public class TraceConsumer implements EventHandler>> } } finally { payloadSize = 0; - writer.scheduleFlush(); + writer.disruptor.scheduleFlush(); } } } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceSerializingDisruptor.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceSerializingDisruptor.java new file mode 100644 index 0000000000..6404f66891 --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceSerializingDisruptor.java @@ -0,0 +1,119 @@ +package datadog.trace.common.writer.ddagent; + +import static java.util.concurrent.TimeUnit.SECONDS; + +import com.lmax.disruptor.SleepingWaitStrategy; +import com.lmax.disruptor.dsl.Disruptor; +import com.lmax.disruptor.dsl.ProducerType; +import datadog.opentracing.DDSpan; +import datadog.trace.common.util.DaemonThreadFactory; +import datadog.trace.common.writer.DDAgentWriter; +import java.io.Closeable; +import java.util.List; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class TraceSerializingDisruptor implements Closeable { + private static final ThreadFactory DISRUPTOR_THREAD_FACTORY = + new DaemonThreadFactory("dd-trace-disruptor"); + private static final DisruptorEvent.TraceTranslator TRACE_TRANSLATOR = + new DisruptorEvent.TraceTranslator(); + private static final DisruptorEvent.FlushTranslator FLUSH_TRANSLATOR = + new DisruptorEvent.FlushTranslator(); + private final FlushTask flushTask = new FlushTask(); + + private final Disruptor>> disruptor; + private final DDAgentWriter writer; + + public volatile boolean running = false; + + private final AtomicReference> flushSchedule = new AtomicReference<>(); + + public TraceSerializingDisruptor( + final int disruptorSize, final DDAgentWriter writer, final TraceConsumer handler) { + disruptor = + new Disruptor<>( + new DisruptorEvent.Factory>(), + Math.max(2, Integer.highestOneBit(disruptorSize - 1) << 1), // Next power of 2 + DISRUPTOR_THREAD_FACTORY, + ProducerType.MULTI, + new SleepingWaitStrategy(0, TimeUnit.MILLISECONDS.toNanos(5))); + this.writer = writer; + disruptor.handleEventsWith(handler); + } + + public void start() { + disruptor.start(); + running = true; + scheduleFlush(); + } + + @Override + public void close() { + running = false; + disruptor.shutdown(); + } + + public boolean tryPublish(final List trace) { + return disruptor.getRingBuffer().tryPublishEvent(TRACE_TRANSLATOR, trace); + } + + /** This method will block until the flush is complete. */ + public boolean flush() { + if (running) { + log.info("Flushing any remaining traces."); + // Register with the phaser so we can block until the flush completion. + writer.apiPhaser.register(); + disruptor.publishEvent(FLUSH_TRANSLATOR); + try { + // Allow thread to be interrupted. + writer.apiPhaser.awaitAdvanceInterruptibly(writer.apiPhaser.arriveAndDeregister()); + + return true; + } catch (final InterruptedException e) { + log.warn("Waiting for flush interrupted.", e); + + return false; + } + } else { + return false; + } + } + + public void scheduleFlush() { + if (writer.flushFrequencySeconds > 0 && !writer.scheduledWriterExecutor.isShutdown()) { + final ScheduledFuture previous = + flushSchedule.getAndSet( + writer.scheduledWriterExecutor.schedule( + flushTask, writer.flushFrequencySeconds, SECONDS)); + + final boolean previousIncomplete = (previous != null); + if (previousIncomplete) { + previous.cancel(true); + } + + writer.monitor.onScheduleFlush(writer, previousIncomplete); + } + } + + private class FlushTask implements Runnable { + @Override + public void run() { + // Don't call flush() because it would block the thread also used for sending the traces. + disruptor.publishEvent(FLUSH_TRANSLATOR); + } + } + + // Exposing some statistics for consumption by monitors + public final long getDisruptorCapacity() { + return disruptor.getRingBuffer().getBufferSize(); + } + + public final long getDisruptorRemainingCapacity() { + return disruptor.getRingBuffer().remainingCapacity(); + } +} diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy index 07835abe5d..c4b7c043be 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy @@ -34,7 +34,7 @@ class DDAgentWriterTest extends DDSpecification { when: writer.write(trace) writer.write(trace) - writer.flush() + writer.disruptor.flush() then: 2 * api.serializeTrace(_) >> { trace -> callRealMethod() } @@ -57,7 +57,7 @@ class DDAgentWriterTest extends DDSpecification { (1..traceCount).each { writer.write(trace) } - writer.flush() + writer.disruptor.flush() then: _ * api.serializeTrace(_) >> { trace -> callRealMethod() } @@ -97,7 +97,7 @@ class DDAgentWriterTest extends DDSpecification { writer.write(trace) } // Flush the remaining 2 - writer.flush() + writer.disruptor.flush() then: 2 * api.serializeTrace(_) >> { trace -> callRealMethod() } @@ -118,7 +118,7 @@ class DDAgentWriterTest extends DDSpecification { def phaser = writer.apiPhaser phaser.register() writer.start() - writer.flush() + writer.disruptor.flush() when: (1..5).each { @@ -153,7 +153,7 @@ class DDAgentWriterTest extends DDSpecification { // Busywait because we don't want to fill up the ring buffer } } - writer.flush() + writer.disruptor.flush() then: (maxedPayloadTraceCount + 1) * api.serializeTrace(_) >> { trace -> callRealMethod() } @@ -193,7 +193,7 @@ class DDAgentWriterTest extends DDSpecification { when: writer.close() writer.write([]) - writer.flush() + writer.disruptor.flush() then: 0 * _ @@ -208,7 +208,7 @@ class DDAgentWriterTest extends DDSpecification { when: writer.write([]) - writer.flush() + writer.disruptor.flush() then: 1 * api.serializeTrace(_) >> { trace -> callRealMethod() } @@ -265,7 +265,7 @@ class DDAgentWriterTest extends DDSpecification { when: writer.write(minimalTrace) - writer.flush() + writer.disruptor.flush() then: 1 * monitor.onPublish(writer, minimalTrace) @@ -314,7 +314,7 @@ class DDAgentWriterTest extends DDSpecification { when: writer.write(minimalTrace) - writer.flush() + writer.disruptor.flush() then: 1 * monitor.onPublish(writer, minimalTrace) @@ -356,7 +356,7 @@ class DDAgentWriterTest extends DDSpecification { when: writer.write(minimalTrace) - writer.flush() + writer.disruptor.flush() then: 1 * monitor.onPublish(writer, minimalTrace) @@ -438,7 +438,7 @@ class DDAgentWriterTest extends DDSpecification { // sanity check coordination mechanism of test // release to allow response to be generated responseSemaphore.release() - writer.flush() + writer.disruptor.flush() // reacquire semaphore to stall further responses responseSemaphore.acquire() @@ -538,7 +538,7 @@ class DDAgentWriterTest extends DDSpecification { t1.join() t2.join() - writer.flush() + writer.disruptor.flush() then: def totalTraces = 100 + 100 @@ -585,7 +585,7 @@ class DDAgentWriterTest extends DDSpecification { when: writer.write(minimalTrace) - writer.flush() + writer.disruptor.flush() then: numTracesAccepted == 1 @@ -633,7 +633,7 @@ class DDAgentWriterTest extends DDSpecification { when: writer.write(minimalTrace) - writer.flush() + writer.disruptor.flush() then: numRequests == 1 From 24e2fe6da707c8fe2cc2e767a39fb6ae7f82ef48 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 20 Dec 2019 14:36:45 -0800 Subject: [PATCH 07/27] Move DDApi and rename to DDAgentApi Reduce visibility back to what it was before refactoring. --- .../java/datadog/opentracing/DDTracer.java | 8 +++--- .../common/sampling/RateByServiceSampler.java | 2 +- .../trace/common/writer/DDAgentWriter.java | 21 ++++++++------ .../datadog/trace/common/writer/Writer.java | 5 ++-- .../{DDApi.java => ddagent/DDAgentApi.java} | 14 +++++----- .../common/writer/ddagent/DisruptorEvent.java | 14 +++++++--- .../trace/common/writer/ddagent/Monitor.java | 15 +++++----- .../common/writer/ddagent/TraceConsumer.java | 11 ++++---- .../ddagent/TraceSerializingDisruptor.java | 6 ++-- ...DDApiTest.groovy => DDAgentApiTest.groovy} | 22 +++++++-------- .../trace/api/writer/DDAgentWriterTest.groovy | 28 +++++++++---------- .../groovy/DDApiIntegrationTest.groovy | 12 ++++---- 12 files changed, 82 insertions(+), 76 deletions(-) rename dd-trace-ot/src/main/java/datadog/trace/common/writer/{DDApi.java => ddagent/DDAgentApi.java} (97%) rename dd-trace-ot/src/test/groovy/datadog/trace/api/writer/{DDApiTest.groovy => DDAgentApiTest.groovy} (90%) diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java index 85908feb28..ad5bed5270 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -14,8 +14,8 @@ import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.common.sampling.PrioritySampler; import datadog.trace.common.sampling.Sampler; import datadog.trace.common.writer.DDAgentWriter; -import datadog.trace.common.writer.DDApi; import datadog.trace.common.writer.Writer; +import datadog.trace.common.writer.ddagent.DDAgentApi; import datadog.trace.context.ScopeListener; import io.opentracing.References; import io.opentracing.Scope; @@ -246,9 +246,9 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace extractor = HttpCodec.createExtractor(Config.get(), taggedHeaders); if (this.writer instanceof DDAgentWriter) { - final DDApi api = ((DDAgentWriter) this.writer).getApi(); - if (sampler instanceof DDApi.ResponseListener) { - api.addResponseListener((DDApi.ResponseListener) this.sampler); + final DDAgentApi api = ((DDAgentWriter) this.writer).getApi(); + if (sampler instanceof DDAgentApi.ResponseListener) { + api.addResponseListener((DDAgentApi.ResponseListener) this.sampler); } } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/sampling/RateByServiceSampler.java b/dd-trace-ot/src/main/java/datadog/trace/common/sampling/RateByServiceSampler.java index c5b8242d76..d96c2f3592 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/sampling/RateByServiceSampler.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/sampling/RateByServiceSampler.java @@ -7,7 +7,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.NumericNode; import datadog.opentracing.DDSpan; import datadog.trace.api.sampling.PrioritySampling; -import datadog.trace.common.writer.DDApi.ResponseListener; +import datadog.trace.common.writer.ddagent.DDAgentApi.ResponseListener; import java.util.HashMap; import java.util.Iterator; import java.util.Map; diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java index a822c540ae..76ea6b8df9 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java @@ -7,6 +7,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; import datadog.opentracing.DDSpan; import datadog.trace.common.util.DaemonThreadFactory; +import datadog.trace.common.writer.ddagent.DDAgentApi; import datadog.trace.common.writer.ddagent.Monitor; import datadog.trace.common.writer.ddagent.TraceConsumer; import datadog.trace.common.writer.ddagent.TraceSerializingDisruptor; @@ -34,7 +35,7 @@ public class DDAgentWriter implements Writer { private static final ThreadFactory SCHEDULED_FLUSH_THREAD_FACTORY = new DaemonThreadFactory("dd-trace-writer"); - private final DDApi api; + private final DDAgentApi api; public final int flushFrequencySeconds; public final TraceSerializingDisruptor disruptor; @@ -46,16 +47,17 @@ public class DDAgentWriter implements Writer { public DDAgentWriter() { this( - new DDApi(DEFAULT_AGENT_HOST, DEFAULT_TRACE_AGENT_PORT, DEFAULT_AGENT_UNIX_DOMAIN_SOCKET), + new DDAgentApi( + DEFAULT_AGENT_HOST, DEFAULT_TRACE_AGENT_PORT, DEFAULT_AGENT_UNIX_DOMAIN_SOCKET), new Monitor.Noop()); } - public DDAgentWriter(final DDApi api, final Monitor monitor) { + public DDAgentWriter(final DDAgentApi api, final Monitor monitor) { this(api, monitor, DISRUPTOR_BUFFER_SIZE, SENDER_QUEUE_SIZE, FLUSH_PAYLOAD_DELAY); } /** Old signature (pre-Monitor) used in tests */ - private DDAgentWriter(final DDApi api) { + private DDAgentWriter(final DDAgentApi api) { this(api, new Monitor.Noop()); } @@ -67,7 +69,7 @@ public class DDAgentWriter implements Writer { * @param flushFrequencySeconds value < 1 disables scheduled flushes */ private DDAgentWriter( - final DDApi api, + final DDAgentApi api, final int disruptorSize, final int senderQueueSize, final int flushFrequencySeconds) { @@ -76,7 +78,7 @@ public class DDAgentWriter implements Writer { // DQH - TODO - Update the tests & remove this private DDAgentWriter( - final DDApi api, + final DDAgentApi api, final Monitor monitor, final int disruptorSize, final int flushFrequencySeconds) { @@ -84,12 +86,13 @@ public class DDAgentWriter implements Writer { } // DQH - TODO - Update the tests & remove this - private DDAgentWriter(final DDApi api, final int disruptorSize, final int flushFrequencySeconds) { + private DDAgentWriter( + final DDAgentApi api, final int disruptorSize, final int flushFrequencySeconds) { this(api, new Monitor.Noop(), disruptorSize, SENDER_QUEUE_SIZE, flushFrequencySeconds); } private DDAgentWriter( - final DDApi api, + final DDAgentApi api, final Monitor monitor, final int disruptorSize, final int senderQueueSize, @@ -147,7 +150,7 @@ public class DDAgentWriter implements Writer { traceCount.incrementAndGet(); } - public DDApi getApi() { + public DDAgentApi getApi() { return api; } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java index 669bcfd9fc..b14cbfdd23 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java @@ -2,6 +2,7 @@ package datadog.trace.common.writer; import datadog.opentracing.DDSpan; import datadog.trace.api.Config; +import datadog.trace.common.writer.ddagent.DDAgentApi; import datadog.trace.common.writer.ddagent.Monitor; import java.io.Closeable; import java.util.List; @@ -66,8 +67,8 @@ public interface Writer extends Closeable { return new DDAgentWriter(createApi(config), createMonitor(config)); } - private static DDApi createApi(final Config config) { - return new DDApi( + private static DDAgentApi createApi(final Config config) { + return new DDAgentApi( config.getAgentHost(), config.getAgentPort(), config.getAgentUnixDomainSocket()); } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java similarity index 97% rename from dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java rename to dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java index 2e42f3194f..cc01adc62c 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java @@ -1,4 +1,4 @@ -package datadog.trace.common.writer; +package datadog.trace.common.writer.ddagent; import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.JsonProcessingException; @@ -28,7 +28,7 @@ import org.msgpack.jackson.dataformat.MessagePackFactory; /** The API pointing to a DD agent */ @Slf4j -public class DDApi { +public class DDAgentApi { private static final String DATADOG_META_LANG = "Datadog-Meta-Lang"; private static final String DATADOG_META_LANG_VERSION = "Datadog-Meta-Lang-Version"; private static final String DATADOG_META_LANG_INTERPRETER = "Datadog-Meta-Lang-Interpreter"; @@ -53,7 +53,7 @@ public class DDApi { private final OkHttpClient httpClient; private final HttpUrl tracesUrl; - public DDApi(final String host, final int port, final String unixDomainSocketPath) { + public DDAgentApi(final String host, final int port, final String unixDomainSocketPath) { this( host, port, @@ -61,7 +61,7 @@ public class DDApi { unixDomainSocketPath); } - DDApi( + DDAgentApi( final String host, final int port, final boolean v4EndpointsAvailable, @@ -89,7 +89,7 @@ public class DDApi { * @return a Response object -- encapsulating success of communication, sending, and result * parsing */ - public Response sendTraces(final List> traces) { + Response sendTraces(final List> traces) { final List serializedTraces = new ArrayList<>(traces.size()); int sizeInBytes = 0; for (final List trace : traces) { @@ -107,11 +107,11 @@ public class DDApi { return sendSerializedTraces(serializedTraces.size(), sizeInBytes, serializedTraces); } - public byte[] serializeTrace(final List trace) throws JsonProcessingException { + byte[] serializeTrace(final List trace) throws JsonProcessingException { return OBJECT_MAPPER.writeValueAsBytes(trace); } - public Response sendSerializedTraces( + Response sendSerializedTraces( final int representativeCount, final Integer sizeInBytes, final List traces) { try { final RequestBody body = diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DisruptorEvent.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DisruptorEvent.java index 30db86f21a..66ff7a499b 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DisruptorEvent.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DisruptorEvent.java @@ -6,19 +6,22 @@ import com.lmax.disruptor.EventTranslatorOneArg; import datadog.opentracing.DDSpan; import java.util.List; -public class DisruptorEvent { +class DisruptorEvent { public volatile boolean shouldFlush = false; public volatile T data = null; - public static class Factory implements EventFactory> { + static class Factory implements EventFactory> { @Override public DisruptorEvent newInstance() { return new DisruptorEvent<>(); } } - public static class TraceTranslator + static class TraceTranslator implements EventTranslatorOneArg>, List> { + static final DisruptorEvent.TraceTranslator TRACE_TRANSLATOR = + new DisruptorEvent.TraceTranslator(); + @Override public void translateTo( final DisruptorEvent> event, final long sequence, final List trace) { @@ -26,7 +29,10 @@ public class DisruptorEvent { } } - public static class FlushTranslator implements EventTranslator>> { + static class FlushTranslator implements EventTranslator>> { + static final DisruptorEvent.FlushTranslator FLUSH_TRANSLATOR = + new DisruptorEvent.FlushTranslator(); + @Override public void translateTo(final DisruptorEvent> event, final long sequence) { event.shouldFlush = true; diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/Monitor.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/Monitor.java index 12afd9906c..298fb94b93 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/Monitor.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/Monitor.java @@ -5,7 +5,6 @@ import com.timgroup.statsd.StatsDClient; import datadog.opentracing.DDSpan; import datadog.opentracing.DDTraceOTInfo; import datadog.trace.common.writer.DDAgentWriter; -import datadog.trace.common.writer.DDApi; import java.util.List; /** @@ -43,13 +42,13 @@ public interface Monitor { final DDAgentWriter agentWriter, final int representativeCount, final int sizeInBytes, - final DDApi.Response response); + final DDAgentApi.Response response); void onFailedSend( final DDAgentWriter agentWriter, final int representativeCount, final int sizeInBytes, - final DDApi.Response response); + final DDAgentApi.Response response); final class StatsD implements Monitor { public static final String PREFIX = "datadog.tracer"; @@ -138,7 +137,7 @@ public interface Monitor { final DDAgentWriter agentWriter, final int representativeCount, final int sizeInBytes, - final DDApi.Response response) { + final DDAgentApi.Response response) { onSendAttempt(agentWriter, representativeCount, sizeInBytes, response); } @@ -147,7 +146,7 @@ public interface Monitor { final DDAgentWriter agentWriter, final int representativeCount, final int sizeInBytes, - final DDApi.Response response) { + final DDAgentApi.Response response) { onSendAttempt(agentWriter, representativeCount, sizeInBytes, response); } @@ -155,7 +154,7 @@ public interface Monitor { final DDAgentWriter agentWriter, final int representativeCount, final int sizeInBytes, - final DDApi.Response response) { + final DDAgentApi.Response response) { statsd.incrementCounter("api.requests"); statsd.recordGaugeValue("queue.length", representativeCount); // TODO: missing queue.spans (# of spans being sent) @@ -215,14 +214,14 @@ public interface Monitor { final DDAgentWriter agentWriter, final int representativeCount, final int sizeInBytes, - final DDApi.Response response) {} + final DDAgentApi.Response response) {} @Override public void onFailedSend( final DDAgentWriter agentWriter, final int representativeCount, final int sizeInBytes, - final DDApi.Response response) {} + final DDAgentApi.Response response) {} @Override public String toString() { diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceConsumer.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceConsumer.java index 921961eb02..1081f4352e 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceConsumer.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceConsumer.java @@ -4,7 +4,6 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.lmax.disruptor.EventHandler; import datadog.opentracing.DDSpan; import datadog.trace.common.writer.DDAgentWriter; -import datadog.trace.common.writer.DDApi; import java.util.ArrayList; import java.util.List; import java.util.concurrent.RejectedExecutionException; @@ -74,7 +73,7 @@ public class TraceConsumer implements EventHandler>> } if (writer.scheduledWriterExecutor.isShutdown()) { writer.monitor.onFailedSend( - writer, traceCount.get(), payloadSize, DDApi.Response.failed(-1)); + writer, traceCount.get(), payloadSize, DDAgentApi.Response.failed(-1)); writer.apiPhaser.arrive(); // Allow flush to return return; } @@ -93,7 +92,7 @@ public class TraceConsumer implements EventHandler>> senderSemaphore.acquire(); } catch (final InterruptedException e) { writer.monitor.onFailedSend( - writer, representativeCount, sizeInBytes, DDApi.Response.failed(e)); + writer, representativeCount, sizeInBytes, DDAgentApi.Response.failed(e)); // Finally, we'll schedule another flush // Any threads awaiting the flush will continue to wait @@ -106,7 +105,7 @@ public class TraceConsumer implements EventHandler>> senderSemaphore.release(); try { - final DDApi.Response response = + final DDAgentApi.Response response = writer .getApi() .sendSerializedTraces(representativeCount, sizeInBytes, toSend); @@ -132,7 +131,7 @@ public class TraceConsumer implements EventHandler>> // However, just to be safe to start, create a failed Response to handle any // spurious Throwable-s. writer.monitor.onFailedSend( - writer, representativeCount, sizeInBytes, DDApi.Response.failed(e)); + writer, representativeCount, sizeInBytes, DDAgentApi.Response.failed(e)); } finally { writer.apiPhaser.arrive(); // Flush completed. } @@ -140,7 +139,7 @@ public class TraceConsumer implements EventHandler>> }); } catch (final RejectedExecutionException ex) { writer.monitor.onFailedSend( - writer, representativeCount, sizeInBytes, DDApi.Response.failed(ex)); + writer, representativeCount, sizeInBytes, DDAgentApi.Response.failed(ex)); writer.apiPhaser.arrive(); // Allow flush to return } } finally { diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceSerializingDisruptor.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceSerializingDisruptor.java index 6404f66891..f878cbb178 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceSerializingDisruptor.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/TraceSerializingDisruptor.java @@ -1,5 +1,7 @@ package datadog.trace.common.writer.ddagent; +import static datadog.trace.common.writer.ddagent.DisruptorEvent.FlushTranslator.FLUSH_TRANSLATOR; +import static datadog.trace.common.writer.ddagent.DisruptorEvent.TraceTranslator.TRACE_TRANSLATOR; import static java.util.concurrent.TimeUnit.SECONDS; import com.lmax.disruptor.SleepingWaitStrategy; @@ -20,10 +22,6 @@ import lombok.extern.slf4j.Slf4j; public class TraceSerializingDisruptor implements Closeable { private static final ThreadFactory DISRUPTOR_THREAD_FACTORY = new DaemonThreadFactory("dd-trace-disruptor"); - private static final DisruptorEvent.TraceTranslator TRACE_TRANSLATOR = - new DisruptorEvent.TraceTranslator(); - private static final DisruptorEvent.FlushTranslator FLUSH_TRANSLATOR = - new DisruptorEvent.FlushTranslator(); private final FlushTask flushTask = new FlushTask(); private final Disruptor>> disruptor; diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDApiTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentApiTest.groovy similarity index 90% rename from dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDApiTest.groovy rename to dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentApiTest.groovy index 17c8388cd6..07bd326685 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDApiTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentApiTest.groovy @@ -3,8 +3,8 @@ package datadog.trace.api.writer import com.fasterxml.jackson.core.type.TypeReference import com.fasterxml.jackson.databind.JsonNode import datadog.opentracing.SpanFactory -import datadog.trace.common.writer.DDApi -import datadog.trace.common.writer.DDApi.ResponseListener +import datadog.trace.common.writer.ddagent.DDAgentApi +import datadog.trace.common.writer.ddagent.DDAgentApi.ResponseListener import datadog.trace.util.test.DDSpecification import java.util.concurrent.atomic.AtomicLong @@ -12,8 +12,8 @@ import java.util.concurrent.atomic.AtomicReference import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer -class DDApiTest extends DDSpecification { - static mapper = DDApi.OBJECT_MAPPER +class DDAgentApiTest extends DDSpecification { + static mapper = DDAgentApi.OBJECT_MAPPER def "sending an empty list of traces returns no errors"() { setup: @@ -30,7 +30,7 @@ class DDApiTest extends DDSpecification { } } } - def client = new DDApi("localhost", agent.address.port, null) + def client = new DDAgentApi("localhost", agent.address.port, null) expect: client.tracesUrl.toString() == "http://localhost:${agent.address.port}/v0.4/traces" @@ -51,7 +51,7 @@ class DDApiTest extends DDSpecification { } } } - def client = new DDApi("localhost", agent.address.port, null) + def client = new DDAgentApi("localhost", agent.address.port, null) expect: client.tracesUrl.toString() == "http://localhost:${agent.address.port}/v0.3/traces" @@ -73,7 +73,7 @@ class DDApiTest extends DDSpecification { } } } - def client = new DDApi("localhost", agent.address.port, null) + def client = new DDAgentApi("localhost", agent.address.port, null) expect: client.tracesUrl.toString() == "http://localhost:${agent.address.port}/v0.4/traces" @@ -136,7 +136,7 @@ class DDApiTest extends DDSpecification { } } } - def client = new DDApi("localhost", agent.address.port, null) + def client = new DDAgentApi("localhost", agent.address.port, null) client.addResponseListener(responseListener) when: @@ -162,7 +162,7 @@ class DDApiTest extends DDSpecification { } } } - def client = new DDApi("localhost", v3Agent.address.port, null) + def client = new DDAgentApi("localhost", v3Agent.address.port, null) expect: client.tracesUrl.toString() == "http://localhost:${v3Agent.address.port}/v0.3/traces" @@ -189,7 +189,7 @@ class DDApiTest extends DDSpecification { } } def port = badPort ? 999 : agent.address.port - def client = new DDApi("localhost", port, null) + def client = new DDAgentApi("localhost", port, null) expect: client.tracesUrl.toString() == "http://localhost:${port}/$endpointVersion/traces" @@ -216,7 +216,7 @@ class DDApiTest extends DDSpecification { } } } - def client = new DDApi("localhost", agent.address.port, null) + def client = new DDAgentApi("localhost", agent.address.port, null) when: def success = client.sendTraces(traces).success() diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy index c4b7c043be..37de61bf23 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentWriterTest.groovy @@ -7,7 +7,7 @@ import datadog.opentracing.DDTracer import datadog.opentracing.PendingTrace import datadog.trace.api.sampling.PrioritySampling import datadog.trace.common.writer.DDAgentWriter -import datadog.trace.common.writer.DDApi +import datadog.trace.common.writer.ddagent.DDAgentApi import datadog.trace.common.writer.ddagent.Monitor import datadog.trace.common.writer.ddagent.TraceConsumer import datadog.trace.util.test.DDSpecification @@ -24,7 +24,7 @@ import static datadog.trace.common.writer.DDAgentWriter.DISRUPTOR_BUFFER_SIZE @Timeout(20) class DDAgentWriterTest extends DDSpecification { - def api = Mock(DDApi) + def api = Mock(DDAgentApi) def "test happy path"() { setup: @@ -180,7 +180,7 @@ class DDAgentWriterTest extends DDSpecification { Mock(DDTracer)) minimalSpan = new DDSpan(0, minimalContext) minimalTrace = [minimalSpan] - traceSize = DDApi.OBJECT_MAPPER.writeValueAsBytes(minimalTrace).length + traceSize = DDAgentApi.OBJECT_MAPPER.writeValueAsBytes(minimalTrace).length maxedPayloadTraceCount = ((int) (TraceConsumer.FLUSH_PAYLOAD_BYTES / traceSize)) + 1 } @@ -253,7 +253,7 @@ class DDAgentWriterTest extends DDSpecification { } } } - def api = new DDApi("localhost", agent.address.port, null) + def api = new DDAgentApi("localhost", agent.address.port, null) def monitor = Mock(Monitor) def writer = new DDAgentWriter(api, monitor) @@ -302,7 +302,7 @@ class DDAgentWriterTest extends DDSpecification { } } } - def api = new DDApi("localhost", agent.address.port, null) + def api = new DDAgentApi("localhost", agent.address.port, null) def monitor = Mock(Monitor) def writer = new DDAgentWriter(api, monitor) @@ -336,13 +336,13 @@ class DDAgentWriterTest extends DDSpecification { setup: def minimalTrace = createMinimalTrace() - def api = new DDApi("localhost", 8192, null) { - DDApi.Response sendSerializedTraces( + def api = new DDAgentApi("localhost", 8192, null) { + DDAgentApi.Response sendSerializedTraces( int representativeCount, Integer sizeInBytes, List traces) { // simulating a communication failure to a server - return DDApi.Response.failed(new IOException("comm error")) + return DDAgentApi.Response.failed(new IOException("comm error")) } } def monitor = Mock(Monitor) @@ -400,7 +400,7 @@ class DDAgentWriterTest extends DDSpecification { } } } - def api = new DDApi("localhost", agent.address.port, null) + def api = new DDAgentApi("localhost", agent.address.port, null) // This test focuses just on failed publish, so not verifying every callback def monitor = Stub(Monitor) @@ -505,7 +505,7 @@ class DDAgentWriterTest extends DDSpecification { } } } - def api = new DDApi("localhost", agent.address.port, null) + def api = new DDAgentApi("localhost", agent.address.port, null) // This test focuses just on failed publish, so not verifying every callback def monitor = Stub(Monitor) @@ -566,7 +566,7 @@ class DDAgentWriterTest extends DDSpecification { } } } - def api = new DDApi("localhost", agent.address.port, null) + def api = new DDAgentApi("localhost", agent.address.port, null) def statsd = Stub(StatsDClient) statsd.incrementCounter("queue.accepted") >> { stat -> @@ -606,13 +606,13 @@ class DDAgentWriterTest extends DDSpecification { def minimalTrace = createMinimalTrace() // DQH -- need to set-up a dummy agent for the final send callback to work - def api = new DDApi("localhost", 8192, null) { - DDApi.Response sendSerializedTraces( + def api = new DDAgentApi("localhost", 8192, null) { + DDAgentApi.Response sendSerializedTraces( int representativeCount, Integer sizeInBytes, List traces) { // simulating a communication failure to a server - return DDApi.Response.failed(new IOException("comm error")) + return DDAgentApi.Response.failed(new IOException("comm error")) } } diff --git a/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy b/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy index ef45512c5d..1d5ee8297f 100644 --- a/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy +++ b/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy @@ -4,8 +4,8 @@ import datadog.opentracing.DDSpanContext import datadog.opentracing.DDTracer import datadog.opentracing.PendingTrace import datadog.trace.api.sampling.PrioritySampling -import datadog.trace.common.writer.DDApi import datadog.trace.common.writer.ListWriter +import datadog.trace.common.writer.ddagent.DDAgentApi import datadog.trace.util.test.DDSpecification import org.testcontainers.containers.GenericContainer import org.testcontainers.containers.startupcheck.MinimumDurationRunningStartupCheckStrategy @@ -20,7 +20,7 @@ class DDApiIntegrationTest { // Do not run tests locally on Java7 since testcontainers are not compatible with Java7 // It is fine to run on CI because CI provides rabbitmq externally, not through testcontainers @Requires({ "true" == System.getenv("CI") || jvm.java8Compatible }) - static class DDApiIntegrationV4Test extends DDSpecification { + static class DDAgentApiIntegrationV4Test extends DDSpecification { static final WRITER = new ListWriter() static final TRACER = new DDTracer(WRITER) static final CONTEXT = new DDSpanContext( @@ -64,7 +64,7 @@ class DDApiIntegrationTest { def endpoint = new AtomicReference(null) def agentResponse = new AtomicReference(null) - DDApi.ResponseListener responseListener = { String receivedEndpoint, JsonNode responseJson -> + DDAgentApi.ResponseListener responseListener = { String receivedEndpoint, JsonNode responseJson -> endpoint.set(receivedEndpoint) agentResponse.set(responseJson.toString()) } @@ -109,10 +109,10 @@ class DDApiIntegrationTest { } def setup() { - api = new DDApi(agentContainerHost, agentContainerPort, v4(), null) + api = new DDAgentApi(agentContainerHost, agentContainerPort, v4(), null) api.addResponseListener(responseListener) - unixDomainSocketApi = new DDApi(SOMEHOST, SOMEPORT, v4(), socketPath.toString()) + unixDomainSocketApi = new DDAgentApi(SOMEHOST, SOMEPORT, v4(), socketPath.toString()) unixDomainSocketApi.addResponseListener(responseListener) } @@ -159,7 +159,7 @@ class DDApiIntegrationTest { } @Requires({ "true" == System.getenv("CI") || jvm.java8Compatible }) - static class DDApiIntegrationV3Test extends DDApiIntegrationV4Test { + static class DDAgentApiIntegrationV3Test extends DDAgentApiIntegrationV4Test { boolean v4() { return false } From 0a89f2a57c6429bc0fa8b317793e9c938ae126ff Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 20 Dec 2019 14:49:31 -0800 Subject: [PATCH 08/27] Extract DDAgentResponseListener from DDApi. Reduce references to DDApi --- .../src/main/java/datadog/opentracing/DDTracer.java | 9 +++------ .../trace/common/sampling/RateByServiceSampler.java | 4 ++-- .../datadog/trace/common/writer/DDAgentWriter.java | 5 +++++ .../trace/common/writer/ddagent/DDAgentApi.java | 11 +++-------- .../writer/ddagent/DDAgentResponseListener.java | 8 ++++++++ .../datadog/trace/api/writer/DDAgentApiTest.groovy | 4 ++-- .../traceAgentTest/groovy/DDApiIntegrationTest.groovy | 3 ++- 7 files changed, 25 insertions(+), 19 deletions(-) create mode 100644 dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DDAgentResponseListener.java diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java index ad5bed5270..ae5a0c4301 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -15,7 +15,7 @@ import datadog.trace.common.sampling.PrioritySampler; import datadog.trace.common.sampling.Sampler; import datadog.trace.common.writer.DDAgentWriter; import datadog.trace.common.writer.Writer; -import datadog.trace.common.writer.ddagent.DDAgentApi; +import datadog.trace.common.writer.ddagent.DDAgentResponseListener; import datadog.trace.context.ScopeListener; import io.opentracing.References; import io.opentracing.Scope; @@ -245,11 +245,8 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace injector = HttpCodec.createInjector(Config.get()); extractor = HttpCodec.createExtractor(Config.get(), taggedHeaders); - if (this.writer instanceof DDAgentWriter) { - final DDAgentApi api = ((DDAgentWriter) this.writer).getApi(); - if (sampler instanceof DDAgentApi.ResponseListener) { - api.addResponseListener((DDAgentApi.ResponseListener) this.sampler); - } + if (this.writer instanceof DDAgentWriter && sampler instanceof DDAgentResponseListener) { + ((DDAgentWriter) this.writer).addResponseListener((DDAgentResponseListener) this.sampler); } log.info("New instance: {}", this); diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/sampling/RateByServiceSampler.java b/dd-trace-ot/src/main/java/datadog/trace/common/sampling/RateByServiceSampler.java index d96c2f3592..b8191b1a8f 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/sampling/RateByServiceSampler.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/sampling/RateByServiceSampler.java @@ -7,7 +7,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.NumericNode; import datadog.opentracing.DDSpan; import datadog.trace.api.sampling.PrioritySampling; -import datadog.trace.common.writer.ddagent.DDAgentApi.ResponseListener; +import datadog.trace.common.writer.ddagent.DDAgentResponseListener; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -19,7 +19,7 @@ import lombok.extern.slf4j.Slf4j; *

The configuration of (serviceName,env)->rate is configured by the core agent. */ @Slf4j -public class RateByServiceSampler implements Sampler, PrioritySampler, ResponseListener { +public class RateByServiceSampler implements Sampler, PrioritySampler, DDAgentResponseListener { public static final String SAMPLING_AGENT_RATE = "_dd.agent_psr"; /** Key for setting the default/baseline rate */ diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java index 76ea6b8df9..c63fe9ff8f 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java @@ -8,6 +8,7 @@ import static java.util.concurrent.TimeUnit.SECONDS; import datadog.opentracing.DDSpan; import datadog.trace.common.util.DaemonThreadFactory; import datadog.trace.common.writer.ddagent.DDAgentApi; +import datadog.trace.common.writer.ddagent.DDAgentResponseListener; import datadog.trace.common.writer.ddagent.Monitor; import datadog.trace.common.writer.ddagent.TraceConsumer; import datadog.trace.common.writer.ddagent.TraceSerializingDisruptor; @@ -110,6 +111,10 @@ public class DDAgentWriter implements Writer { apiPhaser.register(); // Register on behalf of the scheduled executor thread. } + public void addResponseListener(final DDAgentResponseListener listener) { + api.addResponseListener(listener); + } + // Exposing some statistics for consumption by monitors public final long getDisruptorCapacity() { return disruptor.getDisruptorCapacity(); diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java index cc01adc62c..17a607c102 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DDAgentApi.java @@ -43,7 +43,7 @@ public class DDAgentApi { private static final String TRACES_ENDPOINT_V4 = "v0.4/traces"; private static final long MILLISECONDS_BETWEEN_ERROR_LOG = TimeUnit.MINUTES.toMillis(5); - private final List responseListeners = new ArrayList<>(); + private final List responseListeners = new ArrayList<>(); private volatile long nextAllowedLogTime = 0; @@ -76,7 +76,7 @@ public class DDAgentApi { } } - public void addResponseListener(final ResponseListener listener) { + public void addResponseListener(final DDAgentResponseListener listener) { if (!responseListeners.contains(listener)) { responseListeners.add(listener); } @@ -186,7 +186,7 @@ public class DDAgentApi { final JsonNode parsedResponse = OBJECT_MAPPER.readTree(responseString); final String endpoint = tracesUrl.toString(); - for (final ResponseListener listener : responseListeners) { + for (final DDAgentResponseListener listener : responseListeners) { listener.onResponse(endpoint, parsedResponse); } return Response.success(response.code(), parsedResponse); @@ -364,9 +364,4 @@ public class DDAgentApi { return exception; } } - - public interface ResponseListener { - /** Invoked after the api receives a response from the core agent. */ - void onResponse(String endpoint, JsonNode responseJson); - } } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DDAgentResponseListener.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DDAgentResponseListener.java new file mode 100644 index 0000000000..a0cbc72fda --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ddagent/DDAgentResponseListener.java @@ -0,0 +1,8 @@ +package datadog.trace.common.writer.ddagent; + +import com.fasterxml.jackson.databind.JsonNode; + +public interface DDAgentResponseListener { + /** Invoked after the api receives a response from the core agent. */ + void onResponse(String endpoint, JsonNode responseJson); +} diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentApiTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentApiTest.groovy index 07bd326685..8d5ed0dcdd 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentApiTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDAgentApiTest.groovy @@ -4,7 +4,7 @@ import com.fasterxml.jackson.core.type.TypeReference import com.fasterxml.jackson.databind.JsonNode import datadog.opentracing.SpanFactory import datadog.trace.common.writer.ddagent.DDAgentApi -import datadog.trace.common.writer.ddagent.DDAgentApi.ResponseListener +import datadog.trace.common.writer.ddagent.DDAgentResponseListener import datadog.trace.util.test.DDSpecification import java.util.concurrent.atomic.AtomicLong @@ -125,7 +125,7 @@ class DDAgentApiTest extends DDSpecification { def "Api ResponseListeners see 200 responses"() { setup: def agentResponse = new AtomicReference(null) - ResponseListener responseListener = { String endpoint, JsonNode responseJson -> + DDAgentResponseListener responseListener = { String endpoint, JsonNode responseJson -> agentResponse.set(responseJson.toString()) } def agent = httpServer { diff --git a/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy b/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy index 1d5ee8297f..00db0a213d 100644 --- a/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy +++ b/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy @@ -6,6 +6,7 @@ import datadog.opentracing.PendingTrace import datadog.trace.api.sampling.PrioritySampling import datadog.trace.common.writer.ListWriter import datadog.trace.common.writer.ddagent.DDAgentApi +import datadog.trace.common.writer.ddagent.DDAgentResponseListener import datadog.trace.util.test.DDSpecification import org.testcontainers.containers.GenericContainer import org.testcontainers.containers.startupcheck.MinimumDurationRunningStartupCheckStrategy @@ -64,7 +65,7 @@ class DDApiIntegrationTest { def endpoint = new AtomicReference(null) def agentResponse = new AtomicReference(null) - DDAgentApi.ResponseListener responseListener = { String receivedEndpoint, JsonNode responseJson -> + DDAgentResponseListener responseListener = { String receivedEndpoint, JsonNode responseJson -> endpoint.set(receivedEndpoint) agentResponse.set(responseJson.toString()) } From a4b0dcbc9fc7e39b47f6b33fdb725ce048dd80c5 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Mon, 23 Dec 2019 19:05:34 +0100 Subject: [PATCH 09/27] [rmi] add instrumentation names to client and server decorators --- .../trace/instrumentation/rmi/client/RmiClientDecorator.java | 2 +- .../trace/instrumentation/rmi/server/RmiServerDecorator.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/client/RmiClientDecorator.java b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/client/RmiClientDecorator.java index e11102c443..ca76daf591 100644 --- a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/client/RmiClientDecorator.java +++ b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/client/RmiClientDecorator.java @@ -8,7 +8,7 @@ public class RmiClientDecorator extends ClientDecorator { @Override protected String[] instrumentationNames() { - return new String[] {"rmi"}; + return new String[] {"rmi", "rmi-client"}; } @Override diff --git a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/server/RmiServerDecorator.java b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/server/RmiServerDecorator.java index 155d64c7b2..a71836f156 100644 --- a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/server/RmiServerDecorator.java +++ b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/server/RmiServerDecorator.java @@ -8,7 +8,7 @@ public class RmiServerDecorator extends ServerDecorator { @Override protected String[] instrumentationNames() { - return new String[] {"rmi"}; + return new String[] {"rmi", "rmi-server"}; } @Override From bbaf7278a6eddab994b29a7b12d5f32081dea147 Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Tue, 24 Dec 2019 11:59:29 +0100 Subject: [PATCH 10/27] [rmi] remove rmi client service name --- .../trace/instrumentation/rmi/client/RmiClientDecorator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/client/RmiClientDecorator.java b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/client/RmiClientDecorator.java index ca76daf591..7af5775948 100644 --- a/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/client/RmiClientDecorator.java +++ b/dd-java-agent/instrumentation/rmi/src/main/java/datadog/trace/instrumentation/rmi/client/RmiClientDecorator.java @@ -23,6 +23,6 @@ public class RmiClientDecorator extends ClientDecorator { @Override protected String service() { - return "rmi"; + return null; } } From 4947dc3bd31c53d45a69e2c9f48f0c58b9a0ce84 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 2 Jan 2020 11:35:21 -0800 Subject: [PATCH 11/27] Add various tags for AWS SDK v1.x This brings it inline with the v2 instrumentation. --- .../aws-java-sdk-1.11.0.gradle | 19 +++ .../aws/v0/AWSClientInstrumentation.java | 12 +- .../aws/v0/AWSHttpClientInstrumentation.java | 18 +-- .../aws/v0/AwsSdkClientDecorator.java | 23 ++- .../aws/v0/OnErrorDecorator.java | 22 +++ .../aws/v0/RequestInstrumentation.java | 144 ++++++++++++++++++ .../instrumentation/aws/v0/RequestMeta.java | 19 +++ .../aws/v0/TracingRequestHandler.java | 27 ++-- .../src/test/groovy/AWSClientTest.groovy | 64 ++++++-- .../groovy/AWSClientTest.groovy | 30 ++-- .../aws/v2/AwsSdkClientDecorator.java | 14 +- 11 files changed, 332 insertions(+), 60 deletions(-) create mode 100644 dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/OnErrorDecorator.java create mode 100644 dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/RequestInstrumentation.java create mode 100644 dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/RequestMeta.java diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/aws-java-sdk-1.11.0.gradle b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/aws-java-sdk-1.11.0.gradle index 9f99706cbd..5d1758d60f 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/aws-java-sdk-1.11.0.gradle +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/aws-java-sdk-1.11.0.gradle @@ -39,9 +39,16 @@ dependencies { // Include httpclient instrumentation for testing because it is a dependency for aws-sdk. testCompile project(':dd-java-agent:instrumentation:apache-httpclient-4') + testCompile group: 'com.amazonaws', name: 'aws-java-sdk-s3', version: '1.11.106' testCompile group: 'com.amazonaws', name: 'aws-java-sdk-rds', version: '1.11.106' testCompile group: 'com.amazonaws', name: 'aws-java-sdk-ec2', version: '1.11.106' + testCompile group: 'com.amazonaws', name: 'aws-java-sdk-kinesis', version: '1.11.106' + testCompile group: 'com.amazonaws', name: 'aws-java-sdk-sqs', version: '1.11.106' + testCompile group: 'com.amazonaws', name: 'aws-java-sdk-dynamodb', version: '1.11.106' + + // needed for kinesis: + testCompile group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-cbor', version: versions.jackson test_before_1_11_106Compile(group: 'com.amazonaws', name: 'aws-java-sdk-s3', version: '1.11.0') { force = true @@ -52,10 +59,22 @@ dependencies { test_before_1_11_106Compile(group: 'com.amazonaws', name: 'aws-java-sdk-ec2', version: '1.11.0') { force = true } + test_before_1_11_106Compile(group: 'com.amazonaws', name: 'aws-java-sdk-kinesis', version: '1.11.0') { + force = true + } + test_before_1_11_106Compile(group: 'com.amazonaws', name: 'aws-java-sdk-sqs', version: '1.11.0') { + force = true + } + test_before_1_11_106Compile(group: 'com.amazonaws', name: 'aws-java-sdk-dynamodb', version: '1.11.0') { + force = true + } latestDepTestCompile group: 'com.amazonaws', name: 'aws-java-sdk-s3', version: '+' latestDepTestCompile group: 'com.amazonaws', name: 'aws-java-sdk-rds', version: '+' latestDepTestCompile group: 'com.amazonaws', name: 'aws-java-sdk-ec2', version: '+' + latestDepTestCompile group: 'com.amazonaws', name: 'aws-java-sdk-kinesis', version: '+' + latestDepTestCompile group: 'com.amazonaws', name: 'aws-java-sdk-sqs', version: '+' + latestDepTestCompile group: 'com.amazonaws', name: 'aws-java-sdk-dynamodb', version: '+' } test.dependsOn test_before_1_11_106 diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSClientInstrumentation.java b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSClientInstrumentation.java index 149737ad04..a23c1f0966 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSClientInstrumentation.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSClientInstrumentation.java @@ -5,9 +5,11 @@ import static net.bytebuddy.matcher.ElementMatchers.declaresField; import static net.bytebuddy.matcher.ElementMatchers.isConstructor; import static net.bytebuddy.matcher.ElementMatchers.named; +import com.amazonaws.AmazonWebServiceRequest; import com.amazonaws.handlers.RequestHandler2; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.InstrumentationContext; import java.util.List; import java.util.Map; import net.bytebuddy.asm.Advice; @@ -39,6 +41,7 @@ public final class AWSClientInstrumentation extends Instrumenter.Default { "datadog.trace.agent.decorator.ClientDecorator", "datadog.trace.agent.decorator.HttpClientDecorator", packageName + ".AwsSdkClientDecorator", + packageName + ".RequestMeta", packageName + ".TracingRequestHandler", }; } @@ -49,6 +52,11 @@ public final class AWSClientInstrumentation extends Instrumenter.Default { isConstructor(), AWSClientInstrumentation.class.getName() + "$AWSClientAdvice"); } + @Override + public Map contextStore() { + return singletonMap("com.amazonaws.AmazonWebServiceRequest", packageName + ".RequestMeta"); + } + public static class AWSClientAdvice { // Since we're instrumenting the constructor, we can't add onThrowable. @Advice.OnMethodExit(suppress = Throwable.class) @@ -62,7 +70,9 @@ public final class AWSClientInstrumentation extends Instrumenter.Default { } } if (!hasDDHandler) { - handlers.add(TracingRequestHandler.INSTANCE); + handlers.add( + new TracingRequestHandler( + InstrumentationContext.get(AmazonWebServiceRequest.class, RequestMeta.class))); } } } diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSHttpClientInstrumentation.java b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSHttpClientInstrumentation.java index da23fafaf7..04ff1dfdef 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSHttpClientInstrumentation.java @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.aws.v0; -import static datadog.trace.instrumentation.aws.v0.AwsSdkClientDecorator.DECORATE; +import static datadog.trace.instrumentation.aws.v0.OnErrorDecorator.DECORATE; +import static datadog.trace.instrumentation.aws.v0.RequestMeta.SCOPE_CONTEXT_KEY; import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isAbstract; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -40,10 +41,8 @@ public class AWSHttpClientInstrumentation extends Instrumenter.Default { public String[] helperClassNames() { return new String[] { "datadog.trace.agent.decorator.BaseDecorator", - "datadog.trace.agent.decorator.ClientDecorator", - "datadog.trace.agent.decorator.HttpClientDecorator", - packageName + ".AwsSdkClientDecorator", - packageName + ".TracingRequestHandler", + packageName + ".OnErrorDecorator", + packageName + ".RequestMeta", }; } @@ -60,9 +59,9 @@ public class AWSHttpClientInstrumentation extends Instrumenter.Default { @Advice.Argument(value = 0, optional = true) final Request request, @Advice.Thrown final Throwable throwable) { if (throwable != null) { - final AgentScope scope = request.getHandlerContext(TracingRequestHandler.SCOPE_CONTEXT_KEY); + final AgentScope scope = request.getHandlerContext(SCOPE_CONTEXT_KEY); if (scope != null) { - request.addHandlerContext(TracingRequestHandler.SCOPE_CONTEXT_KEY, null); + request.addHandlerContext(SCOPE_CONTEXT_KEY, null); DECORATE.onError(scope.span(), throwable); DECORATE.beforeFinish(scope.span()); scope.close(); @@ -96,10 +95,9 @@ public class AWSHttpClientInstrumentation extends Instrumenter.Default { @Advice.FieldValue("request") final Request request, @Advice.Thrown final Throwable throwable) { if (throwable != null) { - final AgentScope scope = - request.getHandlerContext(TracingRequestHandler.SCOPE_CONTEXT_KEY); + final AgentScope scope = request.getHandlerContext(SCOPE_CONTEXT_KEY); if (scope != null) { - request.addHandlerContext(TracingRequestHandler.SCOPE_CONTEXT_KEY, null); + request.addHandlerContext(SCOPE_CONTEXT_KEY, null); DECORATE.onError(scope.span(), throwable); DECORATE.beforeFinish(scope.span()); scope.close(); diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AwsSdkClientDecorator.java b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AwsSdkClientDecorator.java index 8f743eb49e..a845cfee6a 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AwsSdkClientDecorator.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AwsSdkClientDecorator.java @@ -1,10 +1,12 @@ package datadog.trace.instrumentation.aws.v0; +import com.amazonaws.AmazonWebServiceRequest; import com.amazonaws.AmazonWebServiceResponse; import com.amazonaws.Request; import com.amazonaws.Response; import datadog.trace.agent.decorator.HttpClientDecorator; import datadog.trace.api.DDTags; +import datadog.trace.bootstrap.ContextStore; import datadog.trace.instrumentation.api.AgentSpan; import java.net.URI; import java.net.URISyntaxException; @@ -12,12 +14,17 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; public class AwsSdkClientDecorator extends HttpClientDecorator { - public static final AwsSdkClientDecorator DECORATE = new AwsSdkClientDecorator(); static final String COMPONENT_NAME = "java-aws-sdk"; private final Map serviceNames = new ConcurrentHashMap<>(); private final Map operationNames = new ConcurrentHashMap<>(); + private final ContextStore contextStore; + + public AwsSdkClientDecorator( + final ContextStore contextStore) { + this.contextStore = contextStore; + } @Override public AgentSpan onRequest(final AgentSpan span, final Request request) { @@ -25,7 +32,8 @@ public class AwsSdkClientDecorator extends HttpClientDecorator awsOperation = request.getOriginalRequest().getClass(); + final AmazonWebServiceRequest originalRequest = request.getOriginalRequest(); + final Class awsOperation = originalRequest.getClass(); span.setTag("aws.agent", COMPONENT_NAME); span.setTag("aws.service", awsServiceName); @@ -36,6 +44,17 @@ public class AwsSdkClientDecorator extends HttpClientDecorator typeMatcher() { + return safeHasSuperType(named("com.amazonaws.AmazonWebServiceRequest")); + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".RequestMeta", + }; + } + + @Override + public Map, String> transformers() { + final Map, String> transformers = new HashMap<>(); + transformers.put( + named("setBucketName").and(takesArgument(0, String.class)), + RequestInstrumentation.class.getName() + "$BucketNameAdvice"); + transformers.put( + named("setQueueUrl").and(takesArgument(0, String.class)), + RequestInstrumentation.class.getName() + "$QueueUrlAdvice"); + transformers.put( + named("setQueueName").and(takesArgument(0, String.class)), + RequestInstrumentation.class.getName() + "$QueueNameAdvice"); + transformers.put( + named("setStreamName").and(takesArgument(0, String.class)), + RequestInstrumentation.class.getName() + "$StreamNameAdvice"); + transformers.put( + named("setTableName").and(takesArgument(0, String.class)), + RequestInstrumentation.class.getName() + "$TableNameAdvice"); + return transformers; + } + + @Override + public Map contextStore() { + return singletonMap("com.amazonaws.AmazonWebServiceRequest", packageName + ".RequestMeta"); + } + + public static class BucketNameAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void methodEnter( + @Advice.Argument(0) final String value, + @Advice.This final AmazonWebServiceRequest request) { + final ContextStore contextStore = + InstrumentationContext.get(AmazonWebServiceRequest.class, RequestMeta.class); + RequestMeta requestMeta = contextStore.get(request); + if (requestMeta == null) { + requestMeta = new RequestMeta(); + contextStore.put(request, requestMeta); + } + requestMeta.setBucketName(value); + } + } + + public static class QueueUrlAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void methodEnter( + @Advice.Argument(0) final String value, + @Advice.This final AmazonWebServiceRequest request) { + final ContextStore contextStore = + InstrumentationContext.get(AmazonWebServiceRequest.class, RequestMeta.class); + RequestMeta requestMeta = contextStore.get(request); + if (requestMeta == null) { + requestMeta = new RequestMeta(); + contextStore.put(request, requestMeta); + } + requestMeta.setQueueUrl(value); + } + } + + public static class QueueNameAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void methodEnter( + @Advice.Argument(0) final String value, + @Advice.This final AmazonWebServiceRequest request) { + final ContextStore contextStore = + InstrumentationContext.get(AmazonWebServiceRequest.class, RequestMeta.class); + RequestMeta requestMeta = contextStore.get(request); + if (requestMeta == null) { + requestMeta = new RequestMeta(); + contextStore.put(request, requestMeta); + } + requestMeta.setQueueName(value); + } + } + + public static class StreamNameAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void methodEnter( + @Advice.Argument(0) final String value, + @Advice.This final AmazonWebServiceRequest request) { + final ContextStore contextStore = + InstrumentationContext.get(AmazonWebServiceRequest.class, RequestMeta.class); + RequestMeta requestMeta = contextStore.get(request); + if (requestMeta == null) { + requestMeta = new RequestMeta(); + contextStore.put(request, requestMeta); + } + requestMeta.setStreamName(value); + } + } + + public static class TableNameAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void methodEnter( + @Advice.Argument(0) final String value, + @Advice.This final AmazonWebServiceRequest request) { + final ContextStore contextStore = + InstrumentationContext.get(AmazonWebServiceRequest.class, RequestMeta.class); + RequestMeta requestMeta = contextStore.get(request); + if (requestMeta == null) { + requestMeta = new RequestMeta(); + contextStore.put(request, requestMeta); + } + requestMeta.setTableName(value); + } + } +} diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/RequestMeta.java b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/RequestMeta.java new file mode 100644 index 0000000000..ef807e6240 --- /dev/null +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/RequestMeta.java @@ -0,0 +1,19 @@ +package datadog.trace.instrumentation.aws.v0; + +import com.amazonaws.handlers.HandlerContextKey; +import datadog.trace.instrumentation.api.AgentScope; +import lombok.Data; + +@Data +public class RequestMeta { + // Note: aws1.x sdk doesn't have any truly async clients so we can store scope in request context + // safely. + public static final HandlerContextKey SCOPE_CONTEXT_KEY = + new HandlerContextKey<>("DatadogScope"); + + private String bucketName; + private String queueUrl; + private String queueName; + private String streamName; + private String tableName; +} diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java index 204e64810d..4639eb60e7 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java @@ -2,24 +2,25 @@ package datadog.trace.instrumentation.aws.v0; import static datadog.trace.instrumentation.api.AgentTracer.activateSpan; import static datadog.trace.instrumentation.api.AgentTracer.startSpan; -import static datadog.trace.instrumentation.aws.v0.AwsSdkClientDecorator.DECORATE; +import static datadog.trace.instrumentation.aws.v0.RequestMeta.SCOPE_CONTEXT_KEY; import com.amazonaws.AmazonWebServiceRequest; import com.amazonaws.Request; import com.amazonaws.Response; -import com.amazonaws.handlers.HandlerContextKey; import com.amazonaws.handlers.RequestHandler2; +import datadog.trace.bootstrap.ContextStore; import datadog.trace.instrumentation.api.AgentScope; import datadog.trace.instrumentation.api.AgentSpan; /** Tracing Request Handler */ public class TracingRequestHandler extends RequestHandler2 { - public static TracingRequestHandler INSTANCE = new TracingRequestHandler(); - // Note: aws1.x sdk doesn't have any truly async clients so we can store scope in request context - // safely. - public static final HandlerContextKey SCOPE_CONTEXT_KEY = - new HandlerContextKey<>("DatadogScope"); + private final AwsSdkClientDecorator decorate; + + public TracingRequestHandler( + final ContextStore contextStore) { + decorate = new AwsSdkClientDecorator(contextStore); + } @Override public AmazonWebServiceRequest beforeMarshalling(final AmazonWebServiceRequest request) { @@ -29,8 +30,8 @@ public class TracingRequestHandler extends RequestHandler2 { @Override public void beforeRequest(final Request request) { final AgentSpan span = startSpan("aws.command"); - DECORATE.afterStart(span); - DECORATE.onRequest(span, request); + decorate.afterStart(span); + decorate.onRequest(span, request); request.addHandlerContext(SCOPE_CONTEXT_KEY, activateSpan(span, true)); } @@ -39,8 +40,8 @@ public class TracingRequestHandler extends RequestHandler2 { final AgentScope scope = request.getHandlerContext(SCOPE_CONTEXT_KEY); if (scope != null) { request.addHandlerContext(SCOPE_CONTEXT_KEY, null); - DECORATE.onResponse(scope.span(), response); - DECORATE.beforeFinish(scope.span()); + decorate.onResponse(scope.span(), response); + decorate.beforeFinish(scope.span()); scope.close(); } } @@ -50,8 +51,8 @@ public class TracingRequestHandler extends RequestHandler2 { final AgentScope scope = request.getHandlerContext(SCOPE_CONTEXT_KEY); if (scope != null) { request.addHandlerContext(SCOPE_CONTEXT_KEY, null); - DECORATE.onError(scope.span(), e); - DECORATE.beforeFinish(scope.span()); + decorate.onError(scope.span(), e); + decorate.beforeFinish(scope.span()); scope.close(); } } diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy index af10ce4e00..1e526774ab 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy @@ -16,11 +16,18 @@ import com.amazonaws.client.builder.AwsClientBuilder import com.amazonaws.handlers.RequestHandler2 import com.amazonaws.regions.Regions import com.amazonaws.retry.PredefinedRetryPolicies +import com.amazonaws.services.dynamodbv2.AmazonDynamoDBClientBuilder +import com.amazonaws.services.dynamodbv2.model.CreateTableRequest import com.amazonaws.services.ec2.AmazonEC2ClientBuilder +import com.amazonaws.services.kinesis.AmazonKinesisClientBuilder +import com.amazonaws.services.kinesis.model.DeleteStreamRequest import com.amazonaws.services.rds.AmazonRDSClientBuilder import com.amazonaws.services.rds.model.DeleteOptionGroupRequest import com.amazonaws.services.s3.AmazonS3Client import com.amazonaws.services.s3.AmazonS3ClientBuilder +import com.amazonaws.services.sqs.AmazonSQSClientBuilder +import com.amazonaws.services.sqs.model.CreateQueueRequest +import com.amazonaws.services.sqs.model.SendMessageRequest import datadog.trace.agent.test.AgentTestRunner import datadog.trace.api.DDSpanTypes import datadog.trace.instrumentation.api.Tags @@ -143,12 +150,15 @@ class AWSClientTest extends AgentTestRunner { "aws.endpoint" "$server.address" "aws.operation" "${operation}Request" "aws.agent" "java-aws-sdk" + for (def addedTag : additionalTags) { + "$addedTag.key" "$addedTag.value" + } defaultTags() } } span(1) { operationName "http.request" - resourceName "$method /$url" + resourceName "$method $path" spanType DDSpanTypes.HTTP_CLIENT errored false childOf(span(0)) @@ -157,7 +167,7 @@ class AWSClientTest extends AgentTestRunner { "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT "$Tags.PEER_HOSTNAME" "localhost" "$Tags.PEER_PORT" server.address.port - "$Tags.HTTP_URL" "$server.address/$url" + "$Tags.HTTP_URL" "${server.address}${path}" "$Tags.HTTP_METHOD" "$method" "$Tags.HTTP_STATUS" 200 defaultTags() @@ -169,23 +179,41 @@ class AWSClientTest extends AgentTestRunner { server.lastRequest.headers.get("x-datadog-parent-id") == null where: - service | operation | method | url | handlerCount | call | body | client - "S3" | "CreateBucket" | "PUT" | "testbucket/" | 1 | { client -> client.createBucket("testbucket") } | "" | AmazonS3ClientBuilder.standard().withPathStyleAccessEnabled(true).withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() - "S3" | "GetObject" | "GET" | "someBucket/someKey" | 1 | { client -> client.getObject("someBucket", "someKey") } | "" | AmazonS3ClientBuilder.standard().withPathStyleAccessEnabled(true).withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() - "EC2" | "AllocateAddress" | "POST" | "" | 4 | { client -> client.allocateAddress() } | """ - - 59dbff89-35bd-4eac-99ed-be587EXAMPLE - 192.0.2.1 - standard - - """ | AmazonEC2ClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() - "RDS" | "DeleteOptionGroup" | "POST" | "" | 5 | { client -> client.deleteOptionGroup(new DeleteOptionGroupRequest()) } | """ + service | operation | method | path | handlerCount | client | call | additionalTags | body + "S3" | "CreateBucket" | "PUT" | "/testbucket/" | 1 | AmazonS3ClientBuilder.standard().withPathStyleAccessEnabled(true).withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { client -> client.createBucket("testbucket") } | ["aws.bucket.name": "testbucket"] | "" + "S3" | "GetObject" | "GET" | "/someBucket/someKey" | 1 | AmazonS3ClientBuilder.standard().withPathStyleAccessEnabled(true).withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { client -> client.getObject("someBucket", "someKey") } | ["aws.bucket.name": "someBucket"] | "" + "DynamoDBv2" | "CreateTable" | "POST" | "/" | 1 | AmazonDynamoDBClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { c -> c.createTable(new CreateTableRequest("sometable", null)) } | ["aws.table.name": "sometable"] | "" + "Kinesis" | "DeleteStream" | "POST" | "/" | 1 | AmazonKinesisClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { c -> c.deleteStream(new DeleteStreamRequest().withStreamName("somestream")) } | ["aws.stream.name": "somestream"] | "" + "SQS" | "CreateQueue" | "POST" | "/" | 4 | AmazonSQSClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { c -> c.createQueue(new CreateQueueRequest("somequeue")) } | ["aws.queue.name": "somequeue"] | """ + + https://queue.amazonaws.com/123456789012/MyQueue + 7a62c49f-347e-4fc4-9331-6e8e7a96aa73 + + """ + "SQS" | "SendMessage" | "POST" | "/someurl" | 4 | AmazonSQSClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { c -> c.sendMessage(new SendMessageRequest("someurl", "")) } | ["aws.queue.url": "someurl"] | """ + + + d41d8cd98f00b204e9800998ecf8427e + 3ae8f24a165a8cedc005670c81a27295 + 5fea7756-0ea4-451a-a703-a558b933e274 + + 27daac76-34dd-47df-bd01-1f6e873584a0 + + """ + "EC2" | "AllocateAddress" | "POST" | "/" | 4 | AmazonEC2ClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { client -> client.allocateAddress() } | [:] | """ + + 59dbff89-35bd-4eac-99ed-be587EXAMPLE + 192.0.2.1 + standard + + """ + "RDS" | "DeleteOptionGroup" | "POST" | "/" | 5 | AmazonRDSClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { client -> client.deleteOptionGroup(new DeleteOptionGroupRequest()) } | [:] | """ 0ac9cda2-bbf4-11d3-f92b-31fa5e8dbc99 - """ | AmazonRDSClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() + """ } def "send #operation request to closed port"() { @@ -216,6 +244,9 @@ class AWSClientTest extends AgentTestRunner { "aws.endpoint" "http://localhost:${UNUSABLE_PORT}" "aws.operation" "${operation}Request" "aws.agent" "java-aws-sdk" + for (def addedTag : additionalTags) { + "$addedTag.key" "$addedTag.value" + } errorTags SdkClientException, ~/Unable to execute HTTP request/ defaultTags() } @@ -241,8 +272,8 @@ class AWSClientTest extends AgentTestRunner { } where: - service | operation | method | url | call | body | client - "S3" | "GetObject" | "GET" | "someBucket/someKey" | { client -> client.getObject("someBucket", "someKey") } | "" | new AmazonS3Client(CREDENTIALS_PROVIDER_CHAIN, new ClientConfiguration().withRetryPolicy(PredefinedRetryPolicies.getDefaultRetryPolicyWithCustomMaxRetries(0))).withEndpoint("http://localhost:${UNUSABLE_PORT}") + service | operation | method | url | call | additionalTags | body | client + "S3" | "GetObject" | "GET" | "someBucket/someKey" | { client -> client.getObject("someBucket", "someKey") } | ["aws.bucket.name": "someBucket"] | "" | new AmazonS3Client(CREDENTIALS_PROVIDER_CHAIN, new ClientConfiguration().withRetryPolicy(PredefinedRetryPolicies.getDefaultRetryPolicyWithCustomMaxRetries(0))).withEndpoint("http://localhost:${UNUSABLE_PORT}") } def "naughty request handler doesn't break the trace"() { @@ -325,6 +356,7 @@ class AWSClientTest extends AgentTestRunner { "aws.endpoint" "$server.address" "aws.operation" "GetObjectRequest" "aws.agent" "java-aws-sdk" + "aws.bucket.name" "someBucket" try { errorTags AmazonClientException, ~/Unable to execute HTTP request/ } catch (AssertionError e) { diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_before_1_11_106/groovy/AWSClientTest.groovy b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_before_1_11_106/groovy/AWSClientTest.groovy index e466674178..c8278c321b 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_before_1_11_106/groovy/AWSClientTest.groovy +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_before_1_11_106/groovy/AWSClientTest.groovy @@ -113,12 +113,15 @@ class AWSClientTest extends AgentTestRunner { "aws.endpoint" "$server.address" "aws.operation" "${operation}Request" "aws.agent" "java-aws-sdk" + for (def addedTag : additionalTags) { + "$addedTag.key" "$addedTag.value" + } defaultTags() } } span(1) { operationName "http.request" - resourceName "$method /$url" + resourceName "$method $path" spanType DDSpanTypes.HTTP_CLIENT errored false childOf(span(0)) @@ -127,7 +130,7 @@ class AWSClientTest extends AgentTestRunner { "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT "$Tags.PEER_HOSTNAME" "localhost" "$Tags.PEER_PORT" server.address.port - "$Tags.HTTP_URL" "$server.address/$url" + "$Tags.HTTP_URL" "${server.address}${path}" "$Tags.HTTP_METHOD" "$method" "$Tags.HTTP_STATUS" 200 defaultTags() @@ -139,23 +142,23 @@ class AWSClientTest extends AgentTestRunner { server.lastRequest.headers.get("x-datadog-parent-id") == null where: - service | operation | method | url | handlerCount | call | body | client - "S3" | "CreateBucket" | "PUT" | "testbucket/" | 1 | { client -> client.setS3ClientOptions(S3ClientOptions.builder().setPathStyleAccess(true).build()); client.createBucket("testbucket") } | "" | new AmazonS3Client().withEndpoint("http://localhost:$server.address.port") - "S3" | "GetObject" | "GET" | "someBucket/someKey" | 1 | { client -> client.getObject("someBucket", "someKey") } | "" | new AmazonS3Client().withEndpoint("http://localhost:$server.address.port") - "EC2" | "AllocateAddress" | "POST" | "" | 4 | { client -> client.allocateAddress() } | """ + service | operation | method | path | handlerCount | client | additionalTags | call | body + "S3" | "CreateBucket" | "PUT" | "/testbucket/" | 1 | new AmazonS3Client().withEndpoint("http://localhost:$server.address.port") | ["aws.bucket.name": "testbucket"] | { client -> client.setS3ClientOptions(S3ClientOptions.builder().setPathStyleAccess(true).build()); client.createBucket("testbucket") } | "" + "S3" | "GetObject" | "GET" | "/someBucket/someKey" | 1 | new AmazonS3Client().withEndpoint("http://localhost:$server.address.port") | ["aws.bucket.name": "someBucket"] | { client -> client.getObject("someBucket", "someKey") } | "" + "EC2" | "AllocateAddress" | "POST" | "/" | 4 | new AmazonEC2Client().withEndpoint("http://localhost:$server.address.port") | [:] | { client -> client.allocateAddress() } | """ 59dbff89-35bd-4eac-99ed-be587EXAMPLE 192.0.2.1 standard - """ | new AmazonEC2Client().withEndpoint("http://localhost:$server.address.port") - "RDS" | "DeleteOptionGroup" | "POST" | "" | 1 | { client -> client.deleteOptionGroup(new DeleteOptionGroupRequest()) } | """ + """ + "RDS" | "DeleteOptionGroup" | "POST" | "/" | 1 | new AmazonRDSClient().withEndpoint("http://localhost:$server.address.port") | [:] | { client -> client.deleteOptionGroup(new DeleteOptionGroupRequest()) } | """ 0ac9cda2-bbf4-11d3-f92b-31fa5e8dbc99 - """ | new AmazonRDSClient().withEndpoint("http://localhost:$server.address.port") + """ } def "send #operation request to closed port"() { @@ -186,6 +189,9 @@ class AWSClientTest extends AgentTestRunner { "aws.endpoint" "http://localhost:${UNUSABLE_PORT}" "aws.operation" "${operation}Request" "aws.agent" "java-aws-sdk" + for (def addedTag : additionalTags) { + "$addedTag.key" "$addedTag.value" + } errorTags AmazonClientException, ~/Unable to execute HTTP request/ defaultTags() } @@ -211,8 +217,8 @@ class AWSClientTest extends AgentTestRunner { } where: - service | operation | method | url | call | body | client - "S3" | "GetObject" | "GET" | "someBucket/someKey" | { client -> client.getObject("someBucket", "someKey") } | "" | new AmazonS3Client(CREDENTIALS_PROVIDER_CHAIN, new ClientConfiguration().withRetryPolicy(PredefinedRetryPolicies.getDefaultRetryPolicyWithCustomMaxRetries(0))).withEndpoint("http://localhost:${UNUSABLE_PORT}") + service | operation | method | url | call | additionalTags | body | client + "S3" | "GetObject" | "GET" | "someBucket/someKey" | { client -> client.getObject("someBucket", "someKey") } | ["aws.bucket.name": "someBucket"] | "" | new AmazonS3Client(CREDENTIALS_PROVIDER_CHAIN, new ClientConfiguration().withRetryPolicy(PredefinedRetryPolicies.getDefaultRetryPolicyWithCustomMaxRetries(0))).withEndpoint("http://localhost:${UNUSABLE_PORT}") } def "naughty request handler doesn't break the trace"() { @@ -249,6 +255,7 @@ class AWSClientTest extends AgentTestRunner { "aws.endpoint" "https://s3.amazonaws.com" "aws.operation" "GetObjectRequest" "aws.agent" "java-aws-sdk" + "aws.bucket.name" "someBucket" errorTags RuntimeException, "bad handler" defaultTags() } @@ -295,6 +302,7 @@ class AWSClientTest extends AgentTestRunner { "aws.endpoint" "http://localhost:$server.address.port" "aws.operation" "GetObjectRequest" "aws.agent" "java-aws-sdk" + "aws.bucket.name" "someBucket" errorTags AmazonClientException, ~/Unable to execute HTTP request/ defaultTags() } diff --git a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java8/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecorator.java b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java8/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecorator.java index 57cec8695c..42cee63615 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java8/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecorator.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java8/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecorator.java @@ -22,21 +22,21 @@ public class AwsSdkClientDecorator extends HttpClientDecorator span.setTag("aws.bucket.name", name)); - // DynamoDB - request - .getValueForField("TableName", String.class) - .ifPresent(name -> span.setTag("aws.table.name", name)); // SQS - request - .getValueForField("QueueName", String.class) - .ifPresent(name -> span.setTag("aws.queue.name", name)); request .getValueForField("QueueUrl", String.class) .ifPresent(name -> span.setTag("aws.queue.url", name)); + request + .getValueForField("QueueName", String.class) + .ifPresent(name -> span.setTag("aws.queue.name", name)); // Kinesis request .getValueForField("StreamName", String.class) .ifPresent(name -> span.setTag("aws.stream.name", name)); + // DynamoDB + request + .getValueForField("TableName", String.class) + .ifPresent(name -> span.setTag("aws.table.name", name)); return span; } From 5e8af8439a9806cf23cde583f89e80530539d50f Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 7 Jan 2020 17:10:01 -0800 Subject: [PATCH 12/27] Optimize HasSuperMethodMatcher logic Instead of filtering then iterating, just iterate through everything and apply filter inline. This will help avoid allocation for filter iterator and improve if early match is found. --- .../trace/agent/tooling/ByteBuddyElementMatchers.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java index 6dbaa8e594..8b23e03422 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java @@ -328,9 +328,8 @@ public class ByteBuddyElementMatchers { final Set checkedInterfaces = new HashSet<>(); while (declaringType != null) { - for (final MethodDescription methodDescription : - declaringType.getDeclaredMethods().filter(signatureMatcher)) { - if (matcher.matches(methodDescription)) { + for (final MethodDescription methodDescription : declaringType.getDeclaredMethods()) { + if (signatureMatcher.matches(methodDescription) && matcher.matches(methodDescription)) { return true; } } From 7cb24f35c04e2b63c4bad31e57ba46770cbe8fd6 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 8 Jan 2020 13:45:36 -0800 Subject: [PATCH 13/27] Better error handling for getClientInfo in JDBCDecorator An exception there caused a span to be created but then never finished. --- .../instrumentation/jdbc/JDBCDecorator.java | 7 +- .../trace/instrumentation/jdbc/JDBCUtils.java | 17 +- .../groovy/JDBCInstrumentationTest.groovy | 51 +- .../TestConnection.groovy} | 21 +- .../groovy/test/TestDatabaseMetaData.groovy | 889 ++++++++++++++++++ .../src/test/groovy/test/TestDriver.groovy | 45 + .../src/test/groovy/test/TestStatement.groovy | 235 +++++ .../JSPInstrumentationBasicTests.groovy | 45 + 8 files changed, 1292 insertions(+), 18 deletions(-) rename dd-java-agent/instrumentation/jdbc/src/test/groovy/{DummyThrowingConnection.groovy => test/TestConnection.groovy} (91%) create mode 100644 dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDatabaseMetaData.groovy create mode 100644 dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDriver.groovy create mode 100644 dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestStatement.groovy diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index 2cb4595bfe..f2ab9271c1 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -69,7 +69,12 @@ public class JDBCDecorator extends DatabaseClientDecorator { final DatabaseMetaData metaData = connection.getMetaData(); final String url = metaData.getURL(); if (url != null) { - dbInfo = JDBCConnectionUrlParser.parse(url, connection.getClientInfo()); + try { + dbInfo = JDBCConnectionUrlParser.parse(url, connection.getClientInfo()); + } catch (final Exception ex) { + // getClientInfo is likely not allowed. + dbInfo = JDBCConnectionUrlParser.parse(url, null); + } } else { dbInfo = DBInfo.DEFAULT; } diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCUtils.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCUtils.java index a7883679cc..9f2624c9d3 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCUtils.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCUtils.java @@ -30,13 +30,16 @@ public abstract class JDBCUtils { connection = connection.unwrap(Connection.class); } } catch (final Exception | AbstractMethodError e) { - // Attempt to work around c3po delegating to an connection that doesn't support unwrapping. - final Class connectionClass = connection.getClass(); - if (connectionClass.getName().equals("com.mchange.v2.c3p0.impl.NewProxyConnection")) { - final Field inner = connectionClass.getDeclaredField("inner"); - inner.setAccessible(true); - c3poField = inner; - return (Connection) c3poField.get(connection); + if (connection != null) { + // Attempt to work around c3po delegating to an connection that doesn't support + // unwrapping. + final Class connectionClass = connection.getClass(); + if (connectionClass.getName().equals("com.mchange.v2.c3p0.impl.NewProxyConnection")) { + final Field inner = connectionClass.getDeclaredField("inner"); + inner.setAccessible(true); + c3poField = inner; + return (Connection) c3poField.get(connection); + } } // perhaps wrapping isn't supported? diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy index 974c5c3784..54b5bee265 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy @@ -10,6 +10,8 @@ import org.h2.Driver import org.hsqldb.jdbc.JDBCDriver import spock.lang.Shared import spock.lang.Unroll +import test.TestConnection +import test.TestStatement import javax.sql.DataSource import java.sql.CallableStatement @@ -475,7 +477,6 @@ class JDBCInstrumentationTest extends AgentTestRunner { "derby" | cpDatasources.get("hikari").get("derby").getConnection() | "APP" | "CREATE TABLE PS_DERBY_HIKARI (id INTEGER not NULL, PRIMARY KEY ( id ))" "h2" | cpDatasources.get("c3p0").get("h2").getConnection() | null | "CREATE TABLE PS_H2_C3P0 (id INTEGER not NULL, PRIMARY KEY ( id ))" "derby" | cpDatasources.get("c3p0").get("derby").getConnection() | "APP" | "CREATE TABLE PS_DERBY_C3P0 (id INTEGER not NULL, PRIMARY KEY ( id ))" - } @Unroll @@ -485,7 +486,7 @@ class JDBCInstrumentationTest extends AgentTestRunner { when: try { - connection = new DummyThrowingConnection() + connection = new TestConnection(true) } catch (Exception e) { connection = driverClass.connect(url, null) } @@ -549,6 +550,52 @@ class JDBCInstrumentationTest extends AgentTestRunner { false | "derby" | new EmbeddedDriver() | "jdbc:derby:memory:" + dbName + ";create=true" | "APP" | "SELECT 3 FROM SYSIBM.SYSDUMMY1" } + def "test getClientInfo exception"() { + setup: + Connection connection = new TestConnection(false) + + when: + Statement statement = null + runUnderTrace("parent") { + statement = connection.createStatement() + return statement.executeQuery(query) + } + + then: + assertTraces(1) { + trace(0, 2) { + basicSpan(it, 0, "parent") + span(1) { + operationName "${database}.query" + serviceName database + resourceName query + spanType DDSpanTypes.SQL + childOf span(0) + errored false + tags { + "$Tags.COMPONENT" "java-jdbc-statement" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.DB_TYPE" database + "span.origin.type" TestStatement.name + defaultTags() + } + } + } + } + + cleanup: + if (statement != null) { + statement.close() + } + if (connection != null) { + connection.close() + } + + where: + database = "testdb" + query = "testing 123" + } + @Unroll def "#connectionPoolName connections should be cached in case of wrapped connections"() { setup: diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/DummyThrowingConnection.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestConnection.groovy similarity index 91% rename from dd-java-agent/instrumentation/jdbc/src/test/groovy/DummyThrowingConnection.groovy rename to dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestConnection.groovy index 018c27c172..a52de68ef3 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/DummyThrowingConnection.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestConnection.groovy @@ -1,3 +1,5 @@ +package test + import java.sql.Array import java.sql.Blob import java.sql.CallableStatement @@ -17,16 +19,19 @@ import java.util.concurrent.Executor /** - * A JDBC connection class that throws an exception in the constructor, used to test + * A JDBC connection class that optionally throws an exception in the constructor, used to test */ -class DummyThrowingConnection implements Connection { - DummyThrowingConnection() { - throw new RuntimeException("Dummy exception") +class TestConnection implements Connection { + TestConnection(boolean throwException) { + if (throwException) { + throw new RuntimeException("connection exception") + } } + @Override Statement createStatement() throws SQLException { - return null + return new TestStatement(this) } @Override @@ -76,7 +81,7 @@ class DummyThrowingConnection implements Connection { @Override DatabaseMetaData getMetaData() throws SQLException { - return null + return new TestDatabaseMetaData() } @Override @@ -241,12 +246,12 @@ class DummyThrowingConnection implements Connection { @Override String getClientInfo(String name) throws SQLException { - return null + throw new UnsupportedOperationException("Test 123") } @Override Properties getClientInfo() throws SQLException { - return null + throw new UnsupportedOperationException("Test 123") } @Override diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDatabaseMetaData.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDatabaseMetaData.groovy new file mode 100644 index 0000000000..198f1832dd --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDatabaseMetaData.groovy @@ -0,0 +1,889 @@ +package test + +import java.sql.Connection +import java.sql.DatabaseMetaData +import java.sql.ResultSet +import java.sql.RowIdLifetime +import java.sql.SQLException + +class TestDatabaseMetaData implements DatabaseMetaData { + @Override + boolean allProceduresAreCallable() throws SQLException { + return false + } + + @Override + boolean allTablesAreSelectable() throws SQLException { + return false + } + + @Override + String getURL() throws SQLException { + return "jdbc:testdb://localhost" + } + + @Override + String getUserName() throws SQLException { + return null + } + + @Override + boolean isReadOnly() throws SQLException { + return false + } + + @Override + boolean nullsAreSortedHigh() throws SQLException { + return false + } + + @Override + boolean nullsAreSortedLow() throws SQLException { + return false + } + + @Override + boolean nullsAreSortedAtStart() throws SQLException { + return false + } + + @Override + boolean nullsAreSortedAtEnd() throws SQLException { + return false + } + + @Override + String getDatabaseProductName() throws SQLException { + return null + } + + @Override + String getDatabaseProductVersion() throws SQLException { + return null + } + + @Override + String getDriverName() throws SQLException { + return null + } + + @Override + String getDriverVersion() throws SQLException { + return null + } + + @Override + int getDriverMajorVersion() { + return 0 + } + + @Override + int getDriverMinorVersion() { + return 0 + } + + @Override + boolean usesLocalFiles() throws SQLException { + return false + } + + @Override + boolean usesLocalFilePerTable() throws SQLException { + return false + } + + @Override + boolean supportsMixedCaseIdentifiers() throws SQLException { + return false + } + + @Override + boolean storesUpperCaseIdentifiers() throws SQLException { + return false + } + + @Override + boolean storesLowerCaseIdentifiers() throws SQLException { + return false + } + + @Override + boolean storesMixedCaseIdentifiers() throws SQLException { + return false + } + + @Override + boolean supportsMixedCaseQuotedIdentifiers() throws SQLException { + return false + } + + @Override + boolean storesUpperCaseQuotedIdentifiers() throws SQLException { + return false + } + + @Override + boolean storesLowerCaseQuotedIdentifiers() throws SQLException { + return false + } + + @Override + boolean storesMixedCaseQuotedIdentifiers() throws SQLException { + return false + } + + @Override + String getIdentifierQuoteString() throws SQLException { + return null + } + + @Override + String getSQLKeywords() throws SQLException { + return null + } + + @Override + String getNumericFunctions() throws SQLException { + return null + } + + @Override + String getStringFunctions() throws SQLException { + return null + } + + @Override + String getSystemFunctions() throws SQLException { + return null + } + + @Override + String getTimeDateFunctions() throws SQLException { + return null + } + + @Override + String getSearchStringEscape() throws SQLException { + return null + } + + @Override + String getExtraNameCharacters() throws SQLException { + return null + } + + @Override + boolean supportsAlterTableWithAddColumn() throws SQLException { + return false + } + + @Override + boolean supportsAlterTableWithDropColumn() throws SQLException { + return false + } + + @Override + boolean supportsColumnAliasing() throws SQLException { + return false + } + + @Override + boolean nullPlusNonNullIsNull() throws SQLException { + return false + } + + @Override + boolean supportsConvert() throws SQLException { + return false + } + + @Override + boolean supportsConvert(int fromType, int toType) throws SQLException { + return false + } + + @Override + boolean supportsTableCorrelationNames() throws SQLException { + return false + } + + @Override + boolean supportsDifferentTableCorrelationNames() throws SQLException { + return false + } + + @Override + boolean supportsExpressionsInOrderBy() throws SQLException { + return false + } + + @Override + boolean supportsOrderByUnrelated() throws SQLException { + return false + } + + @Override + boolean supportsGroupBy() throws SQLException { + return false + } + + @Override + boolean supportsGroupByUnrelated() throws SQLException { + return false + } + + @Override + boolean supportsGroupByBeyondSelect() throws SQLException { + return false + } + + @Override + boolean supportsLikeEscapeClause() throws SQLException { + return false + } + + @Override + boolean supportsMultipleResultSets() throws SQLException { + return false + } + + @Override + boolean supportsMultipleTransactions() throws SQLException { + return false + } + + @Override + boolean supportsNonNullableColumns() throws SQLException { + return false + } + + @Override + boolean supportsMinimumSQLGrammar() throws SQLException { + return false + } + + @Override + boolean supportsCoreSQLGrammar() throws SQLException { + return false + } + + @Override + boolean supportsExtendedSQLGrammar() throws SQLException { + return false + } + + @Override + boolean supportsANSI92EntryLevelSQL() throws SQLException { + return false + } + + @Override + boolean supportsANSI92IntermediateSQL() throws SQLException { + return false + } + + @Override + boolean supportsANSI92FullSQL() throws SQLException { + return false + } + + @Override + boolean supportsIntegrityEnhancementFacility() throws SQLException { + return false + } + + @Override + boolean supportsOuterJoins() throws SQLException { + return false + } + + @Override + boolean supportsFullOuterJoins() throws SQLException { + return false + } + + @Override + boolean supportsLimitedOuterJoins() throws SQLException { + return false + } + + @Override + String getSchemaTerm() throws SQLException { + return null + } + + @Override + String getProcedureTerm() throws SQLException { + return null + } + + @Override + String getCatalogTerm() throws SQLException { + return null + } + + @Override + boolean isCatalogAtStart() throws SQLException { + return false + } + + @Override + String getCatalogSeparator() throws SQLException { + return null + } + + @Override + boolean supportsSchemasInDataManipulation() throws SQLException { + return false + } + + @Override + boolean supportsSchemasInProcedureCalls() throws SQLException { + return false + } + + @Override + boolean supportsSchemasInTableDefinitions() throws SQLException { + return false + } + + @Override + boolean supportsSchemasInIndexDefinitions() throws SQLException { + return false + } + + @Override + boolean supportsSchemasInPrivilegeDefinitions() throws SQLException { + return false + } + + @Override + boolean supportsCatalogsInDataManipulation() throws SQLException { + return false + } + + @Override + boolean supportsCatalogsInProcedureCalls() throws SQLException { + return false + } + + @Override + boolean supportsCatalogsInTableDefinitions() throws SQLException { + return false + } + + @Override + boolean supportsCatalogsInIndexDefinitions() throws SQLException { + return false + } + + @Override + boolean supportsCatalogsInPrivilegeDefinitions() throws SQLException { + return false + } + + @Override + boolean supportsPositionedDelete() throws SQLException { + return false + } + + @Override + boolean supportsPositionedUpdate() throws SQLException { + return false + } + + @Override + boolean supportsSelectForUpdate() throws SQLException { + return false + } + + @Override + boolean supportsStoredProcedures() throws SQLException { + return false + } + + @Override + boolean supportsSubqueriesInComparisons() throws SQLException { + return false + } + + @Override + boolean supportsSubqueriesInExists() throws SQLException { + return false + } + + @Override + boolean supportsSubqueriesInIns() throws SQLException { + return false + } + + @Override + boolean supportsSubqueriesInQuantifieds() throws SQLException { + return false + } + + @Override + boolean supportsCorrelatedSubqueries() throws SQLException { + return false + } + + @Override + boolean supportsUnion() throws SQLException { + return false + } + + @Override + boolean supportsUnionAll() throws SQLException { + return false + } + + @Override + boolean supportsOpenCursorsAcrossCommit() throws SQLException { + return false + } + + @Override + boolean supportsOpenCursorsAcrossRollback() throws SQLException { + return false + } + + @Override + boolean supportsOpenStatementsAcrossCommit() throws SQLException { + return false + } + + @Override + boolean supportsOpenStatementsAcrossRollback() throws SQLException { + return false + } + + @Override + int getMaxBinaryLiteralLength() throws SQLException { + return 0 + } + + @Override + int getMaxCharLiteralLength() throws SQLException { + return 0 + } + + @Override + int getMaxColumnNameLength() throws SQLException { + return 0 + } + + @Override + int getMaxColumnsInGroupBy() throws SQLException { + return 0 + } + + @Override + int getMaxColumnsInIndex() throws SQLException { + return 0 + } + + @Override + int getMaxColumnsInOrderBy() throws SQLException { + return 0 + } + + @Override + int getMaxColumnsInSelect() throws SQLException { + return 0 + } + + @Override + int getMaxColumnsInTable() throws SQLException { + return 0 + } + + @Override + int getMaxConnections() throws SQLException { + return 0 + } + + @Override + int getMaxCursorNameLength() throws SQLException { + return 0 + } + + @Override + int getMaxIndexLength() throws SQLException { + return 0 + } + + @Override + int getMaxSchemaNameLength() throws SQLException { + return 0 + } + + @Override + int getMaxProcedureNameLength() throws SQLException { + return 0 + } + + @Override + int getMaxCatalogNameLength() throws SQLException { + return 0 + } + + @Override + int getMaxRowSize() throws SQLException { + return 0 + } + + @Override + boolean doesMaxRowSizeIncludeBlobs() throws SQLException { + return false + } + + @Override + int getMaxStatementLength() throws SQLException { + return 0 + } + + @Override + int getMaxStatements() throws SQLException { + return 0 + } + + @Override + int getMaxTableNameLength() throws SQLException { + return 0 + } + + @Override + int getMaxTablesInSelect() throws SQLException { + return 0 + } + + @Override + int getMaxUserNameLength() throws SQLException { + return 0 + } + + @Override + int getDefaultTransactionIsolation() throws SQLException { + return 0 + } + + @Override + boolean supportsTransactions() throws SQLException { + return false + } + + @Override + boolean supportsTransactionIsolationLevel(int level) throws SQLException { + return false + } + + @Override + boolean supportsDataDefinitionAndDataManipulationTransactions() throws SQLException { + return false + } + + @Override + boolean supportsDataManipulationTransactionsOnly() throws SQLException { + return false + } + + @Override + boolean dataDefinitionCausesTransactionCommit() throws SQLException { + return false + } + + @Override + boolean dataDefinitionIgnoredInTransactions() throws SQLException { + return false + } + + @Override + ResultSet getProcedures(String catalog, String schemaPattern, String procedureNamePattern) throws SQLException { + return null + } + + @Override + ResultSet getProcedureColumns(String catalog, String schemaPattern, String procedureNamePattern, String columnNamePattern) throws SQLException { + return null + } + + @Override + ResultSet getTables(String catalog, String schemaPattern, String tableNamePattern, String[] types) throws SQLException { + return null + } + + @Override + ResultSet getSchemas() throws SQLException { + return null + } + + @Override + ResultSet getCatalogs() throws SQLException { + return null + } + + @Override + ResultSet getTableTypes() throws SQLException { + return null + } + + @Override + ResultSet getColumns(String catalog, String schemaPattern, String tableNamePattern, String columnNamePattern) throws SQLException { + return null + } + + @Override + ResultSet getColumnPrivileges(String catalog, String schema, String table, String columnNamePattern) throws SQLException { + return null + } + + @Override + ResultSet getTablePrivileges(String catalog, String schemaPattern, String tableNamePattern) throws SQLException { + return null + } + + @Override + ResultSet getBestRowIdentifier(String catalog, String schema, String table, int scope, boolean nullable) throws SQLException { + return null + } + + @Override + ResultSet getVersionColumns(String catalog, String schema, String table) throws SQLException { + return null + } + + @Override + ResultSet getPrimaryKeys(String catalog, String schema, String table) throws SQLException { + return null + } + + @Override + ResultSet getImportedKeys(String catalog, String schema, String table) throws SQLException { + return null + } + + @Override + ResultSet getExportedKeys(String catalog, String schema, String table) throws SQLException { + return null + } + + @Override + ResultSet getCrossReference(String parentCatalog, String parentSchema, String parentTable, String foreignCatalog, String foreignSchema, String foreignTable) throws SQLException { + return null + } + + @Override + ResultSet getTypeInfo() throws SQLException { + return null + } + + @Override + ResultSet getIndexInfo(String catalog, String schema, String table, boolean unique, boolean approximate) throws SQLException { + return null + } + + @Override + boolean supportsResultSetType(int type) throws SQLException { + return false + } + + @Override + boolean supportsResultSetConcurrency(int type, int concurrency) throws SQLException { + return false + } + + @Override + boolean ownUpdatesAreVisible(int type) throws SQLException { + return false + } + + @Override + boolean ownDeletesAreVisible(int type) throws SQLException { + return false + } + + @Override + boolean ownInsertsAreVisible(int type) throws SQLException { + return false + } + + @Override + boolean othersUpdatesAreVisible(int type) throws SQLException { + return false + } + + @Override + boolean othersDeletesAreVisible(int type) throws SQLException { + return false + } + + @Override + boolean othersInsertsAreVisible(int type) throws SQLException { + return false + } + + @Override + boolean updatesAreDetected(int type) throws SQLException { + return false + } + + @Override + boolean deletesAreDetected(int type) throws SQLException { + return false + } + + @Override + boolean insertsAreDetected(int type) throws SQLException { + return false + } + + @Override + boolean supportsBatchUpdates() throws SQLException { + return false + } + + @Override + ResultSet getUDTs(String catalog, String schemaPattern, String typeNamePattern, int[] types) throws SQLException { + return null + } + + @Override + Connection getConnection() throws SQLException { + return null + } + + @Override + boolean supportsSavepoints() throws SQLException { + return false + } + + @Override + boolean supportsNamedParameters() throws SQLException { + return false + } + + @Override + boolean supportsMultipleOpenResults() throws SQLException { + return false + } + + @Override + boolean supportsGetGeneratedKeys() throws SQLException { + return false + } + + @Override + ResultSet getSuperTypes(String catalog, String schemaPattern, String typeNamePattern) throws SQLException { + return null + } + + @Override + ResultSet getSuperTables(String catalog, String schemaPattern, String tableNamePattern) throws SQLException { + return null + } + + @Override + ResultSet getAttributes(String catalog, String schemaPattern, String typeNamePattern, String attributeNamePattern) throws SQLException { + return null + } + + @Override + boolean supportsResultSetHoldability(int holdability) throws SQLException { + return false + } + + @Override + int getResultSetHoldability() throws SQLException { + return 0 + } + + @Override + int getDatabaseMajorVersion() throws SQLException { + return 0 + } + + @Override + int getDatabaseMinorVersion() throws SQLException { + return 0 + } + + @Override + int getJDBCMajorVersion() throws SQLException { + return 0 + } + + @Override + int getJDBCMinorVersion() throws SQLException { + return 0 + } + + @Override + int getSQLStateType() throws SQLException { + return 0 + } + + @Override + boolean locatorsUpdateCopy() throws SQLException { + return false + } + + @Override + boolean supportsStatementPooling() throws SQLException { + return false + } + + @Override + RowIdLifetime getRowIdLifetime() throws SQLException { + return null + } + + @Override + ResultSet getSchemas(String catalog, String schemaPattern) throws SQLException { + return null + } + + @Override + boolean supportsStoredFunctionsUsingCallSyntax() throws SQLException { + return false + } + + @Override + boolean autoCommitFailureClosesAllResultSets() throws SQLException { + return false + } + + @Override + ResultSet getClientInfoProperties() throws SQLException { + return null + } + + @Override + ResultSet getFunctions(String catalog, String schemaPattern, String functionNamePattern) throws SQLException { + return null + } + + @Override + ResultSet getFunctionColumns(String catalog, String schemaPattern, String functionNamePattern, String columnNamePattern) throws SQLException { + return null + } + + @Override + ResultSet getPseudoColumns(String catalog, String schemaPattern, String tableNamePattern, String columnNamePattern) throws SQLException { + return null + } + + @Override + boolean generatedKeyAlwaysReturned() throws SQLException { + return false + } + + @Override + def T unwrap(Class iface) throws SQLException { + return null + } + + @Override + boolean isWrapperFor(Class iface) throws SQLException { + return false + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDriver.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDriver.groovy new file mode 100644 index 0000000000..7983583ad2 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDriver.groovy @@ -0,0 +1,45 @@ +package test + +import java.sql.Connection +import java.sql.Driver +import java.sql.DriverPropertyInfo +import java.sql.SQLException +import java.sql.SQLFeatureNotSupportedException +import java.util.logging.Logger + +class TestDriver implements Driver { + @Override + Connection connect(String url, Properties info) throws SQLException { + return new TestConnection("connectException=true" == url) + } + + @Override + boolean acceptsURL(String url) throws SQLException { + return false + } + + @Override + DriverPropertyInfo[] getPropertyInfo(String url, Properties info) throws SQLException { + return new DriverPropertyInfo[0] + } + + @Override + int getMajorVersion() { + return 0 + } + + @Override + int getMinorVersion() { + return 0 + } + + @Override + boolean jdbcCompliant() { + return false + } + + @Override + Logger getParentLogger() throws SQLFeatureNotSupportedException { + return null + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestStatement.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestStatement.groovy new file mode 100644 index 0000000000..4d87091a6a --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestStatement.groovy @@ -0,0 +1,235 @@ +package test + +import java.sql.Connection +import java.sql.ResultSet +import java.sql.SQLException +import java.sql.SQLWarning +import java.sql.Statement + +class TestStatement implements Statement { + final Connection connection + + TestStatement(Connection connection) { + this.connection = connection + } + + @Override + ResultSet executeQuery(String sql) throws SQLException { + return null + } + + @Override + int executeUpdate(String sql) throws SQLException { + return 0 + } + + @Override + void close() throws SQLException { + + } + + @Override + int getMaxFieldSize() throws SQLException { + return 0 + } + + @Override + void setMaxFieldSize(int max) throws SQLException { + + } + + @Override + int getMaxRows() throws SQLException { + return 0 + } + + @Override + void setMaxRows(int max) throws SQLException { + + } + + @Override + void setEscapeProcessing(boolean enable) throws SQLException { + + } + + @Override + int getQueryTimeout() throws SQLException { + return 0 + } + + @Override + void setQueryTimeout(int seconds) throws SQLException { + + } + + @Override + void cancel() throws SQLException { + + } + + @Override + SQLWarning getWarnings() throws SQLException { + return null + } + + @Override + void clearWarnings() throws SQLException { + + } + + @Override + void setCursorName(String name) throws SQLException { + + } + + @Override + boolean execute(String sql) throws SQLException { + return false + } + + @Override + ResultSet getResultSet() throws SQLException { + return null + } + + @Override + int getUpdateCount() throws SQLException { + return 0 + } + + @Override + boolean getMoreResults() throws SQLException { + return false + } + + @Override + void setFetchDirection(int direction) throws SQLException { + + } + + @Override + int getFetchDirection() throws SQLException { + return 0 + } + + @Override + void setFetchSize(int rows) throws SQLException { + + } + + @Override + int getFetchSize() throws SQLException { + return 0 + } + + @Override + int getResultSetConcurrency() throws SQLException { + return 0 + } + + @Override + int getResultSetType() throws SQLException { + return 0 + } + + @Override + void addBatch(String sql) throws SQLException { + + } + + @Override + void clearBatch() throws SQLException { + + } + + @Override + int[] executeBatch() throws SQLException { + return new int[0] + } + + @Override + Connection getConnection() throws SQLException { + return connection + } + + @Override + boolean getMoreResults(int current) throws SQLException { + return false + } + + @Override + ResultSet getGeneratedKeys() throws SQLException { + return null + } + + @Override + int executeUpdate(String sql, int autoGeneratedKeys) throws SQLException { + return 0 + } + + @Override + int executeUpdate(String sql, int[] columnIndexes) throws SQLException { + return 0 + } + + @Override + int executeUpdate(String sql, String[] columnNames) throws SQLException { + return 0 + } + + @Override + boolean execute(String sql, int autoGeneratedKeys) throws SQLException { + return false + } + + @Override + boolean execute(String sql, int[] columnIndexes) throws SQLException { + return false + } + + @Override + boolean execute(String sql, String[] columnNames) throws SQLException { + return false + } + + @Override + int getResultSetHoldability() throws SQLException { + return 0 + } + + @Override + boolean isClosed() throws SQLException { + return false + } + + @Override + void setPoolable(boolean poolable) throws SQLException { + + } + + @Override + boolean isPoolable() throws SQLException { + return false + } + + @Override + void closeOnCompletion() throws SQLException { + + } + + @Override + boolean isCloseOnCompletion() throws SQLException { + return false + } + + @Override + def T unwrap(Class iface) throws SQLException { + return null + } + + @Override + boolean isWrapperFor(Class iface) throws SQLException { + return false + } +} diff --git a/dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy b/dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy index 26535e877c..7d433c387c 100644 --- a/dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy +++ b/dd-java-agent/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy @@ -640,4 +640,49 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { "normal" | "compileError.jsp" | "compileError_jsp" | "" "forward" | "forwards/forwardWithCompileError.jsp" | "forwardWithCompileError_jsp" | "forwards." } + + def "direct static file reference"() { + setup: + String reqUrl = baseUrl + "/$staticFile" + def req = new Request.Builder().url(new URL(reqUrl)).get().build() + + when: + Response res = client.newCall(req).execute() + + then: + res.code() == HttpStatus.OK_200 + assertTraces(1) { + trace(0, 1) { + span(0) { + parent() + serviceName jspWebappContext + operationName "servlet.request" + // FIXME: this is not a great resource name for serving static content. + resourceName "GET /$jspWebappContext/$staticFile" + spanType DDSpanTypes.HTTP_SERVER + errored false + tags { + "$Tags.COMPONENT" "java-web-servlet" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER + "$Tags.PEER_HOSTNAME" "127.0.0.1" + "$Tags.PEER_HOST_IPV4" "127.0.0.1" + "$Tags.PEER_PORT" Integer + "$Tags.HTTP_URL" "http://localhost:$port/$jspWebappContext/$staticFile" + "$Tags.HTTP_METHOD" "GET" + "$Tags.HTTP_STATUS" 200 + "span.origin.type" "org.apache.catalina.core.ApplicationFilterChain" + "servlet.context" "/$jspWebappContext" + "servlet.path" "/$staticFile" + defaultTags() + } + } + } + } + + cleanup: + res.close() + + where: + staticFile = "common/hello.html" + } } From 43fbf28035e062501ab2a88881eb2ad7d60158cc Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 8 Jan 2020 16:11:07 -0800 Subject: [PATCH 14/27] Revert "Remove experimental jdbc and servlet integrations until further evaluation" This reverts commit 2432a92230db759af80c8c8702485cf3df692766. --- .../jdbc/DataSourceDecorator.java | 22 ++ .../jdbc/DataSourceInstrumentation.java | 81 ++++++ .../groovy/JDBCInstrumentationTest.groovy | 66 +++++ .../instrumentation/servlet/servlet.gradle | 20 ++ .../servlet/ServletRequestSetter.java | 14 + .../RequestDispatcherDecorator.java | 22 ++ .../RequestDispatcherInstrumentation.java | 108 +++++++ .../ServletContextInstrumentation.java | 62 ++++ .../servlet/filter/FilterDecorator.java | 22 ++ .../servlet/filter/FilterInstrumentation.java | 89 ++++++ .../servlet/http/HttpServletDecorator.java | 22 ++ .../http/HttpServletInstrumentation.java | 112 +++++++ .../http/HttpServletResponseDecorator.java | 22 ++ .../HttpServletResponseInstrumentation.java | 105 +++++++ .../servlet/src/test/groovy/FilterTest.groovy | 105 +++++++ .../groovy/HttpServletResponseTest.groovy | 275 ++++++++++++++++++ .../src/test/groovy/HttpServletTest.groovy | 125 ++++++++ .../test/groovy/RequestDispatcherTest.groovy | 97 ++++++ .../test/groovy/RequestDispatcherUtils.java | 181 ++++++++++++ 19 files changed, 1550 insertions(+) create mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceDecorator.java create mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceInstrumentation.java create mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/ServletRequestSetter.java create mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java create mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java create mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java create mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterDecorator.java create mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterInstrumentation.java create mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletDecorator.java create mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java create mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseDecorator.java create mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseInstrumentation.java create mode 100644 dd-java-agent/instrumentation/servlet/src/test/groovy/FilterTest.groovy create mode 100644 dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy create mode 100644 dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletTest.groovy create mode 100644 dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy create mode 100644 dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherUtils.java diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceDecorator.java new file mode 100644 index 0000000000..d4e75e1455 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceDecorator.java @@ -0,0 +1,22 @@ +package datadog.trace.instrumentation.jdbc; + +import datadog.trace.agent.decorator.BaseDecorator; + +public class DataSourceDecorator extends BaseDecorator { + public static final DataSourceDecorator DECORATE = new DataSourceDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"jdbc-beta", "jdbc-datasource"}; + } + + @Override + protected String component() { + return "java-jdbc-connection"; + } + + @Override + protected String spanType() { + return null; + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceInstrumentation.java new file mode 100644 index 0000000000..43091ae449 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceInstrumentation.java @@ -0,0 +1,81 @@ +package datadog.trace.instrumentation.jdbc; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static datadog.trace.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.instrumentation.api.AgentTracer.activeSpan; +import static datadog.trace.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.jdbc.DataSourceDecorator.DECORATE; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.api.DDTags; +import datadog.trace.instrumentation.api.AgentScope; +import datadog.trace.instrumentation.api.AgentSpan; +import java.util.Map; +import javax.sql.DataSource; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class DataSourceInstrumentation extends Instrumenter.Default { + public DataSourceInstrumentation() { + super("jdbc-beta", "jdbc-datasource"); + } + + @Override + public boolean defaultEnabled() { + return false; + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", packageName + ".DataSourceDecorator", + }; + } + + @Override + public ElementMatcher typeMatcher() { + return not(isInterface()).and(safeHasSuperType(named("javax.sql.DataSource"))); + } + + @Override + public Map, String> transformers() { + return singletonMap(named("getConnection"), GetConnectionAdvice.class.getName()); + } + + public static class GetConnectionAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope start(@Advice.This final DataSource ds) { + if (activeSpan() == null) { + // Don't want to generate a new top-level span + return null; + } + + final AgentSpan span = startSpan("database.connection"); + DECORATE.afterStart(span); + + span.setTag(DDTags.RESOURCE_NAME, ds.getClass().getSimpleName() + ".getConnection"); + + return activateSpan(span, true).setAsyncPropagation(true); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) { + if (scope == null) { + return; + } + DECORATE.onError(scope, throwable); + DECORATE.beforeFinish(scope); + scope.close(); + } + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy index 54b5bee265..0560e5ec0c 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy @@ -5,8 +5,11 @@ import datadog.trace.agent.test.AgentTestRunner import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import datadog.trace.instrumentation.api.Tags +import javax.sql.DataSource +import org.apache.derby.jdbc.EmbeddedDataSource import org.apache.derby.jdbc.EmbeddedDriver import org.h2.Driver +import org.h2.jdbcx.JdbcDataSource import org.hsqldb.jdbc.JDBCDriver import spock.lang.Shared import spock.lang.Unroll @@ -25,6 +28,9 @@ import static datadog.trace.agent.test.utils.TraceUtils.basicSpan import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace class JDBCInstrumentationTest extends AgentTestRunner { + static { + System.setProperty("dd.integration.jdbc-beta.enabled", "true") + } @Shared def dbName = "jdbcUnitTest" @@ -550,6 +556,66 @@ class JDBCInstrumentationTest extends AgentTestRunner { false | "derby" | new EmbeddedDriver() | "jdbc:derby:memory:" + dbName + ";create=true" | "APP" | "SELECT 3 FROM SYSIBM.SYSDUMMY1" } + def "calling #datasource.class.simpleName getConnection generates a span when under existing trace"() { + setup: + assert datasource instanceof DataSource + init?.call(datasource) + + when: + datasource.getConnection().close() + + then: + !TEST_WRITER.any { it.any { it.operationName == "database.connection" } } + TEST_WRITER.clear() + + when: + runUnderTrace("parent") { + datasource.getConnection().close() + } + + then: + assertTraces(1) { + trace(0, recursive ? 3 : 2) { + basicSpan(it, 0, "parent") + + span(1) { + operationName "database.connection" + resourceName "${datasource.class.simpleName}.getConnection" + childOf span(0) + tags { + "$Tags.COMPONENT" "java-jdbc-connection" + defaultTags() + } + } + if (recursive) { + span(2) { + operationName "database.connection" + resourceName "${datasource.class.simpleName}.getConnection" + childOf span(1) + tags { + "$Tags.COMPONENT" "java-jdbc-connection" + defaultTags() + } + } + } + } + } + + where: + datasource | init + new JdbcDataSource() | { ds -> ds.setURL(jdbcUrls.get("h2")) } + new EmbeddedDataSource() | { ds -> ds.jdbcurl = jdbcUrls.get("derby") } + cpDatasources.get("hikari").get("h2") | null + cpDatasources.get("hikari").get("derby") | null + cpDatasources.get("c3p0").get("h2") | null + cpDatasources.get("c3p0").get("derby") | null + + // Tomcat's pool doesn't work because the getConnection method is + // implemented in a parent class that doesn't implement DataSource + + recursive = datasource instanceof EmbeddedDataSource + } + def "test getClientInfo exception"() { setup: Connection connection = new TestConnection(false) diff --git a/dd-java-agent/instrumentation/servlet/servlet.gradle b/dd-java-agent/instrumentation/servlet/servlet.gradle index ff6901ae6f..1f4583dd52 100644 --- a/dd-java-agent/instrumentation/servlet/servlet.gradle +++ b/dd-java-agent/instrumentation/servlet/servlet.gradle @@ -1 +1,21 @@ +muzzle { + pass { + group = "javax.servlet" + module = 'javax.servlet-api' + versions = "[,]" + assertInverse = true + } + pass { + group = "javax.servlet" + module = 'servlet-api' + versions = "[,]" + } +} + apply from: "${rootDir}/gradle/java.gradle" + +dependencies { + compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.3' + + testCompile group: 'javax.servlet', name: 'servlet-api', version: '2.3' +} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/ServletRequestSetter.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/ServletRequestSetter.java new file mode 100644 index 0000000000..8719726664 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/ServletRequestSetter.java @@ -0,0 +1,14 @@ +package datadog.trace.instrumentation.servlet; + +import datadog.trace.instrumentation.api.AgentPropagation; +import javax.servlet.ServletRequest; + +/** Inject into request attributes since the request headers can't be modified. */ +public class ServletRequestSetter implements AgentPropagation.Setter { + public static final ServletRequestSetter SETTER = new ServletRequestSetter(); + + @Override + public void set(final ServletRequest carrier, final String key, final String value) { + carrier.setAttribute(key, value); + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java new file mode 100644 index 0000000000..1e0550cfbe --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java @@ -0,0 +1,22 @@ +package datadog.trace.instrumentation.servlet.dispatcher; + +import datadog.trace.agent.decorator.BaseDecorator; + +public class RequestDispatcherDecorator extends BaseDecorator { + public static final RequestDispatcherDecorator DECORATE = new RequestDispatcherDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"servlet-beta", "servlet-dispatcher"}; + } + + @Override + protected String spanType() { + return null; + } + + @Override + protected String component() { + return "java-web-servlet-dispatcher"; + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java new file mode 100644 index 0000000000..662ddc629b --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java @@ -0,0 +1,108 @@ +package datadog.trace.instrumentation.servlet.dispatcher; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static datadog.trace.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.instrumentation.api.AgentTracer.activeSpan; +import static datadog.trace.instrumentation.api.AgentTracer.propagate; +import static datadog.trace.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.servlet.ServletRequestSetter.SETTER; +import static datadog.trace.instrumentation.servlet.dispatcher.RequestDispatcherDecorator.DECORATE; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.api.DDTags; +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.instrumentation.api.AgentScope; +import datadog.trace.instrumentation.api.AgentSpan; +import java.util.Map; +import javax.servlet.RequestDispatcher; +import javax.servlet.ServletRequest; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class RequestDispatcherInstrumentation extends Instrumenter.Default { + public RequestDispatcherInstrumentation() { + super("servlet-beta", "servlet-dispatcher"); + } + + @Override + public boolean defaultEnabled() { + return false; + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.instrumentation.servlet.ServletRequestSetter", + "datadog.trace.agent.decorator.BaseDecorator", + packageName + ".RequestDispatcherDecorator", + }; + } + + @Override + public ElementMatcher typeMatcher() { + return not(isInterface()).and(safeHasSuperType(named("javax.servlet.RequestDispatcher"))); + } + + @Override + public Map contextStore() { + return singletonMap("javax.servlet.RequestDispatcher", String.class.getName()); + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("forward") + .or(named("include")) + .and(takesArgument(0, named("javax.servlet.ServletRequest"))) + .and(takesArgument(1, named("javax.servlet.ServletResponse"))) + .and(isPublic()), + RequestDispatcherAdvice.class.getName()); + } + + public static class RequestDispatcherAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope start( + @Advice.Origin("#m") final String method, + @Advice.This final RequestDispatcher dispatcher, + @Advice.Argument(0) final ServletRequest request) { + if (activeSpan() == null) { + // Don't want to generate a new top-level span + return null; + } + + final AgentSpan span = startSpan("servlet." + method); + DECORATE.afterStart(span); + + final String target = + InstrumentationContext.get(RequestDispatcher.class, String.class).get(dispatcher); + span.setTag(DDTags.RESOURCE_NAME, target); + + // In case we lose context, inject trace into to the request. + propagate().inject(span, request, SETTER); + + return activateSpan(span, true).setAsyncPropagation(true); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stop( + @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) { + if (scope == null) { + return; + } + DECORATE.onError(scope, throwable); + DECORATE.beforeFinish(scope); + scope.close(); + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java new file mode 100644 index 0000000000..3426135c6b --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java @@ -0,0 +1,62 @@ +package datadog.trace.instrumentation.servlet.dispatcher; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.InstrumentationContext; +import java.util.Map; +import javax.servlet.RequestDispatcher; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class ServletContextInstrumentation extends Instrumenter.Default { + public ServletContextInstrumentation() { + super("servlet-beta", "servlet-dispatcher"); + } + + @Override + public boolean defaultEnabled() { + return false; + } + + @Override + public ElementMatcher typeMatcher() { + return not(isInterface()).and(safeHasSuperType(named("javax.servlet.ServletContext"))); + } + + @Override + public Map contextStore() { + return singletonMap("javax.servlet.RequestDispatcher", String.class.getName()); + } + + @Override + public Map, String> transformers() { + return singletonMap( + returns(named("javax.servlet.RequestDispatcher")) + .and(takesArgument(0, String.class)) + // javax.servlet.ServletContext.getRequestDispatcher + // javax.servlet.ServletContext.getNamedDispatcher + .and(isPublic()), + RequestDispatcherTargetAdvice.class.getName()); + } + + public static class RequestDispatcherTargetAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void saveTarget( + @Advice.Argument(0) final String target, + @Advice.Return final RequestDispatcher dispatcher) { + InstrumentationContext.get(RequestDispatcher.class, String.class).put(dispatcher, target); + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterDecorator.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterDecorator.java new file mode 100644 index 0000000000..c5dfc98f19 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterDecorator.java @@ -0,0 +1,22 @@ +package datadog.trace.instrumentation.servlet.filter; + +import datadog.trace.agent.decorator.BaseDecorator; + +public class FilterDecorator extends BaseDecorator { + public static final FilterDecorator DECORATE = new FilterDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"servlet-beta", "servlet-filter"}; + } + + @Override + protected String spanType() { + return null; + } + + @Override + protected String component() { + return "java-web-servlet-filter"; + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterInstrumentation.java new file mode 100644 index 0000000000..f94814b908 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterInstrumentation.java @@ -0,0 +1,89 @@ +package datadog.trace.instrumentation.servlet.filter; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static datadog.trace.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.instrumentation.api.AgentTracer.activeSpan; +import static datadog.trace.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.servlet.filter.FilterDecorator.DECORATE; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.api.DDTags; +import datadog.trace.instrumentation.api.AgentScope; +import datadog.trace.instrumentation.api.AgentSpan; +import java.util.Map; +import javax.servlet.Filter; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class FilterInstrumentation extends Instrumenter.Default { + public FilterInstrumentation() { + super("servlet-beta", "servlet-filter"); + } + + @Override + public boolean defaultEnabled() { + return false; + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", packageName + ".FilterDecorator", + }; + } + + @Override + public ElementMatcher typeMatcher() { + return not(isInterface()).and(safeHasSuperType(named("javax.servlet.Filter"))); + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("doFilter") + .and(takesArgument(0, named("javax.servlet.ServletRequest"))) + .and(takesArgument(1, named("javax.servlet.ServletResponse"))) + .and(isPublic()), + FilterAdvice.class.getName()); + } + + public static class FilterAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope start(@Advice.This final Filter filter) { + if (activeSpan() == null) { + // Don't want to generate a new top-level span + return null; + } + + final AgentSpan span = startSpan("servlet.filter"); + DECORATE.afterStart(span); + + // Here we use "this" instead of "the method target" to distinguish abstract filter instances. + span.setTag(DDTags.RESOURCE_NAME, filter.getClass().getSimpleName() + ".doFilter"); + + return activateSpan(span, true).setAsyncPropagation(true); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) { + if (scope == null) { + return; + } + DECORATE.onError(scope, throwable); + DECORATE.beforeFinish(scope); + scope.close(); + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletDecorator.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletDecorator.java new file mode 100644 index 0000000000..b4a8d93324 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletDecorator.java @@ -0,0 +1,22 @@ +package datadog.trace.instrumentation.servlet.http; + +import datadog.trace.agent.decorator.BaseDecorator; + +public class HttpServletDecorator extends BaseDecorator { + public static final HttpServletDecorator DECORATE = new HttpServletDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"servlet-beta", "servlet-service"}; + } + + @Override + protected String spanType() { + return null; + } + + @Override + protected String component() { + return "java-web-servlet-service"; + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java new file mode 100644 index 0000000000..faba631452 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java @@ -0,0 +1,112 @@ +package datadog.trace.instrumentation.servlet.http; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static datadog.trace.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.instrumentation.api.AgentTracer.activeSpan; +import static datadog.trace.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.servlet.http.HttpServletDecorator.DECORATE; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isProtected; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.api.DDTags; +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.instrumentation.api.AgentScope; +import datadog.trace.instrumentation.api.AgentSpan; +import java.lang.reflect.Method; +import java.util.Map; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class HttpServletInstrumentation extends Instrumenter.Default { + public HttpServletInstrumentation() { + super("servlet-beta", "servlet-service"); + } + + @Override + public boolean defaultEnabled() { + return false; + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", packageName + ".HttpServletDecorator", + }; + } + + @Override + public ElementMatcher typeMatcher() { + return not(isInterface()).and(safeHasSuperType(named("javax.servlet.http.HttpServlet"))); + } + + /** + * Here we are instrumenting the protected method for HttpServlet. This should ensure that this + * advice is always called after Servlet3Instrumentation which is instrumenting the public method. + */ + @Override + public Map, String> transformers() { + return singletonMap( + named("service") + .or(nameStartsWith("do")) // doGet, doPost, etc + .and(takesArgument(0, named("javax.servlet.http.HttpServletRequest"))) + .and(takesArgument(1, named("javax.servlet.http.HttpServletResponse"))) + .and(isProtected().or(isPublic())), + HttpServletAdvice.class.getName()); + } + + @Override + public Map contextStore() { + return singletonMap( + "javax.servlet.http.HttpServletResponse", "javax.servlet.http.HttpServletRequest"); + } + + public static class HttpServletAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope start( + @Advice.Origin final Method method, + @Advice.Argument(0) final HttpServletRequest request, + @Advice.Argument(1) final HttpServletResponse response) { + // For use by HttpServletResponseInstrumentation: + InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class) + .put(response, request); + + if (activeSpan() == null) { + // Don't want to generate a new top-level span + return null; + } + + final AgentSpan span = startSpan("servlet." + method.getName()); + DECORATE.afterStart(span); + + // Here we use the Method instead of "this.class.name" to distinguish calls to "super". + span.setTag(DDTags.RESOURCE_NAME, DECORATE.spanNameForMethod(method)); + + return activateSpan(span, true).setAsyncPropagation(true); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) { + if (scope == null) { + return; + } + DECORATE.onError(scope, throwable); + DECORATE.beforeFinish(scope); + scope.close(); + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseDecorator.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseDecorator.java new file mode 100644 index 0000000000..782aa99247 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseDecorator.java @@ -0,0 +1,22 @@ +package datadog.trace.instrumentation.servlet.http; + +import datadog.trace.agent.decorator.BaseDecorator; + +public class HttpServletResponseDecorator extends BaseDecorator { + public static final HttpServletResponseDecorator DECORATE = new HttpServletResponseDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"servlet-beta", "servlet-response"}; + } + + @Override + protected String spanType() { + return null; + } + + @Override + protected String component() { + return "java-web-servlet-response"; + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseInstrumentation.java new file mode 100644 index 0000000000..68ae6400b0 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseInstrumentation.java @@ -0,0 +1,105 @@ +package datadog.trace.instrumentation.servlet.http; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static datadog.trace.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.instrumentation.api.AgentTracer.activeSpan; +import static datadog.trace.instrumentation.api.AgentTracer.propagate; +import static datadog.trace.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.servlet.ServletRequestSetter.SETTER; +import static datadog.trace.instrumentation.servlet.http.HttpServletResponseDecorator.DECORATE; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.api.DDTags; +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.instrumentation.api.AgentScope; +import datadog.trace.instrumentation.api.AgentSpan; +import java.util.Map; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class HttpServletResponseInstrumentation extends Instrumenter.Default { + public HttpServletResponseInstrumentation() { + super("servlet-beta", "servlet-response"); + } + + @Override + public boolean defaultEnabled() { + return false; + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.instrumentation.servlet.ServletRequestSetter", + "datadog.trace.agent.decorator.BaseDecorator", + packageName + ".HttpServletResponseDecorator", + }; + } + + @Override + public ElementMatcher typeMatcher() { + return not(isInterface()) + .and(safeHasSuperType(named("javax.servlet.http.HttpServletResponse"))); + } + + @Override + public Map, String> transformers() { + return singletonMap(named("sendError").or(named("sendRedirect")), SendAdvice.class.getName()); + } + + @Override + public Map contextStore() { + return singletonMap( + "javax.servlet.http.HttpServletResponse", "javax.servlet.http.HttpServletRequest"); + } + + public static class SendAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope start( + @Advice.Origin("#m") final String method, @Advice.This final HttpServletResponse resp) { + if (activeSpan() == null) { + // Don't want to generate a new top-level span + return null; + } + + final HttpServletRequest req = + InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class).get(resp); + if (req == null) { + // Missing the response->request linking... probably in a wrapped instance. + return null; + } + + final AgentSpan span = startSpan("servlet.response"); + DECORATE.afterStart(span); + + span.setTag(DDTags.RESOURCE_NAME, "HttpServletResponse." + method); + + // In case we lose context, inject trace into to the request. + propagate().inject(span, req, SETTER); + + return activateSpan(span, true).setAsyncPropagation(true); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) { + if (scope == null) { + return; + } + DECORATE.onError(scope, throwable); + DECORATE.beforeFinish(scope); + scope.close(); + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/FilterTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/FilterTest.groovy new file mode 100644 index 0000000000..fc7ccb5aa5 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/FilterTest.groovy @@ -0,0 +1,105 @@ +import datadog.trace.agent.test.AgentTestRunner +import javax.servlet.Filter +import javax.servlet.FilterChain +import javax.servlet.FilterConfig +import javax.servlet.ServletException +import javax.servlet.ServletRequest +import javax.servlet.ServletResponse + +import static datadog.trace.agent.test.utils.TraceUtils.basicSpan +import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace + +class FilterTest extends AgentTestRunner { + static { + System.setProperty("dd.integration.servlet-beta.enabled", "true") + } + + def "test doFilter no-parent"() { + when: + filter.doFilter(null, null, null) + + then: + assertTraces(0) {} + + where: + filter = new TestFilter() + } + + def "test doFilter with parent"() { + when: + runUnderTrace("parent") { + filter.doFilter(null, null, null) + } + + then: + assertTraces(1) { + trace(0, 2) { + basicSpan(it, 0, "parent") + span(1) { + operationName "servlet.filter" + resourceName "${filter.class.simpleName}.doFilter" + childOf span(0) + tags { + "component" "java-web-servlet-filter" + defaultTags() + } + } + } + } + + where: + filter << [new TestFilter(), new TestFilter() {}] + } + + def "test doFilter exception"() { + setup: + def ex = new Exception("some error") + def filter = new TestFilter() { + @Override + void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) { + throw ex + } + } + + when: + runUnderTrace("parent") { + filter.doFilter(null, null, null) + } + + then: + def th = thrown(Exception) + th == ex + + assertTraces(1) { + trace(0, 2) { + basicSpan(it, 0, "parent", null, ex) + span(1) { + operationName "servlet.filter" + resourceName "${filter.class.simpleName}.doFilter" + childOf span(0) + errored true + tags { + "component" "java-web-servlet-filter" + defaultTags() + errorTags(ex.class, ex.message) + } + } + } + } + } + + static class TestFilter implements Filter { + + @Override + void init(FilterConfig filterConfig) throws ServletException { + } + + @Override + void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException { + } + + @Override + void destroy() { + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy new file mode 100644 index 0000000000..9073a8044b --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy @@ -0,0 +1,275 @@ +import datadog.trace.agent.test.AgentTestRunner +import groovy.servlet.AbstractHttpServlet +import javax.servlet.ServletOutputStream +import javax.servlet.http.Cookie +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletResponse +import spock.lang.Subject + +import static datadog.trace.agent.test.utils.TraceUtils.basicSpan +import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace + +class HttpServletResponseTest extends AgentTestRunner { + static { + System.setProperty("dd.integration.servlet-beta.enabled", "true") + } + + @Subject + def response = new TestResponse() + def request = Mock(HttpServletRequest) { + getMethod() >> "GET" + getProtocol() >> "TEST" + } + + def setup() { + def servlet = new AbstractHttpServlet() {} + // We need to call service so HttpServletAdvice can link the request to the response. + servlet.service(request, response) + } + + def "test send no-parent"() { + when: + response.sendError(0) + response.sendError(0, "") + response.sendRedirect("") + + then: + assertTraces(0) {} + } + + def "test send with parent"() { + when: + runUnderTrace("parent") { + response.sendError(0) + response.sendError(0, "") + response.sendRedirect("") + } + + then: + assertTraces(1) { + trace(0, 4) { + basicSpan(it, 0, "parent") + span(1) { + operationName "servlet.response" + resourceName "HttpServletResponse.sendRedirect" + childOf span(0) + tags { + "component" "java-web-servlet-response" + defaultTags() + } + } + span(2) { + operationName "servlet.response" + resourceName "HttpServletResponse.sendError" + childOf span(0) + tags { + "component" "java-web-servlet-response" + defaultTags() + } + } + span(3) { + operationName "servlet.response" + resourceName "HttpServletResponse.sendError" + childOf span(0) + tags { + "component" "java-web-servlet-response" + defaultTags() + } + } + } + } + } + + def "test send with exception"() { + setup: + def ex = new Exception("some error") + def response = new TestResponse() { + @Override + void sendRedirect(String s) { + throw ex + } + } + def servlet = new AbstractHttpServlet() {} + // We need to call service so HttpServletAdvice can link the request to the response. + servlet.service(request, response) + + when: + runUnderTrace("parent") { + response.sendRedirect("") + } + + then: + def th = thrown(Exception) + th == ex + + assertTraces(1) { + trace(0, 2) { + basicSpan(it, 0, "parent", null, ex) + span(1) { + operationName "servlet.response" + resourceName "HttpServletResponse.sendRedirect" + childOf span(0) + errored true + tags { + "component" "java-web-servlet-response" + defaultTags() + errorTags(ex.class, ex.message) + } + } + } + } + } + + static class TestResponse implements HttpServletResponse { + + @Override + void addCookie(Cookie cookie) { + + } + + @Override + boolean containsHeader(String s) { + return false + } + + @Override + String encodeURL(String s) { + return null + } + + @Override + String encodeRedirectURL(String s) { + return null + } + + @Override + String encodeUrl(String s) { + return null + } + + @Override + String encodeRedirectUrl(String s) { + return null + } + + @Override + void sendError(int i, String s) throws IOException { + + } + + @Override + void sendError(int i) throws IOException { + + } + + @Override + void sendRedirect(String s) throws IOException { + + } + + @Override + void setDateHeader(String s, long l) { + + } + + @Override + void addDateHeader(String s, long l) { + + } + + @Override + void setHeader(String s, String s1) { + + } + + @Override + void addHeader(String s, String s1) { + + } + + @Override + void setIntHeader(String s, int i) { + + } + + @Override + void addIntHeader(String s, int i) { + + } + + @Override + void setStatus(int i) { + + } + + @Override + void setStatus(int i, String s) { + + } + + @Override + String getCharacterEncoding() { + return null + } + + @Override + ServletOutputStream getOutputStream() throws IOException { + return null + } + + @Override + PrintWriter getWriter() throws IOException { + return null + } + + @Override + void setContentLength(int i) { + + } + + @Override + void setContentType(String s) { + + } + + @Override + void setBufferSize(int i) { + + } + + @Override + int getBufferSize() { + return 0 + } + + @Override + void flushBuffer() throws IOException { + + } + + @Override + void resetBuffer() { + + } + + @Override + boolean isCommitted() { + return false + } + + @Override + void reset() { + + } + + @Override + void setLocale(Locale locale) { + + } + + @Override + Locale getLocale() { + return null + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletTest.groovy new file mode 100644 index 0000000000..1c5251509a --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletTest.groovy @@ -0,0 +1,125 @@ +import datadog.trace.agent.test.AgentTestRunner +import groovy.servlet.AbstractHttpServlet +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletResponse + +import static datadog.trace.agent.test.utils.TraceUtils.basicSpan +import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace + +class HttpServletTest extends AgentTestRunner { + static { + System.setProperty("dd.integration.servlet-beta.enabled", "true") + } + + def req = Mock(HttpServletRequest) { + getMethod() >> "GET" + getProtocol() >> "TEST" + } + def resp = Mock(HttpServletResponse) + + def "test service no-parent"() { + when: + servlet.service(req, resp) + + then: + assertTraces(0) {} + + where: + servlet = new TestServlet() + } + + def "test service with parent"() { + when: + runUnderTrace("parent") { + servlet.service(req, resp) + } + + then: + assertTraces(1) { + trace(0, 3) { + basicSpan(it, 0, "parent") + span(1) { + operationName "servlet.service" + resourceName "HttpServlet.service" + childOf span(0) + tags { + "component" "java-web-servlet-service" + defaultTags() + } + } + span(2) { + operationName "servlet.doGet" + resourceName "${expectedResourceName}.doGet" + childOf span(1) + tags { + "component" "java-web-servlet-service" + defaultTags() + } + } + } + } + + where: + servlet << [new TestServlet(), new TestServlet() { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) { + } + }] + + expectedResourceName = servlet.class.anonymousClass ? servlet.class.name : servlet.class.simpleName + } + + def "test service exception"() { + setup: + def ex = new Exception("some error") + def servlet = new TestServlet() { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) { + throw ex + } + } + + when: + runUnderTrace("parent") { + servlet.service(req, resp) + } + + then: + def th = thrown(Exception) + th == ex + + assertTraces(1) { + trace(0, 3) { + basicSpan(it, 0, "parent", null, ex) + span(1) { + operationName "servlet.service" + resourceName "HttpServlet.service" + childOf span(0) + errored true + tags { + "component" "java-web-servlet-service" + defaultTags() + errorTags(ex.class, ex.message) + } + } + span(2) { + operationName "servlet.doGet" + resourceName "${servlet.class.name}.doGet" + childOf span(1) + errored true + tags { + "component" "java-web-servlet-service" + defaultTags() + errorTags(ex.class, ex.message) + } + } + } + } + } + + static class TestServlet extends AbstractHttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) { + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy new file mode 100644 index 0000000000..b5a5e0f3c6 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy @@ -0,0 +1,97 @@ +import datadog.trace.agent.test.AgentTestRunner +import javax.servlet.ServletException +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletResponse + +import static datadog.trace.agent.test.utils.TraceUtils.basicSpan +import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace + +class RequestDispatcherTest extends AgentTestRunner { + static { + System.setProperty("dd.integration.servlet-beta.enabled", "true") + } + + def dispatcher = new RequestDispatcherUtils(Mock(HttpServletRequest), Mock(HttpServletResponse)) + + def "test dispatch no-parent"() { + when: + dispatcher.forward("") + dispatcher.include("") + + then: + assertTraces(0) {} + } + + def "test dispatcher #method with parent"() { + when: + runUnderTrace("parent") { + dispatcher."$method"(target) + } + + then: + assertTraces(1) { + trace(0, 2) { + basicSpan(it, 0, "parent") + span(1) { + operationName "servlet.$operation" + resourceName target + childOf span(0) + tags { + "component" "java-web-servlet-dispatcher" + defaultTags() + } + } + } + } + + where: + operation | method + "forward" | "forward" + "forward" | "forwardNamed" + "include" | "include" + "include" | "includeNamed" + + target = "test-$method" + } + + def "test dispatcher #method exception"() { + setup: + def ex = new ServletException("some error") + def dispatcher = new RequestDispatcherUtils(Mock(HttpServletRequest), Mock(HttpServletResponse), ex) + + when: + runUnderTrace("parent") { + dispatcher."$method"(target) + } + + then: + def th = thrown(ServletException) + th == ex + + assertTraces(1) { + trace(0, 2) { + basicSpan(it, 0, "parent", null, ex) + span(1) { + operationName "servlet.$operation" + resourceName target + childOf span(0) + errored true + tags { + "component" "java-web-servlet-dispatcher" + defaultTags() + errorTags(ex.class, ex.message) + } + } + } + } + + where: + operation | method + "forward" | "forward" + "forward" | "forwardNamed" + "include" | "include" + "include" | "includeNamed" + + target = "test-$method" + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherUtils.java b/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherUtils.java new file mode 100644 index 0000000000..2f08ad74ed --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherUtils.java @@ -0,0 +1,181 @@ +import java.io.IOException; +import java.io.InputStream; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.Enumeration; +import java.util.Set; +import javax.servlet.RequestDispatcher; +import javax.servlet.Servlet; +import javax.servlet.ServletContext; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; + +public class RequestDispatcherUtils { + private final ServletRequest req; + private final ServletResponse resp; + private final ServletException toThrow; + + public RequestDispatcherUtils(final ServletRequest req, final ServletResponse resp) { + this.req = req; + this.resp = resp; + toThrow = null; + } + + public RequestDispatcherUtils( + final ServletRequest req, final ServletResponse resp, final ServletException toThrow) { + this.req = req; + this.resp = resp; + this.toThrow = toThrow; + } + + /* RequestDispatcher can't be visible to groovy otherwise things break, so everything is + * encapsulated in here where groovy doesn't need to access it. + */ + + void forward(final String target) throws ServletException, IOException { + new TestContext().getRequestDispatcher(target).forward(req, resp); + } + + void include(final String target) throws ServletException, IOException { + new TestContext().getRequestDispatcher(target).include(req, resp); + } + + void forwardNamed(final String target) throws ServletException, IOException { + new TestContext().getNamedDispatcher(target).forward(req, resp); + } + + void includeNamed(final String target) throws ServletException, IOException { + new TestContext().getNamedDispatcher(target).include(req, resp); + } + + class TestContext implements ServletContext { + @Override + public ServletContext getContext(final String s) { + return null; + } + + @Override + public int getMajorVersion() { + return 0; + } + + @Override + public int getMinorVersion() { + return 0; + } + + @Override + public String getMimeType(final String s) { + return null; + } + + @Override + public Set getResourcePaths(final String s) { + return null; + } + + @Override + public URL getResource(final String s) throws MalformedURLException { + return null; + } + + @Override + public InputStream getResourceAsStream(final String s) { + return null; + } + + @Override + public RequestDispatcher getRequestDispatcher(final String s) { + return new TestDispatcher(); + } + + @Override + public RequestDispatcher getNamedDispatcher(final String s) { + return new TestDispatcher(); + } + + @Override + public Servlet getServlet(final String s) throws ServletException { + return null; + } + + @Override + public Enumeration getServlets() { + return null; + } + + @Override + public Enumeration getServletNames() { + return null; + } + + @Override + public void log(final String s) {} + + @Override + public void log(final Exception e, final String s) {} + + @Override + public void log(final String s, final Throwable throwable) {} + + @Override + public String getRealPath(final String s) { + return null; + } + + @Override + public String getServerInfo() { + return null; + } + + @Override + public String getInitParameter(final String s) { + return null; + } + + @Override + public Enumeration getInitParameterNames() { + return null; + } + + @Override + public Object getAttribute(final String s) { + return null; + } + + @Override + public Enumeration getAttributeNames() { + return null; + } + + @Override + public void setAttribute(final String s, final Object o) {} + + @Override + public void removeAttribute(final String s) {} + + @Override + public String getServletContextName() { + return null; + } + } + + class TestDispatcher implements RequestDispatcher { + @Override + public void forward(final ServletRequest servletRequest, final ServletResponse servletResponse) + throws ServletException, IOException { + if (toThrow != null) { + throw toThrow; + } + } + + @Override + public void include(final ServletRequest servletRequest, final ServletResponse servletResponse) + throws ServletException, IOException { + if (toThrow != null) { + throw toThrow; + } + } + } +} From e440eba9a19f7476f08a84949b00c2c9ad05e43e Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 10 Jan 2020 12:52:54 -0800 Subject: [PATCH 15/27] Enable Servlet request and response dispatcher instrumentation by default. Change config for Servlet Filter and Service and JDBC DataSource instrumentation so it must be explicitly enabled (won't be enabled by accident if all of servlet is enabled). --- .../jdbc/DataSourceDecorator.java | 2 +- .../jdbc/DataSourceInstrumentation.java | 2 +- .../groovy/JDBCInstrumentationTest.groovy | 3 +-- .../RequestDispatcherDecorator.java | 2 +- .../RequestDispatcherInstrumentation.java | 19 ++++++++++++------- .../servlet/filter/FilterDecorator.java | 2 +- .../servlet/filter/FilterInstrumentation.java | 2 +- .../servlet/http/HttpServletDecorator.java | 2 +- .../http/HttpServletInstrumentation.java | 2 +- .../http/HttpServletResponseDecorator.java | 2 +- .../HttpServletResponseInstrumentation.java | 7 +------ .../servlet/src/test/groovy/FilterTest.groovy | 3 ++- .../groovy/HttpServletResponseTest.groovy | 6 ++---- .../src/test/groovy/HttpServletTest.groovy | 3 ++- .../test/groovy/RequestDispatcherTest.groovy | 4 +--- 15 files changed, 29 insertions(+), 32 deletions(-) diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceDecorator.java index d4e75e1455..baee43cde0 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceDecorator.java @@ -7,7 +7,7 @@ public class DataSourceDecorator extends BaseDecorator { @Override protected String[] instrumentationNames() { - return new String[] {"jdbc-beta", "jdbc-datasource"}; + return new String[] {"jdbc-datasource"}; } @Override diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceInstrumentation.java index 43091ae449..e5930375c8 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DataSourceInstrumentation.java @@ -25,7 +25,7 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) public final class DataSourceInstrumentation extends Instrumenter.Default { public DataSourceInstrumentation() { - super("jdbc-beta", "jdbc-datasource"); + super("jdbc-datasource"); } @Override diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy index 0560e5ec0c..c1d2a9fc25 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy @@ -5,7 +5,6 @@ import datadog.trace.agent.test.AgentTestRunner import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import datadog.trace.instrumentation.api.Tags -import javax.sql.DataSource import org.apache.derby.jdbc.EmbeddedDataSource import org.apache.derby.jdbc.EmbeddedDriver import org.h2.Driver @@ -29,7 +28,7 @@ import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace class JDBCInstrumentationTest extends AgentTestRunner { static { - System.setProperty("dd.integration.jdbc-beta.enabled", "true") + System.setProperty("dd.integration.jdbc-datasource.enabled", "true") } @Shared diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java index 1e0550cfbe..b20f3c19e7 100644 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java @@ -7,7 +7,7 @@ public class RequestDispatcherDecorator extends BaseDecorator { @Override protected String[] instrumentationNames() { - return new String[] {"servlet-beta", "servlet-dispatcher"}; + return new String[] {"servlet", "servlet-dispatcher"}; } @Override diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java index 662ddc629b..86ddbd3c69 100644 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java @@ -1,5 +1,6 @@ package datadog.trace.instrumentation.servlet.dispatcher; +import static datadog.trace.agent.decorator.HttpServerDecorator.DD_SPAN_ATTRIBUTE; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static datadog.trace.instrumentation.api.AgentTracer.activateSpan; import static datadog.trace.instrumentation.api.AgentTracer.activeSpan; @@ -31,12 +32,7 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) public final class RequestDispatcherInstrumentation extends Instrumenter.Default { public RequestDispatcherInstrumentation() { - super("servlet-beta", "servlet-dispatcher"); - } - - @Override - public boolean defaultEnabled() { - return false; + super("servlet", "servlet-dispatcher"); } @Override @@ -91,15 +87,24 @@ public final class RequestDispatcherInstrumentation extends Instrumenter.Default // In case we lose context, inject trace into to the request. propagate().inject(span, request, SETTER); + // temporarily remove from request to avoid spring resource name bubbling up: + request.removeAttribute(DD_SPAN_ATTRIBUTE); + return activateSpan(span, true).setAsyncPropagation(true); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stop( - @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) { + @Advice.Enter final AgentScope scope, + @Advice.Argument(0) final ServletRequest request, + @Advice.Thrown final Throwable throwable) { if (scope == null) { return; } + + // now add it back... + request.setAttribute(DD_SPAN_ATTRIBUTE, scope.span()); + DECORATE.onError(scope, throwable); DECORATE.beforeFinish(scope); scope.close(); diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterDecorator.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterDecorator.java index c5dfc98f19..576fbe8e09 100644 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterDecorator.java +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterDecorator.java @@ -7,7 +7,7 @@ public class FilterDecorator extends BaseDecorator { @Override protected String[] instrumentationNames() { - return new String[] {"servlet-beta", "servlet-filter"}; + return new String[] {"servlet-filter"}; } @Override diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterInstrumentation.java index f94814b908..6a5a98be45 100644 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/filter/FilterInstrumentation.java @@ -27,7 +27,7 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) public final class FilterInstrumentation extends Instrumenter.Default { public FilterInstrumentation() { - super("servlet-beta", "servlet-filter"); + super("servlet-filter"); } @Override diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletDecorator.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletDecorator.java index b4a8d93324..f81a4a91d5 100644 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletDecorator.java +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletDecorator.java @@ -7,7 +7,7 @@ public class HttpServletDecorator extends BaseDecorator { @Override protected String[] instrumentationNames() { - return new String[] {"servlet-beta", "servlet-service"}; + return new String[] {"servlet-service"}; } @Override diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java index faba631452..ad4c7a3190 100644 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java @@ -32,7 +32,7 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) public final class HttpServletInstrumentation extends Instrumenter.Default { public HttpServletInstrumentation() { - super("servlet-beta", "servlet-service"); + super("servlet-service"); } @Override diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseDecorator.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseDecorator.java index 782aa99247..be986c1ad3 100644 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseDecorator.java +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseDecorator.java @@ -7,7 +7,7 @@ public class HttpServletResponseDecorator extends BaseDecorator { @Override protected String[] instrumentationNames() { - return new String[] {"servlet-beta", "servlet-response"}; + return new String[] {"servlet", "servlet-response"}; } @Override diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseInstrumentation.java index 68ae6400b0..8f8c9717ec 100644 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletResponseInstrumentation.java @@ -29,12 +29,7 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) public final class HttpServletResponseInstrumentation extends Instrumenter.Default { public HttpServletResponseInstrumentation() { - super("servlet-beta", "servlet-response"); - } - - @Override - public boolean defaultEnabled() { - return false; + super("servlet", "servlet-response"); } @Override diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/FilterTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/FilterTest.groovy index fc7ccb5aa5..9a6b53ab2a 100644 --- a/dd-java-agent/instrumentation/servlet/src/test/groovy/FilterTest.groovy +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/FilterTest.groovy @@ -1,4 +1,5 @@ import datadog.trace.agent.test.AgentTestRunner + import javax.servlet.Filter import javax.servlet.FilterChain import javax.servlet.FilterConfig @@ -11,7 +12,7 @@ import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace class FilterTest extends AgentTestRunner { static { - System.setProperty("dd.integration.servlet-beta.enabled", "true") + System.setProperty("dd.integration.servlet-filter.enabled", "true") } def "test doFilter no-parent"() { diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy index 9073a8044b..1bc6f2612e 100644 --- a/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy @@ -1,18 +1,16 @@ import datadog.trace.agent.test.AgentTestRunner import groovy.servlet.AbstractHttpServlet +import spock.lang.Subject + import javax.servlet.ServletOutputStream import javax.servlet.http.Cookie import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse -import spock.lang.Subject import static datadog.trace.agent.test.utils.TraceUtils.basicSpan import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace class HttpServletResponseTest extends AgentTestRunner { - static { - System.setProperty("dd.integration.servlet-beta.enabled", "true") - } @Subject def response = new TestResponse() diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletTest.groovy index 1c5251509a..6df40c49a5 100644 --- a/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletTest.groovy +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletTest.groovy @@ -1,5 +1,6 @@ import datadog.trace.agent.test.AgentTestRunner import groovy.servlet.AbstractHttpServlet + import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse @@ -8,7 +9,7 @@ import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace class HttpServletTest extends AgentTestRunner { static { - System.setProperty("dd.integration.servlet-beta.enabled", "true") + System.setProperty("dd.integration.servlet-service.enabled", "true") } def req = Mock(HttpServletRequest) { diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy index b5a5e0f3c6..f352051753 100644 --- a/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy @@ -1,4 +1,5 @@ import datadog.trace.agent.test.AgentTestRunner + import javax.servlet.ServletException import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse @@ -7,9 +8,6 @@ import static datadog.trace.agent.test.utils.TraceUtils.basicSpan import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace class RequestDispatcherTest extends AgentTestRunner { - static { - System.setProperty("dd.integration.servlet-beta.enabled", "true") - } def dispatcher = new RequestDispatcherUtils(Mock(HttpServletRequest), Mock(HttpServletResponse)) From 49425e9963decb7e33de7cf9083a594a8746d553 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 13 Jan 2020 10:48:35 -0800 Subject: [PATCH 16/27] Move request-response linking to main servlet advice servlet-service is off by default, so we can't rely on it being called there. --- .../servlet2/Servlet2Advice.java | 9 +++++++-- .../servlet2/Servlet2Instrumentation.java | 6 ++++++ .../HttpServletRequestExtractAdapter.java | 3 +-- .../servlet3/Servlet3Advice.java | 10 +++++++++- .../servlet3/Servlet3Instrumentation.java | 6 ++++++ .../instrumentation/servlet/servlet.gradle | 8 ++++++++ .../ServletContextInstrumentation.java | 7 +------ .../http/HttpServletInstrumentation.java | 17 +---------------- .../test/groovy/HttpServletResponseTest.groovy | 13 +++++++++++-- 9 files changed, 50 insertions(+), 29 deletions(-) diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java index 87735d6a17..2264d9945b 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java @@ -9,6 +9,7 @@ import static datadog.trace.instrumentation.servlet2.HttpServletRequestExtractAd import static datadog.trace.instrumentation.servlet2.Servlet2Decorator.DECORATE; import datadog.trace.api.DDTags; +import datadog.trace.bootstrap.InstrumentationContext; import datadog.trace.instrumentation.api.AgentScope; import datadog.trace.instrumentation.api.AgentSpan; import datadog.trace.instrumentation.api.Tags; @@ -36,12 +37,16 @@ public class Servlet2Advice { return null; } + final HttpServletRequest httpServletRequest = (HttpServletRequest) request; + if (response instanceof HttpServletResponse) { + // For use by HttpServletResponseInstrumentation: + InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class) + .put((HttpServletResponse) response, httpServletRequest); + response = new StatusSavingHttpServletResponseWrapper((HttpServletResponse) response); } - final HttpServletRequest httpServletRequest = (HttpServletRequest) request; - final AgentSpan.Context extractedContext = propagate().extract(httpServletRequest, GETTER); final AgentSpan span = diff --git a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java index 5f309738d4..8490d7a237 100644 --- a/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java @@ -49,6 +49,12 @@ public final class Servlet2Instrumentation extends Instrumenter.Default { named("javax.servlet.FilterChain").or(named("javax.servlet.http.HttpServlet")))); } + @Override + public Map contextStore() { + return singletonMap( + "javax.servlet.http.HttpServletResponse", "javax.servlet.http.HttpServletRequest"); + } + /** * Here we are instrumenting the public method for HttpServlet. This should ensure that this * advice is always called before HttpServletInstrumentation which is instrumenting the protected diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServletRequestExtractAdapter.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServletRequestExtractAdapter.java index e8b1fff393..3b904d12ff 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServletRequestExtractAdapter.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/HttpServletRequestExtractAdapter.java @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.servlet3; import datadog.trace.instrumentation.api.AgentPropagation; -import java.util.ArrayList; import java.util.Collections; import java.util.List; import javax.servlet.http.HttpServletRequest; @@ -14,7 +13,7 @@ public class HttpServletRequestExtractAdapter @Override public List keys(final HttpServletRequest carrier) { - final ArrayList keys = Collections.list(carrier.getHeaderNames()); + final List keys = Collections.list(carrier.getHeaderNames()); keys.addAll(Collections.list(carrier.getAttributeNames())); return keys; } diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java index dbc1a4419d..407520288a 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Advice.java @@ -9,6 +9,7 @@ import static datadog.trace.instrumentation.servlet3.HttpServletRequestExtractAd import static datadog.trace.instrumentation.servlet3.Servlet3Decorator.DECORATE; import datadog.trace.api.DDTags; +import datadog.trace.bootstrap.InstrumentationContext; import datadog.trace.instrumentation.api.AgentScope; import datadog.trace.instrumentation.api.AgentSpan; import datadog.trace.instrumentation.api.Tags; @@ -24,7 +25,10 @@ public class Servlet3Advice { @Advice.OnMethodEnter(suppress = Throwable.class) public static AgentScope onEnter( - @Advice.This final Object servlet, @Advice.Argument(0) final ServletRequest request) { + @Advice.This final Object servlet, + @Advice.Argument(0) final ServletRequest request, + @Advice.Argument(1) final ServletResponse response) { + final boolean hasActiveTrace = activeSpan() != null; final boolean hasServletTrace = request.getAttribute(DD_SPAN_ATTRIBUTE) instanceof AgentSpan; final boolean invalidRequest = !(request instanceof HttpServletRequest); @@ -35,6 +39,10 @@ public class Servlet3Advice { final HttpServletRequest httpServletRequest = (HttpServletRequest) request; + // For use by HttpServletResponseInstrumentation: + InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class) + .put((HttpServletResponse) response, httpServletRequest); + final AgentSpan.Context extractedContext = propagate().extract(httpServletRequest, GETTER); final AgentSpan span = diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Instrumentation.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Instrumentation.java index 0fb9455ac6..42c982b169 100644 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Instrumentation.java +++ b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/Servlet3Instrumentation.java @@ -41,6 +41,12 @@ public final class Servlet3Instrumentation extends Instrumenter.Default { named("javax.servlet.FilterChain").or(named("javax.servlet.http.HttpServlet")))); } + @Override + public Map contextStore() { + return singletonMap( + "javax.servlet.http.HttpServletResponse", "javax.servlet.http.HttpServletRequest"); + } + /** * Here we are instrumenting the public method for HttpServlet. This should ensure that this * advice is always called before HttpServletInstrumentation which is instrumenting the protected diff --git a/dd-java-agent/instrumentation/servlet/servlet.gradle b/dd-java-agent/instrumentation/servlet/servlet.gradle index 1f4583dd52..50f9d1e4b1 100644 --- a/dd-java-agent/instrumentation/servlet/servlet.gradle +++ b/dd-java-agent/instrumentation/servlet/servlet.gradle @@ -18,4 +18,12 @@ dependencies { compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.3' testCompile group: 'javax.servlet', name: 'servlet-api', version: '2.3' + + // servlet request instrumentation required for linking request to response. + testCompile project(':dd-java-agent:instrumentation:servlet:request-2') + + // Don't want to conflict with jetty from the test server. + testCompile(project(':dd-java-agent:testing')) { + exclude group: 'org.eclipse.jetty', module: 'jetty-server' + } } diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java index 3426135c6b..9c3df78076 100644 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java @@ -22,12 +22,7 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) public final class ServletContextInstrumentation extends Instrumenter.Default { public ServletContextInstrumentation() { - super("servlet-beta", "servlet-dispatcher"); - } - - @Override - public boolean defaultEnabled() { - return false; + super("servlet", "servlet-dispatcher"); } @Override diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java index ad4c7a3190..dbcfcf488e 100644 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletInstrumentation.java @@ -17,13 +17,10 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.api.DDTags; -import datadog.trace.bootstrap.InstrumentationContext; import datadog.trace.instrumentation.api.AgentScope; import datadog.trace.instrumentation.api.AgentSpan; import java.lang.reflect.Method; import java.util.Map; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; @@ -67,22 +64,10 @@ public final class HttpServletInstrumentation extends Instrumenter.Default { HttpServletAdvice.class.getName()); } - @Override - public Map contextStore() { - return singletonMap( - "javax.servlet.http.HttpServletResponse", "javax.servlet.http.HttpServletRequest"); - } - public static class HttpServletAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static AgentScope start( - @Advice.Origin final Method method, - @Advice.Argument(0) final HttpServletRequest request, - @Advice.Argument(1) final HttpServletResponse response) { - // For use by HttpServletResponseInstrumentation: - InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class) - .put(response, request); + public static AgentScope start(@Advice.Origin final Method method) { if (activeSpan() == null) { // Don't want to generate a new top-level span diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy index 1bc6f2612e..f82e5904ac 100644 --- a/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy @@ -3,12 +3,15 @@ import groovy.servlet.AbstractHttpServlet import spock.lang.Subject import javax.servlet.ServletOutputStream +import javax.servlet.ServletRequest +import javax.servlet.ServletResponse import javax.servlet.http.Cookie import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse import static datadog.trace.agent.test.utils.TraceUtils.basicSpan import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace +import static java.util.Collections.emptyEnumeration class HttpServletResponseTest extends AgentTestRunner { @@ -17,12 +20,16 @@ class HttpServletResponseTest extends AgentTestRunner { def request = Mock(HttpServletRequest) { getMethod() >> "GET" getProtocol() >> "TEST" + getHeaderNames() >> emptyEnumeration() + getAttributeNames() >> emptyEnumeration() } def setup() { def servlet = new AbstractHttpServlet() {} // We need to call service so HttpServletAdvice can link the request to the response. - servlet.service(request, response) + servlet.service((ServletRequest) request, (ServletResponse) response) + assert response.__datadogContext$javax$servlet$http$HttpServletResponse != null + TEST_WRITER.clear() } def "test send no-parent"() { @@ -89,7 +96,9 @@ class HttpServletResponseTest extends AgentTestRunner { } def servlet = new AbstractHttpServlet() {} // We need to call service so HttpServletAdvice can link the request to the response. - servlet.service(request, response) + servlet.service((ServletRequest) request, (ServletResponse) response) + assert response.__datadogContext$javax$servlet$http$HttpServletResponse != null + TEST_WRITER.clear() when: runUnderTrace("parent") { From 13b84416b954ca4fda9300248f7b19ea19f0508e Mon Sep 17 00:00:00 2001 From: Bruce Yu Date: Wed, 27 Nov 2019 16:37:17 -0500 Subject: [PATCH 17/27] Blacklisting headers with -bin suffixes in GrpcExtractAdapter --- .../grpc/server/GrpcExtractAdapter.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/GrpcExtractAdapter.java b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/GrpcExtractAdapter.java index 67a5598bda..d655ba2f48 100644 --- a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/GrpcExtractAdapter.java +++ b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/GrpcExtractAdapter.java @@ -2,6 +2,8 @@ package datadog.trace.instrumentation.grpc.server; import datadog.trace.instrumentation.api.AgentPropagation; import io.grpc.Metadata; +import java.util.ArrayList; +import java.util.List; public final class GrpcExtractAdapter implements AgentPropagation.Getter { @@ -9,7 +11,15 @@ public final class GrpcExtractAdapter implements AgentPropagation.Getter keys(final Metadata carrier) { - return carrier.keys(); + List keys = new ArrayList<>(); + + for (String key : carrier.keys()) { + if (!key.endsWith(Metadata.BINARY_HEADER_SUFFIX)){ + keys.add(key); + } + } + + return keys; } @Override From 25397fd1284263ed9b84193540e623a66a2128c8 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 13 Jan 2020 11:16:49 -0800 Subject: [PATCH 18/27] Fix formatting and add test. --- .../grpc/server/GrpcExtractAdapter.java | 2 +- .../grpc-1.5/src/test/groovy/GrpcTest.groovy | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/GrpcExtractAdapter.java b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/GrpcExtractAdapter.java index d655ba2f48..cd78c3a1e3 100644 --- a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/GrpcExtractAdapter.java +++ b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/GrpcExtractAdapter.java @@ -14,7 +14,7 @@ public final class GrpcExtractAdapter implements AgentPropagation.Getter keys = new ArrayList<>(); for (String key : carrier.keys()) { - if (!key.endsWith(Metadata.BINARY_HEADER_SUFFIX)){ + if (!key.endsWith(Metadata.BINARY_HEADER_SUFFIX)) { keys.add(key); } } diff --git a/dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcTest.groovy b/dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcTest.groovy index 090e934bfb..9d94b0cd25 100644 --- a/dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcTest.groovy +++ b/dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcTest.groovy @@ -1,10 +1,12 @@ import datadog.trace.agent.test.AgentTestRunner import datadog.trace.api.DDSpanTypes import datadog.trace.instrumentation.api.Tags +import datadog.trace.instrumentation.grpc.server.GrpcExtractAdapter import example.GreeterGrpc import example.Helloworld import io.grpc.BindableService import io.grpc.ManagedChannel +import io.grpc.Metadata import io.grpc.Server import io.grpc.Status import io.grpc.StatusRuntimeException @@ -284,4 +286,17 @@ class GrpcTest extends AgentTestRunner { "Status - description" | Status.PERMISSION_DENIED.withDescription("some description") "StatusRuntime - description" | Status.UNIMPLEMENTED.withDescription("some description") } + + def "skip binary headers"() { + setup: + def meta = new Metadata() + meta.put(Metadata.Key. of("test", Metadata.ASCII_STRING_MARSHALLER), "val") + meta.put(Metadata.Key. of("test-bin", Metadata.BINARY_BYTE_MARSHALLER), "bin-val".bytes) + + when: + def keys = GrpcExtractAdapter.GETTER.keys(meta) + + then: + keys == ["test"] + } } From 8b5d89501b756b61f32ef101075b7a97c4b8c816 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 13 Jan 2020 14:19:03 -0500 Subject: [PATCH 19/27] Upgrade jmxfetch to latest version --- dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle b/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle index bee388bec6..40cd4bab16 100644 --- a/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle +++ b/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle @@ -4,10 +4,7 @@ plugins { apply from: "${rootDir}/gradle/java.gradle" dependencies { - compile('com.datadoghq:jmxfetch:0.32.1') { - exclude group: 'org.slf4j', module: 'slf4j-log4j12' - exclude group: 'log4j', module: 'log4j' - } + compile('com.datadoghq:jmxfetch:0.34.0') compile deps.slf4j compile project(':dd-trace-api') } From d55d13e2397143c05575629ead260f5945df2675 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 13 Jan 2020 14:45:19 -0500 Subject: [PATCH 20/27] Exclude some dependencies from jmxfetch --- dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle b/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle index 40cd4bab16..4596bc32b0 100644 --- a/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle +++ b/dd-java-agent/agent-jmxfetch/agent-jmxfetch.gradle @@ -4,7 +4,11 @@ plugins { apply from: "${rootDir}/gradle/java.gradle" dependencies { - compile('com.datadoghq:jmxfetch:0.34.0') + compile('com.datadoghq:jmxfetch:0.34.0') { + exclude group: 'org.apache.logging.log4j', module: 'log4j-slf4j-impl' + exclude group: 'org.apache.logging.log4j', module: 'log4j-core' + exclude group: 'org.slf4j', module: 'slf4j-api' + } compile deps.slf4j compile project(':dd-trace-api') } From d96d8e3c71c9b914dfedd1b76b07a4cc3bd2b5f0 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 13 Jan 2020 14:46:06 -0500 Subject: [PATCH 21/27] Upgrade gradle --- gradle/wrapper/gradle-wrapper.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 562e2c8841..1ba7206f88 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,5 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-6.0-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-6.0.1-all.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists From dcc57267971a451c0f816ce2a76ef8aa7d3d95ab Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 13 Jan 2020 15:05:45 -0500 Subject: [PATCH 22/27] Fix jmxfetch log level and log location config Looks like this got lost in b505c6054382b97dcfc5c9ac1c73fb19fe91d42a --- .../src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java index 384d366369..b23a4ff853 100644 --- a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java +++ b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java @@ -84,7 +84,9 @@ public class JMXFetch { .metricConfigFiles(metricsConfigs) .refreshBeansPeriod(refreshBeansPeriod) .globalTags(globalTags) - .reporter(ReporterFactory.getReporter(reporter)); + .reporter(ReporterFactory.getReporter(reporter)) + .logLocation(logLocation) + .logLevel(logLevel); if (checkPeriod != null) { configBuilder.checkPeriod(checkPeriod); From a0f0d45c746aa0b03848e57540b5ffeb2393a0d1 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 13 Jan 2020 12:10:57 -0800 Subject: [PATCH 23/27] Adjust version compatibility to only 2.x They recently released 3.0.0 which is not compatible with our instrumentation. --- .../instrumentation/couchbase-2.0/couchbase-2.0.gradle | 2 +- .../instrumentation/couchbase-2.6/couchbase-2.6.gradle | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/couchbase-2.0/couchbase-2.0.gradle b/dd-java-agent/instrumentation/couchbase-2.0/couchbase-2.0.gradle index 9688f69594..3011663337 100644 --- a/dd-java-agent/instrumentation/couchbase-2.0/couchbase-2.0.gradle +++ b/dd-java-agent/instrumentation/couchbase-2.0/couchbase-2.0.gradle @@ -29,7 +29,7 @@ muzzle { pass { group = 'com.couchbase.client' module = 'java-client' - versions = "[2.7.9,)" + versions = "[2.7.9,3.0.0)" } fail { group = 'com.couchbase.client' diff --git a/dd-java-agent/instrumentation/couchbase-2.6/couchbase-2.6.gradle b/dd-java-agent/instrumentation/couchbase-2.6/couchbase-2.6.gradle index d7b82e2aa5..6de1660a7f 100644 --- a/dd-java-agent/instrumentation/couchbase-2.6/couchbase-2.6.gradle +++ b/dd-java-agent/instrumentation/couchbase-2.6/couchbase-2.6.gradle @@ -34,7 +34,7 @@ muzzle { pass { group = 'com.couchbase.client' module = 'java-client' - versions = "[2.7.9,)" + versions = "[2.7.9,3.0.0)" } fail { group = 'com.couchbase.client' From cb7faee63ff345059fcdd8fa05e3a46dca913439 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 13 Jan 2020 15:23:04 -0500 Subject: [PATCH 24/27] Remove logging config from jmxfetch setup It already uses slf4j --- .../java/datadog/trace/agent/jmxfetch/JMXFetch.java | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java index b23a4ff853..8b08ed8929 100644 --- a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java +++ b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java @@ -55,11 +55,9 @@ public class JMXFetch { final Integer refreshBeansPeriod = config.getJmxFetchRefreshBeansPeriod(); final Map globalTags = config.getMergedJmxTags(); final String reporter = getReporter(config); - final String logLocation = getLogLocation(); - final String logLevel = getLogLevel(); log.info( - "JMXFetch config: {} {} {} {} {} {} {} {} {} {}", + "JMXFetch config: {} {} {} {} {} {} {} {}", jmxFetchConfigDir, jmxFetchConfigs, internalMetricsConfigs, @@ -67,9 +65,7 @@ public class JMXFetch { checkPeriod, refreshBeansPeriod, globalTags, - reporter, - logLocation, - logLevel); + reporter); final AppConfig.AppConfigBuilder configBuilder = AppConfig.builder() @@ -84,9 +80,7 @@ public class JMXFetch { .metricConfigFiles(metricsConfigs) .refreshBeansPeriod(refreshBeansPeriod) .globalTags(globalTags) - .reporter(ReporterFactory.getReporter(reporter)) - .logLocation(logLocation) - .logLevel(logLevel); + .reporter(ReporterFactory.getReporter(reporter)); if (checkPeriod != null) { configBuilder.checkPeriod(checkPeriod); From 77cdd210c684a89f0ca0a1ad634fc3490b555497 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 13 Jan 2020 12:31:38 -0800 Subject: [PATCH 25/27] Also update latestDepTest ranges. --- .../instrumentation/couchbase-2.0/couchbase-2.0.gradle | 2 +- .../instrumentation/couchbase-2.6/couchbase-2.6.gradle | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/couchbase-2.0/couchbase-2.0.gradle b/dd-java-agent/instrumentation/couchbase-2.0/couchbase-2.0.gradle index 3011663337..c2b8259498 100644 --- a/dd-java-agent/instrumentation/couchbase-2.0/couchbase-2.0.gradle +++ b/dd-java-agent/instrumentation/couchbase-2.0/couchbase-2.0.gradle @@ -59,6 +59,6 @@ dependencies { testCompile group: 'com.couchbase.client', name: 'java-client', version: '2.5.0' latestDepTestCompile group: 'org.springframework.data', name: 'spring-data-couchbase', version: '3.+' - latestDepTestCompile group: 'com.couchbase.client', name: 'java-client', version: '2.6+' + latestDepTestCompile group: 'com.couchbase.client', name: 'java-client', version: '2.+' latestDepTestCompile group: 'com.couchbase.client', name: 'encryption', version: '+' } diff --git a/dd-java-agent/instrumentation/couchbase-2.6/couchbase-2.6.gradle b/dd-java-agent/instrumentation/couchbase-2.6/couchbase-2.6.gradle index 6de1660a7f..1da26d145a 100644 --- a/dd-java-agent/instrumentation/couchbase-2.6/couchbase-2.6.gradle +++ b/dd-java-agent/instrumentation/couchbase-2.6/couchbase-2.6.gradle @@ -58,6 +58,6 @@ dependencies { testCompile group: 'com.couchbase.client', name: 'encryption', version: '1.0.0' latestDepTestCompile group: 'org.springframework.data', name: 'spring-data-couchbase', version: '3.1+' - latestDepTestCompile group: 'com.couchbase.client', name: 'java-client', version: '2.6+' + latestDepTestCompile group: 'com.couchbase.client', name: 'java-client', version: '2.+' latestDepTestCompile group: 'com.couchbase.client', name: 'encryption', version: '+' } From 5fe26f004b0123e974bd05cb2152c262539dbf8d Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 13 Jan 2020 12:52:41 -0800 Subject: [PATCH 26/27] Remove more declared method filtering for optimization Same logic that was applied in #1169 --- .../trace/agent/tooling/ByteBuddyElementMatchers.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java index 6dbaa8e594..b94a19e4ab 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ByteBuddyElementMatchers.java @@ -328,9 +328,8 @@ public class ByteBuddyElementMatchers { final Set checkedInterfaces = new HashSet<>(); while (declaringType != null) { - for (final MethodDescription methodDescription : - declaringType.getDeclaredMethods().filter(signatureMatcher)) { - if (matcher.matches(methodDescription)) { + for (final MethodDescription methodDescription : declaringType.getDeclaredMethods()) { + if (signatureMatcher.matches(methodDescription) && matcher.matches(methodDescription)) { return true; } } @@ -349,9 +348,8 @@ public class ByteBuddyElementMatchers { for (final TypeDefinition type : interfaces) { if (!checkedInterfaces.contains(type)) { checkedInterfaces.add(type); - for (final MethodDescription methodDescription : - type.getDeclaredMethods().filter(signatureMatcher)) { - if (matcher.matches(methodDescription)) { + for (final MethodDescription methodDescription : type.getDeclaredMethods()) { + if (signatureMatcher.matches(methodDescription) && matcher.matches(methodDescription)) { return true; } } From 7fabcaaecbb4cec0e6124f3e2ca1543fda672515 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 14 Jan 2020 10:48:14 -0800 Subject: [PATCH 27/27] Version 0.41.0 --- dd-trace-java.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dd-trace-java.gradle b/dd-trace-java.gradle index 9bd8059084..96b93f05b7 100644 --- a/dd-trace-java.gradle +++ b/dd-trace-java.gradle @@ -16,7 +16,7 @@ def isCI = System.getenv("CI") != null allprojects { group = 'com.datadoghq' - version = '0.41.0-SNAPSHOT' + version = '0.41.0' if (isCI) { buildDir = "${rootDir}/workspace/${projectDir.path.replace(rootDir.path, '')}/build/"