Remove server span peer name (open-telemetry#5404)

trask · RashmiRam · commit 352da1acab46 · 2022-05-23T12:23:31.000+05:30
* Fix server -&gt; client reference

* Remove server span peer name
diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/InetSocketAddressNetServerAttributesGetter.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/InetSocketAddressNetServerAttributesGetter.java
@@ -22,16 +22,6 @@ public abstract class InetSocketAddressNetServerAttributesGetter<REQUEST>
   @Nullable
   public abstract InetSocketAddress getAddress(REQUEST request);
 
-  @Override
-  @Nullable
-  public final String peerName(REQUEST request) {
-    InetSocketAddress address = getAddress(request);
-    if (address == null) {
-      return null;
-    }
-    return address.getHostString();
-  }
-
   @Override
   @Nullable
   public final Integer peerPort(REQUEST request) {
diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java
@@ -36,11 +36,7 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
     set(attributes, SemanticAttributes.NET_TRANSPORT, getter.transport(request));
 
     String peerIp = getter.peerIp(request);
-    String peerName = getter.peerName(request);
 
-    if (peerName != null && !peerName.equals(peerIp)) {
-      set(attributes, SemanticAttributes.NET_PEER_NAME, peerName);
-    }
     set(attributes, SemanticAttributes.NET_PEER_IP, peerIp);
 
     Integer peerPort = getter.peerPort(request);
diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesGetter.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesGetter.java
@@ -9,7 +9,7 @@
 
 /**
  * An interface for getting server-based network attributes. It adapts a vendor-specific request
- * type into the 4 common attributes (transport, peerName, peerPort, peerIp).
+ * type into the 3 common attributes (transport, peerPort, peerIp).
  *
  * <p>Instrumentation authors will create implementations of this interface for their specific
  * server library/framework. It will be used by the {@link NetServerAttributesExtractor} to obtain
@@ -20,9 +20,6 @@ public interface NetServerAttributesGetter<REQUEST> {
   @Nullable
   String transport(REQUEST request);
 
-  @Nullable
-  String peerName(REQUEST request);
-
   @Nullable
   Integer peerPort(REQUEST request);
 
diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java
@@ -784,11 +784,6 @@ public String transport(REQUEST request) {
       return null;
     }
 
-    @Override
-    public String peerName(REQUEST request) {
-      return null;
-    }
-
     @Override
     public Integer peerPort(REQUEST request) {
       return null;
diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/net/InetSocketAddressNetServerAttributesGetterTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/net/InetSocketAddressNetServerAttributesGetterTest.java
@@ -66,7 +66,6 @@ void fullAddress() {
         .containsOnly(
             entry(SemanticAttributes.NET_TRANSPORT, SemanticAttributes.NetTransportValues.IP_TCP),
             entry(SemanticAttributes.NET_PEER_IP, request.getAddress().getHostAddress()),
-            entry(SemanticAttributes.NET_PEER_NAME, "github.com"),
             entry(SemanticAttributes.NET_PEER_PORT, 123L));
 
     assertThat(endAttributes.build()).isEmpty();
@@ -94,7 +93,6 @@ void unresolved() {
     assertThat(startAttributes.build())
         .containsOnly(
             entry(SemanticAttributes.NET_TRANSPORT, SemanticAttributes.NetTransportValues.IP_TCP),
-            entry(SemanticAttributes.NET_PEER_NAME, "github.com"),
             entry(SemanticAttributes.NET_PEER_PORT, 123L));
 
     assertThat(endAttributes.build()).isEmpty();
diff --git a/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractorTest.java b/instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractorTest.java
@@ -26,11 +26,6 @@ public String transport(Map<String, String> request) {
       return request.get("transport");
     }
 
-    @Override
-    public String peerName(Map<String, String> request) {
-      return request.get("peerName");
-    }
-
     @Override
     public Integer peerPort(Map<String, String> request) {
       return Integer.valueOf(request.get("peerPort"));
@@ -72,7 +67,6 @@ void normal() {
     assertThat(startAttributes.build())
         .containsOnly(
             entry(SemanticAttributes.NET_TRANSPORT, "TCP"),
-            entry(SemanticAttributes.NET_PEER_NAME, "github.com"),
             entry(SemanticAttributes.NET_PEER_PORT, 123L),
             entry(SemanticAttributes.NET_PEER_IP, "1.2.3.4"));
 
diff --git a/instrumentation/grizzly-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grizzly/GrizzlyNetAttributesGetter.java b/instrumentation/grizzly-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/grizzly/GrizzlyNetAttributesGetter.java
@@ -17,12 +17,6 @@ public String transport(HttpRequestPacket request) {
     return null;
   }
 
-  @Nullable
-  @Override
-  public String peerName(HttpRequestPacket request) {
-    return request.getRemoteHost();
-  }
-
   @Override
   public Integer peerPort(HttpRequestPacket request) {
     return request.getRemotePort();
diff --git a/instrumentation/ktor-1.0/library/src/main/kotlin/io/opentelemetry/instrumentation/ktor/v1_0/KtorNetServerAttributesGetter.kt b/instrumentation/ktor-1.0/library/src/main/kotlin/io/opentelemetry/instrumentation/ktor/v1_0/KtorNetServerAttributesGetter.kt
@@ -6,7 +6,6 @@
 package io.opentelemetry.instrumentation.ktor.v1_0
 
 import io.ktor.request.*
-import io.ktor.response.*
 import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesGetter
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
 
@@ -15,14 +14,6 @@ internal class KtorNetServerAttributesGetter : NetServerAttributesGetter<Applica
     return SemanticAttributes.NetTransportValues.IP_TCP
   }
 
-  override fun peerName(request: ApplicationRequest): String? {
-    var remote = request.local.remoteHost
-    if (remote != null && "unknown" != remote && !isIpAddress(remote)) {
-      return remote
-    }
-    return null
-  }
-
   override fun peerPort(request: ApplicationRequest): Int? {
     return null
   }
diff --git a/instrumentation/liberty/compile-stub/src/main/java/com/ibm/ws/http/dispatcher/internal/channel/HttpDispatcherLink.java b/instrumentation/liberty/compile-stub/src/main/java/com/ibm/ws/http/dispatcher/internal/channel/HttpDispatcherLink.java
@@ -19,10 +19,6 @@ public String getRemoteHostAddress() {
     throw new UnsupportedOperationException();
   }
 
-  public String getRemoteHostName(boolean canonical) {
-    throw new UnsupportedOperationException();
-  }
-
   public int getRequestedPort() {
     throw new UnsupportedOperationException();
   }
diff --git a/instrumentation/liberty/liberty-dispatcher/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/dispatcher/LibertyDispatcherNetAttributesGetter.java b/instrumentation/liberty/liberty-dispatcher/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/dispatcher/LibertyDispatcherNetAttributesGetter.java
@@ -17,12 +17,6 @@ public String transport(LibertyRequest libertyRequest) {
     return SemanticAttributes.NetTransportValues.IP_TCP;
   }
 
-  @Override
-  @Nullable
-  public String peerName(LibertyRequest libertyRequest) {
-    return libertyRequest.peerName();
-  }
-
   @Override
   @Nullable
   public Integer peerPort(LibertyRequest libertyRequest) {
diff --git a/instrumentation/liberty/liberty-dispatcher/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/dispatcher/LibertyRequest.java b/instrumentation/liberty/liberty-dispatcher/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/dispatcher/LibertyRequest.java
@@ -71,10 +71,6 @@ public String peerIp() {
     return httpDispatcherLink.getRemoteHostAddress();
   }
 
-  public String peerName() {
-    return httpDispatcherLink.getRemoteHostName(false);
-  }
-
   public String getProtocol() {
     return httpRequestMessage.getVersion();
   }
diff --git a/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/internal/RatpackNetAttributesGetter.java b/instrumentation/ratpack/ratpack-1.7/library/src/main/java/io/opentelemetry/instrumentation/ratpack/internal/RatpackNetAttributesGetter.java
@@ -16,17 +16,10 @@
  */
 public final class RatpackNetAttributesGetter implements NetServerAttributesGetter<Request> {
   @Override
-  @Nullable
   public String transport(Request request) {
     return SemanticAttributes.NetTransportValues.IP_TCP;
   }
 
-  @Override
-  @Nullable
-  public String peerName(Request request) {
-    return null;
-  }
-
   @Override
   public Integer peerPort(Request request) {
     return request.getRemoteAddress().getPort();
diff --git a/instrumentation/restlet/restlet-1.0/library/src/main/java/io/opentelemetry/instrumentation/restlet/v1_0/RestletNetAttributesGetter.java b/instrumentation/restlet/restlet-1.0/library/src/main/java/io/opentelemetry/instrumentation/restlet/v1_0/RestletNetAttributesGetter.java
@@ -16,12 +16,6 @@ public String transport(Request request) {
     return SemanticAttributes.NetTransportValues.IP_TCP;
   }
 
-  @Override
-  @Nullable
-  public String peerName(Request request) {
-    return null;
-  }
-
   @Override
   public Integer peerPort(Request request) {
     return request.getClientInfo().getPort();
diff --git a/instrumentation/restlet/restlet-2.0/library/src/main/java/io/opentelemetry/instrumentation/restlet/v2_0/internal/RestletNetAttributesGetter.java b/instrumentation/restlet/restlet-2.0/library/src/main/java/io/opentelemetry/instrumentation/restlet/v2_0/internal/RestletNetAttributesGetter.java
@@ -16,12 +16,6 @@ public String transport(Request request) {
     return SemanticAttributes.NetTransportValues.IP_TCP;
   }
 
-  @Override
-  @Nullable
-  public String peerName(Request request) {
-    return null;
-  }
-
   @Override
   public Integer peerPort(Request request) {
     return request.getClientInfo().getPort();
diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5Accessor.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5Accessor.java
@@ -123,11 +123,6 @@ public Integer getRequestRemotePort(HttpServletRequest request) {
     return request.getRemotePort();
   }
 
-  @Override
-  public String getRequestRemoteHost(HttpServletRequest request) {
-    return request.getRemoteHost();
-  }
-
   @Override
   public int getRequestContentLength(HttpServletRequest request) {
     return request.getContentLength();
diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletAccessor.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletAccessor.java
@@ -57,8 +57,6 @@ public interface ServletAccessor<REQUEST, RESPONSE> {
 
   Integer getRequestRemotePort(REQUEST request);
 
-  String getRequestRemoteHost(REQUEST request);
-
   int getRequestContentLength(REQUEST request);
 
   void addRequestAsyncListener(
diff --git a/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletNetAttributesGetter.java b/instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletNetAttributesGetter.java
@@ -23,12 +23,6 @@ public String transport(ServletRequestContext<REQUEST> requestContext) {
     return SemanticAttributes.NetTransportValues.IP_TCP;
   }
 
-  @Override
-  @Nullable
-  public String peerName(ServletRequestContext<REQUEST> requestContext) {
-    return accessor.getRequestRemoteHost(requestContext.request());
-  }
-
   @Override
   @Nullable
   public Integer peerPort(ServletRequestContext<REQUEST> requestContext) {
diff --git a/instrumentation/servlet/servlet-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/JavaxServletAccessor.java b/instrumentation/servlet/servlet-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/JavaxServletAccessor.java
@@ -70,11 +70,6 @@ public String getRequestRemoteAddr(HttpServletRequest request) {
     return request.getRemoteAddr();
   }
 
-  @Override
-  public String getRequestRemoteHost(HttpServletRequest httpServletRequest) {
-    return httpServletRequest.getRemoteHost();
-  }
-
   @Override
   public String getRequestHeader(HttpServletRequest request, String name) {
     return request.getHeader(name);
diff --git a/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcNetAttributesGetter.java b/instrumentation/spring/spring-webmvc-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/SpringWebMvcNetAttributesGetter.java
@@ -17,12 +17,6 @@ public String transport(HttpServletRequest request) {
     return SemanticAttributes.NetTransportValues.IP_TCP;
   }
 
-  @Override
-  @Nullable
-  public String peerName(HttpServletRequest request) {
-    return request.getRemoteHost();
-  }
-
   @Override
   public Integer peerPort(HttpServletRequest request) {
     return request.getRemotePort();
diff --git a/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatNetAttributesGetter.java b/instrumentation/tomcat/tomcat-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/tomcat/common/TomcatNetAttributesGetter.java
@@ -19,14 +19,6 @@ public String transport(Request request) {
     return SemanticAttributes.NetTransportValues.IP_TCP;
   }
 
-  @Override
-  @Nullable
-  public String peerName(Request request) {
-    // not using request.action(ActionCode.REQ_HOST_ATTRIBUTE, request) since that calls
-    // InetAddress.getHostName() which trigger reverse name lookup
-    return null;
-  }
-
   @Override
   @Nullable
   public Integer peerPort(Request request) {
diff --git a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy
@@ -167,7 +167,6 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
     [
       SemanticAttributes.HTTP_ROUTE,
       SemanticAttributes.NET_TRANSPORT,
-      SemanticAttributes.NET_PEER_NAME,
       SemanticAttributes.NET_PEER_PORT
     ] as Set
   }
@@ -695,10 +694,6 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
         if (httpAttributes.contains(SemanticAttributes.NET_TRANSPORT)) {
           "$SemanticAttributes.NET_TRANSPORT" IP_TCP
         }
-        if (httpAttributes.contains(SemanticAttributes.NET_PEER_NAME)) {
-          // net.peer.name resolves to "127.0.0.1" on windows which is same as net.peer.ip so then not captured
-          "$SemanticAttributes.NET_PEER_NAME" { it == null || it == address.host }
-        }
         if (httpAttributes.contains(SemanticAttributes.NET_PEER_PORT)) {
           "$SemanticAttributes.NET_PEER_PORT" { (it instanceof Long && it.intValue() != port) }
         }
@@ -760,10 +755,6 @@ abstract class HttpServerTest<SERVER> extends InstrumentationSpecification imple
         if (httpAttributes.contains(SemanticAttributes.NET_TRANSPORT)) {
           "$SemanticAttributes.NET_TRANSPORT" IP_TCP
         }
-        if (httpAttributes.contains(SemanticAttributes.NET_PEER_NAME)) {
-          // net.peer.name resolves to "127.0.0.1" on windows which is same as net.peer.ip so then not captured
-          "$SemanticAttributes.NET_PEER_NAME" { it == null || it == address.host }
-        }
         if (httpAttributes.contains(SemanticAttributes.NET_PEER_PORT)) {
           "$SemanticAttributes.NET_PEER_PORT" { (it instanceof Long && it.intValue() != port) }
         }

Original file line number	Diff line number	Diff line change
`@@ -19,10 +19,6 @@ public String getRemoteHostAddress() {`
`19`	`19`	`throw new UnsupportedOperationException();`
`20`	`20`	`}`
`21`	`21`
`22`		`- public String getRemoteHostName(boolean canonical) {`
`23`		`- throw new UnsupportedOperationException();`
`24`		`- }`
`25`		`-`
`26`	`22`	`public int getRequestedPort() {`
`27`	`23`	`throw new UnsupportedOperationException();`
`28`	`24`	`}`