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

@@ -6,7 +6,6 @@
 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;
@@ -17,23 +16,22 @@
 import org.springframework.http.HttpHeaders;
 
 class HeaderUtil {
-  private static final MethodHandle GET_HEADERS;
+  @Nullable private static final MethodHandle GET_HEADERS;
 
   static {
     GET_HEADERS =
-        nonNullHandle(
+        firstAvailableHandle(
             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 MethodHandle nonNullHandle(
+  @Nullable
+  private static MethodHandle firstAvailableHandle(
       @Nullable MethodHandle first, Supplier<? extends MethodHandle> supplier) {
-    return (first != null)
-        ? first
-        : requireNonNull(supplier.get(), "Could not find suitable get method on HttpHeaders");
+    return first != null ? first : supplier.get();
   }
 
   @Nullable
@@ -45,20 +43,19 @@ private static MethodHandle findGetHeadersMethod(MethodType methodType) {
     }
   }
 
-  // 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 GET_HEADERS.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();
+    if (GET_HEADERS != null) {
+      try {
+        List<String> result = (List<String>) GET_HEADERS.invoke(headers, name);
+        if (result != null) {
+          return result;
+        }
+      } catch (Throwable t) {
+        // ignore
       }
-      return result;
-    } catch (Throwable t) {
-      throw new IllegalStateException(t);
     }
+    return emptyList();
   }
 
   private HeaderUtil() {}

instrumentation/spring/spring-webflux/spring-webflux-5.3/library/build.gradle.kts

@@ -1,5 +1,6 @@
 plugins {
   id("otel.library-instrumentation")
+  id("otel.nullaway-conventions")
 }
 
 dependencies {

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

@@ -10,6 +10,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.instrumentation.api.semconv.http.HttpServerRoute;
 import io.opentelemetry.instrumentation.api.semconv.http.HttpServerRouteSource;
+import javax.annotation.Nullable;
 import org.reactivestreams.Subscription;
 import org.springframework.core.Ordered;
 import org.springframework.http.server.reactive.ServerHttpResponse;
@@ -115,7 +116,7 @@ public void onComplete() {
       actual.onComplete();
     }
 
-    private void onTerminal(Context currentContext, Throwable t) {
+    private void onTerminal(Context currentContext, @Nullable Throwable t) {
       ServerHttpResponse response = exchange.getResponse();
       if (response.isCommitted()) {
         end(currentContext, t);
@@ -128,13 +129,16 @@ private void onTerminal(Context currentContext, Throwable t) {
       }
     }
 
-    private void end(Context currentContext, Throwable t) {
+    private void end(Context currentContext, @Nullable Throwable t) {
       // Update HTTP route now, because during instrumenter.start()
       // the HTTP route isn't available from the exchange attributes, but is afterwards
       HttpServerRoute.update(
           currentContext,
           HttpServerRouteSource.CONTROLLER,
-          (context, exchange) -> WebfluxServerHttpAttributesGetter.INSTANCE.getHttpRoute(exchange),
+          (context, exchange) ->
+              exchange == null
+                  ? null
+                  : WebfluxServerHttpAttributesGetter.INSTANCE.getHttpRoute(exchange),
           exchange);
       instrumenter.end(currentContext, exchange, exchange, t);
     }

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

@@ -22,9 +22,9 @@ enum WebfluxServerHttpAttributesGetter
     implements HttpServerAttributesGetter<ServerWebExchange, ServerWebExchange> {
   INSTANCE;
 
-  private static final MethodHandle GET_RAW_STATUS_CODE;
-  private static final MethodHandle GET_STATUS_CODE;
-  private static final MethodHandle STATUS_CODE_VALUE;
+  @Nullable private static final MethodHandle GET_RAW_STATUS_CODE;
+  @Nullable private static final MethodHandle GET_STATUS_CODE;
+  @Nullable private static final MethodHandle STATUS_CODE_VALUE;
 
   static {
     MethodHandle getRawStatusCode = null;
@@ -61,6 +61,7 @@ enum WebfluxServerHttpAttributesGetter
     STATUS_CODE_VALUE = statusCodeValue;
   }
 
+  @Nullable
   private static Integer getStatusCode(ServerHttpResponse response) {
     if (GET_RAW_STATUS_CODE != null) {
       try {

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

@@ -11,6 +11,8 @@
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.util.List;
+import java.util.function.Supplier;
+import javax.annotation.Nullable;
 import org.springframework.http.HttpHeaders;
 
 /**
@@ -19,31 +21,44 @@
  * guarantees are made.
  */
 public final class HeaderUtil {
-  private static final MethodHandle GET_HEADERS;
+
+  @Nullable private static final MethodHandle GET_HEADERS;
 
   static {
-    // since spring web 7.0
-    GET_HEADERS = findGetHeadersMethod(MethodType.methodType(List.class, String.class, List.class));
+    GET_HEADERS =
+        firstAvailableHandle(
+            findGetHeadersMethod(
+                MethodType.methodType(List.class, String.class)), // since spring web 7.0
+            () ->
+                findGetHeadersMethod(
+                    MethodType.methodType(List.class, Object.class))); // before spring web 7.0
+  }
+
+  @Nullable
+  private static MethodHandle firstAvailableHandle(
+      @Nullable MethodHandle first, Supplier<? extends MethodHandle> supplier) {
+    return first != null ? first : supplier.get();
   }
 
+  @Nullable
   private static MethodHandle findGetHeadersMethod(MethodType methodType) {
     try {
-      return MethodHandles.lookup().findVirtual(HttpHeaders.class, "getOrDefault", methodType);
+      return MethodHandles.lookup().findVirtual(HttpHeaders.class, "get", methodType);
     } catch (Throwable 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 GET_HEADERS.invoke result
   public static List<String> getHeader(HttpHeaders headers, String name) {
-    if (headers != null) {
+    if (GET_HEADERS != null) {
       try {
-        return (List<String>) GET_HEADERS.invoke(headers, name, emptyList());
+        List<String> result = (List<String>) GET_HEADERS.invoke(headers, name);
+        if (result != null) {
+          return result;
+        }
       } catch (Throwable t) {
-        throw new IllegalStateException(t);
+        // ignore
       }
     }
     return emptyList();

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

@@ -6,13 +6,17 @@
 package io.opentelemetry.instrumentation.spring.webflux.v5_3.internal;
 
 import io.opentelemetry.context.propagation.TextMapSetter;
+import javax.annotation.Nullable;
 import org.springframework.web.reactive.function.client.ClientRequest;
 
 enum HttpHeadersSetter implements TextMapSetter<ClientRequest.Builder> {
   INSTANCE;
 
   @Override
-  public void set(ClientRequest.Builder carrier, String key, String value) {
+  public void set(@Nullable ClientRequest.Builder carrier, String key, String value) {
+    if (carrier == null) {
+      return;
+    }
     carrier.headers(httpHeaders -> httpHeaders.set(key, value));
   }
 }

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

@@ -7,6 +7,7 @@
 
 import io.opentelemetry.instrumentation.api.incubator.builder.internal.DefaultHttpClientInstrumenterBuilder;
 import io.opentelemetry.instrumentation.api.incubator.builder.internal.DefaultHttpServerInstrumenterBuilder;
+import io.opentelemetry.instrumentation.api.internal.Initializer;
 import io.opentelemetry.instrumentation.spring.webflux.v5_3.SpringWebfluxClientTelemetryBuilder;
 import io.opentelemetry.instrumentation.spring.webflux.v5_3.SpringWebfluxServerTelemetryBuilder;
 import java.util.function.Function;
@@ -40,6 +41,7 @@ private SpringWebfluxBuilderUtil() {}
     return serverBuilderExtractor;
   }
 
+  @Initializer
   public static void setServerBuilderExtractor(
       Function<
               SpringWebfluxServerTelemetryBuilder,
@@ -55,6 +57,7 @@ public static void setServerBuilderExtractor(
     return clientBuilderExtractor;
   }
 
+  @Initializer
   public static void setClientBuilderExtractor(
       Function<
               SpringWebfluxClientTelemetryBuilder,