Make JMS more resilent to property failure
Combine into single module and add test.
Replace classLoaderMatcher with muzzle check.
Thanks for the tip
DELETE
dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageConsumerInstrumentation.java
-package datadog.trace.instrumentation.jms1;
-
-import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses;
-import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName;
-import static io.opentracing.log.Fields.ERROR_OBJECT;
-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.takesArguments;
-
-import com.google.auto.service.AutoService;
-import datadog.trace.agent.tooling.Instrumenter;
-import datadog.trace.api.DDSpanTypes;
-import datadog.trace.api.DDTags;
-import datadog.trace.instrumentation.jms.util.MessagePropertyTextMap;
-import io.opentracing.Scope;
-import io.opentracing.Span;
-import io.opentracing.SpanContext;
-import io.opentracing.Tracer;
-import io.opentracing.propagation.Format;
-import io.opentracing.tag.Tags;
-import io.opentracing.util.GlobalTracer;
-import java.lang.reflect.Method;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.concurrent.TimeUnit;
-import javax.jms.Message;
-import javax.jms.MessageConsumer;
-import net.bytebuddy.asm.Advice;
-import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.matcher.ElementMatcher;
-
-@AutoService(Instrumenter.class)
-public final class JMS1MessageConsumerInstrumentation extends Instrumenter.Default {
- public static final String[] JMS1_HELPER_CLASS_NAMES =
- new String[] {
- "datadog.trace.instrumentation.jms.util.JmsUtil",
- "datadog.trace.instrumentation.jms.util.MessagePropertyTextMap"
- };
-
- public JMS1MessageConsumerInstrumentation() {
- super("jms", "jms-1");
- }
-
- @Override
- public ElementMatcher<TypeDescription> typeMatcher() {
- return not(isInterface()).and(safeHasSuperType(named("javax.jms.MessageConsumer")));
- }
-
- @Override
- public ElementMatcher<ClassLoader> classLoaderMatcher() {
- return not(classLoaderHasClasses("javax.jms.JMSContext", "javax.jms.CompletionListener"));
- }
-
- @Override
- public String[] helperClassNames() {
- return JMS1_HELPER_CLASS_NAMES;
- }
-
- @Override
- public Map<ElementMatcher, String> transformers() {
- final Map<ElementMatcher, String> transformers = new HashMap<>();
- transformers.put(
- named("receive").and(takesArguments(0).or(takesArguments(1))).and(isPublic()),
- ConsumerAdvice.class.getName());
- transformers.put(
- named("receiveNoWait").and(takesArguments(0)).and(isPublic()),
- ConsumerAdvice.class.getName());
- return transformers;
- }
-
- public static class ConsumerAdvice {
-
- @Advice.OnMethodEnter
- public static long startSpan() {
- return System.currentTimeMillis();
- }
-
- @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
- public static void stopSpan(
- @Advice.This final MessageConsumer consumer,
- @Advice.Enter final long startTime,
- @Advice.Origin final Method method,
- @Advice.Return final Message message,
- @Advice.Thrown final Throwable throwable) {
- Tracer.SpanBuilder spanBuilder =
- GlobalTracer.get()
- .buildSpan("jms.consume")
- .withTag(DDTags.SERVICE_NAME, "jms")
- .withTag(DDTags.SPAN_TYPE, DDSpanTypes.MESSAGE_CONSUMER)
- .withTag(Tags.COMPONENT.getKey(), "jms1")
- .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CONSUMER)
- .withTag("span.origin.type", consumer.getClass().getName())
- .withStartTimestamp(TimeUnit.MILLISECONDS.toMicros(startTime));
-
- if (message == null) {
- spanBuilder = spanBuilder.withTag(DDTags.RESOURCE_NAME, "JMS " + method.getName());
- } else {
- spanBuilder =
- spanBuilder.withTag(
- DDTags.RESOURCE_NAME, "Consumed from " + toResourceName(message, null));
-
- final SpanContext extractedContext =
- GlobalTracer.get()
- .extract(Format.Builtin.TEXT_MAP, new MessagePropertyTextMap(message));
- if (extractedContext != null) {
- spanBuilder = spanBuilder.asChildOf(extractedContext);
- }
- }
-
- final Scope scope = spanBuilder.startActive(true);
- final Span span = scope.span();
-
- if (throwable != null) {
- Tags.ERROR.set(span, Boolean.TRUE);
- span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
- }
-
- scope.close();
- }
- }
-}

DELETE
dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageListenerInstrumentation.java
-package datadog.trace.instrumentation.jms1;
-
-import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses;
-import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName;
-import static io.opentracing.log.Fields.ERROR_OBJECT;
-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.DDSpanTypes;
-import datadog.trace.api.DDTags;
-import datadog.trace.context.TraceScope;
-import datadog.trace.instrumentation.jms.util.MessagePropertyTextMap;
-import io.opentracing.Scope;
-import io.opentracing.Span;
-import io.opentracing.SpanContext;
-import io.opentracing.propagation.Format;
-import io.opentracing.tag.Tags;
-import io.opentracing.util.GlobalTracer;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-import javax.jms.Message;
-import javax.jms.MessageListener;
-import net.bytebuddy.asm.Advice;
-import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.matcher.ElementMatcher;
-
-@AutoService(Instrumenter.class)
-public final class JMS1MessageListenerInstrumentation extends Instrumenter.Default {
-
- public JMS1MessageListenerInstrumentation() {
- super("jms", "jms-1");
- }
-
- @Override
- public ElementMatcher<TypeDescription> typeMatcher() {
- return not(isInterface()).and(safeHasSuperType(named("javax.jms.MessageListener")));
- }
-
- @Override
- public ElementMatcher<ClassLoader> classLoaderMatcher() {
- return not(classLoaderHasClasses("javax.jms.JMSContext", "javax.jms.CompletionListener"));
- }
-
- @Override
- public String[] helperClassNames() {
- return JMS1MessageConsumerInstrumentation.JMS1_HELPER_CLASS_NAMES;
- }
-
- @Override
- public Map<ElementMatcher, String> transformers() {
- final Map<ElementMatcher, String> transformers = new HashMap<>();
- transformers.put(
- named("onMessage").and(takesArgument(0, named("javax.jms.Message"))).and(isPublic()),
- MessageListenerAdvice.class.getName());
- return transformers;
- }
-
- public static class MessageListenerAdvice {
-
- @Advice.OnMethodEnter(suppress = Throwable.class)
- public static Scope startSpan(
- @Advice.Argument(0) final Message message, @Advice.This final MessageListener listener) {
-
- final SpanContext extractedContext =
- GlobalTracer.get().extract(Format.Builtin.TEXT_MAP, new MessagePropertyTextMap(message));
-
- final Scope scope =
- GlobalTracer.get()
- .buildSpan("jms.onMessage")
- .asChildOf(extractedContext)
- .withTag(DDTags.SERVICE_NAME, "jms")
- .withTag(DDTags.SPAN_TYPE, DDSpanTypes.MESSAGE_CONSUMER)
- .withTag(DDTags.RESOURCE_NAME, "Received from " + toResourceName(message, null))
- .withTag(Tags.COMPONENT.getKey(), "jms1")
- .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CONSUMER)
- .withTag("span.origin.type", listener.getClass().getName())
- .startActive(true);
-
- if (scope instanceof TraceScope) {
- ((TraceScope) scope).setAsyncPropagation(true);
- }
-
- return scope;
- }
-
- @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
- public static void stopSpan(
- @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) {
-
- if (scope != null) {
- if (throwable != null) {
- final Span span = scope.span();
- Tags.ERROR.set(span, Boolean.TRUE);
- span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
- }
- scope.close();
- }
- }
- }
-}

DELETE
dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms1/JMS1MessageProducerInstrumentation.java
-package datadog.trace.instrumentation.jms1;
-
-import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses;
-import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName;
-import static io.opentracing.log.Fields.ERROR_OBJECT;
-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.DDSpanTypes;
-import datadog.trace.api.DDTags;
-import datadog.trace.instrumentation.jms.util.MessagePropertyTextMap;
-import io.opentracing.Scope;
-import io.opentracing.Span;
-import io.opentracing.propagation.Format;
-import io.opentracing.tag.Tags;
-import io.opentracing.util.GlobalTracer;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-import javax.jms.Destination;
-import javax.jms.JMSException;
-import javax.jms.Message;
-import javax.jms.MessageProducer;
-import net.bytebuddy.asm.Advice;
-import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.matcher.ElementMatcher;
-
-@AutoService(Instrumenter.class)
-public final class JMS1MessageProducerInstrumentation extends Instrumenter.Default {
-
- public JMS1MessageProducerInstrumentation() {
- super("jms", "jms-1");
- }
-
- @Override
- public ElementMatcher<TypeDescription> typeMatcher() {
- return not(isInterface()).and(safeHasSuperType(named("javax.jms.MessageProducer")));
- }
-
- @Override
- public ElementMatcher<ClassLoader> classLoaderMatcher() {
- return not(classLoaderHasClasses("javax.jms.JMSContext", "javax.jms.CompletionListener"));
- }
-
- @Override
- public String[] helperClassNames() {
- return JMS1MessageConsumerInstrumentation.JMS1_HELPER_CLASS_NAMES;
- }
-
- @Override
- public Map<ElementMatcher, String> transformers() {
- final Map<ElementMatcher, String> transformers = new HashMap<>();
- transformers.put(
- named("send").and(takesArgument(0, named("javax.jms.Message"))).and(isPublic()),
- ProducerAdvice.class.getName());
- transformers.put(
- named("send")
- .and(takesArgument(0, named("javax.jms.Destination")))
- .and(takesArgument(1, named("javax.jms.Message")))
- .and(isPublic()),
- ProducerWithDestinationAdvice.class.getName());
- return transformers;
- }
-
- public static class ProducerAdvice {
-
- @Advice.OnMethodEnter(suppress = Throwable.class)
- public static Scope startSpan(
- @Advice.Argument(0) final Message message, @Advice.This final MessageProducer producer) {
- Destination defaultDestination;
- try {
- defaultDestination = producer.getDestination();
- } catch (final JMSException e) {
- defaultDestination = null;
- }
- final Scope scope =
- GlobalTracer.get()
- .buildSpan("jms.produce")
- .withTag(DDTags.SERVICE_NAME, "jms")
- .withTag(
- DDTags.RESOURCE_NAME,
- "Produced for " + toResourceName(message, defaultDestination))
- .withTag(Tags.COMPONENT.getKey(), "jms1")
- .withTag(DDTags.SPAN_TYPE, DDSpanTypes.MESSAGE_PRODUCER)
- .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_PRODUCER)
- .withTag("span.origin.type", producer.getClass().getName())
- .startActive(true);
-
- GlobalTracer.get()
- .inject(
- scope.span().context(), Format.Builtin.TEXT_MAP, new MessagePropertyTextMap(message));
-
- return scope;
- }
-
- @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
- public static void stopSpan(
- @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) {
-
- if (scope != null) {
- if (throwable != null) {
- final Span span = scope.span();
- Tags.ERROR.set(span, Boolean.TRUE);
- span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
- }
- scope.close();
- }
- }
- }
-
- public static class ProducerWithDestinationAdvice {
-
- @Advice.OnMethodEnter(suppress = Throwable.class)
- public static Scope startSpan(
- @Advice.Argument(0) final Destination destination,
- @Advice.Argument(1) final Message message,
- @Advice.This final MessageProducer producer) {
- final Scope scope =
- GlobalTracer.get()
- .buildSpan("jms.produce")
- .withTag(DDTags.SERVICE_NAME, "jms")
- .withTag(DDTags.SPAN_TYPE, DDSpanTypes.MESSAGE_PRODUCER)
- .withTag(DDTags.RESOURCE_NAME, "Produced for " + toResourceName(message, destination))
- .withTag(Tags.COMPONENT.getKey(), "jms1")
- .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_PRODUCER)
- .withTag("span.origin.type", producer.getClass().getName())
- .startActive(true);
-
- GlobalTracer.get()
- .inject(
- scope.span().context(), Format.Builtin.TEXT_MAP, new MessagePropertyTextMap(message));
-
- return scope;
- }
-
- @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
- public static void stopSpan(
- @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) {
-
- if (scope != null) {
- if (throwable != null) {
- final Span span = scope.span();
- Tags.ERROR.set(span, Boolean.TRUE);
- span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
- }
- scope.close();
- }
- }
- }
-}

DELETE
dd-java-agent/instrumentation/jms-2/jms-2.gradle
-apply plugin: 'version-scan'
-
-versionScan {
- group = "javax.jms"
- module = "javax.jms-api"
- versions = "[2,)"
- legacyModule = "jms-api"
- verifyPresent = [
- "javax.jms.JMSContext" : null,
- "javax.jms.CompletionListener": null,
- ]
-}
-
-apply from: "${rootDir}/gradle/java.gradle"
-
-dependencies {
- // use jms1 helpers
- compile(project(':dd-java-agent:instrumentation:jms-1')) {
- transitive = false
- }
- compileOnly group: 'javax.jms', name: 'javax.jms-api', version: '2.0.1'
-
- compile deps.bytebuddy
- compile deps.opentracing
- annotationProcessor deps.autoservice
- implementation deps.autoservice
-
- compile project(':dd-java-agent:agent-tooling')
-
- testCompile project(':dd-java-agent:testing')
- testCompile group: 'org.hornetq', name: 'hornetq-jms-client', version: '2.4.7.Final'
- testCompile group: 'org.hornetq', name: 'hornetq-jms-server', version: '2.4.7.Final'
-
-}

RENAME
dd-java-agent/instrumentation/jms-1/jms-1.gradle
dd-java-agent/instrumentation/jms/jms.gradle
@@ -11,8 +11,28 @@ versionScan {
]
}
+muzzle {
+ pass {
+ group = "javax.jms"
+ module = "jms-api"
+ versions = "(,)"
+ }
+ pass {
+ group = "javax.jms"
+ module = "javax.jms-api"
+ versions = "(,)"
+ }
+}
+
apply from: "${rootDir}/gradle/java.gradle"
+apply plugin: 'org.unbroken-dome.test-sets'
+
+testSets {
+ latestDepTest {
+ }
+}
+
dependencies {
compileOnly group: 'javax.jms', name: 'jms-api', version: '1.1-rev-1'
@@ -27,4 +47,7 @@ dependencies {
testCompile group: 'org.apache.activemq.tooling', name: 'activemq-junit', version: '5.14.5'
testCompile group: 'org.apache.activemq', name: 'activemq-pool', version: '5.14.5'
testCompile group: 'org.apache.activemq', name: 'activemq-broker', version: '5.14.5'
+
+ latestDepTestCompile group: 'org.hornetq', name: 'hornetq-jms-client', version: '2.4.7.Final'
+ latestDepTestCompile group: 'org.hornetq', name: 'hornetq-jms-server', version: '2.4.7.Final'
}

RENAME
dd-java-agent/instrumentation/jms-2/src/test/groovy/JMS2Test.groovy
dd-java-agent/instrumentation/jms/src/latestDepTest/groovy/JMS2Test.groovy
@@ -17,6 +17,7 @@ import org.hornetq.core.remoting.impl.netty.NettyAcceptorFactory
import org.hornetq.core.server.HornetQServers
import org.hornetq.jms.client.HornetQMessageConsumer
import org.hornetq.jms.client.HornetQMessageProducer
+import org.hornetq.jms.client.HornetQTextMessage
import spock.lang.Shared
import javax.jms.Message
@@ -32,7 +33,9 @@ class JMS2Test extends AgentTestRunner {
@Shared
String messageText = "a message"
@Shared
- static Session session
+ Session session
+
+ HornetQTextMessage message = session.createTextMessage(messageText)
def setupSpec() {
def tempDir = Files.createTempDir()
@@ -73,7 +76,6 @@ class JMS2Test extends AgentTestRunner {
setup:
def producer = session.createProducer(destination)
def consumer = session.createConsumer(destination)
- def message = session.createTextMessage(messageText)
producer.send(message)
@@ -95,7 +97,7 @@ class JMS2Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_CONSUMER
- "${Tags.COMPONENT.key}" "jms2"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "consumer"
"span.origin.type" HornetQMessageConsumer.name
}
@@ -129,7 +131,6 @@ class JMS2Test extends AgentTestRunner {
}
}
- def message = session.createTextMessage(messageText)
producer.send(message)
lock.countDown()
@@ -148,7 +149,7 @@ class JMS2Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_CONSUMER
- "${Tags.COMPONENT.key}" "jms2"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "consumer"
"span.origin.type" { t -> t.contains("JMS2Test") }
}
@@ -192,7 +193,7 @@ class JMS2Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_CONSUMER
- "${Tags.COMPONENT.key}" "jms2"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "consumer"
"span.origin.type" HornetQMessageConsumer.name
}
@@ -231,7 +232,7 @@ class JMS2Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_CONSUMER
- "${Tags.COMPONENT.key}" "jms2"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "consumer"
"span.origin.type" HornetQMessageConsumer.name
}
@@ -261,7 +262,7 @@ class JMS2Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_PRODUCER
- "${Tags.COMPONENT.key}" "jms2"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "producer"
"span.origin.type" HornetQMessageProducer.name
}
@@ -282,7 +283,7 @@ class JMS2Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_CONSUMER
- "${Tags.COMPONENT.key}" "jms2"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "consumer"
"span.origin.type" origin
}

RENAME
dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageConsumerInstrumentation.java
dd-java-agent/instrumentation/jms/src/main/java/datadog/trace/instrumentation/jms/JMSMessageConsumerInstrumentation.java
-package datadog.trace.instrumentation.jms2;
+package datadog.trace.instrumentation.jms;
import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses;
-import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName;
+import static datadog.trace.instrumentation.jms.JmsUtil.toResourceName;
import static io.opentracing.log.Fields.ERROR_OBJECT;
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
@@ -14,7 +13,6 @@ import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.api.DDSpanTypes;
import datadog.trace.api.DDTags;
-import datadog.trace.instrumentation.jms.util.MessagePropertyTextMap;
import io.opentracing.Scope;
import io.opentracing.Span;
import io.opentracing.SpanContext;
@@ -34,15 +32,10 @@ import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
@AutoService(Instrumenter.class)
-public final class JMS2MessageConsumerInstrumentation extends Instrumenter.Default {
- public static final String[] JMS2_HELPER_CLASS_NAMES =
- new String[] {
- "datadog.trace.instrumentation.jms.util.JmsUtil",
- "datadog.trace.instrumentation.jms.util.MessagePropertyTextMap"
- };
+public final class JMSMessageConsumerInstrumentation extends Instrumenter.Default {
- public JMS2MessageConsumerInstrumentation() {
- super("jms", "jms-2");
+ public JMSMessageConsumerInstrumentation() {
+ super("jms", "jms-1", "jms-2");
}
@Override
@@ -50,14 +43,9 @@ public final class JMS2MessageConsumerInstrumentation extends Instrumenter.Defau
return not(isInterface()).and(safeHasSuperType(named("javax.jms.MessageConsumer")));
}
- @Override
- public ElementMatcher<ClassLoader> classLoaderMatcher() {
- return classLoaderHasClasses("javax.jms.JMSContext", "javax.jms.CompletionListener");
- }
-
@Override
public String[] helperClassNames() {
- return JMS2_HELPER_CLASS_NAMES;
+ return new String[] {packageName + ".JmsUtil", packageName + ".MessagePropertyTextMap"};
}
@Override
@@ -91,7 +79,7 @@ public final class JMS2MessageConsumerInstrumentation extends Instrumenter.Defau
.buildSpan("jms.consume")
.withTag(DDTags.SERVICE_NAME, "jms")
.withTag(DDTags.SPAN_TYPE, DDSpanTypes.MESSAGE_CONSUMER)
- .withTag(Tags.COMPONENT.getKey(), "jms2")
+ .withTag(Tags.COMPONENT.getKey(), "jms")
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CONSUMER)
.withTag("span.origin.type", consumer.getClass().getName())
.withStartTimestamp(TimeUnit.MILLISECONDS.toMicros(startTime));

RENAME
dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageListenerInstrumentation.java
dd-java-agent/instrumentation/jms/src/main/java/datadog/trace/instrumentation/jms/JMSMessageListenerInstrumentation.java
-package datadog.trace.instrumentation.jms2;
+package datadog.trace.instrumentation.jms;
import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses;
-import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName;
+import static datadog.trace.instrumentation.jms.JmsUtil.toResourceName;
import static io.opentracing.log.Fields.ERROR_OBJECT;
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
@@ -15,7 +14,6 @@ import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.api.DDSpanTypes;
import datadog.trace.api.DDTags;
import datadog.trace.context.TraceScope;
-import datadog.trace.instrumentation.jms.util.MessagePropertyTextMap;
import io.opentracing.Scope;
import io.opentracing.Span;
import io.opentracing.SpanContext;
@@ -32,10 +30,10 @@ import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
@AutoService(Instrumenter.class)
-public final class JMS2MessageListenerInstrumentation extends Instrumenter.Default {
+public final class JMSMessageListenerInstrumentation extends Instrumenter.Default {
- public JMS2MessageListenerInstrumentation() {
- super("jms", "jms-2");
+ public JMSMessageListenerInstrumentation() {
+ super("jms", "jms-1", "jms-2");
}
@Override
@@ -43,14 +41,9 @@ public final class JMS2MessageListenerInstrumentation extends Instrumenter.Defau
return not(isInterface()).and(safeHasSuperType(named("javax.jms.MessageListener")));
}
- @Override
- public ElementMatcher<ClassLoader> classLoaderMatcher() {
- return classLoaderHasClasses("javax.jms.JMSContext", "javax.jms.CompletionListener");
- }
-
@Override
public String[] helperClassNames() {
- return JMS2MessageConsumerInstrumentation.JMS2_HELPER_CLASS_NAMES;
+ return new String[] {packageName + ".JmsUtil", packageName + ".MessagePropertyTextMap"};
}
@Override
@@ -78,7 +71,7 @@ public final class JMS2MessageListenerInstrumentation extends Instrumenter.Defau
.withTag(DDTags.SERVICE_NAME, "jms")
.withTag(DDTags.SPAN_TYPE, DDSpanTypes.MESSAGE_CONSUMER)
.withTag(DDTags.RESOURCE_NAME, "Received from " + toResourceName(message, null))
- .withTag(Tags.COMPONENT.getKey(), "jms2")
+ .withTag(Tags.COMPONENT.getKey(), "jms")
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CONSUMER)
.withTag("span.origin.type", listener.getClass().getName())
.startActive(true);

RENAME
dd-java-agent/instrumentation/jms-2/src/main/java/datadog/trace/instrumentation/jms2/JMS2MessageProducerInstrumentation.java
dd-java-agent/instrumentation/jms/src/main/java/datadog/trace/instrumentation/jms/JMSMessageProducerInstrumentation.java
-package datadog.trace.instrumentation.jms2;
+package datadog.trace.instrumentation.jms;
import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses;
-import static datadog.trace.instrumentation.jms.util.JmsUtil.toResourceName;
+import static datadog.trace.instrumentation.jms.JmsUtil.toResourceName;
import static io.opentracing.log.Fields.ERROR_OBJECT;
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
@@ -14,7 +13,6 @@ import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.api.DDSpanTypes;
import datadog.trace.api.DDTags;
-import datadog.trace.instrumentation.jms.util.MessagePropertyTextMap;
import io.opentracing.Scope;
import io.opentracing.Span;
import io.opentracing.propagation.Format;
@@ -32,10 +30,10 @@ import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
@AutoService(Instrumenter.class)
-public final class JMS2MessageProducerInstrumentation extends Instrumenter.Default {
+public final class JMSMessageProducerInstrumentation extends Instrumenter.Default {
- public JMS2MessageProducerInstrumentation() {
- super("jms", "jms-2");
+ public JMSMessageProducerInstrumentation() {
+ super("jms", "jms-1", "jms-2");
}
@Override
@@ -43,14 +41,9 @@ public final class JMS2MessageProducerInstrumentation extends Instrumenter.Defau
return not(isInterface()).and(safeHasSuperType(named("javax.jms.MessageProducer")));
}
- @Override
- public ElementMatcher<ClassLoader> classLoaderMatcher() {
- return classLoaderHasClasses("javax.jms.JMSContext", "javax.jms.CompletionListener");
- }
-
@Override
public String[] helperClassNames() {
- return JMS2MessageConsumerInstrumentation.JMS2_HELPER_CLASS_NAMES;
+ return new String[] {packageName + ".JmsUtil", packageName + ".MessagePropertyTextMap"};
}
@Override
@@ -83,11 +76,11 @@ public final class JMS2MessageProducerInstrumentation extends Instrumenter.Defau
GlobalTracer.get()
.buildSpan("jms.produce")
.withTag(DDTags.SERVICE_NAME, "jms")
- .withTag(DDTags.SPAN_TYPE, DDSpanTypes.MESSAGE_PRODUCER)
.withTag(
DDTags.RESOURCE_NAME,
"Produced for " + toResourceName(message, defaultDestination))
- .withTag(Tags.COMPONENT.getKey(), "jms2")
+ .withTag(Tags.COMPONENT.getKey(), "jms")
+ .withTag(DDTags.SPAN_TYPE, DDSpanTypes.MESSAGE_PRODUCER)
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_PRODUCER)
.withTag("span.origin.type", producer.getClass().getName())
.startActive(true);
@@ -127,7 +120,7 @@ public final class JMS2MessageProducerInstrumentation extends Instrumenter.Defau
.withTag(DDTags.SERVICE_NAME, "jms")
.withTag(DDTags.SPAN_TYPE, DDSpanTypes.MESSAGE_PRODUCER)
.withTag(DDTags.RESOURCE_NAME, "Produced for " + toResourceName(message, destination))
- .withTag(Tags.COMPONENT.getKey(), "jms2")
+ .withTag(Tags.COMPONENT.getKey(), "jms")
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_PRODUCER)
.withTag("span.origin.type", producer.getClass().getName())
.startActive(true);
@@ -135,6 +128,7 @@ public final class JMS2MessageProducerInstrumentation extends Instrumenter.Defau
GlobalTracer.get()
.inject(
scope.span().context(), Format.Builtin.TEXT_MAP, new MessagePropertyTextMap(message));
+
return scope;
}

RENAME
dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms/util/JmsUtil.java
dd-java-agent/instrumentation/jms/src/main/java/datadog/trace/instrumentation/jms/JmsUtil.java
-package datadog.trace.instrumentation.jms.util;
+package datadog.trace.instrumentation.jms;
import javax.jms.Destination;
import javax.jms.Message;

RENAME
dd-java-agent/instrumentation/jms-1/src/main/java/datadog/trace/instrumentation/jms/util/MessagePropertyTextMap.java
dd-java-agent/instrumentation/jms/src/main/java/datadog/trace/instrumentation/jms/MessagePropertyTextMap.java
-package datadog.trace.instrumentation.jms.util;
+package datadog.trace.instrumentation.jms;
import io.opentracing.propagation.TextMap;
import java.util.Enumeration;
@@ -7,7 +7,9 @@ import java.util.Iterator;
import java.util.Map;
import javax.jms.JMSException;
import javax.jms.Message;
+import lombok.extern.slf4j.Slf4j;
+@Slf4j
public class MessagePropertyTextMap implements TextMap {
static final String DASH = "__dash__";
@@ -39,10 +41,13 @@ public class MessagePropertyTextMap implements TextMap {
@Override
public void put(final String key, final String value) {
+ final String propName = key.replace("-", DASH);
try {
- message.setStringProperty(key.replace("-", DASH), value);
+ message.setStringProperty(propName, value);
} catch (final JMSException e) {
- throw new RuntimeException(e);
+ if (log.isDebugEnabled()) {
+ log.debug("Failure setting jms property: " + propName, e);
+ }
}
}
}

RENAME
dd-java-agent/instrumentation/jms-1/src/test/groovy/JMS1Test.groovy
dd-java-agent/instrumentation/jms/src/test/groovy/JMS1Test.groovy
@@ -6,6 +6,7 @@ import io.opentracing.tag.Tags
import org.apache.activemq.ActiveMQConnectionFactory
import org.apache.activemq.ActiveMQMessageConsumer
import org.apache.activemq.ActiveMQMessageProducer
+import org.apache.activemq.command.ActiveMQTextMessage
import org.apache.activemq.junit.EmbeddedActiveMQBroker
import spock.lang.Shared
@@ -25,7 +26,7 @@ class JMS1Test extends AgentTestRunner {
@Shared
Session session
- def message = session.createTextMessage(messageText)
+ ActiveMQTextMessage message = session.createTextMessage(messageText)
def setupSpec() {
EmbeddedActiveMQBroker broker = new EmbeddedActiveMQBroker()
@@ -62,7 +63,7 @@ class JMS1Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_CONSUMER
- "${Tags.COMPONENT.key}" "jms1"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "consumer"
"span.origin.type" ActiveMQMessageConsumer.name
}
@@ -114,7 +115,7 @@ class JMS1Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_CONSUMER
- "${Tags.COMPONENT.key}" "jms1"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "consumer"
"span.origin.type" { t -> t.contains("JMS1Test") }
}
@@ -158,7 +159,7 @@ class JMS1Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_CONSUMER
- "${Tags.COMPONENT.key}" "jms1"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "consumer"
"span.origin.type" ActiveMQMessageConsumer.name
}
@@ -197,7 +198,7 @@ class JMS1Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_CONSUMER
- "${Tags.COMPONENT.key}" "jms1"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "consumer"
"span.origin.type" ActiveMQMessageConsumer.name
}
@@ -214,6 +215,61 @@ class JMS1Test extends AgentTestRunner {
session.createTopic("someTopic") | "Topic someTopic"
}
+ def "sending a read-only message to #jmsResourceName fails"() {
+ setup:
+ def producer = session.createProducer(destination)
+ def consumer = session.createConsumer(destination)
+
+ expect:
+ !message.isReadOnlyProperties()
+
+ when:
+ message.setReadOnlyProperties(true)
+ and:
+ producer.send(message)
+
+ TextMessage receivedMessage = consumer.receive()
+
+ then:
+ receivedMessage.text == messageText
+
+ // This will result in a logged failure because we tried to
+ // write properties in MessagePropertyTextMap when readOnlyProperties = true.
+ // The consumer span will also not be linked to the parent.
+ assertTraces(TEST_WRITER, 2) {
+ producerTrace(it, 0, jmsResourceName)
+ trace(1, 1) { // Consumer trace
+ span(0) {
+ parent()
+ serviceName "jms"
+ operationName "jms.consume"
+ resourceName "Consumed from $jmsResourceName"
+ spanType DDSpanTypes.MESSAGE_PRODUCER
+ errored false
+
+ tags {
+ defaultTags()
+ "${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_CONSUMER
+ "${Tags.COMPONENT.key}" "jms"
+ "${Tags.SPAN_KIND.key}" "consumer"
+ "span.origin.type" ActiveMQMessageConsumer.name
+ }
+ }
+ }
+ }
+
+ cleanup:
+ producer.close()
+ consumer.close()
+
+ where:
+ destination | jmsResourceName
+ session.createQueue("someQueue") | "Queue someQueue"
+ session.createTopic("someTopic") | "Topic someTopic"
+ session.createTemporaryQueue() | "Temporary Queue"
+ session.createTemporaryTopic() | "Temporary Topic"
+ }
+
def producerTrace(ListWriterAssert writer, int index, String jmsResourceName) {
writer.trace(index, 3) {
span(0) {
@@ -227,7 +283,7 @@ class JMS1Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_PRODUCER
- "${Tags.COMPONENT.key}" "jms1"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "producer"
"span.origin.type" ActiveMQMessageProducer.name
}
@@ -243,7 +299,7 @@ class JMS1Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_PRODUCER
- "${Tags.COMPONENT.key}" "jms1"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "producer"
"span.origin.type" ActiveMQMessageProducer.name
}
@@ -259,7 +315,7 @@ class JMS1Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_PRODUCER
- "${Tags.COMPONENT.key}" "jms1"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "producer"
"span.origin.type" ActiveMQMessageProducer.name
}
@@ -280,7 +336,7 @@ class JMS1Test extends AgentTestRunner {
tags {
defaultTags()
"${DDTags.SPAN_TYPE}" DDSpanTypes.MESSAGE_CONSUMER
- "${Tags.COMPONENT.key}" "jms1"
+ "${Tags.COMPONENT.key}" "jms"
"${Tags.SPAN_KIND.key}" "consumer"
"span.origin.type" origin
}

MODIFY
settings.gradle
settings.gradle
@@ -30,8 +30,7 @@ include ':dd-java-agent:instrumentation:jboss-classloading'
include ':dd-java-agent:instrumentation:jdbc'
include ':dd-java-agent:instrumentation:jedis-1.4'
include ':dd-java-agent:instrumentation:jetty-8'
-include ':dd-java-agent:instrumentation:jms-1'
-include ':dd-java-agent:instrumentation:jms-2'
+include ':dd-java-agent:instrumentation:jms'
include ':dd-java-agent:instrumentation:jsp-2.3'
include ':dd-java-agent:instrumentation:kafka-clients-0.11'
include ':dd-java-agent:instrumentation:kafka-streams-0.11'

Make JMS more resilent to property failure

Combine into single module and add test.
Replace classLoaderMatcher with muzzle check.

Thanks for the tip @adriancole.