Use links instead of overriding parent
Also, message receive span kind should be CLIENT.
MODIFY
instrumentation/jms-1.1/src/latestDepTest/groovy/JMS2Test.groovy
instrumentation/jms-1.1/src/latestDepTest/groovy/JMS2Test.groovy
@@ -29,6 +29,7 @@ import javax.jms.TextMessage
import java.util.concurrent.CountDownLatch
import java.util.concurrent.atomic.AtomicReference
+import static io.opentelemetry.trace.Span.Kind.CLIENT
import static io.opentelemetry.trace.Span.Kind.CONSUMER
import static io.opentelemetry.trace.Span.Kind.PRODUCER
@@ -93,10 +94,12 @@ class JMS2Test extends AgentTestRunner {
expect:
receivedMessage.text == messageText
- assertTraces(1) {
- trace(0, 2) {
+ assertTraces(2) {
+ trace(0, 1) {
producerSpan(it, 0, jmsResourceName)
- consumerSpan(it, 1, jmsResourceName, false, HornetQMessageConsumer, span(0))
+ }
+ trace(1, 1) {
+ consumerSpan(it, 0, jmsResourceName, false, HornetQMessageConsumer, traces[0][0])
}
}
@@ -165,7 +168,7 @@ class JMS2Test extends AgentTestRunner {
span(0) {
parent()
operationName "jms.consume"
- spanKind CONSUMER
+ spanKind CLIENT
errored false
tags {
"$MoreTags.SERVICE_NAME" "jms"
@@ -201,7 +204,7 @@ class JMS2Test extends AgentTestRunner {
span(0) {
parent()
operationName "jms.consume"
- spanKind CONSUMER
+ spanKind CLIENT
errored false
tags {
"$MoreTags.SERVICE_NAME" "jms"
@@ -239,15 +242,17 @@ class JMS2Test extends AgentTestRunner {
}
}
- static consumerSpan(TraceAssert trace, int index, String jmsResourceName, boolean messageListener, Class origin, Object parentSpan) {
+ static consumerSpan(TraceAssert trace, int index, String jmsResourceName, boolean messageListener, Class origin, Object parentOrLinkSpan) {
trace.span(index) {
- childOf((SpanData) parentSpan)
if (messageListener) {
operationName "jms.onMessage"
+ childOf((SpanData) parentOrLinkSpan)
+ spanKind CONSUMER
} else {
operationName "jms.consume"
+ hasLink((SpanData) parentOrLinkSpan)
+ spanKind CLIENT
}
- spanKind CONSUMER
errored false
tags {

MODIFY
instrumentation/jms-1.1/src/latestDepTest/groovy/SpringTemplateJMS2Test.groovy
instrumentation/jms-1.1/src/latestDepTest/groovy/SpringTemplateJMS2Test.groovy
@@ -74,10 +74,12 @@ class SpringTemplateJMS2Test extends AgentTestRunner {
expect:
receivedMessage.text == messageText
- assertTraces(1) {
- trace(0, 2) {
+ assertTraces(2) {
+ trace(0, 1) {
producerSpan(it, 0, jmsResourceName)
- consumerSpan(it, 1, jmsResourceName, false, HornetQMessageConsumer, span(0))
+ }
+ trace(1, 1) {
+ consumerSpan(it, 0, jmsResourceName, false, HornetQMessageConsumer, traces[0][0])
}
}
@@ -103,14 +105,18 @@ class SpringTemplateJMS2Test extends AgentTestRunner {
expect:
receivedMessage.text == "responded!"
- assertTraces(2) {
- trace(0, 2) {
+ assertTraces(4) {
+ trace(0, 1) {
producerSpan(it, 0, jmsResourceName)
- consumerSpan(it, 1, jmsResourceName, false, HornetQMessageConsumer, span(0))
}
- trace(1, 2) {
+ trace(1, 1) {
+ consumerSpan(it, 0, jmsResourceName, false, HornetQMessageConsumer, traces[0][0])
+ }
+ trace(2, 1) {
producerSpan(it, 0, "Temporary Queue") // receive doesn't propagate the trace, so this is a root
- consumerSpan(it, 1, "Temporary Queue", false, HornetQMessageConsumer, span(0))
+ }
+ trace(3, 1) {
+ consumerSpan(it, 0, "Temporary Queue", false, HornetQMessageConsumer, traces[2][0])
}
}

MODIFY
instrumentation/jms-1.1/src/main/java/io/opentelemetry/auto/instrumentation/jms/JMSMessageConsumerInstrumentation.java
instrumentation/jms-1.1/src/main/java/io/opentelemetry/auto/instrumentation/jms/JMSMessageConsumerInstrumentation.java
@@ -4,7 +4,7 @@ import static io.opentelemetry.auto.instrumentation.jms.JMSDecorator.CONSUMER_DE
import static io.opentelemetry.auto.instrumentation.jms.JMSDecorator.TRACER;
import static io.opentelemetry.auto.instrumentation.jms.MessageExtractAdapter.GETTER;
import static io.opentelemetry.auto.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static io.opentelemetry.trace.Span.Kind.CONSUMER;
+import static io.opentelemetry.trace.Span.Kind.CLIENT;
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -15,7 +15,6 @@ import com.google.auto.service.AutoService;
import io.opentelemetry.auto.tooling.Instrumenter;
import io.opentelemetry.context.Scope;
import io.opentelemetry.trace.Span;
-import io.opentelemetry.trace.SpanContext;
import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.Map;
@@ -81,18 +80,14 @@ public final class JMSMessageConsumerInstrumentation extends Instrumenter.Defaul
final Span.Builder spanBuilder =
TRACER
.spanBuilder("jms.consume")
- .setSpanKind(CONSUMER)
+ .setSpanKind(CLIENT)
.setStartTimestamp(TimeUnit.MILLISECONDS.toNanos(startTime));
if (message != null) {
try {
- final SpanContext extractedContext = TRACER.getHttpTextFormat().extract(message, GETTER);
- spanBuilder.setParent(extractedContext);
+ spanBuilder.addLink(TRACER.getHttpTextFormat().extract(message, GETTER));
} catch (final IllegalArgumentException e) {
- // Couldn't extract a context. We should treat this as a root span.
- spanBuilder.setNoParent();
+ // Couldn't extract a context
}
- } else {
- spanBuilder.setNoParent();
}
final Span span = spanBuilder.startSpan();
span.setAttribute("span.origin.type", consumer.getClass().getName());

MODIFY
instrumentation/jms-1.1/src/test/groovy/JMS1Test.groovy
instrumentation/jms-1.1/src/test/groovy/JMS1Test.groovy
@@ -19,6 +19,7 @@ import javax.jms.TextMessage
import java.util.concurrent.CountDownLatch
import java.util.concurrent.atomic.AtomicReference
+import static io.opentelemetry.trace.Span.Kind.CLIENT
import static io.opentelemetry.trace.Span.Kind.CONSUMER
import static io.opentelemetry.trace.Span.Kind.PRODUCER
@@ -56,10 +57,12 @@ class JMS1Test extends AgentTestRunner {
expect:
receivedMessage.text == messageText
- assertTraces(1) {
- trace(0, 2) {
+ assertTraces(2) {
+ trace(0, 1) {
producerSpan(it, 0, jmsResourceName)
- consumerSpan(it, 1, jmsResourceName, false, ActiveMQMessageConsumer, span(0))
+ }
+ trace(1, 1) {
+ consumerSpan(it, 0, jmsResourceName, false, ActiveMQMessageConsumer, traces[0][0])
}
}
@@ -128,7 +131,7 @@ class JMS1Test extends AgentTestRunner {
span(0) {
parent()
operationName "jms.consume"
- spanKind CONSUMER
+ spanKind CLIENT
errored false
tags {
"$MoreTags.SERVICE_NAME" "jms"
@@ -164,7 +167,7 @@ class JMS1Test extends AgentTestRunner {
span(0) {
parent()
operationName "jms.consume"
- spanKind CONSUMER
+ spanKind CLIENT
errored false
tags {
"$MoreTags.SERVICE_NAME" "jms"
@@ -215,7 +218,7 @@ class JMS1Test extends AgentTestRunner {
span(0) {
parent()
operationName "jms.consume"
- spanKind CONSUMER
+ spanKind CLIENT
errored false
tags {
"$MoreTags.SERVICE_NAME" "jms"
@@ -256,16 +259,19 @@ class JMS1Test extends AgentTestRunner {
}
}
- static consumerSpan(TraceAssert trace, int index, String jmsResourceName, boolean messageListener, Class origin, Object parentSpan) {
+ static consumerSpan(TraceAssert trace, int index, String jmsResourceName, boolean messageListener, Class origin, Object parentOrLinkedSpan) {
trace.span(index) {
if (messageListener) {
operationName "jms.onMessage"
+ spanKind CONSUMER
+ childOf((SpanData) parentOrLinkedSpan)
} else {
operationName "jms.consume"
+ spanKind CLIENT
+ parent()
+ hasLink((SpanData) parentOrLinkedSpan)
}
- spanKind CONSUMER
errored false
- childOf((SpanData) parentSpan)
tags {
"$MoreTags.SERVICE_NAME" "jms"
"$MoreTags.RESOURCE_NAME" messageListener ? "Received from $jmsResourceName" : "Consumed from $jmsResourceName"

MODIFY
instrumentation/jms-1.1/src/test/groovy/SpringTemplateJMS1Test.groovy
instrumentation/jms-1.1/src/test/groovy/SpringTemplateJMS1Test.groovy
@@ -48,10 +48,12 @@ class SpringTemplateJMS1Test extends AgentTestRunner {
expect:
receivedMessage.text == messageText
- assertTraces(1) {
- trace(0, 2) {
+ assertTraces(2) {
+ trace(0, 1) {
producerSpan(it, 0, jmsResourceName)
- consumerSpan(it, 1, jmsResourceName, false, ActiveMQMessageConsumer, span(0))
+ }
+ trace(1, 1) {
+ consumerSpan(it, 0, jmsResourceName, false, ActiveMQMessageConsumer, traces[0][0])
}
}
@@ -81,14 +83,18 @@ class SpringTemplateJMS1Test extends AgentTestRunner {
expect:
receivedMessage.text == "responded!"
- assertTraces(2) {
- trace(0, 2) {
+ assertTraces(4) {
+ trace(0, 1) {
producerSpan(it, 0, jmsResourceName)
- consumerSpan(it, 1, jmsResourceName, false, ActiveMQMessageConsumer, span(0))
}
- trace(1, 2) {
+ trace(1, 1) {
+ consumerSpan(it, 0, jmsResourceName, false, ActiveMQMessageConsumer, traces[0][0])
+ }
+ trace(2, 1) {
producerSpan(it, 0, "Temporary Queue") // receive doesn't propagate the trace, so this is a root
- consumerSpan(it, 1, "Temporary Queue", false, ActiveMQMessageConsumer, span(0))
+ }
+ trace(3, 1) {
+ consumerSpan(it, 0, "Temporary Queue", false, ActiveMQMessageConsumer, traces[2][0])
}
}

MODIFY
instrumentation/kafka-clients-0.11/src/main/java/io/opentelemetry/auto/instrumentation/kafka_clients/TracingIterator.java
instrumentation/kafka-clients-0.11/src/main/java/io/opentelemetry/auto/instrumentation/kafka_clients/TracingIterator.java
@@ -6,7 +6,6 @@ import static io.opentelemetry.trace.Span.Kind.CONSUMER;
import io.opentelemetry.auto.instrumentation.api.SpanWithScope;
import io.opentelemetry.trace.Span;
-import io.opentelemetry.trace.SpanContext;
import java.util.Iterator;
import lombok.extern.slf4j.Slf4j;
import org.apache.kafka.clients.consumer.ConsumerRecord;
@@ -57,12 +56,9 @@ public class TracingIterator implements Iterator<ConsumerRecord> {
if (next != null) {
final Span.Builder spanBuilder = TRACER.spanBuilder(operationName).setSpanKind(CONSUMER);
try {
- final SpanContext extractedContext =
- TRACER.getHttpTextFormat().extract(next.headers(), GETTER);
- spanBuilder.setParent(extractedContext);
+ spanBuilder.addLink(TRACER.getHttpTextFormat().extract(next.headers(), GETTER));
} catch (final IllegalArgumentException e) {
- // Couldn't extract a context. We should treat this as a root span.
- spanBuilder.setNoParent();
+ // Couldn't extract a context
}
final Span span = spanBuilder.startSpan();
decorator.afterStart(span);

MODIFY
instrumentation/kafka-clients-0.11/src/test/groovy/KafkaClientTest.groovy
instrumentation/kafka-clients-0.11/src/test/groovy/KafkaClientTest.groovy
@@ -81,8 +81,8 @@ class KafkaClientTest extends AgentTestRunner {
received.value() == greeting
received.key() == null
- assertTraces(1) {
- trace(0, 2) {
+ assertTraces(2) {
+ trace(0, 1) {
span(0) {
operationName "kafka.produce"
spanKind PRODUCER
@@ -95,11 +95,13 @@ class KafkaClientTest extends AgentTestRunner {
"$Tags.COMPONENT" "java-kafka"
}
}
- span(1) {
+ }
+ trace(1, 1) {
+ span(0) {
operationName "kafka.consume"
spanKind CONSUMER
errored false
- childOf span(0)
+ hasLink traces[0][0]
tags {
"$MoreTags.SERVICE_NAME" "kafka"
"$MoreTags.RESOURCE_NAME" "Consume Topic $SHARED_TOPIC"
@@ -152,8 +154,8 @@ class KafkaClientTest extends AgentTestRunner {
first.value() == greeting
first.key() == null
- assertTraces(1) {
- trace(0, 2) {
+ assertTraces(2) {
+ trace(0, 1) {
span(0) {
operationName "kafka.produce"
spanKind PRODUCER
@@ -167,11 +169,13 @@ class KafkaClientTest extends AgentTestRunner {
"kafka.partition" { it >= 0 }
}
}
- span(1) {
+ }
+ trace(1, 1) {
+ span(0) {
operationName "kafka.consume"
spanKind CONSUMER
errored false
- childOf span(0)
+ hasLink traces[0][0]
tags {
"$MoreTags.SERVICE_NAME" "kafka"
"$MoreTags.RESOURCE_NAME" "Consume Topic $SHARED_TOPIC"

MODIFY
instrumentation/kafka-streams-0.11/src/test/groovy/KafkaStreamsTest.groovy
instrumentation/kafka-streams-0.11/src/test/groovy/KafkaStreamsTest.groovy
@@ -119,8 +119,8 @@ class KafkaStreamsTest extends AgentTestRunner {
received.value() == greeting.toLowerCase()
received.key() == null
- assertTraces(1) {
- trace(0, 5) {
+ assertTraces(3) {
+ trace(0, 3) {
// PRODUCER span 0
span(0) {
operationName "kafka.produce"
@@ -134,7 +134,7 @@ class KafkaStreamsTest extends AgentTestRunner {
"$Tags.COMPONENT" "java-kafka"
}
}
- // CONSUMER span 0
+ // STREAMING span 1
span(1) {
operationName "kafka.consume"
spanKind CONSUMER
@@ -147,43 +147,47 @@ class KafkaStreamsTest extends AgentTestRunner {
"$Tags.COMPONENT" "java-kafka"
"partition" { it >= 0 }
"offset" 0
+ "asdf" "testing"
}
}
- // STREAMING span 1
+ // STREAMING span 0
span(2) {
- operationName "kafka.consume"
- spanKind CONSUMER
+ operationName "kafka.produce"
+ spanKind PRODUCER
errored false
- childOf span(0)
+ childOf span(1)
tags {
"$MoreTags.SERVICE_NAME" "kafka"
- "$MoreTags.RESOURCE_NAME" "Consume Topic $STREAM_PENDING"
+ "$MoreTags.RESOURCE_NAME" "Produce Topic $STREAM_PROCESSED"
"$MoreTags.SPAN_TYPE" "queue"
"$Tags.COMPONENT" "java-kafka"
- "partition" { it >= 0 }
- "offset" 0
- "asdf" "testing"
}
}
- // STREAMING span 0
- span(3) {
- operationName "kafka.produce"
- spanKind PRODUCER
+ }
+ trace(1, 1) {
+ // CONSUMER span 0
+ span(0) {
+ operationName "kafka.consume"
+ spanKind CONSUMER
errored false
- childOf span(2)
+ hasLink traces[0][0]
tags {
"$MoreTags.SERVICE_NAME" "kafka"
- "$MoreTags.RESOURCE_NAME" "Produce Topic $STREAM_PROCESSED"
+ "$MoreTags.RESOURCE_NAME" "Consume Topic $STREAM_PENDING"
"$MoreTags.SPAN_TYPE" "queue"
"$Tags.COMPONENT" "java-kafka"
+ "partition" { it >= 0 }
+ "offset" 0
+ }
}
}
+ trace(2, 1) {
// CONSUMER span 0
- span(4) {
+ span(0) {
operationName "kafka.consume"
spanKind CONSUMER
errored false
- childOf span(3)
+ hasLink traces[0][2]
tags {
"$MoreTags.SERVICE_NAME" "kafka"
"$MoreTags.RESOURCE_NAME" "Consume Topic $STREAM_PROCESSED"
@@ -209,8 +213,8 @@ class KafkaStreamsTest extends AgentTestRunner {
return null
}
})
- spanContext.traceId == TEST_WRITER.traces[0][3].traceId
- spanContext.spanId == TEST_WRITER.traces[0][3].spanId
+ spanContext.traceId == TEST_WRITER.traces[0][2].traceId
+ spanContext.spanId == TEST_WRITER.traces[0][2].spanId
cleanup:

MODIFY
instrumentation/rabbitmq-amqp-2.7/src/main/java/io/opentelemetry/auto/instrumentation/rabbitmq/amqp/RabbitChannelInstrumentation.java
instrumentation/rabbitmq-amqp-2.7/src/main/java/io/opentelemetry/auto/instrumentation/rabbitmq/amqp/RabbitChannelInstrumentation.java
@@ -9,7 +9,6 @@ import static io.opentelemetry.auto.instrumentation.rabbitmq.amqp.TextMapExtract
import static io.opentelemetry.auto.instrumentation.rabbitmq.amqp.TextMapInjectAdapter.SETTER;
import static io.opentelemetry.auto.tooling.ByteBuddyElementMatchers.safeHasSuperType;
import static io.opentelemetry.trace.Span.Kind.CLIENT;
-import static io.opentelemetry.trace.Span.Kind.CONSUMER;
import static io.opentelemetry.trace.Span.Kind.PRODUCER;
import static net.bytebuddy.matcher.ElementMatchers.canThrow;
import static net.bytebuddy.matcher.ElementMatchers.isGetter;
@@ -37,7 +36,6 @@ import io.opentelemetry.auto.instrumentation.api.Tags;
import io.opentelemetry.auto.tooling.Instrumenter;
import io.opentelemetry.context.Scope;
import io.opentelemetry.trace.Span;
-import io.opentelemetry.trace.SpanContext;
import java.io.IOException;
import java.util.HashMap;
import java.util.LinkedHashMap;
@@ -222,14 +220,13 @@ public class RabbitChannelInstrumentation extends Instrumenter.Default {
if (callDepth > 0) {
return;
}
- final SpanContext parentContext = null;
- // TODO: it would be better if we could actually have span wrapped into the scope started in
- // OnMethodEnter
+ // can't create span and put into scope in method enter above, because can't add links after
+ // span creation
final Span.Builder spanBuilder =
TRACER
.spanBuilder("amqp.command")
- .setSpanKind(CONSUMER)
+ .setSpanKind(CLIENT)
.setStartTimestamp(TimeUnit.MILLISECONDS.toNanos(startTime));
if (response != null && response.getProps() != null) {
@@ -237,10 +234,9 @@ public class RabbitChannelInstrumentation extends Instrumenter.Default {
if (headers != null) {
try {
- spanBuilder.setParent(TRACER.getHttpTextFormat().extract(headers, GETTER));
+ spanBuilder.addLink(TRACER.getHttpTextFormat().extract(headers, GETTER));
} catch (final IllegalArgumentException e) {
// couldn't extract a context
- spanBuilder.setNoParent();
}
}
}

MODIFY
instrumentation/rabbitmq-amqp-2.7/src/test/groovy/RabbitMQTest.groovy
instrumentation/rabbitmq-amqp-2.7/src/test/groovy/RabbitMQTest.groovy
@@ -136,13 +136,15 @@ class RabbitMQTest extends AgentTestRunner {
new String(response.getBody()) == "Hello, world!"
and:
- assertTraces(2) {
+ assertTraces(3) {
trace(0, 1) {
rabbitSpan(it, 0, "queue.declare")
}
- trace(1, 2) {
+ trace(1, 1) {
rabbitSpan(it, 0, "basic.publish <default> -> <generated>")
- rabbitSpan(it, 1, "basic.get <generated>", true, span(0))
+ }
+ trace(2, 1) {
+ rabbitSpan(it, 0, "basic.get <generated>", true, traces[1][0])
}
}
}
@@ -285,13 +287,15 @@ class RabbitMQTest extends AgentTestRunner {
message == "foo"
and:
- assertTraces(2) {
+ assertTraces(3) {
trace(0, 1) {
rabbitSpan(it, "queue.declare")
}
- trace(1, 2) {
+ trace(1, 1) {
rabbitSpan(it, 0, "basic.publish <default> -> some-routing-queue")
- rabbitSpan(it, 1, "basic.get $queue.name", true, span(0))
+ }
+ trace(2, 1) {
+ rabbitSpan(it, 0, "basic.get $queue.name", true, traces[1][0])
}
}
}
@@ -312,7 +316,7 @@ class RabbitMQTest extends AgentTestRunner {
int index,
String resource,
Boolean distributedRootSpan = false,
- Object parentSpan = null,
+ Object parentOrLinkSpan = null,
Throwable exception = null,
String errorMsg = null
) {
@@ -324,7 +328,7 @@ class RabbitMQTest extends AgentTestRunner {
spanKind PRODUCER
break
case "basic.get":
- spanKind CONSUMER
+ spanKind CLIENT
break
case "basic.deliver":
spanKind CONSUMER
@@ -333,8 +337,12 @@ class RabbitMQTest extends AgentTestRunner {
spanKind CLIENT
}
- if (parentSpan) {
- childOf((SpanData) parentSpan)
+ if (parentOrLinkSpan) {
+ if (trace.span(index).attributes.get("amqp.command")?.stringValue == "basic.get") {
+ hasLink((SpanData) parentOrLinkSpan)
+ } else {
+ childOf((SpanData) parentOrLinkSpan)
+ }
} else {
parent()
}

MODIFY
testing/src/main/groovy/io/opentelemetry/auto/test/asserts/SpanAssert.groovy
testing/src/main/groovy/io/opentelemetry/auto/test/asserts/SpanAssert.groovy
@@ -74,6 +74,17 @@ class SpanAssert {
traceId(parent.traceId.toLowerBase16())
}
+ def hasLink(SpanData linked) {
+ def found
+ for (def link : span.links) {
+ if (link.context.traceId == linked.traceId && link.context.spanId == linked.spanId) {
+ found = true
+ break
+ }
+ }
+ assert found
+ }
+
def errored(boolean errored) {
if (errored) {
assert span.status != Status.OK

Use links instead of overriding parent (#158)

Also, message receive span kind should be CLIENT.