Remove capturing lambda (optimization) (#4014)

* Remove capturing lambda * Update instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNameSupplier.java Co-authored-by: Mateusz Rzeszutek <[email protected]> * Update instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNameTwoArgSupplier.java Co-authored-by: Mateusz Rzeszutek <[email protected]> * Update instrumentation/jsf/jsf-common/library/src/main/java/io/opentelemetry/instrumentation/jsf/JsfServerSpanNaming.java Co-authored-by: Mateusz Rzeszutek <[email protected]> * checkstyle Co-authored-by: Mateusz Rzeszutek <[email protected]>
open-telemetry · Aug 31, 2021 · 6533596 · 6533596
1 parent 01ea967
commit 6533596
Show file tree

Hide file tree

Showing 34 changed files with 356 additions and 288 deletions.
diff --git a/...pi/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNameSupplier.java b/...pi/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNameSupplier.java
@@ -0,0 +1,16 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.servlet;
+
+import io.opentelemetry.context.Context;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+@FunctionalInterface
+public interface ServerSpanNameSupplier<T> {
+
+  @Nullable
+  String get(Context context, T arg);
+}
diff --git a/.../main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNameTwoArgSupplier.java b/.../main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNameTwoArgSupplier.java
@@ -0,0 +1,16 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.api.servlet;
+
+import io.opentelemetry.context.Context;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+@FunctionalInterface
+public interface ServerSpanNameTwoArgSupplier<T, U> {
+
+  @Nullable
+  String get(Context context, T arg1, U arg2);
+}
diff --git a/...tion-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNaming.java b/...tion-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServerSpanNaming.java
@@ -10,13 +10,19 @@
 import io.opentelemetry.context.ContextKey;
 import io.opentelemetry.instrumentation.api.tracer.ServerSpan;
 import java.util.function.Supplier;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /** Helper container for tracking whether instrumentation should update server span name or not. */
 public final class ServerSpanNaming {
 
   private static final ContextKey<ServerSpanNaming> CONTEXT_KEY =
       ContextKey.named("opentelemetry-servlet-span-naming-key");
 
+  // this is just to support deprecated methods
+  @Deprecated
+  private static final ServerSpanNameSupplier<Supplier<String>> ZERO_ARG_ADAPTER =
+      (context, supplier) -> supplier.get();
+
   public static Context init(Context context, Source initialSource) {
     ServerSpanNaming serverSpanNaming = context.get(CONTEXT_KEY);
     if (serverSpanNaming != null) {
@@ -39,16 +45,39 @@ private ServerSpanNaming(Source initialSource) {
   /**
    * If there is a server span in the context, and {@link #init(Context, Source)} has been called to
    * populate a {@code ServerSpanName} into the context, then this method will update the server
-   * span name using the provided {@link Supplier} if and only if the last {@link Source} to update
-   * the span name using this method has strictly lower priority than the provided {@link Source},
-   * and the value returned from the {@link Supplier} is non-null.
+   * span name using the provided {@link ServerSpanNameSupplier} if and only if the last {@link
+   * Source} to update the span name using this method has strictly lower priority than the provided
+   * {@link Source}, and the value returned from the {@link ServerSpanNameSupplier} is non-null.
    *
    * <p>If there is a server span in the context, and {@link #init(Context, Source)} has NOT been
    * called to populate a {@code ServerSpanName} into the context, then this method will update the
-   * server span name using the provided {@link Supplier} if the value returned from it is non-null.
+   * server span name using the provided {@link ServerSpanNameSupplier} if the value returned from
+   * it is non-null.
    */
-  public static void updateServerSpanName(
-      Context context, Source source, Supplier<String> serverSpanName) {
+  public static <T> void updateServerSpanName(
+      Context context, Source source, ServerSpanNameSupplier<T> serverSpanName, T arg1) {
+    updateServerSpanName(context, source, OneArgAdapter.getInstance(), arg1, serverSpanName);
+  }
+
+  /**
+   * If there is a server span in the context, and {@link #init(Context, Source)} has been called to
+   * populate a {@code ServerSpanName} into the context, then this method will update the server
+   * span name using the provided {@link ServerSpanNameTwoArgSupplier} if and only if the last
+   * {@link Source} to update the span name using this method has strictly lower priority than the
+   * provided {@link Source}, and the value returned from the {@link ServerSpanNameTwoArgSupplier}
+   * is non-null.
+   *
+   * <p>If there is a server span in the context, and {@link #init(Context, Source)} has NOT been
+   * called to populate a {@code ServerSpanName} into the context, then this method will update the
+   * server span name using the provided {@link ServerSpanNameTwoArgSupplier} if the value returned
+   * from it is non-null.
+   */
+  public static <T, U> void updateServerSpanName(
+      Context context,
+      Source source,
+      ServerSpanNameTwoArgSupplier<T, U> serverSpanName,
+      T arg1,
+      U arg2) {
     Span serverSpan = ServerSpan.fromContextOrNull(context);
     // checking isRecording() is a helpful optimization for more expensive suppliers
     // (e.g. Spring MVC instrumentation's HandlerAdapterInstrumentation)
@@ -57,7 +86,7 @@ public static void updateServerSpanName(
     }
     ServerSpanNaming serverSpanNaming = context.get(CONTEXT_KEY);
     if (serverSpanNaming == null) {
-      String name = serverSpanName.get();
+      String name = serverSpanName.get(context, arg1, arg2);
       if (name != null && !name.isEmpty()) {
         serverSpan.updateName(name);
       }
@@ -68,7 +97,7 @@ public static void updateServerSpanName(
     boolean onlyIfBetterName =
         !source.useFirst && source.order == serverSpanNaming.updatedBySource.order;
     if (source.order > serverSpanNaming.updatedBySource.order || onlyIfBetterName) {
-      String name = serverSpanName.get();
+      String name = serverSpanName.get(context, arg1, arg2);
       if (name != null
           && !name.isEmpty()
           && (!onlyIfBetterName || serverSpanNaming.isBetterName(name))) {
@@ -79,8 +108,10 @@ public static void updateServerSpanName(
     }
   }
 
-  private boolean isBetterName(String name) {
-    return name.length() > nameLength;
+  @Deprecated
+  public static void updateServerSpanName(
+      Context context, Source source, Supplier<String> serverSpanName) {
+    updateServerSpanName(context, source, ZERO_ARG_ADAPTER, serverSpanName);
   }
 
   // TODO (trask) migrate the one usage (ServletHttpServerTracer) to ServerSpanNaming.init() once we
@@ -96,6 +127,10 @@ public static void updateSource(Context context, Source source) {
     }
   }
 
+  private boolean isBetterName(String name) {
+    return name.length() > nameLength;
+  }
+
   public enum Source {
     CONTAINER(1),
     // for servlet filters we try to find the best name which isn't necessarily from the first
@@ -116,4 +151,21 @@ public enum Source {
       this.useFirst = useFirst;
     }
   }
+
+  private static class OneArgAdapter<T>
+      implements ServerSpanNameTwoArgSupplier<T, ServerSpanNameSupplier<T>> {
+
+    private static final OneArgAdapter<Object> INSTANCE = new OneArgAdapter<>();
+
+    @SuppressWarnings("unchecked")
+    static <T> OneArgAdapter<T> getInstance() {
+      return (OneArgAdapter<T>) INSTANCE;
+    }
+
+    @Override
+    public @Nullable String get(
+        Context context, T arg, ServerSpanNameSupplier<T> serverSpanNameSupplier) {
+      return serverSpanNameSupplier.get(context, arg);
+    }
+  }
 }
diff --git a/...c/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsServerSpanNaming.java b/...c/main/java/io/opentelemetry/javaagent/instrumentation/grails/GrailsServerSpanNaming.java
@@ -5,23 +5,20 @@
 
 package io.opentelemetry.javaagent.instrumentation.grails;
 
-import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.servlet.ServerSpanNameSupplier;
 import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
-import java.util.function.Supplier;
 import org.grails.web.mapping.mvc.GrailsControllerUrlMappingInfo;
 
 public class GrailsServerSpanNaming {
 
-  public static Supplier<String> getServerSpanNameSupplier(
-      Context context, GrailsControllerUrlMappingInfo info) {
-    return () -> getServerSpanName(context, info);
-  }
+  public static final ServerSpanNameSupplier<GrailsControllerUrlMappingInfo> SERVER_SPAN_NAME =
+      (context, info) -> {
+        String action =
+            info.getActionName() != null
+                ? info.getActionName()
+                : info.getControllerClass().getDefaultAction();
+        return ServletContextPath.prepend(context, "/" + info.getControllerName() + "/" + action);
+      };
 
-  private static String getServerSpanName(Context context, GrailsControllerUrlMappingInfo info) {
-    String action =
-        info.getActionName() != null
-            ? info.getActionName()
-            : info.getControllerClass().getDefaultAction();
-    return ServletContextPath.prepend(context, "/" + info.getControllerName() + "/" + action);
-  }
+  private GrailsServerSpanNaming() {}
 }
diff --git a/...emetry/javaagent/instrumentation/grails/UrlMappingsInfoHandlerAdapterInstrumentation.java b/...emetry/javaagent/instrumentation/grails/UrlMappingsInfoHandlerAdapterInstrumentation.java
@@ -51,8 +51,8 @@ public static void nameSpan(@Advice.Argument(2) Object handler) {
         ServerSpanNaming.updateServerSpanName(
             parentContext,
             CONTROLLER,
-            GrailsServerSpanNaming.getServerSpanNameSupplier(
-                parentContext, (GrailsControllerUrlMappingInfo) handler));
+            GrailsServerSpanNaming.SERVER_SPAN_NAME,
+            (GrailsControllerUrlMappingInfo) handler);
       }
     }
   }

diff --git a/...o/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsAnnotationsInstrumentation.java b/...o/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsAnnotationsInstrumentation.java
@@ -82,7 +82,8 @@ public static void nameSpan(
       ServerSpanNaming.updateServerSpanName(
           parentContext,
           ServerSpanNaming.Source.CONTROLLER,
-          JaxrsServerSpanNaming.getServerSpanNameSupplier(parentContext, handlerData));
+          JaxrsServerSpanNaming.SERVER_SPAN_NAME,
+          handlerData);
 
       if (!instrumenter().shouldStart(parentContext, handlerData)) {
         return;

diff --git a/...ain/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsServerSpanNaming.java b/...ain/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v1_0/JaxrsServerSpanNaming.java
@@ -5,28 +5,25 @@
 
 package io.opentelemetry.javaagent.instrumentation.jaxrs.v1_0;
 
-import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.servlet.ServerSpanNameSupplier;
 import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
 import io.opentelemetry.javaagent.bootstrap.jaxrs.JaxrsContextPath;
-import java.util.function.Supplier;
 
 public class JaxrsServerSpanNaming {
 
-  public static Supplier<String> getServerSpanNameSupplier(
-      Context context, HandlerData handlerData) {
-    return () -> {
-      String pathBasedSpanName = handlerData.getServerSpanName();
-      // If path based name is empty skip prepending context path so that path based name would
-      // remain as an empty string for which we skip updating span name. Path base span name is
-      // empty when method and class don't have a jax-rs path annotation, this can happen when
-      // creating an "abort" span, see RequestContextHelper.
-      if (!pathBasedSpanName.isEmpty()) {
-        pathBasedSpanName = JaxrsContextPath.prepend(context, pathBasedSpanName);
-        pathBasedSpanName = ServletContextPath.prepend(context, pathBasedSpanName);
-      }
-      return pathBasedSpanName;
-    };
-  }
+  public static final ServerSpanNameSupplier<HandlerData> SERVER_SPAN_NAME =
+      (context, handlerData) -> {
+        String pathBasedSpanName = handlerData.getServerSpanName();
+        // If path based name is empty skip prepending context path so that path based name would
+        // remain as an empty string for which we skip updating span name. Path base span name is
+        // empty when method and class don't have a jax-rs path annotation, this can happen when
+        // creating an "abort" span, see RequestContextHelper.
+        if (!pathBasedSpanName.isEmpty()) {
+          pathBasedSpanName = JaxrsContextPath.prepend(context, pathBasedSpanName);
+          pathBasedSpanName = ServletContextPath.prepend(context, pathBasedSpanName);
+        }
+        return pathBasedSpanName;
+      };
 
   private JaxrsServerSpanNaming() {}
 }
diff --git a/...ntelemetry/javaagent/instrumentation/jaxrs/v2_0/DefaultRequestContextInstrumentation.java b/...ntelemetry/javaagent/instrumentation/jaxrs/v2_0/DefaultRequestContextInstrumentation.java
@@ -64,7 +64,8 @@ public static void createGenericSpan(
       ServerSpanNaming.updateServerSpanName(
           parentContext,
           ServerSpanNaming.Source.CONTROLLER,
-          JaxrsServerSpanNaming.getServerSpanNameSupplier(parentContext, handlerData));
+          JaxrsServerSpanNaming.SERVER_SPAN_NAME,
+          handlerData);
 
       if (!instrumenter().shouldStart(parentContext, handlerData)) {
         return;

diff --git a/...o/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxrsAnnotationsInstrumentation.java b/...o/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxrsAnnotationsInstrumentation.java
@@ -107,7 +107,8 @@ public static void nameSpan(
       ServerSpanNaming.updateServerSpanName(
           parentContext,
           ServerSpanNaming.Source.CONTROLLER,
-          JaxrsServerSpanNaming.getServerSpanNameSupplier(parentContext, handlerData));
+          JaxrsServerSpanNaming.SERVER_SPAN_NAME,
+          handlerData);
 
       if (!instrumenter().shouldStart(parentContext, handlerData)) {
         return;

diff --git a/...ain/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxrsServerSpanNaming.java b/...ain/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/JaxrsServerSpanNaming.java
@@ -5,28 +5,25 @@
 
 package io.opentelemetry.javaagent.instrumentation.jaxrs.v2_0;
 
-import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.servlet.ServerSpanNameSupplier;
 import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
 import io.opentelemetry.javaagent.bootstrap.jaxrs.JaxrsContextPath;
-import java.util.function.Supplier;
 
 public class JaxrsServerSpanNaming {
 
-  public static Supplier<String> getServerSpanNameSupplier(
-      Context context, HandlerData handlerData) {
-    return () -> {
-      String pathBasedSpanName = handlerData.getServerSpanName();
-      // If path based name is empty skip prepending context path so that path based name would
-      // remain as an empty string for which we skip updating span name. Path base span name is
-      // empty when method and class don't have a jax-rs path annotation, this can happen when
-      // creating an "abort" span, see RequestContextHelper.
-      if (!pathBasedSpanName.isEmpty()) {
-        pathBasedSpanName = JaxrsContextPath.prepend(context, pathBasedSpanName);
-        pathBasedSpanName = ServletContextPath.prepend(context, pathBasedSpanName);
-      }
-      return pathBasedSpanName;
-    };
-  }
+  public static final ServerSpanNameSupplier<HandlerData> SERVER_SPAN_NAME =
+      (context, handlerData) -> {
+        String pathBasedSpanName = handlerData.getServerSpanName();
+        // If path based name is empty skip prepending context path so that path based name would
+        // remain as an empty string for which we skip updating span name. Path base span name is
+        // empty when method and class don't have a jax-rs path annotation, this can happen when
+        // creating an "abort" span, see RequestContextHelper.
+        if (!pathBasedSpanName.isEmpty()) {
+          pathBasedSpanName = JaxrsContextPath.prepend(context, pathBasedSpanName);
+          pathBasedSpanName = ServletContextPath.prepend(context, pathBasedSpanName);
+        }
+        return pathBasedSpanName;
+      };
 
   private JaxrsServerSpanNaming() {}
 }
diff --git a/...main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/RequestContextHelper.java b/...main/java/io/opentelemetry/javaagent/instrumentation/jaxrs/v2_0/RequestContextHelper.java
@@ -30,7 +30,8 @@ public static Context createOrUpdateAbortSpan(
     ServerSpanNaming.updateServerSpanName(
         parentContext,
         ServerSpanNaming.Source.CONTROLLER,
-        JaxrsServerSpanNaming.getServerSpanNameSupplier(parentContext, handlerData));
+        JaxrsServerSpanNaming.SERVER_SPAN_NAME,
+        handlerData);
 
     if (currentSpan != null && currentSpan != serverSpan) {
       // there's already an active span, and it's not the same as the server (servlet) span,

diff --git a/...0-axis2-1.6/library/src/main/java/io/opentelemetry/instrumentation/axis2/Axis2Helper.java b/...0-axis2-1.6/library/src/main/java/io/opentelemetry/instrumentation/axis2/Axis2Helper.java
@@ -25,9 +25,7 @@ public static void start(MessageContext message) {
 
     Axis2Request request = new Axis2Request(message);
     ServerSpanNaming.updateServerSpanName(
-        parentContext,
-        CONTROLLER,
-        Axis2ServerSpanNaming.getServerSpanNameSupplier(parentContext, request));
+        parentContext, CONTROLLER, Axis2ServerSpanNaming.SERVER_SPAN_NAME, request);
 
     if (!instrumenter().shouldStart(parentContext, request)) {
       return;

diff --git a/...6/library/src/main/java/io/opentelemetry/instrumentation/axis2/Axis2ServerSpanNaming.java b/...6/library/src/main/java/io/opentelemetry/instrumentation/axis2/Axis2ServerSpanNaming.java
@@ -5,32 +5,28 @@
 
 package io.opentelemetry.instrumentation.axis2;
 
-import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.servlet.ServerSpanNameSupplier;
 import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
-import java.util.function.Supplier;
 import javax.servlet.http.HttpServletRequest;
 import org.apache.axis2.jaxws.core.MessageContext;
 
 public class Axis2ServerSpanNaming {
 
-  private Axis2ServerSpanNaming() {}
-
-  public static Supplier<String> getServerSpanNameSupplier(Context context, Axis2Request request) {
-    return () -> getServerSpanName(context, request);
-  }
+  public static final ServerSpanNameSupplier<Axis2Request> SERVER_SPAN_NAME =
+      (context, axis2Request) -> {
+        String spanName = axis2Request.spanName();
+        MessageContext message = axis2Request.message();
+        HttpServletRequest request =
+            (HttpServletRequest) message.getMEPContext().get("transport.http.servletRequest");
+        if (request != null) {
+          String servletPath = request.getServletPath();
+          if (!servletPath.isEmpty()) {
+            spanName = servletPath + "/" + spanName;
+          }
+        }
 
-  private static String getServerSpanName(Context context, Axis2Request axis2Request) {
-    String spanName = axis2Request.spanName();
-    MessageContext message = axis2Request.message();
-    HttpServletRequest request =
-        (HttpServletRequest) message.getMEPContext().get("transport.http.servletRequest");
-    if (request != null) {
-      String servletPath = request.getServletPath();
-      if (!servletPath.isEmpty()) {
-        spanName = servletPath + "/" + spanName;
-      }
-    }
+        return ServletContextPath.prepend(context, spanName);
+      };
 
-    return ServletContextPath.prepend(context, spanName);
-  }
+  private Axis2ServerSpanNaming() {}
 }