Remove some Optional usages (#8190)

I applied [this comment](#8131 (comment)) to the whole codebase and removed some `Optional`s that were used in the hot path
open-telemetry · Apr 3, 2023 · 46e5219 · 46e5219
1 parent d6271cc
commit 46e5219
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 39 deletions.
diff --git a/...java/io/opentelemetry/javaagent/instrumentation/rabbitmq/ReceiveRequestTextMapGetter.java b/...java/io/opentelemetry/javaagent/instrumentation/rabbitmq/ReceiveRequestTextMapGetter.java
@@ -5,36 +5,47 @@
 
 package io.opentelemetry.javaagent.instrumentation.rabbitmq;
 
+import static java.util.Collections.emptySet;
+
 import com.rabbitmq.client.AMQP;
 import com.rabbitmq.client.GetResponse;
 import io.opentelemetry.context.propagation.TextMapGetter;
-import java.util.Collections;
 import java.util.Map;
-import java.util.Optional;
 import javax.annotation.Nullable;
 
 enum ReceiveRequestTextMapGetter implements TextMapGetter<ReceiveRequest> {
   INSTANCE;
 
   @Override
   public Iterable<String> keys(ReceiveRequest carrier) {
-    return Optional.of(carrier)
-        .map(ReceiveRequest::getResponse)
-        .map(GetResponse::getProps)
-        .map(AMQP.BasicProperties::getHeaders)
-        .map(Map::keySet)
-        .orElse(Collections.emptySet());
+    Map<String, Object> headers = getHeaders(carrier);
+    return headers == null ? emptySet() : headers.keySet();
   }
 
   @Nullable
   @Override
   public String get(@Nullable ReceiveRequest carrier, String key) {
-    return Optional.ofNullable(carrier)
-        .map(ReceiveRequest::getResponse)
-        .map(GetResponse::getProps)
-        .map(AMQP.BasicProperties::getHeaders)
-        .map(headers -> headers.get(key))
-        .map(Object::toString)
-        .orElse(null);
+    Map<String, Object> headers = getHeaders(carrier);
+    if (headers == null) {
+      return null;
+    }
+    Object value = headers.get(key);
+    return value == null ? null : value.toString();
+  }
+
+  @Nullable
+  private static Map<String, Object> getHeaders(@Nullable ReceiveRequest carrier) {
+    if (carrier == null) {
+      return null;
+    }
+    GetResponse response = carrier.getResponse();
+    if (response == null) {
+      return null;
+    }
+    AMQP.BasicProperties props = response.getProps();
+    if (props == null) {
+      return null;
+    }
+    return props.getHeaders();
   }
 }
diff --git a/...emetry/instrumentation/runtimetelemetryjfr/internal/AbstractThreadDispatchingHandler.java b/...emetry/instrumentation/runtimetelemetryjfr/internal/AbstractThreadDispatchingHandler.java
@@ -30,12 +30,9 @@ protected AbstractThreadDispatchingHandler(ThreadGrouper grouper) {
 
   @Override
   public void accept(RecordedEvent ev) {
-    grouper
-        .groupedName(ev)
-        .ifPresent(
-            groupedThreadName ->
-                perThread
-                    .computeIfAbsent(groupedThreadName, this::createPerThreadSummarizer)
-                    .accept(ev));
+    String groupedName = grouper.groupedName(ev);
+    if (groupedName != null) {
+      perThread.computeIfAbsent(groupedName, this::createPerThreadSummarizer).accept(ev);
+    }
   }
 }
diff --git a/...ain/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/internal/ThreadGrouper.java b/...ain/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/internal/ThreadGrouper.java
@@ -5,7 +5,7 @@
 
 package io.opentelemetry.instrumentation.runtimetelemetryjfr.internal;
 
-import java.util.Optional;
+import javax.annotation.Nullable;
 import jdk.jfr.consumer.RecordedEvent;
 import jdk.jfr.consumer.RecordedThread;
 
@@ -14,13 +14,15 @@
  * any time.
  */
 public final class ThreadGrouper {
+
   // FIXME doesn't actually do any grouping, but should be safe for now
-  public Optional<String> groupedName(RecordedEvent ev) {
+  @Nullable
+  public String groupedName(RecordedEvent ev) {
     Object thisField = ev.getValue("eventThread");
     if (thisField instanceof RecordedThread) {
       RecordedThread thread = (RecordedThread) thisField;
-      return Optional.of(thread.getJavaName());
+      return thread.getJavaName();
     }
-    return Optional.empty();
+    return null;
   }
 }
diff --git a/.../opentelemetry/instrumentation/spring/webflux/v5_3/WebfluxServerHttpAttributesGetter.java b/.../opentelemetry/instrumentation/spring/webflux/v5_3/WebfluxServerHttpAttributesGetter.java
@@ -8,7 +8,6 @@
 import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerAttributesGetter;
 import java.util.Collections;
 import java.util.List;
-import java.util.Optional;
 import javax.annotation.Nullable;
 import org.springframework.web.reactive.HandlerMapping;
 import org.springframework.web.server.ServerWebExchange;
@@ -55,10 +54,7 @@ public String getTarget(ServerWebExchange request) {
     if (path == null && query == null) {
       return null;
     }
-    if (query != null) {
-      query = "?" + query;
-    }
-    return Optional.ofNullable(path).orElse("") + Optional.ofNullable(query).orElse("");
+    return (path == null ? "" : path) + (query == null ? "" : "?" + query);
   }
 
   @Nullable

diff --git a/...y/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/HttpRouteSupport.java b/...y/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/HttpRouteSupport.java
@@ -11,7 +11,6 @@
 import io.opentelemetry.context.Context;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Optional;
 import java.util.concurrent.atomic.AtomicBoolean;
 import javax.annotation.Nullable;
 import javax.servlet.FilterConfig;
@@ -65,9 +64,13 @@ public void onApplicationEvent(ContextRefreshedEvent event) {
   boolean hasMappings() {
     if (contextRefreshTriggered.compareAndSet(true, false)) {
       // reload the handler mappings only if the web app context was recently refreshed
-      Optional.ofNullable(dispatcherServlet)
-          .map(DispatcherServlet::getHandlerMappings)
-          .ifPresent(this::setHandlerMappings);
+      DispatcherServlet dispatcherServlet = this.dispatcherServlet;
+      if (dispatcherServlet != null) {
+        List<HandlerMapping> mappings = dispatcherServlet.getHandlerMappings();
+        if (mappings != null) {
+          setHandlerMappings(mappings);
+        }
+      }
     }
     return handlerMappings != null;
   }

diff --git a/...y/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v6_0/HttpRouteSupport.java b/...y/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v6_0/HttpRouteSupport.java
@@ -13,7 +13,6 @@
 import jakarta.servlet.http.HttpServletRequest;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Optional;
 import java.util.concurrent.atomic.AtomicBoolean;
 import javax.annotation.Nullable;
 import org.springframework.context.ApplicationListener;
@@ -65,9 +64,13 @@ public void onApplicationEvent(ContextRefreshedEvent event) {
   boolean hasMappings() {
     if (contextRefreshTriggered.compareAndSet(true, false)) {
       // reload the handler mappings only if the web app context was recently refreshed
-      Optional.ofNullable(dispatcherServlet)
-          .map(DispatcherServlet::getHandlerMappings)
-          .ifPresent(this::setHandlerMappings);
+      DispatcherServlet dispatcherServlet = this.dispatcherServlet;
+      if (dispatcherServlet != null) {
+        List<HandlerMapping> mappings = dispatcherServlet.getHandlerMappings();
+        if (mappings != null) {
+          setHandlerMappings(mappings);
+        }
+      }
     }
     return handlerMappings != null;
   }