Instrumentation API changes: TimeExtractor with Context idea

docs/contributing/using-instrumenter-api.md

@@ -325,31 +325,31 @@ In some cases, the instrumented library provides a way to retrieve accurate time
 operation starts and ends. The `TimeExtractor` interface can be used to
 feed this information into OpenTelemetry trace and metrics data.
 
-`extractStartTime()` can only extract the timestamp from the request. `extractEndTime()`
-accepts the request, an optional response, and an optional `Throwable` error. Consider the following
-example:
+`extractStartTime()` can extract the timestamp from the parent `Context` and the
+request. `extractEndTime()` accepts the `Context` that was returned from `Instrumenter#start()` and
+the request. Consider the following example:
 
 ```java
 class MyTimeExtractor implements TimeExtractor<Request, Response> {
 
   @Override
-  public Instant extractStartTime(Request request) {
+  public Instant extractStartTime(Context parentContext, Request request) {
     return request.startTimestamp();
   }
 
   @Override
-  public Instant extractEndTime(Request request, @Nullable Response response, @Nullable Throwable error) {
-    if (response != null) {
-      return response.endTimestamp();
-    }
-    return request.clock().now();
+  public Instant extractEndTime(Context context, Request request) {
+    Instant endTime = context.get(END_TIME);
+    return endTime == null ?
+        request.clock().now() :
+        endTime;
   }
 }
 ```
 
-The sample implementations above use the request to retrieve the start timestamp. The response is
-used to compute the end time if it is available; in case it is missing (for example, when an error
-occurs) the same time source is used to compute the current timestamp.
+The sample implementations above use the request to retrieve the start timestamp. The end time is
+retrieved from the context; in case it is missing (for example, when the context was not updated
+because of an error) the same time source is used to compute the current timestamp.
 
 You can set the time extractor in the `InstrumenterBuilder` using the `setTimeExtractor()`
 method.

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java

@@ -138,7 +138,7 @@ public Context start(Context parentContext, REQUEST request) {
 
     Instant startTime = null;
     if (timeExtractor != null) {
-      startTime = timeExtractor.extractStartTime(request);
+      startTime = timeExtractor.extractStartTime(parentContext, request);
       spanBuilder.setStartTimestamp(startTime);
     }
 
@@ -203,7 +203,7 @@ public void end(
 
     Instant endTime = null;
     if (timeExtractor != null) {
-      endTime = timeExtractor.extractEndTime(request, response, error);
+      endTime = timeExtractor.extractEndTime(context, request);
     }
 
     if (!operationListeners.isEmpty()) {

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/TimeExtractor.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter;
 
+import io.opentelemetry.context.Context;
 import java.time.Instant;
 import javax.annotation.Nullable;
 
@@ -18,9 +19,36 @@
  */
 public interface TimeExtractor<REQUEST, RESPONSE> {
 
+  /**
+   * Returns the timestamp marking the start of the request processing.
+   *
+   * @deprecated Use {@link #extractStartTime(Context, Object)} instead.
+   */
+  @Deprecated
+  default Instant extractStartTime(REQUEST request) {
+    throw new UnsupportedOperationException(
+        "This method variant is deprecated and will be removed in the next minor release.");
+  }
+
   /** Returns the timestamp marking the start of the request processing. */
-  Instant extractStartTime(REQUEST request);
+  default Instant extractStartTime(Context parentContext, REQUEST request) {
+    return extractStartTime(request);
+  }
+
+  /**
+   * Returns the timestamp marking the end of the request processing.
+   *
+   * @deprecated Use {@link #extractEndTime(Context, Object)} instead.
+   */
+  @Deprecated
+  default Instant extractEndTime(
+      REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error) {
+    throw new UnsupportedOperationException(
+        "This method variant is deprecated and will be removed in the next minor release.");
+  }
 
   /** Returns the timestamp marking the end of the request processing. */
-  Instant extractEndTime(REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error);
+  default Instant extractEndTime(Context context, REQUEST request) {
+    return extractEndTime(request, null, null);
+  }
 }

instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java

@@ -128,15 +128,16 @@ public void extract(
 
   static class TestTimeExtractor implements TimeExtractor<Instant, Instant> {
 
+    static final ContextKey<Instant> END_TIME = ContextKey.named("endTime");
+
     @Override
-    public Instant extractStartTime(Instant request) {
+    public Instant extractStartTime(Context parentContext, Instant request) {
       return request;
     }
 
     @Override
-    public Instant extractEndTime(
-        Instant request, @Nullable Instant response, @Nullable Throwable error) {
-      return response;
+    public Instant extractEndTime(Context context, Instant request) {
+      return context.get(END_TIME);
     }
   }
 
@@ -453,7 +454,7 @@ void shouldStartSpanWithGivenStartTime() {
 
     // when
     Context context = instrumenter.start(Context.root(), startTime);
-    instrumenter.end(context, startTime, endTime, null);
+    instrumenter.end(context.with(TestTimeExtractor.END_TIME, endTime), startTime, endTime, null);
 
     // then
     otelTesting

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

@@ -5,20 +5,19 @@
 
 package io.opentelemetry.javaagent.instrumentation.jms;
 
+import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.TimeExtractor;
 import java.time.Instant;
-import javax.annotation.Nullable;
 
 class JmsMessageTimeExtractor implements TimeExtractor<MessageWithDestination, Void> {
 
   @Override
-  public Instant extractStartTime(MessageWithDestination request) {
+  public Instant extractStartTime(Context parentContext, MessageWithDestination request) {
     return request.startTime();
   }
 
   @Override
-  public Instant extractEndTime(
-      MessageWithDestination request, @Nullable Void unused, @Nullable Throwable error) {
+  public Instant extractEndTime(Context context, MessageWithDestination request) {
     return request.endTime();
   }
 }

instrumentation/kafka/kafka-clients/kafka-clients-common/library/src/main/java/io/opentelemetry/instrumentation/kafka/internal/KafkaConsumerTimeExtractor.java

@@ -5,20 +5,19 @@
 
 package io.opentelemetry.instrumentation.kafka.internal;
 
+import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.TimeExtractor;
 import java.time.Instant;
-import javax.annotation.Nullable;
 
 class KafkaConsumerTimeExtractor implements TimeExtractor<ReceivedRecords, Void> {
 
   @Override
-  public Instant extractStartTime(ReceivedRecords request) {
+  public Instant extractStartTime(Context parentContext, ReceivedRecords request) {
     return request.startTime();
   }
 
   @Override
-  public Instant extractEndTime(
-      ReceivedRecords request, @Nullable Void unused, @Nullable Throwable error) {
+  public Instant extractEndTime(Context context, ReceivedRecords request) {
     return request.now();
   }
 }

instrumentation/netty/netty-3.8/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/client/NettyConnectionTimeExtractor.java

@@ -5,22 +5,21 @@
 
 package io.opentelemetry.javaagent.instrumentation.netty.v3_8.client;
 
+import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.TimeExtractor;
 import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;
 import java.time.Instant;
-import javax.annotation.Nullable;
 import org.jboss.netty.channel.Channel;
 
 class NettyConnectionTimeExtractor implements TimeExtractor<NettyConnectionRequest, Channel> {
 
   @Override
-  public Instant extractStartTime(NettyConnectionRequest request) {
+  public Instant extractStartTime(Context parentContext, NettyConnectionRequest request) {
     return request.timer().startTime();
   }
 
   @Override
-  public Instant extractEndTime(
-      NettyConnectionRequest request, @Nullable Channel channel, @Nullable Throwable error) {
+  public Instant extractEndTime(Context context, NettyConnectionRequest request) {
     return request.timer().now();
   }
 }

instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettyConnectionTimeExtractor.java

@@ -6,21 +6,20 @@
 package io.opentelemetry.javaagent.instrumentation.netty.v4.common.client;
 
 import io.netty.channel.Channel;
+import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.TimeExtractor;
 import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;
 import java.time.Instant;
-import javax.annotation.Nullable;
 
 class NettyConnectionTimeExtractor implements TimeExtractor<NettyConnectionRequest, Channel> {
 
   @Override
-  public Instant extractStartTime(NettyConnectionRequest request) {
+  public Instant extractStartTime(Context parentContext, NettyConnectionRequest request) {
     return request.timer().startTime();
   }
 
   @Override
-  public Instant extractEndTime(
-      NettyConnectionRequest request, @Nullable Channel channel, @Nullable Throwable error) {
+  public Instant extractEndTime(Context context, NettyConnectionRequest request) {
     return request.timer().now();
   }
 }

instrumentation/netty/netty-4-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v4/common/client/NettySslTimeExtractor.java

@@ -5,20 +5,19 @@
 
 package io.opentelemetry.javaagent.instrumentation.netty.v4.common.client;
 
+import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.TimeExtractor;
 import java.time.Instant;
-import javax.annotation.Nullable;
 
 class NettySslTimeExtractor implements TimeExtractor<NettySslRequest, Void> {
 
   @Override
-  public Instant extractStartTime(NettySslRequest request) {
+  public Instant extractStartTime(Context parentContext, NettySslRequest request) {
     return request.timer().startTime();
   }
 
   @Override
-  public Instant extractEndTime(
-      NettySslRequest request, @Nullable Void unused, @Nullable Throwable error) {
+  public Instant extractEndTime(Context context, NettySslRequest request) {
     return request.timer().now();
   }
 }

instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitChannelInstrumentation.java

@@ -209,8 +209,10 @@ public static void extractAndStartSpan(
       }
 
       Context parentContext = Java8BytecodeBridge.currentContext();
-      ReceiveRequest request =
-          ReceiveRequest.create(queue, timer, response, channel.getConnection());
+      // pass the started timer to the instrumenter
+      parentContext = parentContext.with(timer);
+
+      ReceiveRequest request = ReceiveRequest.create(queue, response, channel.getConnection());
       if (!receiveInstrumenter().shouldStart(parentContext, request)) {
         return;
       }

instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/RabbitSingletons.java

@@ -72,7 +72,7 @@ private static Instrumenter<ReceiveRequest, GetResponse> createReceiveInstrument
     return Instrumenter.<ReceiveRequest, GetResponse>builder(
             GlobalOpenTelemetry.get(), instrumentationName, ReceiveRequest::spanName)
         .addAttributesExtractors(extractors)
-        .setTimeExtractor(new RabbitReceiveTimeExtractor())
+        .setTimeExtractor(Timer.timeExtractor())
         .newInstrumenter(SpanKindExtractor.alwaysClient());
   }
 

instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/ReceiveRequest.java

@@ -8,21 +8,17 @@
 import com.google.auto.value.AutoValue;
 import com.rabbitmq.client.Connection;
 import com.rabbitmq.client.GetResponse;
-import java.time.Instant;
 import javax.annotation.Nullable;
 
 @AutoValue
 public abstract class ReceiveRequest {
 
-  public static ReceiveRequest create(
-      String queue, Timer timer, GetResponse response, Connection connection) {
-    return new AutoValue_ReceiveRequest(queue, timer, response, connection);
+  public static ReceiveRequest create(String queue, GetResponse response, Connection connection) {
+    return new AutoValue_ReceiveRequest(queue, response, connection);
   }
 
   public abstract String getQueue();
 
-  public abstract Timer getTimer();
-
   @Nullable
   public abstract GetResponse getResponse();
 
@@ -32,12 +28,4 @@ String spanName() {
     String queue = getQueue();
     return (queue.startsWith("amq.gen-") ? "<generated>" : queue) + " receive";
   }
-
-  Instant startTime() {
-    return getTimer().startTime();
-  }
-
-  Instant now() {
-    return getTimer().now();
-  }
 }