Factor in `X-Forwarded-Host`/`Forwarded` when capturing `server.address` and `server.port` (#9721)

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwardedForAddressAndPortExtractor.java

@@ -5,14 +5,18 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter.http;
 
+import static io.opentelemetry.instrumentation.api.instrumenter.http.HeaderParsingHelper.notFound;
+import static io.opentelemetry.instrumentation.api.instrumenter.http.HeaderParsingHelper.setPort;
+
 import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPortExtractor;
 import java.util.Locale;
 
-final class ForwardedAddressAndPortExtractor<REQUEST> implements AddressAndPortExtractor<REQUEST> {
+final class ForwardedForAddressAndPortExtractor<REQUEST>
+    implements AddressAndPortExtractor<REQUEST> {
 
   private final HttpServerAttributesGetter<REQUEST, ?> getter;
 
-  ForwardedAddressAndPortExtractor(HttpServerAttributesGetter<REQUEST, ?> getter) {
+  ForwardedForAddressAndPortExtractor(HttpServerAttributesGetter<REQUEST, ?> getter) {
     this.getter = getter;
   }
 
@@ -38,7 +42,7 @@ final class ForwardedAddressAndPortExtractor<REQUEST> implements AddressAndPortE
     if (start < 0) {
       return false;
     }
-    start += 4; // start is now the index after for=
+    start += "for=".length(); // start is now the index after for=
     if (start >= forwarded.length() - 1) { // the value after for= must not be empty
       return false;
     }
@@ -132,10 +136,6 @@ final class ForwardedAddressAndPortExtractor<REQUEST> implements AddressAndPortE
     return true;
   }
 
-  private static boolean notFound(int pos, int end) {
-    return pos < 0 || pos >= end;
-  }
-
   private static int findPortEnd(String header, int start, int end) {
     int numberEnd = start;
     while (numberEnd < end && Character.isDigit(header.charAt(numberEnd))) {
@@ -143,15 +143,4 @@ final class ForwardedAddressAndPortExtractor<REQUEST> implements AddressAndPortE
     }
     return numberEnd;
   }
-
-  private static void setPort(AddressPortSink sink, String header, int start, int end) {
-    if (start == end) {
-      return;
-    }
-    try {
-      sink.setPort(Integer.parseInt(header.substring(start, end)));
-    } catch (NumberFormatException ignored) {
-      // malformed port, ignoring
-    }
-  }
 }

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwardedHostAddressAndPortExtractor.java

@@ -0,0 +1,90 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.http;
+
+import static io.opentelemetry.instrumentation.api.instrumenter.http.HeaderParsingHelper.notFound;
+import static io.opentelemetry.instrumentation.api.instrumenter.http.HeaderParsingHelper.setPort;
+
+import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPortExtractor;
+import java.util.Locale;
+
+final class ForwardedHostAddressAndPortExtractor<REQUEST>
+    implements AddressAndPortExtractor<REQUEST> {
+
+  private final HttpCommonAttributesGetter<REQUEST, ?> getter;
+
+  ForwardedHostAddressAndPortExtractor(HttpCommonAttributesGetter<REQUEST, ?> getter) {
+    this.getter = getter;
+  }
+
+  @Override
+  public void extract(AddressPortSink sink, REQUEST request) {
+    // try Forwarded
+    for (String forwarded : getter.getHttpRequestHeader(request, "forwarded")) {
+      if (extractFromForwardedHeader(sink, forwarded)) {
+        return;
+      }
+    }
+
+    // try X-Forwarded-Host
+    for (String forwardedHost : getter.getHttpRequestHeader(request, "x-forwarded-host")) {
+      if (extractHost(sink, forwardedHost, 0, forwardedHost.length())) {
+        return;
+      }
+    }
+
+    // try Host
+    for (String host : getter.getHttpRequestHeader(request, "host")) {
+      if (extractHost(sink, host, 0, host.length())) {
+        return;
+      }
+    }
+  }
+
+  private static boolean extractFromForwardedHeader(AddressPortSink sink, String forwarded) {
+    int start = forwarded.toLowerCase(Locale.ROOT).indexOf("host=");
+    if (start < 0) {
+      return false;
+    }
+    start += "host=".length(); // start is now the index after host=
+    if (start >= forwarded.length() - 1) { // the value after host= must not be empty
+      return false;
+    }
+    // find the end of the `host=<address>` section
+    int end = forwarded.indexOf(';', start);
+    if (end < 0) {
+      end = forwarded.length();
+    }
+    return extractHost(sink, forwarded, start, end);
+  }
+
+  private static boolean extractHost(AddressPortSink sink, String host, int start, int end) {
+    if (start >= end) {
+      return false;
+    }
+
+    // skip quotes
+    if (host.charAt(start) == '"') {
+      // try to find the end of the quote
+      int quoteEnd = host.indexOf('"', start + 1);
+      if (notFound(quoteEnd, end)) {
+        // malformed header value
+        return false;
+      }
+      return extractHost(sink, host, start + 1, quoteEnd);
+    }
+
+    int hostHeaderSeparator = host.indexOf(':', start);
+    if (notFound(hostHeaderSeparator, end)) {
+      sink.setAddress(host.substring(start, end));
+    } else {
+      sink.setAddress(host.substring(start, hostHeaderSeparator));
+      setPort(sink, host, hostHeaderSeparator + 1, end);
+    }
+
+    return true;
+  }
+}

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HeaderParsingHelper.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.http;
+
+import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPortExtractor.AddressPortSink;
+
+final class HeaderParsingHelper {
+
+  static boolean notFound(int pos, int end) {
+    return pos < 0 || pos >= end;
+  }
+
+  static void setPort(AddressPortSink sink, String header, int start, int end) {
+    if (start == end) {
+      return;
+    }
+    try {
+      sink.setPort(Integer.parseInt(header.substring(start, end)));
+    } catch (NumberFormatException ignored) {
+      // malformed port, ignoring
+    }
+  }
+
+  private HeaderParsingHelper() {}
+}

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HostAddressAndPortExtractor.java

@@ -5,16 +5,13 @@
 
 package io.opentelemetry.instrumentation.api.instrumenter.http;
 
+import static io.opentelemetry.instrumentation.api.instrumenter.http.HeaderParsingHelper.setPort;
 import static io.opentelemetry.instrumentation.api.instrumenter.http.HttpCommonAttributesExtractor.firstHeaderValue;
-import static java.util.logging.Level.FINE;
 
 import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPortExtractor;
-import java.util.logging.Logger;
 
 final class HostAddressAndPortExtractor<REQUEST> implements AddressAndPortExtractor<REQUEST> {
 
-  private static final Logger logger = Logger.getLogger(HttpCommonAttributesGetter.class.getName());
-
   private final HttpCommonAttributesGetter<REQUEST, ?> getter;
 
   HostAddressAndPortExtractor(HttpCommonAttributesGetter<REQUEST, ?> getter) {
@@ -31,14 +28,9 @@ final class HostAddressAndPortExtractor<REQUEST> implements AddressAndPortExtrac
     int hostHeaderSeparator = host.indexOf(':');
     if (hostHeaderSeparator == -1) {
       sink.setAddress(host);
-      return;
+    } else {
-    }
+      sink.setAddress(host.substring(0, hostHeaderSeparator));
-
+      setPort(sink, host, hostHeaderSeparator + 1, host.length());
-    sink.setAddress(host.substring(0, hostHeaderSeparator));
-    try {
-      sink.setPort(Integer.parseInt(host.substring(hostHeaderSeparator + 1)));
-    } catch (NumberFormatException e) {
-      logger.log(FINE, e.getMessage(), e);
     }
   }
 }

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorBuilder.java

@@ -54,10 +54,10 @@ public final class HttpServerAttributesExtractorBuilder<REQUEST, RESPONSE> {
 
     clientAddressPortExtractor =
         new ClientAddressAndPortExtractor<>(
-            netAttributesGetter, new ForwardedAddressAndPortExtractor<>(httpAttributesGetter));
+            netAttributesGetter, new ForwardedForAddressAndPortExtractor<>(httpAttributesGetter));
     serverAddressPortExtractor =
         new ServerAddressAndPortExtractor<>(
-            netAttributesGetter, new HostAddressAndPortExtractor<>(httpAttributesGetter));
+            netAttributesGetter, new ForwardedHostAddressAndPortExtractor<>(httpAttributesGetter));
   }
 
   /**

instrumentation-api-semconv/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/network/internal/AddressAndPort.java

@@ -30,4 +30,9 @@ public final class AddressAndPort implements AddressAndPortExtractor.AddressPort
   public String getAddress() {
     return address;
   }
+
+  @Nullable
+  public Integer getPort() {
+    return port;
+  }
 }

instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwardedForAddressAndPortExtractorTest.java

@@ -12,7 +12,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.params.provider.Arguments.arguments;
 import static org.mockito.Mockito.doReturn;
 
-import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPortExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPort;
 import java.util.List;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
@@ -27,11 +27,11 @@ import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @ExtendWith(MockitoExtension.class)
-class ForwardedAddressAndPortExtractorTest {
+class ForwardedForAddressAndPortExtractorTest {
 
   @Mock HttpServerAttributesGetter<String, String> getter;
 
-  @InjectMocks ForwardedAddressAndPortExtractor<String> underTest;
+  @InjectMocks ForwardedForAddressAndPortExtractor<String> underTest;
 
   @ParameterizedTest
   @ArgumentsSource(ForwardedArgs.class)
@@ -42,8 +42,8 @@ class ForwardedAddressAndPortExtractorTest {
     AddressAndPort sink = new AddressAndPort();
     underTest.extract(sink, "request");
 
-    assertThat(sink.address).isEqualTo(expectedAddress);
+    assertThat(sink.getAddress()).isEqualTo(expectedAddress);
-    assertThat(sink.port).isEqualTo(expectedPort);
+    assertThat(sink.getPort()).isEqualTo(expectedPort);
   }
 
   static final class ForwardedArgs implements ArgumentsProvider {
@@ -100,8 +100,8 @@ class ForwardedAddressAndPortExtractorTest {
     AddressAndPort sink = new AddressAndPort();
     underTest.extract(sink, "request");
 
-    assertThat(sink.address).isEqualTo(expectedAddress);
+    assertThat(sink.getAddress()).isEqualTo(expectedAddress);
-    assertThat(sink.port).isEqualTo(expectedPort);
+    assertThat(sink.getPort()).isEqualTo(expectedPort);
   }
 
   static final class ForwardedForArgs implements ArgumentsProvider {
@@ -147,20 +147,4 @@ class ForwardedAddressAndPortExtractorTest {
           arguments(asList("1.2.3.4", "::1"), "1.2.3.4", null));
     }
   }
-
-  static final class AddressAndPort implements AddressAndPortExtractor.AddressPortSink {
-
-    @Nullable String address = null;
-    @Nullable Integer port = null;
-
-    @Override
-    public void setAddress(String address) {
-      this.address = address;
-    }
-
-    @Override
-    public void setPort(Integer port) {
-      this.port = port;
-    }
-  }
 }

instrumentation-api-semconv/src/test/java/io/opentelemetry/instrumentation/api/instrumenter/http/ForwardedHostAddressAndPortExtractorTest.java

@@ -0,0 +1,132 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.instrumenter.http;
+
+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.assertThat;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+import static org.mockito.Mockito.doReturn;
+
+import io.opentelemetry.instrumentation.api.instrumenter.network.internal.AddressAndPort;
+import java.util.List;
+import java.util.stream.Stream;
+import javax.annotation.Nullable;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.ArgumentsProvider;
+import org.junit.jupiter.params.provider.ArgumentsSource;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class ForwardedHostAddressAndPortExtractorTest {
+
+  private static final String REQUEST = "request";
+
+  @Mock HttpCommonAttributesGetter<String, String> getter;
+
+  @InjectMocks ForwardedHostAddressAndPortExtractor<String> underTest;
+
+  @ParameterizedTest
+  @ArgumentsSource(ForwardedArgs.class)
+  void shouldParseForwarded(
+      List<String> headers, @Nullable String expectedAddress, @Nullable Integer expectedPort) {
+    doReturn(headers).when(getter).getHttpRequestHeader(REQUEST, "forwarded");
+
+    AddressAndPort sink = new AddressAndPort();
+    underTest.extract(sink, REQUEST);
+
+    assertThat(sink.getAddress()).isEqualTo(expectedAddress);
+    assertThat(sink.getPort()).isEqualTo(expectedPort);
+  }
+
+  static final class ForwardedArgs implements ArgumentsProvider {
+
+    @Override
+    public Stream<? extends Arguments> provideArguments(ExtensionContext extensionContext) {
+      return Stream.of(
+          // empty/invalid headers
+          arguments(singletonList(""), null, null),
+          arguments(singletonList("host="), null, null),
+          arguments(singletonList("host=;"), null, null),
+          arguments(singletonList("host=\""), null, null),
+          arguments(singletonList("host=\"\""), null, null),
+          arguments(singletonList("host=\"example.com"), null, null),
+          arguments(singletonList("by=1.2.3.4, test=abc"), null, null),
+          arguments(singletonList("host=example.com"), "example.com", null),
+          arguments(singletonList("host=\"example.com\""), "example.com", null),
+          arguments(singletonList("host=example.com; test=abc:1234"), "example.com", null),
+          arguments(singletonList("host=\"example.com\"; test=abc:1234"), "example.com", null),
+          arguments(singletonList("host=example.com:port"), "example.com", null),
+          arguments(singletonList("host=\"example.com:port\""), "example.com", null),
+          arguments(singletonList("host=example.com:42"), "example.com", 42),
+          arguments(singletonList("host=\"example.com:42\""), "example.com", 42),
+          arguments(singletonList("host=example.com:42; test=abc:1234"), "example.com", 42),
+          arguments(singletonList("host=\"example.com:42\"; test=abc:1234"), "example.com", 42),
+
+          // multiple headers
+          arguments(
+              asList("proto=https", "host=example.com", "host=github.com:1234"),
+              "example.com",
+              null));
+    }
+  }
+
+  @ParameterizedTest
+  @ArgumentsSource(HostArgs.class)
+  void shouldParseForwardedHost(
+      List<String> headers, @Nullable String expectedAddress, @Nullable Integer expectedPort) {
+    doReturn(emptyList()).when(getter).getHttpRequestHeader(REQUEST, "forwarded");
+    doReturn(headers).when(getter).getHttpRequestHeader(REQUEST, "x-forwarded-host");
+
+    AddressAndPort sink = new AddressAndPort();
+    underTest.extract(sink, REQUEST);
+
+    assertThat(sink.getAddress()).isEqualTo(expectedAddress);
+    assertThat(sink.getPort()).isEqualTo(expectedPort);
+  }
+
+  @ParameterizedTest
+  @ArgumentsSource(HostArgs.class)
+  void shouldParseHost(
+      List<String> headers, @Nullable String expectedAddress, @Nullable Integer expectedPort) {
+    doReturn(emptyList()).when(getter).getHttpRequestHeader(REQUEST, "forwarded");
+    doReturn(emptyList()).when(getter).getHttpRequestHeader(REQUEST, "x-forwarded-host");
+    doReturn(headers).when(getter).getHttpRequestHeader(REQUEST, "host");
+
+    AddressAndPort sink = new AddressAndPort();
+    underTest.extract(sink, REQUEST);
+
+    assertThat(sink.getAddress()).isEqualTo(expectedAddress);
+    assertThat(sink.getPort()).isEqualTo(expectedPort);
+  }
+
+  static final class HostArgs implements ArgumentsProvider {
+
+    @Override
+    public Stream<? extends Arguments> provideArguments(ExtensionContext extensionContext) {
+      return Stream.of(
+          // empty/invalid headers
+          arguments(singletonList(""), null, null),
+          arguments(singletonList("\""), null, null),
+          arguments(singletonList("\"\""), null, null),
+          arguments(singletonList("example.com"), "example.com", null),
+          arguments(singletonList("example.com:port"), "example.com", null),
+          arguments(singletonList("example.com:42"), "example.com", 42),
+          arguments(singletonList("\"example.com\""), "example.com", null),
+          arguments(singletonList("\"example.com:port\""), "example.com", null),
+          arguments(singletonList("\"example.com:42\""), "example.com", 42),
+
+          // multiple headers
+          arguments(asList("example.com", "github.com:1234"), "example.com", null));
+    }
+  }
+}

instrumentation-api-semconv/src/testStableHttpSemconv/java/io/opentelemetry/instrumentation/api/instrumenter/http/HttpServerAttributesExtractorStableSemconvTest.java

@@ -413,6 +413,59 @@ class HttpServerAttributesExtractorStableSemconvTest {
         .containsOnly(entry(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 202L));
   }
 
+  @Test
+  void shouldExtractServerAddressAndPortFromForwardedHeader() {
+    Map<String, String> request = new HashMap<>();
+    request.put("header.forwarded", "host=example.com:42");
+    request.put("header.x-forwarded-host", "opentelemetry.io:987");
+    request.put("header.host", "github.com:123");
+
+    Map<String, String> response = new HashMap<>();
+    response.put("statusCode", "200");
+
+    AttributesExtractor<Map<String, String>, Map<String, String>> extractor =
+        HttpServerAttributesExtractor.create(new TestHttpServerAttributesGetter());
+
+    AttributesBuilder startAttributes = Attributes.builder();
+    extractor.onStart(startAttributes, Context.root(), request);
+
+    assertThat(startAttributes.build())
+        .containsOnly(
+            entry(SemanticAttributes.SERVER_ADDRESS, "example.com"),
+            entry(SemanticAttributes.SERVER_PORT, 42L));
+
+    AttributesBuilder endAttributes = Attributes.builder();
+    extractor.onEnd(endAttributes, Context.root(), request, response, null);
+    assertThat(endAttributes.build())
+        .containsOnly(entry(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200L));
+  }
+
+  @Test
+  void shouldExtractServerAddressAndPortFromForwardedHostHeader() {
+    Map<String, String> request = new HashMap<>();
+    request.put("header.x-forwarded-host", "opentelemetry.io:987");
+    request.put("header.host", "github.com:123");
+
+    Map<String, String> response = new HashMap<>();
+    response.put("statusCode", "200");
+
+    AttributesExtractor<Map<String, String>, Map<String, String>> extractor =
+        HttpServerAttributesExtractor.create(new TestHttpServerAttributesGetter());
+
+    AttributesBuilder startAttributes = Attributes.builder();
+    extractor.onStart(startAttributes, Context.root(), request);
+
+    assertThat(startAttributes.build())
+        .containsOnly(
+            entry(SemanticAttributes.SERVER_ADDRESS, "opentelemetry.io"),
+            entry(SemanticAttributes.SERVER_PORT, 987L));
+
+    AttributesBuilder endAttributes = Attributes.builder();
+    extractor.onEnd(endAttributes, Context.root(), request, response, null);
+    assertThat(endAttributes.build())
+        .containsOnly(entry(SemanticAttributes.HTTP_RESPONSE_STATUS_CODE, 200L));
+  }
+
   @Test
   void shouldExtractServerAddressAndPortFromHostHeader() {
     Map<String, String> request = new HashMap<>();