ServletContextPath.prepend doesn't work when server span is created from app server integration

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/AppServerBridge.java

@@ -8,6 +8,7 @@ package io.opentelemetry.instrumentation.api.servlet;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.ContextKey;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * Helper container for Context attributes for transferring certain information between servlet
@@ -39,7 +40,15 @@ public class AppServerBridge {
    * @return new context with AppServerBridge attached.
    */
   public static Context init(Context ctx, boolean shouldRecordException) {
-    return ctx.with(AppServerBridge.CONTEXT_KEY, new AppServerBridge(shouldRecordException));
+    Context context =
+        ctx.with(AppServerBridge.CONTEXT_KEY, new AppServerBridge(shouldRecordException));
+    // Add context for storing servlet context path. Servlet context path is updated from servlet
+    // integration.
+    if (context.get(ServletContextPath.CONTEXT_KEY) == null) {
+      context = context.with(ServletContextPath.CONTEXT_KEY, new AtomicReference<>(null));
+    }
+
+    return context;
   }
 
   private final AtomicBoolean servletUpdatedServerSpanName = new AtomicBoolean(false);

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/servlet/ServletContextPath.java

@@ -7,6 +7,7 @@ package io.opentelemetry.instrumentation.api.servlet;
 
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.ContextKey;
+import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * The context key here is used to propagate the servlet context path throughout the request, so
@@ -22,11 +23,12 @@ public class ServletContextPath {
 
   // Keeps track of the servlet context path that needs to be prepended to the route when updating
   // the span name
-  public static final ContextKey<String> CONTEXT_KEY =
+  public static final ContextKey<AtomicReference<String>> CONTEXT_KEY =
       ContextKey.named("opentelemetry-servlet-context-path-key");
 
   public static String prepend(Context context, String spanName) {
-    String value = context.get(CONTEXT_KEY);
+    AtomicReference<String> valueReference = context.get(CONTEXT_KEY);
+    String value = valueReference != null ? valueReference.get() : null;
     // checking isEmpty just to avoid unnecessary string concat / allocation
     if (value != null && !value.isEmpty()) {
       return value + spanName;

instrumentation-core/servlet-2.2/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java

@@ -15,6 +15,7 @@ import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.security.Principal;
+import java.util.concurrent.atomic.AtomicReference;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import org.slf4j.Logger;
@@ -27,9 +28,13 @@ public abstract class ServletHttpServerTracer<RESPONSE>
 
   public Context startSpan(HttpServletRequest request) {
     Context context = startSpan(request, request, request, getSpanName(request));
+    return addContextPathContext(context, request);
+  }
+
+  protected Context addContextPathContext(Context context, HttpServletRequest request) {
     String contextPath = request.getContextPath();
     if (contextPath != null && !contextPath.isEmpty() && !contextPath.equals("/")) {
-      context = context.with(ServletContextPath.CONTEXT_KEY, contextPath);
+      context = context.with(ServletContextPath.CONTEXT_KEY, new AtomicReference<>(contextPath));
     }
     return context;
   }
@@ -153,19 +158,41 @@ public abstract class ServletHttpServerTracer<RESPONSE>
     return contextPath + servletPath;
   }
 
+  /**
+   * When server spans are managed by app server instrumentation servlet instrumentation should set
+   * information that was not available from app server instrumentation.
+   */
+  public void updateServerSpan(Context attachedContext, HttpServletRequest request) {
+    updateServerSpanName(attachedContext, request);
+    updateContextPath(attachedContext, request);
+  }
+
   /**
    * When server spans are managed by app server instrumentation, servlet must update server span
    * name only once and only during the first pass through the servlet stack. There are potential
    * forward and other scenarios, where servlet path may change, but we don't want this to be
    * reflected in the span name.
    */
-  public void updateServerSpanNameOnce(Context attachedContext, HttpServletRequest request) {
+  private void updateServerSpanName(Context attachedContext, HttpServletRequest request) {
+    // Update name only when server span wasn't created by servlet integration
+    // and has not been already updated
     if (AppServerBridge.shouldUpdateServerSpanName(attachedContext)) {
       updateSpanName(Span.fromContext(attachedContext), request);
       AppServerBridge.setServletUpdatedServerSpanName(attachedContext, true);
     }
   }
 
+  private void updateContextPath(Context attachedContext, HttpServletRequest request) {
+    // Update context path if it isn't already set
+    AtomicReference<String> reference = attachedContext.get(ServletContextPath.CONTEXT_KEY);
+    if (reference != null && reference.get() == null) {
+      String contextPath = request.getContextPath();
+      if (contextPath != null && !contextPath.isEmpty() && !contextPath.equals("/")) {
+        reference.compareAndSet(null, contextPath);
+      }
+    }
+  }
+
   public void updateSpanName(HttpServletRequest request) {
     updateSpanName(getServerSpan(request), request);
   }

instrumentation/liberty/liberty/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/liberty/LibertyHttpServerTracer.java

@@ -6,7 +6,6 @@
 package io.opentelemetry.javaagent.instrumentation.liberty;
 
 import io.opentelemetry.context.Context;
-import io.opentelemetry.instrumentation.api.servlet.ServletContextPath;
 import io.opentelemetry.javaagent.instrumentation.servlet.v3_0.Servlet3HttpServerTracer;
 import javax.servlet.http.HttpServletRequest;
 
@@ -23,11 +22,7 @@ public class LibertyHttpServerTracer extends Servlet3HttpServerTracer {
     // span name will be updated a bit later when calling request.getServletPath() works
     // see LibertyUpdateSpanAdvice
     Context context = startSpan(request, request, request, "HTTP " + request.getMethod());
-    String contextPath = request.getContextPath();
+    return addContextPathContext(context, request);
-    if (contextPath != null && !contextPath.isEmpty() && !contextPath.equals("/")) {
-      context = context.with(ServletContextPath.CONTEXT_KEY, contextPath);
-    }
-    return context;
   }
 
   @Override

instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java

@@ -37,7 +37,7 @@ public class Servlet2Advice {
 
     Context serverContext = tracer().getServerContext(httpServletRequest);
     if (serverContext != null) {
-      tracer().updateServerSpanNameOnce(serverContext, httpServletRequest);
+      tracer().updateServerSpan(serverContext, httpServletRequest);
       return;
     }
 

instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java

@@ -40,14 +40,14 @@ public class Servlet3Advice {
         scope = attachedContext.makeCurrent();
       }
 
-      tracer().updateServerSpanNameOnce(attachedContext, httpServletRequest);
+      tracer().updateServerSpan(attachedContext, httpServletRequest);
       // We are inside nested servlet/filter/app-server span, don't create new span
       return;
     }
 
     Context parentContext = Java8BytecodeBridge.currentContext();
     if (parentContext != null && Java8BytecodeBridge.spanFromContext(parentContext).isRecording()) {
-      tracer().updateServerSpanNameOnce(parentContext, httpServletRequest);
+      tracer().updateServerSpan(parentContext, httpServletRequest);
       // We are inside nested servlet/filter/app-server span, don't create new span
       return;
     }

instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy

@@ -14,6 +14,7 @@ import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
 import javax.servlet.DispatcherType
 import org.apache.struts2.dispatcher.ng.filter.StrutsPrepareAndExecuteFilter
 import org.eclipse.jetty.server.Server
+import org.eclipse.jetty.server.handler.HandlerCollection
 import org.eclipse.jetty.servlet.DefaultServlet
 import org.eclipse.jetty.servlet.ServletContextHandler
 import org.eclipse.jetty.util.resource.FileResource
@@ -79,7 +80,11 @@ class Struts2ActionSpanTest extends HttpServerTest<Server> {
     context.setContextPath(getContextPath())
     def resource = new FileResource(getClass().getResource("/"))
     context.setBaseResource(resource)
-    server.setHandler(context)
+    // jetty integration is disabled for some handler classes, using HandlerCollection here
+    // enables jetty integration
+    HandlerCollection handlerCollection = new HandlerCollection()
+    handlerCollection.addHandler(context)
+    server.setHandler(handlerCollection)
 
     context.addServlet(DefaultServlet, "/")
     context.addFilter(StrutsPrepareAndExecuteFilter, "/*", EnumSet.of(DispatcherType.REQUEST))