Implement error.type in spring-webflux and reactor-netty instrumentations (#9967)

Author: Mateusz Rzeszutek

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/semconv/http/HttpCommonAttributesGetter.java

@@ -76,7 +76,7 @@ public interface HttpCommonAttributesGetter<REQUEST, RESPONSE> {
    */
   @Nullable
   default String getErrorType(
-      REQUEST request, @Nullable RESPONSE response, @Nullable Throwable throwable) {
+      REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error) {
     return null;
   }
 }

instrumentation/reactor/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyHttpClientAttributesGetter.java

@@ -72,13 +72,15 @@ public String getNetworkProtocolVersion(
   @Nullable
   @Override
   public String getServerAddress(HttpClientRequest request) {
-    return getHost(request);
+    String resourceUrl = request.resourceUrl();
+    return resourceUrl == null ? null : UrlParser.getHost(resourceUrl);
   }
 
   @Nullable
   @Override
   public Integer getServerPort(HttpClientRequest request) {
-    return getPort(request);
+    String resourceUrl = request.resourceUrl();
+    return resourceUrl == null ? null : UrlParser.getPort(resourceUrl);
   }
 
   @Nullable
@@ -99,14 +101,14 @@ public InetSocketAddress getNetworkPeerInetSocketAddress(
   }
 
   @Nullable
-  private static String getHost(HttpClientRequest request) {
-    String resourceUrl = request.resourceUrl();
-    return resourceUrl == null ? null : UrlParser.getHost(resourceUrl);
-  }
-
-  @Nullable
-  private static Integer getPort(HttpClientRequest request) {
-    String resourceUrl = request.resourceUrl();
-    return resourceUrl == null ? null : UrlParser.getPort(resourceUrl);
+  @Override
+  public String getErrorType(
+      HttpClientRequest request, @Nullable HttpClientResponse response, @Nullable Throwable error) {
+    // if both response and error are null it means the request has been cancelled -- see the
+    // ConnectionWrapper class
+    if (response == null && error == null) {
+      return "cancelled";
+    }
+    return null;
   }
 }

instrumentation/reactor/reactor-netty/reactor-netty-1.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/AbstractReactorNettyHttpClientTest.java

@@ -310,7 +310,7 @@ void shouldEndSpanOnMonoTimeout() {
                             equalTo(SemanticAttributes.URL_FULL, uri.toString()),
                             equalTo(SemanticAttributes.SERVER_ADDRESS, "localhost"),
                             equalTo(SemanticAttributes.SERVER_PORT, uri.getPort()),
-                            equalTo(HttpAttributes.ERROR_TYPE, "_OTHER")),
+                            equalTo(HttpAttributes.ERROR_TYPE, "cancelled")),
                 span ->
                     span.hasName("test-http-server")
                         .hasKind(SpanKind.SERVER)

instrumentation/spring/spring-webflux/spring-webflux-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/webflux/v5_0/client/WebClientHelper.java

@@ -14,7 +14,6 @@
 import io.opentelemetry.instrumentation.spring.webflux.v5_3.internal.WebClientHttpAttributesGetter;
 import io.opentelemetry.instrumentation.spring.webflux.v5_3.internal.WebClientTracingFilter;
 import io.opentelemetry.javaagent.bootstrap.internal.CommonConfig;
-import io.opentelemetry.javaagent.bootstrap.internal.InstrumentationConfig;
 import java.util.List;
 import org.springframework.web.reactive.function.client.ClientRequest;
 import org.springframework.web.reactive.function.client.ClientResponse;
@@ -35,9 +34,6 @@ public final class WebClientHelper {
               HttpClientPeerServiceAttributesExtractor.create(
                   WebClientHttpAttributesGetter.INSTANCE,
                   CommonConfig.get().getPeerServiceResolver())),
-          InstrumentationConfig.get()
-              .getBoolean(
-                  "otel.instrumentation.spring-webflux.experimental-span-attributes", false),
           CommonConfig.get().shouldEmitExperimentalHttpClientTelemetry());
 
   public static void addFilter(List<ExchangeFilterFunction> exchangeFilterFunctions) {

instrumentation/spring/spring-webflux/spring-webflux-5.3/library/README.md

@@ -45,6 +45,11 @@ interface.
 a `WebFilter` and using the OpenTelemetry Reactor instrumentation to ensure context is
 passed around correctly.
 
+### Web client instrumentation
+
+The `WebClient` instrumentation will emit the `error.type` attribute with value `cancelled` whenever
+an outgoing HTTP request is cancelled.
+
 ### Setup
 
 Here is how to set up client and server instrumentation respectively:

instrumentation/spring/spring-webflux/spring-webflux-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webflux/v5_3/SpringWebfluxTelemetryBuilder.java

@@ -53,9 +53,8 @@ public final class SpringWebfluxTelemetryBuilder {
       clientExtractorConfigurer = builder -> {};
   private Consumer<HttpSpanNameExtractorBuilder<ClientRequest>> clientSpanNameExtractorConfigurer =
       builder -> {};
-  private boolean captureExperimentalSpanAttributes = false;
-  private boolean emitExperimentalHttpClientMetrics = false;
-  private boolean emitExperimentalHttpServerMetrics = false;
+  private boolean emitExperimentalHttpClientTelemetry = false;
+  private boolean emitExperimentalHttpServerTelemetry = false;
 
   SpringWebfluxTelemetryBuilder(OpenTelemetry openTelemetry) {
     this.openTelemetry = openTelemetry;
@@ -100,18 +99,6 @@ public SpringWebfluxTelemetryBuilder setCapturedClientResponseHeaders(
     return this;
   }
 
-  /**
-   * Sets whether experimental attributes should be set to spans. These attributes may be changed or
-   * removed in the future, so only enable this if you know you do not require attributes filled by
-   * this instrumentation to be stable across versions.
-   */
-  @CanIgnoreReturnValue
-  public SpringWebfluxTelemetryBuilder setCaptureExperimentalSpanAttributes(
-      boolean captureExperimentalSpanAttributes) {
-    this.captureExperimentalSpanAttributes = captureExperimentalSpanAttributes;
-    return this;
-  }
-
   /**
    * Adds an additional {@link AttributesExtractor} to invoke to set attributes to instrumented
    * items.
@@ -178,26 +165,26 @@ public SpringWebfluxTelemetryBuilder setKnownMethods(Set<String> knownMethods) {
   /**
    * Configures the instrumentation to emit experimental HTTP client metrics.
    *
-   * @param emitExperimentalHttpClientMetrics {@code true} if the experimental HTTP client metrics
+   * @param emitExperimentalHttpClientTelemetry {@code true} if the experimental HTTP client metrics
    *     are to be emitted.
    */
   @CanIgnoreReturnValue
-  public SpringWebfluxTelemetryBuilder setEmitExperimentalHttpClientMetrics(
-      boolean emitExperimentalHttpClientMetrics) {
-    this.emitExperimentalHttpClientMetrics = emitExperimentalHttpClientMetrics;
+  public SpringWebfluxTelemetryBuilder setEmitExperimentalHttpClientTelemetry(
+      boolean emitExperimentalHttpClientTelemetry) {
+    this.emitExperimentalHttpClientTelemetry = emitExperimentalHttpClientTelemetry;
     return this;
   }
 
   /**
    * Configures the instrumentation to emit experimental HTTP server metrics.
    *
-   * @param emitExperimentalHttpServerMetrics {@code true} if the experimental HTTP server metrics
+   * @param emitExperimentalHttpServerTelemetry {@code true} if the experimental HTTP server metrics
    *     are to be emitted.
    */
   @CanIgnoreReturnValue
-  public SpringWebfluxTelemetryBuilder setEmitExperimentalHttpServerMetrics(
-      boolean emitExperimentalHttpServerMetrics) {
-    this.emitExperimentalHttpServerMetrics = emitExperimentalHttpServerMetrics;
+  public SpringWebfluxTelemetryBuilder setEmitExperimentalHttpServerTelemetry(
+      boolean emitExperimentalHttpServerTelemetry) {
+    this.emitExperimentalHttpServerTelemetry = emitExperimentalHttpServerTelemetry;
     return this;
   }
 
@@ -213,8 +200,7 @@ public SpringWebfluxTelemetry build() {
             clientExtractorConfigurer,
             clientSpanNameExtractorConfigurer,
             clientAdditionalExtractors,
-            captureExperimentalSpanAttributes,
-            emitExperimentalHttpClientMetrics);
+            emitExperimentalHttpClientTelemetry);
 
     Instrumenter<ServerWebExchange, ServerWebExchange> serverInstrumenter =
         buildServerInstrumenter();
@@ -234,7 +220,7 @@ private Instrumenter<ServerWebExchange, ServerWebExchange> buildServerInstrument
             .addAttributesExtractors(serverAdditionalExtractors)
             .addContextCustomizer(httpServerRouteBuilder.build())
             .addOperationMetrics(HttpServerMetrics.get());
-    if (emitExperimentalHttpServerMetrics) {
+    if (emitExperimentalHttpServerTelemetry) {
       builder
           .addAttributesExtractor(HttpExperimentalAttributesExtractor.create(getter))
           .addOperationMetrics(HttpServerExperimentalMetrics.get());

instrumentation/spring/spring-webflux/spring-webflux-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webflux/v5_3/internal/ClientInstrumenterFactory.java

@@ -40,8 +40,7 @@ public static Instrumenter<ClientRequest, ClientResponse> create(
           extractorConfigurer,
       Consumer<HttpSpanNameExtractorBuilder<ClientRequest>> spanNameExtractorConfigurer,
       List<AttributesExtractor<ClientRequest, ClientResponse>> additionalExtractors,
-      boolean captureExperimentalSpanAttributes,
-      boolean emitExperimentalHttpClientMetrics) {
+      boolean emitExperimentalHttpClientTelemetry) {
 
     WebClientHttpAttributesGetter httpAttributesGetter = WebClientHttpAttributesGetter.INSTANCE;
 
@@ -61,10 +60,7 @@ public static Instrumenter<ClientRequest, ClientResponse> create(
             .addAttributesExtractors(additionalExtractors)
             .addOperationMetrics(HttpClientMetrics.get());
 
-    if (captureExperimentalSpanAttributes) {
-      clientBuilder.addAttributesExtractor(new WebClientExperimentalAttributesExtractor());
-    }
-    if (emitExperimentalHttpClientMetrics) {
+    if (emitExperimentalHttpClientTelemetry) {
       clientBuilder
           .addAttributesExtractor(HttpExperimentalAttributesExtractor.create(httpAttributesGetter))
           .addOperationMetrics(HttpClientExperimentalMetrics.get());

instrumentation/spring/spring-webflux/spring-webflux-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webflux/v5_3/internal/WebClientExperimentalAttributesExtractor.java

@@ -59,4 +59,16 @@ public String getServerAddress(ClientRequest request) {
   public Integer getServerPort(ClientRequest request) {
     return request.url().getPort();
   }
+
+  @Nullable
+  @Override
+  public String getErrorType(
+      ClientRequest request, @Nullable ClientResponse response, @Nullable Throwable error) {
+    // if both response and error are null it means the request has been cancelled -- see the
+    // WebClientTracingFilter class
+    if (response == null && error == null) {
+      return "cancelled";
+    }
+    return null;
+  }
 }

instrumentation/spring/spring-webflux/spring-webflux-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webflux/v5_3/internal/WebClientHttpAttributesGetter.java

@@ -5,20 +5,29 @@
 
 package io.opentelemetry.instrumentation.spring.webflux.client;
 
+import static io.opentelemetry.api.trace.SpanKind.CLIENT;
+import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
+import static java.util.Collections.emptyMap;
 import static java.util.Objects.requireNonNull;
+import static org.assertj.core.api.Assertions.catchThrowable;
 
 import io.opentelemetry.api.common.AttributeKey;
+import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.instrumentation.api.semconv.http.internal.HttpAttributes;
 import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpClientTest;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientResult;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpClientTestOptions;
+import io.opentelemetry.sdk.trace.data.StatusData;
 import io.opentelemetry.semconv.SemanticAttributes;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.net.URI;
+import java.time.Duration;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import org.junit.jupiter.api.Test;
 import org.springframework.http.HttpMethod;
 import org.springframework.web.reactive.function.client.ClientResponse;
 import org.springframework.web.reactive.function.client.WebClient;
@@ -142,4 +151,44 @@ private static int getStatusCode(ClientResponse response) {
       throw new AssertionError(e);
     }
   }
+
+  @Test
+  void shouldEndSpanOnMonoTimeout() {
+    URI uri = resolveAddress("/read-timeout");
+    Throwable thrown =
+        catchThrowable(
+            () ->
+                testing.runWithSpan(
+                    "parent",
+                    () ->
+                        buildRequest("GET", uri, emptyMap())
+                            .exchange()
+                            // apply Mono timeout that is way shorter than HTTP request timeout
+                            .timeout(Duration.ofSeconds(1))
+                            .block()));
+
+    testing.waitAndAssertTraces(
+        trace ->
+            trace.hasSpansSatisfyingExactly(
+                span ->
+                    span.hasName("parent")
+                        .hasKind(SpanKind.INTERNAL)
+                        .hasNoParent()
+                        .hasStatus(StatusData.error())
+                        .hasException(thrown),
+                span ->
+                    span.hasName("GET")
+                        .hasKind(CLIENT)
+                        .hasParent(trace.getSpan(0))
+                        .hasAttributesSatisfyingExactly(
+                            equalTo(SemanticAttributes.HTTP_REQUEST_METHOD, "GET"),
+                            equalTo(SemanticAttributes.URL_FULL, uri.toString()),
+                            equalTo(SemanticAttributes.SERVER_ADDRESS, "localhost"),
+                            equalTo(SemanticAttributes.SERVER_PORT, uri.getPort()),
+                            equalTo(HttpAttributes.ERROR_TYPE, "cancelled")),
+                span ->
+                    span.hasName("test-http-server")
+                        .hasKind(SpanKind.SERVER)
+                        .hasParent(trace.getSpan(1))));
+  }
 }