Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove server span peer name #5404

Merged
merged 3 commits into from
Feb 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -5,31 +5,33 @@

package io.opentelemetry.instrumentation.spring.web;

import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesGetter;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesGetter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import javax.annotation.Nullable;
import org.springframework.http.HttpRequest;
import org.springframework.http.client.ClientHttpResponse;

final class SpringWebNetAttributesGetter implements NetServerAttributesGetter<HttpRequest> {
final class SpringWebNetAttributesGetter
implements NetClientAttributesGetter<HttpRequest, ClientHttpResponse> {
@Override
public String transport(HttpRequest httpRequest) {
public String transport(HttpRequest httpRequest, @Nullable ClientHttpResponse response) {
return SemanticAttributes.NetTransportValues.IP_TCP;
}

@Override
@Nullable
public String peerName(HttpRequest httpRequest) {
public String peerName(HttpRequest httpRequest, @Nullable ClientHttpResponse response) {
return httpRequest.getURI().getHost();
}

@Override
public Integer peerPort(HttpRequest httpRequest) {
public Integer peerPort(HttpRequest httpRequest, @Nullable ClientHttpResponse response) {
return httpRequest.getURI().getPort();
}

@Override
@Nullable
public String peerIp(HttpRequest httpRequest) {
public String peerIp(HttpRequest httpRequest, @Nullable ClientHttpResponse response) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientMetrics;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.NetClientAttributesExtractor;
import java.util.ArrayList;
import java.util.List;
import org.springframework.http.HttpRequest;
Expand Down Expand Up @@ -71,7 +71,7 @@ public SpringWebTracing build() {
.setSpanStatusExtractor(HttpSpanStatusExtractor.create(httpAttributeGetter))
.addAttributesExtractor(
HttpClientAttributesExtractor.create(httpAttributeGetter, capturedHttpHeaders))
.addAttributesExtractor(NetServerAttributesExtractor.create(netAttributesGetter))
.addAttributesExtractor(NetClientAttributesExtractor.create(netAttributesGetter))
.addAttributesExtractors(additionalExtractors)
.addRequestMetrics(HttpClientMetrics.get())
.newClientInstrumenter(HttpRequestSetter.INSTANCE);
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