diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java index c35719504360..d7b35bc7d3af 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractor.java @@ -82,7 +82,9 @@ public static HttpClientAttributesExtractorBuilder( - netAttributesGetter, this::shouldCapturePeerPort); + netAttributesGetter, + this::shouldCapturePeerPort, + new HttpNetNamePortGetter<>(httpAttributesGetter)); } @Override diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java index 41197c6a7cfb..576615269a46 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java @@ -9,12 +9,15 @@ import static io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeadersUtil.requestAttributeKey; import static io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHttpHeadersUtil.responseAttributeKey; import static io.opentelemetry.instrumentation.api.internal.AttributesExtractorUtil.internalSet; +import static java.util.logging.Level.FINE; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.net.internal.FallbackNamePortGetter; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.util.List; +import java.util.logging.Logger; import javax.annotation.Nullable; /** @@ -26,6 +29,8 @@ abstract class HttpCommonAttributesExtractor< REQUEST, RESPONSE, GETTER extends HttpCommonAttributesGetter> implements AttributesExtractor { + private static final Logger logger = Logger.getLogger(HttpCommonAttributesGetter.class.getName()); + final GETTER getter; private final List capturedRequestHeaders; private final List capturedResponseHeaders; @@ -114,4 +119,43 @@ private static Long parseNumber(@Nullable String number) { return null; } } + + static final class HttpNetNamePortGetter implements FallbackNamePortGetter { + + private final HttpCommonAttributesGetter getter; + + HttpNetNamePortGetter(HttpCommonAttributesGetter getter) { + this.getter = getter; + } + + @Nullable + @Override + public String name(REQUEST request) { + String host = firstHeaderValue(getter.requestHeader(request, "host")); + if (host == null) { + return null; + } + int hostHeaderSeparator = host.indexOf(':'); + return hostHeaderSeparator == -1 ? host : host.substring(0, hostHeaderSeparator); + } + + @Nullable + @Override + public Integer port(REQUEST request) { + String host = firstHeaderValue(getter.requestHeader(request, "host")); + if (host == null) { + return null; + } + int hostHeaderSeparator = host.indexOf(':'); + if (hostHeaderSeparator == -1) { + return null; + } + try { + return Integer.parseInt(host.substring(hostHeaderSeparator + 1)); + } catch (NumberFormatException e) { + logger.log(FINE, e.getMessage(), e); + } + return null; + } + } } diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java index ef3db8c73c54..fe6d8df02f75 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java @@ -79,7 +79,9 @@ public static HttpServerAttributesExtractorBuilder( - netAttributesGetter, this::shouldCaptureHostPort); + netAttributesGetter, + this::shouldCaptureHostPort, + new HttpNetNamePortGetter<>(httpAttributesGetter)); this.httpRouteHolderGetter = httpRouteHolderGetter; } @@ -95,7 +97,7 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST internalSet(attributes, SemanticAttributes.HTTP_ROUTE, getter.route(request)); internalSet(attributes, SemanticAttributes.HTTP_CLIENT_IP, clientIp(request)); - internalNetExtractor.onStart(attributes, request, host(request)); + internalNetExtractor.onStart(attributes, request); } private boolean shouldCaptureHostPort(int port, REQUEST request) { @@ -122,11 +124,6 @@ public void onEnd( internalSet(attributes, SemanticAttributes.HTTP_ROUTE, httpRouteHolderGetter.apply(context)); } - @Nullable - private String host(REQUEST request) { - return firstHeaderValue(getter.requestHeader(request, "host")); - } - @Nullable private String forwardedProto(REQUEST request) { // try Forwarded diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java index 7c5255bd7d77..0d1391af7d9f 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetClientAttributesExtractor.java @@ -8,6 +8,7 @@ import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.net.internal.FallbackNamePortGetter; import io.opentelemetry.instrumentation.api.instrumenter.net.internal.InternalNetClientAttributesExtractor; import javax.annotation.Nullable; @@ -31,7 +32,9 @@ public static NetClientAttributesExtractor getter) { - internalExtractor = new InternalNetClientAttributesExtractor<>(getter, (port, request) -> true); + internalExtractor = + new InternalNetClientAttributesExtractor<>( + getter, (port, request) -> true, FallbackNamePortGetter.noop()); } @Override diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java index 25095371998a..16bfd1b90bd0 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/NetServerAttributesExtractor.java @@ -8,6 +8,7 @@ import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.net.internal.FallbackNamePortGetter; import io.opentelemetry.instrumentation.api.instrumenter.net.internal.InternalNetServerAttributesExtractor; import javax.annotation.Nullable; @@ -29,12 +30,13 @@ public static NetServerAttributesExtractor getter) { internalExtractor = - new InternalNetServerAttributesExtractor<>(getter, (integer, request) -> true); + new InternalNetServerAttributesExtractor<>( + getter, (integer, request) -> true, FallbackNamePortGetter.noop()); } @Override public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) { - internalExtractor.onStart(attributes, request, null); + internalExtractor.onStart(attributes, request); } @Override diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/FallbackNamePortGetter.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/FallbackNamePortGetter.java new file mode 100644 index 000000000000..93fc1d468eab --- /dev/null +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/FallbackNamePortGetter.java @@ -0,0 +1,26 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter.net.internal; + +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 interface FallbackNamePortGetter { + + @Nullable + String name(REQUEST request); + + @Nullable + Integer port(REQUEST request); + + @SuppressWarnings("unchecked") + static FallbackNamePortGetter noop() { + return (FallbackNamePortGetter) NoopNamePortGetter.INSTANCE; + } +} diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/InternalNetClientAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/InternalNetClientAttributesExtractor.java index 55d7f7c44a6e..31c14f11f0d5 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/InternalNetClientAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/InternalNetClientAttributesExtractor.java @@ -21,22 +21,24 @@ public final class InternalNetClientAttributesExtractor { private final NetClientAttributesGetter getter; private final BiPredicate capturePeerPortCondition; + private final FallbackNamePortGetter fallbackNamePortGetter; public InternalNetClientAttributesExtractor( NetClientAttributesGetter getter, - BiPredicate capturePeerPortCondition) { + BiPredicate capturePeerPortCondition, + FallbackNamePortGetter fallbackNamePortGetter) { this.getter = getter; this.capturePeerPortCondition = capturePeerPortCondition; + this.fallbackNamePortGetter = fallbackNamePortGetter; } public void onStart(AttributesBuilder attributes, REQUEST request) { - String peerName = getter.peerName(request); - Integer peerPort = getter.peerPort(request); - - // TODO: add host header parsing + String peerName = extractPeerName(request); if (peerName != null) { internalSet(attributes, SemanticAttributes.NET_PEER_NAME, peerName); + + Integer peerPort = extractPeerPort(request); if (peerPort != null && peerPort > 0 && capturePeerPortCondition.test(peerPort, request)) { internalSet(attributes, SemanticAttributes.NET_PEER_PORT, (long) peerPort); } @@ -47,13 +49,13 @@ public void onEnd(AttributesBuilder attributes, REQUEST request, @Nullable RESPO internalSet(attributes, SemanticAttributes.NET_TRANSPORT, getter.transport(request, response)); - String peerName = getter.peerName(request); - Integer peerPort = getter.peerPort(request); + String peerName = extractPeerName(request); String sockPeerAddr = getter.sockPeerAddr(request, response); if (sockPeerAddr != null && !sockPeerAddr.equals(peerName)) { internalSet(attributes, SemanticAttributes.NET_SOCK_PEER_ADDR, sockPeerAddr); + Integer peerPort = extractPeerPort(request); Integer sockPeerPort = getter.sockPeerPort(request, response); if (sockPeerPort != null && sockPeerPort > 0 && !sockPeerPort.equals(peerPort)) { internalSet(attributes, SemanticAttributes.NET_SOCK_PEER_PORT, (long) sockPeerPort); @@ -70,4 +72,20 @@ public void onEnd(AttributesBuilder attributes, REQUEST request, @Nullable RESPO } } } + + private String extractPeerName(REQUEST request) { + String peerName = getter.peerName(request); + if (peerName == null) { + peerName = fallbackNamePortGetter.name(request); + } + return peerName; + } + + private Integer extractPeerPort(REQUEST request) { + Integer peerPort = getter.peerPort(request); + if (peerPort == null) { + peerPort = fallbackNamePortGetter.port(request); + } + return peerPort; + } } diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/InternalNetServerAttributesExtractor.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/InternalNetServerAttributesExtractor.java index ae68af195dba..3b6255c931bb 100644 --- a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/InternalNetServerAttributesExtractor.java +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/InternalNetServerAttributesExtractor.java @@ -6,14 +6,11 @@ package io.opentelemetry.instrumentation.api.instrumenter.net.internal; import static io.opentelemetry.instrumentation.api.internal.AttributesExtractorUtil.internalSet; -import static java.util.logging.Level.FINE; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.instrumentation.api.instrumenter.net.NetServerAttributesGetter; import io.opentelemetry.semconv.trace.attributes.SemanticAttributes; import java.util.function.BiPredicate; -import java.util.logging.Logger; -import javax.annotation.Nullable; /** * This class is internal and is hence not for public use. Its APIs are unstable and can change at @@ -21,20 +18,20 @@ */ public final class InternalNetServerAttributesExtractor { - private static final Logger logger = - Logger.getLogger(InternalNetServerAttributesExtractor.class.getName()); - private final NetServerAttributesGetter getter; private final BiPredicate captureHostPortCondition; + private final FallbackNamePortGetter fallbackNamePortGetter; public InternalNetServerAttributesExtractor( NetServerAttributesGetter getter, - BiPredicate captureHostPortCondition) { + BiPredicate captureHostPortCondition, + FallbackNamePortGetter fallbackNamePortGetter) { this.getter = getter; this.captureHostPortCondition = captureHostPortCondition; + this.fallbackNamePortGetter = fallbackNamePortGetter; } - public void onStart(AttributesBuilder attributes, REQUEST request, @Nullable String hostHeader) { + public void onStart(AttributesBuilder attributes, REQUEST request) { internalSet(attributes, SemanticAttributes.NET_TRANSPORT, getter.transport(request)); boolean setSockFamily = false; @@ -51,24 +48,8 @@ public void onStart(AttributesBuilder attributes, REQUEST request, @Nullable Str } } - String hostName = getter.hostName(request); - Integer hostPort = getter.hostPort(request); - - int hostHeaderSeparator = -1; - if (hostHeader != null) { - hostHeaderSeparator = hostHeader.indexOf(':'); - } - if (hostName == null && hostHeader != null) { - hostName = - hostHeaderSeparator == -1 ? hostHeader : hostHeader.substring(0, hostHeaderSeparator); - } - if (hostPort == null && hostHeader != null && hostHeaderSeparator != -1) { - try { - hostPort = Integer.parseInt(hostHeader.substring(hostHeaderSeparator + 1)); - } catch (NumberFormatException e) { - logger.log(FINE, e.getMessage(), e); - } - } + String hostName = extractHostName(request); + Integer hostPort = extractHostPort(request); if (hostName != null) { internalSet(attributes, SemanticAttributes.NET_HOST_NAME, hostName); @@ -97,4 +78,20 @@ public void onStart(AttributesBuilder attributes, REQUEST request, @Nullable Str } } } + + private String extractHostName(REQUEST request) { + String peerName = getter.hostName(request); + if (peerName == null) { + peerName = fallbackNamePortGetter.name(request); + } + return peerName; + } + + private Integer extractHostPort(REQUEST request) { + Integer peerPort = getter.hostPort(request); + if (peerPort == null) { + peerPort = fallbackNamePortGetter.port(request); + } + return peerPort; + } } diff --git a/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/NoopNamePortGetter.java b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/NoopNamePortGetter.java new file mode 100644 index 000000000000..27b97b57a54f --- /dev/null +++ b/instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/net/internal/NoopNamePortGetter.java @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.instrumenter.net.internal; + +import javax.annotation.Nullable; + +enum NoopNamePortGetter implements FallbackNamePortGetter { + INSTANCE; + + @Nullable + @Override + public String name(Object o) { + return null; + } + + @Nullable + @Override + public Integer port(Object o) { + return null; + } +} diff --git a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java index fba7ba9336f0..70aca1e1baa1 100644 --- a/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java +++ b/instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpClientAttributesExtractorTest.java @@ -203,6 +203,44 @@ void invalidStatusCode() { assertThat(attributes.build()).isEmpty(); } + @Test + void extractNetPeerNameAndPortFromHostHeader() { + Map request = new HashMap<>(); + request.put("header.host", "thehost:777"); + + HttpClientAttributesExtractor, Map> extractor = + HttpClientAttributesExtractor.create( + new TestHttpClientAttributesGetter(), new TestNetClientAttributesGetter()); + + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, Context.root(), request); + + assertThat(attributes.build()) + .containsOnly( + entry(SemanticAttributes.NET_PEER_NAME, "thehost"), + entry(SemanticAttributes.NET_PEER_PORT, 777L)); + } + + @Test + void extractNetHostAndPortFromNetAttributesGetter() { + Map request = new HashMap<>(); + request.put("header.host", "notthehost:77777"); // this should have lower precedence + request.put("peerName", "thehost"); + request.put("peerPort", "777"); + + HttpClientAttributesExtractor, Map> extractor = + HttpClientAttributesExtractor.create( + new TestHttpClientAttributesGetter(), new TestNetClientAttributesGetter()); + + AttributesBuilder attributes = Attributes.builder(); + extractor.onStart(attributes, Context.root(), request); + + assertThat(attributes.build()) + .containsOnly( + entry(SemanticAttributes.NET_PEER_NAME, "thehost"), + entry(SemanticAttributes.NET_PEER_PORT, 777L)); + } + @ParameterizedTest @ArgumentsSource(DefaultPeerPortArgumentSource.class) void defaultPeerPort(int peerPort, String url) {