From 5341441b3a943608637d869a7407711541c1eb56 Mon Sep 17 00:00:00 2001 From: mateuszrzeszutek Date: Thu, 3 Sep 2020 16:06:55 +0200 Subject: [PATCH] Fix Servlet instrumentation for servlet-api 3.1+ - remove output stream wrapper --- .../src/test/groovy/DropwizardTest.groovy | 2 - .../test/groovy/JaxRsHttpServerTest.groovy | 10 +- .../jetty/JettyHandlerInstrumentation.java | 4 - .../JSPInstrumentationBasicTests.groovy | 8 - .../JSPInstrumentationForwardTests.groovy | 6 - .../test/groovy/GlassFishServerTest.groovy | 6 - .../servlet/servlet-3.0/servlet-3.0.gradle | 5 +- .../v3_0/AsyncContextInstrumentation.java | 5 - .../v3_0/CountingHttpServletRequest.java | 41 ----- .../v3_0/CountingHttpServletResponse.java | 145 ------------------ .../auto/servlet/v3_0/Servlet3Advice.java | 6 - .../v3_0/Servlet3HttpServerTracer.java | 10 -- .../servlet/v3_0/Servlet3Instrumentation.java | 4 - .../test/groovy/AbstractServlet3Test.groovy | 2 - .../test/boot/SpringBootBasedTest.groovy | 4 +- .../test/filter/ServletFilterTest.groovy | 2 - 16 files changed, 5 insertions(+), 255 deletions(-) delete mode 100644 instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/CountingHttpServletRequest.java delete mode 100644 instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/CountingHttpServletResponse.java diff --git a/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy b/instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy index 73af57fd9e5d..4f434328c617 100644 --- 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 { "${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 index 74d650e2fcbc..4fd78ff685a7 100644 --- 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 extends HttpServerTest { 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 extends HttpServerTest { 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 extends HttpServerTest { String traceID, String parentID, String method, - Long responseContentLength, String path, URI fullUrl, boolean isError, @@ -139,11 +138,6 @@ abstract class JaxRsHttpServerTest extends HttpServerTest { "${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/instrumentation/jetty-8.0/src/main/java/io/opentelemetry/instrumentation/auto/jetty/JettyHandlerInstrumentation.java b/instrumentation/jetty-8.0/src/main/java/io/opentelemetry/instrumentation/auto/jetty/JettyHandlerInstrumentation.java index b04e304465b4..fe117a60ab1d 100644 --- a/instrumentation/jetty-8.0/src/main/java/io/opentelemetry/instrumentation/auto/jetty/JettyHandlerInstrumentation.java +++ b/instrumentation/jetty-8.0/src/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 index af810a130678..c7c25498cd15 100644 --- 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 index d89786b43706..c55193b8f121 100644 --- 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 index c0afcb1d1334..4f2b61583262 100644 --- 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 { "${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 index 0efbf209d707..57c33caf37d9 100644 --- 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/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/AsyncContextInstrumentation.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/AsyncContextInstrumentation.java index e95e2cbd5dc3..919042c33078 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/AsyncContextInstrumentation.java +++ b/instrumentation/servlet/servlet-3.0/src/main/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/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/CountingHttpServletRequest.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/CountingHttpServletRequest.java deleted file mode 100644 index d4cc4fdf4f4c..000000000000 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/CountingHttpServletRequest.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.opentelemetry.instrumentation.auto.servlet.v3_0; - -import javax.servlet.AsyncContext; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletRequestWrapper; -import javax.servlet.http.HttpServletResponse; - -/** - * TODO(anuraaga): Implement counting, for now it just ensures startAsync is called with the wrapped - * objects to allow counting the response. - */ -public class CountingHttpServletRequest extends HttpServletRequestWrapper { - - private final HttpServletResponse response; - - public CountingHttpServletRequest(HttpServletRequest request, HttpServletResponse response) { - super(request); - this.response = response; - } - - @Override - public AsyncContext startAsync() throws IllegalStateException { - return startAsync(this, response); - } -} diff --git a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/CountingHttpServletResponse.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/CountingHttpServletResponse.java deleted file mode 100644 index 90b8e8b27ea2..000000000000 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/CountingHttpServletResponse.java +++ /dev/null @@ -1,145 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.opentelemetry.instrumentation.auto.servlet.v3_0; - -import java.io.IOException; -import java.io.PrintWriter; -import java.io.Writer; -import javax.servlet.ServletOutputStream; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpServletResponseWrapper; - -/** HttpServletResponseWrapper since servlet 2.3, not applicable to 2.2 */ -public class CountingHttpServletResponse extends HttpServletResponseWrapper { - private CountingServletOutputStream outputStream = null; - private CountingPrintWriter printWriter = null; - private int errorLength = 0; - - /** - * Constructs a response adaptor wrapping the given response. - * - * @throws IllegalArgumentException if the response is null - */ - public CountingHttpServletResponse(HttpServletResponse response) { - super(response); - } - - @Override - public ServletOutputStream getOutputStream() throws IOException { - if (outputStream == null) { - outputStream = new CountingServletOutputStream(super.getOutputStream()); - } - return outputStream; - } - - @Override - public PrintWriter getWriter() throws IOException { - if (printWriter == null) { - printWriter = new CountingPrintWriter(super.getWriter()); - } - return printWriter; - } - - public int getContentLength() { - int contentLength = errorLength; - if (outputStream != null) { - contentLength += outputStream.counter; - } - if (printWriter != null) { - contentLength += printWriter.counter; - } - return contentLength; - } - - /** sendError bypasses the servlet response writers and writes directly to the response */ - @Override - public void sendError(int sc, String msg) throws IOException { - super.sendError(sc, msg); - if (msg != null) { - errorLength += msg.length(); - } - } - - static class CountingServletOutputStream extends ServletOutputStream { - - private final ServletOutputStream delegate; - private int counter = 0; - - public CountingServletOutputStream(ServletOutputStream delegate) { - this.delegate = delegate; - } - - @Override - public void write(int b) throws IOException { - delegate.write(b); - counter++; - } - - @Override - public void write(byte[] b) throws IOException { - delegate.write(b); - counter += b.length; - } - - @Override - public void write(byte[] b, int off, int len) throws IOException { - delegate.write(b, off, len); - counter += len; - } - - @Override - public void flush() throws IOException { - delegate.flush(); - } - - @Override - public void close() throws IOException { - delegate.close(); - } - } - - static class CountingPrintWriter extends PrintWriter { - - private int counter = 0; - - /** - * write(String s) and write(char[] buf) are not overridden because they delegate to another - * write function which would result in their write being counted twice. - */ - public CountingPrintWriter(Writer out) { - super(out); - } - - @Override - public void write(int c) { - super.write(c); - counter++; - } - - @Override - public void write(char[] buf, int off, int len) { - super.write(buf, off, len); - counter += len; - } - - @Override - public void write(String s, int off, int len) { - super.write(s, off, len); - counter += len; - } - } -} diff --git a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3Advice.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3Advice.java index fda6122196f8..a61058c3f77a 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3Advice.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3Advice.java @@ -56,12 +56,6 @@ public static void onEnter( span = TRACER.startSpan(httpServletRequest, httpServletRequest, method); scope = TRACER.startScope(span, httpServletRequest); - if (!(response instanceof CountingHttpServletResponse)) { - response = new CountingHttpServletResponse((HttpServletResponse) response); - request = - new CountingHttpServletRequest( - (HttpServletRequest) request, (HttpServletResponse) response); - } } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) diff --git a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3HttpServerTracer.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3HttpServerTracer.java index e97c2006bede..7715c104b267 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3HttpServerTracer.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3HttpServerTracer.java @@ -22,7 +22,6 @@ import io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Status; -import io.opentelemetry.trace.attributes.SemanticAttributes; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -48,7 +47,6 @@ protected int responseStatus(HttpServletResponse httpServletResponse) { @Override public void endExceptionally( Span span, Throwable throwable, HttpServletResponse response, long timestamp) { - captureContentLength(span, response); if (response.isCommitted()) { super.endExceptionally(span, throwable, response, timestamp); } else { @@ -62,7 +60,6 @@ public void endExceptionally( @Override public void end(Span span, HttpServletResponse response, long timestamp) { - captureContentLength(span, response); super.end(span, response, timestamp); } @@ -72,13 +69,6 @@ public void onTimeout(Span span, long timeout) { span.end(); } - private static void captureContentLength(Span span, HttpServletResponse response) { - if (response instanceof CountingHttpServletResponse) { - SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.set( - span, ((CountingHttpServletResponse) response).getContentLength()); - } - } - /* Given request already has a context associated with it. As there should not be nested spans of kind SERVER, we should NOT create a new span here. diff --git a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3Instrumentation.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3Instrumentation.java index 067609c9088f..a28ee4662c00 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3Instrumentation.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3Instrumentation.java @@ -54,10 +54,6 @@ public String[] helperClassNames() { return new String[] { "io.opentelemetry.instrumentation.servlet.HttpServletRequestGetter", "io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer", - packageName + ".CountingHttpServletRequest", - packageName + ".CountingHttpServletResponse", - packageName + ".CountingHttpServletResponse$CountingServletOutputStream", - packageName + ".CountingHttpServletResponse$CountingPrintWriter", packageName + ".Servlet3Advice", packageName + ".Servlet3HttpServerTracer", packageName + ".TagSettingAsyncListener" diff --git a/instrumentation/servlet/servlet-3.0/src/test/groovy/AbstractServlet3Test.groovy b/instrumentation/servlet/servlet-3.0/src/test/groovy/AbstractServlet3Test.groovy index aed248c0df92..8920c454c724 100644 --- a/instrumentation/servlet/servlet-3.0/src/test/groovy/AbstractServlet3Test.groovy +++ b/instrumentation/servlet/servlet-3.0/src/test/groovy/AbstractServlet3Test.groovy @@ -93,8 +93,6 @@ abstract class AbstractServlet3Test extends HttpServerTest "${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()}" {responseContentLength == null || it == responseContentLength} "servlet.path" endpoint.path "servlet.context" "" if (endpoint.query) { diff --git a/instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/filter/ServletFilterTest.groovy b/instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/filter/ServletFilterTest.groovy index 76df626a2692..a077d2f75505 100644 --- a/instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/filter/ServletFilterTest.groovy +++ b/instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/filter/ServletFilterTest.groovy @@ -112,8 +112,6 @@ class ServletFilterTest extends HttpServerTest { "${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 || endpoint == EXCEPTION || endpoint == ERROR } "servlet.path" endpoint.path "servlet.context" "" if (endpoint.query) {