Use nano time in JMS consumer Instrumenter (#3986)

* Use nano time in JMS consumer Instrumenter

* fix DoNotMockAutoValue

instrumentation/jms-1.1/javaagent-unit-tests/src/test/java/io/opentelemetry/javaagent/instrumentation/jms/MessageWithDestinationTest.java

@@ -20,6 +20,7 @@ import javax.jms.Queue;
 import javax.jms.TemporaryQueue;
 import javax.jms.TemporaryTopic;
 import javax.jms.Topic;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -38,6 +39,12 @@ class MessageWithDestinationTest {
   @Mock Queue queue;
   @Mock TemporaryQueue temporaryQueue;
   @Mock Destination destination;
+  @Mock Timer timer;
+
+  @BeforeEach
+  void setUp() {
+    given(timer.startTime()).willReturn(START_TIME);
+  }
 
   @Test
   void shouldCreateMessageWithUnknownDestination() throws JMSException {
@@ -46,16 +53,11 @@ class MessageWithDestinationTest {
 
     // when
     MessageWithDestination result =
-        MessageWithDestination.create(message, MessageOperation.SEND, null, START_TIME);
+        MessageWithDestination.create(message, MessageOperation.SEND, null, timer);
 
     // then
     assertMessage(
-        MessageOperation.SEND,
+        MessageOperation.SEND, "unknown", "unknown", /* expectedTemporary= */ false, result);
-        "unknown",
-        "unknown",
-        /* expectedTemporary= */ false,
-        START_TIME,
-        result);
   }
 
   @Test
@@ -65,16 +67,11 @@ class MessageWithDestinationTest {
 
     // when
     MessageWithDestination result =
-        MessageWithDestination.create(message, MessageOperation.SEND, destination, START_TIME);
+        MessageWithDestination.create(message, MessageOperation.SEND, destination, timer);
 
     // then
     assertMessage(
-        MessageOperation.SEND,
+        MessageOperation.SEND, "unknown", "unknown", /* expectedTemporary= */ false, result);
-        "unknown",
-        "unknown",
-        /* expectedTemporary= */ false,
-        START_TIME,
-        result);
   }
 
   @ParameterizedTest
@@ -97,16 +94,11 @@ class MessageWithDestinationTest {
 
     // when
     MessageWithDestination result =
-        MessageWithDestination.create(message, MessageOperation.RECEIVE, null);
+        MessageWithDestination.create(message, MessageOperation.RECEIVE, null, timer);
 
     // then
     assertMessage(
-        MessageOperation.RECEIVE,
+        MessageOperation.RECEIVE, "queue", expectedDestinationName, expectedTemporary, result);
-        "queue",
-        expectedDestinationName,
-        expectedTemporary,
-        null,
-        result);
   }
 
   @ParameterizedTest
@@ -129,16 +121,11 @@ class MessageWithDestinationTest {
 
     // when
     MessageWithDestination result =
-        MessageWithDestination.create(message, MessageOperation.RECEIVE, null);
+        MessageWithDestination.create(message, MessageOperation.RECEIVE, null, timer);
 
     // then
     assertMessage(
-        MessageOperation.RECEIVE,
+        MessageOperation.RECEIVE, "topic", expectedDestinationName, expectedTemporary, result);
-        "topic",
-        expectedDestinationName,
-        expectedTemporary,
-        null,
-        result);
   }
 
   static Stream<Arguments> destinations() {
@@ -154,14 +141,13 @@ class MessageWithDestinationTest {
       String expectedDestinationKind,
       String expectedDestinationName,
       boolean expectedTemporary,
-      Instant expectedStartTime,
       MessageWithDestination actual) {
 
-    assertSame(message, actual.getMessage());
+    assertSame(message, actual.message());
-    assertSame(expectedMessageOperation, actual.getMessageOperation());
+    assertSame(expectedMessageOperation, actual.messageOperation());
-    assertEquals(expectedDestinationKind, actual.getDestinationKind());
+    assertEquals(expectedDestinationKind, actual.destinationKind());
-    assertEquals(expectedDestinationName, actual.getDestinationName());
+    assertEquals(expectedDestinationName, actual.destinationName());
     assertEquals(expectedTemporary, actual.isTemporaryDestination());
-    assertEquals(expectedStartTime, actual.getStartTime());
+    assertEquals(START_TIME, actual.startTime());
   }
 }

instrumentation/jms-1.1/javaagent-unit-tests/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker

@@ -0,0 +1 @@
+mock-maker-inline

instrumentation/jms-1.1/javaagent/build.gradle.kts

@@ -38,6 +38,9 @@ tasks {
 val versions: Map<String, String> by project
 
 dependencies {
+  compileOnly("com.google.auto.value:auto-value-annotations")
+  annotationProcessor("com.google.auto.value:auto-value")
+
   compileOnly("javax.jms:jms-api:1.1-rev-1")
 
   testImplementation("javax.annotation:javax.annotation-api:1.3.2")

instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsMessageAttributesExtractor.java

@@ -25,13 +25,13 @@ public class JmsMessageAttributesExtractor
   @Nullable
   @Override
   protected String destinationKind(MessageWithDestination messageWithDestination) {
-    return messageWithDestination.getDestinationKind();
+    return messageWithDestination.destinationKind();
   }
 
   @Nullable
   @Override
   protected String destination(MessageWithDestination messageWithDestination) {
-    return messageWithDestination.getDestinationName();
+    return messageWithDestination.destinationName();
   }
 
   @Override
@@ -61,7 +61,7 @@ public class JmsMessageAttributesExtractor
   @Override
   protected String conversationId(MessageWithDestination messageWithDestination) {
     try {
-      return messageWithDestination.getMessage().getJMSCorrelationID();
+      return messageWithDestination.message().getJMSCorrelationID();
     } catch (JMSException e) {
       logger.debug("Failure getting JMS correlation id", e);
       return null;
@@ -82,14 +82,14 @@ public class JmsMessageAttributesExtractor
 
   @Override
   protected MessageOperation operation(MessageWithDestination messageWithDestination) {
-    return messageWithDestination.getMessageOperation();
+    return messageWithDestination.messageOperation();
   }
 
   @Nullable
   @Override
   protected String messageId(MessageWithDestination messageWithDestination, Void unused) {
     try {
-      return messageWithDestination.getMessage().getJMSMessageID();
+      return messageWithDestination.message().getJMSMessageID();
     } catch (JMSException e) {
       logger.debug("Failure getting JMS message id", e);
       return null;

instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsMessageConsumerInstrumentation.java

@@ -17,7 +17,6 @@ import io.opentelemetry.instrumentation.api.instrumenter.messaging.MessageOperat
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
-import java.time.Instant;
 import javax.jms.Message;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
@@ -49,13 +48,13 @@ public class JmsMessageConsumerInstrumentation implements TypeInstrumentation {
   public static class ConsumerAdvice {
 
     @Advice.OnMethodEnter
-    public static Instant onEnter() {
+    public static Timer onEnter() {
-      return Instant.now();
+      return Timer.start();
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stopSpan(
-        @Advice.Enter Instant startTime,
+        @Advice.Enter Timer timer,
         @Advice.Return Message message,
         @Advice.Thrown Throwable throwable) {
       if (message == null) {
@@ -65,7 +64,7 @@ public class JmsMessageConsumerInstrumentation implements TypeInstrumentation {
 
       Context parentContext = Java8BytecodeBridge.currentContext();
       MessageWithDestination request =
-          MessageWithDestination.create(message, MessageOperation.RECEIVE, null, startTime);
+          MessageWithDestination.create(message, MessageOperation.RECEIVE, null, timer);
 
       if (consumerInstrumenter().shouldStart(parentContext, request)) {
         Context context = consumerInstrumenter().start(parentContext, request);

instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/JmsSingletons.java

@@ -11,7 +11,6 @@ import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.messaging.MessagingSpanNameExtractor;
-import java.time.Instant;
 
 public final class JmsSingletons {
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.jms-1.1";
@@ -37,7 +36,7 @@ public final class JmsSingletons {
                 otel, INSTRUMENTATION_NAME, spanNameExtractor)
             .addAttributesExtractor(attributesExtractor)
             .setTimeExtractors(
-                MessageWithDestination::getStartTime, (request, response) -> Instant.now())
+                MessageWithDestination::startTime, (request, response) -> request.endTime())
             .newInstrumenter(SpanKindExtractor.alwaysConsumer());
     LISTENER_INSTRUMENTER =
         Instrumenter.<MessageWithDestination, Void>newBuilder(

instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/MessagePropertyGetter.java

@@ -14,7 +14,7 @@ public final class MessagePropertyGetter implements TextMapGetter<MessageWithDes
   @Override
   public Iterable<String> keys(MessageWithDestination message) {
     try {
-      return Collections.list(message.getMessage().getPropertyNames());
+      return Collections.list(message.message().getPropertyNames());
     } catch (JMSException e) {
       return Collections.emptyList();
     }
@@ -25,7 +25,7 @@ public final class MessagePropertyGetter implements TextMapGetter<MessageWithDes
     String propName = key.replace("-", MessagePropertySetter.DASH);
     final Object value;
     try {
-      value = carrier.getMessage().getObjectProperty(propName);
+      value = carrier.message().getObjectProperty(propName);
     } catch (JMSException e) {
       throw new IllegalStateException(e);
     }

instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/MessagePropertySetter.java

@@ -20,7 +20,7 @@ final class MessagePropertySetter implements TextMapSetter<MessageWithDestinatio
   public void set(MessageWithDestination carrier, String key, String value) {
     String propName = key.replace("-", DASH);
     try {
-      carrier.getMessage().setStringProperty(propName, value);
+      carrier.message().setStringProperty(propName, value);
     } catch (JMSException e) {
       if (logger.isDebugEnabled()) {
         logger.debug("Failure setting jms property: {}", propName, e);

instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/MessageWithDestination.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.jms;
 
+import com.google.auto.value.AutoValue;
 import io.opentelemetry.instrumentation.api.instrumenter.messaging.MessageOperation;
 import java.time.Instant;
 import javax.jms.Destination;
@@ -14,69 +15,40 @@ import javax.jms.Queue;
 import javax.jms.TemporaryQueue;
 import javax.jms.TemporaryTopic;
 import javax.jms.Topic;
-import org.checkerframework.checker.nullness.qual.Nullable;
 
-public final class MessageWithDestination {
+@AutoValue
+public abstract class MessageWithDestination {
+
   // visible for tests
   static final String TIBCO_TMP_PREFIX = "$TMP$";
 
-  private final Message message;
+  public abstract Message message();
-  private final MessageOperation messageOperation;
-  private final String destinationName;
-  private final String destinationKind;
-  private final boolean temporaryDestination;
-  private final Instant startTime;
 
-  private MessageWithDestination(
+  public abstract MessageOperation messageOperation();
-      Message message,
+
-      MessageOperation messageOperation,
+  public abstract String destinationName();
-      String destinationName,
+
-      String destinationKind,
+  public abstract String destinationKind();
-      boolean temporary,
+
-      Instant startTime) {
+  public abstract boolean isTemporaryDestination();
-    this.message = message;
+
-    this.messageOperation = messageOperation;
+  abstract Timer timer();
-    this.destinationName = destinationName;
+
-    this.destinationKind = destinationKind;
+  public Instant startTime() {
-    this.temporaryDestination = temporary;
+    return timer().startTime();
-    this.startTime = startTime;
   }
 
-  public Message getMessage() {
+  public Instant endTime() {
-    return message;
+    return timer().endTime();
-  }
-
-  public MessageOperation getMessageOperation() {
-    return messageOperation;
-  }
-
-  public String getDestinationName() {
-    return destinationName;
-  }
-
-  public String getDestinationKind() {
-    return destinationKind;
-  }
-
-  public boolean isTemporaryDestination() {
-    return temporaryDestination;
-  }
-
-  @Nullable
-  public Instant getStartTime() {
-    return startTime;
   }
 
   public static MessageWithDestination create(
       Message message, MessageOperation operation, Destination fallbackDestination) {
-    return create(message, operation, fallbackDestination, null);
+    return create(message, operation, fallbackDestination, Timer.start());
   }
 
   public static MessageWithDestination create(
-      Message message,
+      Message message, MessageOperation operation, Destination fallbackDestination, Timer timer) {
-      MessageOperation operation,
-      Destination fallbackDestination,
-      @Nullable Instant startTime) {
     Destination jmsDestination = null;
     try {
       jmsDestination = message.getJMSDestination();
@@ -88,17 +60,17 @@ public final class MessageWithDestination {
     }
 
     if (jmsDestination instanceof Queue) {
-      return createMessageWithQueue(message, operation, (Queue) jmsDestination, startTime);
+      return createMessageWithQueue(message, operation, (Queue) jmsDestination, timer);
     }
     if (jmsDestination instanceof Topic) {
-      return createMessageWithTopic(message, operation, (Topic) jmsDestination, startTime);
+      return createMessageWithTopic(message, operation, (Topic) jmsDestination, timer);
     }
-    return new MessageWithDestination(
+    return new AutoValue_MessageWithDestination(
-        message, operation, "unknown", "unknown", /* temporary= */ false, startTime);
+        message, operation, "unknown", "unknown", /* isTemporaryDestination= */ false, timer);
   }
 
   private static MessageWithDestination createMessageWithQueue(
-      Message message, MessageOperation operation, Queue destination, @Nullable Instant startTime) {
+      Message message, MessageOperation operation, Queue destination, Timer timer) {
     String queueName;
     try {
       queueName = destination.getQueueName();
@@ -109,11 +81,12 @@ public final class MessageWithDestination {
     boolean temporary =
         destination instanceof TemporaryQueue || queueName.startsWith(TIBCO_TMP_PREFIX);
 
-    return new MessageWithDestination(message, operation, queueName, "queue", temporary, startTime);
+    return new AutoValue_MessageWithDestination(
+        message, operation, queueName, "queue", temporary, timer);
   }
 
   private static MessageWithDestination createMessageWithTopic(
-      Message message, MessageOperation operation, Topic destination, @Nullable Instant startTime) {
+      Message message, MessageOperation operation, Topic destination, Timer timer) {
     String topicName;
     try {
       topicName = destination.getTopicName();
@@ -124,6 +97,7 @@ public final class MessageWithDestination {
     boolean temporary =
         destination instanceof TemporaryTopic || topicName.startsWith(TIBCO_TMP_PREFIX);
 
-    return new MessageWithDestination(message, operation, topicName, "topic", temporary, startTime);
+    return new AutoValue_MessageWithDestination(
+        message, operation, topicName, "topic", temporary, timer);
   }
 }

instrumentation/jms-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jms/Timer.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.jms;
+
+import java.time.Instant;
+
+public final class Timer {
+
+  public static Timer start() {
+    return new Timer(Instant.now(), System.nanoTime());
+  }
+
+  private final Instant startTime;
+  private final long startNanoTime;
+
+  private Timer(Instant startTime, long startNanoTime) {
+    this.startTime = startTime;
+    this.startNanoTime = startNanoTime;
+  }
+
+  public Instant startTime() {
+    return startTime;
+  }
+
+  public Instant endTime() {
+    long durationNanos = System.nanoTime() - startNanoTime;
+    return startTime().plusNanos(durationNanos);
+  }
+}