Refactor HTTP attributes extractors to use composition over inheritance (#5267)

* Refactor HTTP attributes extractors to use composition over inheritance

* Rename remaining variables: *Extractor to *Getter

Author: Mateusz Rzeszutek

instrumentation-api/src/jmh/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBenchmark.java

@@ -9,6 +9,7 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeaders;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientAttributesGetter;
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetServerAttributesGetter;
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
@@ -40,8 +41,10 @@ public class InstrumenterBenchmark {
       Instrumenter.<Void, Void>builder(
               OpenTelemetry.noop(),
               "benchmark",
-              HttpSpanNameExtractor.create(ConstantHttpAttributesExtractor.INSTANCE))
-          .addAttributesExtractor(ConstantHttpAttributesExtractor.INSTANCE)
+              HttpSpanNameExtractor.create(ConstantHttpAttributesGetter.INSTANCE))
+          .addAttributesExtractor(
+              HttpClientAttributesExtractor.create(
+                  ConstantHttpAttributesGetter.INSTANCE, CapturedHttpHeaders.empty()))
           .addAttributesExtractor(
               NetServerAttributesExtractor.create(new ConstantNetAttributesGetter()))
           .newInstrumenter();
@@ -58,72 +61,61 @@ public Context startEnd() {
     return context;
   }
 
-  static class ConstantHttpAttributesExtractor extends HttpClientAttributesExtractor<Void, Void> {
-    static final HttpClientAttributesExtractor<Void, Void> INSTANCE =
-        new ConstantHttpAttributesExtractor();
-
-    public ConstantHttpAttributesExtractor() {
-      super(CapturedHttpHeaders.empty());
-    }
+  enum ConstantHttpAttributesGetter implements HttpClientAttributesGetter<Void, Void> {
+    INSTANCE;
 
     @Override
-    @Nullable
-    protected String method(Void unused) {
+    public String method(Void unused) {
       return "GET";
     }
 
     @Override
-    @Nullable
-    protected String url(Void unused) {
+    public String url(Void unused) {
       return "https://opentelemetry.io/benchmark";
     }
 
     @Override
-    protected List<String> requestHeader(Void unused, String name) {
+    public List<String> requestHeader(Void unused, String name) {
       if (name.equalsIgnoreCase("user-agent")) {
         return Collections.singletonList("OpenTelemetryBot");
       }
       return Collections.emptyList();
     }
 
     @Override
-    @Nullable
-    protected Long requestContentLength(Void unused, @Nullable Void unused2) {
+    public Long requestContentLength(Void unused, @Nullable Void unused2) {
       return 100L;
     }
 
     @Override
     @Nullable
-    protected Long requestContentLengthUncompressed(Void unused, @Nullable Void unused2) {
+    public Long requestContentLengthUncompressed(Void unused, @Nullable Void unused2) {
       return null;
     }
 
     @Override
-    @Nullable
-    protected String flavor(Void unused, @Nullable Void unused2) {
+    public String flavor(Void unused, @Nullable Void unused2) {
       return SemanticAttributes.HttpFlavorValues.HTTP_2_0;
     }
 
     @Override
-    @Nullable
-    protected Integer statusCode(Void unused, Void unused2) {
+    public Integer statusCode(Void unused, Void unused2) {
       return 200;
     }
 
     @Override
-    @Nullable
-    protected Long responseContentLength(Void unused, Void unused2) {
+    public Long responseContentLength(Void unused, Void unused2) {
       return 100L;
     }
 
     @Override
     @Nullable
-    protected Long responseContentLengthUncompressed(Void unused, Void unused2) {
+    public Long responseContentLengthUncompressed(Void unused, Void unused2) {
       return null;
     }
 
     @Override
-    protected List<String> responseHeader(Void unused, Void unused2, String name) {
+    public List<String> responseHeader(Void unused, Void unused2, String name) {
       return Collections.emptyList();
     }
   }

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

@@ -6,9 +6,7 @@
 package io.opentelemetry.instrumentation.api.instrumenter.http;
 
 import io.opentelemetry.api.common.AttributesBuilder;
-import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.config.Config;
-import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import javax.annotation.Nullable;
 
@@ -21,53 +19,48 @@
  * return {@code null} from the protected attribute methods, but implement as many as possible for
  * best compliance with the OpenTelemetry specification.
  */
-public abstract class HttpClientAttributesExtractor<REQUEST, RESPONSE>
-    extends HttpCommonAttributesExtractor<REQUEST, RESPONSE> {
+public final class HttpClientAttributesExtractor<REQUEST, RESPONSE>
+    extends HttpCommonAttributesExtractor<
+        REQUEST, RESPONSE, HttpClientAttributesGetter<REQUEST, RESPONSE>> {
 
+  /** Creates the HTTP client attributes extractor with default configuration. */
+  public static <REQUEST, RESPONSE> HttpClientAttributesExtractor<REQUEST, RESPONSE> create(
+      HttpClientAttributesGetter<REQUEST, RESPONSE> getter) {
+    return create(getter, CapturedHttpHeaders.client(Config.get()));
+  }
+
+  // TODO: there should be a builder for all optional attributes
   /**
    * Creates the HTTP client attributes extractor.
    *
    * @param capturedHttpHeaders A configuration object specifying which HTTP request and response
    *     headers should be captured as span attributes.
    */
-  protected HttpClientAttributesExtractor(CapturedHttpHeaders capturedHttpHeaders) {
-    super(capturedHttpHeaders);
+  public static <REQUEST, RESPONSE> HttpClientAttributesExtractor<REQUEST, RESPONSE> create(
+      HttpClientAttributesGetter<REQUEST, RESPONSE> getter,
+      CapturedHttpHeaders capturedHttpHeaders) {
+    return new HttpClientAttributesExtractor<>(getter, capturedHttpHeaders);
   }
 
-  /** Creates the HTTP client attributes extractor with default configuration. */
-  protected HttpClientAttributesExtractor() {
-    this(CapturedHttpHeaders.client(Config.get()));
+  private HttpClientAttributesExtractor(
+      HttpClientAttributesGetter<REQUEST, RESPONSE> getter,
+      CapturedHttpHeaders capturedHttpHeaders) {
+    super(getter, capturedHttpHeaders);
   }
 
   @Override
-  public final void onStart(AttributesBuilder attributes, REQUEST request) {
+  public void onStart(AttributesBuilder attributes, REQUEST request) {
     super.onStart(attributes, request);
-    set(attributes, SemanticAttributes.HTTP_URL, url(request));
+    set(attributes, SemanticAttributes.HTTP_URL, getter.url(request));
   }
 
   @Override
-  public final void onEnd(
+  public void onEnd(
       AttributesBuilder attributes,
       REQUEST request,
       @Nullable RESPONSE response,
       @Nullable Throwable error) {
     super.onEnd(attributes, request, response, error);
-    set(attributes, SemanticAttributes.HTTP_FLAVOR, flavor(request, response));
+    set(attributes, SemanticAttributes.HTTP_FLAVOR, getter.flavor(request, response));
   }
-
-  // Attributes that always exist in a request
-
-  @Nullable
-  protected abstract String url(REQUEST request);
-
-  // Attributes which are not always available when the request is ready.
-
-  /**
-   * Extracts the {@code http.flavor} span attribute.
-   *
-   * <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, whether
-   * {@code response} is {@code null} or not.
-   */
-  @Nullable
-  protected abstract String flavor(REQUEST request, @Nullable RESPONSE response);
 }

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

@@ -0,0 +1,37 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.http;
+
+import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import javax.annotation.Nullable;
+
+/**
+ * An interface for getting HTTP client attributes.
+ *
+ * <p>Instrumentation authors will create implementations of this interface for their specific
+ * library/framework. It will be used by the {@link HttpClientAttributesExtractor} to obtain the
+ * various HTTP client attributes in a type-generic way.
+ */
+public interface HttpClientAttributesGetter<REQUEST, RESPONSE>
+    extends HttpCommonAttributesGetter<REQUEST, RESPONSE> {
+
+  // Attributes that always exist in a request
+
+  @Nullable
+  String url(REQUEST request);
+
+  // Attributes which are not always available when the request is ready.
+
+  /**
+   * Extracts the {@code http.flavor} span attribute.
+   *
+   * <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, whether
+   * {@code response} is {@code null} or not.
+   */
+  @Nullable
+  String flavor(REQUEST request, @Nullable RESPONSE response);
+}

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

@@ -9,9 +9,7 @@
 import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpHeaderAttributes.responseAttributeKey;
 
 import io.opentelemetry.api.common.AttributesBuilder;
-import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
-import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import java.util.List;
 import javax.annotation.Nullable;
@@ -21,22 +19,25 @@
  * href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#common-attributes">HTTP
  * attributes</a> that are common to client and server instrumentations.
  */
-public abstract class HttpCommonAttributesExtractor<REQUEST, RESPONSE>
+abstract class HttpCommonAttributesExtractor<
+        REQUEST, RESPONSE, GETTER extends HttpCommonAttributesGetter<REQUEST, RESPONSE>>
     implements AttributesExtractor<REQUEST, RESPONSE> {
 
+  final GETTER getter;
   private final CapturedHttpHeaders capturedHttpHeaders;
 
-  HttpCommonAttributesExtractor(CapturedHttpHeaders capturedHttpHeaders) {
+  HttpCommonAttributesExtractor(GETTER getter, CapturedHttpHeaders capturedHttpHeaders) {
+    this.getter = getter;
     this.capturedHttpHeaders = capturedHttpHeaders;
   }
 
   @Override
   public void onStart(AttributesBuilder attributes, REQUEST request) {
-    set(attributes, SemanticAttributes.HTTP_METHOD, method(request));
+    set(attributes, SemanticAttributes.HTTP_METHOD, getter.method(request));
     set(attributes, SemanticAttributes.HTTP_USER_AGENT, userAgent(request));
 
     for (String name : capturedHttpHeaders.requestHeaders()) {
-      List<String> values = requestHeader(request, name);
+      List<String> values = getter.requestHeader(request, name);
       if (!values.isEmpty()) {
         set(attributes, requestAttributeKey(name), values);
       }
@@ -53,114 +54,40 @@ public void onEnd(
     set(
         attributes,
         SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH,
-        requestContentLength(request, response));
+        getter.requestContentLength(request, response));
     set(
         attributes,
         SemanticAttributes.HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED,
-        requestContentLengthUncompressed(request, response));
+        getter.requestContentLengthUncompressed(request, response));
 
     if (response != null) {
-      Integer statusCode = statusCode(request, response);
+      Integer statusCode = getter.statusCode(request, response);
       if (statusCode != null && statusCode > 0) {
         set(attributes, SemanticAttributes.HTTP_STATUS_CODE, (long) statusCode);
       }
       set(
           attributes,
           SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH,
-          responseContentLength(request, response));
+          getter.responseContentLength(request, response));
       set(
           attributes,
           SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED,
-          responseContentLengthUncompressed(request, response));
+          getter.responseContentLengthUncompressed(request, response));
 
       for (String name : capturedHttpHeaders.responseHeaders()) {
-        List<String> values = responseHeader(request, response, name);
+        List<String> values = getter.responseHeader(request, response, name);
         if (!values.isEmpty()) {
           set(attributes, responseAttributeKey(name), values);
         }
       }
     }
   }
 
-  // Attributes that always exist in a request
-
-  @Nullable
-  protected abstract String method(REQUEST request);
-
   @Nullable
   private String userAgent(REQUEST request) {
-    return firstHeaderValue(requestHeader(request, "user-agent"));
+    return firstHeaderValue(getter.requestHeader(request, "user-agent"));
   }
 
-  /**
-   * Extracts all values of header named {@code name} from the request, or an empty list if there
-   * were none.
-   *
-   * <p>Implementations of this method <b>must not</b> return a null value; an empty list should be
-   * returned instead.
-   */
-  protected abstract List<String> requestHeader(REQUEST request, String name);
-
-  // Attributes which are not always available when the request is ready.
-
-  /**
-   * Extracts the {@code http.request_content_length} span attribute.
-   *
-   * <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, whether
-   * {@code response} is {@code null} or not.
-   */
-  @Nullable
-  protected abstract Long requestContentLength(REQUEST request, @Nullable RESPONSE response);
-
-  /**
-   * Extracts the {@code http.request_content_length_uncompressed} span attribute.
-   *
-   * <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, whether
-   * {@code response} is {@code null} or not.
-   */
-  @Nullable
-  protected abstract Long requestContentLengthUncompressed(
-      REQUEST request, @Nullable RESPONSE response);
-
-  /**
-   * Extracts the {@code http.status_code} span attribute.
-   *
-   * <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, only when
-   * {@code response} is non-{@code null}.
-   */
-  @Nullable
-  protected abstract Integer statusCode(REQUEST request, RESPONSE response);
-
-  /**
-   * Extracts the {@code http.response_content_length} span attribute.
-   *
-   * <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, only when
-   * {@code response} is non-{@code null}.
-   */
-  @Nullable
-  protected abstract Long responseContentLength(REQUEST request, RESPONSE response);
-
-  /**
-   * Extracts the {@code http.response_content_length_uncompressed} span attribute.
-   *
-   * <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, only when
-   * {@code response} is non-{@code null}.
-   */
-  @Nullable
-  protected abstract Long responseContentLengthUncompressed(REQUEST request, RESPONSE response);
-
-  /**
-   * Extracts all values of header named {@code name} from the response, or an empty list if there
-   * were none.
-   *
-   * <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, only when
-   * {@code response} is non-{@code null}.
-   *
-   * <p>Implementations of this method <b>must not</b> return a null value; an empty list should be
-   * returned instead.
-   */
-  protected abstract List<String> responseHeader(REQUEST request, RESPONSE response, String name);
-
   @Nullable
   static String firstHeaderValue(List<String> values) {
     return values.isEmpty() ? null : values.get(0);