Code review sweep (run 25084109220)

instrumentation/spring/spring-integration-4.1/testing/build.gradle.kts

@@ -13,3 +13,11 @@ dependencies {
   compileOnly("org.springframework.cloud:spring-cloud-stream:2.2.1.RELEASE")
   compileOnly("org.springframework.cloud:spring-cloud-stream-binder-rabbit:2.2.1.RELEASE")
 }
+
+tasks {
+  javadoc {
+    // Abstract test bases in this module are package-private and consumed by
+    // sibling test sources in the same package; nothing public to document.
+    enabled = false
+  }
+}

instrumentation/spring/spring-integration-4.1/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/integration/v4_1/AbstractComplexPropagationTest.java

@@ -33,14 +33,14 @@
 import org.springframework.messaging.Message;
 import org.springframework.messaging.SubscribableChannel;
 
-public abstract class AbstractComplexPropagationTest {
+abstract class AbstractComplexPropagationTest {
 
   private final Class<?> additionalContextClass;
-  protected final InstrumentationExtension testing;
+  private final InstrumentationExtension testing;
 
   private ConfigurableApplicationContext applicationContext;
 
-  public AbstractComplexPropagationTest(
+  AbstractComplexPropagationTest(
       InstrumentationExtension testing, Class<?> additionalContextClass) {
     this.testing = testing;
     this.additionalContextClass = additionalContextClass;

instrumentation/spring/spring-integration-4.1/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/integration/v4_1/AbstractSpringCloudStreamProducerTest.java

@@ -12,16 +12,16 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
-public abstract class AbstractSpringCloudStreamProducerTest {
+abstract class AbstractSpringCloudStreamProducerTest {
 
   private static final boolean HAS_PRODUCER_SPAN =
       Boolean.getBoolean("otel.instrumentation.spring-integration.producer.enabled");
 
   @RegisterExtension private final RabbitExtension rabbit;
 
-  protected final InstrumentationExtension testing;
+  private final InstrumentationExtension testing;
 
-  public AbstractSpringCloudStreamProducerTest(
+  AbstractSpringCloudStreamProducerTest(
       InstrumentationExtension testing, Class<?> additionalContextClass) {
     this.testing = testing;
     rabbit = new RabbitExtension(additionalContextClass);

instrumentation/spring/spring-integration-4.1/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/integration/v4_1/AbstractSpringCloudStreamRabbitTest.java

@@ -10,13 +10,13 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
-public abstract class AbstractSpringCloudStreamRabbitTest {
+abstract class AbstractSpringCloudStreamRabbitTest {
 
   @RegisterExtension private final RabbitExtension rabbit;
 
-  protected final InstrumentationExtension testing;
+  private final InstrumentationExtension testing;
 
-  public AbstractSpringCloudStreamRabbitTest(
+  AbstractSpringCloudStreamRabbitTest(
       InstrumentationExtension testing, Class<?> additionalContextClass) {
     this.testing = testing;
     rabbit = new RabbitExtension(additionalContextClass);

instrumentation/spring/spring-integration-4.1/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/integration/v4_1/AbstractSpringIntegrationTracingTest.java

@@ -37,7 +37,7 @@
 
 abstract class AbstractSpringIntegrationTracingTest {
 
-  protected final InstrumentationExtension testing;
+  private final InstrumentationExtension testing;
 
   private final Class<?> additionalContextClass;
 

instrumentation/spring/spring-integration-4.1/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/integration/v4_1/RabbitExtension.java

@@ -29,19 +29,19 @@
 import org.testcontainers.containers.GenericContainer;
 import org.testcontainers.containers.wait.strategy.Wait;
 
-public class RabbitExtension implements BeforeEachCallback, AfterEachCallback {
+class RabbitExtension implements BeforeEachCallback, AfterEachCallback {
 
   private final Class<?> additionalContextClass;
 
   private GenericContainer<?> rabbitMqContainer;
   private ConfigurableApplicationContext producerContext;
   private ConfigurableApplicationContext consumerContext;
 
-  public RabbitExtension(Class<?> additionalContextClass) {
+  RabbitExtension(Class<?> additionalContextClass) {
     this.additionalContextClass = additionalContextClass;
   }
 
-  public <T> T getBean(String name, Class<T> requiredType) {
+  <T> T getBean(String name, Class<T> requiredType) {
     return producerContext.getBean(name, requiredType);
   }
 

instrumentation/spring/spring-jms/spring-jms-2.0/javaagent/build.gradle.kts

@@ -67,6 +67,10 @@ tasks {
   // this does not apply to testReceiveSpansDisabled
   test {
     jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
+    systemProperty(
+      "metadataConfig",
+      "otel.instrumentation.messaging.experimental.receive-telemetry.enabled=true",
+    )
   }
 
   check {

instrumentation/spring/spring-jms/spring-jms-2.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/jms/v2_0/SpringTemplateTest.java

@@ -57,7 +57,7 @@ class SpringTemplateTest extends AbstractJmsTest {
   private static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
 
   private static HornetQServer server;
-  private static final String messageText = "a message";
+  private static final String MESSAGE_TEXT = "a message";
   private static JmsTemplate template;
   private static Session session;
   private static Connection connection;
@@ -112,11 +112,11 @@ static void setup() throws Exception {
   @Test
   void sendingMessageToDestinationNameGeneratesSpans() throws JMSException {
     Queue queue = session.createQueue("SpringTemplateJms2");
-    template.convertAndSend(queue, messageText);
+    template.convertAndSend(queue, MESSAGE_TEXT);
     TextMessage receivedMessage = (TextMessage) template.receive(queue);
 
     assertThat(receivedMessage).isNotNull();
-    assertThat(receivedMessage.getText()).isEqualTo(messageText);
+    assertThat(receivedMessage.getText()).isEqualTo(MESSAGE_TEXT);
 
     String receivedMsgId = receivedMessage.getJMSMessageID();
     AtomicReference<SpanData> producerSpan = new AtomicReference<>();
@@ -148,7 +148,7 @@ void sendAndReceiveMessageGeneratesSpans() throws JMSException {
           TextMessage msg = (TextMessage) template.receive(queue);
           assertThat(msg).isNotNull();
           try {
-            assertThat(msg.getText()).isEqualTo(messageText);
+            assertThat(msg.getText()).isEqualTo(MESSAGE_TEXT);
             msgId.set(msg.getJMSMessageID());
             // There's a chance this might be reported last, messing up the assertion.
             template.send(
@@ -167,7 +167,8 @@ void sendAndReceiveMessageGeneratesSpans() throws JMSException {
             template.sendAndReceive(
                 queue,
                 session ->
-                    requireNonNull(template.getMessageConverter()).toMessage(messageText, session));
+                    requireNonNull(template.getMessageConverter())
+                        .toMessage(MESSAGE_TEXT, session));
 
     assertThat(receivedMessage).isNotNull();
     assertThat(receivedMessage.getText()).isEqualTo("responded!");
@@ -219,7 +220,7 @@ void captureMessageHeaderAsSpanAttribute() throws JMSException {
     Queue queue = session.createQueue("SpringTemplateJms2");
     template.convertAndSend(
         queue,
-        messageText,
+        MESSAGE_TEXT,
         new MessagePostProcessor() {
           @Override
           public @NotNull Message postProcessMessage(@NotNull Message message) throws JMSException {
@@ -231,7 +232,7 @@ void captureMessageHeaderAsSpanAttribute() throws JMSException {
     TextMessage receivedMessage = (TextMessage) template.receive(queue);
 
     assertThat(receivedMessage).isNotNull();
-    assertThat(receivedMessage.getText()).isEqualTo(messageText);
+    assertThat(receivedMessage.getText()).isEqualTo(MESSAGE_TEXT);
 
     String receivedMsgId = receivedMessage.getJMSMessageID();
     AtomicReference<SpanData> producerSpan = new AtomicReference<>();

instrumentation/spring/spring-jms/spring-jms-2.0/testing/src/main/java/io/opentelemetry/instrumentation/spring/jms/v2_0/AbstractJmsTest.java

@@ -24,6 +24,7 @@
 import io.opentelemetry.sdk.trace.data.SpanData;
 import java.util.ArrayList;
 import java.util.List;
+import javax.annotation.Nullable;
 
 public abstract class AbstractJmsTest {
 
@@ -62,12 +63,12 @@ protected List<AttributeAssertion> producerAttributeAssertions(
 
   protected void assertConsumerSpan(
       SpanDataAssert span,
-      SpanData producer,
-      SpanData parent,
+      @Nullable SpanData producer,
+      @Nullable SpanData parent,
       String destinationName,
       String operation,
       boolean testHeaders,
-      String msgId) {
+      @Nullable String msgId) {
     span.hasName(destinationName + " " + operation).hasKind(CONSUMER);
     if (parent != null) {
       span.hasParent(parent);
@@ -83,7 +84,7 @@ protected void assertConsumerSpan(
 
   @SuppressWarnings("deprecation") // using deprecated semconv
   protected List<AttributeAssertion> consumerAttributeAssertions(
-      String destinationName, boolean testHeaders, String operation, String msgId) {
+      String destinationName, boolean testHeaders, String operation, @Nullable String msgId) {
     List<AttributeAssertion> attributeAssertions =
         new ArrayList<>(
             asList(

instrumentation/spring/spring-jms/spring-jms-6.0/javaagent/build.gradle.kts

@@ -23,6 +23,7 @@ dependencies {
   compileOnly("jakarta.jms:jakarta.jms-api:3.0.0")
 
   testInstrumentation(project(":instrumentation:jms:jms-3.0:javaagent"))
+  testInstrumentation(project(":instrumentation:spring:spring-jms:spring-jms-2.0:javaagent"))
 
   testImplementation("org.apache.activemq:artemis-jakarta-client:2.27.1")
 
@@ -55,6 +56,10 @@ tasks {
       excludeTestsMatching("SpringListenerSuppressReceiveSpansTest")
     }
     jvmArgs("-Dotel.instrumentation.messaging.experimental.receive-telemetry.enabled=true")
+    systemProperty(
+      "metadataConfig",
+      "otel.instrumentation.messaging.experimental.receive-telemetry.enabled=true",
+    )
   }
 
   check {

instrumentation/spring/spring-jms/spring-jms-6.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/jms/v6_0/AbstractSpringJmsListenerTest.java

@@ -16,8 +16,6 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeoutException;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -59,8 +57,7 @@ static void setUp() {
   @ParameterizedTest
   @ValueSource(classes = {AnnotatedListenerConfig.class, ManualListenerConfig.class})
   @SuppressWarnings("unchecked")
-  void testSpringJmsListener(Class<?> configClass)
-      throws ExecutionException, InterruptedException, TimeoutException {
+  void testSpringJmsListener(Class<?> configClass) throws Exception {
     // given
     SpringApplication app = new SpringApplication(configClass);
     app.setDefaultProperties(defaultConfig());

instrumentation/spring/spring-jms/spring-jms-6.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/spring/jms/v6_0/SpringJmsListenerTest.java

@@ -24,8 +24,6 @@
 import io.opentelemetry.sdk.trace.data.SpanData;
 import jakarta.jms.ConnectionFactory;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicReference;
 import org.assertj.core.api.AbstractStringAssert;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -85,8 +83,7 @@ void assertSpringJmsListener() {
   @ParameterizedTest
   @ValueSource(classes = {AnnotatedListenerConfig.class, ManualListenerConfig.class})
   @SuppressWarnings("unchecked")
-  void shouldCaptureHeaders(Class<?> configClass)
-      throws ExecutionException, InterruptedException, TimeoutException {
+  void shouldCaptureHeaders(Class<?> configClass) throws Exception {
     // given
     SpringApplication app = new SpringApplication(configClass);
     app.setDefaultProperties(defaultConfig());