Fill HTTP_CLIENT_IP in ServerInstrumenter (#3756)

benchmark/src/jmh/java/io/opentelemetry/benchmark/InstrumenterBenchmark.java

@@ -113,11 +113,6 @@ static class ConstantHttpAttributesExtractor extends HttpAttributesExtractor<Voi
       return null;
     }
 
-    @Override
-    protected @Nullable String clientIp(Void unused, @Nullable Void unused2) {
-      return null;
-    }
-
     @Override
     protected @Nullable Integer statusCode(Void unused, Void unused2) {
       return 200;

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/ServerInstrumenter.java

@@ -5,10 +5,15 @@
 
 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.HttpAttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.net.NetAttributesExtractor;
 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> {
 
@@ -17,7 +22,7 @@ final class ServerInstrumenter<REQUEST, RESPONSE> extends Instrumenter<REQUEST,
 
   ServerInstrumenter(
       InstrumenterBuilder<REQUEST, RESPONSE> builder, TextMapGetter<REQUEST> getter) {
-    super(builder);
+    super(addClientIpExtractor(builder, getter));
     this.propagators = builder.openTelemetry.getPropagators();
     this.getter = getter;
   }
@@ -29,4 +34,133 @@ public Context start(Context parentContext, REQUEST request) {
     Context extracted = propagators.getTextMapPropagator().extract(parentContext, request, getter);
     return super.start(extracted, request);
   }
+
+  private static <REQUEST, RESPONSE> InstrumenterBuilder<REQUEST, RESPONSE> addClientIpExtractor(
+      InstrumenterBuilder<REQUEST, RESPONSE> builder, TextMapGetter<REQUEST> getter) {
+    HttpAttributesExtractor<REQUEST, RESPONSE> httpAttributesExtractor = null;
+    NetAttributesExtractor<REQUEST, RESPONSE> netAttributesExtractor = null;
+    for (AttributesExtractor<? super REQUEST, ? super RESPONSE> extractor :
+        builder.attributesExtractors) {
+      if (extractor instanceof NetAttributesExtractor) {
+        netAttributesExtractor = (NetAttributesExtractor<REQUEST, RESPONSE>) extractor;
+      } else if (extractor instanceof HttpAttributesExtractor) {
+        httpAttributesExtractor = (HttpAttributesExtractor<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, netAttributesExtractor));
+    return builder;
+  }
+
+  private static class HttpClientIpExtractor<REQUEST, RESPONSE>
+      extends AttributesExtractor<REQUEST, RESPONSE> {
+
+    private final TextMapGetter<REQUEST> getter;
+    @Nullable private final NetAttributesExtractor<REQUEST, RESPONSE> netAttributesExtractor;
+
+    HttpClientIpExtractor(
+        TextMapGetter<REQUEST> getter,
+        @Nullable NetAttributesExtractor<REQUEST, RESPONSE> netAttributesExtractor) {
+      this.getter = getter;
+      this.netAttributesExtractor = netAttributesExtractor;
+    }
+
+    @Override
+    protected void onStart(AttributesBuilder attributes, REQUEST request) {}
+
+    @Override
+    protected void onEnd(
+        AttributesBuilder attributes, REQUEST request, @Nullable RESPONSE response) {
+      String clientIp = getForwardedClientIp(request);
+      if (clientIp == null && netAttributesExtractor != null) {
+        clientIp = netAttributesExtractor.peerIp(request, response);
+      }
+      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) {
+        forwarded = extractForwardedFor(forwarded);
+        if (forwarded != null) {
+          return 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);
+  }
 }

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

@@ -48,7 +48,6 @@ protected final void onEnd(
         requestContentLengthUncompressed(request, response));
     set(attributes, SemanticAttributes.HTTP_FLAVOR, flavor(request, response));
     set(attributes, SemanticAttributes.HTTP_SERVER_NAME, serverName(request, response));
-    set(attributes, SemanticAttributes.HTTP_CLIENT_IP, clientIp(request, response));
     if (response != null) {
       Integer statusCode = statusCode(request, response);
       if (statusCode != null) {
@@ -127,15 +126,6 @@ protected abstract Long requestContentLengthUncompressed(
   @Nullable
   protected abstract String serverName(REQUEST request, @Nullable RESPONSE response);
 
-  /**
-   * Extracts the {@code http.client_ip} span attribute.
-   *
-   * <p>This is called from {@link Instrumenter#end(Context, Object, Object, Throwable)}, whether
-   * {@code response} is {@code null} or not.
-   */
-  @Nullable
-  protected abstract String clientIp(REQUEST request, @Nullable RESPONSE response);
-
   /**
    * Extracts the {@code http.status_code} span attribute.
    *