Capture servlet request parameters

docs/config/common.md

@@ -49,3 +49,16 @@ These configuration options are supported by all HTTP client and server instrume
 
 > **Note**: The property/environment variable names listed in the table are still experimental,
 > and thus are subject to change.
+
+## Capturing servlet request parameters
+
+You can configure the agent to capture predefined HTTP request parameter as span attributes for
+requests that are handled by Servlet API.
+Use the following property to define which servlet request parameters you want to capture:
+
+| System property                                                        | Environment variable                                                   | Description |
+| ---------------------------------------------------------------------- | ---------------------------------------------------------------------- | ----------- |
+| `otel.instrumentation.servlet.experimental.capture-request-parameters` | `OTEL_INSTRUMENTATION_SERVLET_EXPERIMENTAL_CAPTURE_REQUEST_PARAMETERS` | A comma-separated list of request parameter names.
+
+> **Note**: The property/environment variable names listed in the table are still experimental,
+> and thus are subject to change.

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

@@ -18,35 +18,13 @@ public class AppServerBridge {
   private static final ContextKey<AppServerBridge> CONTEXT_KEY =
       ContextKey.named("opentelemetry-servlet-app-server-bridge");
 
-  /**
-   * Attach AppServerBridge to context.
-   *
-   * @param ctx server context
-   * @return new context with AppServerBridge attached.
-   */
-  public static Context init(Context ctx) {
-    return init(ctx, /* shouldRecordException= */ true);
-  }
-
-  /**
-   * Attach AppServerBridge to context.
-   *
-   * @param ctx server context
-   * @param shouldRecordException whether servlet integration should record exception thrown during
-   *     servlet invocation in server span. Use <code>false</code> on servers where exceptions
-   *     thrown during servlet invocation are propagated to the method where server span is closed
-   *     and can be added to server span there and <code>true</code> otherwise.
-   * @return new context with AppServerBridge attached.
-   */
-  public static Context init(Context ctx, boolean shouldRecordException) {
-    return ctx.with(AppServerBridge.CONTEXT_KEY, new AppServerBridge(shouldRecordException));
-  }
-
   private final boolean servletShouldRecordException;
+  private boolean captureServletAttributes;
   private Throwable exception;
 
-  private AppServerBridge(boolean shouldRecordException) {
-    servletShouldRecordException = shouldRecordException;
+  private AppServerBridge(Builder builder) {
+    servletShouldRecordException = builder.recordException;
+    captureServletAttributes = builder.captureServletAttributes;
   }
 
   /**
@@ -78,6 +56,23 @@ public static Throwable getException(Context context) {
     return null;
   }
 
+  /**
+   * Test whether servlet attributes should be captured. This method will return true only on the
+   * first call with given context.
+   *
+   * @param context server context
+   * @return true when servlet attributes should be captured
+   */
+  public static boolean captureServletAttributes(Context context) {
+    AppServerBridge appServerBridge = context.get(AppServerBridge.CONTEXT_KEY);
+    if (appServerBridge != null) {
+      boolean result = appServerBridge.captureServletAttributes;
+      appServerBridge.captureServletAttributes = false;
+      return result;
+    }
+    return false;
+  }
+
   /**
    * Class used as key in CallDepthThreadLocalMap for counting servlet invocation depth in
    * Servlet3Advice and Servlet2Advice. We can not use helper classes like Servlet3Advice and
@@ -91,4 +86,43 @@ class Key {}
 
     return Key.class;
   }
+
+  public static class Builder {
+    boolean recordException;
+    boolean captureServletAttributes;
+
+    /**
+     * Use on servers where exceptions thrown during servlet invocation are not propagated to the
+     * method where server span is closed. Recorded exception can be retrieved by calling {@link
+     * #getException(Context)}
+     *
+     * @return this builder.
+     */
+    public Builder recordException() {
+      recordException = true;
+      return this;
+    }
+
+    /**
+     * Use on servers where server instrumentation is not based on servlet instrumentation. Setting
+     * this flag lets servlet instrumentation know that it should augment server span with servlet
+     * specific attributes.
+     *
+     * @return this builder.
+     */
+    public Builder captureServletAttributes() {
+      captureServletAttributes = true;
+      return this;
+    }
+
+    /**
+     * Attach AppServerBridge to context.
+     *
+     * @param context server context
+     * @return new context with AppServerBridge attached.
+     */
+    public Context init(Context context) {
+      return context.with(AppServerBridge.CONTEXT_KEY, new AppServerBridge(this));
+    }
+  }
 }

instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11Singletons.java

@@ -28,7 +28,7 @@ public final class Jetty11Singletons {
               .addContextCustomizer(
                   (context, request, attributes) -> {
                     context = ServerSpanNaming.init(context, CONTAINER);
-                    return AppServerBridge.init(context, /* shouldRecordException= */ false);
+                    return new AppServerBridge.Builder().init(context);
                   })
               .build(INSTRUMENTATION_NAME, Servlet5Accessor.INSTANCE);
 

instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8Singletons.java

@@ -28,7 +28,7 @@ public final class Jetty8Singletons {
               .addContextCustomizer(
                   (context, request, attributes) -> {
                     context = ServerSpanNaming.init(context, CONTAINER);
-                    return AppServerBridge.init(context, /* shouldRecordException= */ false);
+                    return new AppServerBridge.Builder().init(context);
                   })
               .build(INSTRUMENTATION_NAME, Servlet3Accessor.INSTANCE);
 

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

@@ -27,7 +27,7 @@ public final class LibertySingletons {
               .addContextCustomizer(
                   (context, request, attributes) -> {
                     context = ServerSpanNaming.init(context, CONTAINER);
-                    return AppServerBridge.init(context);
+                    return new AppServerBridge.Builder().recordException().init(context);
                   })
               .build(INSTRUMENTATION_NAME, Servlet3Accessor.INSTANCE);
 

instrumentation/servlet/servlet-3.0/javaagent/build.gradle.kts

@@ -35,3 +35,7 @@ dependencies {
   latestDepTestLibrary("org.apache.tomcat.embed:tomcat-embed-core:9.+")
   latestDepTestLibrary("org.apache.tomcat.embed:tomcat-embed-jasper:9.+")
 }
+
+tasks.withType<Test>().configureEach {
+  jvmArgs("-Dotel.instrumentation.servlet.experimental.capture-request-parameters=test-parameter")
+}

instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.groovy

@@ -14,6 +14,7 @@ import javax.servlet.Servlet
 
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.AUTH_REQUIRED
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_HEADERS
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_PARAMETERS
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.INDEXED_CHILD
@@ -48,6 +49,7 @@ abstract class AbstractServlet3Test<SERVER, CONTEXT> extends HttpServerTest<SERV
     addServlet(context, AUTH_REQUIRED.path, servlet)
     addServlet(context, INDEXED_CHILD.path, servlet)
     addServlet(context, CAPTURE_HEADERS.path, servlet)
+    addServlet(context, CAPTURE_PARAMETERS.path, servlet)
   }
 
   protected ServerEndpoint lastRequest
@@ -66,6 +68,11 @@ abstract class AbstractServlet3Test<SERVER, CONTEXT> extends HttpServerTest<SERV
     ]
   }
 
+  @Override
+  boolean testCapturedRequestParameters() {
+    true
+  }
+
   boolean errorEndpointUsesSendError() {
     true
   }

instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServlet3Test.groovy

@@ -14,6 +14,7 @@ import javax.servlet.http.HttpServletRequest
 
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.AUTH_REQUIRED
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_HEADERS
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_PARAMETERS
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.INDEXED_CHILD
@@ -174,6 +175,7 @@ class JettyServlet3TestForward extends JettyDispatchTest {
     addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Forward)
     addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Forward)
     addServlet(context, "/dispatch" + CAPTURE_HEADERS.path, RequestDispatcherServlet.Forward)
+    addServlet(context, "/dispatch" + CAPTURE_PARAMETERS.path, RequestDispatcherServlet.Forward)
     addServlet(context, "/dispatch" + INDEXED_CHILD.path, RequestDispatcherServlet.Forward)
   }
 }
@@ -209,6 +211,7 @@ class JettyServlet3TestInclude extends JettyDispatchTest {
     addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include)
     addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Include)
     addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Include)
+    addServlet(context, "/dispatch" + CAPTURE_PARAMETERS.path, RequestDispatcherServlet.Include)
     addServlet(context, "/dispatch" + INDEXED_CHILD.path, RequestDispatcherServlet.Include)
   }
 }
@@ -236,6 +239,7 @@ class JettyServlet3TestDispatchImmediate extends JettyDispatchTest {
     addServlet(context, "/dispatch" + REDIRECT.path, TestServlet3.DispatchImmediate)
     addServlet(context, "/dispatch" + AUTH_REQUIRED.path, TestServlet3.DispatchImmediate)
     addServlet(context, "/dispatch" + CAPTURE_HEADERS.path, TestServlet3.DispatchImmediate)
+    addServlet(context, "/dispatch" + CAPTURE_PARAMETERS.path, TestServlet3.DispatchImmediate)
     addServlet(context, "/dispatch" + INDEXED_CHILD.path, TestServlet3.DispatchImmediate)
     addServlet(context, "/dispatch/recursive", TestServlet3.DispatchRecursive)
   }
@@ -263,6 +267,7 @@ class JettyServlet3TestDispatchAsync extends JettyDispatchTest {
     addServlet(context, "/dispatch" + REDIRECT.path, TestServlet3.DispatchAsync)
     addServlet(context, "/dispatch" + AUTH_REQUIRED.path, TestServlet3.DispatchAsync)
     addServlet(context, "/dispatch" + CAPTURE_HEADERS.path, TestServlet3.DispatchAsync)
+    addServlet(context, "/dispatch" + CAPTURE_PARAMETERS.path, TestServlet3.DispatchAsync)
     addServlet(context, "/dispatch" + INDEXED_CHILD.path, TestServlet3.DispatchAsync)
     addServlet(context, "/dispatch/recursive", TestServlet3.DispatchRecursive)
   }

instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/JettyServletHandlerTest.groovy

@@ -40,6 +40,9 @@ class JettyServletHandlerTest extends AbstractServlet3Test<Server, ServletHandle
 
   @Override
   String expectedServerSpanName(ServerEndpoint endpoint) {
+    if (endpoint == ServerEndpoint.CAPTURE_PARAMETERS) {
+      return "HTTP POST"
+    }
     "HTTP GET"
   }
 

instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TestServlet3.groovy

@@ -13,6 +13,7 @@ import javax.servlet.http.HttpServletResponse
 import java.util.concurrent.CountDownLatch
 
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_HEADERS
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_PARAMETERS
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.INDEXED_CHILD
@@ -54,6 +55,10 @@ class TestServlet3 {
             resp.status = endpoint.status
             resp.writer.print(endpoint.body)
             break
+          case CAPTURE_PARAMETERS:
+            resp.status = endpoint.status
+            resp.writer.print(endpoint.body)
+            break
           case ERROR:
             resp.sendError(endpoint.status, endpoint.body)
             break
@@ -104,6 +109,11 @@ class TestServlet3 {
                 resp.writer.print(endpoint.body)
                 context.complete()
                 break
+              case CAPTURE_PARAMETERS:
+                resp.status = endpoint.status
+                resp.writer.print(endpoint.body)
+                context.complete()
+                break
               case ERROR:
                 resp.status = endpoint.status
                 resp.writer.print(endpoint.body)
@@ -154,6 +164,10 @@ class TestServlet3 {
               resp.status = endpoint.status
               resp.writer.print(endpoint.body)
               break
+            case CAPTURE_PARAMETERS:
+              resp.status = endpoint.status
+              resp.writer.print(endpoint.body)
+              break
             case ERROR:
               resp.sendError(endpoint.status, endpoint.body)
               break

instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy

@@ -28,6 +28,7 @@ import java.util.concurrent.TimeoutException
 
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.AUTH_REQUIRED
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_HEADERS
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_PARAMETERS
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.INDEXED_CHILD
@@ -351,6 +352,7 @@ class TomcatServlet3TestForward extends TomcatDispatchTest {
     addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Forward)
     addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Forward)
     addServlet(context, "/dispatch" + CAPTURE_HEADERS.path, RequestDispatcherServlet.Forward)
+    addServlet(context, "/dispatch" + CAPTURE_PARAMETERS.path, RequestDispatcherServlet.Forward)
     addServlet(context, "/dispatch" + INDEXED_CHILD.path, RequestDispatcherServlet.Forward)
   }
 }
@@ -391,6 +393,7 @@ class TomcatServlet3TestInclude extends TomcatDispatchTest {
     addServlet(context, "/dispatch" + ERROR.path, RequestDispatcherServlet.Include)
     addServlet(context, "/dispatch" + EXCEPTION.path, RequestDispatcherServlet.Include)
     addServlet(context, "/dispatch" + AUTH_REQUIRED.path, RequestDispatcherServlet.Include)
+    addServlet(context, "/dispatch" + CAPTURE_PARAMETERS.path, RequestDispatcherServlet.Include)
     addServlet(context, "/dispatch" + INDEXED_CHILD.path, RequestDispatcherServlet.Include)
   }
 }
@@ -417,6 +420,7 @@ class TomcatServlet3TestDispatchImmediate extends TomcatDispatchTest {
     addServlet(context, "/dispatch" + REDIRECT.path, TestServlet3.DispatchImmediate)
     addServlet(context, "/dispatch" + AUTH_REQUIRED.path, TestServlet3.DispatchImmediate)
     addServlet(context, "/dispatch" + CAPTURE_HEADERS.path, TestServlet3.DispatchImmediate)
+    addServlet(context, "/dispatch" + CAPTURE_PARAMETERS.path, TestServlet3.DispatchImmediate)
     addServlet(context, "/dispatch" + INDEXED_CHILD.path, TestServlet3.DispatchImmediate)
     addServlet(context, "/dispatch/recursive", TestServlet3.DispatchRecursive)
   }
@@ -439,6 +443,7 @@ class TomcatServlet3TestDispatchAsync extends TomcatDispatchTest {
     addServlet(context, "/dispatch" + REDIRECT.path, TestServlet3.DispatchAsync)
     addServlet(context, "/dispatch" + AUTH_REQUIRED.path, TestServlet3.DispatchAsync)
     addServlet(context, "/dispatch" + CAPTURE_HEADERS.path, TestServlet3.DispatchAsync)
+    addServlet(context, "/dispatch" + CAPTURE_PARAMETERS.path, TestServlet3.DispatchAsync)
     addServlet(context, "/dispatch" + INDEXED_CHILD.path, TestServlet3.DispatchAsync)
     addServlet(context, "/dispatch/recursive", TestServlet3.DispatchRecursive)
   }

instrumentation/servlet/servlet-5.0/javaagent/build.gradle.kts

@@ -21,3 +21,7 @@ dependencies {
   testLibrary("org.apache.tomcat.embed:tomcat-embed-core:10.0.0")
   testLibrary("org.apache.tomcat.embed:tomcat-embed-jasper:10.0.0")
 }
+
+tasks.withType<Test>().configureEach {
+  jvmArgs("-Dotel.instrumentation.servlet.experimental.capture-request-parameters=test-parameter")
+}

instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5Accessor.java

@@ -14,6 +14,7 @@
 import jakarta.servlet.http.HttpServletResponse;
 import java.security.Principal;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -95,6 +96,13 @@ public Iterable<String> getRequestHeaderNames(HttpServletRequest httpServletRequ
     return Collections.list(httpServletRequest.getHeaderNames());
   }
 
+  @Override
+  public List<String> getRequestParameterValues(
+      HttpServletRequest httpServletRequest, String name) {
+    String[] values = httpServletRequest.getParameterValues(name);
+    return values == null ? Collections.emptyList() : Arrays.asList(values);
+  }
+
   @Override
   public String getRequestServletPath(HttpServletRequest request) {
     return request.getServletPath();

instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5Test.groovy

@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.CAPTURE_PARAMETERS
+
 import io.opentelemetry.api.common.AttributeKey
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
@@ -47,6 +49,7 @@ abstract class AbstractServlet5Test<SERVER, CONTEXT> extends HttpServerTest<SERV
     addServlet(context, AUTH_REQUIRED.path, servlet)
     addServlet(context, INDEXED_CHILD.path, servlet)
     addServlet(context, CAPTURE_HEADERS.path, servlet)
+    addServlet(context, CAPTURE_PARAMETERS.path, servlet)
   }
 
   protected ServerEndpoint lastRequest
@@ -57,6 +60,11 @@ abstract class AbstractServlet5Test<SERVER, CONTEXT> extends HttpServerTest<SERV
     super.request(uri, method)
   }
 
+  @Override
+  boolean testCapturedRequestParameters() {
+    true
+  }
+
   boolean errorEndpointUsesSendError() {
     true
   }