Add error parameter to EndTimeExtractor and AttributesExtractor#onEnd()

instrumentation-api-annotation-support/src/main/java/io/opentelemetry/instrumentation/api/annotation/support/MethodSpanAttributesExtractor.java

@@ -57,7 +57,10 @@ protected void onStart(AttributesBuilder attributes, REQUEST request) {
 
   @Override
   protected void onEnd(
-      AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {}
+      AttributesBuilder attributes,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error) {}
 
   /**
    * Creates a binding of the parameters of the traced method to span attributes.

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

@@ -15,10 +15,10 @@
 /**
  * Extractor of {@link io.opentelemetry.api.common.Attributes} for a given request and response.
  * Will be called {@linkplain #onStart(AttributesBuilder, Object) on start} with just the {@link
- * REQUEST} and again {@linkplain #onEnd(AttributesBuilder, Object, Object) on end} with both {@link
- * REQUEST} and {@link RESPONSE} to allow populating attributes at each stage of a request's
- * lifecycle. It is best to populate as much as possible in {@link #onStart(AttributesBuilder,
- * Object)} to have it available during sampling.
+ * REQUEST} and again {@linkplain #onEnd(AttributesBuilder, Object, Object, Throwable) on end} with
+ * both {@link REQUEST} and {@link RESPONSE} to allow populating attributes at each stage of a
+ * request's lifecycle. It is best to populate as much as possible in {@link
+ * #onStart(AttributesBuilder, Object)} to have it available during sampling.
  *
  * @see DbAttributesExtractor
  * @see HttpAttributesExtractor
@@ -32,11 +32,14 @@ public abstract class AttributesExtractor<REQUEST, RESPONSE> {
   protected abstract void onStart(AttributesBuilder attributes, REQUEST request);
 
   /**
-   * Extracts attributes from the {@link REQUEST} and {@link RESPONSE} into the {@link
-   * AttributesBuilder} at the end of a request.
+   * Extracts attributes from the {@link REQUEST} and either {@link RESPONSE} or {@code error} into
+   * the {@link AttributesBuilder} at the end of a request.
    */
   protected abstract void onEnd(
-      AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response);
+      AttributesBuilder attributes,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error);
 
   /**
    * Sets the {@code value} with the given {@code key} to the {@link AttributesBuilder} if {@code

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

@@ -17,5 +17,5 @@
 public interface EndTimeExtractor<REQUEST, RESPONSE> {
 
   /** Returns the timestamp marking the end of the response processing. */
-  Instant extract(REQUEST request, @Nullable RESPONSE response);
+  Instant extract(REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error);
 }

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

@@ -165,30 +165,29 @@ public void end(
       Context context, REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error) {
     Span span = Span.fromContext(context);
 
+    if (error != null) {
+      error = errorCauseExtractor.extractCause(error);
+      span.recordException(error);
+    }
+
     UnsafeAttributes attributesBuilder = new UnsafeAttributes();
     for (AttributesExtractor<? super REQUEST, ? super RESPONSE> extractor : attributesExtractors) {
-      extractor.onEnd(attributesBuilder, request, response);
+      extractor.onEnd(attributesBuilder, request, response, error);
     }
     Attributes attributes = attributesBuilder;
+    span.setAllAttributes(attributes);
 
     for (RequestListener requestListener : requestListeners) {
       requestListener.end(context, attributes);
     }
 
-    span.setAllAttributes(attributes);
-
-    if (error != null) {
-      error = errorCauseExtractor.extractCause(error);
-      span.recordException(error);
-    }
-
     StatusCode statusCode = spanStatusExtractor.extract(request, response, error);
     if (statusCode != StatusCode.UNSET) {
       span.setStatus(statusCode);
     }
 
     if (endTimeExtractor != null) {
-      span.end(endTimeExtractor.extract(request, response));
+      span.end(endTimeExtractor.extract(request, response, error));
     } else {
       span.end();
     }

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

@@ -21,8 +21,8 @@ public interface RequestListener {
    * the start and end of the request, e.g., an in-progress span, it should be added to the passed
    * in {@link Context} and returned.
    */
-  Context start(Context context, Attributes requestAttributes);
+  Context start(Context context, Attributes startAttributes);
 
   /** Listener method that is called at the end of a request. */
-  void end(Context context, Attributes responseAttributes);
+  void end(Context context, Attributes endAttributes);
 }

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

@@ -73,7 +73,10 @@ protected void onStart(AttributesBuilder attributes, REQUEST request) {}
 
     @Override
     protected void onEnd(
-        AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {
+        AttributesBuilder attributes,
+        REQUEST request,
+        @Nullable RESPONSE response,
+        @Nullable Throwable error) {
       String clientIp = getForwardedClientIp(request);
       if (clientIp == null && netAttributesExtractor != null) {
         clientIp = netAttributesExtractor.peerIp(request, response);

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/code/CodeAttributesExtractor.java

@@ -31,7 +31,10 @@ protected final void onStart(AttributesBuilder attributes, REQUEST request) {
 
   @Override
   protected final void onEnd(
-      AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {}
+      AttributesBuilder attributes,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error) {}
 
   @Nullable
   protected abstract Class<?> codeClass(REQUEST request);

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/db/DbAttributesExtractor.java

@@ -33,7 +33,10 @@ protected void onStart(AttributesBuilder attributes, REQUEST request) {
 
   @Override
   protected final void onEnd(
-      AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {}
+      AttributesBuilder attributes,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error) {}
 
   @Nullable
   protected abstract String system(REQUEST request);

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpAttributesExtractor.java

@@ -37,7 +37,10 @@ protected final void onStart(AttributesBuilder attributes, REQUEST request) {
 
   @Override
   protected final void onEnd(
-      AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {
+      AttributesBuilder attributes,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error) {
     set(
         attributes,
         SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH,

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientMetrics.java

@@ -60,16 +60,16 @@ private HttpClientMetrics(Meter meter) {
   }
 
   @Override
-  public Context start(Context context, Attributes requestAttributes) {
+  public Context start(Context context, Attributes startAttributes) {
     long startTimeNanos = System.nanoTime();
 
     return context.with(
         HTTP_CLIENT_REQUEST_METRICS_STATE,
-        new AutoValue_HttpClientMetrics_State(requestAttributes, startTimeNanos));
+        new AutoValue_HttpClientMetrics_State(startAttributes, startTimeNanos));
   }
 
   @Override
-  public void end(Context context, Attributes responseAttributes) {
+  public void end(Context context, Attributes endAttributes) {
     State state = context.get(HTTP_CLIENT_REQUEST_METRICS_STATE);
     if (state == null) {
       logger.debug(

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerMetrics.java

@@ -70,17 +70,17 @@ private HttpServerMetrics(Meter meter) {
   }
 
   @Override
-  public Context start(Context context, Attributes requestAttributes) {
+  public Context start(Context context, Attributes startAttributes) {
     long startTimeNanos = System.nanoTime();
-    activeRequests.add(1, applyActiveRequestsView(requestAttributes));
+    activeRequests.add(1, applyActiveRequestsView(startAttributes));
 
     return context.with(
         HTTP_SERVER_REQUEST_METRICS_STATE,
-        new AutoValue_HttpServerMetrics_State(requestAttributes, startTimeNanos));
+        new AutoValue_HttpServerMetrics_State(startAttributes, startTimeNanos));
   }
 
   @Override
-  public void end(Context context, Attributes responseAttributes) {
+  public void end(Context context, Attributes endAttributes) {
     State state = context.get(HTTP_SERVER_REQUEST_METRICS_STATE);
     if (state == null) {
       logger.debug(

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/messaging/MessagingAttributesExtractor.java

@@ -54,7 +54,10 @@ protected final void onStart(AttributesBuilder attributes, REQUEST request) {
 
   @Override
   protected final void onEnd(
-      AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {
+      AttributesBuilder attributes,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error) {
     set(attributes, SemanticAttributes.MESSAGING_MESSAGE_ID, messageId(request, response));
   }
 

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetAttributesExtractor.java

@@ -32,7 +32,10 @@ protected final void onStart(AttributesBuilder attributes, REQUEST request) {
 
   @Override
   protected final void onEnd(
-      AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {
+      AttributesBuilder attributes,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error) {
     set(attributes, SemanticAttributes.NET_PEER_IP, peerIp(request, response));
     set(attributes, SemanticAttributes.NET_PEER_NAME, peerName(request, response));
     Integer peerPort = peerPort(request, response);

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/rpc/RpcAttributesExtractor.java

@@ -31,7 +31,10 @@ protected final void onStart(AttributesBuilder attributes, REQUEST request) {
 
   @Override
   protected final void onEnd(
-      AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {
+      AttributesBuilder attributes,
+      REQUEST request,
+      @Nullable RESPONSE response,
+      @Nullable Throwable error) {
     // No response attributes
   }
 

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

@@ -13,6 +13,7 @@
 import io.opentelemetry.api.common.AttributesBuilder;
 import java.util.HashMap;
 import java.util.Map;
+import javax.annotation.Nullable;
 import org.junit.jupiter.api.Test;
 
 class AttributesExtractorTest {
@@ -28,23 +29,39 @@ protected void onStart(AttributesBuilder attributes, Map<String, String> request
 
     @Override
     protected void onEnd(
-        AttributesBuilder attributes, Map<String, String> request, Map<String, String> response) {
-      set(attributes, AttributeKey.stringKey("food"), response.get("food"));
-      set(attributes, AttributeKey.stringKey("number"), request.get("number"));
+        AttributesBuilder attributes,
+        Map<String, String> request,
+        @Nullable Map<String, String> response,
+        @Nullable Throwable error) {
+      if (response != null) {
+        set(attributes, AttributeKey.stringKey("food"), response.get("food"));
+        set(attributes, AttributeKey.stringKey("number"), request.get("number"));
+      }
+      if (error != null) {
+        set(attributes, AttributeKey.stringKey("full_error_class"), error.getClass().getName());
+      }
     }
   }
 
   @Test
   void normal() {
     TestAttributesExtractor extractor = new TestAttributesExtractor();
+
     Map<String, String> request = new HashMap<>();
     request.put("animal", "cat");
     Map<String, String> response = new HashMap<>();
     response.put("food", "pizza");
+    Exception error = new RuntimeException();
+
     AttributesBuilder attributesBuilder = Attributes.builder();
     extractor.onStart(attributesBuilder, request);
-    extractor.onEnd(attributesBuilder, request, response);
+    extractor.onEnd(attributesBuilder, request, response, null);
+    extractor.onEnd(attributesBuilder, request, null, error);
+
     assertThat(attributesBuilder.build())
-        .containsOnly(attributeEntry("animal", "cat"), attributeEntry("food", "pizza"));
+        .containsOnly(
+            attributeEntry("animal", "cat"),
+            attributeEntry("food", "pizza"),
+            attributeEntry("full_error_class", "java.lang.RuntimeException"));
   }
 }

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

@@ -37,6 +37,7 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import javax.annotation.Nullable;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.api.extension.RegisterExtension;
@@ -80,7 +81,10 @@ protected void onStart(AttributesBuilder attributes, Map<String, String> request
 
     @Override
     protected void onEnd(
-        AttributesBuilder attributes, Map<String, String> request, Map<String, String> response) {
+        AttributesBuilder attributes,
+        Map<String, String> request,
+        Map<String, String> response,
+        @Nullable Throwable error) {
       attributes.put("resp1", response.get("resp1"));
       attributes.put("resp2", response.get("resp2"));
     }
@@ -97,7 +101,10 @@ protected void onStart(AttributesBuilder attributes, Map<String, String> request
 
     @Override
     protected void onEnd(
-        AttributesBuilder attributes, Map<String, String> request, Map<String, String> response) {
+        AttributesBuilder attributes,
+        Map<String, String> request,
+        Map<String, String> response,
+        @Nullable Throwable error) {
       attributes.put("resp3", response.get("resp3"));
       attributes.put("resp2", response.get("resp2_2"));
     }
@@ -490,7 +497,7 @@ void shouldStartSpanWithGivenStartTime() {
     Instrumenter<Instant, Instant> instrumenter =
         Instrumenter.<Instant, Instant>newBuilder(
                 otelTesting.getOpenTelemetry(), "test", request -> "test span")
-            .setTimeExtractors(request -> request, (request, response) -> response)
+            .setTimeExtractors(request -> request, (request, response, error) -> response)
             .newInstrumenter();
 
     Instant startTime = Instant.ofEpochSecond(100);

instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/code/CodeAttributesExtractorTest.java

@@ -61,7 +61,7 @@ void shouldExtractAllAttributes() {
     underTest.onStart(startAttributes, request);
 
     AttributesBuilder endAttributes = Attributes.builder();
-    underTest.onEnd(endAttributes, request, null);
+    underTest.onEnd(endAttributes, request, null, null);
 
     // then
     assertThat(startAttributes.build())

instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/db/DbAttributesExtractorTest.java

@@ -66,7 +66,7 @@ void shouldExtractAllAvailableAttributes() {
     underTest.onStart(startAttributes, request);
 
     AttributesBuilder endAttributes = Attributes.builder();
-    underTest.onEnd(endAttributes, request, null);
+    underTest.onEnd(endAttributes, request, null, null);
 
     // then
     assertThat(startAttributes.build())

instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/db/SqlAttributesExtractorTest.java

@@ -70,7 +70,7 @@ void shouldExtractAllAttributes() {
     underTest.onStart(startAttributes, request);
 
     AttributesBuilder endAttributes = Attributes.builder();
-    underTest.onEnd(endAttributes, request, null);
+    underTest.onEnd(endAttributes, request, null, null);
 
     // then
     assertThat(startAttributes.build())

instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpAttributesExtractorTest.java

@@ -127,7 +127,7 @@ void normal() {
             entry(SemanticAttributes.HTTP_SCHEME, "https"),
             entry(SemanticAttributes.HTTP_USER_AGENT, "okhttp 3.x"));
 
-    extractor.onEnd(attributes, request, response);
+    extractor.onEnd(attributes, request, response, null);
     assertThat(attributes.build())
         .containsOnly(
             entry(SemanticAttributes.HTTP_METHOD, "POST"),

instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/messaging/MessagingAttributesExtractorTest.java

@@ -120,7 +120,7 @@ void shouldExtractAllAvailableAttributes(
     underTest.onStart(startAttributes, request);
 
     AttributesBuilder endAttributes = Attributes.builder();
-    underTest.onEnd(endAttributes, request, "42");
+    underTest.onEnd(endAttributes, request, "42", null);
 
     // then
     List<MapEntry<AttributeKey<?>, Object>> expectedEntries = new ArrayList<>();
@@ -168,7 +168,7 @@ void shouldExtractNoAttributesIfNoneAreAvailable() {
     underTest.onStart(startAttributes, Collections.emptyMap());
 
     AttributesBuilder endAttributes = Attributes.builder();
-    underTest.onEnd(endAttributes, Collections.emptyMap(), null);
+    underTest.onEnd(endAttributes, Collections.emptyMap(), null, null);
 
     // then
     assertThat(startAttributes.build().isEmpty()).isTrue();