Skip to content

Commit

Permalink
Fix Servlet instrumentation for servlet-api 3.1+ - remove output stre…
Browse files Browse the repository at this point in the history 
…am wrapper
  • Loading branch information
mateuszrzeszutek committed Sep 4, 2020
1 parent 1217882 commit 9386e20
Show file tree
Hide file tree
Showing 8 changed files with 6 additions and 70 deletions.
2 changes: 1 addition & 1 deletion instrumentation/dropwizard-testing/src/test/groovy/DropwizardTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ class DropwizardTest extends HttpServerTest<DropwizardTestSupport> {
"${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 }
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" { it == 0 || /* async */it == null }
"servlet.context" ""
"servlet.path" ""
if (endpoint.query) {
Expand Down
11 changes: 3 additions & 8 deletions instrumentation/jaxrs/jaxrs-2.0/src/test/groovy/JaxRsHttpServerTest.groovy
View file
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,7 @@ 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
}
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" Long
"servlet.path" String
"servlet.context" String
if (query) {
Expand Down
2 changes: 0 additions & 2 deletions ...rc/main/java/io/opentelemetry/instrumentation/auto/jetty/JettyHandlerInstrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,7 @@ 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
2 changes: 1 addition & 1 deletion instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class GlassFishServerTest extends HttpServerTest<GlassFish> {
"${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 }
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" { it == 0 || 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: 0 additions & 5 deletions .../java/io/opentelemetry/instrumentation/auto/servlet/v3_0/AsyncContextInstrumentation.java
View file
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
51 changes: 0 additions & 51 deletions .../java/io/opentelemetry/instrumentation/auto/servlet/v3_0/CountingHttpServletResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,11 @@
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;

Expand All @@ -38,14 +36,6 @@ 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) {
Expand All @@ -56,9 +46,6 @@ public PrintWriter getWriter() throws IOException {

public int getContentLength() {
int contentLength = errorLength;
if (outputStream != null) {
contentLength += outputStream.counter;
}
if (printWriter != null) {
contentLength += printWriter.counter;
}
Expand All @@ -74,44 +61,6 @@ public void sendError(int sc, String msg) throws IOException {
}
}

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;
Expand Down
1 change: 0 additions & 1 deletion ...main/java/io/opentelemetry/instrumentation/auto/servlet/v3_0/Servlet3Instrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ public String[] helperClassNames() {
"io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer",
packageName + ".CountingHttpServletRequest",
packageName + ".CountingHttpServletResponse",
packageName + ".CountingHttpServletResponse$CountingServletOutputStream",
packageName + ".CountingHttpServletResponse$CountingPrintWriter",
packageName + ".Servlet3Advice",
packageName + ".Servlet3HttpServerTracer",
Expand Down
2 changes: 1 addition & 1 deletion ...ng-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/boot/SpringBootBasedTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
"${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
"${SemanticAttributes.HTTP_RESPONSE_CONTENT_LENGTH.key()}" 0
"servlet.path" endpoint.path
"servlet.context" ""
if (endpoint.query) {
Expand Down

0 comments on commit 9386e20

Please sign in to comment.