Always add trace_id and span_id attributes to servlet requests (#15485)

Author: laurit

instrumentation/servlet/README.md

@@ -2,10 +2,11 @@
 
 ## Settings
 
-| System property                                                        | Type    | Default | Description                                         |
-|------------------------------------------------------------------------| ------- | ------- |-----------------------------------------------------|
-| `otel.instrumentation.servlet.experimental-span-attributes`            | Boolean | `false` | Enable the capture of experimental span attributes. |
-| `otel.instrumentation.servlet.experimental.capture-request-parameters` | List    | Empty   | Request parameters to be captured (experimental).   |
+| System property                                                            | Type    | Default | Description                                         |
+|----------------------------------------------------------------------------|---------|---------|-----------------------------------------------------|
+| `otel.instrumentation.servlet.experimental-span-attributes`                | Boolean | `false` | Enable the capture of experimental span attributes. |
+| `otel.instrumentation.servlet.experimental.capture-request-parameters`     | List    | Empty   | Request parameters to be captured (experimental).   |
+| `otel.instrumentation.servlet.experimental.add-trace-id-request-attribute` | Boolean | `true`  | Add `trace_id` and `span_id` as request attributes. |
 
 ### A word about version
 

instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/BaseServletHelper.java

@@ -15,6 +15,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan;
 import io.opentelemetry.instrumentation.api.semconv.http.HttpServerRoute;
 import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig;
+import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
 import io.opentelemetry.javaagent.bootstrap.servlet.AppServerBridge;
 import io.opentelemetry.javaagent.bootstrap.servlet.MappingResolver;
 import io.opentelemetry.javaagent.bootstrap.servlet.ServletAsyncContext;
@@ -23,8 +24,12 @@
 import java.security.Principal;
 import java.util.function.Function;
 
-@SuppressWarnings("deprecation") // using deprecated semconv
 public abstract class BaseServletHelper<REQUEST, RESPONSE> {
+  private static final boolean ADD_TRACE_ID_REQUEST_ATTRIBUTE =
+      AgentInstrumentationConfig.get()
+          .getBoolean(
+              "otel.instrumentation.servlet.experimental.add-trace-id-request-attribute", true);
+
   protected final Instrumenter<ServletRequestContext<REQUEST>, ServletResponseContext<RESPONSE>>
       instrumenter;
   protected final ServletAccessor<REQUEST, RESPONSE> accessor;
@@ -53,13 +58,7 @@ public Context start(Context parentContext, ServletRequestContext<REQUEST> reque
     Context context = instrumenter.start(parentContext, requestContext);
 
     REQUEST request = requestContext.request();
-    SpanContext spanContext = Span.fromContext(context).getSpanContext();
-    // we do this e.g. so that servlet containers can use these values in their access logs
-    // TODO: These are only available when using servlet instrumentation or when server
-    //  instrumentation extends servlet instrumentation e.g. jetty. Either remove or make sure they
-    //  also work on tomcat and wildfly.
-    accessor.setRequestAttribute(request, "trace_id", spanContext.getTraceId());
-    accessor.setRequestAttribute(request, "span_id", spanContext.getSpanId());
+    addRequestAttributes(request, context);
 
     context = addServletContextPath(context, request);
     context = addAsyncContext(context);
@@ -69,6 +68,16 @@ public Context start(Context parentContext, ServletRequestContext<REQUEST> reque
     return context;
   }
 
+  private void addRequestAttributes(REQUEST request, Context context) {
+    if (!ADD_TRACE_ID_REQUEST_ATTRIBUTE) {
+      return;
+    }
+
+    SpanContext spanContext = Span.fromContext(context).getSpanContext();
+    accessor.setRequestAttribute(request, "trace_id", spanContext.getTraceId());
+    accessor.setRequestAttribute(request, "span_id", spanContext.getSpanId());
+  }
+
   protected Context addServletContextPath(Context context, REQUEST request) {
     return ServletContextPath.init(context, contextPathExtractor, request);
   }
@@ -100,6 +109,8 @@ public Context updateContext(
           result, servlet ? SERVER : SERVER_FILTER, spanNameProvider, mappingResolver, request);
     }
 
+    addRequestAttributes(request, context);
+
     return result;
   }
 

instrumentation/tomcat/tomcat-10.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/tomcat/v10_0/TestServlet.java

@@ -18,6 +18,14 @@ class TestServlet extends HttpServlet {
   protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException {
     String path = req.getServletPath();
 
+    // these are set by servlet instrumentation
+    if (req.getAttribute("trace_id") == null) {
+      throw new IllegalStateException("trace_id attribute not found");
+    }
+    if (req.getAttribute("span_id") == null) {
+      throw new IllegalStateException("span_id attribute not found");
+    }
+
     ServerEndpoint serverEndpoint = ServerEndpoint.forPath(path);
     if (serverEndpoint != null) {
       AbstractHttpServerTest.controller(

instrumentation/tomcat/tomcat-7.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/tomcat/v7_0/TestServlet.java

@@ -18,6 +18,14 @@ class TestServlet extends HttpServlet {
   protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException {
     String path = req.getServletPath();
 
+    // these are set by servlet instrumentation
+    if (req.getAttribute("trace_id") == null) {
+      throw new IllegalStateException("trace_id attribute not found");
+    }
+    if (req.getAttribute("span_id") == null) {
+      throw new IllegalStateException("span_id attribute not found");
+    }
+
     ServerEndpoint serverEndpoint = ServerEndpoint.forPath(path);
     if (serverEndpoint != null) {
       AbstractHttpServerTest.controller(