pr review

Author: zeitlinger

instrumentation/spring/spring-web/spring-web-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/web/v3_1/HeaderUtil.java

@@ -3,43 +3,49 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.instrumentation.spring.webflux.v5_3;
+package io.opentelemetry.instrumentation.spring.web.v3_1;
 
 import static java.util.Collections.emptyList;
+import static java.util.Objects.requireNonNull;
 
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.util.List;
+import java.util.function.Supplier;
 import org.springframework.http.HttpHeaders;
 
 class HeaderUtil {
   private static final MethodHandle GET_HEADERS;
 
   static {
     GET_HEADERS =
-        isSpring7OrNewer()
-            ? findGetHeadersMethod(MethodType.methodType(List.class, String.class))
-            : findGetHeadersMethod(MethodType.methodType(List.class, Object.class));
+        requireNonNullElseGet(
+            findGetHeadersMethod(
+                MethodType.methodType(List.class, String.class)), // since spring web 7.0
+            () ->
+                findGetHeadersMethod(
+                    MethodType.methodType(List.class, Object.class))); // before spring web 7.0
   }
 
-  private static boolean isSpring7OrNewer() {
-    try {
-      Class.forName("org.springframework.core.Nullness");
-      return true;
-    } catch (ClassNotFoundException e) {
-      return false;
-    }
+  // copied from java.util.Objects in Java 9+
+  private static <T> T requireNonNullElseGet(T obj, Supplier<? extends T> supplier) {
+    return (obj != null)
+        ? obj
+        : requireNonNull(requireNonNull(supplier, "supplier").get(), "supplier.get()");
   }
 
   private static MethodHandle findGetHeadersMethod(MethodType methodType) {
     try {
       return MethodHandles.lookup().findVirtual(HttpHeaders.class, "get", methodType);
     } catch (Throwable t) {
-      throw new IllegalStateException(t);
+      return null;
     }
   }
 
+  // before spring web 7.0 HttpHeaders implements Map<String, List<String>>, this triggers
+  // errorprone BadInstanceof warning since errorpone is not aware that this instanceof check does
+  // not pass with spring web 7.0+
   @SuppressWarnings("unchecked") // casting MethodHandle.invoke result
   static List<String> getHeader(HttpHeaders headers, String name) {
     try {

instrumentation/spring/spring-web/spring-web-3.1/library/src/main/java/io/opentelemetry/instrumentation/spring/web/v3_1/SpringWebHttpAttributesGetter.java

@@ -5,15 +5,12 @@
 
 package io.opentelemetry.instrumentation.spring.web.v3_1;
 
-import static java.util.Collections.emptyList;
-
 import io.opentelemetry.instrumentation.api.semconv.http.HttpClientAttributesGetter;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.util.List;
 import javax.annotation.Nullable;
-import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpRequest;
 import org.springframework.http.client.ClientHttpResponse;
 
@@ -23,7 +20,6 @@ enum SpringWebHttpAttributesGetter
 
   @Nullable private static final MethodHandle GET_STATUS_CODE;
   @Nullable private static final MethodHandle STATUS_CODE_VALUE;
-  private static final MethodHandle GET_HEADERS;
 
   static {
     MethodHandles.Lookup lookup = MethodHandles.publicLookup();
@@ -58,27 +54,6 @@ enum SpringWebHttpAttributesGetter
 
     GET_STATUS_CODE = getStatusCode;
     STATUS_CODE_VALUE = statusCodeValue;
-    GET_HEADERS =
-        isSpring7OrNewer()
-            ? findGetHeadersMethod(MethodType.methodType(List.class, String.class))
-            : findGetHeadersMethod(MethodType.methodType(List.class, Object.class));
-  }
-
-  private static boolean isSpring7OrNewer() {
-    try {
-      Class.forName("org.springframework.core.Nullness");
-      return true;
-    } catch (ClassNotFoundException e) {
-      return false;
-    }
-  }
-
-  private static MethodHandle findGetHeadersMethod(MethodType methodType) {
-    try {
-      return MethodHandles.lookup().findVirtual(HttpHeaders.class, "get", methodType);
-    } catch (Throwable t) {
-      return null;
-    }
   }
 
   @Override
@@ -94,20 +69,7 @@ public String getUrlFull(HttpRequest httpRequest) {
 
   @Override
   public List<String> getHttpRequestHeader(HttpRequest httpRequest, String name) {
-    return getHeader(httpRequest.getHeaders(), name);
-  }
-
-  @SuppressWarnings("unchecked") // casting MethodHandle.invoke result
-  static List<String> getHeader(HttpHeaders headers, String name) {
-    try {
-      List<String> result = (List<String>) GET_HEADERS.invoke(headers, name);
-      if (result == null) {
-        return emptyList();
-      }
-      return result;
-    } catch (Throwable t) {
-      throw new IllegalStateException(t);
-    }
+    return HeaderUtil.getHeader(httpRequest.getHeaders(), name);
   }
 
   @Override
@@ -129,7 +91,7 @@ public Integer getHttpResponseStatusCode(
   @Override
   public List<String> getHttpResponseHeader(
       HttpRequest httpRequest, ClientHttpResponse clientHttpResponse, String name) {
-    return getHeader(clientHttpResponse.getHeaders(), name);
+    return HeaderUtil.getHeader(clientHttpResponse.getHeaders(), name);
   }
 
   @Override

instrumentation/spring/spring-webflux/spring-webflux-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webflux/v5_3/internal/WebClientHttpAttributesGetter.java

@@ -5,15 +5,9 @@
 
 package io.opentelemetry.instrumentation.spring.webflux.v5_3.internal;
 
-import static java.util.Collections.emptyList;
-
 import io.opentelemetry.instrumentation.api.semconv.http.HttpClientAttributesGetter;
-import java.lang.invoke.MethodHandle;
-import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodType;
 import java.util.List;
 import javax.annotation.Nullable;
-import org.springframework.http.HttpHeaders;
 import org.springframework.web.reactive.function.client.ClientRequest;
 import org.springframework.web.reactive.function.client.ClientResponse;
 
@@ -25,32 +19,6 @@ public enum WebClientHttpAttributesGetter
     implements HttpClientAttributesGetter<ClientRequest, ClientResponse> {
   INSTANCE;
 
-  private static final MethodHandle GET_HEADERS;
-
-  static {
-    GET_HEADERS =
-        isSpring7OrNewer()
-            ? findGetHeadersMethod(MethodType.methodType(List.class, String.class))
-            : findGetHeadersMethod(MethodType.methodType(List.class, Object.class));
-  }
-
-  private static boolean isSpring7OrNewer() {
-    try {
-      Class.forName("org.springframework.core.Nullness");
-      return true;
-    } catch (ClassNotFoundException e) {
-      return false;
-    }
-  }
-
-  private static MethodHandle findGetHeadersMethod(MethodType methodType) {
-    try {
-      return MethodHandles.lookup().findVirtual(HttpHeaders.class, "get", methodType);
-    } catch (Throwable t) {
-      throw new IllegalStateException(t);
-    }
-  }
-
   @Override
   public String getUrlFull(ClientRequest request) {
     return request.url().toString();
@@ -62,17 +30,8 @@ public String getHttpRequestMethod(ClientRequest request) {
   }
 
   @Override
-  @SuppressWarnings("unchecked") // casting MethodHandle.invoke result
   public List<String> getHttpRequestHeader(ClientRequest request, String name) {
-    try {
-      List<String> result = (List<String>) GET_HEADERS.invoke(request.headers(), name);
-      if (result == null) {
-        return emptyList();
-      }
-      return result;
-    } catch (Throwable t) {
-      throw new IllegalStateException(t);
-    }
+    return HeaderUtil.getHeader(request.headers(), name);
   }
 
   @Override