pr review

open-telemetry · Aug 29, 2024 · 4832af6 · 4832af6
1 parent 4f9a5ea
commit 4832af6
Show file tree

Hide file tree

Showing 23 changed files with 56 additions and 110 deletions.
diff --git a/...t/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/TestServlet2.java b/...t/src/test/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/TestServlet2.java
@@ -33,7 +33,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) {
             resp.setContentType("text/plain");
             if (SUCCESS.equals(endpoint)) {
               resp.setStatus(endpoint.getStatus());
-                      resp.getWriter().print(endpoint.getBody());
+              resp.getWriter().print(endpoint.getBody());
             } else if (QUERY_PARAM.equals(endpoint)) {
               resp.setStatus(endpoint.getStatus());
               resp.getWriter().print(req.getQueryString());

diff --git a/...st/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/AbstractServlet3Test.java b/...st/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/AbstractServlet3Test.java
@@ -77,9 +77,10 @@ protected boolean hasResponseSpan(ServerEndpoint endpoint) {
 
   public abstract Class<? extends Servlet> servlet();
 
-  public abstract void addServlet(CONTEXT context, String path, Class<? extends Servlet> servlet);
+  public abstract void addServlet(CONTEXT context, String path, Class<? extends Servlet> servlet)
+      throws Exception;
 
-  protected void setupServlets(CONTEXT context) {
+  protected void setupServlets(CONTEXT context) throws Exception {
     Class<? extends Servlet> servlet = servlet();
 
     addServlet(context, SUCCESS.getPath(), servlet);

diff --git a/...java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/HttpServletResponseTest.java b/...java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/HttpServletResponseTest.java
@@ -119,17 +119,7 @@ public void sendRedirect(String s) {
     servlet.service(request, response);
     testing.clearData();
 
-    assertThatCode(
-            () ->
-                runWithSpan(
-                    "parent",
-                    () -> {
-                      try {
-                        response.sendRedirect("");
-                      } catch (IOException e) {
-                        throw new RuntimeException(e);
-                      }
-                    }))
+    assertThatCode(() -> runWithSpan("parent", () -> response.sendRedirect("")))
         .isInstanceOf(RuntimeException.class)
         .hasMessage("some error");
 

diff --git a/...java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/jetty/JettyServlet3Test.java b/...java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/jetty/JettyServlet3Test.java
@@ -123,7 +123,8 @@ public void stopServer(Server server) throws Exception {
 
   @Override
   public void addServlet(
-      ServletContextHandler servletContext, String path, Class<? extends Servlet> servlet) {
+      ServletContextHandler servletContext, String path, Class<? extends Servlet> servlet)
+      throws Exception {
     servletContext.addServlet(servlet, path);
   }
 }
diff --git a/...o/opentelemetry/javaagent/instrumentation/servlet/v3_0/jetty/JettyServletHandlerTest.java b/...o/opentelemetry/javaagent/instrumentation/servlet/v3_0/jetty/JettyServletHandlerTest.java
@@ -103,7 +103,8 @@ protected void handleErrorPage(
 
   @Override
   public void addServlet(
-      ServletHandler servletHandler, String path, Class<? extends Servlet> servlet) {
+      ServletHandler servletHandler, String path, Class<? extends Servlet> servlet)
+      throws Exception {
     servletHandler.addServletWithMapping(servlet, path);
   }
 

diff --git a/...javaagent/instrumentation/servlet/v3_0/jetty/dispatch/JettyServlet3DispatchAsyncTest.java b/...javaagent/instrumentation/servlet/v3_0/jetty/dispatch/JettyServlet3DispatchAsyncTest.java
@@ -31,7 +31,7 @@ public boolean isAsyncTest() {
   }
 
   @Override
-  protected void setupServlets(ServletContextHandler context) {
+  protected void setupServlets(ServletContextHandler context) throws Exception {
     super.setupServlets(context);
     addServlet(
         context, "/dispatch" + HTML_PRINT_WRITER.getPath(), TestServlet3.DispatchAsync.class);

diff --git a/...agent/instrumentation/servlet/v3_0/jetty/dispatch/JettyServlet3DispatchImmediateTest.java b/...agent/instrumentation/servlet/v3_0/jetty/dispatch/JettyServlet3DispatchImmediateTest.java
@@ -36,7 +36,7 @@ public boolean errorEndpointUsesSendError() {
   }
 
   @Override
-  protected void setupServlets(ServletContextHandler context) {
+  protected void setupServlets(ServletContextHandler context) throws Exception {
     super.setupServlets(context);
     addServlet(
         context, "/dispatch" + HTML_PRINT_WRITER.getPath(), TestServlet3.DispatchImmediate.class);

diff --git a/...metry/javaagent/instrumentation/servlet/v3_0/jetty/dispatch/JettyServlet3ForwardTest.java b/...metry/javaagent/instrumentation/servlet/v3_0/jetty/dispatch/JettyServlet3ForwardTest.java
@@ -27,7 +27,7 @@ public Class<? extends Servlet> servlet() {
   }
 
   @Override
-  protected void setupServlets(ServletContextHandler context) {
+  protected void setupServlets(ServletContextHandler context) throws Exception {
     super.setupServlets(context);
 
     addServlet(context, "/dispatch" + SUCCESS.getPath(), RequestDispatcherServlet.Forward.class);

diff --git a/...metry/javaagent/instrumentation/servlet/v3_0/jetty/dispatch/JettyServlet3IncludeTest.java b/...metry/javaagent/instrumentation/servlet/v3_0/jetty/dispatch/JettyServlet3IncludeTest.java
@@ -35,7 +35,7 @@ protected void configure(HttpServerTestOptions options) {
   }
 
   @Override
-  protected void setupServlets(ServletContextHandler context) {
+  protected void setupServlets(ServletContextHandler context) throws Exception {
     super.setupServlets(context);
 
     addServlet(context, "/dispatch" + SUCCESS.getPath(), RequestDispatcherServlet.Include.class);

diff --git a/...telemetry/javaagent/instrumentation/servlet/v3_0/mapping/AbstractServlet3MappingTest.java b/...telemetry/javaagent/instrumentation/servlet/v3_0/mapping/AbstractServlet3MappingTest.java
@@ -41,9 +41,10 @@ void cleanup() {
     cleanupServer();
   }
 
-  public abstract void addServlet(CONTEXT context, String path, Class<? extends Servlet> servlet);
+  public abstract void addServlet(CONTEXT context, String path, Class<? extends Servlet> servlet)
+      throws Exception;
 
-  protected void setupServlets(CONTEXT context) {
+  protected void setupServlets(CONTEXT context) throws Exception {
     addServlet(context, "/prefix/*", TestServlet.class);
     addServlet(context, "*.suffix", TestServlet.class);
   }
@@ -58,7 +59,7 @@ protected void setupServlets(CONTEXT context) {
     ".suffix, /*.suffix, true",
     "suffix, /*, false",
   })
-  void test_path__path(String path, String route, boolean success) {
+  void testPath(String path, String route, boolean success) {
 
     AggregatedHttpResponse response =
         client.get(address.resolve(path).toString()).aggregate().join();

diff --git a/...pentelemetry/javaagent/instrumentation/servlet/v3_0/mapping/JettyServlet3MappingTest.java b/...pentelemetry/javaagent/instrumentation/servlet/v3_0/mapping/JettyServlet3MappingTest.java
@@ -16,31 +16,23 @@
 class JettyServlet3MappingTest extends AbstractServlet3MappingTest<Server, ServletContextHandler> {
 
   @Override
-  protected Server setupServer() {
+  protected Server setupServer() throws Exception {
     Server server = new Server(port);
     ServletContextHandler handler = new ServletContextHandler(null, getContextPath());
     setupServlets(handler);
     server.setHandler(handler);
-    try {
-      server.start();
-    } catch (Exception e) {
-      throw new RuntimeException(e);
-    }
+    server.start();
     return server;
   }
 
   @Override
-  public void stopServer(Server server) {
-    try {
-      server.stop();
-    } catch (Exception e) {
-      throw new RuntimeException(e);
-    }
+  public void stopServer(Server server) throws Exception {
+    server.stop();
     server.destroy();
   }
 
   @Override
-  protected void setupServlets(ServletContextHandler handler) {
+  protected void setupServlets(ServletContextHandler handler) throws Exception {
     super.setupServlets(handler);
 
     addServlet(handler, "/", DefaultServlet.class);

diff --git a/...metry/javaagent/instrumentation/servlet/v3_0/mapping/TomcatServlet3FilterMappingTest.java b/...metry/javaagent/instrumentation/servlet/v3_0/mapping/TomcatServlet3FilterMappingTest.java
@@ -21,15 +21,11 @@
 import org.apache.tomcat.util.descriptor.web.FilterMap;
 
 abstract class TomcatServlet3FilterMappingTest extends TomcatServlet3MappingTest {
-  @SuppressWarnings("ClassNewInstance")
-  public void addFilter(Context servletContext, String path, Class<? extends Filter> filter) {
+  public void addFilter(Context servletContext, String path, Class<? extends Filter> filter)
+      throws Exception {
     String name = UUID.randomUUID().toString();
     FilterDef filterDef = new FilterDef();
-    try {
-      filterDef.setFilter(filter.newInstance());
-    } catch (Exception e) {
-      throw new RuntimeException(e);
-    }
+    filterDef.setFilter(filter.getConstructor().newInstance());
     filterDef.setFilterName(name);
     servletContext.addFilterDef(filterDef);
     FilterMap filterMap = new FilterMap();
@@ -38,16 +34,11 @@ public void addFilter(Context servletContext, String path, Class<? extends Filte
     servletContext.addFilterMap(filterMap);
   }
 
-  @SuppressWarnings("ClassNewInstance")
   public void addFilterWithServletName(
-      Context servletContext, String servletName, Class<? extends Filter> filter) {
+      Context servletContext, String servletName, Class<? extends Filter> filter) throws Exception {
     String name = UUID.randomUUID().toString();
     FilterDef filterDef = new FilterDef();
-    try {
-      filterDef.setFilter(filter.newInstance());
-    } catch (Exception e) {
-      throw new RuntimeException(e);
-    }
+    filterDef.setFilter(filter.getConstructor().newInstance());
     filterDef.setFilterName(name);
     servletContext.addFilterDef(filterDef);
     FilterMap filterMap = new FilterMap();

diff --git a/...gent/instrumentation/servlet/v3_0/mapping/TomcatServlet3FilterServletNameMappingTest.java b/...gent/instrumentation/servlet/v3_0/mapping/TomcatServlet3FilterServletNameMappingTest.java
@@ -10,7 +10,7 @@
 
 class TomcatServlet3FilterServletNameMappingTest extends TomcatServlet3FilterMappingTest {
   @Override
-  protected void setupServlets(Context context) {
+  protected void setupServlets(Context context) throws Exception {
     Tomcat.addServlet(context, "prefix-servlet", new DefaultServlet());
     context.addServletMappingDecoded("/prefix/*", "prefix-servlet");
     Tomcat.addServlet(context, "suffix-servlet", new DefaultServlet());

diff --git a/...agent/instrumentation/servlet/v3_0/mapping/TomcatServlet3FilterUrlPatternMappingTest.java b/...agent/instrumentation/servlet/v3_0/mapping/TomcatServlet3FilterUrlPatternMappingTest.java
@@ -9,7 +9,7 @@
 
 class TomcatServlet3FilterUrlPatternMappingTest extends TomcatServlet3FilterMappingTest {
   @Override
-  protected void setupServlets(Context context) {
+  protected void setupServlets(Context context) throws Exception {
     addFilter(context, "/*", FirstFilter.class);
     addFilter(context, "/prefix/*", TestFilter.class);
     addFilter(context, "*.suffix", TestFilter.class);

diff --git a/...entelemetry/javaagent/instrumentation/servlet/v3_0/mapping/TomcatServlet3MappingTest.java b/...entelemetry/javaagent/instrumentation/servlet/v3_0/mapping/TomcatServlet3MappingTest.java
@@ -6,7 +6,6 @@
 package io.opentelemetry.javaagent.instrumentation.servlet.v3_0.mapping;
 
 import java.io.File;
-import java.io.IOException;
 import java.nio.file.Files;
 import java.util.UUID;
 import javax.servlet.Servlet;
@@ -19,12 +18,7 @@ class TomcatServlet3MappingTest extends AbstractServlet3MappingTest<Tomcat, Cont
   protected Tomcat setupServer() throws Exception {
     Tomcat tomcatServer = new Tomcat();
 
-    File baseDir;
-    try {
-      baseDir = Files.createTempDirectory("tomcat").toFile();
-    } catch (IOException e) {
-      throw new RuntimeException(e);
-    }
+    File baseDir = Files.createTempDirectory("tomcat").toFile();
     baseDir.deleteOnExit();
     tomcatServer.setBaseDir(baseDir.getAbsolutePath());
 
@@ -44,33 +38,21 @@ protected Tomcat setupServer() throws Exception {
 
     setupServlets(servletContext);
 
-    try {
-      tomcatServer.start();
-    } catch (LifecycleException e) {
-      throw new RuntimeException(e);
-    }
+    tomcatServer.start();
     return tomcatServer;
   }
 
   @Override
-  public void stopServer(Tomcat server) {
-    try {
-      server.stop();
-      server.destroy();
-    } catch (LifecycleException e) {
-      throw new RuntimeException(e);
-    }
+  public void stopServer(Tomcat server) throws LifecycleException {
+    server.stop();
+    server.destroy();
   }
 
-  @SuppressWarnings("ClassNewInstance")
   @Override
-  public void addServlet(Context context, String path, Class<? extends Servlet> servlet) {
+  public void addServlet(Context context, String path, Class<? extends Servlet> servlet)
+      throws Exception {
     String name = UUID.randomUUID().toString();
-    try {
-      Tomcat.addServlet(context, name, servlet.newInstance());
-    } catch (Exception e) {
-      throw new RuntimeException(e);
-    }
+    Tomcat.addServlet(context, name, servlet.getConstructor().newInstance());
     context.addServletMappingDecoded(path, name);
   }
 

diff --git a/...va/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/tomcat/TestAccessLogValve.java b/...va/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/tomcat/TestAccessLogValve.java
@@ -18,6 +18,13 @@
 import org.apache.catalina.valves.ValveBase;
 
 class TestAccessLogValve extends ValveBase implements AccessLog {
+
+  public final List<Map.Entry<String, String>> getLoggedIds() {
+    return loggedIds;
+  }
+
+  private final List<Map.Entry<String, String>> loggedIds = new ArrayList<>();
+
   public TestAccessLogValve() {
     super(true);
   }
@@ -71,10 +78,4 @@ public boolean getRequestAttributesEnabled() {
   public void invoke(Request request, Response response) throws IOException, ServletException {
     getNext().invoke(request, response);
   }
-
-  public final List<Map.Entry<String, String>> getLoggedIds() {
-    return loggedIds;
-  }
-
-  private final List<Map.Entry<String, String>> loggedIds = new ArrayList<>();
 }
diff --git a/...est/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/tomcat/TestServlet3.java b/...est/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/tomcat/TestServlet3.java
@@ -296,13 +296,9 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) {
   @WebServlet(asyncSupported = true)
   public static class DispatchRecursive extends HttpServlet {
     @Override
-    protected void service(HttpServletRequest req, HttpServletResponse resp) {
+    protected void service(HttpServletRequest req, HttpServletResponse resp) throws IOException {
       if (req.getServletPath().equals("/recursive")) {
-        try {
-          resp.getWriter().print("Hello Recursive");
-        } catch (IOException e) {
-          throw new RuntimeException(e);
-        }
+        resp.getWriter().print("Hello Recursive");
       }
 
       int depth = Integer.parseInt(req.getParameter("depth"));

diff --git a/...va/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/tomcat/TomcatServlet3Test.java b/...va/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/tomcat/TomcatServlet3Test.java
@@ -110,7 +110,6 @@ protected Tomcat setupServer() throws Exception {
     // Speed up startup by disabling jar scanning:
     servletContext.getJarScanner().setJarScanFilter((jarScanType, jarName) -> false);
 
-    //    setupAuthentication(tomcatServer, servletContext)
     setupServlets(servletContext);
 
     ((StandardHost) tomcatServer.getHost())
@@ -129,24 +128,16 @@ void setUp() {
   }
 
   @Override
-  public void stopServer(Tomcat server) {
-    try {
-      server.stop();
-      server.destroy();
-    } catch (LifecycleException e) {
-      throw new RuntimeException(e);
-    }
+  public void stopServer(Tomcat server) throws LifecycleException {
+    server.stop();
+    server.destroy();
   }
 
-  @SuppressWarnings("ClassNewInstance")
   @Override
-  public void addServlet(Context servletContext, String path, Class<? extends Servlet> servlet) {
+  public void addServlet(Context servletContext, String path, Class<? extends Servlet> servlet)
+      throws Exception {
     String name = UUID.randomUUID().toString();
-    try {
-      Tomcat.addServlet(servletContext, name, servlet.newInstance());
-    } catch (Exception e) {
-      throw new RuntimeException(e);
-    }
+    Tomcat.addServlet(servletContext, name, servlet.getConstructor().newInstance());
     servletContext.addServletMappingDecoded(path, name);
   }
 

diff --git a/...vaagent/instrumentation/servlet/v3_0/tomcat/dispatch/TomcatServlet3DispatchAsyncTest.java b/...vaagent/instrumentation/servlet/v3_0/tomcat/dispatch/TomcatServlet3DispatchAsyncTest.java
@@ -41,7 +41,7 @@ public Class<? extends Servlet> servlet() {
   }
 
   @Override
-  protected void setupServlets(Context context) {
+  protected void setupServlets(Context context) throws Exception {
     super.setupServlets(context);
 
     addServlet(context, "/dispatch" + SUCCESS.getPath(), TestServlet3.DispatchAsync.class);

diff --git a/...ent/instrumentation/servlet/v3_0/tomcat/dispatch/TomcatServlet3DispatchImmediateTest.java b/...ent/instrumentation/servlet/v3_0/tomcat/dispatch/TomcatServlet3DispatchImmediateTest.java
@@ -41,7 +41,7 @@ protected void configure(HttpServerTestOptions options) {
   }
 
   @Override
-  protected void setupServlets(Context context) {
+  protected void setupServlets(Context context) throws Exception {
     super.setupServlets(context);
 
     addServlet(context, "/dispatch" + SUCCESS.getPath(), TestServlet3.DispatchImmediate.class);

diff --git a/...try/javaagent/instrumentation/servlet/v3_0/tomcat/dispatch/TomcatServlet3ForwardTest.java b/...try/javaagent/instrumentation/servlet/v3_0/tomcat/dispatch/TomcatServlet3ForwardTest.java
@@ -42,7 +42,7 @@ protected void configure(HttpServerTestOptions options) {
   }
 
   @Override
-  protected void setupServlets(Context context) {
+  protected void setupServlets(Context context) throws Exception {
     super.setupServlets(context);
 
     addServlet(context, "/dispatch" + SUCCESS.getPath(), RequestDispatcherServlet.Forward.class);

diff --git a/...try/javaagent/instrumentation/servlet/v3_0/tomcat/dispatch/TomcatServlet3IncludeTest.java b/...try/javaagent/instrumentation/servlet/v3_0/tomcat/dispatch/TomcatServlet3IncludeTest.java
@@ -44,7 +44,7 @@ protected void configure(HttpServerTestOptions options) {
   }
 
   @Override
-  protected void setupServlets(Context context) {
+  protected void setupServlets(Context context) throws Exception {
     super.setupServlets(context);
 
     addServlet(context, "/dispatch" + SUCCESS.getPath(), RequestDispatcherServlet.Include.class);

diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TomcatServlet5Test.groovy b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/TomcatServlet5Test.groovy
@@ -96,7 +96,6 @@ abstract class TomcatServlet5Test extends AbstractServlet5Test<Tomcat, Context>
       }
     })
 
-//    setupAuthentication(tomcatServer, servletContext)
     setupServlets(servletContext)
 
     (tomcatServer.host as StandardHost).errorReportValveClass = ErrorHandlerValve.name