split HttpStatusConverter into client and server implementations, and…

… create two HttpSpanStatusExtractor.create methods, one for server and one for client.
open-telemetry · Oct 18, 2021 · 917f3d6 · 917f3d6
1 parent 23232bd
commit 917f3d6
Show file tree

Hide file tree

Showing 46 changed files with 419 additions and 291 deletions.
diff --git a/...in/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanStatusExtractor.java b/...in/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanStatusExtractor.java
@@ -5,7 +5,6 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter;
 
-import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.api.trace.StatusCode;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
@@ -16,7 +15,7 @@ final class DefaultSpanStatusExtractor<REQUEST, RESPONSE>
 
   @Override
   public StatusCode extract(
-      REQUEST request, @Nullable RESPONSE response, SpanKind kind, @Nullable Throwable error) {
+      REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error) {
     if (error != null) {
       return StatusCode.ERROR;
     }

diff --git a/...ion-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java b/...ion-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java
@@ -202,7 +202,7 @@ public void end(
     }
 
     SpanKind kind = spanKindExtractor.extract(request);
-    StatusCode statusCode = spanStatusExtractor.extract(request, response, kind, error);
+    StatusCode statusCode = spanStatusExtractor.extract(request, response, error);
     if (statusCode != StatusCode.UNSET) {
       span.setStatus(statusCode);
     }

diff --git a/.../src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusExtractor.java b/.../src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanStatusExtractor.java
@@ -5,7 +5,6 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter;
 
-import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.api.trace.StatusCode;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
@@ -26,6 +25,5 @@ static <REQUEST, RESPONSE> SpanStatusExtractor<REQUEST, RESPONSE> getDefault() {
   }
 
   /** Returns the {@link StatusCode}. */
-  StatusCode extract(
-      REQUEST request, @Nullable RESPONSE response, SpanKind kind, @Nullable Throwable error);
+  StatusCode extract(REQUEST request, @Nullable RESPONSE response, @Nullable Throwable error);
 }
diff --git a/.../java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java b/.../java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpSpanStatusExtractor.java
@@ -5,7 +5,6 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter.http;
 
-import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.api.trace.StatusCode;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanStatusExtractor;
 import io.opentelemetry.instrumentation.api.tracer.HttpStatusConverter;
@@ -20,36 +19,49 @@
 public final class HttpSpanStatusExtractor<REQUEST, RESPONSE>
     implements SpanStatusExtractor<REQUEST, RESPONSE> {
 
+  private final HttpStatusConverter statusConverter;
+
+  /**
+   * Returns the {@link SpanStatusExtractor} for HTTP requests, which will use the HTTP status code
+   * to determine the {@link StatusCode} if available or fallback to {@linkplain #getDefault() the
+   * default status} otherwise.
+   */
+  public static <REQUEST, RESPONSE> SpanStatusExtractor<REQUEST, RESPONSE> createClient(
+      HttpCommonAttributesExtractor<? super REQUEST, ? super RESPONSE> attributesExtractor) {
+    return new HttpSpanStatusExtractor<>(attributesExtractor, HttpStatusConverter.CLIENT);
+  }
+
   /**
    * Returns the {@link SpanStatusExtractor} for HTTP requests, which will use the HTTP status code
    * to determine the {@link StatusCode} if available or fallback to {@linkplain #getDefault() the
    * default status} otherwise.
    */
-  public static <REQUEST, RESPONSE> SpanStatusExtractor<REQUEST, RESPONSE> create(
+  public static <REQUEST, RESPONSE> SpanStatusExtractor<REQUEST, RESPONSE> createServer(
       HttpCommonAttributesExtractor<? super REQUEST, ? super RESPONSE> attributesExtractor) {
-    return new HttpSpanStatusExtractor<>(attributesExtractor);
+    return new HttpSpanStatusExtractor<>(attributesExtractor, HttpStatusConverter.SERVER);
   }
 
   private final HttpCommonAttributesExtractor<? super REQUEST, ? super RESPONSE>
       attributesExtractor;
 
   private HttpSpanStatusExtractor(
-      HttpCommonAttributesExtractor<? super REQUEST, ? super RESPONSE> attributesExtractor) {
+      HttpCommonAttributesExtractor<? super REQUEST, ? super RESPONSE> attributesExtractor,
+      HttpStatusConverter statusConverter) {
     this.attributesExtractor = attributesExtractor;
+    this.statusConverter = statusConverter;
   }
 
   @Override
-  public StatusCode extract(
-      REQUEST request, @Nullable RESPONSE response, SpanKind kind, @Nullable Throwable error) {
+  public StatusCode extract(REQUEST request, @Nullable RESPONSE response, Throwable error) {
     if (response != null) {
       Integer statusCode = attributesExtractor.statusCode(request, response);
       if (statusCode != null) {
-        StatusCode statusCodeObj = HttpStatusConverter.statusFromHttpStatus(statusCode, kind);
+        StatusCode statusCodeObj = statusConverter.statusFromHttpStatus(statusCode);
         if (statusCodeObj == StatusCode.ERROR) {
           return statusCodeObj;
         }
       }
     }
-    return SpanStatusExtractor.getDefault().extract(request, response, kind, error);
+    return SpanStatusExtractor.getDefault().extract(request, response, error);
   }
 }
diff --git a/.../src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientStatusConverter.java b/.../src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientStatusConverter.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.tracer;
+
+import io.opentelemetry.api.trace.StatusCode;
+
+final class HttpClientStatusConverter implements HttpStatusConverter {
+
+  static final HttpStatusConverter INSTANCE = new HttpClientStatusConverter();
+
+  // https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#status
+  @Override
+  public StatusCode statusFromHttpStatus(int httpStatus) {
+    if (httpStatus >= 100 && httpStatus < 400) {
+      return StatusCode.UNSET;
+    }
+
+    return StatusCode.ERROR;
+  }
+
+  private HttpClientStatusConverter() {}
+}
diff --git a/...ation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java b/...ation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java
@@ -211,7 +211,7 @@ protected void onResponse(Span span, RESPONSE response) {
       Integer status = status(response);
       if (status != null) {
         span.setAttribute(SemanticAttributes.HTTP_STATUS_CODE, (long) status);
-        StatusCode statusCode = HttpStatusConverter.clientStatusFromHttpStatus(status);
+        StatusCode statusCode = HttpStatusConverter.CLIENT.statusFromHttpStatus(status);
         if (statusCode != StatusCode.UNSET) {
           span.setStatus(statusCode);
         }

diff --git a/.../src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerStatusConverter.java b/.../src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerStatusConverter.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.tracer;
+
+import io.opentelemetry.api.trace.StatusCode;
+
+final class HttpServerStatusConverter implements HttpStatusConverter {
+
+  static final HttpStatusConverter INSTANCE = new HttpServerStatusConverter();
+
+  // https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#status
+  @Override
+  public StatusCode statusFromHttpStatus(int httpStatus) {
+    if (httpStatus >= 100 && httpStatus < 500) {
+      return StatusCode.UNSET;
+    }
+
+    return StatusCode.ERROR;
+  }
+
+  private HttpServerStatusConverter() {}
+}
diff --git a/...ation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java b/...ation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpServerTracer.java
@@ -272,7 +272,7 @@ private static String extractIpAddress(String forwarded, int start) {
 
   private static void setStatus(Span span, int status) {
     span.setAttribute(SemanticAttributes.HTTP_STATUS_CODE, (long) status);
-    StatusCode statusCode = HttpStatusConverter.serverStatusFromHttpStatus(status);
+    StatusCode statusCode = HttpStatusConverter.SERVER.statusFromHttpStatus(status);
     if (statusCode != StatusCode.UNSET) {
       span.setStatus(statusCode);
     }

diff --git a/...on-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpStatusConverter.java b/...on-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpStatusConverter.java
@@ -5,33 +5,12 @@
 
 package io.opentelemetry.instrumentation.api.tracer;
 
-import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.api.trace.StatusCode;
 
-// https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md#status
-public final class HttpStatusConverter {
+public interface HttpStatusConverter {
 
-  public static StatusCode statusFromHttpStatus(int httpStatus, SpanKind kind) {
-    return (kind == SpanKind.SERVER)
-        ? serverStatusFromHttpStatus(httpStatus)
-        : clientStatusFromHttpStatus(httpStatus);
-  }
+  HttpStatusConverter SERVER = HttpServerStatusConverter.INSTANCE;
+  HttpStatusConverter CLIENT = HttpClientStatusConverter.INSTANCE;
 
-  private static StatusCode statusFromHttpStatus(int httpStatus, int minError) {
-    if (httpStatus >= 100 && httpStatus < minError) {
-      return StatusCode.UNSET;
-    }
-
-    return StatusCode.ERROR;
-  }
-
-  public static StatusCode clientStatusFromHttpStatus(int httpStatus) {
-    return statusFromHttpStatus(httpStatus, 400);
-  }
-
-  public static StatusCode serverStatusFromHttpStatus(int httpStatus) {
-    return statusFromHttpStatus(httpStatus, 500);
-  }
-
-  private HttpStatusConverter() {}
+  StatusCode statusFromHttpStatus(int httpStatus);
 }
diff --git a/...t/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientStatusConverterTest.groovy b/...t/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientStatusConverterTest.groovy
@@ -0,0 +1,94 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.tracer
+
+import io.opentelemetry.api.trace.StatusCode
+import spock.lang.Specification
+
+class HttpClientStatusConverterTest extends Specification {
+
+  def "test HTTP #httpStatus to OTel #expectedStatus"() {
+    when:
+    def status = HttpStatusConverter.CLIENT.statusFromHttpStatus(httpStatus)
+
+    then:
+    status == expectedStatus
+
+    // https://en.wikipedia.org/wiki/List_of_HTTP_status_codes
+    where:
+    httpStatus | expectedStatus
+    100        | StatusCode.UNSET
+    101        | StatusCode.UNSET
+    102        | StatusCode.UNSET
+    103        | StatusCode.UNSET
+
+    200        | StatusCode.UNSET
+    201        | StatusCode.UNSET
+    202        | StatusCode.UNSET
+    203        | StatusCode.UNSET
+    204        | StatusCode.UNSET
+    205        | StatusCode.UNSET
+    206        | StatusCode.UNSET
+    207        | StatusCode.UNSET
+    208        | StatusCode.UNSET
+    226        | StatusCode.UNSET
+
+    300        | StatusCode.UNSET
+    301        | StatusCode.UNSET
+    302        | StatusCode.UNSET
+    303        | StatusCode.UNSET
+    304        | StatusCode.UNSET
+    305        | StatusCode.UNSET
+    306        | StatusCode.UNSET
+    307        | StatusCode.UNSET
+    308        | StatusCode.UNSET
+
+    400        | StatusCode.ERROR
+    401        | StatusCode.ERROR
+    403        | StatusCode.ERROR
+    404        | StatusCode.ERROR
+    405        | StatusCode.ERROR
+    406        | StatusCode.ERROR
+    407        | StatusCode.ERROR
+    408        | StatusCode.ERROR
+    409        | StatusCode.ERROR
+    410        | StatusCode.ERROR
+    411        | StatusCode.ERROR
+    412        | StatusCode.ERROR
+    413        | StatusCode.ERROR
+    414        | StatusCode.ERROR
+    415        | StatusCode.ERROR
+    416        | StatusCode.ERROR
+    417        | StatusCode.ERROR
+    418        | StatusCode.ERROR
+    421        | StatusCode.ERROR
+    422        | StatusCode.ERROR
+    423        | StatusCode.ERROR
+    424        | StatusCode.ERROR
+    425        | StatusCode.ERROR
+    426        | StatusCode.ERROR
+    428        | StatusCode.ERROR
+    429        | StatusCode.ERROR
+    431        | StatusCode.ERROR
+    451        | StatusCode.ERROR
+
+    500        | StatusCode.ERROR
+    501        | StatusCode.ERROR
+    502        | StatusCode.ERROR
+    503        | StatusCode.ERROR
+    504        | StatusCode.ERROR
+    505        | StatusCode.ERROR
+    506        | StatusCode.ERROR
+    507        | StatusCode.ERROR
+    508        | StatusCode.ERROR
+    510        | StatusCode.ERROR
+    511        | StatusCode.ERROR
+
+    // Don't exist
+    99         | StatusCode.ERROR
+    600        | StatusCode.ERROR
+  }
+}
diff --git a/...i/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientTracerTest.groovy b/...i/src/test/groovy/io/opentelemetry/instrumentation/api/tracer/HttpClientTracerTest.groovy
@@ -5,14 +5,12 @@
 
 package io.opentelemetry.instrumentation.api.tracer
 
-import io.opentelemetry.api.trace.SpanKind
 import io.opentelemetry.api.trace.StatusCode
 import io.opentelemetry.context.propagation.TextMapSetter
 import io.opentelemetry.instrumentation.api.tracer.net.NetPeerAttributes
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
 import spock.lang.Shared
 
-import static io.opentelemetry.instrumentation.api.tracer.HttpStatusConverter.statusFromHttpStatus
 import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_TCP
 
 class HttpClientTracerTest extends BaseTracerTest {
@@ -113,7 +111,7 @@ class HttpClientTracerTest extends BaseTracerTest {
   def "test onResponse"() {
     setup:
     def tracer = newTracer()
-    def statusCode = status != null ? statusFromHttpStatus(status, SpanKind.CLIENT) : null
+    def statusCode = status != null ? HttpStatusConverter.CLIENT.statusFromHttpStatus(status) : null
 
     when:
     tracer.onResponse(span, resp)