HTTP semconv: filter out default peer/host ports

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

@@ -9,6 +9,8 @@
 
 import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
+import io.opentelemetry.instrumentation.api.instrumenter.net.internal.InternalNetClientAttributesExtractor;
 import io.opentelemetry.instrumentation.api.internal.SpanKey;
 import io.opentelemetry.instrumentation.api.internal.SpanKeyProvider;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
@@ -29,32 +31,78 @@ public final class HttpClientAttributesExtractor<REQUEST, RESPONSE>
         REQUEST, RESPONSE, HttpClientAttributesGetter<REQUEST, RESPONSE>>
     implements SpanKeyProvider {
 
-  /** Creates the HTTP client attributes extractor with default configuration. */
+  /**
+   * Creates the HTTP client attributes extractor with default configuration.
+   *
+   * @deprecated Use {@link #create(HttpClientAttributesGetter, NetClientAttributesGetter)} instead.
+   */
+  @Deprecated
   public static <REQUEST, RESPONSE> HttpClientAttributesExtractor<REQUEST, RESPONSE> create(
       HttpClientAttributesGetter<REQUEST, RESPONSE> getter) {
     return builder(getter).build();
   }
 
+  /** Creates the HTTP client attributes extractor with default configuration. */
+  public static <REQUEST, RESPONSE> HttpClientAttributesExtractor<REQUEST, RESPONSE> create(
+      HttpClientAttributesGetter<REQUEST, RESPONSE> httpAttributesGetter,
+      NetClientAttributesGetter<REQUEST, RESPONSE> netAttributesGetter) {
+    return builder(httpAttributesGetter, netAttributesGetter).build();
+  }
+
   /**
    * Returns a new {@link HttpClientAttributesExtractorBuilder} that can be used to configure the
    * HTTP client attributes extractor.
+   *
+   * @deprecated Use {@link #builder(HttpClientAttributesGetter, NetClientAttributesGetter)}
+   *     instead.
    */
+  @Deprecated
   public static <REQUEST, RESPONSE> HttpClientAttributesExtractorBuilder<REQUEST, RESPONSE> builder(
-      HttpClientAttributesGetter<REQUEST, RESPONSE> getter) {
-    return new HttpClientAttributesExtractorBuilder<>(getter);
+      HttpClientAttributesGetter<REQUEST, RESPONSE> httpAttributesGetter) {
+    return builder(httpAttributesGetter, new NoopNetClientAttributesGetter<>());
   }
 
+  /**
+   * Returns a new {@link HttpClientAttributesExtractorBuilder} that can be used to configure the
+   * HTTP client attributes extractor.
+   */
+  public static <REQUEST, RESPONSE> HttpClientAttributesExtractorBuilder<REQUEST, RESPONSE> builder(
+      HttpClientAttributesGetter<REQUEST, RESPONSE> httpAttributesGetter,
+      NetClientAttributesGetter<REQUEST, RESPONSE> netAttributesGetter) {
+    return new HttpClientAttributesExtractorBuilder<>(httpAttributesGetter, netAttributesGetter);
+  }
+
+  private final InternalNetClientAttributesExtractor<REQUEST, RESPONSE> internalNetExtractor;
+
   HttpClientAttributesExtractor(
-      HttpClientAttributesGetter<REQUEST, RESPONSE> getter,
+      HttpClientAttributesGetter<REQUEST, RESPONSE> httpAttributesGetter,
+      NetClientAttributesGetter<REQUEST, RESPONSE> netAttributesGetter,
       List<String> capturedRequestHeaders,
-      List<String> responseHeaders) {
-    super(getter, capturedRequestHeaders, responseHeaders);
+      List<String> capturedResponseHeaders) {
+    super(httpAttributesGetter, capturedRequestHeaders, capturedResponseHeaders);
+    internalNetExtractor =
+        new InternalNetClientAttributesExtractor<>(netAttributesGetter, this::isValidNetPeerPort);
   }
 
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
     super.onStart(attributes, parentContext, request);
+
     internalSet(attributes, SemanticAttributes.HTTP_URL, stripSensitiveData(getter.url(request)));
+
+    internalNetExtractor.onStart(attributes, request);
+  }
+
+  private boolean isValidNetPeerPort(int port, REQUEST request) {
+    String url = getter.url(request);
+    if (url == null) {
+      return true;
+    }
+    // according to spec: extract if not default (80 for http scheme, 443 for https).
+    if ((url.startsWith("http://") && port == 80) || (url.startsWith("https://") && port == 443)) {
+      return false;
+    }
+    return true;
   }
 
   @Nullable
@@ -108,7 +156,10 @@ public void onEnd(
       @Nullable RESPONSE response,
       @Nullable Throwable error) {
     super.onEnd(attributes, context, request, response, error);
+
     internalSet(attributes, SemanticAttributes.HTTP_FLAVOR, getter.flavor(request, response));
+
+    internalNetExtractor.onEnd(attributes, request, response);
   }
 
   /**
@@ -119,4 +170,26 @@ public void onEnd(
   public SpanKey internalGetSpanKey() {
     return SpanKey.HTTP_CLIENT;
   }
+
+  private static final class NoopNetClientAttributesGetter<REQUEST, RESPONSE>
+      implements NetClientAttributesGetter<REQUEST, RESPONSE> {
+
+    @Nullable
+    @Override
+    public String transport(REQUEST request, @Nullable RESPONSE response) {
+      return null;
+    }
+
+    @Nullable
+    @Override
+    public String peerName(REQUEST request) {
+      return null;
+    }
+
+    @Nullable
+    @Override
+    public Integer peerPort(REQUEST request) {
+      return null;
+    }
+  }
 }

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

@@ -8,17 +8,22 @@
 import static java.util.Collections.emptyList;
 
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
 import java.util.List;
 
 /** A builder of {@link HttpClientAttributesExtractor}. */
 public final class HttpClientAttributesExtractorBuilder<REQUEST, RESPONSE> {
 
-  final HttpClientAttributesGetter<REQUEST, RESPONSE> getter;
+  final HttpClientAttributesGetter<REQUEST, RESPONSE> httpAttributesGetter;
+  final NetClientAttributesGetter<REQUEST, RESPONSE> netAttributesGetter;
   List<String> capturedRequestHeaders = emptyList();
   List<String> capturedResponseHeaders = emptyList();
 
-  HttpClientAttributesExtractorBuilder(HttpClientAttributesGetter<REQUEST, RESPONSE> getter) {
-    this.getter = getter;
+  HttpClientAttributesExtractorBuilder(
+      HttpClientAttributesGetter<REQUEST, RESPONSE> httpAttributesGetter,
+      NetClientAttributesGetter<REQUEST, RESPONSE> netAttributesGetter) {
+    this.httpAttributesGetter = httpAttributesGetter;
+    this.netAttributesGetter = netAttributesGetter;
   }
 
   /**
@@ -64,6 +69,6 @@ public HttpClientAttributesExtractorBuilder<REQUEST, RESPONSE> setCapturedRespon
    */
   public HttpClientAttributesExtractor<REQUEST, RESPONSE> build() {
     return new HttpClientAttributesExtractor<>(
-        getter, capturedRequestHeaders, capturedResponseHeaders);
+        httpAttributesGetter, netAttributesGetter, capturedRequestHeaders, capturedResponseHeaders);
   }
 }

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

@@ -77,7 +77,7 @@ public static <REQUEST, RESPONSE> HttpServerAttributesExtractorBuilder<REQUEST,
     return new HttpServerAttributesExtractorBuilder<>(httpAttributesGetter, netAttributesGetter);
   }
 
-  private final NetServerAttributesGetter<REQUEST> netAttributesGetter;
+  private final InternalNetServerAttributesExtractor<REQUEST> internalNetExtractor;
   private final Function<Context, String> httpRouteHolderGetter;
 
   HttpServerAttributesExtractor(
@@ -98,10 +98,11 @@ public static <REQUEST, RESPONSE> HttpServerAttributesExtractorBuilder<REQUEST,
       HttpServerAttributesGetter<REQUEST, RESPONSE> httpAttributesGetter,
       NetServerAttributesGetter<REQUEST> netAttributesGetter,
       List<String> capturedRequestHeaders,
-      List<String> responseHeaders,
+      List<String> capturedResponseHeaders,
       Function<Context, String> httpRouteHolderGetter) {
-    super(httpAttributesGetter, capturedRequestHeaders, responseHeaders);
-    this.netAttributesGetter = netAttributesGetter;
+    super(httpAttributesGetter, capturedRequestHeaders, capturedResponseHeaders);
+    internalNetExtractor =
+        new InternalNetServerAttributesExtractor<>(netAttributesGetter, this::isValidNetHostPort);
     this.httpRouteHolderGetter = httpRouteHolderGetter;
   }
 
@@ -117,8 +118,19 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
     internalSet(attributes, SemanticAttributes.HTTP_ROUTE, getter.route(request));
     internalSet(attributes, SemanticAttributes.HTTP_CLIENT_IP, clientIp(request));
 
-    InternalNetServerAttributesExtractor.onStart(
-        netAttributesGetter, attributes, request, host(request));
+    internalNetExtractor.onStart(attributes, request, host(request));
+  }
+
+  private boolean isValidNetHostPort(int port, REQUEST request) {
+    String scheme = getter.scheme(request);
+    if (scheme == null) {
+      return true;
+    }
+    // according to spec: extract if not default (80 for http scheme, 443 for https).
+    if ((scheme.equals("http") && port == 80) || (scheme.equals("https") && port == 443)) {
+      return false;
+    }
+    return true;
   }
 
   @Override

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

@@ -5,12 +5,10 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter.net;
 
-import static io.opentelemetry.instrumentation.api.internal.AttributesExtractorUtil.internalSet;
-
 import io.opentelemetry.api.common.AttributesBuilder;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
-import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
+import io.opentelemetry.instrumentation.api.instrumenter.net.internal.InternalNetClientAttributesExtractor;
 import javax.annotation.Nullable;
 
 /**
@@ -25,27 +23,20 @@
 public final class NetClientAttributesExtractor<REQUEST, RESPONSE>
     implements AttributesExtractor<REQUEST, RESPONSE> {
 
-  private final NetClientAttributesGetter<REQUEST, RESPONSE> getter;
+  private final InternalNetClientAttributesExtractor<REQUEST, RESPONSE> internalExtractor;
 
   public static <REQUEST, RESPONSE> NetClientAttributesExtractor<REQUEST, RESPONSE> create(
       NetClientAttributesGetter<REQUEST, RESPONSE> getter) {
     return new NetClientAttributesExtractor<>(getter);
   }
 
   private NetClientAttributesExtractor(NetClientAttributesGetter<REQUEST, RESPONSE> getter) {
-    this.getter = getter;
+    internalExtractor = new InternalNetClientAttributesExtractor<>(getter, (port, request) -> true);
   }
 
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
-    String peerName = getter.peerName(request);
-    Integer peerPort = getter.peerPort(request);
-    if (peerName != null) {
-      internalSet(attributes, SemanticAttributes.NET_PEER_NAME, peerName);
-      if (peerPort != null && peerPort > 0) {
-        internalSet(attributes, SemanticAttributes.NET_PEER_PORT, (long) peerPort);
-      }
-    }
+    internalExtractor.onStart(attributes, request);
   }
 
   @Override
@@ -55,30 +46,6 @@ public void onEnd(
       REQUEST request,
       @Nullable RESPONSE response,
       @Nullable Throwable error) {
-
-    internalSet(attributes, SemanticAttributes.NET_TRANSPORT, getter.transport(request, response));
-
-    String peerName = getter.peerName(request);
-    Integer peerPort = getter.peerPort(request);
-
-    String sockPeerAddr = getter.sockPeerAddr(request, response);
-    if (sockPeerAddr != null && !sockPeerAddr.equals(peerName)) {
-      internalSet(attributes, SemanticAttributes.NET_SOCK_PEER_ADDR, sockPeerAddr);
-
-      Integer sockPeerPort = getter.sockPeerPort(request, response);
-      if (sockPeerPort != null && sockPeerPort > 0 && !sockPeerPort.equals(peerPort)) {
-        internalSet(attributes, SemanticAttributes.NET_SOCK_PEER_PORT, (long) sockPeerPort);
-      }
-
-      String sockFamily = getter.sockFamily(request, response);
-      if (sockFamily != null && !SemanticAttributes.NetSockFamilyValues.INET.equals(sockFamily)) {
-        internalSet(attributes, SemanticAttributes.NET_SOCK_FAMILY, sockFamily);
-      }
-
-      String sockPeerName = getter.sockPeerName(request, response);
-      if (sockPeerName != null && !sockPeerName.equals(peerName)) {
-        internalSet(attributes, SemanticAttributes.NET_SOCK_PEER_NAME, sockPeerName);
-      }
-    }
+    internalExtractor.onEnd(attributes, request, response);
   }
 }

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

@@ -20,20 +20,21 @@
 public final class NetServerAttributesExtractor<REQUEST, RESPONSE>
     implements AttributesExtractor<REQUEST, RESPONSE> {
 
-  private final NetServerAttributesGetter<REQUEST> getter;
+  private final InternalNetServerAttributesExtractor<REQUEST> internalExtractor;
 
   public static <REQUEST, RESPONSE> NetServerAttributesExtractor<REQUEST, RESPONSE> create(
       NetServerAttributesGetter<REQUEST> getter) {
     return new NetServerAttributesExtractor<>(getter);
   }
 
   private NetServerAttributesExtractor(NetServerAttributesGetter<REQUEST> getter) {
-    this.getter = getter;
+    internalExtractor =
+        new InternalNetServerAttributesExtractor<>(getter, (integer, request) -> true);
   }
 
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
-    InternalNetServerAttributesExtractor.onStart(getter, attributes, request, null);
+    internalExtractor.onStart(attributes, request, null);
   }
 
   @Override

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/InternalNetClientAttributesExtractor.java

@@ -0,0 +1,73 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.net.internal;
+
+import static io.opentelemetry.instrumentation.api.internal.AttributesExtractorUtil.internalSet;
+
+import io.opentelemetry.api.common.AttributesBuilder;
+import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
+import java.util.function.BiPredicate;
+import javax.annotation.Nullable;
+
+/**
+ * This class is internal and is hence not for public use. Its APIs are unstable and can change at
+ * any time.
+ */
+public final class InternalNetClientAttributesExtractor<REQUEST, RESPONSE> {
+
+  private final NetClientAttributesGetter<REQUEST, RESPONSE> getter;
+  private final BiPredicate<Integer, REQUEST> netPeerPortCondition;
+
+  public InternalNetClientAttributesExtractor(
+      NetClientAttributesGetter<REQUEST, RESPONSE> getter,
+      BiPredicate<Integer, REQUEST> netPeerPortCondition) {
+    this.getter = getter;
+    this.netPeerPortCondition = netPeerPortCondition;
+  }
+
+  public void onStart(AttributesBuilder attributes, REQUEST request) {
+    String peerName = getter.peerName(request);
+    Integer peerPort = getter.peerPort(request);
+
+    // TODO: add host header parsing
+
+    if (peerName != null) {
+      internalSet(attributes, SemanticAttributes.NET_PEER_NAME, peerName);
+      if (peerPort != null && peerPort > 0 && netPeerPortCondition.test(peerPort, request)) {
+        internalSet(attributes, SemanticAttributes.NET_PEER_PORT, (long) peerPort);
+      }
+    }
+  }
+
+  public void onEnd(AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {
+
+    internalSet(attributes, SemanticAttributes.NET_TRANSPORT, getter.transport(request, response));
+
+    String peerName = getter.peerName(request);
+    Integer peerPort = getter.peerPort(request);
+
+    String sockPeerAddr = getter.sockPeerAddr(request, response);
+    if (sockPeerAddr != null && !sockPeerAddr.equals(peerName)) {
+      internalSet(attributes, SemanticAttributes.NET_SOCK_PEER_ADDR, sockPeerAddr);
+
+      Integer sockPeerPort = getter.sockPeerPort(request, response);
+      if (sockPeerPort != null && sockPeerPort > 0 && !sockPeerPort.equals(peerPort)) {
+        internalSet(attributes, SemanticAttributes.NET_SOCK_PEER_PORT, (long) sockPeerPort);
+      }
+
+      String sockFamily = getter.sockFamily(request, response);
+      if (sockFamily != null && !SemanticAttributes.NetSockFamilyValues.INET.equals(sockFamily)) {
+        internalSet(attributes, SemanticAttributes.NET_SOCK_FAMILY, sockFamily);
+      }
+
+      String sockPeerName = getter.sockPeerName(request, response);
+      if (sockPeerName != null && !sockPeerName.equals(peerName)) {
+        internalSet(attributes, SemanticAttributes.NET_SOCK_PEER_NAME, sockPeerName);
+      }
+    }
+  }
+}