Skip to content

Commit

Permalink
Remove server span peer name (open-telemetry#5404)
Browse files Browse the repository at this point in the history
* Fix server -> client reference

* Remove server span peer name
  • Loading branch information
trask authored and RashmiRam committed May 23, 2022
1 parent 64ad76f commit 352da1a
Show file tree
Hide file tree
Showing 21 changed files with 1 addition and 120 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -20,9 +20,6 @@ public interface NetServerAttributesGetter<REQUEST> {
@Nullable
String transport(REQUEST request);

@Nullable
String peerName(REQUEST request);

@Nullable
Integer peerPort(REQUEST request);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -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"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ public String getRemoteHostAddress() {
throw new UnsupportedOperationException();
}

public String getRemoteHostName(boolean canonical) {
throw new UnsupportedOperationException();
}

public int getRequestedPort() {
throw new UnsupportedOperationException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ public String peerIp() {
return httpDispatcherLink.getRemoteHostAddress();
}

public String peerName() {
return httpDispatcherLink.getRemoteHostName(false);
}

public String getProtocol() {
return httpRequestMessage.getVersion();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,6 @@ public interface ServletAccessor<REQUEST, RESPONSE> {

Integer getRequestRemotePort(REQUEST request);

String getRequestRemoteHost(REQUEST request);

int getRequestContentLength(REQUEST request);

void addRequestAsyncListener(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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) }
}
Expand Down Expand Up @@ -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) }
}
Expand Down

0 comments on commit 352da1a

Please sign in to comment.