From 1ca727659302402c80975b2dda8ff461de8a9b04 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 19 Feb 2021 15:57:11 +0200 Subject: [PATCH] Remove HttpServerTest.testExceptionBody and redirectHasBody (#2346) --- .../AkkaHttpServerInstrumentationTest.groovy | 5 --- .../src/test/groovy/DropwizardTest.groovy | 4 --- .../src/test/groovy/GrizzlyTest.groovy | 5 --- .../src/test/groovy/JettyHandlerTest.groovy | 5 --- .../test/groovy/server/PlayServerTest.groovy | 5 --- .../test/groovy/server/PlayServerTest.groovy | 5 --- .../test/groovy/GlassFishServerTest.groovy | 5 --- .../src/test/groovy/TomcatServlet3Test.groovy | 5 --- .../test/filter/ServletFilterTest.groovy | 5 --- .../test/groovy/Struts2ActionSpanTest.groovy | 5 --- .../instrumentation/tomcat7/TestServlet.java | 3 -- .../tomcat7/TomcatHandlerTest.groovy | 33 ------------------- .../src/test/groovy/UndertowServerTest.groovy | 5 --- .../server/VertxRxHttpServerTest.groovy | 5 --- .../groovy/server/VertxHttpServerTest.groovy | 5 --- .../test/base/HttpServerTest.groovy | 12 ------- 16 files changed, 112 deletions(-) diff --git a/instrumentation/akka-http-10.0/javaagent/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy b/instrumentation/akka-http-10.0/javaagent/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy index 88f410ee94..77dac38131 100644 --- a/instrumentation/akka-http-10.0/javaagent/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy +++ b/instrumentation/akka-http-10.0/javaagent/src/test/groovy/AkkaHttpServerInstrumentationTest.groovy @@ -8,11 +8,6 @@ import io.opentelemetry.instrumentation.test.base.HttpServerTest abstract class AkkaHttpServerInstrumentationTest extends HttpServerTest implements AgentTestTrait { - @Override - boolean testExceptionBody() { - false - } - // FIXME: This doesn't work because we don't support bindAndHandle. // @Override // def startServer(int port) { diff --git a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy index 549e852440..e74d212928 100644 --- a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy +++ b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy @@ -71,10 +71,6 @@ class DropwizardTest extends HttpServerTest implements Ag true } - boolean testExceptionBody() { - false - } - @Override void handlerSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { trace.span(index) { diff --git a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyTest.groovy b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyTest.groovy index 7f94cd6379..0c144e428a 100644 --- a/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyTest.groovy +++ b/instrumentation/grizzly-2.0/javaagent/src/test/groovy/GrizzlyTest.groovy @@ -106,9 +106,4 @@ class GrizzlyTest extends HttpServerTest implements AgentTestTrait { } } } - - @Override - boolean testExceptionBody() { - false - } } diff --git a/instrumentation/jetty-8.0/javaagent/src/test/groovy/JettyHandlerTest.groovy b/instrumentation/jetty-8.0/javaagent/src/test/groovy/JettyHandlerTest.groovy index e1588b799c..600d3061f1 100644 --- a/instrumentation/jetty-8.0/javaagent/src/test/groovy/JettyHandlerTest.groovy +++ b/instrumentation/jetty-8.0/javaagent/src/test/groovy/JettyHandlerTest.groovy @@ -59,11 +59,6 @@ class JettyHandlerTest extends HttpServerTest implements AgentTestTrait server.stop() } - @Override - boolean testExceptionBody() { - false - } - @Override boolean hasResponseSpan(ServerEndpoint endpoint) { endpoint == REDIRECT || endpoint == ERROR diff --git a/instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy b/instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy index 8a425a7aef..98faec9eca 100644 --- a/instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy +++ b/instrumentation/play/play-2.4/javaagent/src/test/groovy/server/PlayServerTest.groovy @@ -65,11 +65,6 @@ class PlayServerTest extends HttpServerTest implements AgentTestTrait { true } - boolean testExceptionBody() { - // I can't figure out how to set a proper exception handler to customize the response body. - false - } - @Override void handlerSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { trace.span(index) { diff --git a/instrumentation/play/play-2.6/javaagent/src/test/groovy/server/PlayServerTest.groovy b/instrumentation/play/play-2.6/javaagent/src/test/groovy/server/PlayServerTest.groovy index 80edfb5991..03034cabf1 100644 --- a/instrumentation/play/play-2.6/javaagent/src/test/groovy/server/PlayServerTest.groovy +++ b/instrumentation/play/play-2.6/javaagent/src/test/groovy/server/PlayServerTest.groovy @@ -67,11 +67,6 @@ class PlayServerTest extends HttpServerTest implements AgentTestTrait { true } - boolean testExceptionBody() { - // I can't figure out how to set a proper exception handler to customize the response body. - false - } - @Override void handlerSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { trace.span(index) { diff --git a/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy b/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy index fe6cc8141b..5f75864723 100644 --- a/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy +++ b/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy @@ -65,11 +65,6 @@ class GlassFishServerTest extends HttpServerTest implements AgentTest server.stop() } - @Override - boolean redirectHasBody() { - true - } - @Override boolean hasResponseSpan(ServerEndpoint endpoint) { endpoint == REDIRECT || endpoint == ERROR || endpoint == NOT_FOUND diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy index 2e1f4c7dad..602b7a8951 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/TomcatServlet3Test.groovy @@ -32,11 +32,6 @@ import spock.lang.Unroll @Unroll abstract class TomcatServlet3Test extends AbstractServlet3Test { - @Override - boolean testExceptionBody() { - return false - } - @Override Class expectedExceptionClass() { ServletException diff --git a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/filter/ServletFilterTest.groovy b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/filter/ServletFilterTest.groovy index df7186098c..60a3bc6107 100644 --- a/instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/filter/ServletFilterTest.groovy +++ b/instrumentation/spring/spring-webmvc-3.1/javaagent/src/test/groovy/test/filter/ServletFilterTest.groovy @@ -72,11 +72,6 @@ class ServletFilterTest extends HttpServerTest i true } - @Override - boolean testExceptionBody() { - false - } - @Override boolean testNotFound() { // FIXME: the instrumentation adds an extra controller span which is not consistent. diff --git a/instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy b/instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy index beaa62b19f..fcd7f19235 100644 --- a/instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy +++ b/instrumentation/struts-2.3/javaagent/src/test/groovy/Struts2ActionSpanTest.groovy @@ -33,11 +33,6 @@ class Struts2ActionSpanTest extends HttpServerTest implements AgentTestT return true } - @Override - boolean testExceptionBody() { - return false - } - @Override boolean testErrorBody() { return false diff --git a/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TestServlet.java b/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TestServlet.java index 1a4598bb54..472097079c 100644 --- a/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TestServlet.java +++ b/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TestServlet.java @@ -35,9 +35,6 @@ public class TestServlet extends HttpServlet { } return null; }); - } else if ("/errorPage".equals(path)) { - resp.getWriter().print(HttpServerTest.ServerEndpoint.EXCEPTION.getBody()); - resp.setStatus(500); } else { resp.getWriter().println("No cookie for you: " + path); resp.setStatus(400); diff --git a/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TomcatHandlerTest.groovy b/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TomcatHandlerTest.groovy index 91c314146a..107d675b19 100644 --- a/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TomcatHandlerTest.groovy +++ b/instrumentation/tomcat-7.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/tomcat7/TomcatHandlerTest.groovy @@ -5,19 +5,14 @@ package io.opentelemetry.javaagent.instrumentation.tomcat7 -import static io.opentelemetry.api.trace.SpanKind.INTERNAL 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.REDIRECT -import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS import io.opentelemetry.instrumentation.test.AgentTestTrait import io.opentelemetry.instrumentation.test.asserts.TraceAssert import io.opentelemetry.instrumentation.test.base.HttpServerTest -import io.opentelemetry.sdk.trace.data.SpanData import org.apache.catalina.Context import org.apache.catalina.startup.Tomcat -import org.apache.tomcat.util.descriptor.web.ErrorPage class TomcatHandlerTest extends HttpServerTest implements AgentTestTrait { @@ -42,12 +37,6 @@ class TomcatHandlerTest extends HttpServerTest implements AgentTestTrait Tomcat.addServlet(ctx, "testServlet", new TestServlet()) - def errorPage = new ErrorPage() - errorPage.setLocation("/errorPage") - errorPage.setErrorCode(500) - ctx.addErrorPage(errorPage) - ctx.addServletMappingDecoded("/errorPage", "testServlet") - // Mapping servlet to /* will result in all requests have a name of just a context. ServerEndpoint.values().each { ctx.addServletMappingDecoded(it.path, "testServlet") @@ -62,28 +51,6 @@ class TomcatHandlerTest extends HttpServerTest implements AgentTestTrait tomcat.getServer().stop() } - @Override - boolean testExceptionBody() { - false - } - - @Override - boolean hasErrorPageSpans(ServerEndpoint endpoint) { - endpoint == ERROR || endpoint == EXCEPTION - } - - @Override - void errorPageSpans(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { - trace.span(index) { - name "ApplicationDispatcher.forward" - kind INTERNAL - errored false - childOf((SpanData) parent) - attributes { - } - } - } - @Override boolean hasResponseSpan(ServerEndpoint endpoint) { endpoint == REDIRECT || endpoint == ERROR diff --git a/instrumentation/undertow/javaagent/src/test/groovy/UndertowServerTest.groovy b/instrumentation/undertow/javaagent/src/test/groovy/UndertowServerTest.groovy index 8f4cf43aa0..80550cc8aa 100644 --- a/instrumentation/undertow/javaagent/src/test/groovy/UndertowServerTest.groovy +++ b/instrumentation/undertow/javaagent/src/test/groovy/UndertowServerTest.groovy @@ -66,9 +66,4 @@ class UndertowServerTest extends HttpServerTest implements AgentTestTr String expectedServerSpanName(ServerEndpoint endpoint) { return "PathHandler.handleRequest" } - - @Override - boolean testExceptionBody() { - false - } } diff --git a/instrumentation/vertx-reactive-3.5/javaagent/src/test/groovy/server/VertxRxHttpServerTest.groovy b/instrumentation/vertx-reactive-3.5/javaagent/src/test/groovy/server/VertxRxHttpServerTest.groovy index 0826df91db..bb16589732 100644 --- a/instrumentation/vertx-reactive-3.5/javaagent/src/test/groovy/server/VertxRxHttpServerTest.groovy +++ b/instrumentation/vertx-reactive-3.5/javaagent/src/test/groovy/server/VertxRxHttpServerTest.groovy @@ -53,11 +53,6 @@ class VertxRxHttpServerTest extends HttpServerTest implements AgentTestTr server.close() } - @Override - boolean testExceptionBody() { - return false - } - @Override boolean testException() { // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 diff --git a/instrumentation/vertx-web-3.0/src/test/groovy/server/VertxHttpServerTest.groovy b/instrumentation/vertx-web-3.0/src/test/groovy/server/VertxHttpServerTest.groovy index c7188f2e91..eb7fd7b8e9 100644 --- a/instrumentation/vertx-web-3.0/src/test/groovy/server/VertxHttpServerTest.groovy +++ b/instrumentation/vertx-web-3.0/src/test/groovy/server/VertxHttpServerTest.groovy @@ -48,11 +48,6 @@ class VertxHttpServerTest extends HttpServerTest implements AgentTestTrai server.close() } - @Override - boolean testExceptionBody() { - return false - } - @Override boolean testException() { // TODO(anuraaga): https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807 diff --git a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy index 5d61c97965..0d9470d3e1 100644 --- a/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy +++ b/testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpServerTest.groovy @@ -67,10 +67,6 @@ abstract class HttpServerTest extends InstrumentationSpecification imple false } - boolean redirectHasBody() { - false - } - boolean testNotFound() { true } @@ -83,10 +79,6 @@ abstract class HttpServerTest extends InstrumentationSpecification imple true } - boolean testExceptionBody() { - true - } - boolean testException() { true } @@ -261,7 +253,6 @@ abstract class HttpServerTest extends InstrumentationSpecification imple response.code() == REDIRECT.status response.header("location") == REDIRECT.body || response.header("location") == "${address.resolve(REDIRECT.body)}" - response.body().contentLength() < 1 || redirectHasBody() and: assertTheTraces(1, null, null, method, REDIRECT, null, response) @@ -299,9 +290,6 @@ abstract class HttpServerTest extends InstrumentationSpecification imple expect: response.code() == EXCEPTION.status - if (testExceptionBody()) { - assert response.body().string() == EXCEPTION.body - } and: assertTheTraces(1, null, null, method, EXCEPTION, EXCEPTION.body, response)