Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Servlet instrumentation for servlet-api 3.1+ - remove output stream wrapper #1167

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 2 additions & 3 deletions instrumentation/servlet/servlet-3.0/servlet-3.0.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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.+'
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
};
}

Expand Down

This file was deleted.

Loading