Fix Servlet instrumentation for servlet-api 3.1+ - remove output stre…

…am wrapper (#1167)
open-telemetry · Sep 9, 2020 · 79b2ce3 · 79b2ce3
1 parent c1556fa
commit 79b2ce3
Show file tree

Hide file tree

Showing 16 changed files with 5 additions and 255 deletions.
diff --git a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy
@@ -121,8 +121,6 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> {
         "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
         "${SemanticAttributes.HTTP_USER_AGENT.key()}" TEST_USER_AGENT
         "${SemanticAttributes.HTTP_CLIENT_IP.key()}" TEST_CLIENT_IP
-        // exception bodies are not yet recorded
-        "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" { it == responseContentLength || /* async */it == null }
         "servlet.context" ""
         "servlet.path" ""
         if (endpoint.query) {

diff --git a/instrumentation/jaxrs/jaxrs-2.0/src/test/groovy/JaxRsHttpServerTest.groovy b/instrumentation/jaxrs/jaxrs-2.0/src/test/groovy/JaxRsHttpServerTest.groovy
@@ -88,7 +88,7 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> {
                   String method = "GET",
                   Long responseContentLength = null,
                   ServerEndpoint endpoint = SUCCESS) {
-    serverSpan(trace, index, traceID, parentID, method, responseContentLength,
+    serverSpan(trace, index, traceID, parentID, method,
       endpoint == PATH_PARAM ? "/path/{id}/param" : endpoint.resolvePath(address).path,
       endpoint.resolve(address),
       endpoint.errored,
@@ -101,7 +101,7 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> {
                        HttpUrl url,
                        int statusCode) {
     def rawUrl = url.url()
-    serverSpan(trace, index, null, null, "GET", null,
+    serverSpan(trace, index, null, null, "GET",
       rawUrl.path,
       rawUrl.toURI(),
       statusCode >= 500,
@@ -114,7 +114,6 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> {
                   String traceID,
                   String parentID,
                   String method,
-                  Long responseContentLength,
                   String path,
                   URI fullUrl,
                   boolean isError,
@@ -139,11 +138,6 @@ abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> {
         "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
         "${SemanticAttributes.HTTP_USER_AGENT.key()}" TEST_USER_AGENT
         "${SemanticAttributes.HTTP_CLIENT_IP.key()}" TEST_CLIENT_IP
-        if (responseContentLength) {
-          "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" responseContentLength
-        } else {
-          "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
-        }
         "servlet.path" String
         "servlet.context" String
         if (query) {

diff --git a/...rc/main/java/io/opentelemetry/instrumentation/auto/jetty/JettyHandlerInstrumentation.java b/...rc/main/java/io/opentelemetry/instrumentation/auto/jetty/JettyHandlerInstrumentation.java
@@ -59,10 +59,6 @@ public String[] helperClassNames() {
       "io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer",
       "io.opentelemetry.instrumentation.auto.servlet.v3_0.Servlet3HttpServerTracer",
       "io.opentelemetry.instrumentation.auto.servlet.v3_0.TagSettingAsyncListener",
-      "io.opentelemetry.instrumentation.auto.servlet.v3_0.CountingHttpServletRequest",
-      "io.opentelemetry.instrumentation.auto.servlet.v3_0.CountingHttpServletResponse",
-      "io.opentelemetry.instrumentation.auto.servlet.v3_0.CountingHttpServletResponse$CountingServletOutputStream",
-      "io.opentelemetry.instrumentation.auto.servlet.v3_0.CountingHttpServletResponse$CountingPrintWriter",
       packageName + ".JettyHttpServerTracer",
     };
   }

diff --git a/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy b/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy
@@ -114,7 +114,6 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/$jspFileName"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
@@ -178,7 +177,6 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/getQuery.jsp?$queryString"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
@@ -239,7 +237,6 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/post.jsp"
             "${SemanticAttributes.HTTP_METHOD.key()}" "POST"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
@@ -309,7 +306,6 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/$jspFileName"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 500
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
@@ -384,7 +380,6 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/includes/includeHtml.jsp"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
@@ -441,7 +436,6 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/includes/includeMulti.jsp"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
@@ -537,7 +531,6 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/$jspFileName"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 500
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
@@ -595,7 +588,6 @@ class JSPInstrumentationBasicTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/$staticFile"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"

diff --git a/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationForwardTests.groovy b/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationForwardTests.groovy
@@ -112,7 +112,6 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/$forwardFromFileName"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
@@ -194,7 +193,6 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/forwards/forwardToHtml.jsp"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
@@ -251,7 +249,6 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/forwards/forwardToIncludeMulti.jsp"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
@@ -368,7 +365,6 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/forwards/forwardToJspForward.jsp"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 200
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
@@ -466,7 +462,6 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/forwards/forwardToCompileError.jsp"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 500
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"
@@ -535,7 +530,6 @@ class JSPInstrumentationForwardTests extends AgentTestRunner {
             "${SemanticAttributes.HTTP_URL.key()}" "http://localhost:$port/$jspWebappContext/forwards/forwardToNonExistent.jsp"
             "${SemanticAttributes.HTTP_METHOD.key()}" "GET"
             "${SemanticAttributes.HTTP_STATUS_CODE.key()}" 404
-            "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
             "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
             "${SemanticAttributes.HTTP_USER_AGENT.key()}" String
             "${SemanticAttributes.HTTP_CLIENT_IP.key()}" "127.0.0.1"

diff --git a/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy b/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy
@@ -14,10 +14,7 @@
  * limitations under the License.
  */
 
-import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
-import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
-import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.REDIRECT
 import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.SUCCESS
 import static io.opentelemetry.trace.Span.Kind.SERVER
 
@@ -108,9 +105,6 @@ class GlassFishServerTest extends HttpServerTest<GlassFish> {
         "${SemanticAttributes.HTTP_FLAVOR.key()}" "HTTP/1.1"
         "${SemanticAttributes.HTTP_USER_AGENT.key()}" TEST_USER_AGENT
         "${SemanticAttributes.HTTP_CLIENT_IP.key()}" TEST_CLIENT_IP
-        // exception bodies are not yet recorded
-        // TODO(anuraaga): Bodies do seem to be recorded for these endpoints here, update to make assertion more precise.
-        "${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" { it == responseContentLength || endpoint == EXCEPTION || endpoint == ERROR || endpoint == NOT_FOUND || endpoint == REDIRECT }
         "servlet.context" "/$context"
         "servlet.path" endpoint.path
         if (endpoint.query) {

diff --git a/instrumentation/servlet/servlet-3.0/servlet-3.0.gradle b/instrumentation/servlet/servlet-3.0/servlet-3.0.gradle
@@ -31,7 +31,6 @@ dependencies {
   latestDepTestLibrary group: 'org.eclipse.jetty', name: 'jetty-server', version: '9.+'
   latestDepTestLibrary group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '9.+'
 
-  // FIXME: 9.0.24 seems to have changed something...
-  latestDepTestLibrary group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '9.0.22'
-  latestDepTestLibrary group: 'org.apache.tomcat.embed', name: 'tomcat-embed-jasper', version: '9.0.22'
+  latestDepTestLibrary group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '9.+'
+  latestDepTestLibrary group: 'org.apache.tomcat.embed', name: 'tomcat-embed-jasper', version: '9.+'
 }
diff --git a/.../java/io/opentelemetry/instrumentation/auto/servlet/v3_0/AsyncContextInstrumentation.java b/.../java/io/opentelemetry/instrumentation/auto/servlet/v3_0/AsyncContextInstrumentation.java
@@ -50,11 +50,6 @@ public String[] helperClassNames() {
     return new String[] {
       "io.opentelemetry.instrumentation.servlet.HttpServletRequestGetter",
       "io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer",
-      packageName + ".CountingHttpServletResponse",
-      packageName + ".CountingHttpServletResponse$CountingServletOutputStream",
-      packageName + ".CountingHttpServletResponse$CountingPrintWriter",
-      packageName + ".TagSettingAsyncListener",
-      packageName + ".Servlet3HttpServerTracer"
     };
   }
 

diff --git a/...n/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/CountingHttpServletRequest.java b/...n/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/CountingHttpServletRequest.java