Disable the messaging receive span telemetry by default (open-telemet…

…ry#5500) * Disable the messaging receive span telemetry by default * fix spring-kafka tests * remove no longer needed link from the kafka-clients library instrumentation Co-authored-by: Trask Stalnaker <[email protected]>
RashmiRam · May 23, 2022 · bc8175e · bc8175e
1 parent 3fff33e
commit bc8175e
Show file tree

Hide file tree

Showing 11 changed files with 37 additions and 30 deletions.
diff --git a/docs/config/common.md b/docs/config/common.md
@@ -53,12 +53,27 @@ These configuration options are supported by all HTTP client and server instrume
 ## Capturing servlet request parameters
 
 You can configure the agent to capture predefined HTTP request parameter as span attributes for
-requests that are handled by Servlet API.
-Use the following property to define which servlet request parameters you want to capture:
+requests that are handled by Servlet API. Use the following property to define which servlet request
+parameters you want to capture:
 
 | System property                                                        | Environment variable                                                   | Description |
 | ---------------------------------------------------------------------- | ---------------------------------------------------------------------- | ----------- |
 | `otel.instrumentation.servlet.experimental.capture-request-parameters` | `OTEL_INSTRUMENTATION_SERVLET_EXPERIMENTAL_CAPTURE_REQUEST_PARAMETERS` | A comma-separated list of request parameter names.
 
 > **Note**: The property/environment variable names listed in the table are still experimental,
 > and thus are subject to change.
+
+## Capturing consumer message receive telemetry in messaging instrumentations
+
+You can configure the agent to capture the consumer message receive telemetry in messaging
+instrumentation. Use the following property to enable it:
+
+| System property                                                         | Environment variable                                                    | Description |
+| ----------------------------------------------------------------------- | ----------------------------------------------------------------------- | ----------- |
+| `otel.instrumentation.messaging.experimental.receive-telemetry.enabled` | `OTEL_INSTRUMENTATION_MESSAGING_EXPERIMENTAL_RECEIVE_TELEMETRY_ENABLED` | Enables the consumer message receive telemetry. The default value is `false`.
+
+Note that this will cause the consumer side to start a new trace, with only a span link connecting
+it to the producer trace.
+
+> **Note**: The property/environment variable names listed in the table are still experimental,
+> and thus are subject to change.
diff --git a/...ion-api/src/main/java/io/opentelemetry/instrumentation/api/config/ExperimentalConfig.java b/...ion-api/src/main/java/io/opentelemetry/instrumentation/api/config/ExperimentalConfig.java
@@ -32,8 +32,13 @@ public boolean suppressViewSpans() {
     return config.getBoolean("otel.instrumentation.common.experimental.suppress-view-spans", false);
   }
 
-  public boolean suppressMessagingReceiveSpans() {
+  public boolean messagingReceiveInstrumentationEnabled() {
+    // TODO: remove that `suppress...` flag after 1.13 release
+    boolean receiveSpansSuppressed =
+        config.getBoolean(
+            "otel.instrumentation.common.experimental.suppress-messaging-receive-spans", true);
     return config.getBoolean(
-        "otel.instrumentation.common.experimental.suppress-messaging-receive-spans", false);
+        "otel.instrumentation.messaging.experimental.receive-telemetry.enabled",
+        !receiveSpansSuppressed);
   }
 }
diff --git a/instrumentation/jms-1.1/javaagent/build.gradle.kts b/instrumentation/jms-1.1/javaagent/build.gradle.kts
@@ -30,7 +30,6 @@ tasks {
       isFailOnNoMatchingTests = false
     }
     include("**/SpringListenerJms1SuppressReceiveSpansTest.*")
-    jvmArgs("-Dotel.instrumentation.common.experimental.suppress-messaging-receive-spans=true")
   }
 
   val jms2Test by existing(Test::class) {
@@ -39,13 +38,13 @@ tasks {
       // running a single test in the default test set will fail
       isFailOnNoMatchingTests = false
     }
+    jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
   }
 
   val jms2TestReceiveSpansDisabled by existing(Test::class) {
     filter {
       isFailOnNoMatchingTests = false
     }
-    jvmArgs("-Dotel.instrumentation.common.experimental.suppress-messaging-receive-spans=true")
   }
 
   test {
@@ -57,6 +56,7 @@ tasks {
       excludeTestsMatching("SpringListenerJms1SuppressReceiveSpansTest")
       isFailOnNoMatchingTests = false
     }
+    jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
   }
 }
 

diff --git a/...javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsSingletons.java b/...javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsSingletons.java
@@ -46,7 +46,7 @@ private static Instrumenter<MessageWithDestination, Void> buildConsumerInstrumen
             GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, spanNameExtractor)
         .addAttributesExtractor(attributesExtractor)
         .setTimeExtractor(new JmsMessageTimeExtractor())
-        .setDisabled(ExperimentalConfig.get().suppressMessagingReceiveSpans())
+        .setDisabled(!ExperimentalConfig.get().messagingReceiveInstrumentationEnabled())
         .newInstrumenter(SpanKindExtractor.alwaysConsumer());
   }
 

diff --git a/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/build.gradle.kts b/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/build.gradle.kts
@@ -47,7 +47,6 @@ tasks {
       isFailOnNoMatchingTests = false
     }
     include("**/KafkaClientSuppressReceiveSpansTest.*")
-    jvmArgs("-Dotel.instrumentation.common.experimental.suppress-messaging-receive-spans=true")
   }
 
   test {
@@ -58,5 +57,6 @@ tasks {
       excludeTestsMatching("KafkaClientSuppressReceiveSpansTest")
       isFailOnNoMatchingTests = false
     }
+    jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
   }
 }
diff --git a/...y/io/opentelemetry/instrumentation/kafkaclients/KafkaClientPropagationDisabledTest.groovy b/...y/io/opentelemetry/instrumentation/kafkaclients/KafkaClientPropagationDisabledTest.groovy
@@ -60,22 +60,10 @@ class KafkaClientPropagationDisabledTest extends KafkaClientPropagationBaseTest
           }
         }
       }
-      trace(1, 3) {
+      trace(1, 2) {
         span(0) {
-          name SHARED_TOPIC + " receive"
-          kind CONSUMER
-          hasNoParent()
-          attributes {
-            "$SemanticAttributes.MESSAGING_SYSTEM" "kafka"
-            "$SemanticAttributes.MESSAGING_DESTINATION" SHARED_TOPIC
-            "$SemanticAttributes.MESSAGING_DESTINATION_KIND" "topic"
-            "$SemanticAttributes.MESSAGING_OPERATION" "receive"
-          }
-        }
-        span(1) {
           name SHARED_TOPIC + " process"
           kind CONSUMER
-          childOf span(0)
           hasNoLinks()
           attributes {
             "$SemanticAttributes.MESSAGING_SYSTEM" "kafka"
@@ -87,9 +75,9 @@ class KafkaClientPropagationDisabledTest extends KafkaClientPropagationBaseTest
             "kafka.offset" Long
             "kafka.record.queue_time_ms" { it >= 0 }
           }
-          span(2) {
+          span(1) {
             name "processing"
-            childOf span(1)
+            childOf span(0)
           }
         }
       }

diff --git a/...ary/src/test/groovy/io/opentelemetry/instrumentation/kafkaclients/InterceptorsTest.groovy b/...ary/src/test/groovy/io/opentelemetry/instrumentation/kafkaclients/InterceptorsTest.groovy
@@ -80,7 +80,6 @@ class InterceptorsTest extends KafkaClientBaseTest implements LibraryTestTrait {
           name SHARED_TOPIC + " receive"
           kind CONSUMER
           childOf span(1)
-          hasLink(span(1))
           attributes {
             "$SemanticAttributes.MESSAGING_SYSTEM" "kafka"
             "$SemanticAttributes.MESSAGING_DESTINATION" SHARED_TOPIC

diff --git a/...library/src/test/groovy/io/opentelemetry/instrumentation/kafkaclients/WrappersTest.groovy b/...library/src/test/groovy/io/opentelemetry/instrumentation/kafkaclients/WrappersTest.groovy
@@ -68,7 +68,6 @@ class WrappersTest extends KafkaClientBaseTest implements LibraryTestTrait {
           name SHARED_TOPIC + " receive"
           kind CONSUMER
           childOf span(1)
-          hasLink(span(1))
           attributes {
             "$SemanticAttributes.MESSAGING_SYSTEM" "kafka"
             "$SemanticAttributes.MESSAGING_DESTINATION" SHARED_TOPIC

diff --git a/...c/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaInstrumenterFactory.java b/...c/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaInstrumenterFactory.java
@@ -67,7 +67,7 @@ public static Instrumenter<ReceivedRecords, Void> createConsumerReceiveInstrumen
         .addAttributesExtractor(attributesExtractor)
         .addAttributesExtractors(extractors)
         .setTimeExtractor(new KafkaConsumerTimeExtractor())
-        .setDisabled(ExperimentalConfig.get().suppressMessagingReceiveSpans())
+        .setDisabled(!ExperimentalConfig.get().messagingReceiveInstrumentationEnabled())
         .newInstrumenter(SpanKindExtractor.alwaysConsumer());
   }
 
@@ -102,13 +102,13 @@ public static Instrumenter<ReceivedRecords, Void> createConsumerReceiveInstrumen
 
     if (!KafkaPropagation.isPropagationEnabled()) {
       return builder.newInstrumenter(SpanKindExtractor.alwaysConsumer());
-    } else if (ExperimentalConfig.get().suppressMessagingReceiveSpans()) {
-      return builder.newConsumerInstrumenter(KafkaConsumerRecordGetter.INSTANCE);
-    } else {
+    } else if (ExperimentalConfig.get().messagingReceiveInstrumentationEnabled()) {
       builder.addSpanLinksExtractor(
           SpanLinksExtractor.fromUpstreamRequest(
               GlobalOpenTelemetry.getPropagators(), KafkaConsumerRecordGetter.INSTANCE));
       return builder.newInstrumenter(SpanKindExtractor.alwaysConsumer());
+    } else {
+      return builder.newConsumerInstrumenter(KafkaConsumerRecordGetter.INSTANCE);
     }
   }
 

diff --git a/instrumentation/kafka/kafka-streams-0.11/javaagent/build.gradle.kts b/instrumentation/kafka/kafka-streams-0.11/javaagent/build.gradle.kts
@@ -37,7 +37,6 @@ tasks {
       isFailOnNoMatchingTests = false
     }
     include("**/KafkaStreamsSuppressReceiveSpansTest.*")
-    jvmArgs("-Dotel.instrumentation.common.experimental.suppress-messaging-receive-spans=true")
   }
 
   test {
@@ -46,5 +45,6 @@ tasks {
       excludeTestsMatching("KafkaStreamsSuppressReceiveSpansTest")
       isFailOnNoMatchingTests = false
     }
+    jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
   }
 }
diff --git a/instrumentation/spring/spring-kafka-2.7/javaagent/build.gradle.kts b/instrumentation/spring/spring-kafka-2.7/javaagent/build.gradle.kts
@@ -34,5 +34,6 @@ tasks {
 
     // TODO run tests both with and without experimental span attributes
     jvmArgs("-Dotel.instrumentation.kafka.experimental-span-attributes=true")
+    jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
   }
 }