Reduce overhead of unsampled requests (#3681)

* Optimize sampled out requests * Comment
open-telemetry · Jul 28, 2021 · 91b302a · 91b302a
1 parent 07250d3
commit 91b302a
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 49 deletions.
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
@@ -50,7 +50,9 @@ private ServerSpanNaming(Source initialSource) {
   public static void updateServerSpanName(
       Context context, Source source, Supplier<String> serverSpanName) {
     Span serverSpan = ServerSpan.fromContextOrNull(context);
-    if (serverSpan == null) {
+    // checking isRecording() is a helpful optimization for more expensive suppliers
+    // (e.g. Spring MVC instrumentation's HandlerAdapterInstrumentation)
+    if (serverSpan == null || !serverSpan.isRecording()) {
       return;
     }
     ServerSpanNaming serverSpanNaming = context.get(CONTEXT_KEY);

diff --git a/...o/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerAdapterInstrumentation.java b/...o/opentelemetry/javaagent/instrumentation/springwebmvc/HandlerAdapterInstrumentation.java
@@ -5,6 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.springwebmvc;
 
+import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER;
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
 import static io.opentelemetry.javaagent.instrumentation.springwebmvc.SpringWebMvcSingletons.handlerInstrumenter;
@@ -18,6 +19,7 @@
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
+import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
 import io.opentelemetry.instrumentation.api.tracer.ServerSpan;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
@@ -69,7 +71,10 @@ public static void nameResourceAndStartSpan(
       // TODO (trask) is it important to check serverSpan != null here?
       if (serverSpan != null) {
         // Name the parent span based on the matching pattern
-        ServerNameUpdater.updateServerSpanName(parentContext, request);
+        ServerSpanNaming.updateServerSpanName(
+            parentContext,
+            CONTROLLER,
+            SpringWebMvcServerSpanNaming.getServerSpanNameSupplier(parentContext, request));
         // Now create a span for handler/controller execution.
         context = handlerInstrumenter().start(parentContext, handler);
         if (context != null) {

diff --git a/...pentelemetry/javaagent/instrumentation/springwebmvc/HandlerMappingResourceNameFilter.java b/...pentelemetry/javaagent/instrumentation/springwebmvc/HandlerMappingResourceNameFilter.java
@@ -5,7 +5,10 @@
 
 package io.opentelemetry.javaagent.instrumentation.springwebmvc;
 
+import static io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming.Source.CONTROLLER;
+
 import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
@@ -38,19 +41,20 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
       return;
     }
 
-    Context context = Context.current();
-
     if (handlerMappings != null) {
-      try {
-        if (findMapping((HttpServletRequest) request)) {
-
-          // Name the parent span based on the matching pattern
-          // Let the parent span resource name be set with the attribute set in findMapping.
-          ServerNameUpdater.updateServerSpanName(context, (HttpServletRequest) request);
-        }
-      } catch (Exception ignored) {
-        // mapping.getHandler() threw exception.  Ignore
-      }
+      Context context = Context.current();
+      ServerSpanNaming.updateServerSpanName(
+          context,
+          CONTROLLER,
+          () -> {
+            if (findMapping((HttpServletRequest) request)) {
+              // Name the parent span based on the matching pattern
+              // Let the parent span resource name be set with the attribute set in findMapping.
+              return SpringWebMvcServerSpanNaming.getServerSpanName(
+                  context, (HttpServletRequest) request);
+            }
+            return null;
+          });
     }
 
     filterChain.doFilter(request, response);
@@ -64,12 +68,16 @@ public void destroy() {}
    * as an attribute on the request. This attribute is read by SpringWebMvcDecorator.onRequest and
    * set as the resource name.
    */
-  private boolean findMapping(HttpServletRequest request) throws Exception {
-    for (HandlerMapping mapping : handlerMappings) {
-      HandlerExecutionChain handler = mapping.getHandler(request);
-      if (handler != null) {
-        return true;
+  private boolean findMapping(HttpServletRequest request) {
+    try {
+      for (HandlerMapping mapping : handlerMappings) {
+        HandlerExecutionChain handler = mapping.getHandler(request);
+        if (handler != null) {
+          return true;
+        }
       }
+    } catch (Exception ignored) {
+      // mapping.getHandler() threw exception.  Ignore
     }
     return false;
   }

diff --git a/.../main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/ServerNameUpdater.java b/.../main/java/io/opentelemetry/javaagent/instrumentation/springwebmvc/ServerNameUpdater.java
diff --git a/...io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcServerSpanNaming.java b/...io/opentelemetry/javaagent/instrumentation/springwebmvc/SpringWebMvcServerSpanNaming.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.springwebmvc;
+
+import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
+import java.util.function.Supplier;
+import javax.servlet.http.HttpServletRequest;
+import org.springframework.web.servlet.HandlerMapping;
+
+public class SpringWebMvcServerSpanNaming {
+
+  public static Supplier<String> getServerSpanNameSupplier(
+      Context context, HttpServletRequest request) {
+    return () -> getServerSpanName(context, request);
+  }
+
+  public static String getServerSpanName(Context context, HttpServletRequest request) {
+    Object bestMatchingPattern =
+        request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE);
+    if (bestMatchingPattern != null) {
+      return ServletContextPath.prepend(context, bestMatchingPattern.toString());
+    }
+    return null;
+  }
+}