Code review sweep (run 25235735340)

instrumentation/servlet/servlet-3.0/testing/src/main/java/io/opentelemetry/instrumentation/servlet/v3_0/AbstractServlet3Test.java

@@ -20,6 +20,7 @@
 
 import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.instrumentation.api.internal.HttpConstants;
+import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension;
 import io.opentelemetry.instrumentation.testing.junit.http.AbstractHttpServerTest;
 import io.opentelemetry.instrumentation.testing.junit.http.HttpServerTestOptions;
 import io.opentelemetry.instrumentation.testing.junit.http.ServerEndpoint;
@@ -30,9 +31,12 @@
 import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpResponse;
 import javax.servlet.Servlet;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
 public abstract class AbstractServlet3Test<SERVER, CONTEXT> extends AbstractHttpServerTest<SERVER> {
 
+  @RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
+
   public static final ServerEndpoint HTML_PRINT_WRITER =
       new ServerEndpoint(
           "HTML_PRINT_WRITER",
@@ -157,6 +161,8 @@ void snippetInjectionWithServletOutputStream() {
 
     ExperimentalSnippetHolder.setSnippet(
         "\n  <script type=\"text/javascript\"> Test Test</script>");
+    cleanup.deferCleanup(() -> ExperimentalSnippetHolder.setSnippet(""));
+
     AggregatedHttpRequest request = request(HTML_SERVLET_OUTPUT_STREAM, "GET");
     AggregatedHttpResponse response = client.execute(request).aggregate().join();
 
@@ -176,8 +182,6 @@ void snippetInjectionWithServletOutputStream() {
     assertThat(response.contentUtf8()).isEqualTo(result);
     assertThat(response.headers().contentLength()).isEqualTo(result.length());
 
-    ExperimentalSnippetHolder.setSnippet("");
-
     String expectedRoute = expectedHttpRoute(HTML_SERVLET_OUTPUT_STREAM, "GET");
     testing()
         .waitAndAssertTraces(
@@ -198,6 +202,8 @@ void snippetInjectionWithPrintWriter() {
     assumeTrue(isAgentTest());
 
     ExperimentalSnippetHolder.setSnippet("\n  <script type=\"text/javascript\"> Test </script>");
+    cleanup.deferCleanup(() -> ExperimentalSnippetHolder.setSnippet(""));
+
     AggregatedHttpRequest request = request(HTML_PRINT_WRITER, "GET");
     AggregatedHttpResponse response = client.execute(request).aggregate().join();
 
@@ -218,8 +224,6 @@ void snippetInjectionWithPrintWriter() {
     assertThat(response.contentUtf8()).isEqualTo(result);
     assertThat(response.headers().contentLength()).isEqualTo(result.length());
 
-    ExperimentalSnippetHolder.setSnippet("");
-
     String expectedRoute = expectedHttpRoute(HTML_PRINT_WRITER, "GET");
     testing()
         .waitAndAssertTraces(

instrumentation/servlet/servlet-3.0/testing/src/main/java/io/opentelemetry/instrumentation/servlet/v3_0/tomcat/AbstractTomcatServlet3Test.java

@@ -97,7 +97,7 @@ protected Tomcat setupServer() throws Exception {
     tomcatServer.setPort(port);
     tomcatServer.getConnector().setEnableLookups(true); // get localhost instead of 127.0.0.1
 
-    File applicationDir = new File(baseDir, "/webapps/ROOT");
+    File applicationDir = new File(baseDir, "webapps/ROOT");
     applicationDir.mkdirs();
 
     Context servletContext =

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

@@ -8,9 +8,8 @@
 import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseMutator;
 import jakarta.servlet.http.HttpServletResponse;
 
-public enum Servlet5HttpServerResponseMutator
+public class Servlet5HttpServerResponseMutator
     implements HttpServerResponseMutator<HttpServletResponse> {
-  INSTANCE;
 
   @Override
   public void appendHeader(HttpServletResponse response, String name, String value) {

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

@@ -84,7 +84,7 @@ && helper().needsRescoping(currentContext, attachedContext)) {
             .customize(
                 contextToUpdate,
                 (HttpServletResponse) response,
-                Servlet5HttpServerResponseMutator.INSTANCE);
+                new Servlet5HttpServerResponseMutator());
       }
     }
 

instrumentation/servlet/servlet-5.0/jetty11-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/jetty/JettyServlet5Test.java

@@ -23,7 +23,7 @@ public abstract class JettyServlet5Test
     extends AbstractServlet5Test<Server, ServletContextHandler> {
 
   @RegisterExtension
-  protected static final InstrumentationExtension testing =
+  private static final InstrumentationExtension testing =
       HttpServerInstrumentationExtension.forAgent();
 
   @Override

instrumentation/servlet/servlet-5.0/jetty11-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/jetty/JettyServletHandlerTest.java

@@ -27,7 +27,7 @@
 class JettyServletHandlerTest extends AbstractServlet5Test<Server, ServletHandler> {
 
   @RegisterExtension
-  protected static final InstrumentationExtension testing =
+  private static final InstrumentationExtension testing =
       HttpServerInstrumentationExtension.forAgent();
 
   @Override

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

@@ -21,6 +21,7 @@
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.List;
+import javax.annotation.Nullable;
 
 /**
  * This class is internal and is hence not for public use. Its APIs are unstable and can change at
@@ -47,17 +48,19 @@ public String getRequestUri(HttpServletRequest request) {
   }
 
   @Override
+  @Nullable
   public String getRequestQueryString(HttpServletRequest request) {
     return request.getQueryString();
   }
 
   @Override
+  @Nullable
   public Object getRequestAttribute(HttpServletRequest request, String name) {
     return request.getAttribute(name);
   }
 
   @Override
-  public void setRequestAttribute(HttpServletRequest request, String name, Object value) {
+  public void setRequestAttribute(HttpServletRequest request, String name, @Nullable Object value) {
     request.setAttribute(name, value);
   }
 
@@ -92,6 +95,7 @@ public Integer getRequestLocalPort(HttpServletRequest request) {
   }
 
   @Override
+  @Nullable
   public String getRequestHeader(HttpServletRequest request, String name) {
     return request.getHeader(name);
   }
@@ -121,11 +125,13 @@ public String getRequestServletPath(HttpServletRequest request) {
   }
 
   @Override
+  @Nullable
   public String getRequestPathInfo(HttpServletRequest request) {
     return request.getPathInfo();
   }
 
   @Override
+  @Nullable
   public Principal getRequestUserPrincipal(HttpServletRequest request) {
     return request.getUserPrincipal();
   }

instrumentation/servlet/servlet-5.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/tomcat/AbstractTomcatServlet5Test.java

@@ -97,7 +97,7 @@ protected Tomcat setupServer() throws Exception {
     tomcatServer.setPort(port);
     tomcatServer.getConnector().setEnableLookups(true); // get localhost instead of 127.0.0.1
 
-    File applicationDir = new File(baseDir, "/webapps/ROOT");
+    File applicationDir = new File(baseDir, "webapps/ROOT");
     applicationDir.mkdirs();
 
     Context servletContext =
@@ -122,7 +122,7 @@ protected void configureServer(Context servletContext) {}
 
   @BeforeEach
   void setUp() {
-    accessLogValve.getLoggedIds().clear();
+    accessLogValve.clearLoggedIds();
     testing().clearAllExportedData();
   }
 

instrumentation/servlet/servlet-5.0/testing/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/tomcat/TestAccessLogValve.java

@@ -27,7 +27,15 @@ public TestAccessLogValve() {
   }
 
   public List<Map.Entry<String, String>> getLoggedIds() {
-    return loggedIds;
+    synchronized (loggedIds) {
+      return new ArrayList<>(loggedIds);
+    }
+  }
+
+  public void clearLoggedIds() {
+    synchronized (loggedIds) {
+      loggedIds.clear();
+    }
   }
 
   @Override
@@ -61,7 +69,7 @@ public void waitForLoggedIds(int expected) {
         toWait = endTime - System.currentTimeMillis();
       }
 
-      if (toWait <= 0) {
+      if (loggedIds.size() < expected) {
         throw new IllegalStateException(
             "Timeout waiting for " + expected + " access log ids, got " + loggedIds.size());
       }

instrumentation/servlet/servlet-5.0/tomcat-testing/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/tomcat/mapping/TomcatServlet5MappingTest.java

@@ -28,7 +28,7 @@ protected Tomcat setupServer() throws Exception {
     tomcatServer.setPort(port);
     tomcatServer.getConnector().setEnableLookups(true); // get localhost instead of 127.0.0.1
 
-    File applicationDir = new File(baseDir, "/webapps/ROOT");
+    File applicationDir = new File(baseDir, "webapps/ROOT");
     Files.createDirectories(applicationDir.toPath());
 
     Context servletContext =