Move clientIp extraction to HttpServerAttributesExtractor
MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/ServerInstrumenter.java
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/ServerInstrumenter.java
package io.opentelemetry.instrumentation.api.instrumenter;
-import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.ContextPropagators;
import io.opentelemetry.context.propagation.TextMapGetter;
-import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesExtractor;
import io.opentelemetry.instrumentation.api.internal.ContextPropagationDebug;
-import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
-import org.checkerframework.checker.nullness.qual.Nullable;
final class ServerInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST, RESPONSE> {
@@ -21,7 +17,7 @@ final class ServerInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST,
ServerInstrumenter(
InstrumenterBuilder<REQUEST, RESPONSE> builder, TextMapGetter<REQUEST> getter) {
- super(addClientIpExtractor(builder, getter));
+ super(builder);
this.propagators = builder.openTelemetry.getPropagators();
this.getter = getter;
}
@@ -33,125 +29,4 @@ final class ServerInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST,
Context extracted = propagators.getTextMapPropagator().extract(parentContext, request, getter);
return super.start(extracted, request);
}
-
- // TODO: move that to HttpServerAttributesExtractor, now that we have a method for extracting
- // header values there
- private static <REQUEST, RESPONSE> InstrumenterBuilder<REQUEST, RESPONSE> addClientIpExtractor(
- InstrumenterBuilder<REQUEST, RESPONSE> builder, TextMapGetter<REQUEST> getter) {
- HttpServerAttributesExtractor<REQUEST, RESPONSE> httpAttributesExtractor = null;
- for (AttributesExtractor<? super REQUEST, ? super RESPONSE> extractor :
- builder.attributesExtractors) {
- if (extractor instanceof HttpServerAttributesExtractor) {
- httpAttributesExtractor = (HttpServerAttributesExtractor<REQUEST, RESPONSE>) extractor;
- }
- }
- if (httpAttributesExtractor == null) {
- // Don't add HTTP_CLIENT_IP if there are no HTTP attributes registered.
- return builder;
- }
- builder.addAttributesExtractor(new HttpClientIpExtractor<>(getter));
- return builder;
- }
-
- private static class HttpClientIpExtractor<REQUEST, RESPONSE>
- extends AttributesExtractor<REQUEST, RESPONSE> {
-
- private final TextMapGetter<REQUEST> getter;
-
- HttpClientIpExtractor(TextMapGetter<REQUEST> getter) {
- this.getter = getter;
- }
-
- @Override
- protected void onStart(AttributesBuilder attributes, REQUEST request) {}
-
- @Override
- protected void onEnd(
- AttributesBuilder attributes,
- REQUEST request,
- @Nullable RESPONSE response,
- @Nullable Throwable error) {
- String clientIp = getForwardedClientIp(request);
- set(attributes, SemanticAttributes.HTTP_CLIENT_IP, clientIp);
- }
-
- @Nullable
- // Visible for testing
- String getForwardedClientIp(REQUEST request) {
- // try Forwarded
- String forwarded = getter.get(request, "Forwarded");
- if (forwarded != null) {
- forwarded = extractForwarded(forwarded);
- if (forwarded != null) {
- return forwarded;
- }
- }
-
- // try X-Forwarded-For
- forwarded = getter.get(request, "X-Forwarded-For");
- if (forwarded != null) {
- return extractForwardedFor(forwarded);
- }
-
- return null;
- }
- }
-
- // VisibleForTesting
- @Nullable
- static String extractForwarded(String forwarded) {
- int start = forwarded.toLowerCase().indexOf("for=");
- if (start < 0) {
- return null;
- }
- start += 4; // start is now the index after for=
- if (start >= forwarded.length() - 1) { // the value after for= must not be empty
- return null;
- }
- return extractIpAddress(forwarded, start);
- }
-
- // VisibleForTesting
- @Nullable
- static String extractForwardedFor(String forwarded) {
- return extractIpAddress(forwarded, 0);
- }
-
- // from https://www.rfc-editor.org/rfc/rfc7239
- // "Note that IPv6 addresses may not be quoted in
- // X-Forwarded-For and may not be enclosed by square brackets, but they
- // are quoted and enclosed in square brackets in Forwarded"
- // and also (applying to Forwarded but not X-Forwarded-For)
- // "It is important to note that an IPv6 address and any nodename with
- // node-port specified MUST be quoted, since ':' is not an allowed
- // character in 'token'."
- @Nullable
- private static String extractIpAddress(String forwarded, int start) {
- if (forwarded.length() == start) {
- return null;
- }
- if (forwarded.charAt(start) == '"') {
- return extractIpAddress(forwarded, start + 1);
- }
- if (forwarded.charAt(start) == '[') {
- int end = forwarded.indexOf(']', start + 1);
- if (end == -1) {
- return null;
- }
- return forwarded.substring(start + 1, end);
- }
- boolean inIpv4 = false;
- for (int i = start; i < forwarded.length() - 1; i++) {
- char c = forwarded.charAt(i);
- if (c == '.') {
- inIpv4 = true;
- } else if (c == ',' || c == ';' || c == '"' || (inIpv4 && c == ':')) {
- if (i == start) { // empty string
- return null;
- }
- return forwarded.substring(start, i);
- }
- }
- return forwarded.substring(start);
- }
}

ADD
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwarderHeaderParser.java
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.http;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+final class ForwarderHeaderParser {
+
+ // VisibleForTesting
+ @Nullable
+ static String extractForwarded(String forwarded) {
+ int start = forwarded.toLowerCase().indexOf("for=");
+ if (start < 0) {
+ return null;
+ }
+ start += 4; // start is now the index after for=
+ if (start >= forwarded.length() - 1) { // the value after for= must not be empty
+ return null;
+ }
+ return extractIpAddress(forwarded, start);
+ }
+
+ // VisibleForTesting
+ @Nullable
+ static String extractForwardedFor(String forwarded) {
+ return extractIpAddress(forwarded, 0);
+ }
+
+ // from https://www.rfc-editor.org/rfc/rfc7239
+ // "Note that IPv6 addresses may not be quoted in
+ // X-Forwarded-For and may not be enclosed by square brackets, but they
+ // are quoted and enclosed in square brackets in Forwarded"
+ // and also (applying to Forwarded but not X-Forwarded-For)
+ // "It is important to note that an IPv6 address and any nodename with
+ // node-port specified MUST be quoted, since ':' is not an allowed
+ // character in 'token'."
+ @Nullable
+ private static String extractIpAddress(String forwarded, int start) {
+ if (forwarded.length() == start) {
+ return null;
+ }
+ if (forwarded.charAt(start) == '"') {
+ return extractIpAddress(forwarded, start + 1);
+ }
+ if (forwarded.charAt(start) == '[') {
+ int end = forwarded.indexOf(']', start + 1);
+ if (end == -1) {
+ return null;
+ }
+ return forwarded.substring(start + 1, end);
+ }
+ boolean inIpv4 = false;
+ for (int i = start; i < forwarded.length() - 1; i++) {
+ char c = forwarded.charAt(i);
+ if (c == '.') {
+ inIpv4 = true;
+ } else if (c == ',' || c == ';' || c == '"' || (inIpv4 && c == ':')) {
+ if (i == start) { // empty string
+ return null;
+ }
+ return forwarded.substring(start, i);
+ }
+ }
+ return forwarded.substring(start);
+ }
+
+ private ForwarderHeaderParser() {}
+}

MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpCommonAttributesExtractor.java
@@ -89,8 +89,7 @@ public abstract class HttpCommonAttributesExtractor<REQUEST, RESPONSE>
@Nullable
private String userAgent(REQUEST request) {
- List<String> values = requestHeader(request, "user-agent");
- return values.isEmpty() ? null : values.get(0);
+ return firstHeaderValue(requestHeader(request, "user-agent"));
}
/**
@@ -161,4 +160,9 @@ public abstract class HttpCommonAttributesExtractor<REQUEST, RESPONSE>
* returned instead.
*/
protected abstract List<String> responseHeader(REQUEST request, RESPONSE response, String name);
+
+ @Nullable
+ static String firstHeaderValue(List<String> values) {
+ return values.isEmpty() ? null : values.get(0);
+ }
}

MODIFY
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractor.java
package io.opentelemetry.instrumentation.api.instrumenter.http;
+import static io.opentelemetry.instrumentation.api.instrumenter.http.ForwarderHeaderParser.extractForwarded;
+import static io.opentelemetry.instrumentation.api.instrumenter.http.ForwarderHeaderParser.extractForwardedFor;
+
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
-import java.util.List;
import org.checkerframework.checker.nullness.qual.Nullable;
/**
@@ -43,6 +45,7 @@ public abstract class HttpServerAttributesExtractor<REQUEST, RESPONSE>
set(attributes, SemanticAttributes.HTTP_HOST, host(request));
set(attributes, SemanticAttributes.HTTP_TARGET, target(request));
set(attributes, SemanticAttributes.HTTP_ROUTE, route(request));
+ set(attributes, SemanticAttributes.HTTP_CLIENT_IP, clientIp(request));
}
@Override
@@ -66,8 +69,7 @@ public abstract class HttpServerAttributesExtractor<REQUEST, RESPONSE>
@Nullable
private String host(REQUEST request) {
- List<String> values = requestHeader(request, "host");
- return values.isEmpty() ? null : values.get(0);
+ return firstHeaderValue(requestHeader(request, "host"));
}
@Nullable
@@ -76,6 +78,26 @@ public abstract class HttpServerAttributesExtractor<REQUEST, RESPONSE>
@Nullable
protected abstract String scheme(REQUEST request);
+ @Nullable
+ private String clientIp(REQUEST request) {
+ // try Forwarded
+ String forwarded = firstHeaderValue(requestHeader(request, "forwarded"));
+ if (forwarded != null) {
+ forwarded = extractForwarded(forwarded);
+ if (forwarded != null) {
+ return forwarded;
+ }
+ }
+
+ // try X-Forwarded-For
+ forwarded = firstHeaderValue(requestHeader(request, "x-forwarded-for"));
+ if (forwarded != null) {
+ return extractForwardedFor(forwarded);
+ }
+
+ return null;
+ }
+
// Attributes which are not always available when the request is ready.
/**

MODIFY
instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java
instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterTest.java
@@ -60,8 +60,7 @@ class InstrumenterTest {
entry("req2_2", "req2_2_value"),
entry("req3", "req3_value"),
entry("linkTraceId", LINK_TRACE_ID),
- entry("linkSpanId", LINK_SPAN_ID),
- entry("Forwarded", "for=1.1.1.1"))
+ entry("linkSpanId", LINK_SPAN_ID))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)));
private static final Map<String, String> RESPONSE =
@@ -297,49 +296,8 @@ class InstrumenterTest {
.hasAttributesSatisfying(
attributes ->
assertThat(attributes)
- .containsEntry(SemanticAttributes.NET_PEER_IP, "2.2.2.2")
.containsEntry(
- SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1"))));
- }
-
- @Test
- void server_http_xForwardedFor() {
- Instrumenter<Map<String, String>, Map<String, String>> instrumenter =
- Instrumenter.<Map<String, String>, Map<String, String>>newBuilder(
- otelTesting.getOpenTelemetry(), "test", unused -> "span")
- .addAttributesExtractors(
- mockHttpServerAttributes,
- new ConstantNetPeerIpExtractor<>("2.2.2.2"),
- new AttributesExtractor1(),
- new AttributesExtractor2())
- .addSpanLinksExtractor(new LinksExtractor())
- .newServerInstrumenter(new MapGetter());
-
- Map<String, String> request = new HashMap<>(REQUEST);
- request.remove("Forwarded");
- request.put("X-Forwarded-For", "1.1.1.1");
-
- Context context = instrumenter.start(Context.root(), request);
- SpanContext spanContext = Span.fromContext(context).getSpanContext();
-
- assertThat(spanContext.isValid()).isTrue();
- assertThat(SpanKey.SERVER.fromContextOrNull(context).getSpanContext()).isEqualTo(spanContext);
-
- instrumenter.end(context, request, RESPONSE, null);
-
- otelTesting
- .assertTraces()
- .hasTracesSatisfyingExactly(
- trace ->
- trace.hasSpansSatisfyingExactly(
- span ->
- span.hasName("span")
- .hasAttributesSatisfying(
- attributes ->
- assertThat(attributes)
- .containsEntry(SemanticAttributes.NET_PEER_IP, "2.2.2.2")
- .containsEntry(
- SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1"))));
+ SemanticAttributes.NET_PEER_IP, "2.2.2.2"))));
}
@Test
@@ -517,220 +475,6 @@ class InstrumenterTest {
assertThat(context.get(testKey)).isEqualTo("testVal");
}
- @Test
- void extractForwarded() {
- assertThat(ServerInstrumenter.extractForwarded("for=1.1.1.1")).isEqualTo("1.1.1.1");
- }
-
- @Test
- void extractForwardedIpv6() {
- assertThat(
- ServerInstrumenter.extractForwarded(
- "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]\""))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedWithPort() {
- assertThat(ServerInstrumenter.extractForwarded("for=\"1.1.1.1:2222\"")).isEqualTo("1.1.1.1");
- }
-
- @Test
- void extractForwardedIpv6WithPort() {
- assertThat(
- ServerInstrumenter.extractForwarded(
- "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\""))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedCaps() {
- assertThat(ServerInstrumenter.extractForwarded("For=1.1.1.1")).isEqualTo("1.1.1.1");
- }
-
- @Test
- void extractForwardedMalformed() {
- assertThat(ServerInstrumenter.extractForwarded("for=;for=1.1.1.1")).isNull();
- }
-
- @Test
- void extractForwardedEmpty() {
- assertThat(ServerInstrumenter.extractForwarded("")).isNull();
- }
-
- @Test
- void extractForwardedEmptyValue() {
- assertThat(ServerInstrumenter.extractForwarded("for=")).isNull();
- }
-
- @Test
- void extractForwardedEmptyValueWithSemicolon() {
- assertThat(ServerInstrumenter.extractForwarded("for=;")).isNull();
- }
-
- @Test
- void extractForwardedNoFor() {
- assertThat(ServerInstrumenter.extractForwarded("by=1.1.1.1;test=1.1.1.1")).isNull();
- }
-
- @Test
- void extractForwardedMultiple() {
- assertThat(ServerInstrumenter.extractForwarded("for=1.1.1.1;for=1.2.3.4")).isEqualTo("1.1.1.1");
- }
-
- @Test
- void extractForwardedMultipleIpV6() {
- assertThat(
- ServerInstrumenter.extractForwarded(
- "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]\";for=1.2.3.4"))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedMultipleWithPort() {
- assertThat(ServerInstrumenter.extractForwarded("for=\"1.1.1.1:2222\";for=1.2.3.4"))
- .isEqualTo("1.1.1.1");
- }
-
- @Test
- void extractForwardedMultipleIpV6WithPort() {
- assertThat(
- ServerInstrumenter.extractForwarded(
- "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\";for=1.2.3.4"))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedMixedSplitter() {
- assertThat(
- ServerInstrumenter.extractForwarded("test=abcd; by=1.2.3.4, for=1.1.1.1;for=1.2.3.4"))
- .isEqualTo("1.1.1.1");
- }
-
- @Test
- void extractForwardedMixedSplitterIpv6() {
- assertThat(
- ServerInstrumenter.extractForwarded(
- "test=abcd; by=1.2.3.4, for=\"[1111:1111:1111:1111:1111:1111:1111:1111]\";for=1.2.3.4"))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedMixedSplitterWithPort() {
- assertThat(
- ServerInstrumenter.extractForwarded(
- "test=abcd; by=1.2.3.4, for=\"1.1.1.1:2222\";for=1.2.3.4"))
- .isEqualTo("1.1.1.1");
- }
-
- @Test
- void extractForwardedMixedSplitterIpv6WithPort() {
- assertThat(
- ServerInstrumenter.extractForwarded(
- "test=abcd; by=1.2.3.4, for=\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\";for=1.2.3.4"))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedFor() {
- assertThat(ServerInstrumenter.extractForwardedFor("1.1.1.1")).isEqualTo("1.1.1.1");
- }
-
- @Test
- void extractForwardedForIpv6() {
- assertThat(
- ServerInstrumenter.extractForwardedFor("\"[1111:1111:1111:1111:1111:1111:1111:1111]\""))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedForIpv6Unquoted() {
- assertThat(ServerInstrumenter.extractForwardedFor("[1111:1111:1111:1111:1111:1111:1111:1111]"))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedForIpv6Unbracketed() {
- assertThat(ServerInstrumenter.extractForwardedFor("1111:1111:1111:1111:1111:1111:1111:1111"))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedForWithPort() {
- assertThat(ServerInstrumenter.extractForwardedFor("1.1.1.1:2222")).isEqualTo("1.1.1.1");
- }
-
- @Test
- void extractForwardedForIpv6WithPort() {
- assertThat(
- ServerInstrumenter.extractForwardedFor(
- "\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\""))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedForIpv6UnquotedWithPort() {
- assertThat(
- ServerInstrumenter.extractForwardedFor(
- "[1111:1111:1111:1111:1111:1111:1111:1111]:2222"))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedForEmpty() {
- assertThat(ServerInstrumenter.extractForwardedFor("")).isNull();
- }
-
- @Test
- void extractForwardedForMultiple() {
- assertThat(ServerInstrumenter.extractForwardedFor("1.1.1.1,1.2.3.4")).isEqualTo("1.1.1.1");
- }
-
- @Test
- void extractForwardedForMultipleIpv6() {
- assertThat(
- ServerInstrumenter.extractForwardedFor(
- "\"[1111:1111:1111:1111:1111:1111:1111:1111]\",1.2.3.4"))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedForMultipleIpv6Unquoted() {
- assertThat(
- ServerInstrumenter.extractForwardedFor(
- "[1111:1111:1111:1111:1111:1111:1111:1111],1.2.3.4"))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedForMultipleIpv6Unbracketed() {
- assertThat(
- ServerInstrumenter.extractForwardedFor(
- "1111:1111:1111:1111:1111:1111:1111:1111,1.2.3.4"))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedForMultipleWithPort() {
- assertThat(ServerInstrumenter.extractForwardedFor("1.1.1.1:2222,1.2.3.4")).isEqualTo("1.1.1.1");
- }
-
- @Test
- void extractForwardedForMultipleIpv6WithPort() {
- assertThat(
- ServerInstrumenter.extractForwardedFor(
- "\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\",1.2.3.4"))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
- @Test
- void extractForwardedForMultipleIpv6UnquotedWithPort() {
- assertThat(
- ServerInstrumenter.extractForwardedFor(
- "[1111:1111:1111:1111:1111:1111:1111:1111]:2222,1.2.3.4"))
- .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
- }
-
@Test
void clientNestedSpansSuppressed_whenInstrumentationTypeDisabled() {
// this test depends on default config option for InstrumentationType

ADD
instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwarderHeaderParserTest.java
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.http;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.jupiter.api.Test;
+
+class ForwarderHeaderParserTest {
+
+ @Test
+ void extractForwarded() {
+ assertThat(ForwarderHeaderParser.extractForwarded("for=1.1.1.1")).isEqualTo("1.1.1.1");
+ }
+
+ @Test
+ void extractForwardedIpv6() {
+ assertThat(
+ ForwarderHeaderParser.extractForwarded(
+ "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]\""))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedWithPort() {
+ assertThat(ForwarderHeaderParser.extractForwarded("for=\"1.1.1.1:2222\"")).isEqualTo("1.1.1.1");
+ }
+
+ @Test
+ void extractForwardedIpv6WithPort() {
+ assertThat(
+ ForwarderHeaderParser.extractForwarded(
+ "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\""))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedCaps() {
+ assertThat(ForwarderHeaderParser.extractForwarded("For=1.1.1.1")).isEqualTo("1.1.1.1");
+ }
+
+ @Test
+ void extractForwardedMalformed() {
+ assertThat(ForwarderHeaderParser.extractForwarded("for=;for=1.1.1.1")).isNull();
+ }
+
+ @Test
+ void extractForwardedEmpty() {
+ assertThat(ForwarderHeaderParser.extractForwarded("")).isNull();
+ }
+
+ @Test
+ void extractForwardedEmptyValue() {
+ assertThat(ForwarderHeaderParser.extractForwarded("for=")).isNull();
+ }
+
+ @Test
+ void extractForwardedEmptyValueWithSemicolon() {
+ assertThat(ForwarderHeaderParser.extractForwarded("for=;")).isNull();
+ }
+
+ @Test
+ void extractForwardedNoFor() {
+ assertThat(ForwarderHeaderParser.extractForwarded("by=1.1.1.1;test=1.1.1.1")).isNull();
+ }
+
+ @Test
+ void extractForwardedMultiple() {
+ assertThat(ForwarderHeaderParser.extractForwarded("for=1.1.1.1;for=1.2.3.4"))
+ .isEqualTo("1.1.1.1");
+ }
+
+ @Test
+ void extractForwardedMultipleIpV6() {
+ assertThat(
+ ForwarderHeaderParser.extractForwarded(
+ "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]\";for=1.2.3.4"))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedMultipleWithPort() {
+ assertThat(ForwarderHeaderParser.extractForwarded("for=\"1.1.1.1:2222\";for=1.2.3.4"))
+ .isEqualTo("1.1.1.1");
+ }
+
+ @Test
+ void extractForwardedMultipleIpV6WithPort() {
+ assertThat(
+ ForwarderHeaderParser.extractForwarded(
+ "for=\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\";for=1.2.3.4"))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedMixedSplitter() {
+ assertThat(
+ ForwarderHeaderParser.extractForwarded(
+ "test=abcd; by=1.2.3.4, for=1.1.1.1;for=1.2.3.4"))
+ .isEqualTo("1.1.1.1");
+ }
+
+ @Test
+ void extractForwardedMixedSplitterIpv6() {
+ assertThat(
+ ForwarderHeaderParser.extractForwarded(
+ "test=abcd; by=1.2.3.4, for=\"[1111:1111:1111:1111:1111:1111:1111:1111]\";for=1.2.3.4"))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedMixedSplitterWithPort() {
+ assertThat(
+ ForwarderHeaderParser.extractForwarded(
+ "test=abcd; by=1.2.3.4, for=\"1.1.1.1:2222\";for=1.2.3.4"))
+ .isEqualTo("1.1.1.1");
+ }
+
+ @Test
+ void extractForwardedMixedSplitterIpv6WithPort() {
+ assertThat(
+ ForwarderHeaderParser.extractForwarded(
+ "test=abcd; by=1.2.3.4, for=\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\";for=1.2.3.4"))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedFor() {
+ assertThat(ForwarderHeaderParser.extractForwardedFor("1.1.1.1")).isEqualTo("1.1.1.1");
+ }
+
+ @Test
+ void extractForwardedForIpv6() {
+ assertThat(
+ ForwarderHeaderParser.extractForwardedFor(
+ "\"[1111:1111:1111:1111:1111:1111:1111:1111]\""))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedForIpv6Unquoted() {
+ assertThat(
+ ForwarderHeaderParser.extractForwardedFor("[1111:1111:1111:1111:1111:1111:1111:1111]"))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedForIpv6Unbracketed() {
+ assertThat(ForwarderHeaderParser.extractForwardedFor("1111:1111:1111:1111:1111:1111:1111:1111"))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedForWithPort() {
+ assertThat(ForwarderHeaderParser.extractForwardedFor("1.1.1.1:2222")).isEqualTo("1.1.1.1");
+ }
+
+ @Test
+ void extractForwardedForIpv6WithPort() {
+ assertThat(
+ ForwarderHeaderParser.extractForwardedFor(
+ "\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\""))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedForIpv6UnquotedWithPort() {
+ assertThat(
+ ForwarderHeaderParser.extractForwardedFor(
+ "[1111:1111:1111:1111:1111:1111:1111:1111]:2222"))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedForEmpty() {
+ assertThat(ForwarderHeaderParser.extractForwardedFor("")).isNull();
+ }
+
+ @Test
+ void extractForwardedForMultiple() {
+ assertThat(ForwarderHeaderParser.extractForwardedFor("1.1.1.1,1.2.3.4")).isEqualTo("1.1.1.1");
+ }
+
+ @Test
+ void extractForwardedForMultipleIpv6() {
+ assertThat(
+ ForwarderHeaderParser.extractForwardedFor(
+ "\"[1111:1111:1111:1111:1111:1111:1111:1111]\",1.2.3.4"))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedForMultipleIpv6Unquoted() {
+ assertThat(
+ ForwarderHeaderParser.extractForwardedFor(
+ "[1111:1111:1111:1111:1111:1111:1111:1111],1.2.3.4"))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedForMultipleIpv6Unbracketed() {
+ assertThat(
+ ForwarderHeaderParser.extractForwardedFor(
+ "1111:1111:1111:1111:1111:1111:1111:1111,1.2.3.4"))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedForMultipleWithPort() {
+ assertThat(ForwarderHeaderParser.extractForwardedFor("1.1.1.1:2222,1.2.3.4"))
+ .isEqualTo("1.1.1.1");
+ }
+
+ @Test
+ void extractForwardedForMultipleIpv6WithPort() {
+ assertThat(
+ ForwarderHeaderParser.extractForwardedFor(
+ "\"[1111:1111:1111:1111:1111:1111:1111:1111]:2222\",1.2.3.4"))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+
+ @Test
+ void extractForwardedForMultipleIpv6UnquotedWithPort() {
+ assertThat(
+ ForwarderHeaderParser.extractForwardedFor(
+ "[1111:1111:1111:1111:1111:1111:1111:1111]:2222,1.2.3.4"))
+ .isEqualTo("1111:1111:1111:1111:1111:1111:1111:1111");
+ }
+}

MODIFY
instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java
instrumentation-api/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorTest.java
@@ -7,6 +7,7 @@ package io.opentelemetry.instrumentation.api.instrumenter.http;
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.assertj.core.api.Assertions.entry;
@@ -55,23 +56,27 @@ class HttpServerAttributesExtractorTest {
@Override
protected List<String> requestHeader(Map<String, String> request, String name) {
- return asList(request.get("header." + name).split(","));
+ String values = request.get("header." + name);
+ return values == null ? emptyList() : asList(values.split(","));
}
@Override
protected Long requestContentLength(Map<String, String> request, Map<String, String> response) {
- return Long.parseLong(request.get("requestContentLength"));
+ String value = request.get("requestContentLength");
+ return value == null ? null : Long.parseLong(value);
}
@Override
protected Long requestContentLengthUncompressed(
Map<String, String> request, Map<String, String> response) {
- return Long.parseLong(request.get("requestContentLengthUncompressed"));
+ String value = request.get("requestContentLengthUncompressed");
+ return value == null ? null : Long.parseLong(value);
}
@Override
protected Integer statusCode(Map<String, String> request, Map<String, String> response) {
- return Integer.parseInt(response.get("statusCode"));
+ String value = response.get("statusCode");
+ return value == null ? null : Integer.parseInt(value);
}
@Override
@@ -82,19 +87,22 @@ class HttpServerAttributesExtractorTest {
@Override
protected Long responseContentLength(
Map<String, String> request, Map<String, String> response) {
- return Long.parseLong(response.get("responseContentLength"));
+ String value = response.get("responseContentLength");
+ return value == null ? null : Long.parseLong(value);
}
@Override
protected Long responseContentLengthUncompressed(
Map<String, String> request, Map<String, String> response) {
- return Long.parseLong(response.get("responseContentLengthUncompressed"));
+ String value = response.get("responseContentLengthUncompressed");
+ return value == null ? null : Long.parseLong(value);
}
@Override
protected List<String> responseHeader(
Map<String, String> request, Map<String, String> response, String name) {
- return asList(response.get("header." + name).split(","));
+ String values = response.get("header." + name);
+ return values == null ? emptyList() : asList(values.split(","));
}
}
@@ -112,6 +120,7 @@ class HttpServerAttributesExtractorTest {
request.put("serverName", "server");
request.put("header.user-agent", "okhttp 3.x");
request.put("header.host", "github.com");
+ request.put("header.forwarded", "for=1.1.1.1");
request.put("header.custom-request-header", "123,456");
Map<String, String> response = new HashMap<>();
@@ -136,6 +145,7 @@ class HttpServerAttributesExtractorTest {
entry(SemanticAttributes.HTTP_TARGET, "/repositories/1"),
entry(SemanticAttributes.HTTP_USER_AGENT, "okhttp 3.x"),
entry(SemanticAttributes.HTTP_ROUTE, "/repositories/{id}"),
+ entry(SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1"),
entry(
AttributeKey.stringArrayKey("http.request.header.custom_request_header"),
asList("123", "456")));
@@ -149,6 +159,7 @@ class HttpServerAttributesExtractorTest {
entry(SemanticAttributes.HTTP_TARGET, "/repositories/1"),
entry(SemanticAttributes.HTTP_USER_AGENT, "okhttp 3.x"),
entry(SemanticAttributes.HTTP_ROUTE, "/repositories/{id}"),
+ entry(SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1"),
entry(
AttributeKey.stringArrayKey("http.request.header.custom_request_header"),
asList("123", "456")),
@@ -163,4 +174,22 @@ class HttpServerAttributesExtractorTest {
AttributeKey.stringArrayKey("http.response.header.custom_response_header"),
asList("654", "321")));
}
+
+ @Test
+ void extractClientIpFromX_Forwarded_For() {
+ Map<String, String> request = new HashMap<>();
+ request.put("header.x-forwarded-for", "1.1.1.1");
+
+ TestHttpServerAttributesExtractor extractor =
+ new TestHttpServerAttributesExtractor(CapturedHttpHeaders.empty());
+
+ AttributesBuilder attributes = Attributes.builder();
+ extractor.onStart(attributes, request);
+ assertThat(attributes.build())
+ .containsOnly(entry(SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1"));
+
+ extractor.onEnd(attributes, request, null, null);
+ assertThat(attributes.build())
+ .containsOnly(entry(SemanticAttributes.HTTP_CLIENT_IP, "1.1.1.1"));
+ }
}

Move clientIp extraction to HttpServerAttributesExtractor (#4301)

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>