From 46e5219f19db95058cd71148d48ab0cf8f9739a3 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Mon, 3 Apr 2023 09:13:59 +0200 Subject: [PATCH] Remove some Optional usages (#8190) I applied [this comment](https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/8131#discussion_r1151076724) to the whole codebase and removed some `Optional`s that were used in the hot path --- .../rabbitmq/ReceiveRequestTextMapGetter.java | 41 ++++++++++++------- .../AbstractThreadDispatchingHandler.java | 11 ++--- .../internal/ThreadGrouper.java | 10 +++-- .../WebfluxServerHttpAttributesGetter.java | 6 +-- .../spring/webmvc/v5_3/HttpRouteSupport.java | 11 +++-- .../spring/webmvc/v6_0/HttpRouteSupport.java | 11 +++-- 6 files changed, 51 insertions(+), 39 deletions(-) diff --git a/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/ReceiveRequestTextMapGetter.java b/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/ReceiveRequestTextMapGetter.java index 1e34a85fc1..48d9dc1c3e 100644 --- a/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/ReceiveRequestTextMapGetter.java +++ b/instrumentation/rabbitmq-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rabbitmq/ReceiveRequestTextMapGetter.java @@ -5,12 +5,12 @@ 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 { @@ -18,23 +18,34 @@ enum ReceiveRequestTextMapGetter implements TextMapGetter { @Override public Iterable keys(ReceiveRequest carrier) { - return Optional.of(carrier) - .map(ReceiveRequest::getResponse) - .map(GetResponse::getProps) - .map(AMQP.BasicProperties::getHeaders) - .map(Map::keySet) - .orElse(Collections.emptySet()); + Map 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 headers = getHeaders(carrier); + if (headers == null) { + return null; + } + Object value = headers.get(key); + return value == null ? null : value.toString(); + } + + @Nullable + private static Map 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/instrumentation/runtime-telemetry-jfr/library/src/main/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/internal/AbstractThreadDispatchingHandler.java b/instrumentation/runtime-telemetry-jfr/library/src/main/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/internal/AbstractThreadDispatchingHandler.java index 3aef428cd1..9abfbf689e 100644 --- a/instrumentation/runtime-telemetry-jfr/library/src/main/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/internal/AbstractThreadDispatchingHandler.java +++ b/instrumentation/runtime-telemetry-jfr/library/src/main/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/internal/AbstractThreadDispatchingHandler.java @@ -30,12 +30,9 @@ public abstract class AbstractThreadDispatchingHandler implements RecordedEventH @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/instrumentation/runtime-telemetry-jfr/library/src/main/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/internal/ThreadGrouper.java b/instrumentation/runtime-telemetry-jfr/library/src/main/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/internal/ThreadGrouper.java index 5aea6d84be..eb6e9fb6cf 100644 --- a/instrumentation/runtime-telemetry-jfr/library/src/main/java/io/opentelemetry/instrumentation/runtimetelemetryjfr/internal/ThreadGrouper.java +++ b/instrumentation/runtime-telemetry-jfr/library/src/main/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 @@ import jdk.jfr.consumer.RecordedThread; * any time. */ public final class ThreadGrouper { + // FIXME doesn't actually do any grouping, but should be safe for now - public Optional 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/instrumentation/spring/spring-webflux/spring-webflux-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webflux/v5_3/WebfluxServerHttpAttributesGetter.java b/instrumentation/spring/spring-webflux/spring-webflux-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webflux/v5_3/WebfluxServerHttpAttributesGetter.java index 14392fff65..3210a39389 100644 --- a/instrumentation/spring/spring-webflux/spring-webflux-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webflux/v5_3/WebfluxServerHttpAttributesGetter.java +++ b/instrumentation/spring/spring-webflux/spring-webflux-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webflux/v5_3/WebfluxServerHttpAttributesGetter.java @@ -8,7 +8,6 @@ package io.opentelemetry.instrumentation.spring.webflux.v5_3; 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 @@ enum WebfluxServerHttpAttributesGetter 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/instrumentation/spring/spring-webmvc/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/HttpRouteSupport.java b/instrumentation/spring/spring-webmvc/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/HttpRouteSupport.java index cafac0b13a..71e8a25648 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/HttpRouteSupport.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-5.3/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v5_3/HttpRouteSupport.java @@ -11,7 +11,6 @@ import static org.springframework.web.util.ServletRequestPathUtils.PATH_ATTRIBUT 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 @@ final class HttpRouteSupport { 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 mappings = dispatcherServlet.getHandlerMappings(); + if (mappings != null) { + setHandlerMappings(mappings); + } + } } return handlerMappings != null; } diff --git a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v6_0/HttpRouteSupport.java b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v6_0/HttpRouteSupport.java index b0e1926cae..1df0e9a6f4 100644 --- a/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v6_0/HttpRouteSupport.java +++ b/instrumentation/spring/spring-webmvc/spring-webmvc-6.0/library/src/main/java/io/opentelemetry/instrumentation/spring/webmvc/v6_0/HttpRouteSupport.java @@ -13,7 +13,6 @@ import jakarta.servlet.FilterConfig; 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 @@ final class HttpRouteSupport { 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 mappings = dispatcherServlet.getHandlerMappings(); + if (mappings != null) { + setHandlerMappings(mappings); + } + } } return handlerMappings != null; }