Review fix comments and version range.
RENAME
dd-java-agent/instrumentation/rabbitmq-amqp-2.6/rabbitmq-amqp-2.6.gradle
dd-java-agent/instrumentation/rabbitmq-amqp-2.7/rabbitmq-amqp-2.7.gradle
@@ -2,7 +2,7 @@ muzzle {
pass {
group = "com.rabbitmq"
module = 'amqp-client'
- versions = "[2.6.0,)"
+ versions = "[2.7.0,)"
assertInverse = true
}
}
@@ -18,7 +18,7 @@ testSets {
}
dependencies {
- compileOnly group: 'com.rabbitmq', name: 'amqp-client', version: '2.6.0'
+ compileOnly group: 'com.rabbitmq', name: 'amqp-client', version: '2.7.0'
compile project(':dd-trace-ot')
compile project(':dd-java-agent:agent-tooling')
@@ -31,7 +31,7 @@ dependencies {
testCompile project(':dd-java-agent:testing')
- testCompile group: 'com.rabbitmq', name: 'amqp-client', version: '2.6.0'
+ testCompile group: 'com.rabbitmq', name: 'amqp-client', version: '2.7.0'
testCompile group: 'org.springframework.amqp', name: 'spring-rabbit', version: '1.1.0.RELEASE'
testCompile deps.testcontainers
@@ -42,6 +42,6 @@ dependencies {
configurations.testRuntime {
resolutionStrategy {
- force group: 'com.rabbitmq', name: 'amqp-client', version: '2.6.0'
+ force group: 'com.rabbitmq', name: 'amqp-client', version: '2.7.0'
}
}

RENAME
dd-java-agent/instrumentation/rabbitmq-amqp-2.6/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitChannelInstrumentation.java
dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitChannelInstrumentation.java
@@ -17,7 +17,6 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
import com.google.auto.service.AutoService;
import com.rabbitmq.client.AMQP;
import com.rabbitmq.client.Channel;
-import com.rabbitmq.client.Command;
import com.rabbitmq.client.Connection;
import com.rabbitmq.client.Consumer;
import com.rabbitmq.client.GetResponse;
@@ -135,12 +134,6 @@ public class RabbitChannelInstrumentation extends Instrumenter.Default {
CallDepthThreadLocalMap.reset(Channel.class);
}
}
-
- // Added to ensure consistent muzzle validation for all instrumentation.
- public static void muzzleCheck(final Command cmd) {
- com.rabbitmq.client.Method.class.getName();
- cmd.getMethod();
- }
}
public static class ChannelPublishAdvice {
@@ -153,7 +146,8 @@ public class RabbitChannelInstrumentation extends Instrumenter.Default {
final Span span = GlobalTracer.get().activeSpan();
if (span != null) {
- span.setTag(DDTags.RESOURCE_NAME, "basic.publish " + exchange);
+ final String exchangeName = exchange == null || exchange.isEmpty() ? "<default>" : exchange;
+ span.setTag(DDTags.RESOURCE_NAME, "basic.publish " + exchangeName);
span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.MESSAGE_PRODUCER);
span.setTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_PRODUCER);
span.setTag("amqp.exchange", exchange);
@@ -278,7 +272,7 @@ public class RabbitChannelInstrumentation extends Instrumenter.Default {
@Advice.Argument(0) final String queue,
@Advice.Argument(value = 6, readOnly = false) Consumer consumer) {
// We have to save off the queue name here because it isn't available to the consumer later.
- if (consumer != null) {
+ if (consumer != null && !(consumer instanceof TracedDelegatingConsumer)) {
consumer = new TracedDelegatingConsumer(queue, consumer);
}
}

RENAME
dd-java-agent/instrumentation/rabbitmq-amqp-2.6/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitCommandInstrumentation.java
dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/RabbitCommandInstrumentation.java
@@ -31,6 +31,14 @@ public class RabbitCommandInstrumentation extends Instrumenter.Default {
return not(isInterface()).and(safeHasSuperType(named("com.rabbitmq.client.Command")));
}
+ @Override
+ public String[] helperClassNames() {
+ return new String[] {
+ // These are only used by muzzleCheck.
+ packageName + ".TextMapExtractAdapter", packageName + ".TracedDelegatingConsumer",
+ };
+ }
+
@Override
public Map<? extends ElementMatcher, String> transformers() {
return Collections.singletonMap(isConstructor(), CommandConstructorAdvice.class.getName());
@@ -48,5 +56,14 @@ public class RabbitCommandInstrumentation extends Instrumenter.Default {
span.setTag("amqp.command", name);
}
}
+
+ /**
+ * This instrumentation will match with 2.6, but the channel instrumentation only matches with
+ * 2.7 because of TracedDelegatingConsumer. This unused method is added to ensure consistent
+ * muzzle validation by preventing match with 2.6.
+ */
+ public static void muzzleCheck(final TracedDelegatingConsumer consumer) {
+ consumer.handleRecoverOk(null);
+ }
}
}

RENAME
dd-java-agent/instrumentation/rabbitmq-amqp-2.6/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/TextMapExtractAdapter.java
dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/TextMapExtractAdapter.java
RENAME
dd-java-agent/instrumentation/rabbitmq-amqp-2.6/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/TextMapInjectAdapter.java
dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/TextMapInjectAdapter.java
RENAME
dd-java-agent/instrumentation/rabbitmq-amqp-2.6/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/TracedDelegatingConsumer.java
dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/main/java/datadog/trace/instrumentation/rabbitmq/amqp/TracedDelegatingConsumer.java
@@ -24,11 +24,11 @@ import java.util.Map;
* queue name when the message is consumed.
*/
public class TracedDelegatingConsumer implements Consumer {
- private final String queueName;
+ private final String queue;
private final Consumer delegate;
- public TracedDelegatingConsumer(final String queueName, final Consumer delegate) {
- this.queueName = queueName;
+ public TracedDelegatingConsumer(final String queue, final Consumer delegate) {
+ this.queue = queue;
this.delegate = delegate;
}
@@ -53,8 +53,8 @@ public class TracedDelegatingConsumer implements Consumer {
}
@Override
- public void handleRecoverOk() {
- delegate.handleRecoverOk();
+ public void handleRecoverOk(String consumerTag) {
+ delegate.handleRecoverOk(consumerTag);
}
@Override
@@ -73,15 +73,19 @@ public class TracedDelegatingConsumer implements Consumer {
: GlobalTracer.get()
.extract(Format.Builtin.TEXT_MAP, new TextMapExtractAdapter(headers));
- final String resourceName =
- queueName == null ? "basic.deliver null" : "basic.deliver " + queueName;
+ String queueName = queue;
+ if (queue == null || queue.isEmpty()) {
+ queueName = "<default>";
+ } else if (queue.startsWith("amq.gen-")) {
+ queueName = "<generated>";
+ }
scope =
GlobalTracer.get()
.buildSpan("amqp.command")
.asChildOf(parentContext)
.withTag(DDTags.SERVICE_NAME, "rabbitmq")
- .withTag(DDTags.RESOURCE_NAME, resourceName)
+ .withTag(DDTags.RESOURCE_NAME, "basic.deliver " + queueName)
.withTag(DDTags.SPAN_TYPE, DDSpanTypes.MESSAGE_CONSUMER)
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CONSUMER)
.withTag(Tags.COMPONENT.getKey(), "rabbitmq-amqp")

RENAME
dd-java-agent/instrumentation/rabbitmq-amqp-2.6/src/test/groovy/RabbitMQTest.groovy
dd-java-agent/instrumentation/rabbitmq-amqp-2.7/src/test/groovy/RabbitMQTest.groovy
@@ -26,7 +26,7 @@ import spock.lang.Shared
import java.util.concurrent.Phaser
// Do not run tests locally on Java7 since testcontainers are not compatible with Java7
-// It is fine to run on CI because CI provides memcached externally, not through testcontainers
+// It is fine to run on CI because CI provides rabbitmq externally, not through testcontainers
@Requires({ "true" == System.getenv("CI") || jvm.java8Compatible })
class RabbitMQTest extends AgentTestRunner {
@@ -48,7 +48,7 @@ class RabbitMQTest extends AgentTestRunner {
def setupSpec() {
/*
- CI will provide us with memcached container running along side our build.
+ CI will provide us with rabbitmq container running along side our build.
When building locally, however, we need to take matters into our own hands
and we use 'testcontainers' for this.
*/
@@ -121,7 +121,9 @@ class RabbitMQTest extends AgentTestRunner {
def "test rabbit consume #messageCount messages"() {
setup:
channel.exchangeDeclare(exchangeName, "direct", false)
- String queueName = channel.queueDeclare("some-queue", false, true, true, null).getQueue()
+ String queueName = (messageCount % 2 == 0) ?
+ channel.queueDeclare().getQueue() :
+ channel.queueDeclare("some-queue", false, true, true, null).getQueue()
channel.queueBind(queueName, exchangeName, "")
def phaser = new Phaser()
@@ -145,6 +147,7 @@ class RabbitMQTest extends AgentTestRunner {
TEST_WRITER.waitForTraces(3 + (it * 2))
phaser.arriveAndAwaitAdvance()
}
+ def resource = messageCount % 2 == 0 ? "basic.deliver <generated>" : "basic.deliver $queueName"
expect:
assertTraces(4 + (messageCount * 2)) {
@@ -167,7 +170,7 @@ class RabbitMQTest extends AgentTestRunner {
rabbitSpan(it, "basic.publish")
}
trace(3 + (it * 2), 1) {
- rabbitSpan(it, "basic.deliver some-queue", publishSpan)
+ rabbitSpan(it, resource, publishSpan)
}
}
}
@@ -176,7 +179,7 @@ class RabbitMQTest extends AgentTestRunner {
where:
exchangeName = "some-exchange"
- messageCount << (1..3)
+ messageCount << (1..4)
}
def "test rabbit error (#command)"() {

MODIFY
dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy
dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy
@@ -70,6 +70,9 @@ class TagsAssert {
void assertTagsAllVerified() {
def set = new TreeMap<>(tags).keySet()
set.removeAll(assertedTags)
+ // The primary goal is to ensure the set is empty.
+ // tags and assertedTags are included via an "always true" comparison
+ // so they provide better context in the error message.
assert tags.entrySet() != assertedTags && set.isEmpty()
}
}

MODIFY
settings.gradle
settings.gradle
@@ -46,7 +46,7 @@ include ':dd-java-agent:instrumentation:netty-4.1'
include ':dd-java-agent:instrumentation:okhttp-3'
include ':dd-java-agent:instrumentation:osgi-classloading'
include ':dd-java-agent:instrumentation:play-2.4'
-include ':dd-java-agent:instrumentation:rabbitmq-amqp-2.6'
+include ':dd-java-agent:instrumentation:rabbitmq-amqp-2.7'
include ':dd-java-agent:instrumentation:ratpack-1.4'
include ':dd-java-agent:instrumentation:servlet-2'
include ':dd-java-agent:instrumentation:servlet-3'

Review fix comments and version range.