Implement Host header parsing in HTTP client attributes extractor (#7288)

Continuation (and refactoring) of #6892
This commit is contained in:
Mateusz Rzeszutek 2022-11-30 20:28:33 +01:00 committed by GitHub
parent e2f960323e
commit 91d9f1d1c9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 195 additions and 44 deletions

View File

@ -82,7 +82,9 @@ public final class HttpClientAttributesExtractor<REQUEST, RESPONSE>
super(httpAttributesGetter, capturedRequestHeaders, capturedResponseHeaders);
internalNetExtractor =
new InternalNetClientAttributesExtractor<>(
netAttributesGetter, this::shouldCapturePeerPort);
netAttributesGetter,
this::shouldCapturePeerPort,
new HttpNetNamePortGetter<>(httpAttributesGetter));
}
@Override

View File

@ -9,12 +9,15 @@ import static io.opentelemetry.instrumentation.api.instrumenter.http.CapturedHtt
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<REQUEST, RESPONSE>>
implements AttributesExtractor<REQUEST, RESPONSE> {
private static final Logger logger = Logger.getLogger(HttpCommonAttributesGetter.class.getName());
final GETTER getter;
private final List<String> capturedRequestHeaders;
private final List<String> capturedResponseHeaders;
@ -114,4 +119,43 @@ abstract class HttpCommonAttributesExtractor<
return null;
}
}
static final class HttpNetNamePortGetter<REQUEST> implements FallbackNamePortGetter<REQUEST> {
private final HttpCommonAttributesGetter<REQUEST, ?> getter;
HttpNetNamePortGetter(HttpCommonAttributesGetter<REQUEST, ?> 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;
}
}
}

View File

@ -79,7 +79,9 @@ public final class HttpServerAttributesExtractor<REQUEST, RESPONSE>
super(httpAttributesGetter, capturedRequestHeaders, capturedResponseHeaders);
internalNetExtractor =
new InternalNetServerAttributesExtractor<>(
netAttributesGetter, this::shouldCaptureHostPort);
netAttributesGetter,
this::shouldCaptureHostPort,
new HttpNetNamePortGetter<>(httpAttributesGetter));
this.httpRouteHolderGetter = httpRouteHolderGetter;
}
@ -95,7 +97,7 @@ public final class HttpServerAttributesExtractor<REQUEST, RESPONSE>
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 final class HttpServerAttributesExtractor<REQUEST, RESPONSE>
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

View File

@ -8,6 +8,7 @@ package io.opentelemetry.instrumentation.api.instrumenter.net;
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 final class NetClientAttributesExtractor<REQUEST, RESPONSE>
}
private NetClientAttributesExtractor(NetClientAttributesGetter<REQUEST, RESPONSE> getter) {
internalExtractor = new InternalNetClientAttributesExtractor<>(getter, (port, request) -> true);
internalExtractor =
new InternalNetClientAttributesExtractor<>(
getter, (port, request) -> true, FallbackNamePortGetter.noop());
}
@Override

View File

@ -8,6 +8,7 @@ package io.opentelemetry.instrumentation.api.instrumenter.net;
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 final class NetServerAttributesExtractor<REQUEST, RESPONSE>
private NetServerAttributesExtractor(NetServerAttributesGetter<REQUEST> 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

View File

@ -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<REQUEST> {
@Nullable
String name(REQUEST request);
@Nullable
Integer port(REQUEST request);
@SuppressWarnings("unchecked")
static <REQUEST> FallbackNamePortGetter<REQUEST> noop() {
return (FallbackNamePortGetter<REQUEST>) NoopNamePortGetter.INSTANCE;
}
}

View File

@ -21,22 +21,24 @@ public final class InternalNetClientAttributesExtractor<REQUEST, RESPONSE> {
private final NetClientAttributesGetter<REQUEST, RESPONSE> getter;
private final BiPredicate<Integer, REQUEST> capturePeerPortCondition;
private final FallbackNamePortGetter<REQUEST> fallbackNamePortGetter;
public InternalNetClientAttributesExtractor(
NetClientAttributesGetter<REQUEST, RESPONSE> getter,
BiPredicate<Integer, REQUEST> capturePeerPortCondition) {
BiPredicate<Integer, REQUEST> capturePeerPortCondition,
FallbackNamePortGetter<REQUEST> 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 final class InternalNetClientAttributesExtractor<REQUEST, RESPONSE> {
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 final class InternalNetClientAttributesExtractor<REQUEST, RESPONSE> {
}
}
}
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;
}
}

View File

@ -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 @@ import javax.annotation.Nullable;
*/
public final class InternalNetServerAttributesExtractor<REQUEST> {
private static final Logger logger =
Logger.getLogger(InternalNetServerAttributesExtractor.class.getName());
private final NetServerAttributesGetter<REQUEST> getter;
private final BiPredicate<Integer, REQUEST> captureHostPortCondition;
private final FallbackNamePortGetter<REQUEST> fallbackNamePortGetter;
public InternalNetServerAttributesExtractor(
NetServerAttributesGetter<REQUEST> getter,
BiPredicate<Integer, REQUEST> captureHostPortCondition) {
BiPredicate<Integer, REQUEST> captureHostPortCondition,
FallbackNamePortGetter<REQUEST> 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 final class InternalNetServerAttributesExtractor<REQUEST> {
}
}
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 final class InternalNetServerAttributesExtractor<REQUEST> {
}
}
}
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;
}
}

View File

@ -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<Object> {
INSTANCE;
@Nullable
@Override
public String name(Object o) {
return null;
}
@Nullable
@Override
public Integer port(Object o) {
return null;
}
}

View File

@ -203,6 +203,44 @@ class HttpClientAttributesExtractorTest {
assertThat(attributes.build()).isEmpty();
}
@Test
void extractNetPeerNameAndPortFromHostHeader() {
Map<String, String> request = new HashMap<>();
request.put("header.host", "thehost:777");
HttpClientAttributesExtractor<Map<String, String>, Map<String, String>> 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<String, String> request = new HashMap<>();
request.put("header.host", "notthehost:77777"); // this should have lower precedence
request.put("peerName", "thehost");
request.put("peerPort", "777");
HttpClientAttributesExtractor<Map<String, String>, Map<String, String>> 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) {