Fix inconsistent handling of net.peer.port in HTTP instrumentations (#…

…7618) Resolves #4329
open-telemetry · Jan 20, 2023 · 40667ce · 40667ce
1 parent eaac709
commit 40667ce
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 80 deletions.
diff --git a/...pentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientRequest.java b/...pentelemetry/javaagent/instrumentation/apachehttpclient/v4_0/ApacheHttpClientRequest.java
@@ -93,27 +93,14 @@ public String getFlavor() {
     return null;
   }
 
+  @Nullable
   public String getPeerName() {
-    return uri != null ? uri.getHost() : null;
+    return uri == null ? null : uri.getHost();
   }
 
+  @Nullable
   public Integer getPeerPort() {
-    if (uri == null) {
-      return null;
-    }
-    int port = uri.getPort();
-    if (port != -1) {
-      return port;
-    }
-    switch (uri.getScheme()) {
-      case "http":
-        return 80;
-      case "https":
-        return 443;
-      default:
-        logger.log(FINE, "no default port mapping for scheme: {0}", uri.getScheme());
-        return null;
-    }
+    return uri == null ? null : uri.getPort();
   }
 
   @Nullable

diff --git a/.../java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientRequest.java b/.../java/io/opentelemetry/instrumentation/apachehttpclient/v4_3/ApacheHttpClientRequest.java
@@ -99,28 +99,13 @@ String getFlavor() {
   }
 
   @Nullable
-  String getPeerName() {
-    return uri != null ? uri.getHost() : null;
+  public String getPeerName() {
+    return uri == null ? null : uri.getHost();
   }
 
   @Nullable
-  Integer getPeerPort() {
-    if (uri == null) {
-      return null;
-    }
-    int port = uri.getPort();
-    if (port != -1) {
-      return port;
-    }
-    switch (uri.getScheme()) {
-      case "http":
-        return 80;
-      case "https":
-        return 443;
-      default:
-        logger.log(FINE, "no default port mapping for scheme: {0}", uri.getScheme());
-        return null;
-    }
+  public Integer getPeerPort() {
+    return uri == null ? null : uri.getPort();
   }
 
   @Nullable

diff --git a/.../javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientNetAttributesGetter.java b/.../javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpClientNetAttributesGetter.java
@@ -5,19 +5,14 @@
 
 package io.opentelemetry.javaagent.instrumentation.apachehttpclient.v5_0;
 
-import static java.util.logging.Level.FINE;
-
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
-import java.util.logging.Logger;
 import javax.annotation.Nullable;
 import org.apache.hc.core5.http.HttpRequest;
 import org.apache.hc.core5.http.HttpResponse;
 
 final class ApacheHttpClientNetAttributesGetter
     implements NetClientAttributesGetter<HttpRequest, HttpResponse> {
-  private static final Logger logger =
-      Logger.getLogger(ApacheHttpClientNetAttributesGetter.class.getName());
 
   @Override
   public String transport(HttpRequest request, @Nullable HttpResponse response) {
@@ -32,22 +27,6 @@ public String peerName(HttpRequest request) {
 
   @Override
   public Integer peerPort(HttpRequest request) {
-    int port = request.getAuthority().getPort();
-    if (port != -1) {
-      return port;
-    }
-    String scheme = request.getScheme();
-    if (scheme == null) {
-      return 80;
-    }
-    switch (scheme) {
-      case "http":
-        return 80;
-      case "https":
-        return 443;
-      default:
-        logger.log(FINE, "no default port mapping for scheme: {0}", scheme);
-        return null;
-    }
+    return request.getAuthority().getPort();
   }
 }
diff --git a/...ava/io/opentelemetry/javaagent/instrumentation/httpclient/JdkHttpNetAttributesGetter.java b/...ava/io/opentelemetry/javaagent/instrumentation/httpclient/JdkHttpNetAttributesGetter.java
@@ -5,20 +5,15 @@
 
 package io.opentelemetry.javaagent.instrumentation.httpclient;
 
-import static java.util.logging.Level.FINE;
-
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse;
-import java.util.logging.Logger;
 import javax.annotation.Nullable;
 
 public class JdkHttpNetAttributesGetter
     implements NetClientAttributesGetter<HttpRequest, HttpResponse<?>> {
 
-  private static final Logger logger = Logger.getLogger(JdkHttpNetAttributesGetter.class.getName());
-
   @Override
   public String transport(HttpRequest httpRequest, @Nullable HttpResponse<?> response) {
     return SemanticAttributes.NetTransportValues.IP_TCP;
@@ -33,22 +28,6 @@ public String peerName(HttpRequest httpRequest) {
   @Override
   @Nullable
   public Integer peerPort(HttpRequest httpRequest) {
-    int port = httpRequest.uri().getPort();
-    if (port != -1) {
-      return port;
-    }
-    String scheme = httpRequest.uri().getScheme();
-    if (scheme == null) {
-      return 80;
-    }
-    switch (scheme) {
-      case "http":
-        return 80;
-      case "https":
-        return 443;
-      default:
-        logger.log(FINE, "no default port mapping for scheme: {0}", scheme);
-        return null;
-    }
+    return httpRequest.uri().getPort();
   }
 }